Improve multi-value text frame parsing

Rework the parsing of multi-value text frames to reduce duplication
and increase efficiency.

library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java

@@ -10395,7 +10395,7 @@ public final class ExoPlayerTest {
                 new Metadata(
                     new BinaryFrame(/* id= */ "", /* data= */ new byte[0]),
                     new TextInformationFrame(
-                        /* id= */ "TT2", /* description= */ null, /* value= */ "title")))
+                        /* id= */ "TT2", /* description= */ null, /* value= */ Collections.singletonList("title"))))
             .build();
 
     // Set multiple values together.

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/mp4/MetadataUtil.java

@@ -31,6 +31,7 @@ import com.google.android.exoplayer2.metadata.id3.TextInformationFrame;
 import com.google.android.exoplayer2.metadata.mp4.MdtaMetadataEntry;
 import com.google.android.exoplayer2.util.Log;
 import com.google.android.exoplayer2.util.ParsableByteArray;
+import java.util.Collections;
 import org.checkerframework.checker.nullness.compatqual.NullableType;
 
 /** Utilities for handling metadata in MP4. */
@@ -452,7 +453,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
     if (atomType == Atom.TYPE_data) {
       data.skipBytes(8); // version (1), flags (3), empty (4)
       String value = data.readNullTerminatedString(atomSize - 16);
-      return new TextInformationFrame(id, /* description= */ null, value);
+      return new TextInformationFrame(id, /* description= */ null, Collections.singletonList(value));
     }
     Log.w(TAG, "Failed to parse text attribute: " + Atom.getAtomTypeString(type));
     return null;
@@ -484,7 +485,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
     }
     if (value >= 0) {
       return isTextInformationFrame
-          ? new TextInformationFrame(id, /* description= */ null, Integer.toString(value))
+          ? new TextInformationFrame(id, /* description= */ null, Collections.singletonList(Integer.toString(value)))
           : new CommentFrame(C.LANGUAGE_UNDETERMINED, id, Integer.toString(value));
     }
     Log.w(TAG, "Failed to parse uint8 attribute: " + Atom.getAtomTypeString(type));
@@ -505,7 +506,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
         if (count > 0) {
           value += "/" + count;
         }
-        return new TextInformationFrame(attributeName, /* description= */ null, value);
+        return new TextInformationFrame(attributeName, /* description= */ null, Collections.singletonList(value));
       }
     }
     Log.w(TAG, "Failed to parse index/count attribute: " + Atom.getAtomTypeString(type));
@@ -521,7 +522,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
             ? STANDARD_GENRES[genreCode - 1]
             : null;
     if (genreString != null) {
-      return new TextInformationFrame("TCON", /* description= */ null, genreString);
+      return new TextInformationFrame("TCON", /* description= */ null, Collections.singletonList(genreString));
     }
     Log.w(TAG, "Failed to parse standard genre code");
     return null;

library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java

@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.metadata.id3;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.metadata.Metadata;
@@ -25,10 +26,12 @@ import com.google.android.exoplayer2.util.ParsableBitArray;
 import com.google.android.exoplayer2.util.ParsableByteArray;
 import com.google.android.exoplayer2.util.Util;
 import com.google.common.base.Ascii;
+import com.google.common.collect.ImmutableList;
 import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 
