From 4e69f71e52c3600726b00f32dec73092c5419190 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 6 Mar 2018 08:22:41 -0800 Subject: [PATCH] Fix wrong start position in resetInternal. The start position was set to the old start position instead of the current playback position. We need to set the start position to the current playback position to ensure a repreperation with the same media starts from the last playback position. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=188025439 --- .../exoplayer2/ExoPlayerImplInternal.java | 2 +- .../android/exoplayer2/ExoPlayerTest.java | 86 ++++++++++++++++--- .../android/exoplayer2/testutil/Action.java | 30 +++++++ .../exoplayer2/testutil/ActionSchedule.java | 11 +++ 4 files changed, 116 insertions(+), 13 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index ea0e84828a..7dc22a8167 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -792,7 +792,7 @@ import java.util.Collections; resetState ? null : playbackInfo.manifest, resetPosition ? new MediaPeriodId(getFirstPeriodIndex()) : playbackInfo.periodId, // Set the start position to TIME_UNSET so that a subsequent seek to 0 isn't ignored. - resetPosition ? C.TIME_UNSET : playbackInfo.startPositionUs, + resetPosition ? C.TIME_UNSET : playbackInfo.positionUs, resetPosition ? C.TIME_UNSET : playbackInfo.contentPositionUs, playbackInfo.playbackState, /* isLoading= */ false, 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 index 1a4d7c07a4..c25219e24f 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -44,6 +44,7 @@ import com.google.android.exoplayer2.testutil.FakeTrackSelection; import com.google.android.exoplayer2.testutil.FakeTrackSelector; import com.google.android.exoplayer2.testutil.RobolectricUtil; import com.google.android.exoplayer2.upstream.Allocator; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -1170,10 +1171,8 @@ public final class ExoPlayerTest { Timeline timeline = new FakeTimeline(/* windowCount= */ 1); ActionSchedule actionSchedule = new ActionSchedule.Builder("testReprepareAfterPlaybackError") - .waitForPlaybackState(Player.STATE_BUFFERING) - // Cause an internal exception by seeking to an invalid position while the media source - // is still being prepared and the player doesn't immediately know it will fail. - .seek(/* windowIndex= */ 100, /* positionMs= */ 0) + .waitForPlaybackState(Player.STATE_READY) + .throwPlaybackException(ExoPlaybackException.createForSource(new IOException())) .waitForPlaybackState(Player.STATE_IDLE) .prepareSource( new FakeMediaSource(timeline, /* manifest= */ null), @@ -1204,11 +1203,8 @@ public final class ExoPlayerTest { ActionSchedule actionSchedule = new ActionSchedule.Builder("testReprepareAfterPlaybackError") .pause() - .waitForPlaybackState(Player.STATE_BUFFERING) - // Cause an internal exception by seeking to an invalid position while the media source - // is still being prepared and the player doesn't immediately know it will fail. - .seek(/* windowIndex= */ 100, /* positionMs= */ 0) - .waitForSeekProcessed() + .waitForPlaybackState(Player.STATE_READY) + .throwPlaybackException(ExoPlaybackException.createForSource(new IOException())) .waitForPlaybackState(Player.STATE_IDLE) .seek(/* positionMs= */ 50) .waitForSeekProcessed() @@ -1247,8 +1243,7 @@ public final class ExoPlayerTest { testRunner.assertTimelinesEqual(timeline, timeline); testRunner.assertTimelineChangeReasonsEqual( Player.TIMELINE_CHANGE_REASON_PREPARED, Player.TIMELINE_CHANGE_REASON_PREPARED); - testRunner.assertPositionDiscontinuityReasonsEqual( - Player.DISCONTINUITY_REASON_SEEK, Player.DISCONTINUITY_REASON_SEEK); + testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_SEEK); assertThat(positionHolder[0]).isEqualTo(50); assertThat(positionHolder[1]).isEqualTo(50); } @@ -1289,6 +1284,72 @@ public final class ExoPlayerTest { testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED); } + @Test + public void testPlaybackErrorAndReprepareDoesNotResetPosition() throws Exception { + final Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + final long[] positionHolder = new long[3]; + final int[] windowIndexHolder = new int[3]; + final FakeMediaSource secondMediaSource = + new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testPlaybackErrorDoesNotResetPosition") + .pause() + .waitForPlaybackState(Player.STATE_READY) + .playUntilPosition(/* windowIndex= */ 1, /* positionMs= */ 500) + .throwPlaybackException(ExoPlaybackException.createForSource(new IOException())) + .waitForPlaybackState(Player.STATE_IDLE) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + // Position while in error state + positionHolder[0] = player.getCurrentPosition(); + windowIndexHolder[0] = player.getCurrentWindowIndex(); + } + }) + .prepareSource(secondMediaSource, /* resetPosition= */ false, /* resetState= */ false) + .waitForPlaybackState(Player.STATE_BUFFERING) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + // Position while repreparing. + positionHolder[1] = player.getCurrentPosition(); + windowIndexHolder[1] = player.getCurrentWindowIndex(); + secondMediaSource.setNewSourceInfo(timeline, /* newManifest= */ null); + } + }) + .waitForPlaybackState(Player.STATE_READY) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + // Position after repreparation finished. + positionHolder[2] = player.getCurrentPosition(); + windowIndexHolder[2] = player.getCurrentWindowIndex(); + } + }) + .play() + .build(); + ExoPlayerTestRunner testRunner = + new ExoPlayerTestRunner.Builder() + .setTimeline(timeline) + .setActionSchedule(actionSchedule) + .build(); + try { + testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + assertThat(positionHolder[0]).isAtLeast(500L); + assertThat(positionHolder[1]).isEqualTo(positionHolder[0]); + assertThat(positionHolder[2]).isEqualTo(positionHolder[0]); + assertThat(windowIndexHolder[0]).isEqualTo(1); + assertThat(windowIndexHolder[1]).isEqualTo(1); + assertThat(windowIndexHolder[2]).isEqualTo(1); + } + @Test public void testSendMessagesDuringPreparation() throws Exception { Timeline timeline = new FakeTimeline(/* windowCount= */ 1); @@ -1422,7 +1483,8 @@ public final class ExoPlayerTest { new FakeMediaSource(timeline, null), /* resetPosition= */ false, /* resetState= */ true) - .waitForPlaybackState(Player.STATE_READY) + .waitForPlaybackState(Player.STATE_BUFFERING) + .waitForPlaybackState(Player.STATE_ENDED) .build(); new Builder() .setTimeline(timeline) diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java index 1b3639788e..60cf6d278b 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/Action.java @@ -447,6 +447,36 @@ public abstract class Action { } + /** Throws a playback exception on the playback thread. */ + public static final class ThrowPlaybackException extends Action { + + private final ExoPlaybackException exception; + + /** + * @param tag A tag to use for logging. + * @param exception The exception to throw. + */ + public ThrowPlaybackException(String tag, ExoPlaybackException exception) { + super(tag, "ThrowPlaybackException:" + exception); + this.exception = exception; + } + + @Override + protected void doActionImpl( + SimpleExoPlayer player, MappingTrackSelector trackSelector, Surface surface) { + player + .createMessage( + new Target() { + @Override + public void handleMessage(int messageType, Object payload) + throws ExoPlaybackException { + throw exception; + } + }) + .send(); + } + } + /** * Schedules a play action to be executed, waits until the player reaches the specified position, * and pauses the player again. diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java index 31c440553f..2ea8a50a84 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java @@ -40,6 +40,7 @@ import com.google.android.exoplayer2.testutil.Action.SetRepeatMode; import com.google.android.exoplayer2.testutil.Action.SetShuffleModeEnabled; import com.google.android.exoplayer2.testutil.Action.SetVideoSurface; import com.google.android.exoplayer2.testutil.Action.Stop; +import com.google.android.exoplayer2.testutil.Action.ThrowPlaybackException; import com.google.android.exoplayer2.testutil.Action.WaitForPlaybackState; import com.google.android.exoplayer2.testutil.Action.WaitForPositionDiscontinuity; import com.google.android.exoplayer2.testutil.Action.WaitForSeekProcessed; @@ -412,6 +413,16 @@ public final class ActionSchedule { return apply(new ExecuteRunnable(tag, runnable)); } + /** + * Schedules to throw a playback exception on the playback thread. + * + * @param exception The exception to throw. + * @return The builder, for convenience. + */ + public Builder throwPlaybackException(ExoPlaybackException exception) { + return apply(new ThrowPlaybackException(tag, exception)); + } + public ActionSchedule build() { CallbackAction callbackAction = new CallbackAction(tag); apply(callbackAction);