Constrain resolved period positions to be within the period

This is a candidate fix for #8906. As mentioned in that issue,
negative positions within windows might be (kind of) valid in
live streaming scenarios, where the window starts at some
non-zero position within the period. However, negative positions
within periods are definitely not valid. Neither are positions
that exceed the period duration.

There was already logic in ExoPlayerImplInternal to prevent a
resolved seek position from exceeding the period duration. This
fix adds the equivalent constraint for the start of the period.
It also moves the application of the constraints into Timeline.
This has the advantage that the constraints are applied as part
of state masking in ExoPlayerImpl.seekTo, removing any UI flicker
where the invalid seek position is temporarily visible.

Issue: #8906
PiperOrigin-RevId: 395917413
This commit is contained in:
olly 2021-09-10 14:46:32 +01:00 committed by Oliver Woodman
parent 4f06419334
commit 68ee587e25
5 changed files with 30 additions and 19 deletions

View File

@ -24,6 +24,10 @@
`MediaSourceFactory` instances from being re-used by `ExoPlayer`
instances with non-overlapping lifecycles
([#9099](https://github.com/google/ExoPlayer/issues/9099)).
* Better handle invalid seek requests. Seeks to positions that are before
the start or after the end of the media are now handled as seeks to the
start and end respectively
([8906](https://github.com/google/ExoPlayer/issues/8906)).
* Extractors:
* Support TS packets without PTS flag
([#9294](https://github.com/google/ExoPlayer/issues/9294)).

View File

@ -17,6 +17,8 @@ package com.google.android.exoplayer2;
import static com.google.android.exoplayer2.util.Assertions.checkArgument;
import static com.google.android.exoplayer2.util.Assertions.checkState;
import static java.lang.Math.max;
import static java.lang.Math.min;
import android.net.Uri;
import android.os.Bundle;
@ -28,6 +30,7 @@ import androidx.annotation.Nullable;
import com.google.android.exoplayer2.source.ads.AdPlaybackState;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.BundleUtil;
import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util;
import com.google.common.collect.ImmutableList;
import java.lang.annotation.Documented;
@ -1158,7 +1161,9 @@ public abstract class Timeline implements Bundleable {
}
/**
* Converts (windowIndex, windowPositionUs) to the corresponding (periodUid, periodPositionUs).
* Converts {@code (windowIndex, windowPositionUs)} to the corresponding {@code (periodUid,
* periodPositionUs)}. The returned {@code periodPositionUs} is constrained to be non-negative,
* and to be less than the containing period's duration if it is known.
*
* @param window A {@link Window} that may be overwritten.
* @param period A {@link Period} that may be overwritten.
@ -1195,6 +1200,15 @@ public abstract class Timeline implements Bundleable {
}
getPeriod(periodIndex, period, /* setIds= */ true);
long periodPositionUs = windowPositionUs - period.positionInWindowUs;
// The period positions must be less than the period duration, if it is known.
if (period.durationUs != C.TIME_UNSET) {
periodPositionUs = min(periodPositionUs, period.durationUs - 1);
}
// Period positions cannot be negative.
periodPositionUs = max(0, periodPositionUs);
if (periodPositionUs == 9) {
Log.e("XXX", "YYY");
}
return Pair.create(Assertions.checkNotNull(period.uid), periodPositionUs);
}

View File

@ -1273,17 +1273,10 @@ import java.util.concurrent.atomic.AtomicBoolean;
if (!newPlayingPeriodHolder.prepared) {
newPlayingPeriodHolder.info =
newPlayingPeriodHolder.info.copyWithStartPositionUs(periodPositionUs);
} else {
if (newPlayingPeriodHolder.info.durationUs != C.TIME_UNSET
&& periodPositionUs >= newPlayingPeriodHolder.info.durationUs) {
// Make sure seek position doesn't exceed period duration.
periodPositionUs = max(0, newPlayingPeriodHolder.info.durationUs - 1);
}
if (newPlayingPeriodHolder.hasEnabledTracks) {
periodPositionUs = newPlayingPeriodHolder.mediaPeriod.seekToUs(periodPositionUs);
newPlayingPeriodHolder.mediaPeriod.discardBuffer(
periodPositionUs - backBufferDurationUs, retainBackBufferFromKeyframe);
}
} else if (newPlayingPeriodHolder.hasEnabledTracks) {
periodPositionUs = newPlayingPeriodHolder.mediaPeriod.seekToUs(periodPositionUs);
newPlayingPeriodHolder.mediaPeriod.discardBuffer(
periodPositionUs - backBufferDurationUs, retainBackBufferFromKeyframe);
}
resetRendererPosition(periodPositionUs);
maybeContinueLoading();

View File

@ -3663,7 +3663,7 @@ public final class ExoPlayerTest {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000));
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000_000));
final ConcatenatingMediaSource underlyingSource = new ConcatenatingMediaSource();
CompositeMediaSource<Void> delegatingMediaSource =
new CompositeMediaSource<Void>() {
@ -5416,7 +5416,7 @@ public final class ExoPlayerTest {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000));
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000_000));
ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource(
/* isAtomic= */ false,
@ -5455,7 +5455,7 @@ public final class ExoPlayerTest {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000));
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000_000));
ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource(/* isAtomic= */ false);
int[] currentWindowIndices = new int[2];
@ -5526,7 +5526,7 @@ public final class ExoPlayerTest {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000));
/* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 10_000_000));
ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource(/* isAtomic= */ false);
int[] currentWindowIndices = new int[2];
@ -10729,7 +10729,7 @@ public final class ExoPlayerTest {
TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY);
player.seekForward();
assertThat(player.getCurrentPosition()).isEqualTo(C.DEFAULT_SEEK_FORWARD_INCREMENT_MS / 2);
assertThat(player.getCurrentPosition()).isEqualTo(C.DEFAULT_SEEK_FORWARD_INCREMENT_MS / 2 - 1);
player.release();
}

View File

@ -79,9 +79,9 @@ public final class SinglePeriodTimelineTest {
timeline.getPeriodPosition(window, period, 0, C.TIME_UNSET, windowDurationUs + 1);
assertThat(position).isNull();
// Should return (0, duration) with a projection equal to window duration.
position = timeline.getPeriodPosition(window, period, 0, C.TIME_UNSET, windowDurationUs);
position = timeline.getPeriodPosition(window, period, 0, C.TIME_UNSET, windowDurationUs - 1);
assertThat(position.first).isEqualTo(timeline.getUidOfPeriod(0));
assertThat(position.second).isEqualTo(windowDurationUs);
assertThat(position.second).isEqualTo(windowDurationUs - 1);
// Should return (0, 0) without a position projection.
position = timeline.getPeriodPosition(window, period, 0, C.TIME_UNSET, 0);
assertThat(position.first).isEqualTo(timeline.getUidOfPeriod(0));