@@ -458,26 +461,9 @@ public final class Id3Decoder extends SimpleMetadataDecoder {
     int descriptionEndIndex = indexOfTerminator(data, 0, encoding);
     String description = new String(data, 0, descriptionEndIndex, charset);
 
-    // In ID3v2.4, text information frames can contain multiple values delimited by a null
-    // terminator. Thus, we after each "end of stream" marker we actually need to keep looking
-    // for more data, at least until the index is equal to the data length.
-    ArrayList<String> values = new ArrayList<>();
-
-    int valueStartIndex = descriptionEndIndex + delimiterLength(encoding);
-    if (valueStartIndex >= data.length) {
-      return new TextInformationFrame("TXXX", description, new String[0]);
-    }
-
-    int valueEndIndex = indexOfTerminator(data, valueStartIndex, encoding);
-    while (valueStartIndex < valueEndIndex) {
-      String value = decodeStringIfValid(data, valueStartIndex, valueEndIndex, charset);
-      values.add(value);
-
-      valueStartIndex = valueEndIndex + delimiterLength(encoding);
-      valueEndIndex = indexOfTerminator(data, valueStartIndex, encoding);
-    }
-
-    return new TextInformationFrame("TXXX", description, values.toArray(new String[0]));
+    List<String> values = decodeTextInformationFrameValues(
+        data, encoding, descriptionEndIndex + delimiterLength(encoding));
+    return new TextInformationFrame("TXXX", description, values);
   }
 
   @Nullable
@@ -489,31 +475,38 @@ public final class Id3Decoder extends SimpleMetadataDecoder {
     }
 
     int encoding = id3Data.readUnsignedByte();
-    String charset = getCharsetName(encoding);
 
     byte[] data = new byte[frameSize - 1];
     id3Data.readBytes(data, 0, frameSize - 1);
 
