diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0a2ba36024..052b568b7d 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -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)). diff --git a/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java b/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java index 189efc3258..31bea4d806 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/Timeline.java @@ -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); } 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 a497f20b09..93b8311894 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 @@ -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(); 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 deb4e780e1..fb2913fd70 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 @@ -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 delegatingMediaSource = new CompositeMediaSource() { @@ -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(); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/source/SinglePeriodTimelineTest.java b/library/core/src/test/java/com/google/android/exoplayer2/source/SinglePeriodTimelineTest.java index 09ac9b6df2..5f5b28262d 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/source/SinglePeriodTimelineTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/source/SinglePeriodTimelineTest.java @@ -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));