Fix VorbisComment.populateMediaMetadata key comparison to ignore case

Issue: #876 #minor-release PiperOrigin-RevId: 590215918 (cherry picked from commit 5580b78)
androidx · Jan 9, 2024 · 5001f70 · 5001f70
1 parent 504bcd8
commit 5001f70
Show file tree

Hide file tree

Showing 19 changed files with 31 additions and 24 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -28,6 +28,9 @@
         ([#693](https://github.com/androidx/media/issues/693)).
 *   Text:
 *   Metadata:
+    *   Fix bug where `MediaMetadata` was only populated from Vorbis comments
+        with upper-case keys
+        ([#876](https://github.com/androidx/media/issues/876)).
 *   DRM:
     *   Extend workaround for spurious ClearKey `https://default.url` license
         URL to API 33+ (previously the workaround only applied on API 33

diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/flac/VorbisComment.java b/libraries/extractor/src/main/java/androidx/media3/extractor/metadata/flac/VorbisComment.java
@@ -23,6 +23,7 @@
 import androidx.media3.common.MediaMetadata;
 import androidx.media3.common.Metadata;
 import androidx.media3.common.util.UnstableApi;
+import com.google.common.base.Ascii;
 
 /**
  * @deprecated Use {@link androidx.media3.extractor.metadata.vorbis.VorbisComment} instead.
@@ -31,18 +32,21 @@
 @UnstableApi
 public class VorbisComment implements Metadata.Entry {
 
-  /** The key. */
+  /** The key in upper case, to ease case-insensitive comparisons. */
   public final String key;
 
   /** The value. */
   public final String value;
 
   /**
-   * @param key The key.
+   * Constructs an instance.
+   *
+   * @param key The key. Must be an ASCII string containing only characters between 0x20 and 0x7D
+   *     (inclusive), excluding 0x3D ('=').
    * @param value The value.
    */
   public VorbisComment(String key, String value) {
-    this.key = key;
+    this.key = Ascii.toUpperCase(key);
     this.value = value;
   }
 

diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/FlacStreamMetadataTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/FlacStreamMetadataTest.java
@@ -75,10 +75,10 @@ public void parseVorbisComments() {
 
     assertThat(metadata.length()).isEqualTo(2);
     VorbisComment commentFrame = (VorbisComment) metadata.get(0);
-    assertThat(commentFrame.key).isEqualTo("Title");
+    assertThat(commentFrame.key).isEqualTo("TITLE");
     assertThat(commentFrame.value).isEqualTo("Song");
     commentFrame = (VorbisComment) metadata.get(1);
-    assertThat(commentFrame.key).isEqualTo("Artist");
+    assertThat(commentFrame.key).isEqualTo("ARTIST");
     assertThat(commentFrame.value).isEqualTo("Singer");
   }
 
@@ -124,7 +124,7 @@ public void parseVorbisCommentWithEqualsInValue() {
 
     assertThat(metadata.length()).isEqualTo(1);
     VorbisComment commentFrame = (VorbisComment) metadata.get(0);
-    assertThat(commentFrame.key).isEqualTo("Title");
+    assertThat(commentFrame.key).isEqualTo("TITLE");
     assertThat(commentFrame.value).isEqualTo("So=ng");
   }
 
@@ -150,7 +150,7 @@ public void parseInvalidVorbisComment() {
 
     assertThat(metadata.length()).isEqualTo(1);
     VorbisComment commentFrame = (VorbisComment) metadata.get(0);
-    assertThat(commentFrame.key).isEqualTo("Artist");
+    assertThat(commentFrame.key).isEqualTo("ARTIST");
     assertThat(commentFrame.value).isEqualTo("Singer");
   }
 }
diff --git a/.../extractor/src/test/java/androidx/media3/extractor/metadata/vorbis/VorbisCommentTest.java b/.../extractor/src/test/java/androidx/media3/extractor/metadata/vorbis/VorbisCommentTest.java
@@ -54,9 +54,9 @@ public void populateMediaMetadata_setsMediaMetadataValues() {
     List<Metadata.Entry> entries =
         ImmutableList.of(
             new VorbisComment("TITLE", title),
-            new VorbisComment("ARTIST", artist),
+            new VorbisComment("ArTisT", artist),
             new VorbisComment("ALBUM", albumTitle),
-            new VorbisComment("ALBUMARTIST", albumArtist),
+            new VorbisComment("albumartist", albumArtist),
             new VorbisComment("DESCRIPTION", description));
     MediaMetadata.Builder builder = MediaMetadata.EMPTY.buildUpon();
 

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.0.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.0.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.1.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.1.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.2.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.2.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.3.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.3.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.unknown_length.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear.opus.unknown_length.dump
@@ -10,7 +10,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.0.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.0.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.1.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.1.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.2.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.2.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.3.dump b/libraries/test_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.3.dump
@@ -13,7 +13,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/...st_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.unknown_length.dump b/...st_data/src/test/assets/extractordumps/ogg/bear_duplicate_header.opus.unknown_length.dump
@@ -10,7 +10,7 @@ track 0:
     sampleMimeType = audio/opus
     channelCount = 2
     sampleRate = 48000
-    metadata = entries=[VC: encoder=Lavf54.20.4]
+    metadata = entries=[VC: ENCODER=Lavf54.20.4]
     initializationData:
       data = length 19, hash BFE794DB
       data = length 8, hash CA22068C

diff --git a/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.0.dump b/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.0.dump
diff --git a/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.1.dump b/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.1.dump
diff --git a/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.2.dump b/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.2.dump
diff --git a/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.3.dump b/...s/test_data/src/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.3.dump
diff --git a/...rc/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.unknown_length.dump b/...rc/test/assets/extractordumps/ogg/bear_vorbis_with_large_metadata.ogg.unknown_length.dump