From 85c4d9870b77760e354b738781d5a8d0257cfee7 Mon Sep 17 00:00:00 2001
From: Alexander Capehart <alex@oxycblt.org>
Date: Tue, 22 Nov 2022 09:28:05 -0700
Subject: [PATCH] Move ID3v2 text frame fallback to ID3v2 decoder

Instead of the ID3v2 text frame falling back to a singleton list of an
empty string when the given values are empty, make that case throw
an exception within the text frame, and move that fallback behavior
into the ID3v2 decoder.
---
 .../exoplayer2/metadata/id3/Id3Decoder.java      | 11 ++++++-----
 .../metadata/id3/TextInformationFrame.java       | 13 ++++++-------
 .../metadata/id3/TextInformationFrameTest.java   | 16 ----------------
 3 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java b/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java
index 444befa1fc..15adda2f8a 100644
--- a/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java
+++ b/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/Id3Decoder.java
@@ -489,12 +489,9 @@ public final class Id3Decoder extends SimpleMetadataDecoder {
     ArrayList<String> values = new ArrayList<>();
 
     if (index >= data.length) {
-      return Collections.emptyList();
+      return Collections.singletonList("");
     }
 
-    // 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);
@@ -506,7 +503,11 @@ public final class Id3Decoder extends SimpleMetadataDecoder {
       valueEndIndex = indexOfTerminator(data, valueStartIndex, encoding);
     }
 
-    return values;
+    if (values.isEmpty()) {
+      return Collections.singletonList("");
+    } else {
+      return values;
+    }
   }
 
   @Nullable
diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java b/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java
index 70fc21e05d..8be0811d96 100644
--- a/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java
+++ b/library/extractor/src/main/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrame.java
@@ -36,19 +36,18 @@ public final class TextInformationFrame extends Id3Frame {
   @Deprecated
   public final String value;
 
+  /** The text values of this frame. Will always have at least one element. */
   @NonNull
   public final List<String> values;
 
   public TextInformationFrame(String id, @Nullable String description, @NonNull List<String> values) {
     super(id);
-    this.description = description;
-    
-    if( values.size() > 0 ) {
-      this.values = ImmutableList.copyOf(values);
-    } else {
-      this.values = ImmutableList.of("");
+    if (values.isEmpty()) {
+      throw new IllegalArgumentException("Text information frames must have at least one value");
     }
-    
+
+    this.description = description;
+    this.values = ImmutableList.copyOf(values);
     this.value = this.values.get(0);
   }
 
diff --git a/library/extractor/src/test/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrameTest.java b/library/extractor/src/test/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrameTest.java
index 8c727bd51a..712b6c0734 100644
--- a/library/extractor/src/test/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrameTest.java
+++ b/library/extractor/src/test/java/com/google/android/exoplayer2/metadata/id3/TextInformationFrameTest.java
@@ -109,21 +109,5 @@ 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 =
-        Collections.singletonList(
-            new TextInformationFrame(/* id= */ "TT2", /* description= */ null, /* values= */ Collections.emptyList())
-        );
-
-    builder = MediaMetadata.EMPTY.buildUpon();
-
-    for (Metadata.Entry entry : entries) {
-      entry.populateMediaMetadata(builder);
-    }
-
-    mediaMetadata = builder.build();
-
-    assertThat(mediaMetadata.title.toString()).isEqualTo("");
   }
 }