MP3: Exclude VBRI frame from ToC position calculations

The current code assumes that the first Table of Contents segment includes the `VBRI` frame, but I don't think this is correct and it should only include real/audible MP3 ata - so this change updates the logic to assume the first ToC segment starts at the frame **after** the `VBRI` frame. Issue: #1904 PiperOrigin-RevId: 700269811 (cherry picked from commit f257e55)
androidx · Nov 26, 2024 · 989c8f4 · 989c8f4
1 parent 7abfa76
commit 989c8f4
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 31 deletions.
diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/mp3/VbriSeeker.java b/libraries/extractor/src/main/java/androidx/media3/extractor/mp3/VbriSeeker.java
@@ -15,8 +15,6 @@
  */
 package androidx.media3.extractor.mp3;
 
-import static java.lang.Math.max;
-
 import androidx.annotation.Nullable;
 import androidx.media3.common.C;
 import androidx.media3.common.util.Log;
@@ -63,15 +61,13 @@ public static VbriSeeker create(
     int entrySize = frame.readUnsignedShort();
     frame.skipBytes(2);
 
-    long minPosition = position + mpegAudioHeader.frameSize;
+    position += mpegAudioHeader.frameSize;
     // Read table of contents entries.
     long[] timesUs = new long[entryCount];
     long[] positions = new long[entryCount];
     for (int index = 0; index < entryCount; index++) {
       timesUs[index] = (index * durationUs) / entryCount;
-      // Ensure positions do not fall within the frame containing the VBRI header. This constraint
-      // will normally only apply to the first entry in the table.
-      positions[index] = max(position, minPosition);
+      positions[index] = position;
       int segmentSize;
       switch (entrySize) {
         case 1:

diff --git a/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.0.dump b/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.0.dump
@@ -2,13 +2,13 @@ seekMap:
   isSeekable = true
   duration = 2808000
   getPosition(0) = [[timeUs=0, position=141]]
-  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10317]]
-  getPosition(1404000) = [[timeUs=1404000, position=19677]]
-  getPosition(2808000) = [[timeUs=2106000, position=29517]]
+  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10413]]
+  getPosition(1404000) = [[timeUs=1404000, position=19773]]
+  getPosition(2808000) = [[timeUs=2106000, position=29613]]
 numberOfTracks = 1
 track 0:
-  total output bytes = 38064
-  sample count = 116
+  total output bytes = 38160
+  sample count = 117
   format 0:
     averageBitrate = 32000
     sampleMimeType = audio/mpeg
@@ -480,4 +480,8 @@ track 0:
     time = 2760000
     flags = 1
     data = length 120, hash F562B52F
+  sample 116:
+    time = 2784000
+    flags = 1
+    data = length 96, hash FF8D5B98
 tracksEnded = true
diff --git a/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.1.dump b/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.1.dump
@@ -2,13 +2,13 @@ seekMap:
   isSeekable = true
   duration = 2808000
   getPosition(0) = [[timeUs=0, position=141]]
-  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10317]]
-  getPosition(1404000) = [[timeUs=1404000, position=19677]]
-  getPosition(2808000) = [[timeUs=2106000, position=29517]]
+  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10413]]
+  getPosition(1404000) = [[timeUs=1404000, position=19773]]
+  getPosition(2808000) = [[timeUs=2106000, position=29613]]
 numberOfTracks = 1
 track 0:
-  total output bytes = 27792
-  sample count = 87
+  total output bytes = 27888
+  sample count = 88
   format 0:
     averageBitrate = 32000
     sampleMimeType = audio/mpeg
@@ -364,4 +364,8 @@ track 0:
     time = 2766000
     flags = 1
     data = length 120, hash F562B52F
+  sample 87:
+    time = 2790000
+    flags = 1
+    data = length 96, hash FF8D5B98
 tracksEnded = true
diff --git a/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.2.dump b/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.2.dump
@@ -2,13 +2,13 @@ seekMap:
   isSeekable = true
   duration = 2808000
   getPosition(0) = [[timeUs=0, position=141]]
-  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10317]]
-  getPosition(1404000) = [[timeUs=1404000, position=19677]]
-  getPosition(2808000) = [[timeUs=2106000, position=29517]]
+  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10413]]
+  getPosition(1404000) = [[timeUs=1404000, position=19773]]
+  getPosition(2808000) = [[timeUs=2106000, position=29613]]
 numberOfTracks = 1
 track 0:
-  total output bytes = 18432
-  sample count = 58
+  total output bytes = 18528
+  sample count = 59
   format 0:
     averageBitrate = 32000
     sampleMimeType = audio/mpeg
@@ -248,4 +248,8 @@ track 0:
     time = 2772000
     flags = 1
     data = length 120, hash F562B52F
+  sample 58:
+    time = 2796000
+    flags = 1
+    data = length 96, hash FF8D5B98
 tracksEnded = true
diff --git a/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.3.dump b/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.3.dump
@@ -2,13 +2,13 @@ seekMap:
   isSeekable = true
   duration = 2808000
   getPosition(0) = [[timeUs=0, position=141]]
-  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10317]]
-  getPosition(1404000) = [[timeUs=1404000, position=19677]]
-  getPosition(2808000) = [[timeUs=2106000, position=29517]]
+  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10413]]
+  getPosition(1404000) = [[timeUs=1404000, position=19773]]
+  getPosition(2808000) = [[timeUs=2106000, position=29613]]
 numberOfTracks = 1
 track 0:
-  total output bytes = 8592
-  sample count = 29
+  total output bytes = 8688
+  sample count = 30
   format 0:
     averageBitrate = 32000
     sampleMimeType = audio/mpeg
@@ -132,4 +132,8 @@ track 0:
     time = 2778000
     flags = 1
     data = length 120, hash F562B52F
+  sample 29:
+    time = 2802000
+    flags = 1
+    data = length 96, hash FF8D5B98
 tracksEnded = true
diff --git a/...test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.unknown_length.dump b/...test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.unknown_length.dump
@@ -2,13 +2,13 @@ seekMap:
   isSeekable = true
   duration = 2808000
   getPosition(0) = [[timeUs=0, position=141]]
-  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10317]]
-  getPosition(1404000) = [[timeUs=1404000, position=19677]]
-  getPosition(2808000) = [[timeUs=2106000, position=29517]]
+  getPosition(1) = [[timeUs=0, position=141], [timeUs=702000, position=10413]]
+  getPosition(1404000) = [[timeUs=1404000, position=19773]]
+  getPosition(2808000) = [[timeUs=2106000, position=29613]]
 numberOfTracks = 1
 track 0:
-  total output bytes = 38064
-  sample count = 116
+  total output bytes = 38160
+  sample count = 117
   format 0:
     averageBitrate = 32000
     sampleMimeType = audio/mpeg
@@ -480,4 +480,8 @@ track 0:
     time = 2760000
     flags = 1
     data = length 120, hash F562B52F
+  sample 116:
+    time = 2784000
+    flags = 1
+    data = length 96, hash FF8D5B98
 tracksEnded = true