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: androidx/media#1904 #cherrypick PiperOrigin-RevId: 700269811
2025-04-30 06:46:50 +08:00 · 2024-11-26 02:32:09 -08:00 · 2024-11-26 02:32:09 -08:00 · f257e5511f
commit f257e5511f
parent 6a3def3ccb
6 changed files with 47 additions and 31 deletions
--- 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 @@ import androidx.media3.extractor.SeekPoint;
    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:
--- 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
  track duration = 2808000
  format 0:
    averageBitrate = 32000
@ -482,4 +482,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
--- 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
  track duration = 2808000
  format 0:
    averageBitrate = 32000
@ -366,4 +366,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
--- 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
  track duration = 2808000
  format 0:
    averageBitrate = 32000
@ -250,4 +250,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
--- 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
  track duration = 2808000
  format 0:
    averageBitrate = 32000
@ -134,4 +134,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
--- a/libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-vbri-header.mp3.unknown_length.dump
+++ b/libraries/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
  track duration = 2808000
  format 0:
    averageBitrate = 32000
@ -482,4 +482,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