From ba8c22aba521b46656c5ce580f40c8b6278440ae Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 11 Sep 2018 04:50:13 -0700 Subject: [PATCH] Make seeking when there's a pending reset more obviously correct If there's already a pending reset, we need only update the pending reset position. We can currently end up calling cancelLoading more than once on the loader for the same task, which is hard to reason about (although I don't think anything actually broke in practice). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=212432266 --- .../source/ExtractorMediaPeriod.java | 15 +++- .../source/chunk/ChunkSampleStream.java | 74 ++++++++++--------- .../source/hls/HlsSampleStreamWrapper.java | 13 +++- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java index 7e44322f20..895e4fcf9f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java @@ -368,15 +368,22 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; boolean[] trackIsAudioVideoFlags = preparedState.trackIsAudioVideoFlags; // Treat all seeks into non-seekable media as being to t=0. positionUs = seekMap.isSeekable() ? positionUs : 0; - lastSeekPositionUs = positionUs; + notifyDiscontinuity = false; - // If we're not playing a live stream or pending a reset, see if we can seek within the buffer. + lastSeekPositionUs = positionUs; + if (isPendingReset()) { + // A reset is already pending. We only need to update its position. + pendingResetPositionUs = positionUs; + return positionUs; + } + + // If we're not playing a live stream, try and seek within the buffer. if (dataType != C.DATA_TYPE_MEDIA_PROGRESSIVE_LIVE - && !isPendingReset() && seekInsideBufferUs(trackIsAudioVideoFlags, positionUs)) { return positionUs; } - // We were unable to seek within the buffer, so need to reset. + + // We can't seek inside the buffer, and so need to reset. pendingDeferredRetry = false; pendingResetPositionUs = positionUs; loadingFinished = false; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java index 6fc37ff501..5e7d87853e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/chunk/ChunkSampleStream.java @@ -274,52 +274,54 @@ public class ChunkSampleStream implements SampleStream, S */ public void seekToUs(long positionUs) { lastSeekPositionUs = positionUs; - primarySampleQueue.rewind(); - - // See if we can seek within the primary sample queue. - boolean seekInsideBuffer; if (isPendingReset()) { - seekInsideBuffer = false; - } else { - // Detect whether the seek is to the start of a chunk that's at least partially buffered. - BaseMediaChunk seekToMediaChunk = null; - for (int i = 0; i < mediaChunks.size(); i++) { - BaseMediaChunk mediaChunk = mediaChunks.get(i); - long mediaChunkStartTimeUs = mediaChunk.startTimeUs; - if (mediaChunkStartTimeUs == positionUs && mediaChunk.seekTimeUs == C.TIME_UNSET) { - seekToMediaChunk = mediaChunk; - break; - } else if (mediaChunkStartTimeUs > positionUs) { - // We're not going to find a chunk with a matching start time. - break; - } - } - if (seekToMediaChunk != null) { - // When seeking to the start of a chunk we use the index of the first sample in the chunk - // rather than the seek position. This ensures we seek to the keyframe at the start of the - // chunk even if the sample timestamps are slightly offset from the chunk start times. - seekInsideBuffer = - primarySampleQueue.setReadPosition(seekToMediaChunk.getFirstSampleIndex(0)); - decodeOnlyUntilPositionUs = Long.MIN_VALUE; - } else { - seekInsideBuffer = - primarySampleQueue.advanceTo( - positionUs, - /* toKeyframe= */ true, - /* allowTimeBeyondBuffer= */ positionUs < getNextLoadPositionUs()) - != SampleQueue.ADVANCE_FAILED; - decodeOnlyUntilPositionUs = lastSeekPositionUs; + // A reset is already pending. We only need to update its position. + pendingResetPositionUs = positionUs; + return; + } + + // Detect whether the seek is to the start of a chunk that's at least partially buffered. + BaseMediaChunk seekToMediaChunk = null; + for (int i = 0; i < mediaChunks.size(); i++) { + BaseMediaChunk mediaChunk = mediaChunks.get(i); + long mediaChunkStartTimeUs = mediaChunk.startTimeUs; + if (mediaChunkStartTimeUs == positionUs && mediaChunk.seekTimeUs == C.TIME_UNSET) { + seekToMediaChunk = mediaChunk; + break; + } else if (mediaChunkStartTimeUs > positionUs) { + // We're not going to find a chunk with a matching start time. + break; } } + // See if we can seek inside the primary sample queue. + boolean seekInsideBuffer; + primarySampleQueue.rewind(); + if (seekToMediaChunk != null) { + // When seeking to the start of a chunk we use the index of the first sample in the chunk + // rather than the seek position. This ensures we seek to the keyframe at the start of the + // chunk even if the sample timestamps are slightly offset from the chunk start times. + seekInsideBuffer = + primarySampleQueue.setReadPosition(seekToMediaChunk.getFirstSampleIndex(0)); + decodeOnlyUntilPositionUs = Long.MIN_VALUE; + } else { + seekInsideBuffer = + primarySampleQueue.advanceTo( + positionUs, + /* toKeyframe= */ true, + /* allowTimeBeyondBuffer= */ positionUs < getNextLoadPositionUs()) + != SampleQueue.ADVANCE_FAILED; + decodeOnlyUntilPositionUs = lastSeekPositionUs; + } + if (seekInsideBuffer) { - // We succeeded. Advance the embedded sample queues to the seek position. + // We can seek inside the buffer. Advance the embedded sample queues to the seek position. for (SampleQueue embeddedSampleQueue : embeddedSampleQueues) { embeddedSampleQueue.rewind(); embeddedSampleQueue.advanceTo(positionUs, true, false); } } else { - // We failed, and need to restart. + // We can't seek inside the buffer, and so need to reset. pendingResetPositionUs = positionUs; loadingFinished = false; mediaChunks.clear(); diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java index f25f4312ab..e082ee3e64 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsSampleStreamWrapper.java @@ -386,11 +386,18 @@ import java.util.List; */ public boolean seekToUs(long positionUs, boolean forceReset) { lastSeekPositionUs = positionUs; - // If we're not forced to reset nor have a pending reset, see if we can seek within the buffer. - if (sampleQueuesBuilt && !forceReset && !isPendingReset() && seekInsideBufferUs(positionUs)) { + if (isPendingReset()) { + // A reset is already pending. We only need to update its position. + pendingResetPositionUs = positionUs; + return true; + } + + // If we're not forced to reset, try and seek within the buffer. + if (sampleQueuesBuilt && !forceReset && seekInsideBufferUs(positionUs)) { return false; } - // We were unable to seek within the buffer, so need to reset. + + // We can't seek inside the buffer, and so need to reset. pendingResetPositionUs = positionUs; loadingFinished = false; mediaChunks.clear();