Skip to content

Commit cef3c8f

Browse files
rohitjoinsmarcbaechinger
rohitjoins
authored and
marcbaechinger
committed
Prioritize CBR seeking over index seeking for MP3s
This change alters the seeker selection logic in `Mp3Extractor` to provide a better default user experience for MP3 files that do not contain seeking metadata (e.g., a Xing or VBRI header). MP3 files without any VBR specific metadata are assumed to be Constant Bitrate (CBR), reflecting the evolution of the MP3 specification, where CBR was the original format and Variable Bitrate (VBR) support was added later and requires explicit marking. Previously, if `FLAG_ENABLE_INDEX_SEEKING` was set, the extractor would create an `IndexSeeker` for such files. This provides accurate seeking, but only after the entire file has been scanned to build the index, leaving the media unseekable for a potentially long time. With this change, the logic is updated to default to a `ConstantBitrateSeeker` when no other seeking metadata is available. This provides immediate but potentially less accurate seeking capabilities. The `IndexSeeker` is now used as a fallback if the primary seeker (from metadata or the CBR assumption) is not seekable, for instance, when dealing with a stream of unknown length. Fixes: Issue: #2848 PiperOrigin-RevId: 831861350 (cherry picked from commit f640c25)
1 parent 1ce77b4 commit cef3c8f

9 files changed

Lines changed: 291 additions & 756 deletions

File tree

