Skip to content

Commit f91d520

Browse files
icbakershahdDaghash
authored andcommitted
MP3: Use bytes field from VBRI frame instead of deriving from ToC
The previous code assumed that the `VBRI` Table of Contents (ToC) covers all the MP3 data in the file. In a file with an invalid VBRI ToC where this isn't the case, this results in playback silently stopping mid-playback (and either advancing to the next item, or continuing to count up the playback clock forever). This change considers the `bytes` field to determine the end of the MP3 data, in addition to deriving it from the ToC. If they disagree we log a warning and take the max value. This is because we handle accidentally reading non-MP3 data at the end (or hitting EoF) better than stopping reading valid MP3 data partway through. Issue: #1904 #cherrypick PiperOrigin-RevId: 700319250 (cherry picked from commit 46578ee)
1 parent 989c8f4 commit f91d520

File tree

11 files changed

+3418
-5
lines changed

11 files changed

+3418
-5
lines changed

RELEASENOTES.md

+3
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
* Add support for transmuxing into alternative backwards compatible
2727
formats.
2828
* Extractors:
29+
* MP3: Don't stop playback early when a `VBRI` frame's table of contents
30+
doesn't cover all the MP3 data in a file
31+
([#1904](https://github.com/androidx/media/issues/1904)).
2932
* DataSource:
3033
* Audio:
3134
* Do not bypass `SonicAudioProcessor` when `SpeedChangingAudioProcessor`

libraries/exoplayer/src/test/java/androidx/media3/exoplayer/e2etest/Mp3PlaybackTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public static ImmutableList<String> mediaSamples() {
4444
"bear-id3.mp3",
4545
"bear-id3-numeric-genre.mp3",
4646
"bear-vbr-no-seek-table.mp3",
47+
"bear-vbr-vbri-header-truncated-toc.mp3",
4748
"bear-vbr-xing-header.mp3",
4849
"play-trimmed.mp3",
4950
"test-cbr-info-header.mp3");

libraries/extractor/src/main/java/androidx/media3/extractor/mp3/VbriSeeker.java

+19-5
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package androidx.media3.extractor.mp3;
1717

18+
import static java.lang.Math.max;
19+
1820
import androidx.annotation.Nullable;
1921
import androidx.media3.common.C;
2022
import androidx.media3.common.util.Log;
@@ -47,7 +49,9 @@ public static VbriSeeker create(
4749
long position,
4850
MpegAudioUtil.Header mpegAudioHeader,
4951
ParsableByteArray frame) {
50-
frame.skipBytes(10);
52+
frame.skipBytes(6);
53+
int bytes = frame.readInt();
54+
long endOfMp3Data = position + mpegAudioHeader.frameSize + bytes;
5155
int numFrames = frame.readInt();
5256
if (numFrames <= 0) {
5357
return null;
@@ -87,11 +91,21 @@ public static VbriSeeker create(
8791
}
8892
position += segmentSize * ((long) scale);
8993
}
90-
if (inputLength != C.LENGTH_UNSET && inputLength != position) {
91-
Log.w(TAG, "VBRI data size mismatch: " + inputLength + ", " + position);
94+
if (inputLength != C.LENGTH_UNSET && inputLength != endOfMp3Data) {
95+
Log.w(TAG, "VBRI data size mismatch: " + inputLength + ", " + endOfMp3Data);
96+
}
97+
if (endOfMp3Data != position) {
98+
Log.w(
99+
TAG,
100+
"VBRI bytes and ToC mismatch (using max): "
101+
+ endOfMp3Data
102+
+ ", "
103+
+ position
104+
+ "\nSeeking will be inaccurate.");
105+
endOfMp3Data = max(endOfMp3Data, position);
92106
}
93-
return new VbriSeeker(
94-
timesUs, positions, durationUs, /* dataEndPosition= */ position, mpegAudioHeader.bitrate);
107+
108+
return new VbriSeeker(timesUs, positions, durationUs, endOfMp3Data, mpegAudioHeader.bitrate);
95109
}
96110

97111
private final long[] timesUs;

libraries/extractor/src/test/java/androidx/media3/extractor/mp3/Mp3ExtractorTest.java

+7
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ public void mp3SampleWithVbriHeader() throws Exception {
7777
Mp3Extractor::new, "media/mp3/bear-vbr-vbri-header.mp3", simulationConfig);
7878
}
7979

80+
// https://github.com/androidx/media/issues/1904
81+
@Test
82+
public void mp3SampleWithVbriHeaderWithTruncatedToC() throws Exception {
83+
ExtractorAsserts.assertBehavior(
84+
Mp3Extractor::new, "media/mp3/bear-vbr-vbri-header-truncated-toc.mp3", simulationConfig);
85+
}
86+
8087
@Test
8188
public void mp3SampleWithCbrSeeker() throws Exception {
8289
ExtractorAsserts.assertBehavior(

0 commit comments

Comments
 (0)