diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java index 714dfff676..dad891718e 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -15,6 +15,8 @@ */ package com.google.android.exoplayer2; +import com.google.android.exoplayer2.Player.DefaultEventListener; +import com.google.android.exoplayer2.Player.EventListener; import com.google.android.exoplayer2.source.ConcatenatingMediaSource; import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.TrackGroup; @@ -344,6 +346,39 @@ public final class ExoPlayerTest extends TestCase { assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(3)); } + public void testSeekProcessedCalledWithIllegalSeekPosition() throws Exception { + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testSeekProcessedCalledWithIllegalSeekPosition") + .waitForPlaybackState(Player.STATE_BUFFERING) + // Cause an illegal seek 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. + // Because the media source prepares immediately, the exception will be thrown when the + // player processed the seek. + .seek(/* windowIndex= */ 100, /* positionMs= */ 0) + .waitForPlaybackState(Player.STATE_IDLE) + .build(); + final boolean[] onSeekProcessedCalled = new boolean[1]; + EventListener listener = + new DefaultEventListener() { + @Override + public void onSeekProcessed() { + onSeekProcessedCalled[0] = true; + } + }; + ExoPlayerTestRunner testRunner = + new ExoPlayerTestRunner.Builder() + .setActionSchedule(actionSchedule) + .setEventListener(listener) + .build(); + try { + testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + assertTrue(onSeekProcessedCalled[0]); + } + public void testSeekDiscontinuity() throws Exception { FakeTimeline timeline = new FakeTimeline(1); ActionSchedule actionSchedule = new ActionSchedule.Builder("testSeekDiscontinuity") @@ -808,4 +843,69 @@ public final class ExoPlayerTest extends TestCase { testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED); testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_SEEK); } + + public void testReprepareAfterPlaybackError() throws Exception { + 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_IDLE) + .prepareSource( + new FakeMediaSource(timeline, /* manifest= */ null), + /* resetPosition= */ false, + /* resetState= */ false) + .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. + } + testRunner.assertTimelinesEqual(timeline, timeline); + testRunner.assertTimelineChangeReasonsEqual( + Player.TIMELINE_CHANGE_REASON_PREPARED, Player.TIMELINE_CHANGE_REASON_PREPARED); + } + + public void testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline() throws Exception { + final Timeline timeline = new FakeTimeline(/* windowCount= */ 1); + final FakeMediaSource mediaSource = + new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline") + .waitForPlaybackState(Player.STATE_BUFFERING) + // Cause an internal exception by seeking to an invalid position while the media source + // is still being prepared. The error will be thrown while the player handles the new + // source info. + .seek(/* windowIndex= */ 100, /* positionMs= */ 0) + .executeRunnable( + new Runnable() { + @Override + public void run() { + mediaSource.setNewSourceInfo(timeline, /* manifest= */ null); + } + }) + .waitForPlaybackState(Player.STATE_IDLE) + .build(); + ExoPlayerTestRunner testRunner = + new ExoPlayerTestRunner.Builder() + .setMediaSource(mediaSource) + .setActionSchedule(actionSchedule) + .build(); + try { + testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + testRunner.assertTimelinesEqual(timeline); + testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED); + } } 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 20d75ec1bd..a1fe8c09c5 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 @@ -328,7 +328,7 @@ import java.io.IOException; setSeekParametersInternal((SeekParameters) msg.obj); return true; case MSG_STOP: - stopInternal(/* reset= */ msg.arg1 != 0); + stopInternal(/* reset= */ msg.arg1 != 0, /* acknowledgeStop= */ true); return true; case MSG_RELEASE: releaseInternal(); @@ -353,19 +353,19 @@ import java.io.IOException; } } catch (ExoPlaybackException e) { Log.e(TAG, "Renderer error.", e); + stopInternal(/* reset= */ false, /* acknowledgeStop= */ false); eventHandler.obtainMessage(MSG_ERROR, e).sendToTarget(); - stopInternal(/* reset= */ false); return true; } catch (IOException e) { Log.e(TAG, "Source error.", e); + stopInternal(/* reset= */ false, /* acknowledgeStop= */ false); eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForSource(e)).sendToTarget(); - stopInternal(/* reset= */ false); return true; } catch (RuntimeException e) { Log.e(TAG, "Internal runtime error.", e); + stopInternal(/* reset= */ false, /* acknowledgeStop= */ false); eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForUnexpected(e)) .sendToTarget(); - stopInternal(/* reset= */ false); return true; } } @@ -635,49 +635,50 @@ import java.io.IOException; return; } - Pair periodPosition = resolveSeekPosition(seekPosition); - if (periodPosition == null) { - // The seek position was valid for the timeline that it was performed into, but the - // timeline has changed and a suitable seek position could not be resolved in the new one. - setState(Player.STATE_ENDED); - // Reset, but retain the source so that it can still be used should a seek occur. - resetInternal( - /* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false); - eventHandler - .obtainMessage(MSG_SEEK_ACK, /* seekAdjusted */ 1, 0, playbackInfo) - .sendToTarget(); - return; - } - boolean seekPositionAdjusted = seekPosition.windowPositionUs == C.TIME_UNSET; - int periodIndex = periodPosition.first; - long periodPositionUs = periodPosition.second; - long contentPositionUs = periodPositionUs; - MediaPeriodId periodId = - mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, periodPositionUs); - if (periodId.isAd()) { - seekPositionAdjusted = true; - periodPositionUs = 0; - } try { - if (periodId.equals(playbackInfo.periodId)) { - long adjustedPeriodPositionUs = periodPositionUs; - if (playingPeriodHolder != null) { - adjustedPeriodPositionUs = - playingPeriodHolder.mediaPeriod.getAdjustedSeekPositionUs( - adjustedPeriodPositionUs, SeekParameters.DEFAULT); - } - if ((adjustedPeriodPositionUs / 1000) == (playbackInfo.positionUs / 1000)) { - // Seek will be performed to the current position. Do nothing. - periodPositionUs = playbackInfo.positionUs; - return; - } + Pair periodPosition = resolveSeekPosition(seekPosition); + if (periodPosition == null) { + // The seek position was valid for the timeline that it was performed into, but the + // timeline has changed and a suitable seek position could not be resolved in the new one. + setState(Player.STATE_ENDED); + // Reset, but retain the source so that it can still be used should a seek occur. + resetInternal( + /* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false); + seekPositionAdjusted = true; + return; + } + + int periodIndex = periodPosition.first; + long periodPositionUs = periodPosition.second; + long contentPositionUs = periodPositionUs; + MediaPeriodId periodId = + mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, periodPositionUs); + if (periodId.isAd()) { + seekPositionAdjusted = true; + periodPositionUs = 0; + } + try { + if (periodId.equals(playbackInfo.periodId)) { + long adjustedPeriodPositionUs = periodPositionUs; + if (playingPeriodHolder != null) { + adjustedPeriodPositionUs = + playingPeriodHolder.mediaPeriod.getAdjustedSeekPositionUs( + adjustedPeriodPositionUs, SeekParameters.DEFAULT); + } + if ((adjustedPeriodPositionUs / 1000) == (playbackInfo.positionUs / 1000)) { + // Seek will be performed to the current position. Do nothing. + periodPositionUs = playbackInfo.positionUs; + return; + } + } + long newPeriodPositionUs = seekToPeriodPosition(periodId, periodPositionUs); + seekPositionAdjusted |= periodPositionUs != newPeriodPositionUs; + periodPositionUs = newPeriodPositionUs; + } finally { + playbackInfo = playbackInfo.fromNewPosition(periodId, periodPositionUs, contentPositionUs); } - long newPeriodPositionUs = seekToPeriodPosition(periodId, periodPositionUs); - seekPositionAdjusted |= periodPositionUs != newPeriodPositionUs; - periodPositionUs = newPeriodPositionUs; } finally { - playbackInfo = playbackInfo.fromNewPosition(periodId, periodPositionUs, contentPositionUs); eventHandler.obtainMessage(MSG_SEEK_ACK, seekPositionAdjusted ? 1 : 0, 0, playbackInfo) .sendToTarget(); } @@ -775,12 +776,10 @@ import java.io.IOException; this.seekParameters = seekParameters; } - private void stopInternal(boolean reset) { + private void stopInternal(boolean reset, boolean acknowledgeStop) { resetInternal( /* releaseMediaSource= */ true, /* resetPosition= */ reset, /* resetState= */ reset); - int prepareOrStopAcks = pendingPrepareCount + 1; - pendingPrepareCount = 0; - notifySourceInfoRefresh(prepareOrStopAcks, playbackInfo); + notifySourceInfoRefresh(acknowledgeStop); loadControl.onStopped(); setState(Player.STATE_IDLE); } @@ -1011,15 +1010,13 @@ import java.io.IOException; playbackInfo = playbackInfo.copyWithTimeline(timeline, manifest); if (oldTimeline == null) { - int processedPrepareAcks = pendingPrepareCount; - pendingPrepareCount = 0; if (pendingInitialSeekPosition != null) { Pair periodPosition = resolveSeekPosition(pendingInitialSeekPosition); pendingInitialSeekPosition = null; if (periodPosition == null) { // The seek position was valid for the timeline that it was performed into, but the // timeline has changed and a suitable seek position could not be resolved in the new one. - handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); + handleSourceInfoRefreshEndedPlayback(); } else { int periodIndex = periodPosition.first; long positionUs = periodPosition.second; @@ -1027,11 +1024,11 @@ import java.io.IOException; mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, positionUs); playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : positionUs, positionUs); - notifySourceInfoRefresh(processedPrepareAcks); + notifySourceInfoRefresh(); } } else if (playbackInfo.startPositionUs == C.TIME_UNSET) { if (timeline.isEmpty()) { - handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); + handleSourceInfoRefreshEndedPlayback(); } else { Pair defaultPosition = getPeriodPosition(timeline, timeline.getFirstWindowIndex(shuffleModeEnabled), C.TIME_UNSET); @@ -1041,10 +1038,10 @@ import java.io.IOException; startPositionUs); playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : startPositionUs, startPositionUs); - notifySourceInfoRefresh(processedPrepareAcks); + notifySourceInfoRefresh(); } } else { - notifySourceInfoRefresh(processedPrepareAcks); + notifySourceInfoRefresh(); } return; } @@ -1171,26 +1168,20 @@ import java.io.IOException; } private void handleSourceInfoRefreshEndedPlayback() { - handleSourceInfoRefreshEndedPlayback(0); - } - - private void handleSourceInfoRefreshEndedPlayback(int prepareAcks) { setState(Player.STATE_ENDED); // Reset, but retain the source so that it can still be used should a seek occur. resetInternal( /* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false); - notifySourceInfoRefresh(prepareAcks, playbackInfo); + notifySourceInfoRefresh(); } private void notifySourceInfoRefresh() { - notifySourceInfoRefresh(0); + notifySourceInfoRefresh(/* acknowledgeStop= */ false); } - private void notifySourceInfoRefresh(int prepareOrStopAcks) { - notifySourceInfoRefresh(prepareOrStopAcks, playbackInfo); - } - - private void notifySourceInfoRefresh(int prepareOrStopAcks, PlaybackInfo playbackInfo) { + private void notifySourceInfoRefresh(boolean acknowledgeStop) { + int prepareOrStopAcks = pendingPrepareCount + (acknowledgeStop ? 1 : 0); + pendingPrepareCount = 0; eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, prepareOrStopAcks, 0, playbackInfo) .sendToTarget(); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java index fddeb60bf0..4905fc2233 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java @@ -478,9 +478,8 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener } /** - * Blocks the current thread until the action schedule finished. Also returns when an - * {@link ExoPlaybackException} is thrown. This does not release the test runner and the test must - * still call {@link #blockUntilEnded(long)}. + * Blocks the current thread until the action schedule finished. This does not release the test + * runner and the test must still call {@link #blockUntilEnded(long)}. * * @param timeoutMs The maximum time to wait for the action schedule to finish. * @return This test runner. @@ -611,7 +610,6 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener while (endedCountDownLatch.getCount() > 0) { endedCountDownLatch.countDown(); } - actionScheduleFinishedCountDownLatch.countDown(); } // Player.EventListener