Fix prepare position of DeferredMediaPeriods for windows with non-zer…

…o offset.

If we prepare a deferred media period before the actual timeline is available,
we either prepare with position zero (= the default) or with a non-zero
initial seek position.

So far, the zero (default) position got replaced by the actual default position
(including any potential non-zero window offset) when the timeline became known.

However, a non-zero initial seek position was not corrected by the non-zero
window offset. This is fixed by this change.

Related to that, we always assumed that the deferred media period will the
first period in the actual timeline. This is not true if we prepare with an
offset (either because of an initial seek position or because of a default
window position). So, we also determine the actual first period when the
timeline becomes known.

Issue:#4873

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=215213030

RELEASENOTES.md

@@ -9,6 +9,9 @@
 * HLS:
   * Add constructor to `DefaultHlsExtractorFactory` for adding TS payload reader
     factory flags ([#4861](https://github.com/google/ExoPlayer/issues/4861)).
+* Fix an issue with blind seeking to windows with non-zero offset in a
+  `ConcatenatingMediaSource`
+  ([#4873](https://github.com/google/ExoPlayer/issues/4873)).
 
 ### 2.9.0 ###
 

library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java

@@ -18,6 +18,7 @@
 import android.os.Handler;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.util.Pair;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.ExoPlaybackException;
 import com.google.android.exoplayer2.ExoPlayer;
@@ -65,6 +66,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
   private final boolean isAtomic;
   private final boolean useLazyPreparation;
   private final Timeline.Window window;
+  private final Timeline.Period period;
 
   private @Nullable ExoPlayer player;
   private @Nullable Handler playerApplicationHandler;
@@ -131,6 +133,7 @@ public ConcatenatingMediaSource(
     this.isAtomic = isAtomic;
     this.useLazyPreparation = useLazyPreparation;
     window = new Timeline.Window();
+    period = new Timeline.Period();
     addMediaSources(Arrays.asList(mediaSources));
   }
 
@@ -684,21 +687,53 @@ private void updateMediaSourceInternal(MediaSourceHolder mediaSourceHolder, Time
           windowOffsetUpdate,
           periodOffsetUpdate);
     }
-    mediaSourceHolder.timeline = deferredTimeline.cloneWithNewTimeline(timeline);
-    if (!mediaSourceHolder.isPrepared && !timeline.isEmpty()) {
-      timeline.getWindow(/* windowIndex= */ 0, window);
-      long defaultPeriodPositionUs =
-          window.getPositionInFirstPeriodUs() + window.getDefaultPositionUs();
-      for (int i = 0; i < mediaSourceHolder.activeMediaPeriods.size(); i++) {
-        DeferredMediaPeriod deferredMediaPeriod = mediaSourceHolder.activeMediaPeriods.get(i);
-        deferredMediaPeriod.setDefaultPreparePositionUs(defaultPeriodPositionUs);
+    if (mediaSourceHolder.isPrepared) {
+      mediaSourceHolder.timeline = deferredTimeline.cloneWithUpdatedTimeline(timeline);
+    } else if (timeline.isEmpty()) {
+      mediaSourceHolder.timeline =
+          DeferredTimeline.createWithRealTimeline(timeline, DeferredTimeline.DUMMY_ID);
+    } else {
+      // We should have at most one deferred media period for the DummyTimeline because the duration
+      // is unset and we don't load beyond periods with unset duration. We need to figure out how to
+      // handle the prepare positions of multiple deferred media periods, should that ever change.
+      Assertions.checkState(mediaSourceHolder.activeMediaPeriods.size() <= 1);
+      DeferredMediaPeriod deferredMediaPeriod =
+          mediaSourceHolder.activeMediaPeriods.isEmpty()
+              ? null
+              : mediaSourceHolder.activeMediaPeriods.get(0);
+      // Determine first period and the start position.
+      // This will be:
+      //  1. The default window start position if no deferred period has been created yet.
+      //  2. The non-zero prepare position of the deferred period under the assumption that this is
+      //     a non-zero initial seek position in the window.
+      //  3. The default window start position if the deferred period has a prepare position of zero
+      //     under the assumption that the prepare position of zero was used because it's the
+      //     default position of the DummyTimeline window. Note that this will override an
+      //     intentional seek to zero for a window with a non-zero default position. This is
+      //     unlikely to be a problem as a non-zero default position usually only occurs for live
+      //     playbacks and seeking to zero in a live window would cause BehindLiveWindowExceptions
+      //     anyway.
+      long windowStartPositionUs = window.getDefaultPositionUs();
+      if (deferredMediaPeriod != null) {
+        long periodPreparePositionUs = deferredMediaPeriod.getPreparePositionUs();
+        if (periodPreparePositionUs != 0) {
+          windowStartPositionUs = periodPreparePositionUs;
+        }
+      }
+      Pair<Object, Long> periodPosition =
+          timeline.getPeriodPosition(window, period, /* windowIndex= */ 0, windowStartPositionUs);
+      Object periodUid = periodPosition.first;
+      long periodPositionUs = periodPosition.second;
+      mediaSourceHolder.timeline = DeferredTimeline.createWithRealTimeline(timeline, periodUid);
+      if (deferredMediaPeriod != null) {
+        deferredMediaPeriod.overridePreparePositionUs(periodPositionUs);
         MediaPeriodId idInSource =
             deferredMediaPeriod.id.copyWithPeriodUid(
                 getChildPeriodUid(mediaSourceHolder, deferredMediaPeriod.id.periodUid));
         deferredMediaPeriod.createPeriod(idInSource);
       }
-      mediaSourceHolder.isPrepared = true;
     }
+    mediaSourceHolder.isPrepared = true;
     scheduleListenerNotification(/* actionOnCompletion= */ null);
   }
 
@@ -897,33 +932,49 @@ public int getPeriodCount() {
   }
 
   /**
-   * Timeline used as placeholder for an unprepared media source. After preparation, a copy of the
-   * DeferredTimeline is used to keep the originally assigned first period ID.
+   * Timeline used as placeholder for an unprepared media source. After preparation, a
+   * DeferredTimeline is used to keep the originally assigned dummy period ID.
    */
   private static final class DeferredTimeline extends ForwardingTimeline {
 
     private static final Object DUMMY_ID = new Object();
-    private static final DummyTimeline dummyTimeline = new DummyTimeline();
+    private static final DummyTimeline DUMMY_TIMELINE = new DummyTimeline();
 
     private final Object replacedId;
 
+    /**
+     * Returns an instance with a real timeline, replacing the provided period ID with the already
+     * assigned dummy period ID.
+     *
+     * @param timeline The real timeline.
+     * @param firstPeriodUid The period UID in the timeline which will be replaced by the already
+     *     assigned dummy period UID.
+     */
+    public static DeferredTimeline createWithRealTimeline(
+        Timeline timeline, Object firstPeriodUid) {
+      return new DeferredTimeline(timeline, firstPeriodUid);
+    }
+
+    /** Creates deferred timeline exposing a {@link DummyTimeline}. */
     public DeferredTimeline() {
-      this(dummyTimeline, DUMMY_ID);
+      this(DUMMY_TIMELINE, DUMMY_ID);
     }
 
     private DeferredTimeline(Timeline timeline, Object replacedId) {
       super(timeline);
       this.replacedId = replacedId;
     }
 
-    public DeferredTimeline cloneWithNewTimeline(Timeline timeline) {
-      return new DeferredTimeline(
-          timeline,
-          replacedId == DUMMY_ID && timeline.getPeriodCount() > 0
-              ? timeline.getUidOfPeriod(0)
-              : replacedId);
+    /**
+     * Returns a copy with an updated timeline. This keeps the existing period replacement.
+     *
+     * @param timeline The new timeline.
+     */
+    public DeferredTimeline cloneWithUpdatedTimeline(Timeline timeline) {
+      return new DeferredTimeline(timeline, replacedId);
     }
 
+    /** Returns wrapped timeline. */
     public Timeline getTimeline() {
       return timeline;
     }

library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java

@@ -79,23 +79,19 @@ public void setPrepareErrorListener(PrepareErrorListener listener) {
     this.listener = listener;
   }
 
+  /** Returns the position at which the deferred media period was prepared, in microseconds. */
+  public long getPreparePositionUs() {
+    return preparePositionUs;
+  }
+
   /**
-   * Sets the default prepare position at which to prepare the media period. This value is only used
-   * if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred and the call was
-   * made with a (presumably default) prepare position of 0.
+   * Overrides the default prepare position at which to prepare the media period. This value is only
+   * used if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred.
    *
-   * <p>Note that this will override an intentional seek to zero in the corresponding non-seekable
-   * timeline window. This is unlikely to be a problem as a non-zero default position usually only
-   * occurs for live playbacks and seeking to zero in a live window would cause
-   * BehindLiveWindowExceptions anyway.
-   *
-   * @param defaultPreparePositionUs The actual default prepare position, in microseconds.
+   * @param defaultPreparePositionUs The default prepare position to use, in microseconds.
    */
-  public void setDefaultPreparePositionUs(long defaultPreparePositionUs) {
-    if (preparePositionUs == 0 && defaultPreparePositionUs != 0) {
-      preparePositionOverrideUs = defaultPreparePositionUs;
-      preparePositionUs = defaultPreparePositionUs;
-    }
+  public void overridePreparePositionUs(long defaultPreparePositionUs) {
+    preparePositionOverrideUs = defaultPreparePositionUs;
   }
 
   /**
@@ -108,6 +104,10 @@ public void setDefaultPreparePositionUs(long defaultPreparePositionUs) {
   public void createPeriod(MediaPeriodId id) {
     mediaPeriod = mediaSource.createPeriod(id, allocator);
     if (callback != null) {
+      long preparePositionUs =
+          preparePositionOverrideUs != C.TIME_UNSET
+              ? preparePositionOverrideUs
+              : this.preparePositionUs;
       mediaPeriod.prepare(this, preparePositionUs);
     }
   }
@@ -157,7 +157,7 @@ public TrackGroupArray getTrackGroups() {
   @Override
   public long selectTracks(TrackSelection[] selections, boolean[] mayRetainStreamFlags,
       SampleStream[] streams, boolean[] streamResetFlags, long positionUs) {
-    if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == 0) {
+    if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == preparePositionUs) {
       positionUs = preparePositionOverrideUs;
       preparePositionOverrideUs = C.TIME_UNSET;
     }

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

@@ -60,6 +60,7 @@
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Before;
 import org.junit.Test;
@@ -1346,7 +1347,7 @@ public void testRestartAfterEmptyTimelineWithShuffleModeEnabledUsesCorrectFirstP
                 () ->
                     concatenatingMediaSource.addMediaSources(
                         Arrays.asList(mediaSource, mediaSource)))
-            .waitForTimelineChanged(null)
+            .waitForTimelineChanged()
             .executeRunnable(
                 new PlayerRunnable() {
                   @Override
@@ -2192,14 +2193,14 @@ public void testClippedLoopedPeriodsArePlayedFully() throws Exception {
             startPositionUs + expectedDurationUs);
     Clock clock = new AutoAdvancingFakeClock();
     AtomicReference<Player> playerReference = new AtomicReference<>();
-    AtomicReference<Long> positionAtDiscontinuityMs = new AtomicReference<>();
-    AtomicReference<Long> clockAtStartMs = new AtomicReference<>();
-    AtomicReference<Long> clockAtDiscontinuityMs = new AtomicReference<>();
+    AtomicLong positionAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET);
+    AtomicLong clockAtStartMs = new AtomicLong(C.TIME_UNSET);
+    AtomicLong clockAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET);
     EventListener eventListener =
         new EventListener() {
           @Override
           public void onPlayerStateChanged(boolean playWhenReady, int playbackState) {
-            if (playbackState == Player.STATE_READY && clockAtStartMs.get() == null) {
+            if (playbackState == Player.STATE_READY && clockAtStartMs.get() == C.TIME_UNSET) {
               clockAtStartMs.set(clock.elapsedRealtime());
             }
           }
@@ -2446,6 +2447,93 @@ public void removingLoopingLastPeriodFromPlaylistDoesNotThrow() throws Exception
         .blockUntilEnded(TIMEOUT_MS);
   }
 
+  @Test
+  public void seekToUnpreparedWindowWithNonZeroOffsetInConcatenationStartsAtCorrectPosition()
+      throws Exception {
+    Timeline timeline = new FakeTimeline(/* windowCount= */ 1);
+    FakeMediaSource mediaSource = new FakeMediaSource(/* timeline= */ null, /* manifest= */ null);
+    MediaSource clippedMediaSource =
+        new ClippingMediaSource(
+            mediaSource,
+            /* startPositionUs= */ 3 * C.MICROS_PER_SECOND,
+            /* endPositionUs= */ C.TIME_END_OF_SOURCE);
+    MediaSource concatenatedMediaSource = new ConcatenatingMediaSource(clippedMediaSource);
+    AtomicLong positionWhenReady = new AtomicLong();
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder("seekToUnpreparedWindowWithNonZeroOffsetInConcatenation")
+            .pause()
+            .waitForPlaybackState(Player.STATE_BUFFERING)
+            .seek(/* positionMs= */ 10)
+            .waitForTimelineChanged()
+            .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null))
+            .waitForTimelineChanged()
+            .waitForPlaybackState(Player.STATE_READY)
+            .executeRunnable(
+                new PlayerRunnable() {
+                  @Override
+                  public void run(SimpleExoPlayer player) {
+                    positionWhenReady.set(player.getContentPosition());
+                  }
+                })
+            .play()
+            .build();
+    new Builder()
+        .setMediaSource(concatenatedMediaSource)
+        .setActionSchedule(actionSchedule)
+        .build(context)
+        .start()
+        .blockUntilEnded(TIMEOUT_MS);
+
+    assertThat(positionWhenReady.get()).isEqualTo(10);
+  }
+
+  @Test
+  public void seekToUnpreparedWindowWithMultiplePeriodsInConcatenationStartsAtCorrectPeriod()
+      throws Exception {
+    long periodDurationMs = 5000;
+    Timeline timeline =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                /* periodCount =*/ 2,
+                /* id= */ new Object(),
+                /* isSeekable= */ true,
+                /* isDynamic= */ false,
+                /* durationUs= */ 2 * periodDurationMs * 1000));
+    FakeMediaSource mediaSource = new FakeMediaSource(/* timeline= */ null, /* manifest= */ null);
+    MediaSource concatenatedMediaSource = new ConcatenatingMediaSource(mediaSource);
+    AtomicInteger periodIndexWhenReady = new AtomicInteger();
+    AtomicLong positionWhenReady = new AtomicLong();
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder("seekToUnpreparedWindowWithMultiplePeriodsInConcatenation")
+            .pause()
+            .waitForPlaybackState(Player.STATE_BUFFERING)
+            // Seek 10ms into the second period.
+            .seek(/* positionMs= */ periodDurationMs + 10)
+            .waitForTimelineChanged()
+            .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null))
+            .waitForTimelineChanged()
+            .waitForPlaybackState(Player.STATE_READY)
+            .executeRunnable(
+                new PlayerRunnable() {
+                  @Override
+                  public void run(SimpleExoPlayer player) {
+                    periodIndexWhenReady.set(player.getCurrentPeriodIndex());
+                    positionWhenReady.set(player.getContentPosition());
+                  }
+                })
+            .play()
+            .build();
+    new Builder()
+        .setMediaSource(concatenatedMediaSource)
+        .setActionSchedule(actionSchedule)
+        .build(context)
+        .start()
+        .blockUntilEnded(TIMEOUT_MS);
+
+    assertThat(periodIndexWhenReady.get()).isEqualTo(1);
+    assertThat(positionWhenReady.get()).isEqualTo(periodDurationMs + 10);
+  }
+
   // Internal methods.
 
   private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {

library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java

@@ -585,7 +585,7 @@ public void testDynamicTimelineChange() throws Exception {
                 () ->
                     concatenatedMediaSource.moveMediaSource(
                         /* currentIndex= */ 0, /* newIndex= */ 1))
-            .waitForTimelineChanged(/* expectedTimeline= */ null)
+            .waitForTimelineChanged()
             .play()
             .build();
     TestAnalyticsListener listener = runAnalyticsTest(concatenatedMediaSource, actionSchedule);

testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java

@@ -375,14 +375,23 @@ public Builder sendMessage(
       return apply(new SendMessages(tag, target, windowIndex, positionMs, deleteAfterDelivery));
     }
 
+    /**
+     * Schedules a delay until any timeline change.
+     *
+     * @return The builder, for convenience.
+     */
+    public Builder waitForTimelineChanged() {
+      return apply(new WaitForTimelineChanged(tag, /* expectedTimeline= */ null));
+    }
+
     /**
      * Schedules a delay until the timeline changed to a specified expected timeline.
      *
      * @param expectedTimeline The expected timeline to wait for. If null, wait for any timeline
      *     change.
      * @return The builder, for convenience.
      */
-    public Builder waitForTimelineChanged(@Nullable Timeline expectedTimeline) {
+    public Builder waitForTimelineChanged(Timeline expectedTimeline) {
       return apply(new WaitForTimelineChanged(tag, expectedTimeline));
     }