‎RELEASENOTES.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@
1515
* Transformer:
1616
* Track Selection:
1717
* Extractors:
18+
* MP3: Change `Mp3Extractor` to default to a constant bitrate (CBR)
19+
assumption when no seeking metadata (e.g., Xing, VBRI) is found, even
20+
when `FLAG_ENABLE_INDEX_SEEKING` is set. This is based on the MP3
21+
specification's history, where CBR was standard and VBR requires
22+
explicit headers. This improves immediate seekability for files without
23+
metadata at the cost of potential accuracy for VBR files lacking
24+
headers. Index seeking is now used as a fallback if the CBR assumption
25+
is not seekable (e.g., for streams of unknown length)
26+
([#2848](https://github.com/androidx/media/issues/2848)).
1827
* Inspector:
1928
* Audio:
2029
* Video:

‎libraries/extractor/src/main/java/androidx/media3/extractor/mp3/Mp3Extractor.java‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ public final class Mp3Extractor implements Extractor {
8686
/**
8787
* Flag to force enable seeking using a constant bitrate assumption in cases where seeking would
8888
* otherwise not be possible.
89-
*
90-
* <p>This flag is ignored if {@link #FLAG_ENABLE_INDEX_SEEKING} is set.
9189
*/
9290
public static final int FLAG_ENABLE_CONSTANT_BITRATE_SEEKING = 1;
9391

@@ -101,16 +99,21 @@ public final class Mp3Extractor implements Extractor {
10199
*
102100
* <p>If this flag is set, then the behavior enabled by {@link
103101
* #FLAG_ENABLE_CONSTANT_BITRATE_SEEKING} is implicitly enabled.
104-
*
105-
* <p>This flag is ignored if {@link #FLAG_ENABLE_INDEX_SEEKING} is set.
106102
*/
107103
public static final int FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS = 1 << 1;
108104

109105
/**
110-
* Flag to enable index seeking, in which a time-to-byte mapping is built as the file is read. If
111-
* other seeking metadata (e.g., Xing, VBRI, MLLT) is present in the file, it will be preferred
112-
* for performance reasons. This flag allows index seeking to be used as a fallback in cases where
113-
* no other seeking metadata is available.
106+
* Flag to enable index seeking, in which a time-to-byte mapping is built as the file is read.
107+
*
108+
* <p>This is used as a fallback in two cases:
109+
*
110+
* <ul>
111+
* <li>The file contains metadata indicating it is VBR (e.g. Xing, VBRI or MLLT frames), but
112+
* doesn't contain enough information to support VBR-aware seeking (e.g. table of contents).
113+
* <li>The file is CBR (either indicated by explicit metadata, or absence of any VBR metadata),
114+
* the length of the file (in bytes) is not known, and {@link
115+
* #FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS} is not set.
116+
* </ul>
114117
*
115118
* <p>This seeker may require to scan a significant portion of the file to compute a seek point.
116119
* Therefore, it should only be used if one of the following is true:
@@ -485,19 +488,22 @@ private Seeker computeSeeker(ExtractorInput input) throws IOException {
485488
resultSeeker = seekFrameSeeker;
486489
}
487490

488-
if ((flags & FLAG_ENABLE_INDEX_SEEKING) != 0
489-
&& (resultSeeker == null || !resultSeeker.isSeekable())) {
490-
long durationUs =
491-
resultSeeker != null ? resultSeeker.getDurationUs() : getId3TlenUs(metadata);
492-
long dataEndPosition =
493-
resultSeeker != null ? resultSeeker.getDataEndPosition() : C.INDEX_UNSET;
491+
if (resultSeeker == null) {
492+
// We must assume the file is CBR as we found no seek or VBR info.
493+
resultSeeker =
494+
getConstantBitrateSeeker(
495+
input, (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS) != 0);
496+
}
497+
498+
if ((flags & FLAG_ENABLE_INDEX_SEEKING) != 0 && !resultSeeker.isSeekable()) {
494499
resultSeeker =
495500
new IndexSeeker(
496-
durationUs, /* dataStartPosition= */ input.getPosition(), dataEndPosition);
501+
resultSeeker.getDurationUs(),
502+
/* dataStartPosition= */ input.getPosition(),
503+
resultSeeker.getDataEndPosition());
497504
}
498505

499-
if (resultSeeker != null
500-
&& shouldFallbackToConstantBitrateSeeking(resultSeeker)
506+
if (shouldFallbackToConstantBitrateSeeking(resultSeeker)
501507
&& resultSeeker.getDurationUs() != C.TIME_UNSET
502508
&& (resultSeeker.getDataEndPosition() != C.INDEX_UNSET
503509
|| input.getLength() != C.LENGTH_UNSET)) {
@@ -528,7 +534,7 @@ && shouldFallbackToConstantBitrateSeeking(resultSeeker)
528534
bitrate,
529535
C.LENGTH_UNSET,
530536
/* allowSeeksIfLengthUnknown= */ false);
531-
} else if (resultSeeker == null || shouldFallbackToConstantBitrateSeeking(resultSeeker)) {
537+
} else if (shouldFallbackToConstantBitrateSeeking(resultSeeker)) {
532538
// Either we found no seek or VBR info, so we must assume the file is CBR (even without the
533539
// flag(s) being set), or an 'enable CBR seeking flag' is set and we found some seek info, but
534540
// not enough to do 'enhanced' CBR seeking with. In either case, we fall back to CBR seeking
@@ -542,7 +548,9 @@ && shouldFallbackToConstantBitrateSeeking(resultSeeker)
542548
}
543549

544550
private boolean shouldFallbackToConstantBitrateSeeking(Seeker seeker) {
545-
return !seeker.isSeekable() && (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING) != 0;
551+
return !seeker.isSeekable()
552+
&& !(seeker instanceof ConstantBitrateSeeker)
553+
&& (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING) != 0;
546554
}
547555

548556
/**

‎libraries/extractor/src/test/java/androidx/media3/extractor/mp3/IndexSeekerTest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
@RunWith(AndroidJUnit4.class)
4040
public class IndexSeekerTest {
4141

42-
private static final String TEST_FILE_NO_SEEK_TABLE = "media/mp3/bear-vbr-no-seek-table.mp3";
43-
private static final int TEST_FILE_NO_SEEK_TABLE_DURATION = 2_808_000;
42+
private static final String TEST_FILE_XING_NO_TOC = "media/mp3/bear-vbr-xing-header-no-toc.mp3";
43+
private static final int TEST_FILE_XING_NO_TOC_DURATION = 2_808_000;
4444

4545
private Mp3Extractor extractor;
4646
private FakeExtractorOutput extractorOutput;
@@ -57,7 +57,7 @@ public void setUp() throws Exception {
5757

5858
@Test
5959
public void mp3ExtractorReads_returnsSeekableSeekMap() throws Exception {
60-
Uri fileUri = TestUtil.buildAssetUri(TEST_FILE_NO_SEEK_TABLE);
60+
Uri fileUri = TestUtil.buildAssetUri(TEST_FILE_XING_NO_TOC);
6161

6262
SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
6363

@@ -68,16 +68,16 @@ public void mp3ExtractorReads_returnsSeekableSeekMap() throws Exception {
6868
public void mp3ExtractorReads_correctsInexactDuration() throws Exception {
6969
FakeExtractorOutput extractorOutput =
7070
TestUtil.extractAllSamplesFromFile(
71-
extractor, ApplicationProvider.getApplicationContext(), TEST_FILE_NO_SEEK_TABLE);
71+
extractor, ApplicationProvider.getApplicationContext(), TEST_FILE_XING_NO_TOC);
7272

7373
SeekMap seekMap = extractorOutput.seekMap;
7474

75-
assertThat(seekMap.getDurationUs()).isEqualTo(TEST_FILE_NO_SEEK_TABLE_DURATION);
75+
assertThat(seekMap.getDurationUs()).isEqualTo(TEST_FILE_XING_NO_TOC_DURATION);
7676
}
7777

7878
@Test
7979
public void seeking_handlesSeekToZero() throws Exception {
80-
String fileName = TEST_FILE_NO_SEEK_TABLE;
80+
String fileName = TEST_FILE_XING_NO_TOC;
8181
Uri fileUri = TestUtil.buildAssetUri(fileName);
8282
SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
8383
FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
@@ -95,12 +95,12 @@ public void seeking_handlesSeekToZero() throws Exception {
9595

9696
@Test
9797
public void seeking_handlesSeekToEof() throws Exception {
98-
String fileName = TEST_FILE_NO_SEEK_TABLE;
98+
String fileName = TEST_FILE_XING_NO_TOC;
9999
Uri fileUri = TestUtil.buildAssetUri(fileName);
100100
SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
101101
FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
102102

103-
long targetSeekTimeUs = TEST_FILE_NO_SEEK_TABLE_DURATION;
103+
long targetSeekTimeUs = TEST_FILE_XING_NO_TOC_DURATION;
104104
int extractedFrameIndex =
105105
TestUtil.seekToTimeUs(
106106
extractor, seekMap, targetSeekTimeUs, dataSource, trackOutput, fileUri);
@@ -113,7 +113,7 @@ public void seeking_handlesSeekToEof() throws Exception {
113113

114114
@Test
115115
public void seeking_handlesSeekingBackward() throws Exception {
116-
String fileName = TEST_FILE_NO_SEEK_TABLE;
116+
String fileName = TEST_FILE_XING_NO_TOC;
117117
Uri fileUri = TestUtil.buildAssetUri(fileName);
118118
SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
119119
FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);
@@ -133,7 +133,7 @@ public void seeking_handlesSeekingBackward() throws Exception {
133133

134134
@Test
135135
public void seeking_handlesSeekingForward() throws Exception {
136-
String fileName = TEST_FILE_NO_SEEK_TABLE;
136+
String fileName = TEST_FILE_XING_NO_TOC;
137137
Uri fileUri = TestUtil.buildAssetUri(fileName);
138138
SeekMap seekMap = TestUtil.extractSeekMap(extractor, extractorOutput, dataSource, fileUri);
139139
FakeTrackOutput trackOutput = extractorOutput.trackOutputs.get(0);

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package androidx.media3.extractor.mp3;
1717

18+
import static com.google.common.collect.ImmutableList.toImmutableList;
1819
import static com.google.common.truth.Truth.assertThat;
1920
import static org.junit.Assume.assumeFalse;
2021

@@ -29,7 +30,9 @@
2930
import androidx.media3.test.utils.TestUtil;
3031
import androidx.test.core.app.ApplicationProvider;
3132
import com.google.common.base.Ascii;
33+
import com.google.common.collect.ImmutableList;
3234
import com.google.testing.junit.testparameterinjector.TestParameter;
35+
import com.google.testing.junit.testparameterinjector.TestParameterValuesProvider;
3336
import org.junit.Test;
3437
import org.junit.runner.RunWith;
3538
import org.robolectric.RobolectricTestParameterInjector;
@@ -185,9 +188,19 @@ public void mp3SampleWithCbrSeeker(
185188
simulationConfig);
186189
}
187190

191+
private static final class KnowLengthConfigProvider extends TestParameterValuesProvider {
192+
@Override
193+
protected ImmutableList<ExtractorAsserts.SimulationConfig> provideValues(
194+
TestParameterValuesProvider.Context context) {
195+
return ExtractorAsserts.configs().stream()
196+
.filter(config -> !config.simulateUnknownLength)
197+
.collect(toImmutableList());
198+
}
199+
}
200+
188201
@Test
189-
public void mp3SampleWithIndexSeeker(
190-
@TestParameter(valuesProvider = ExtractorAsserts.ConfigProvider.class)
202+
public void mp3Sample_withIndexSeekingFlag_usesCbrSeekerForKnownLength(
203+
@TestParameter(valuesProvider = KnowLengthConfigProvider.class)
191204
ExtractorAsserts.SimulationConfig simulationConfig)
192205
throws Exception {
193206
ExtractorAsserts.assertBehavior(

‎libraries/test_data/src/test/assets/extractordumps/mp3/bear-vbr-no-seek-table.mp3.0.dump‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
seekMap:
22
isSeekable = true
3-
duration = 2808000
3+
duration = 9540000
44
getPosition(0) = [[timeUs=0, position=224]]
5-
getPosition(1) = [[timeUs=0, position=224], [timeUs=120000, position=2048]]
6-
getPosition(1404000) = [[timeUs=1320000, position=18896], [timeUs=1440000, position=20528]]
7-
getPosition(2808000) = [[timeUs=2760000, position=38168]]
5+
getPosition(1) = [[timeUs=0, position=224], [timeUs=24000, position=320]]
6+
getPosition(4770000) = [[timeUs=4752000, position=19232], [timeUs=4776000, position=19328]]
7+
getPosition(9540000) = [[timeUs=9516000, position=38288]]
88
numberOfTracks = 1
99
track 0:
1010
total output bytes = 38160
1111
sample count = 117
12-
track duration = 2808000
12+
track duration = 9540000
1313
format 0:
14+
averageBitrate = 32000
1415
containerMimeType = audio/mpeg
1516
sampleMimeType = audio/mpeg
1617
maxInputSize = 4096

0 commit comments

Comments
 (0)