Commit 8fce3ce6 by tonihei Committed by Oliver Woodman

Do not throw in resolveSeekPosition on playback thread.

We currently throw if a pending seek position was valid when the user issued
it on the app thread, but can't be resolved on the playback thread because the
timeline changed in the meantime. Throwing in this case seems wrong as the
user could not have known about the issue (and the seek position was actually
valid). Also, in other cases where the currently playing period is no longer
in the new timeline, we gracefully use a subsequent period or transition to
ENDED state instead of throwing. So it seems more consistent to transition to
ENDED state as well.

PiperOrigin-RevId: 236274862
parent b48ceb19
...@@ -1268,20 +1268,8 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1268,20 +1268,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
playbackInfoUpdate.incrementPendingOperationAcks(pendingPrepareCount); playbackInfoUpdate.incrementPendingOperationAcks(pendingPrepareCount);
pendingPrepareCount = 0; pendingPrepareCount = 0;
if (pendingInitialSeekPosition != null) { if (pendingInitialSeekPosition != null) {
Pair<Object, Long> periodPosition; Pair<Object, Long> periodPosition =
try {
periodPosition =
resolveSeekPosition(pendingInitialSeekPosition, /* trySubsequentPeriods= */ true); resolveSeekPosition(pendingInitialSeekPosition, /* trySubsequentPeriods= */ true);
} catch (IllegalSeekPositionException e) {
MediaPeriodId firstMediaPeriodId =
playbackInfo.getDummyFirstMediaPeriodId(shuffleModeEnabled, window);
playbackInfo =
playbackInfo.resetToNewPosition(
firstMediaPeriodId,
/* startPositionUs= */ C.TIME_UNSET,
/* contentPositionUs= */ C.TIME_UNSET);
throw e;
}
pendingInitialSeekPosition = null; pendingInitialSeekPosition = null;
if (periodPosition == null) { if (periodPosition == null) {
// The seek position was valid for the timeline that it was performed into, but the // The seek position was valid for the timeline that it was performed into, but the
...@@ -1481,8 +1469,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1481,8 +1469,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
seekPosition.windowPositionUs); seekPosition.windowPositionUs);
} catch (IndexOutOfBoundsException e) { } catch (IndexOutOfBoundsException e) {
// The window index of the seek position was outside the bounds of the timeline. // The window index of the seek position was outside the bounds of the timeline.
throw new IllegalSeekPositionException( return null;
timeline, seekPosition.windowIndex, C.usToMs(seekPosition.windowPositionUs));
} }
if (timeline == seekTimeline) { if (timeline == seekTimeline) {
// Our internal timeline is the seek timeline, so the mapped position is correct. // Our internal timeline is the seek timeline, so the mapped position is correct.
......
...@@ -549,12 +549,9 @@ public final class ExoPlayerTest { ...@@ -549,12 +549,9 @@ public final class ExoPlayerTest {
ActionSchedule actionSchedule = ActionSchedule actionSchedule =
new ActionSchedule.Builder("testSeekProcessedCalledWithIllegalSeekPosition") new ActionSchedule.Builder("testSeekProcessedCalledWithIllegalSeekPosition")
.waitForPlaybackState(Player.STATE_BUFFERING) .waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an illegal seek exception by seeking to an invalid position while the media // The illegal seek position will end playback.
// source is still being prepared and the player doesn't immediately know it will fail.
// Because the media source prepares immediately, the exception will be thrown when the
// player processed the seek.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0) .seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.waitForPlaybackState(Player.STATE_IDLE) .waitForPlaybackState(Player.STATE_ENDED)
.build(); .build();
final boolean[] onSeekProcessedCalled = new boolean[1]; final boolean[] onSeekProcessedCalled = new boolean[1];
EventListener listener = EventListener listener =
...@@ -566,13 +563,7 @@ public final class ExoPlayerTest { ...@@ -566,13 +563,7 @@ public final class ExoPlayerTest {
}; };
ExoPlayerTestRunner testRunner = ExoPlayerTestRunner testRunner =
new Builder().setActionSchedule(actionSchedule).setEventListener(listener).build(context); new Builder().setActionSchedule(actionSchedule).setEventListener(listener).build(context);
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) {
// Expected exception.
assertThat(e.getUnexpectedException()).isInstanceOf(IllegalSeekPositionException.class);
}
assertThat(onSeekProcessedCalled[0]).isTrue(); assertThat(onSeekProcessedCalled[0]).isTrue();
} }
...@@ -1295,59 +1286,51 @@ public final class ExoPlayerTest { ...@@ -1295,59 +1286,51 @@ public final class ExoPlayerTest {
} }
@Test @Test
public void testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline() throws Exception { public void testInvalidSeekPositionAfterSourceInfoRefreshStillUpdatesTimeline() throws Exception {
final Timeline timeline = new FakeTimeline(/* windowCount= */ 1); final Timeline timeline = new FakeTimeline(/* windowCount= */ 1);
final FakeMediaSource mediaSource = final FakeMediaSource mediaSource =
new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); new FakeMediaSource(/* timeline= */ null, /* manifest= */ null);
ActionSchedule actionSchedule = ActionSchedule actionSchedule =
new ActionSchedule.Builder("testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline") new ActionSchedule.Builder("testInvalidSeekPositionSourceInfoRefreshStillUpdatesTimeline")
.waitForPlaybackState(Player.STATE_BUFFERING) .waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an internal exception by seeking to an invalid position while the media source // Seeking to an invalid position will end playback.
// is still being prepared. The error will be thrown while the player handles the new
// source info.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0) .seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null)) .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null))
.waitForPlaybackState(Player.STATE_IDLE) .waitForPlaybackState(Player.STATE_ENDED)
.build(); .build();
ExoPlayerTestRunner testRunner = ExoPlayerTestRunner testRunner =
new ExoPlayerTestRunner.Builder() new ExoPlayerTestRunner.Builder()
.setMediaSource(mediaSource) .setMediaSource(mediaSource)
.setActionSchedule(actionSchedule) .setActionSchedule(actionSchedule)
.build(context); .build(context);
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) {
// Expected exception.
assertThat(e.getUnexpectedException()).isInstanceOf(IllegalSeekPositionException.class);
}
testRunner.assertTimelinesEqual(timeline); testRunner.assertTimelinesEqual(timeline);
testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED); testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED);
} }
@Test @Test
public void testPlaybackErrorDuringSourceInfoRefreshWithShuffleModeEnabledUsesCorrectFirstPeriod() public void
testInvalidSeekPositionAfterSourceInfoRefreshWithShuffleModeEnabledUsesCorrectFirstPeriod()
throws Exception { throws Exception {
FakeMediaSource mediaSource = FakeMediaSource mediaSource =
new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), /* manifest= */ null); new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), /* manifest= */ null);
ConcatenatingMediaSource concatenatingMediaSource = ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource( new ConcatenatingMediaSource(
/* isAtomic= */ false, new FakeShuffleOrder(0), mediaSource, mediaSource); /* isAtomic= */ false, new FakeShuffleOrder(0), mediaSource, mediaSource);
AtomicInteger windowIndexAfterError = new AtomicInteger(); AtomicInteger windowIndexAfterUpdate = new AtomicInteger();
ActionSchedule actionSchedule = ActionSchedule actionSchedule =
new ActionSchedule.Builder("testPlaybackErrorDuringSourceInfoRefreshUsesCorrectFirstPeriod") new ActionSchedule.Builder("testInvalidSeekPositionSourceInfoRefreshUsesCorrectFirstPeriod")
.setShuffleModeEnabled(true) .setShuffleModeEnabled(true)
.waitForPlaybackState(Player.STATE_BUFFERING) .waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an internal exception by seeking to an invalid position while the media source // Seeking to an invalid position will end playback.
// is still being prepared. The error will be thrown while the player handles the new
// source info.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0) .seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.waitForPlaybackState(Player.STATE_IDLE) .waitForPlaybackState(Player.STATE_ENDED)
.executeRunnable( .executeRunnable(
new PlayerRunnable() { new PlayerRunnable() {
@Override @Override
public void run(SimpleExoPlayer player) { public void run(SimpleExoPlayer player) {
windowIndexAfterError.set(player.getCurrentWindowIndex()); windowIndexAfterUpdate.set(player.getCurrentWindowIndex());
} }
}) })
.build(); .build();
...@@ -1356,14 +1339,9 @@ public final class ExoPlayerTest { ...@@ -1356,14 +1339,9 @@ public final class ExoPlayerTest {
.setMediaSource(concatenatingMediaSource) .setMediaSource(concatenatingMediaSource)
.setActionSchedule(actionSchedule) .setActionSchedule(actionSchedule)
.build(context); .build(context);
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) { assertThat(windowIndexAfterUpdate.get()).isEqualTo(1);
// Expected exception.
assertThat(e.getUnexpectedException()).isInstanceOf(IllegalSeekPositionException.class);
}
assertThat(windowIndexAfterError.get()).isEqualTo(1);
} }
@Test @Test
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment