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

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)).

library/common/src/main/java/com/google/android/exoplayer2/Timeline.java

@@ -17,6 +17,8 @@
 
 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 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 final Pair<Object, Long> getPeriodPosition(
   }
 
   /**
-   * 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 @@ && getPeriod(periodIndex + 1, period).positionInWindowUs <= windowPositionUs) {
     }
     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);
   }
 

library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java

@@ -1273,17 +1273,10 @@ private long seekToPeriodPosition(
       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();

library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java

@@ -3663,7 +3663,7 @@ public void delegatingMediaSourceApproach() throws Exception {
     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 void seekToIndexLargerThanNumberOfPlaylistItems() throws Exception {
     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 void seekToIndexWithEmptyMultiWindowMediaSource() throws Exception {
     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 void seekToIndexWithEmptyMultiWindowMediaSource_usesLazyPreparation() thr
     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 void seekForward_pastDuration_seeksToDuration() throws Exception {
     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();
   }

library/core/src/test/java/com/google/android/exoplayer2/source/SinglePeriodTimelineTest.java

@@ -79,9 +79,9 @@ public void getPeriodPositionDynamicWindowKnownDuration() {
         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));