Skip to content

Commit c7b11aa

Browse files
icbakercopybara-github
authored andcommitted
Refactor Mp3Extractor.computeSeeker control flow
Consolidate CBR & index-seeking fallback logic and improve control flow in `computeSeeker` by using early returns and guard clauses. This change moves the side effect of setting `durationUs` on `realTrackOutput` to the callsite in `readInternal`, to allow the early returns. This came up when I was reviewing Issue: #3198 which modifies this logic a bit. PiperOrigin-RevId: 929301753
1 parent f6ad726 commit c7b11aa

1 file changed

Lines changed: 52 additions & 60 deletions

File tree

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

Lines changed: 52 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ private int readInternal(ExtractorInput input) throws IOException {
296296
}
297297
if (seeker == null) {
298298
seeker = computeSeeker(input);
299+
realTrackOutput.durationUs(seeker.getDurationUs());
299300
extractorOutput.seekMap(seeker);
300301
Metadata metadata;
301302
if (id3Metadata != null && (flags & FLAG_DISABLE_ID3_METADATA) == 0) {
@@ -486,7 +487,6 @@ private boolean peekEndOfStreamOrHeader(ExtractorInput extractorInput) throws IO
486487
}
487488
}
488489

489-
@RequiresNonNull("realTrackOutput")
490490
private Seeker computeSeeker(ExtractorInput input) throws IOException {
491491
// Read past any seek frame and set the seeker based on metadata or a seek frame. Metadata
492492
// takes priority as it can provide greater precision.
@@ -497,76 +497,65 @@ private Seeker computeSeeker(ExtractorInput input) throws IOException {
497497
return new UnseekableSeeker();
498498
}
499499

500-
@Nullable Seeker resultSeeker = null;
500+
Seeker resultSeeker;
501501
if (metadataSeeker != null) {
502502
resultSeeker = metadataSeeker;
503503
} else if (seekFrameSeeker != null) {
504504
resultSeeker = seekFrameSeeker;
505+
} else {
506+
resultSeeker = getConstantBitrateSeeker(input);
505507
}
506508

507-
if (resultSeeker == null) {
508-
// We must assume the file is CBR as we found no seek or VBR info.
509-
resultSeeker =
510-
getConstantBitrateSeeker(
511-
input, (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS) != 0);
509+
if ((flags & FLAG_ENABLE_INDEX_SEEKING) != 0 && !resultSeeker.isSeekable()) {
510+
return new IndexSeeker(
511+
resultSeeker.getDurationUs(),
512+
/* dataStartPosition= */ input.getPosition(),
513+
resultSeeker.getDataEndPosition());
512514
}
513515

514-
if ((flags & FLAG_ENABLE_INDEX_SEEKING) != 0 && !resultSeeker.isSeekable()) {
515-
resultSeeker =
516-
new IndexSeeker(
517-
resultSeeker.getDurationUs(),
518-
/* dataStartPosition= */ input.getPosition(),
519-
resultSeeker.getDataEndPosition());
516+
if (resultSeeker instanceof ConstantBitrateSeeker) {
517+
return resultSeeker;
518+
}
519+
520+
if (!shouldFallbackToConstantBitrateSeeking(resultSeeker)) {
521+
return resultSeeker;
520522
}
521523

522-
if (shouldFallbackToConstantBitrateSeeking(resultSeeker)
523-
&& resultSeeker.getDurationUs() != C.TIME_UNSET
524-
&& (resultSeeker.getDataEndPosition() != C.INDEX_UNSET
525-
|| input.getLength() != C.LENGTH_UNSET)) {
526-
// resultSeeker does not allow seeking, but does provide a duration and constant bitrate
527-
// seeking has been requested, so we can do 'enhanced' CBR seeking using this duration info.
528-
long dataStart =
529-
resultSeeker.getDataStartPosition() != C.INDEX_UNSET
530-
? resultSeeker.getDataStartPosition()
531-
: 0;
532-
long inputLength =
533-
resultSeeker.getDataEndPosition() != C.INDEX_UNSET
534-
? resultSeeker.getDataEndPosition()
535-
: input.getLength();
536-
long audioLength = inputLength - dataStart;
537-
int bitrate =
538-
Ints.saturatedCast(
539-
Util.scaleLargeValue(
540-
audioLength,
541-
Byte.SIZE * C.MICROS_PER_SECOND,
542-
resultSeeker.getDurationUs(),
543-
RoundingMode.HALF_UP));
544-
// inputLength will never be LENGTH_UNSET because of the outer if-condition, so we can pass
545-
// (vacuously) false here for allowSeeksIfLengthUnknown.
546-
resultSeeker =
547-
new ConstantBitrateSeeker(
548-
inputLength,
549-
dataStart,
550-
bitrate,
551-
C.LENGTH_UNSET,
552-
/* allowSeeksIfLengthUnknown= */ false);
553-
} else if (shouldFallbackToConstantBitrateSeeking(resultSeeker)) {
554-
// Either we found no seek or VBR info, so we must assume the file is CBR (even without the
555-
// flag(s) being set), or an 'enable CBR seeking flag' is set and we found some seek info, but
556-
// not enough to do 'enhanced' CBR seeking with. In either case, we fall back to CBR seeking
557-
// without any additional info from the file.
558-
resultSeeker =
559-
getConstantBitrateSeeker(
560-
input, (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS) != 0);
524+
long inputLength =
525+
resultSeeker.getDataEndPosition() != C.INDEX_UNSET
526+
? resultSeeker.getDataEndPosition()
527+
: input.getLength();
528+
if (resultSeeker.getDurationUs() == C.TIME_UNSET || inputLength == C.LENGTH_UNSET) {
529+
// resultSeeker doesn't provide enough info to do 'enhanced' CBR seeking, so we just do
530+
// normal CBR seeking without any additional info from the file.
531+
return getConstantBitrateSeeker(input);
561532
}
562-
realTrackOutput.durationUs(resultSeeker.getDurationUs());
563-
return resultSeeker;
533+
// resultSeeker provides a duration and we know the input length, and CBR seeking has been
534+
// requested, so we can do 'enhanced' CBR seeking using this info.
535+
long dataStart =
536+
resultSeeker.getDataStartPosition() != C.INDEX_UNSET
537+
? resultSeeker.getDataStartPosition()
538+
: 0;
539+
long audioLength = inputLength - dataStart;
540+
int bitrate =
541+
Ints.saturatedCast(
542+
Util.scaleLargeValue(
543+
audioLength,
544+
Byte.SIZE * C.MICROS_PER_SECOND,
545+
resultSeeker.getDurationUs(),
546+
RoundingMode.HALF_UP));
547+
// inputLength will never be LENGTH_UNSET because of the if-condition above, so we can
548+
// pass (vacuously) false here for allowSeeksIfLengthUnknown.
549+
return new ConstantBitrateSeeker(
550+
inputLength,
551+
dataStart,
552+
bitrate,
553+
/* frameSize= */ C.LENGTH_UNSET,
554+
/* allowSeeksIfLengthUnknown= */ false);
564555
}
565556

566557
private boolean shouldFallbackToConstantBitrateSeeking(Seeker seeker) {
567-
return !seeker.isSeekable()
568-
&& !(seeker instanceof ConstantBitrateSeeker)
569-
&& (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING) != 0;
558+
return !seeker.isSeekable() && (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING) != 0;
570559
}
571560

572561
/**
@@ -627,13 +616,16 @@ private Seeker maybeReadSeekFrame(ExtractorInput input) throws IOException {
627616
}
628617

629618
/** Peeks the next frame and returns a {@link ConstantBitrateSeeker} based on its bitrate. */
630-
private Seeker getConstantBitrateSeeker(ExtractorInput input, boolean allowSeeksIfLengthUnknown)
631-
throws IOException {
619+
private Seeker getConstantBitrateSeeker(ExtractorInput input) throws IOException {
632620
input.peekFully(scratch.getData(), 0, 4);
633621
scratch.setPosition(0);
634622
synchronizedHeader.setForHeaderData(scratch.readInt());
635623
return new ConstantBitrateSeeker(
636-
input.getLength(), input.getPosition(), synchronizedHeader, allowSeeksIfLengthUnknown);
624+
input.getLength(),
625+
input.getPosition(),
626+
synchronizedHeader,
627+
/* allowSeeksIfLengthUnknown= */ (flags & FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS)
628+
!= 0);
637629
}
638630

639631
/**

0 commit comments

Comments
 (0)