Rework ID3v2 text frame values interface

Rework the interface for ID3v2 text frames by:
- Switching from a mutable array to ImmutableList
- Never leaving the array empty, instead filling it with "" if there
are no values

This makes the surface safer and more in line with the rest of the
module.

extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java

@@ -811,7 +811,7 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou
         if (entry instanceof TextInformationFrame) {
           TextInformationFrame textFrame = (TextInformationFrame) entry;
           if ("TXXX".equals(textFrame.id)) {
-            streamPlayer.triggerUserTextReceived(textFrame.values[0]);
+            streamPlayer.triggerUserTextReceived(textFrame.values.get(0));
           }
         } else if (entry instanceof EventMessage) {
           EventMessage eventMessage = (EventMessage) entry;

library/extractor/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java

@@ -592,7 +592,7 @@ public final class Mp3Extractor implements Extractor {
         Metadata.Entry entry = metadata.get(i);
         if (entry instanceof TextInformationFrame
             && ((TextInformationFrame) entry).id.equals("TLEN")) {
-          return Util.msToUs(Long.parseLong(((TextInformationFrame) entry).values[0]));
+          return Util.msToUs(Long.parseLong(((TextInformationFrame) entry).values.get(0)));
         }
       }
     }

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

@@ -23,33 +23,32 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 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.Arrays;
 import java.util.List;
 
 /** Text information ID3 frame. */
 public final class TextInformationFrame extends Id3Frame {
-  private final static String MULTI_VALUE_DELIMITER = ", ";
-
   @Nullable public final String description;
 
-  /** @deprecated Use {@code values} instead. */
+  /** @deprecated Use {@code values[0]} instead. */
   @Deprecated
   public final String value;
 
   @NonNull
-  public final String[] values;
+  public final ImmutableList<String> values;
 
   public TextInformationFrame(String id, @Nullable String description, @NonNull String[] values) {
     super(id);
     this.description = description;
-    this.values = values;
     
-    if (values.length > 0) {
+    if( values.length > 0 ) {
-      this.value = values[0];
+      this.values = ImmutableList.copyOf(values);
     } else {
-      this.value = null;
+      this.values = ImmutableList.of("");
     }
+    
+    this.value = this.values.get(0);
   }
 
   /** @deprecated Use {@code TextInformationFrame(String id, String description, String[] values} instead */
@@ -61,8 +60,8 @@ public final class TextInformationFrame extends Id3Frame {
   /* package */ TextInformationFrame(Parcel in) {
     super(castNonNull(in.readString()));
     description = in.readString();
-    values = in.createStringArray();
+    values = ImmutableList.copyOf(in.createStringArray());
-    this.value = values[0];
+    value = values.get(0);
   }
 
   @Override
@@ -73,23 +72,23 @@ public final class TextInformationFrame extends Id3Frame {
     switch (id) {
       case "TT2":
       case "TIT2":
-        builder.setTitle(values[0]);
+        builder.setTitle(values.get(0));
         break;
       case "TP1":
       case "TPE1":
-        builder.setArtist(String.join(MULTI_VALUE_DELIMITER, values));
+        builder.setArtist(String.join(", ", values));
         break;
       case "TP2":
       case "TPE2":
-        builder.setAlbumArtist(String.join(MULTI_VALUE_DELIMITER, values));
+        builder.setAlbumArtist(String.join(", ", values));
         break;
       case "TAL":
       case "TALB":
-        builder.setAlbumTitle(values[0]);
+        builder.setAlbumTitle(values.get(0));
         break;
       case "TRK":
       case "TRCK":
-        String[] trackNumbers = Util.split(values[0], "/");
+        String[] trackNumbers = Util.split(values.get(0), "/");
         try {
           int trackNumber = Integer.parseInt(trackNumbers[0]);
           @Nullable
@@ -103,7 +102,7 @@ public final class TextInformationFrame extends Id3Frame {
       case "TYE":
       case "TYER":
         try {
-          builder.setRecordingYear(Integer.parseInt(values[0]));
+          builder.setRecordingYear(Integer.parseInt(values.get(0)));
         } catch (NumberFormatException e) {
           // Do nothing, invalid input.
         }
@@ -111,7 +110,7 @@ public final class TextInformationFrame extends Id3Frame {
       case "TDA":
       case "TDAT":
         try {
-          String date = values[0];
+          String date = values.get(0);
           int month = Integer.parseInt(date.substring(2, 4));
           int day = Integer.parseInt(date.substring(0, 2));
           builder.setRecordingMonth(month).setRecordingDay(day);
@@ -120,7 +119,7 @@ public final class TextInformationFrame extends Id3Frame {
         }
         break;
       case "TDRC":
-        List<Integer> recordingDate = parseId3v2point4TimestampFrameForDate(values[0]);
+        List<Integer> recordingDate = parseId3v2point4TimestampFrameForDate(values.get(0));
         switch (recordingDate.size()) {
           case 3:
             builder.setRecordingDay(recordingDate.get(2));
@@ -138,7 +137,7 @@ public final class TextInformationFrame extends Id3Frame {
         }
         break;
       case "TDRL":
-        List<Integer> releaseDate = parseId3v2point4TimestampFrameForDate(values[0]);
+        List<Integer> releaseDate = parseId3v2point4TimestampFrameForDate(values.get(0));
         switch (releaseDate.size()) {
           case 3:
             builder.setReleaseDay(releaseDate.get(2));
@@ -157,15 +156,15 @@ public final class TextInformationFrame extends Id3Frame {
         break;
       case "TCM":
       case "TCOM":
-        builder.setComposer(String.join(MULTI_VALUE_DELIMITER, values));
+        builder.setComposer(String.join(", ", values));
         break;
       case "TP3":
       case "TPE3":
-        builder.setConductor(String.join(MULTI_VALUE_DELIMITER, values));
+        builder.setConductor(String.join(", ", values));
         break;
       case "TXT":
       case "TEXT":
-        builder.setWriter(String.join(MULTI_VALUE_DELIMITER, values));
+        builder.setWriter(String.join(", ", values));
         break;
       default:
         break;
@@ -183,7 +182,7 @@ public final class TextInformationFrame extends Id3Frame {
     TextInformationFrame other = (TextInformationFrame) obj;
     return Util.areEqual(id, other.id)
         && Util.areEqual(description, other.description)
-        && Arrays.equals(values, other.values);
+        && values.equals(other.values);
   }
 
   @Override
@@ -191,13 +190,13 @@ public final class TextInformationFrame extends Id3Frame {
     int result = 17;
     result = 31 * result + id.hashCode();
     result = 31 * result + (description != null ? description.hashCode() : 0);
-    result = 31 * result + Arrays.hashCode(values);
+    result = 31 * result + values.hashCode();
     return result;
   }
 
   @Override
   public String toString() {
-    return id + ": description=" + description + ": value=" + String.join(MULTI_VALUE_DELIMITER, values);
+    return id + ": description=" + description + ": value=" + String.join(", ", values);
   }
 
   // Parcelable implementation.
@@ -206,7 +205,7 @@ public final class TextInformationFrame extends Id3Frame {
   public void writeToParcel(Parcel dest, int flags) {
     dest.writeString(id);
     dest.writeString(description);
-    dest.writeStringArray(values);
+    dest.writeStringArray(values.toArray(new String[0]));
   }
 
   public static final Parcelable.Creator<TextInformationFrame> CREATOR =

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

@@ -52,7 +52,7 @@ public final class Id3DecoderTest {
     TextInformationFrame textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.id).isEqualTo("TXXX");
     assertThat(textInformationFrame.description).isEmpty();
-    assertThat(textInformationFrame.values[0]).isEqualTo("mdialog_VINDICO1527664_start");
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("mdialog_VINDICO1527664_start");
 
     // Test UTF-16.
     rawId3 =
@@ -67,7 +67,8 @@ public final class Id3DecoderTest {
     textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.id).isEqualTo("TXXX");
     assertThat(textInformationFrame.description).isEqualTo("Hello World");
-    assertThat(textInformationFrame.values).isEmpty();
+    assertThat(textInformationFrame.values.size()).isEqualTo(1);
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("");
 
     // Test multiple values.
     rawId3 = buildSingleFrameTag(
@@ -80,9 +81,9 @@ public final class Id3DecoderTest {
     assertThat(metadata.length()).isEqualTo(1);
     textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.description).isEqualTo("Hello World");
-    assertThat(textInformationFrame.values.length).isEqualTo(2);
+    assertThat(textInformationFrame.values.size()).isEqualTo(2);
-    assertThat(textInformationFrame.values[0]).isEqualTo("Foo");
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("Foo");
-    assertThat(textInformationFrame.values[1]).isEqualTo("Bar");
+    assertThat(textInformationFrame.values.get(1)).isEqualTo("Bar");
 
     // Test empty.
     rawId3 = buildSingleFrameTag("TXXX", new byte[0]);
@@ -96,7 +97,8 @@ public final class Id3DecoderTest {
     textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.id).isEqualTo("TXXX");
     assertThat(textInformationFrame.description).isEmpty();
-    assertThat(textInformationFrame.values).isEmpty();
+    assertThat(textInformationFrame.values.size()).isEqualTo(1);
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("");
   }
 
   @Test
@@ -110,17 +112,17 @@ public final class Id3DecoderTest {
     TextInformationFrame textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.id).isEqualTo("TIT2");
     assertThat(textInformationFrame.description).isNull();
-    assertThat(textInformationFrame.values.length).isEqualTo(1);
+    assertThat(textInformationFrame.values.size()).isEqualTo(1);
-    assertThat(textInformationFrame.values[0]).isEqualTo("Hello World");
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("Hello World");
 
     // Test multiple values.
     rawId3 = buildSingleFrameTag("TIT2", new byte[] {3, 70, 111, 111, 0, 66, 97, 114, 0});
     metadata = decoder.decode(rawId3, rawId3.length);
     assertThat(metadata.length()).isEqualTo(1);
     textInformationFrame = (TextInformationFrame) metadata.get(0);
-    assertThat(textInformationFrame.values.length).isEqualTo(2);
+    assertThat(textInformationFrame.values.size()).isEqualTo(2);
-    assertThat(textInformationFrame.values[0]).isEqualTo("Foo");
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("Foo");
-    assertThat(textInformationFrame.values[1]).isEqualTo("Bar");
+    assertThat(textInformationFrame.values.get(1)).isEqualTo("Bar");
 
     // Test empty.
     rawId3 = buildSingleFrameTag("TIT2", new byte[0]);
@@ -134,7 +136,8 @@ public final class Id3DecoderTest {
     textInformationFrame = (TextInformationFrame) metadata.get(0);
     assertThat(textInformationFrame.id).isEqualTo("TIT2");
     assertThat(textInformationFrame.description).isNull();
-    assertThat(textInformationFrame.values).isEmpty();
+    assertThat(textInformationFrame.values.size()).isEqualTo(1);
+    assertThat(textInformationFrame.values.get(0)).isEqualTo("");
   }
 
   @Test

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

@@ -62,28 +62,28 @@ public class TextInformationFrameTest {
 
     List<Metadata.Entry> entries =
         ImmutableList.of(
-            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */ title),
+            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */ new String[] { title }),
-            new TextInformationFrame(/* id= */ "TP1", /* description= */ null, /* value= */ artist),
+            new TextInformationFrame(/* id= */ "TP1", /* description= */ null, /* value= */ new String[] { artist }),
             new TextInformationFrame(
-                /* id= */ "TAL", /* description= */ null, /* value= */ albumTitle),
+                /* id= */ "TAL", /* description= */ null, /* value= */ new String[] { albumTitle }),
             new TextInformationFrame(
-                /* id= */ "TP2", /* description= */ null, /* value= */ albumArtist),
+                /* id= */ "TP2", /* description= */ null, /* value= */ new String[] { albumArtist }),
             new TextInformationFrame(
-                /* id= */ "TRK", /* description= */ null, /* value= */ trackNumberInfo),
+                /* id= */ "TRK", /* description= */ null, /* value= */ new String[] { trackNumberInfo}),
             new TextInformationFrame(
-                /* id= */ "TYE", /* description= */ null, /* value= */ recordingYear),
+                /* id= */ "TYE", /* description= */ null, /* value= */ new String[] { recordingYear }),
             new TextInformationFrame(
                 /* id= */ "TDA",
                 /* description= */ null,
-                /* value= */ recordingDay + recordingMonth),
+                /* value= */ new String[] { recordingDay + recordingMonth }),
             new TextInformationFrame(
-                /* id= */ "TDRL", /* description= */ null, /* value= */ releaseDate),
+                /* id= */ "TDRL", /* description= */ null, /* value= */ new String[] { releaseDate }),
             new TextInformationFrame(
-                /* id= */ "TCM", /* description= */ null, /* value= */ composer),
+                /* id= */ "TCM", /* description= */ null, /* value= */ new String[] { composer }),
             new TextInformationFrame(
-                /* id= */ "TP3", /* description= */ null, /* value= */ conductor),
+                /* id= */ "TP3", /* description= */ null, /* value= */ new String[] { conductor }),
             new TextInformationFrame(
-                /* id= */ "TXT", /* description= */ null, /* value= */ writer));
+                /* id= */ "TXT", /* description= */ null, /* value= */ new String[] { writer }));
     MediaMetadata.Builder builder = MediaMetadata.EMPTY.buildUpon();
 
     for (Metadata.Entry entry : entries) {
@@ -107,5 +107,21 @@ public class TextInformationFrameTest {
     assertThat(mediaMetadata.composer.toString()).isEqualTo(composer);
     assertThat(mediaMetadata.conductor.toString()).isEqualTo(conductor);
     assertThat(mediaMetadata.writer.toString()).isEqualTo(writer);
+
+    // Test empty value array
+    entries =
+        ImmutableList.of(
+            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* value= */ new String[0])
+        );
+
+    builder = MediaMetadata.EMPTY.buildUpon();
+
+    for (Metadata.Entry entry : entries) {
+      entry.populateMediaMetadata(builder);
+    }
+
+    mediaMetadata = builder.build();
+
+    assertThat(mediaMetadata.title.toString()).isEqualTo("");
   }
 }