-    // In ID3v2.4, text information frames can contain multiple values delimited by a null
-    // terminator. Thus, we after each "end of stream" marker we actually need to keep looking
-    // for more data, at least until the index is equal to the data length.
+    List<String> values = decodeTextInformationFrameValues(data, encoding, 0);
+    return new TextInformationFrame(id, null, values);
+  }
+
+  @NonNull
+  private static List<String> decodeTextInformationFrameValues(
+      byte[] data, final int encoding, final int index) throws UnsupportedEncodingException {
     ArrayList<String> values = new ArrayList<>();
 
-    int valueStartIndex = 0;
-    if (valueStartIndex >= data.length) {
-      return new TextInformationFrame(id, null, new String[0]);
+    if (index >= data.length) {
+      return Collections.emptyList();
     }
 
+    // In ID3v2.4, text information frames can contain multiple values delimited by a null
+    // terminator. Thus, we after each "end of stream" marker we actually need to keep looking
+    // for more data, at least until the index is equal to the data length.
+    String charset = getCharsetName(encoding);
+    int valueStartIndex = index;
     int valueEndIndex = indexOfTerminator(data, valueStartIndex, encoding);
     while (valueStartIndex < valueEndIndex) {
-      String value = decodeStringIfValid(data, valueStartIndex, valueEndIndex, charset);
+      String value = new String(data, valueStartIndex, valueEndIndex - valueStartIndex, charset);
       values.add(value);
 
       valueStartIndex = valueEndIndex + delimiterLength(encoding);
       valueEndIndex = indexOfTerminator(data, valueStartIndex, encoding);
     }
 
-    return new TextInformationFrame(id, null, values.toArray(new String[0]));
+    return values;
   }
 
   @Nullable

library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java

@@ -25,6 +25,7 @@ import com.google.android.exoplayer2.MediaMetadata;
 import com.google.android.exoplayer2.util.Util;
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /** Text information ID3 frame. */
@@ -38,11 +39,11 @@ public final class TextInformationFrame extends Id3Frame {
   @NonNull
   public final ImmutableList<String> values;
 
-  public TextInformationFrame(String id, @Nullable String description, @NonNull String[] values) {
+  public TextInformationFrame(String id, @Nullable String description, @NonNull List<String> values) {
     super(id);
     this.description = description;
     
-    if( values.length > 0 ) {
+    if( values.size() > 0 ) {
       this.values = ImmutableList.copyOf(values);
     } else {
       this.values = ImmutableList.of("");
@@ -54,7 +55,7 @@ public final class TextInformationFrame extends Id3Frame {
   /** @deprecated Use {@code TextInformationFrame(String id, String description, String[] values} instead */
   @Deprecated
   public TextInformationFrame(String id, @Nullable String description, String value) {
-    this(id, description, new String[] {value } );
+    this(id, description, Collections.singletonList(value) );
   }
 
   /* package */ TextInformationFrame(Parcel in) {

library/extractor/src/test/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrameTest.java

@@ -22,6 +22,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.android.exoplayer2.MediaMetadata;
 import com.google.android.exoplayer2.metadata.Metadata;
 import com.google.common.collect.ImmutableList;
+import java.util.Collections;
 import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -32,7 +33,7 @@ public class TextInformationFrameTest {
 
   @Test
   public void parcelable() {
-    TextInformationFrame textInformationFrameToParcel = new TextInformationFrame("", "", "");
+    TextInformationFrame textInformationFrameToParcel = new TextInformationFrame("", "", Collections.singletonList(""));
 
     Parcel parcel = Parcel.obtain();
     textInformationFrameToParcel.writeToParcel(parcel, 0);
@@ -62,28 +63,29 @@ public class TextInformationFrameTest {
 
     List<Metadata.Entry> entries =
         ImmutableList.of(
-            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */ new String[] { title }),
-            new TextInformationFrame(/* id= */ "TP1", /* description= */ null, /* value= */ new String[] { artist }),
+            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */
+                Collections.singletonList(title)),
+            new TextInformationFrame(/* id= */ "TP1", /* description= */ null, /* values= */ Collections.singletonList( artist )),
             new TextInformationFrame(
-                /* id= */ "TAL", /* description= */ null, /* value= */ new String[] { albumTitle }),
+                /* id= */ "TAL", /* description= */ null, /* values= */ Collections.singletonList( albumTitle )),
             new TextInformationFrame(
-                /* id= */ "TP2", /* description= */ null, /* value= */ new String[] { albumArtist }),
+                /* id= */ "TP2", /* description= */ null, /* values= */ Collections.singletonList( albumArtist )),
             new TextInformationFrame(
-                /* id= */ "TRK", /* description= */ null, /* value= */ new String[] { trackNumberInfo}),
+                /* id= */ "TRK", /* description= */ null, /* values= */ Collections.singletonList( trackNumberInfo)),
             new TextInformationFrame(
-                /* id= */ "TYE", /* description= */ null, /* value= */ new String[] { recordingYear }),
+                /* id= */ "TYE", /* description= */ null, /* values= */ Collections.singletonList( recordingYear )),
             new TextInformationFrame(
                 /* id= */ "TDA",
                 /* description= */ null,
-                /* value= */ new String[] { recordingDay + recordingMonth }),
+                /* value= */ Collections.singletonList( recordingDay + recordingMonth )),
             new TextInformationFrame(
-                /* id= */ "TDRL", /* description= */ null, /* value= */ new String[] { releaseDate }),
+                /* id= */ "TDRL", /* description= */ null, /* values= */ Collections.singletonList( releaseDate )),
             new TextInformationFrame(
-                /* id= */ "TCM", /* description= */ null, /* value= */ new String[] { composer }),
+                /* id= */ "TCM", /* description= */ null, /* values= */ Collections.singletonList( composer )),
             new TextInformationFrame(
-                /* id= */ "TP3", /* description= */ null, /* value= */ new String[] { conductor }),
+                /* id= */ "TP3", /* description= */ null, /* values= */ Collections.singletonList( conductor )),
             new TextInformationFrame(
-                /* id= */ "TXT", /* description= */ null, /* value= */ new String[] { writer }));
+                /* id= */ "TXT", /* description= */ null, /* values= */ Collections.singletonList( writer )));
     MediaMetadata.Builder builder = MediaMetadata.EMPTY.buildUpon();
 
     for (Metadata.Entry entry : entries) {
@@ -110,8 +112,8 @@ public class TextInformationFrameTest {
 
     // Test empty value array
     entries =
-        ImmutableList.of(
-            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */ new String[0])
+        Collections.singletonList(
+            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* values= */ Collections.emptyList())
         );
 
     builder = MediaMetadata.EMPTY.buildUpon();