From 8cc7dfda7dfae000e41aeb50840786547241ecf0 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 10 Nov 2016 07:34:29 -0800 Subject: [PATCH] Fix threading issues between ExoPlayerImpl/ExoPlayerImplInternal ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=138758739 --- .../google/android/exoplayer2/ExoPlayer.java | 34 +++-- .../android/exoplayer2/ExoPlayerImpl.java | 14 +- .../exoplayer2/ExoPlayerImplInternal.java | 131 +++++++++++------- 3 files changed, 112 insertions(+), 67 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java index 31efdd82b1..83e4fd7e30 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayer.java @@ -112,6 +112,19 @@ public interface ExoPlayer { */ interface EventListener { + /** + * Called when the timeline and/or manifest has been refreshed. + *

+ * Note that if the timeline has changed then a position discontinuity may also have occurred. + * For example the current period index may have changed as a result of periods being added or + * removed from the timeline. The will not be reported via a separate call to + * {@link #onPositionDiscontinuity()}. + * + * @param timeline The latest timeline, or null if the timeline is being cleared. + * @param manifest The latest manifest, or null if the manifest is being cleared. + */ + void onTimelineChanged(Timeline timeline, Object manifest); + /** * Called when the available or selected tracks change. * @@ -138,14 +151,6 @@ public interface ExoPlayer { */ void onPlayerStateChanged(boolean playWhenReady, int playbackState); - /** - * Called when timeline and/or manifest has been refreshed. - * - * @param timeline The latest timeline, or null if the timeline is being cleared. - * @param manifest The latest manifest, or null if the manifest is being cleared. - */ - void onTimelineChanged(Timeline timeline, Object manifest); - /** * Called when an error occurs. The playback state will transition to {@link #STATE_IDLE} * immediately after this method is called. The player instance can still be used, and @@ -156,9 +161,14 @@ public interface ExoPlayer { void onPlayerError(ExoPlaybackException error); /** - * Called when a position discontinuity occurs. Position discontinuities occur when seeks are - * performed, when playbacks transition from one period in the timeline to the next, and when - * the player introduces discontinuities internally. + * Called when a position discontinuity occurs without a change to the timeline. A position + * discontinuity occurs when the current window or period index changes (as a result of playback + * transitioning from one period in the timeline to the next), or when the playback position + * jumps within the period currently being played (as a result of a seek being performed, or + * when the source introduces a discontinuity internally). + *

+ * When a position discontinuity occurs as a result of a change to the timeline this method is + * not called. {@link #onTimelineChanged(Timeline, Object)} is called in this case. */ void onPositionDiscontinuity(); @@ -403,7 +413,7 @@ public interface ExoPlayer { Timeline getCurrentTimeline(); /** - * Returns the index of the period currently being played, or {@link C#INDEX_UNSET} if unknown. + * Returns the index of the period currently being played. */ int getCurrentPeriodIndex(); diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index ec736ed3a0..5bf1b599e2 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -20,8 +20,8 @@ import android.os.Handler; import android.os.Looper; import android.os.Message; import android.util.Log; -import android.util.Pair; import com.google.android.exoplayer2.ExoPlayerImplInternal.PlaybackInfo; +import com.google.android.exoplayer2.ExoPlayerImplInternal.SourceInfo; import com.google.android.exoplayer2.ExoPlayerImplInternal.TrackInfo; import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.TrackGroupArray; @@ -329,8 +329,7 @@ import java.util.concurrent.CopyOnWriteArraySet; break; } case ExoPlayerImplInternal.MSG_SEEK_ACK: { - pendingSeekAcks -= msg.arg1; - if (pendingSeekAcks == 0) { + if (--pendingSeekAcks == 0) { playbackInfo = (ExoPlayerImplInternal.PlaybackInfo) msg.obj; for (EventListener listener : listeners) { listener.onPositionDiscontinuity(); @@ -348,10 +347,11 @@ import java.util.concurrent.CopyOnWriteArraySet; break; } case ExoPlayerImplInternal.MSG_SOURCE_INFO_REFRESHED: { - @SuppressWarnings("unchecked") - Pair timelineAndManifest = (Pair) msg.obj; - timeline = timelineAndManifest.first; - manifest = timelineAndManifest.second; + SourceInfo sourceInfo = (SourceInfo) msg.obj; + timeline = sourceInfo.timeline; + manifest = sourceInfo.manifest; + playbackInfo = sourceInfo.playbackInfo; + pendingSeekAcks -= sourceInfo.seekAcks; for (EventListener listener : listeners) { listener.onTimelineChanged(timeline, manifest); } diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 35f5f393be..743015509b 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -79,6 +79,22 @@ import java.io.IOException; } + public static final class SourceInfo { + + public final Timeline timeline; + public final Object manifest; + public final PlaybackInfo playbackInfo; + public final int seekAcks; + + public SourceInfo(Timeline timeline, Object manifest, PlaybackInfo playbackInfo, int seekAcks) { + this.timeline = timeline; + this.manifest = manifest; + this.playbackInfo = playbackInfo; + this.seekAcks = seekAcks; + } + + } + private static final String TAG = "ExoPlayerImplInternal"; // External messages @@ -533,15 +549,14 @@ import java.io.IOException; try { if (periodIndex == playbackInfo.periodIndex - && ((periodPositionUs == C.TIME_UNSET && playbackInfo.positionUs == C.TIME_UNSET) - || ((periodPositionUs / 1000) == (playbackInfo.positionUs / 1000)))) { + && ((periodPositionUs / 1000) == (playbackInfo.positionUs / 1000))) { // Seek position equals the current position. Do nothing. return; } periodPositionUs = seekToPeriodPosition(periodIndex, periodPositionUs); } finally { playbackInfo = new PlaybackInfo(periodIndex, periodPositionUs); - eventHandler.obtainMessage(MSG_SEEK_ACK, 1, 0, playbackInfo).sendToTarget(); + eventHandler.obtainMessage(MSG_SEEK_ACK, playbackInfo).sendToTarget(); } } @@ -551,11 +566,10 @@ import java.io.IOException; rebuffering = false; setState(ExoPlayer.STATE_BUFFERING); - if (periodPositionUs == C.TIME_UNSET || (readingPeriodHolder != playingPeriodHolder - && (periodIndex == playingPeriodHolder.index - || periodIndex == readingPeriodHolder.index))) { - // Clear the timeline because either the seek position is not known, or a renderer is reading - // ahead to the next period and the seek is to either the playing or reading period. + if (readingPeriodHolder != playingPeriodHolder && (periodIndex == playingPeriodHolder.index + || periodIndex == readingPeriodHolder.index)) { + // Clear the timeline because a renderer is reading ahead to the next period and the seek is + // to either the playing or reading period. periodIndex = C.INDEX_UNSET; } @@ -605,9 +619,7 @@ import java.io.IOException; playingPeriodHolder = null; readingPeriodHolder = null; loadingPeriodHolder = null; - if (periodPositionUs != C.TIME_UNSET) { - resetRendererPosition(periodPositionUs); - } + resetRendererPosition(periodPositionUs); } updatePlaybackPositions(); handler.sendEmptyMessage(MSG_DO_SOME_WORK); @@ -821,30 +833,37 @@ import java.io.IOException; private void handleSourceInfoRefreshed(Pair timelineAndManifest) throws ExoPlaybackException, IOException { - eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, timelineAndManifest).sendToTarget(); - Timeline oldTimeline = this.timeline; - this.timeline = timelineAndManifest.first; + Timeline oldTimeline = timeline; + timeline = timelineAndManifest.first; + Object manifest = timelineAndManifest.second; - if (pendingInitialSeekCount > 0) { - Pair periodPosition = resolveSeekPosition(pendingSeekPosition); - if (periodPosition == null) { - // TODO: We should probably propagate an error here. - // We failed to resolve the seek position. Stop the player. - stopInternal(); - return; + if (oldTimeline == null) { + if (pendingInitialSeekCount > 0) { + Pair periodPosition = resolveSeekPosition(pendingSeekPosition); + if (periodPosition == null) { + // TODO: We should probably propagate an error here. + // We failed to resolve the seek position. Stop the player. + finishSourceInfoRefresh(manifest, false); + stopInternal(); + return; + } + playbackInfo = new PlaybackInfo(periodPosition.first, periodPosition.second); + } else if (playbackInfo.startPositionUs == C.TIME_UNSET) { + Pair defaultPosition = getPeriodPosition(0, C.TIME_UNSET); + playbackInfo = new PlaybackInfo(defaultPosition.first, defaultPosition.second); } - playbackInfo = new PlaybackInfo(periodPosition.first, periodPosition.second); - eventHandler.obtainMessage(MSG_SEEK_ACK, pendingInitialSeekCount, 0, playbackInfo) - .sendToTarget(); - pendingInitialSeekCount = 0; - pendingSeekPosition = null; } // Update the loaded periods to take into account the new timeline. if (playingPeriodHolder != null) { int index = timeline.getIndexOfPeriod(playingPeriodHolder.uid); if (index == C.INDEX_UNSET) { - attemptRestart(playingPeriodHolder.index, oldTimeline, timeline); + boolean restarted = attemptRestart(playingPeriodHolder.index, oldTimeline, timeline); + finishSourceInfoRefresh(manifest, true); + if (!restarted) { + // TODO: We should probably propagate an error here. + stopInternal(); + } return; } @@ -873,8 +892,8 @@ import java.io.IOException; long newPositionUs = seekToPeriodPosition(index, playbackInfo.positionUs); if (newPositionUs != playbackInfo.positionUs) { playbackInfo = new PlaybackInfo(index, newPositionUs); - eventHandler.obtainMessage(MSG_POSITION_DISCONTINUITY, playbackInfo).sendToTarget(); } + finishSourceInfoRefresh(manifest, true); return; } @@ -899,7 +918,12 @@ import java.io.IOException; Object uid = loadingPeriodHolder.uid; int index = timeline.getIndexOfPeriod(uid); if (index == C.INDEX_UNSET) { - attemptRestart(loadingPeriodHolder.index, oldTimeline, timeline); + boolean restarted = attemptRestart(playingPeriodHolder.index, oldTimeline, timeline); + finishSourceInfoRefresh(manifest, true); + if (!restarted) { + // TODO: We should probably propagate an error here. + stopInternal(); + } return; } else { int windowIndex = timeline.getPeriod(index, this.period).windowIndex; @@ -908,7 +932,6 @@ import java.io.IOException; } } - // TODO[playlists]: Signal the identifier discontinuity, even if the index hasn't changed. if (oldTimeline != null) { int newPlayingIndex = playingPeriodHolder != null ? playingPeriodHolder.index : loadingPeriodHolder != null ? loadingPeriodHolder.index : C.INDEX_UNSET; @@ -916,18 +939,16 @@ import java.io.IOException; && newPlayingIndex != playbackInfo.periodIndex) { playbackInfo = new PlaybackInfo(newPlayingIndex, playbackInfo.positionUs); updatePlaybackPositions(); - eventHandler.obtainMessage(MSG_POSITION_DISCONTINUITY, playbackInfo).sendToTarget(); } } + finishSourceInfoRefresh(manifest, true); } - private void attemptRestart(int oldPeriodIndex, Timeline oldTimeline, Timeline newTimeline) { + private boolean attemptRestart(int oldPeriodIndex, Timeline oldTimeline, Timeline newTimeline) { int newPeriodIndex = resolveSubsequentPeriod(oldPeriodIndex, oldTimeline, newTimeline); if (newPeriodIndex == C.INDEX_UNSET) { - // TODO: We should probably propagate an error here. // We failed to find a replacement period. Stop the player. - stopInternal(); - return; + return false; } // Release all loaded periods. @@ -943,9 +964,18 @@ import java.io.IOException; timeline.getPeriod(newPeriodIndex, period).windowIndex, C.TIME_UNSET); newPeriodIndex = defaultPosition.first; long newPlayingPositionUs = defaultPosition.second; - playbackInfo = new PlaybackInfo(newPeriodIndex, newPlayingPositionUs); - eventHandler.obtainMessage(MSG_POSITION_DISCONTINUITY, playbackInfo).sendToTarget(); + return true; + } + + private void finishSourceInfoRefresh(Object manifest, boolean processedInitialSeeks) { + SourceInfo sourceInfo = new SourceInfo(timeline, manifest, playbackInfo, + processedInitialSeeks ? pendingInitialSeekCount : 0); + eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, sourceInfo).sendToTarget(); + if (processedInitialSeeks) { + pendingInitialSeekCount = 0; + pendingSeekPosition = null; + } } /** @@ -1076,17 +1106,22 @@ import java.io.IOException; int windowIndex = timeline.getPeriod(newLoadingPeriodIndex, period).windowIndex; boolean isFirstPeriodInWindow = newLoadingPeriodIndex == timeline.getWindow(windowIndex, window).firstPeriodIndex; - long periodStartPositionUs = loadingPeriodHolder == null ? playbackInfo.positionUs - : (isFirstPeriodInWindow ? C.TIME_UNSET : 0); - if (periodStartPositionUs == C.TIME_UNSET) { - // This is the first period of a new window or we don't have a start position, so seek to - // the default position for the window. If we're buffering ahead we also project the - // default position so that it's correct for starting playing the buffered duration of - // time in the future. - long defaultPositionProjectionUs = loadingPeriodHolder == null ? 0 - : (loadingPeriodHolder.rendererPositionOffsetUs - + timeline.getPeriod(loadingPeriodHolder.index, period).getDurationUs() - - loadingPeriodHolder.startPositionUs - rendererPositionUs); + long periodStartPositionUs; + if (loadingPeriodHolder == null) { + periodStartPositionUs = playbackInfo.startPositionUs; + } else if (!isFirstPeriodInWindow) { + // We're starting to buffer a new period in the current window. Always start from the + // beginning of the period. + periodStartPositionUs = 0; + } else { + // We're starting to buffer a new window. When playback transitions to this window we'll + // want it to be from its default start position. The expected delay until playback + // transitions is equal the duration of media that's currently buffered (assuming no + // interruptions). Hence we project the default start position forward by the duration of + // the buffer, and start buffering from this point. + long defaultPositionProjectionUs = loadingPeriodHolder.rendererPositionOffsetUs + + timeline.getPeriod(loadingPeriodHolder.index, period).getDurationUs() + - loadingPeriodHolder.startPositionUs - rendererPositionUs; Pair defaultPosition = getPeriodPosition(timeline, windowIndex, C.TIME_UNSET, Math.max(0, defaultPositionProjectionUs)); if (defaultPosition == null) {