From ca51ed649baec60837a7cec8f46ce513cf4622e2 Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 12 Jun 2024 10:05:45 -0700 Subject: [PATCH] Fix single sample handling ProgressiveMediaPeriod when disabling tracks When deselecting the single sample track and later re-selecting this track, the current shortcuts in ProgressiveMediaPeriod don't handle this case correctly and cause assertion failures. In particular, this change fixes 3 issues: 1. When re-selecting the single sample track, we have cleared the SampleQueue and need to reload the sample. The existing shortcut should only be applied to avoid the reload when starting from a non-zero position. 2. When de-selecting the track, ProgressiveMediaPeriod is left in an inconsistent state where the sample queues are empty but loadingFinished is still true. Fix this by resetting loadingFinished to false. 3. When seeking, we avoid reloading the stream if we can keep inside the existing samples. This logic assumes that all remaining samples will continue to be loaded in the queue. This condition isn't true though for single sample tracks that have been de-selected. They appear to support the seek inside the queue (=no seek necessary, always supported), but still require a new load if there is no ongoing one to load the sample. Fix this by checking this implicit assumption (still loading, or loading finished). PiperOrigin-RevId: 642650248 --- .../exoplayer/source/ProgressiveMediaPeriod.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java index 6e2672525d..444d2168d0 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java @@ -276,11 +276,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; streams[i] = null; } } - // We'll always need to seek if this is a first selection to a non-zero position, or if we're - // making a selection having previously disabled all tracks, except for when we have a single - // sample. + // We'll always need to seek if this is a first selection to a non-zero position (except for + // when we have a single sample only), or if we're making a selection having previously + // disabled all tracks. boolean seekRequired = - !isSingleSample && (seenFirstTrackSelection ? oldEnabledTrackCount == 0 : positionUs != 0); + seenFirstTrackSelection ? oldEnabledTrackCount == 0 : positionUs != 0 && !isSingleSample; // Select new tracks. for (int i = 0; i < selections.length; i++) { if (streams[i] == null && selections[i] != null) { @@ -315,6 +315,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } loader.cancelLoading(); } else { + loadingFinished = false; for (SampleQueue sampleQueue : sampleQueues) { sampleQueue.reset(); } @@ -434,8 +435,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return positionUs; } - // If we're not playing a live stream, try and seek within the buffer. + // If we're not playing a live stream, and when loading will continue (or has finished), try + // and seek within the existing buffer instead of restarting the load. if (dataType != C.DATA_TYPE_MEDIA_PROGRESSIVE_LIVE + && (loadingFinished || loader.isLoading()) && seekInsideBufferUs(trackIsAudioVideoFlags, positionUs)) { return positionUs; }