diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 32a412ed67..e93d336ab9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -1040,10 +1040,17 @@ import java.util.concurrent.atomic.AtomicBoolean; if (!newPlayingPeriodHolder.prepared) { newPlayingPeriodHolder.info = newPlayingPeriodHolder.info.copyWithStartPositionUs(periodPositionUs); - } else if (newPlayingPeriodHolder.hasEnabledTracks) { - periodPositionUs = newPlayingPeriodHolder.mediaPeriod.seekToUs(periodPositionUs); - newPlayingPeriodHolder.mediaPeriod.discardBuffer( - periodPositionUs - backBufferDurationUs, retainBackBufferFromKeyframe); + } else { + if (newPlayingPeriodHolder.info.durationUs != C.TIME_UNSET + && periodPositionUs >= newPlayingPeriodHolder.info.durationUs) { + // Make sure seek position doesn't exceed period duration. + periodPositionUs = Math.max(0, newPlayingPeriodHolder.info.durationUs - 1); + } + if (newPlayingPeriodHolder.hasEnabledTracks) { + periodPositionUs = newPlayingPeriodHolder.mediaPeriod.seekToUs(periodPositionUs); + newPlayingPeriodHolder.mediaPeriod.discardBuffer( + periodPositionUs - backBufferDurationUs, retainBackBufferFromKeyframe); + } } resetRendererPosition(periodPositionUs); maybeContinueLoading(); @@ -1862,7 +1869,7 @@ import java.util.concurrent.atomic.AtomicBoolean; playbackInfo = handlePositionDiscontinuity( playbackInfo.periodId, - playbackInfo.positionUs, + loadingPeriodHolder.info.startPositionUs, playbackInfo.requestedContentPositionUs); } maybeContinueLoading(); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java index 5bbbcbea2a..f1ea4b4751 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodHolder.java @@ -171,9 +171,14 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; prepared = true; trackGroups = mediaPeriod.getTrackGroups(); TrackSelectorResult selectorResult = selectTracks(playbackSpeed, timeline); + long requestedStartPositionUs = info.startPositionUs; + if (info.durationUs != C.TIME_UNSET && requestedStartPositionUs >= info.durationUs) { + // Make sure start position doesn't exceed period duration. + requestedStartPositionUs = Math.max(0, info.durationUs - 1); + } long newStartPositionUs = applyTrackSelection( - selectorResult, info.startPositionUs, /* forceRecreateStreams= */ false); + selectorResult, requestedStartPositionUs, /* forceRecreateStreams= */ false); rendererPositionOffsetUs += info.startPositionUs - newStartPositionUs; info = info.copyWithStartPositionUs(newStartPositionUs); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java index fe5863cbfd..4df879d5d8 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java @@ -730,6 +730,10 @@ import com.google.android.exoplayer2.util.Assertions; adIndexInAdGroup == period.getFirstAdIndexToPlay(adGroupIndex) ? period.getAdResumePositionUs() : 0; + if (durationUs != C.TIME_UNSET && startPositionUs >= durationUs) { + // Ensure start position doesn't exceed duration. + startPositionUs = Math.max(0, durationUs - 1); + } return new MediaPeriodInfo( id, startPositionUs, @@ -761,6 +765,10 @@ import com.google.android.exoplayer2.util.Assertions; endPositionUs == C.TIME_UNSET || endPositionUs == C.TIME_END_OF_SOURCE ? period.durationUs : endPositionUs; + if (durationUs != C.TIME_UNSET && startPositionUs >= durationUs) { + // Ensure start position doesn't exceed duration. + startPositionUs = Math.max(0, durationUs - 1); + } return new MediaPeriodInfo( id, startPositionUs, diff --git a/library/core/src/main/java/com/google/android/exoplayer2/Renderer.java b/library/core/src/main/java/com/google/android/exoplayer2/Renderer.java index 672cfd3b9b..4ae0c0e111 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/Renderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/Renderer.java @@ -369,16 +369,10 @@ public interface Renderer extends PlayerMessage.Target { * {@link SampleStream} in sync with the specified media positions. * *

The renderer may also render the very start of the media at the current position (e.g. the - * first frame of a video stream) while still in the {@link #STATE_ENABLED} state. It's not - * allowed to do that in the following two cases: - * - *

    - *
  1. The initial start of the media after calling {@link #enable(RendererConfiguration, - * Format[], SampleStream, long, boolean, boolean, long)} with {@code - * mayRenderStartOfStream} set to {@code false}. - *
  2. The start of a new stream after calling {@link #replaceStream(Format[], SampleStream, - * long)}. - *
+ * first frame of a video stream) while still in the {@link #STATE_ENABLED} state, unless it's the + * initial start of the media after calling {@link #enable(RendererConfiguration, Format[], + * SampleStream, long, boolean, boolean, long)} with {@code mayRenderStartOfStream} set to {@code + * false}. * *

This method should return quickly, and should not block if the renderer is unable to make * useful progress. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java index 17ac6c0667..142527af7d 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java @@ -91,8 +91,8 @@ public final class MaskingMediaPeriod implements MediaPeriod, MediaPeriod.Callba } /** - * Overrides the default prepare position at which to prepare the media period. This value is only - * used if called before {@link #createPeriod(MediaPeriodId)}. + * Overrides the default prepare position at which to prepare the media period. This method must + * be called before {@link #createPeriod(MediaPeriodId)}. * * @param preparePositionUs The default prepare position to use, in microseconds. */ @@ -100,6 +100,11 @@ public final class MaskingMediaPeriod implements MediaPeriod, MediaPeriod.Callba preparePositionOverrideUs = preparePositionUs; } + /** Returns the prepare position override set by {@link #overridePreparePositionUs(long)}. */ + public long getPreparePositionOverrideUs() { + return preparePositionOverrideUs; + } + /** * Calls {@link MediaSource#createPeriod(MediaPeriodId, Allocator, long)} on the wrapped source * then prepares it if {@link #prepare(Callback, long)} has been called. Call {@link diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java index 657c717ed5..28d9369c83 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java @@ -26,6 +26,7 @@ import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; +import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** * A {@link MediaSource} that masks the {@link Timeline} with a placeholder until the actual media @@ -143,6 +144,11 @@ public final class MaskingMediaSource extends CompositeMediaSource { @Nullable MediaPeriodId idForMaskingPeriodPreparation = null; if (isPrepared) { timeline = timeline.cloneWithUpdatedTimeline(newTimeline); + if (unpreparedMaskingMediaPeriod != null) { + // Reset override in case the duration changed and we need to update our override. + setPreparePositionOverrideToUnpreparedMaskingPeriod( + unpreparedMaskingMediaPeriod.getPreparePositionOverrideUs()); + } } else if (newTimeline.isEmpty()) { timeline = hasRealTimeline @@ -182,7 +188,7 @@ public final class MaskingMediaSource extends CompositeMediaSource { : MaskingTimeline.createWithRealTimeline(newTimeline, windowUid, periodUid); if (unpreparedMaskingMediaPeriod != null) { MaskingMediaPeriod maskingPeriod = unpreparedMaskingMediaPeriod; - maskingPeriod.overridePreparePositionUs(periodPositionUs); + setPreparePositionOverrideToUnpreparedMaskingPeriod(periodPositionUs); idForMaskingPeriodPreparation = maskingPeriod.id.copyWithPeriodUid(getInternalPeriodUid(maskingPeriod.id.periodUid)); } @@ -225,6 +231,19 @@ public final class MaskingMediaSource extends CompositeMediaSource { : internalPeriodUid; } + @RequiresNonNull("unpreparedMaskingMediaPeriod") + private void setPreparePositionOverrideToUnpreparedMaskingPeriod(long preparePositionOverrideUs) { + MaskingMediaPeriod maskingPeriod = unpreparedMaskingMediaPeriod; + long periodDurationUs = timeline.getPeriodByUid(maskingPeriod.id.periodUid, period).durationUs; + if (periodDurationUs != C.TIME_UNSET) { + // Ensure the overridden position doesn't exceed the period duration. + if (preparePositionOverrideUs >= periodDurationUs) { + preparePositionOverrideUs = Math.max(0, periodDurationUs - 1); + } + } + maskingPeriod.overridePreparePositionUs(preparePositionOverrideUs); + } + /** * Timeline used as placeholder for an unprepared media source. After preparation, a * MaskingTimeline is used to keep the originally assigned dummy period ID. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRenderer.java index 44d019f57b..fd89c6574e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRenderer.java @@ -275,7 +275,7 @@ public abstract class SimpleDecoderVideoRenderer extends BaseRenderer { protected void onStreamChanged(Format[] formats, long offsetUs) throws ExoPlaybackException { // TODO: This shouldn't just update the output stream offset as long as there are still buffers // of the previous stream in the decoder. It should also make sure to render the first frame of - // the next stream if the playback position reached the new stream and the renderer is started. + // the next stream if the playback position reached the new stream. outputStreamOffsetUs = offsetUs; super.onStreamChanged(formats, offsetUs); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index 6532734808..e951c67725 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -975,6 +975,70 @@ public final class ExoPlayerTest { .blockUntilEnded(TIMEOUT_MS); } + @Test + public void seekBeforePreparationCompletes_seeksToCorrectPosition() throws Exception { + CountDownLatch createPeriodCalledCountDownLatch = new CountDownLatch(1); + FakeMediaPeriod[] fakeMediaPeriodHolder = new FakeMediaPeriod[1]; + Timeline timeline = new FakeTimeline(/* windowCount= */ 1); + FakeMediaSource mediaSource = + new FakeMediaSource(/* timeline= */ null, Builder.VIDEO_FORMAT) { + @Override + protected FakeMediaPeriod createFakeMediaPeriod( + MediaPeriodId id, + TrackGroupArray trackGroupArray, + Allocator allocator, + EventDispatcher eventDispatcher, + @Nullable TransferListener transferListener) { + // Defer completing preparation of the period until seek has been sent. + fakeMediaPeriodHolder[0] = + new FakeMediaPeriod(trackGroupArray, eventDispatcher, /* deferOnPrepared= */ true); + createPeriodCalledCountDownLatch.countDown(); + return fakeMediaPeriodHolder[0]; + } + }; + AtomicLong positionWhenReady = new AtomicLong(); + ActionSchedule actionSchedule = + new ActionSchedule.Builder(TAG) + .pause() + .waitForPlaybackState(Player.STATE_BUFFERING) + // Ensure we use the MaskingMediaPeriod by delaying the initial timeline update. + .delay(1) + .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline)) + .waitForTimelineChanged() + // Block until createPeriod has been called on the fake media source. + .executeRunnable( + () -> { + try { + createPeriodCalledCountDownLatch.await(); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + }) + // Seek before preparation completes. + .seek(5000) + // Complete preparation of the fake media period. + .executeRunnable(() -> fakeMediaPeriodHolder[0].setPreparationComplete()) + .waitForPlaybackState(Player.STATE_READY) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + positionWhenReady.set(player.getCurrentPosition()); + } + }) + .play() + .build(); + new ExoPlayerTestRunner.Builder() + .initialSeek(/* windowIndex= */ 0, /* positionMs= */ 2000) + .setMediaSources(mediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(positionWhenReady.get()).isAtLeast(5000); + } + @Test public void stopDoesNotResetPosition() throws Exception { Timeline timeline = new FakeTimeline(/* windowCount= */ 1); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java index 4c65e9d546..5b3b321e09 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/MediaPeriodQueueTest.java @@ -184,7 +184,7 @@ public final class MediaPeriodQueueTest { advance(); assertGetNextMediaPeriodInfoReturnsContentMediaPeriod( /* periodUid= */ firstPeriodUid, - /* startPositionUs= */ CONTENT_DURATION_US, + /* startPositionUs= */ CONTENT_DURATION_US - 1, /* requestedContentPositionUs= */ CONTENT_DURATION_US, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, @@ -209,7 +209,7 @@ public final class MediaPeriodQueueTest { setAdGroupFailedToLoad(/* adGroupIndex= */ 0); assertGetNextMediaPeriodInfoReturnsContentMediaPeriod( /* periodUid= */ firstPeriodUid, - /* startPositionUs= */ CONTENT_DURATION_US, + /* startPositionUs= */ CONTENT_DURATION_US - 1, /* requestedContentPositionUs= */ CONTENT_DURATION_US, /* endPositionUs= */ C.TIME_UNSET, /* durationUs= */ CONTENT_DURATION_US, diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java index bba81c7776..f779b70637 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java @@ -674,13 +674,14 @@ public final class AnalyticsCollectorTest { assertThat(listener.getEvents(EVENT_DECODER_FORMAT_CHANGED)) .containsExactly(window0Period1Seq0, window1Period0Seq1); assertThat(listener.getEvents(EVENT_DECODER_DISABLED)).containsExactly(window0Period1Seq0); - assertThat(listener.getEvents(EVENT_DROPPED_VIDEO_FRAMES)).containsExactly(window0Period1Seq0); + assertThat(listener.getEvents(EVENT_DROPPED_VIDEO_FRAMES)) + .containsExactly(window0Period1Seq0, period1Seq0); assertThat(listener.getEvents(EVENT_VIDEO_SIZE_CHANGED)) .containsExactly(window0Period1Seq0, window1Period0Seq1, period1Seq0); assertThat(listener.getEvents(EVENT_RENDERED_FIRST_FRAME)) .containsExactly(window0Period1Seq0, window1Period0Seq1, period1Seq0); assertThat(listener.getEvents(EVENT_VIDEO_FRAME_PROCESSING_OFFSET)) - .containsExactly(window0Period1Seq0); + .containsExactly(window0Period1Seq0, period1Seq0); listener.assertNoMoreEvents(); } @@ -1216,8 +1217,8 @@ public final class AnalyticsCollectorTest { private Format format; private long streamOffsetUs; private boolean renderedFirstFrameAfterReset; - private boolean mayRenderFirstFrameAfterStreamChangeIfNotStarted; - private boolean renderedFirstFrameAfterStreamChange; + private boolean mayRenderFirstFrameAfterEnableIfNotStarted; + private boolean renderedFirstFrameAfterEnable; public FakeVideoRenderer(Handler handler, VideoRendererEventListener eventListener) { super(ExoPlayerTestRunner.Builder.VIDEO_FORMAT); @@ -1230,8 +1231,8 @@ public final class AnalyticsCollectorTest { throws ExoPlaybackException { super.onEnabled(joining, mayRenderStartOfStream); eventDispatcher.enabled(decoderCounters); - mayRenderFirstFrameAfterStreamChangeIfNotStarted = mayRenderStartOfStream; - renderedFirstFrameAfterStreamChange = false; + mayRenderFirstFrameAfterEnableIfNotStarted = mayRenderStartOfStream; + renderedFirstFrameAfterEnable = false; } @Override @@ -1240,8 +1241,6 @@ public final class AnalyticsCollectorTest { streamOffsetUs = offsetUs; if (renderedFirstFrameAfterReset) { renderedFirstFrameAfterReset = false; - renderedFirstFrameAfterStreamChange = false; - mayRenderFirstFrameAfterStreamChangeIfNotStarted = false; } } @@ -1279,9 +1278,8 @@ public final class AnalyticsCollectorTest { protected boolean shouldProcessBuffer(long bufferTimeUs, long playbackPositionUs) { boolean shouldProcess = super.shouldProcessBuffer(bufferTimeUs, playbackPositionUs); boolean shouldRenderFirstFrame = - !renderedFirstFrameAfterStreamChange - ? (getState() == Renderer.STATE_STARTED - || mayRenderFirstFrameAfterStreamChangeIfNotStarted) + !renderedFirstFrameAfterEnable + ? (getState() == Renderer.STATE_STARTED || mayRenderFirstFrameAfterEnableIfNotStarted) : !renderedFirstFrameAfterReset; shouldProcess |= shouldRenderFirstFrame && playbackPositionUs >= streamOffsetUs; if (shouldProcess && !renderedFirstFrameAfterReset) { @@ -1289,7 +1287,7 @@ public final class AnalyticsCollectorTest { format.width, format.height, format.rotationDegrees, format.pixelWidthHeightRatio); eventDispatcher.renderedFirstFrame(/* surface= */ null); renderedFirstFrameAfterReset = true; - renderedFirstFrameAfterStreamChange = true; + renderedFirstFrameAfterEnable = true; } return shouldProcess; } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRendererTest.java b/library/core/src/test/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRendererTest.java index 8e972bd22e..af98fd9ebf 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRendererTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/video/SimpleDecoderVideoRendererTest.java @@ -255,7 +255,7 @@ public final class SimpleDecoderVideoRendererTest { verify(eventListener).onRenderedFirstFrame(any()); } - // TODO: First frame of replaced stream are not yet reported. + // TODO: Fix rendering of first frame at stream transition. @Ignore @Test public void replaceStream_whenStarted_rendersFirstFrameOfNewStream() throws Exception { @@ -286,7 +286,7 @@ public final class SimpleDecoderVideoRendererTest { renderer.start(); boolean replacedStream = false; - for (int i = 0; i < 200; i += 10) { + for (int i = 0; i <= 10; i++) { renderer.render(/* positionUs= */ i * 10, SystemClock.elapsedRealtime() * 1000); if (!replacedStream && renderer.hasReadStreamToEnd()) { renderer.replaceStream(new Format[] {H264_FORMAT}, fakeSampleStream2, /* offsetUs= */ 100); @@ -299,6 +299,8 @@ public final class SimpleDecoderVideoRendererTest { verify(eventListener, times(2)).onRenderedFirstFrame(any()); } + // TODO: Fix rendering of first frame at stream transition. + @Ignore @Test public void replaceStream_whenNotStarted_doesNotRenderFirstFrameOfNewStream() throws Exception { FakeSampleStream fakeSampleStream1 = @@ -327,7 +329,7 @@ public final class SimpleDecoderVideoRendererTest { /* offsetUs */ 0); boolean replacedStream = false; - for (int i = 0; i < 200; i += 10) { + for (int i = 0; i < 10; i++) { renderer.render(/* positionUs= */ i * 10, SystemClock.elapsedRealtime() * 1000); if (!replacedStream && renderer.hasReadStreamToEnd()) { renderer.replaceStream(new Format[] {H264_FORMAT}, fakeSampleStream2, /* offsetUs= */ 100); @@ -338,5 +340,10 @@ public final class SimpleDecoderVideoRendererTest { } verify(eventListener).onRenderedFirstFrame(any()); + + // Render to streamOffsetUs and verify the new first frame gets rendered. + renderer.render(/* positionUs= */ 100, SystemClock.elapsedRealtime() * 1000); + + verify(eventListener, times(2)).onRenderedFirstFrame(any()); } }