Rework vorbis comment parsing

Collapse the two variations of `VorbisUtil.buildMetadata` into a single
method called `VorbisUtil.parseVorbisComments` that only takes a list
of vorbis strings, compared to previously where it would take strings
and picture frame instance. Any code that relied on the old signature
now either concatenates picture frames and vorbis comments or copies
vorbis comments into an existing metadata instance.

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/FlacStreamMetadata.java

@@ -143,7 +143,8 @@ public final class FlacStreamMetadata {
         bitsPerSample,
         totalSamples,
         /* seekTable= */ null,
-        VorbisUtil.buildMetadata(vorbisComments, pictureFrames));
+        new Metadata(pictureFrames)
+          .copyWithAppendedEntriesFrom(VorbisUtil.parseVorbisComments(vorbisComments)));
   }
 
   private FlacStreamMetadata(
@@ -268,8 +269,8 @@ public final class FlacStreamMetadata {
   public FlacStreamMetadata copyWithVorbisComments(List<String> vorbisComments) {
     @Nullable
     Metadata appendedMetadata =
-        getMetadataCopyWithAppendedEntriesFrom(
+        getMetadataCopyWithAppendedEntriesFrom(VorbisUtil.parseVorbisComments(vorbisComments));
-            VorbisUtil.buildMetadata(vorbisComments));
+
     return new FlacStreamMetadata(
         minBlockSizeSamples,
         maxBlockSizeSamples,
@@ -287,8 +288,8 @@ public final class FlacStreamMetadata {
   public FlacStreamMetadata copyWithPictureFrames(List<PictureFrame> pictureFrames) {
     @Nullable
     Metadata appendedMetadata =
-        getMetadataCopyWithAppendedEntriesFrom(
+        getMetadataCopyWithAppendedEntriesFrom(new Metadata(pictureFrames));
-            VorbisUtil.buildMetadata(Collections.emptyList(), pictureFrames));
+
     return new FlacStreamMetadata(
         minBlockSizeSamples,
         maxBlockSizeSamples,

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/VorbisUtil.java

@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.extractor;
 
 import android.util.Base64;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
 import com.google.android.exoplayer2.Format;
@@ -270,31 +271,10 @@ public final class VorbisUtil {
    * All others will be transformed into vorbis comments.
    *
    * @param vorbisComments The raw input of comments, as a key-value pair KEY=VAL.
-   * @return A Metadata instance containing the parsed metadata entries. Null if there are no
+   * @return The fully parsed Metadata instance. Null if no vorbis comments could be parsed.
-   * valid metadata entries that can be parsed.
    */
   @Nullable
-  public static Metadata buildMetadata(List<String> vorbisComments) {
+  public static Metadata parseVorbisComments(@NonNull List<String> vorbisComments) {
-    return buildMetadata(vorbisComments, Collections.emptyList());
-  }
-
-  /**
-   * Builds a metadata instance from Vorbis comments.
-   *
-   * METADATA_BLOCK_PICTURE comments will be transformed into picture frames.
-   * All others will be transformed into vorbis comments.
-   *
-   * @param vorbisComments The raw input of comments, as a key-value pair KEY=VAL.
-   * @param pictureFrames Any picture frames that were parsed beforehand.
-   * @return A Metadata instance containing the parsed metadata entries. Null if there are no
-   * valid metadata entries that can be parsed.
-   */
-  @Nullable
-  public static Metadata buildMetadata(List<String> vorbisComments, List<PictureFrame> pictureFrames) {
-    if (vorbisComments.isEmpty() && pictureFrames.isEmpty()) {
-      return null;
-    }
-
     ArrayList<Metadata.Entry> metadataEntries = new ArrayList<>();
     for (int i = 0; i < vorbisComments.size(); i++) {
       String vorbisComment = vorbisComments.get(i);
@@ -320,7 +300,6 @@ public final class VorbisUtil {
         metadataEntries.add(entry);
       }
     }
-    metadataEntries.addAll(pictureFrames);
 
     return metadataEntries.isEmpty() ? null : new Metadata(metadataEntries);
   }

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OpusReader.java

@@ -95,9 +95,8 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
       VorbisUtil.CommentHeader commentHeader = VorbisUtil.readVorbisCommentHeader(
           packet, false, false);
 
-      @Nullable Metadata vorbisMetadata = VorbisUtil.buildMetadata(
+      @Nullable Metadata vorbisMetadata = VorbisUtil.parseVorbisComments(
-          Arrays.asList(commentHeader.comments)
+          Arrays.asList(commentHeader.comments));
-      );
 
       setupData.format = setupData.format
           .buildUpon()

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/VorbisReader.java

@@ -25,12 +25,12 @@ import com.google.android.exoplayer2.ParserException;
 import com.google.android.exoplayer2.extractor.VorbisUtil;
 import com.google.android.exoplayer2.extractor.VorbisUtil.Mode;
 import com.google.android.exoplayer2.metadata.Metadata;
-import com.google.android.exoplayer2.util.Log;
 import com.google.android.exoplayer2.util.MimeTypes;
 import com.google.android.exoplayer2.util.ParsableByteArray;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
 
 /** {@link StreamReader} to extract Vorbis data out of Ogg byte stream. */
@@ -41,7 +41,7 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
   private boolean seenFirstAudioPacket;
 
   @Nullable private VorbisUtil.VorbisIdHeader vorbisIdHeader;
-  @Nullable private VorbisUtil.CommentHeader commentHeader;
+  @Nullable private VorbisUtil.CommentHeader vorbisCommentHeader;
 
   public static boolean verifyBitstreamType(ParsableByteArray data) {
     try {
@@ -57,7 +57,7 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
     if (headerData) {
       vorbisSetup = null;
       vorbisIdHeader = null;
-      commentHeader = null;
+      vorbisCommentHeader = null;
     }
     previousPacketBlockSize = 0;
     seenFirstAudioPacket = false;
@@ -108,13 +108,14 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
     VorbisSetup vorbisSetup = this.vorbisSetup;
 
     VorbisUtil.VorbisIdHeader idHeader = vorbisSetup.idHeader;
+    VorbisUtil.CommentHeader commentHeader = vorbisSetup.commentHeader;
 
     ArrayList<byte[]> codecInitializationData = new ArrayList<>();
     codecInitializationData.add(idHeader.data);
     codecInitializationData.add(vorbisSetup.setupHeaderData);
 
-    @Nullable Metadata vorbisMetadata = VorbisUtil.buildMetadata(
+    @Nullable Metadata vorbisMetadata = VorbisUtil.parseVorbisComments(
-        Arrays.asList(vorbisSetup.commentHeader.comments));
+        Arrays.asList(commentHeader.comments));
 
     setupData.format =
         new Format.Builder()
@@ -139,12 +140,12 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
       return null;
     }
 
-    if (commentHeader == null) {
+    if (vorbisCommentHeader == null) {
-      commentHeader = VorbisUtil.readVorbisCommentHeader(scratch);
+      vorbisCommentHeader = VorbisUtil.readVorbisCommentHeader(scratch);
       return null;
     }
     VorbisUtil.VorbisIdHeader vorbisIdHeader = this.vorbisIdHeader;
-    VorbisUtil.CommentHeader commentHeader = this.commentHeader;
+    VorbisUtil.CommentHeader commentHeader = this.vorbisCommentHeader;
 
     // the third packet contains the setup header
     byte[] setupHeaderData = new byte[scratch.limit()];

library/extractor/src/main/java/com/google/android/exoplayer2/metadata/vorbis/PictureFrame.java

@@ -24,7 +24,7 @@ import com.google.android.exoplayer2.MediaMetadata;
 import com.google.android.exoplayer2.metadata.Metadata;
 import java.util.Arrays;
 
-/** A picture parsed from a Vorbis Comment or a FLAC metadata block. */
+/** A picture parsed from a Vorbis Comment or a FLAC picture block. */
 public class PictureFrame implements Metadata.Entry {
 
   /** The type of the picture. */