Use correct period-window offset for initial prepare position.

MaskingMediaSource needs to resolve the prepare position set for a MaskingPeriod while the source was still unprepared to the first actual prepare position. It currently assumes that the period-window offset and the default position is zero. This assumption is correct when a PlaceholderTimeline is used, but it may not be true if the real timeline is already known (e.g. when re-preparing a live stream after a playback error). Fix this by using the known timeline at the time of the preparation. Also: - Update a test that should have caught this to use lazy re-preparation. - Change the demo app code to use the recommended way to restart playback after a BehindLiveWindowException. Issue: #8675 PiperOrigin-RevId: 361604191
google · Mar 12, 2021 · bc9fb86 · stevemayhew · Jul 7, 2021 · stevemayhew
1 parent ea0f72e
commit bc9fb86
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 54 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -27,6 +27,9 @@
  `ExoPlayer`.
  * Reset playback speed when live playback speed control becomes unused
  ([#8664](https://github.com/google/ExoPlayer/issues/8664)).
+ * Fix playback position issue when re-preparing playback after a
+ BehindLiveWindowException
+ ([#8675](https://github.com/google/ExoPlayer/issues/8675)).
 * Remove deprecated symbols:
  * Remove `Player.DefaultEventListener`. Use `Player.EventListener`
  instead.

diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java
@@ -440,8 +440,8 @@ public void onPlaybackStateChanged(@Player.State int playbackState) {
  @Override
  public void onPlayerError(@NonNull ExoPlaybackException e) {
  if (isBehindLiveWindow(e)) {
- clearStartPosition();
- initializePlayer();
+ player.seekToDefaultPosition();
+ player.prepare();
  } else {
  updateButtonVisibility();
  showControls();

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java
@@ -178,13 +178,17 @@ protected void onChildSourceInfoRefreshed(
  // anyway.
  newTimeline.getWindow(/* windowIndex= */ 0, window);
  long windowStartPositionUs = window.getDefaultPositionUs();
+ Object windowUid = window.uid;
  if (unpreparedMaskingMediaPeriod != null) {
  long periodPreparePositionUs = unpreparedMaskingMediaPeriod.getPreparePositionUs();
- if (periodPreparePositionUs != 0) {
- windowStartPositionUs = periodPreparePositionUs;
+ timeline.getPeriodByUid(unpreparedMaskingMediaPeriod.id.periodUid, period);
+ long windowPreparePositionUs = period.getPositionInWindowUs() + periodPreparePositionUs;
+ long oldWindowDefaultPositionUs =
+ timeline.getWindow(/* windowIndex= */ 0, window).getDefaultPositionUs();
+ if (windowPreparePositionUs != oldWindowDefaultPositionUs) {
+ windowStartPositionUs = windowPreparePositionUs;
  }
  }
- Object windowUid = window.uid;
  Pair<Object, Long> periodPosition =
  newTimeline.getPeriodPosition(
  window, period, /* windowIndex= */ 0, windowStartPositionUs);

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
@@ -17,6 +17,7 @@
 
 import static com.google.android.exoplayer2.robolectric.RobolectricUtil.runMainLooperUntil;
 import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.playUntilStartOfWindow;
+import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled;
 import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlaybackState;
 import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilReceiveOffloadSchedulingEnabledNewState;
 import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilSleepingForOffload;
@@ -1653,55 +1654,44 @@ public void reprepareAfterPlaybackError() throws Exception {
  }
 
  @Test
- public void seekAndReprepareAfterPlaybackError() throws Exception {
- Timeline timeline = new FakeTimeline();
- final long[] positionHolder = new long[2];
- ActionSchedule actionSchedule =
- new ActionSchedule.Builder(TAG)
- .pause()
- .waitForPlaybackState(Player.STATE_READY)
- .throwPlaybackException(ExoPlaybackException.createForSource(new IOException()))
- .waitForPlaybackState(Player.STATE_IDLE)
- .seek(/* positionMs= */ 50)
- .waitForPendingPlayerCommands()
- .executeRunnable(
- new PlayerRunnable() {
- @Override
- public void run(SimpleExoPlayer player) {
- positionHolder[0] = player.getCurrentPosition();
- }
- })
- .prepare()
- .waitForPlaybackState(Player.STATE_READY)
- .executeRunnable(
- new PlayerRunnable() {
- @Override
- public void run(SimpleExoPlayer player) {
- positionHolder[1] = player.getCurrentPosition();
- }
- })
- .play()
- .build();
- ExoPlayerTestRunner testRunner =
- new ExoPlayerTestRunner.Builder(context)
- .setTimeline(timeline)
- .setActionSchedule(actionSchedule)
- .build();
+ public void seekAndReprepareAfterPlaybackError_keepsSeekPositionAndTimeline() throws Exception {
+ SimpleExoPlayer player = new TestExoPlayerBuilder(context).build();
+ Player.EventListener mockListener = mock(Player.EventListener.class);
+ player.addListener(mockListener);
+ FakeMediaSource fakeMediaSource = new FakeMediaSource();
+ player.setMediaSource(fakeMediaSource);
 
- assertThrows(
- ExoPlaybackException.class,
- () ->
- testRunner
- .start()
- .blockUntilActionScheduleFinished(TIMEOUT_MS)
- .blockUntilEnded(TIMEOUT_MS));
- testRunner.assertTimelinesSame(placeholderTimeline, timeline);
- testRunner.assertTimelineChangeReasonsEqual(
- Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
- Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE);
- testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_SEEK);
- assertThat(positionHolder[0]).isEqualTo(50);
- assertThat(positionHolder[1]).isEqualTo(50);
+ player.prepare();
+ runUntilPlaybackState(player, Player.STATE_READY);
+ player
+ .createMessage(
+ (type, payload) -> {
+ throw ExoPlaybackException.createForSource(new IOException());
+ })
+ .send();
+ runUntilPlaybackState(player, Player.STATE_IDLE);
+ player.seekTo(/* positionMs= */ 50);
+ runUntilPendingCommandsAreFullyHandled(player);
+ long positionAfterSeekHandled = player.getCurrentPosition();
+ // Delay re-preparation to force player to use its masking mechanisms.
+ fakeMediaSource.setAllowPreparation(false);
+ player.prepare();
+ runUntilPendingCommandsAreFullyHandled(player);
+ long positionAfterReprepareHandled = player.getCurrentPosition();
+ fakeMediaSource.setAllowPreparation(true);
+ runUntilPlaybackState(player, Player.STATE_READY);
+ long positionWhenFullyReadyAfterReprepare = player.getCurrentPosition();
+ player.release();
+
+ // Ensure we don't receive further timeline updates when repreparing.
+ verify(mockListener)
+ .onTimelineChanged(any(), eq(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED));
+ verify(mockListener).onTimelineChanged(any(), eq(Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE));
+ verify(mockListener, times(2)).onTimelineChanged(any(), anyInt());
+
+ assertThat(positionAfterSeekHandled).isEqualTo(50);
+ assertThat(positionAfterReprepareHandled).isEqualTo(50);
+ assertThat(positionWhenFullyReadyAfterReprepare).isEqualTo(50);
  }
 
  @Test

diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java
@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.testutil;
 
+import static com.google.android.exoplayer2.util.Assertions.checkState;
 import static com.google.android.exoplayer2.util.Util.castNonNull;
 import static com.google.common.truth.Truth.assertThat;
 
@@ -86,6 +87,7 @@ public Window getWindow(int windowIndex, Window window, long defaultPositionProj
  private final ArrayList<MediaPeriodId> createdMediaPeriods;
  private final DrmSessionManager drmSessionManager;
 
+ private boolean preparationAllowed;
  private @MonotonicNonNull Timeline timeline;
  private boolean preparedSource;
  private boolean releasedSource;
@@ -154,6 +156,22 @@ public FakeMediaSource(
  this.createdMediaPeriods = new ArrayList<>();
  this.drmSessionManager = drmSessionManager;
  this.trackDataFactory = trackDataFactory;
+ preparationAllowed = true;
+ }
+
+ /**
+ * Sets whether the next call to {@link #prepareSource} is allowed to finish. If not allowed, a
+ * later call to this method with {@code allowPreparation} set to true will finish the
+ * preparation.
+ *
+ * @param allowPreparation Whether preparation is allowed to finish.
+ */
+ public synchronized void setAllowPreparation(boolean allowPreparation) {
+ preparationAllowed = allowPreparation;
+ if (allowPreparation && sourceInfoRefreshHandler != null) {
+ sourceInfoRefreshHandler.post(
+ () -> finishSourcePreparation(/* sendManifestLoadEvents= */ true));
+ }
  }
 
  @Nullable
@@ -204,7 +222,7 @@ public synchronized void prepareSourceInternal(@Nullable TransferListener mediaT
  preparedSource = true;
  releasedSource = false;
  sourceInfoRefreshHandler = Util.createHandlerForCurrentLooper();
- if (timeline != null) {
+ if (preparationAllowed && timeline != null) {
  finishSourcePreparation(/* sendManifestLoadEvents= */ true);
  }
  }
@@ -273,11 +291,14 @@ public void setNewSourceInfo(Timeline newTimeline) {
  * Sets a new timeline. If the source is already prepared, this triggers a source info refresh
  * message being sent to the listener.
  *
+ * <p>Must only be called if preparation is {@link #setAllowPreparation(boolean) allowed}.
+ *
  * @param newTimeline The new {@link Timeline}.
  * @param sendManifestLoadEvents Whether to treat this as a manifest refresh and send manifest
  * load events to listeners.
  */
  public synchronized void setNewSourceInfo(Timeline newTimeline, boolean sendManifestLoadEvents) {
+ checkState(preparationAllowed);
  if (sourceInfoRefreshHandler != null) {
  sourceInfoRefreshHandler.post(
  () -> {