Ensure seek and prepare positions never exceed period duration.

Exceeding the period duration may mean that that playback transitions
to another item even if the player is currently paused.

PiperOrigin-RevId: 300133655
This commit is contained in:
tonihei 2020-03-10 18:11:21 +00:00 committed by Oliver Woodman
parent bb33d568ca
commit c6e8e24ada
11 changed files with 144 additions and 37 deletions

View File

@ -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();

View File

@ -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);
}

View File

@ -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,

View File

@ -369,16 +369,10 @@ public interface Renderer extends PlayerMessage.Target {
* {@link SampleStream} in sync with the specified media positions.
*
* <p>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:
*
* <ol>
* <li>The initial start of the media after calling {@link #enable(RendererConfiguration,
* Format[], SampleStream, long, boolean, boolean, long)} with {@code
* mayRenderStartOfStream} set to {@code false}.
* <li>The start of a new stream after calling {@link #replaceStream(Format[], SampleStream,
* long)}.
* </ol>
* 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}.
*
* <p>This method should return quickly, and should not block if the renderer is unable to make
* useful progress.

View File

@ -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

View File

@ -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<Void> {
@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<Void> {
: 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<Void> {
: 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.

View File

@ -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);
}

View File

@ -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);

View File

@ -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,

View File

@ -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;
}

View File

@ -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());
}
}