Skip to content

Commit

Permalink
Use correct period-window offset for initial prepare position.
Browse files Browse the repository at this point in the history 
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: google#8675
PiperOrigin-RevId: 361604191
  • Loading branch information
@tonihei @roblav96
tonihei authored and roblav96 committed Apr 17, 2021
1 parent 22ac429 commit 0127031
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 54 deletions.
3 changes: 3 additions & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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.
Expand Down
4 changes: 2 additions & 2 deletions demos/main/src/main/java/com/google/android/exoplayer2/demo/PlayerActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,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();
Expand Down
10 changes: 7 additions & 3 deletions library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
86 changes: 38 additions & 48 deletions library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1649,55 +1650,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
Expand Down
23 changes: 22 additions & 1 deletion testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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(
() -> {
Expand Down

0 comments on commit 0127031

Please sign in to comment.