From 6b16b08712e9a4cc79eadcb373b3bbd00aabb6a2 Mon Sep 17 00:00:00 2001 From: John Zhen M Date: Thu, 14 Sep 2017 08:44:09 -0700 Subject: [PATCH] -Fixed bad window timeline caused by reusing media source on unblocking. -Fixed timeline recovery skipping. -Fixed timeline updates resumes playing when player is paused. --- .../newpipe/player/BackgroundPlayer.java | 4 +- .../org/schabi/newpipe/player/BasePlayer.java | 101 ++++++++++++------ .../newpipe/player/MainVideoPlayer.java | 4 +- .../newpipe/player/PopupVideoPlayer.java | 4 +- .../schabi/newpipe/player/VideoPlayer.java | 21 ++-- .../player/playback/PlaybackListener.java | 30 +++++- .../player/playback/PlaybackManager.java | 9 +- .../schabi/newpipe/util/NavigationHelper.java | 2 +- 8 files changed, 112 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java index c484bab25..1625179d6 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java @@ -458,8 +458,8 @@ public class BackgroundPlayer extends Service { //////////////////////////////////////////////////////////////////////////*/ @Override - public void onLoading() { - super.onLoading(); + public void onBlocked() { + super.onBlocked(); setControlsOpacity(77); updateNotification(-1); diff --git a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java index 7374f3f7a..dc5ed49f5 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java @@ -136,7 +136,7 @@ public abstract class BasePlayer implements Player.EventListener, public static final String RESTORE_WINDOW_POS = "restore_window_pos"; /*////////////////////////////////////////////////////////////////////////// - // Playlist + // Playback //////////////////////////////////////////////////////////////////////////*/ protected PlaybackManager playbackManager; @@ -157,6 +157,7 @@ public abstract class BasePlayer implements Player.EventListener, protected SimpleExoPlayer simpleExoPlayer; protected boolean isPrepared = false; + protected boolean wasPlaying = false; protected CacheDataSourceFactory cacheDataSourceFactory; protected final DefaultExtractorsFactory extractorsFactory = new DefaultExtractorsFactory(); @@ -218,6 +219,7 @@ public abstract class BasePlayer implements Player.EventListener, final RenderersFactory renderFactory = new DefaultRenderersFactory(context); simpleExoPlayer = ExoPlayerFactory.newSimpleInstance(renderFactory, defaultTrackSelector, loadControl); simpleExoPlayer.addListener(this); + simpleExoPlayer.setPlayWhenReady(true); } public void initListeners() {} @@ -275,10 +277,8 @@ public abstract class BasePlayer implements Player.EventListener, } } - playQueue = new ExternalPlayQueue(serviceId, nextPageUrl, info, index); - playQueue.init(); - - playbackManager = new PlaybackManager(this, playQueue); + final PlayQueue queue = new ExternalPlayQueue(serviceId, nextPageUrl, info, index); + initPlayback(this, queue); } @SuppressWarnings("unchecked") @@ -286,9 +286,16 @@ public abstract class BasePlayer implements Player.EventListener, final Serializable serializable = intent.getSerializableExtra(SinglePlayQueue.STREAM); if (!(serializable instanceof StreamInfo)) return; - playQueue = new SinglePlayQueue((StreamInfo) serializable, PlayQueueItem.DEFAULT_QUALITY); - playQueue.init(); + final PlayQueue queue = new SinglePlayQueue((StreamInfo) serializable, PlayQueueItem.DEFAULT_QUALITY); + initPlayback(this, queue); + } + protected void initPlayback(@NonNull final PlaybackListener listener, @NonNull final PlayQueue queue) { + if (playQueue != null) playQueue.dispose(); + if (playbackManager != null) playbackManager.dispose(); + + playQueue = queue; + playQueue.init(); playbackManager = new PlaybackManager(this, playQueue); } @@ -322,6 +329,14 @@ public abstract class BasePlayer implements Player.EventListener, audioManager.abandonAudioFocus(this); audioManager = null; } + if (playQueue != null) { + playQueue.dispose(); + playQueue = null; + } + if (playbackManager != null) { + playbackManager.dispose(); + playbackManager = null; + } } public void destroy() { @@ -440,7 +455,7 @@ public abstract class BasePlayer implements Player.EventListener, // States Implementation //////////////////////////////////////////////////////////////////////////*/ - public static final int STATE_LOADING = 123; + public static final int STATE_BLOCKED = 123; public static final int STATE_PLAYING = 124; public static final int STATE_BUFFERING = 125; public static final int STATE_PAUSED = 126; @@ -454,8 +469,8 @@ public abstract class BasePlayer implements Player.EventListener, if (DEBUG) Log.d(TAG, "changeState() called with: state = [" + state + "]"); currentState = state; switch (state) { - case STATE_LOADING: - onLoading(); + case STATE_BLOCKED: + onBlocked(); break; case STATE_PLAYING: onPlaying(); @@ -475,8 +490,8 @@ public abstract class BasePlayer implements Player.EventListener, } } - public void onLoading() { - if (DEBUG) Log.d(TAG, "onLoading() called"); + public void onBlocked() { + if (DEBUG) Log.d(TAG, "onBlocked() called"); if (!isProgressLoopRunning()) startProgressLoop(); } @@ -497,6 +512,7 @@ public abstract class BasePlayer implements Player.EventListener, public void onCompleted() { if (DEBUG) Log.d(TAG, "onCompleted() called"); + if (playQueue.getIndex() < playQueue.size() - 1) playQueue.offsetIndex(+1); if (isProgressLoopRunning()) stopProgressLoop(); } @@ -536,6 +552,13 @@ public abstract class BasePlayer implements Player.EventListener, final int currentSourceIndex = playbackManager.getCurrentSourceIndex(); + // Sanity check + if (currentSourceIndex < 0) return; + + // Check if already playing correct window + final boolean isCurrentWindowCorrect = simpleExoPlayer.getCurrentWindowIndex() == currentSourceIndex; + if (isCurrentWindowCorrect && getCurrentState() == STATE_PLAYING) return; + // Check timeline has window if (simpleExoPlayer.getCurrentTimeline().getWindowCount() <= currentSourceIndex) return; @@ -544,23 +567,22 @@ public abstract class BasePlayer implements Player.EventListener, simpleExoPlayer.getCurrentTimeline().getWindow(currentSourceIndex, window); if (window.isDynamic) return; - // Check if already playing correct window - final boolean isCurrentWindowCorrect = simpleExoPlayer.getCurrentWindowIndex() == currentSourceIndex; - if (isCurrentWindowCorrect && getCurrentState() == STATE_PLAYING) return; - - // Check if recovering on correct item - if (isRecovery && queuePos == playQueue.getIndex() && isCurrentWindowCorrect) { - if (DEBUG) Log.d(TAG, "Rewinding to recovery window: " + currentSourceIndex + " at: " + getTimeString((int)videoPos)); - simpleExoPlayer.seekTo(currentSourceIndex, videoPos); - isRecovery = false; - } else if (!isCurrentWindowCorrect) { // Or if on wrong window + // Check if on wrong window + if (!isCurrentWindowCorrect) { final long startPos = currentInfo != null ? currentInfo.start_position : 0; if (DEBUG) Log.d(TAG, "Rewinding to correct window: " + currentSourceIndex + " at: " + getTimeString((int)startPos)); simpleExoPlayer.seekTo(currentSourceIndex, startPos); } + // Check if recovering on correct item + if (isRecovery && queuePos == playQueue.getIndex() && isCurrentWindowCorrect) { + if (DEBUG) Log.d(TAG, "Rewinding to recovery window: " + currentSourceIndex + " at: " + getTimeString((int)videoPos)); + simpleExoPlayer.seekTo(videoPos); + isRecovery = false; + } + // Good to go... - simpleExoPlayer.setPlayWhenReady(true); + simpleExoPlayer.setPlayWhenReady(wasPlaying); } @Override @@ -584,8 +606,8 @@ public abstract class BasePlayer implements Player.EventListener, public void onPlayerStateChanged(boolean playWhenReady, int playbackState) { if (DEBUG) Log.d(TAG, "onPlayerStateChanged() called with: playWhenReady = [" + playWhenReady + "], playbackState = [" + playbackState + "]"); - if (getCurrentState() == STATE_PAUSED_SEEK) { - if (DEBUG) Log.d(TAG, "onPlayerStateChanged() currently on PausedSeek"); + if (getCurrentState() == STATE_PAUSED_SEEK || getCurrentState() == STATE_BLOCKED) { + if (DEBUG) Log.d(TAG, "onPlayerStateChanged() is currently blocked"); return; } @@ -594,7 +616,7 @@ public abstract class BasePlayer implements Player.EventListener, isPrepared = false; break; case Player.STATE_BUFFERING: // 2 - if (isPrepared && getCurrentState() != STATE_LOADING) changeState(STATE_BUFFERING); + if (isPrepared) changeState(STATE_BUFFERING); break; case Player.STATE_READY: //3 if (!isPrepared) { @@ -606,12 +628,10 @@ public abstract class BasePlayer implements Player.EventListener, changeState(playWhenReady ? STATE_PLAYING : STATE_PAUSED); break; case Player.STATE_ENDED: // 4 - if (playQueue.getIndex() < playQueue.size() - 1) { - playQueue.offsetIndex(+1); - break; + if (isPrepared) { + changeState(STATE_COMPLETED); + isPrepared = false; } - changeState(STATE_COMPLETED); - isPrepared = false; break; } } @@ -648,10 +668,21 @@ public abstract class BasePlayer implements Player.EventListener, if (simpleExoPlayer == null) return; if (DEBUG) Log.d(TAG, "Blocking..."); + changeState(STATE_BLOCKED); + + wasPlaying = simpleExoPlayer.getPlayWhenReady(); + simpleExoPlayer.setPlayWhenReady(false); + } + + @Override + public void prepare(final MediaSource mediaSource) { + if (simpleExoPlayer == null) return; + if (DEBUG) Log.d(TAG, "Preparing..."); + simpleExoPlayer.stop(); isPrepared = false; - changeState(STATE_BUFFERING); + simpleExoPlayer.prepare(mediaSource); } @Override @@ -659,7 +690,7 @@ public abstract class BasePlayer implements Player.EventListener, if (simpleExoPlayer == null) return; if (DEBUG) Log.d(TAG, "Unblocking..."); - simpleExoPlayer.prepare(playbackManager.getMediaSource(), true, true); + if (getCurrentState() == STATE_BLOCKED) changeState(STATE_BUFFERING); } @Override @@ -874,6 +905,10 @@ public abstract class BasePlayer implements Player.EventListener, return playQueue != null ? playQueue.getIndex() : -1; } + public long getPlayerCurrentPosition() { + return simpleExoPlayer != null ? simpleExoPlayer.getCurrentPosition() : 0L; + } + public PlayQueue getPlayQueue() { return playQueue; } diff --git a/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java b/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java index b33ead1cf..9906ac8e3 100644 --- a/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java @@ -342,8 +342,8 @@ public class MainVideoPlayer extends Activity { //////////////////////////////////////////////////////////////////////////*/ @Override - public void onLoading() { - super.onLoading(); + public void onBlocked() { + super.onBlocked(); playPauseButton.setImageResource(R.drawable.ic_pause_white); animateView(playPauseButton, AnimationUtils.Type.SCALE_AND_ALPHA, false, 100); getRootView().setKeepScreenOn(true); diff --git a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java index 9b672650d..03fd44278 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java @@ -517,8 +517,8 @@ public class PopupVideoPlayer extends Service { //////////////////////////////////////////////////////////////////////////*/ @Override - public void onLoading() { - super.onLoading(); + public void onBlocked() { + super.onBlocked(); updateNotification(R.drawable.ic_play_arrow_white); } diff --git a/app/src/main/java/org/schabi/newpipe/player/VideoPlayer.java b/app/src/main/java/org/schabi/newpipe/player/VideoPlayer.java index 7ed16f3bb..223ea5306 100644 --- a/app/src/main/java/org/schabi/newpipe/player/VideoPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/VideoPlayer.java @@ -104,7 +104,6 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. private static final float[] PLAYBACK_SPEEDS = {0.5f, 0.75f, 1f, 1.25f, 1.5f, 1.75f, 2f}; private boolean startedFromNewPipe = true; - private boolean wasPlaying = false; /*////////////////////////////////////////////////////////////////////////// // Views @@ -223,10 +222,8 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. final int sortedStreamsIndex = intent.getIntExtra(INDEX_SEL_VIDEO_STREAM, -1); - playQueue = new SinglePlayQueue((StreamInfo) serializable, sortedStreamsIndex); - playQueue.init(); - - playbackManager = new PlaybackManager(this, playQueue); + final PlayQueue queue = new SinglePlayQueue((StreamInfo) serializable, sortedStreamsIndex); + initPlayback(this, queue); } @SuppressWarnings("unchecked") @@ -234,10 +231,8 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. final Serializable serializable = intent.getSerializableExtra(PLAY_QUEUE); if (!(serializable instanceof PlayQueue)) return; - playQueue = (PlayQueue) serializable; - playQueue.init(); - - playbackManager = new PlaybackManager(this, playQueue); + final PlayQueue queue = (PlayQueue) serializable; + initPlayback(this, queue); } @Override @@ -304,7 +299,7 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. //////////////////////////////////////////////////////////////////////////*/ @Override - public void onLoading() { + public void onBlocked() { if (DEBUG) Log.d(TAG, "onLoading() called"); if (!isProgressLoopRunning()) startProgressLoop(); @@ -446,7 +441,7 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. protected void onFullScreenButtonClicked() { if (!isPlayerReady()) return; - changeState(STATE_BUFFERING); + changeState(STATE_BLOCKED); } @Override @@ -523,7 +518,7 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. VideoStream videoStream = getSelectedVideoStream(); qualityTextView.setText(MediaFormat.getNameById(videoStream.format) + " " + videoStream.resolution); - wasPlaying = isPlaying(); + wasPlaying = simpleExoPlayer.getPlayWhenReady(); } private void onPlaybackSpeedClicked() { @@ -549,7 +544,7 @@ public abstract class VideoPlayer extends BasePlayer implements SimpleExoPlayer. if (DEBUG) Log.d(TAG, "onStartTrackingTouch() called with: seekBar = [" + seekBar + "]"); if (getCurrentState() != STATE_PAUSED_SEEK) changeState(STATE_PAUSED_SEEK); - wasPlaying = isPlaying(); + wasPlaying = simpleExoPlayer.getPlayWhenReady(); if (isPlaying()) simpleExoPlayer.setPlayWhenReady(false); showControls(0); diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackListener.java b/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackListener.java index 9474fece6..c56c7fe93 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackListener.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackListener.java @@ -8,13 +8,26 @@ public interface PlaybackListener { /* * Called when the stream at the current queue index is not ready yet. * Signals to the listener to block the player from playing anything. + * + * May be called at any time. * */ void block(); + + /* + * Called when the media source is rebuilt. + * Signals to the listener to prepare the media source again. + * The provided media source is always non-empty. + * + * May be called only after blocking and before unblocking. + * */ + void prepare(final MediaSource mediaSource); + /* * Called when the stream at the current queue index is ready. * Signals to the listener to resume the player. - * May be called at any time, even when the player is unblocked. + * + * May be called only when the player is blocked. * */ void unblock(); @@ -23,15 +36,24 @@ public interface PlaybackListener { * Signals to the listener to synchronize the player's window to the manager's * window. * - * CAN ONLY BE CALLED ONCE UNBLOCKED! + * May be called only when playback is unblocked. * */ void sync(final StreamInfo info, final int sortedStreamsIndex); /* - * Requests the listener to resolve a stream info into a media source respective - * of the listener's implementation (background, popup or main video player), + * Requests the listener to resolve a stream info into a media source + * according to the listener's implementation (background, popup or main video player). + * + * May be called at any time. * */ MediaSource sourceOf(final StreamInfo info, final int sortedStreamsIndex); + /* + * Called when the play queue can no longer to played or used. + * Currently, this means the play queue is empty and complete. + * Signals to the listener that it should shutdown. + * + * May be called at any time. + * */ void shutdown(); } diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackManager.java index 72c90600c..a34a38198 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/PlaybackManager.java @@ -71,11 +71,6 @@ public class PlaybackManager { return sourceToQueueIndex.indexOf(playQueue.getIndex()); } - @NonNull - public DynamicConcatenatingMediaSource getMediaSource() { - return sources; - } - public void dispose() { if (playQueueReactor != null) playQueueReactor.cancel(); @@ -109,7 +104,8 @@ public class PlaybackManager { // why no pattern matching in Java =( switch (event.type()) { case INIT: - isBlocked = true; + tryBlock(); + resetSources(); break; case APPEND: break; @@ -245,6 +241,7 @@ public class PlaybackManager { if (this.sourceToQueueIndex != null) this.sourceToQueueIndex.clear(); this.sources = new DynamicConcatenatingMediaSource(); + playbackListener.prepare(this.sources); } /*////////////////////////////////////////////////////////////////////////// diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java index c2a7f83d3..735145749 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -72,7 +72,7 @@ public class NavigationHelper { .putExtra(BasePlayer.INTENT_TYPE, VideoPlayer.PLAYER_INTENT) .putExtra(VideoPlayer.PLAY_QUEUE, instance.getPlayQueue()) .putExtra(VideoPlayer.RESTORE_QUEUE_INDEX, instance.getCurrentQueueIndex()) - .putExtra(BasePlayer.START_POSITION, instance.getPlayer().getCurrentPosition()) + .putExtra(BasePlayer.START_POSITION, instance.getPlayerCurrentPosition()) .putExtra(BasePlayer.PLAYBACK_SPEED, instance.getPlaybackSpeed()); }