From efc709f36616d7bdce397f307d69e9fd09b4192b Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 28 Nov 2017 04:40:06 -0800 Subject: [PATCH] Remove initial seek counting in ExoPlayerImplInternal. We can acknoledge seeks before preparation finished immediately now, because ExoPlayerImpl won't leave the masking state until the first prepare operation is processed. As a side effect, it also cleans up the responsibility of the callbacks. Prepares are always acknowledged with a SOURCE_INFO_REFRESHED, while seeks are always acknowledged with a SEEK_ACK. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=177144089 --- .../android/exoplayer2/ExoPlayerTest.java | 29 +++++++---- .../android/exoplayer2/ExoPlayerImpl.java | 15 +++--- .../exoplayer2/ExoPlayerImplInternal.java | 48 +++++++++---------- 3 files changed, 49 insertions(+), 43 deletions(-) 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 4c5ac1ac0f..efb7b0e96c 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 @@ -306,11 +306,19 @@ public final class ExoPlayerTest extends TestCase { public void testSeekProcessedCallback() throws Exception { Timeline timeline = new FakeTimeline(/* windowCount= */ 2); ActionSchedule actionSchedule = new ActionSchedule.Builder("testSeekProcessedCallback") - // Initial seek before timeline preparation finished. - .pause().seek(10).waitForPlaybackState(Player.STATE_READY) - // Re-seek to same position, start playback and wait until playback reaches second window. - .seek(10).play().waitForPositionDiscontinuity() - // Seek twice in concession, expecting the first seek to be replaced. + // Initial seek before timeline preparation started. Expect immediate seek processed while + // the player is still in STATE_IDLE. + .pause().seek(5) + // Wait until the media source starts preparing and issue more initial seeks. Expect only + // one seek processed after the source has been prepared. + .waitForPlaybackState(Player.STATE_BUFFERING).seek(2).seek(10) + // Wait until media source prepared and re-seek to same position. Expect a seek processed + // while still being in STATE_READY. + .waitForPlaybackState(Player.STATE_READY).seek(10) + // Start playback and wait until playback reaches second window. + .play().waitForPositionDiscontinuity() + // Seek twice in concession, expecting the first seek to be replaced (and thus except only + // on seek processed callback). .seek(5).seek(60).build(); final List playbackStatesWhenSeekProcessed = new ArrayList<>(); Player.EventListener eventListener = new Player.DefaultEventListener() { @@ -329,10 +337,11 @@ public final class ExoPlayerTest extends TestCase { new ExoPlayerTestRunner.Builder() .setTimeline(timeline).setEventListener(eventListener).setActionSchedule(actionSchedule) .build().start().blockUntilEnded(TIMEOUT_MS); - assertEquals(3, playbackStatesWhenSeekProcessed.size()); - assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(0)); - assertEquals(Player.STATE_READY, (int) playbackStatesWhenSeekProcessed.get(1)); - assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(2)); + assertEquals(4, playbackStatesWhenSeekProcessed.size()); + assertEquals(Player.STATE_IDLE, (int) playbackStatesWhenSeekProcessed.get(0)); + assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(1)); + assertEquals(Player.STATE_READY, (int) playbackStatesWhenSeekProcessed.get(2)); + assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(3)); } public void testSeekDiscontinuity() throws Exception { @@ -742,7 +751,7 @@ public final class ExoPlayerTest extends TestCase { .waitForPlaybackState(Player.STATE_IDLE) // If we were still using the first timeline, this would throw. .seek(/* windowIndex= */ 1, /* positionMs= */ 0) - .prepareSource(secondSource) + .prepareSource(secondSource, /* resetPosition= */ false, /* resetState= */ true) .build(); ExoPlayerTestRunner testRunner = new ExoPlayerTestRunner.Builder() .setTimeline(timeline) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 34dffd0e73..37fccafd08 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -450,8 +450,7 @@ import java.util.concurrent.CopyOnWriteArraySet; } case ExoPlayerImplInternal.MSG_SOURCE_INFO_REFRESHED: { int prepareOrStopAcks = msg.arg1; - int seekAcks = msg.arg2; - handlePlaybackInfo((PlaybackInfo) msg.obj, prepareOrStopAcks, seekAcks, false, + handlePlaybackInfo((PlaybackInfo) msg.obj, prepareOrStopAcks, 0, false, /* ignored */ DISCONTINUITY_REASON_INTERNAL); break; } @@ -510,13 +509,13 @@ import java.util.concurrent.CopyOnWriteArraySet; boolean timelineOrManifestChanged = this.playbackInfo.timeline != playbackInfo.timeline || this.playbackInfo.manifest != playbackInfo.manifest; this.playbackInfo = playbackInfo; - if (playbackInfo.timeline.isEmpty()) { - // Update the masking variables, which are used when the timeline is empty. - maskingPeriodIndex = 0; - maskingWindowIndex = 0; - maskingWindowPositionMs = 0; - } if (timelineOrManifestChanged || waitingForInitialTimeline) { + if (playbackInfo.timeline.isEmpty()) { + // Update the masking variables, which are used when the timeline becomes empty. + maskingPeriodIndex = 0; + maskingWindowIndex = 0; + maskingWindowPositionMs = 0; + } @Player.TimelineChangeReason int reason = waitingForInitialTimeline ? Player.TIMELINE_CHANGE_REASON_PREPARED : Player.TIMELINE_CHANGE_REASON_DYNAMIC; waitingForInitialTimeline = false; 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 f62d36e48b..909f52fad8 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 @@ -125,8 +125,7 @@ import java.io.IOException; private long elapsedRealtimeUs; private int pendingPrepareCount; - private int pendingInitialSeekCount; - private SeekPosition pendingSeekPosition; + private SeekPosition pendingInitialSeekPosition; private long rendererPositionUs; private MediaPeriodHolder loadingPeriodHolder; @@ -631,8 +630,9 @@ import java.io.IOException; private void seekToInternal(SeekPosition seekPosition) throws ExoPlaybackException { Timeline timeline = playbackInfo.timeline; if (timeline == null) { - pendingInitialSeekCount++; - pendingSeekPosition = seekPosition; + pendingInitialSeekPosition = seekPosition; + eventHandler.obtainMessage(MSG_SEEK_ACK, /* seekAdjusted = */ 0, 0, + playbackInfo.copyWithTimeline(Timeline.EMPTY, null)).sendToTarget(); return; } @@ -781,7 +781,7 @@ import java.io.IOException; } int prepareOrStopAcks = pendingPrepareCount + 1; pendingPrepareCount = 0; - notifySourceInfoRefresh(prepareOrStopAcks, 0, publicPlaybackInfo); + notifySourceInfoRefresh(prepareOrStopAcks, publicPlaybackInfo); loadControl.onStopped(); setState(Player.STATE_IDLE); } @@ -825,6 +825,7 @@ import java.io.IOException; ? 0 : timeline.getWindow(timeline.getFirstWindowIndex(shuffleModeEnabled), window) .firstPeriodIndex; + pendingInitialSeekPosition = null; playbackInfo = playbackInfo.fromNewPosition(firstPeriodIndex, C.TIME_UNSET, C.TIME_UNSET); } else { // The new start position is the current playback position. @@ -1009,15 +1010,13 @@ import java.io.IOException; if (oldTimeline == null) { int processedPrepareAcks = pendingPrepareCount; pendingPrepareCount = 0; - if (pendingInitialSeekCount > 0) { - Pair periodPosition = resolveSeekPosition(pendingSeekPosition); - int processedInitialSeekCount = pendingInitialSeekCount; - pendingInitialSeekCount = 0; - pendingSeekPosition = null; + 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, processedInitialSeekCount); + handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); } else { int periodIndex = periodPosition.first; long positionUs = periodPosition.second; @@ -1025,11 +1024,11 @@ import java.io.IOException; mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, positionUs); playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : positionUs, positionUs); - notifySourceInfoRefresh(processedPrepareAcks, processedInitialSeekCount); + notifySourceInfoRefresh(processedPrepareAcks); } } else if (playbackInfo.startPositionUs == C.TIME_UNSET) { if (timeline.isEmpty()) { - handleSourceInfoRefreshEndedPlayback(processedPrepareAcks, 0); + handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); } else { Pair defaultPosition = getPeriodPosition(timeline, timeline.getFirstWindowIndex(shuffleModeEnabled), C.TIME_UNSET); @@ -1039,10 +1038,10 @@ import java.io.IOException; startPositionUs); playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : startPositionUs, startPositionUs); - notifySourceInfoRefresh(processedPrepareAcks, 0); + notifySourceInfoRefresh(processedPrepareAcks); } } else { - notifySourceInfoRefresh(processedPrepareAcks, 0); + notifySourceInfoRefresh(processedPrepareAcks); } return; } @@ -1169,30 +1168,29 @@ import java.io.IOException; } private void handleSourceInfoRefreshEndedPlayback() { - handleSourceInfoRefreshEndedPlayback(0, 0); + handleSourceInfoRefreshEndedPlayback(0); } - private void handleSourceInfoRefreshEndedPlayback(int prepareAcks, int seekAcks) { + 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(false, true); // Set the playback position to 0 for notifying the eventHandler (instead of C.TIME_UNSET). - notifySourceInfoRefresh(prepareAcks, seekAcks, + notifySourceInfoRefresh(prepareAcks, playbackInfo.fromNewPosition(playbackInfo.periodId.periodIndex, 0, C.TIME_UNSET)); } private void notifySourceInfoRefresh() { - notifySourceInfoRefresh(0, 0); + notifySourceInfoRefresh(0); } - private void notifySourceInfoRefresh(int prepareOrStopAcks, int seekAcks) { - notifySourceInfoRefresh(prepareOrStopAcks, seekAcks, playbackInfo); + private void notifySourceInfoRefresh(int prepareOrStopAcks) { + notifySourceInfoRefresh(prepareOrStopAcks, playbackInfo); } - private void notifySourceInfoRefresh(int prepareOrStopAcks, int seekAcks, - PlaybackInfo playbackInfo) { - eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, prepareOrStopAcks, seekAcks, - playbackInfo).sendToTarget(); + private void notifySourceInfoRefresh(int prepareOrStopAcks, PlaybackInfo playbackInfo) { + eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, prepareOrStopAcks, 0, playbackInfo) + .sendToTarget(); } /**