Skip to content

Commit

Permalink
MP3: Use bytes field from VBRI frame instead of deriving from ToC
Browse files Browse the repository at this point in the history
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
  • Loading branch information
icbaker authored and copybara-github committed Nov 26, 2024
1 parent 2a4bae0 commit 46578ee
Show file tree
Hide file tree
Showing 11 changed files with 3,428 additions and 5 deletions.
3 changes: 3 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
* Add support for transmuxing into alternative backwards compatible
formats.
* Extractors:
* MP3: Don't stop playback early when a `VBRI` frame's table of contents
doesn't cover all the MP3 data in a file
([#1904](https://github.com/androidx/media/issues/1904)).
* DataSource:
* Audio:
* Do not bypass `SonicAudioProcessor` when `SpeedChangingAudioProcessor`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public static ImmutableList<String> mediaSamples() {
"bear-id3.mp3",
"bear-id3-numeric-genre.mp3",
"bear-vbr-no-seek-table.mp3",
"bear-vbr-vbri-header-truncated-toc.mp3",
"bear-vbr-xing-header.mp3",
"play-trimmed.mp3",
"test-cbr-info-header.mp3");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
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;
Expand Down Expand Up @@ -47,7 +49,9 @@ public static VbriSeeker create(
long position,
MpegAudioUtil.Header mpegAudioHeader,
ParsableByteArray frame) {
frame.skipBytes(10);
frame.skipBytes(6);
int bytes = frame.readInt();
long endOfMp3Data = position + mpegAudioHeader.frameSize + bytes;
int numFrames = frame.readInt();
if (numFrames <= 0) {
return null;
Expand Down Expand Up @@ -87,11 +91,21 @@ public static VbriSeeker create(
}
position += segmentSize * ((long) scale);
}
if (inputLength != C.LENGTH_UNSET && inputLength != position) {
Log.w(TAG, "VBRI data size mismatch: " + inputLength + ", " + position);
if (inputLength != C.LENGTH_UNSET && inputLength != endOfMp3Data) {
Log.w(TAG, "VBRI data size mismatch: " + inputLength + ", " + endOfMp3Data);
}
if (endOfMp3Data != position) {
Log.w(
TAG,
"VBRI bytes and ToC mismatch (using max): "
+ endOfMp3Data
+ ", "
+ position
+ "\nSeeking will be inaccurate.");
endOfMp3Data = max(endOfMp3Data, position);
}
return new VbriSeeker(
timesUs, positions, durationUs, /* dataEndPosition= */ position, mpegAudioHeader.bitrate);

return new VbriSeeker(timesUs, positions, durationUs, endOfMp3Data, mpegAudioHeader.bitrate);
}

private final long[] timesUs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ public void mp3SampleWithVbriHeader() throws Exception {
Mp3Extractor::new, "media/mp3/bear-vbr-vbri-header.mp3", simulationConfig);
}

// https://github.com/androidx/media/issues/1904
@Test
public void mp3SampleWithVbriHeaderWithTruncatedToC() throws Exception {
ExtractorAsserts.assertBehavior(
Mp3Extractor::new, "media/mp3/bear-vbr-vbri-header-truncated-toc.mp3", simulationConfig);
}

@Test
public void mp3SampleWithCbrSeeker() throws Exception {
ExtractorAsserts.assertBehavior(
Expand Down
Loading

0 comments on commit 46578ee

Please sign in to comment.