From 8fd886c273b07c2f5e2b2648c3cdb9de513fe5c4 Mon Sep 17 00:00:00 2001 From: Domingos Lopes Date: Wed, 9 Nov 2016 13:28:50 -0500 Subject: [PATCH] fix unskipped episodes being removed from queue --- .../core/service/playback/LocalPSMP.java | 9 ++++--- .../service/playback/PlaybackService.java | 27 +++++++++++-------- .../playback/PlaybackServiceMediaPlayer.java | 26 ++++++++++-------- .../core/service/playback/RemotePSMP.java | 18 +++++++------ 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java index cad67539b..dd7bba964 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java @@ -146,7 +146,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { if (!media.getIdentifier().equals(playable.getIdentifier())) { final Playable oldMedia = media; - executor.submit(() -> callback.onPostPlayback(oldMedia, false, true)); + executor.submit(() -> callback.onPostPlayback(oldMedia, false, false, true)); } setPlayerStatus(PlayerStatus.INDETERMINATE, null); @@ -758,7 +758,8 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override - protected Future endPlayback(final boolean wasSkipped, final boolean shouldContinue, final boolean toStoppedState) { + protected Future endPlayback(final boolean hasEnded, final boolean wasSkipped, + final boolean shouldContinue, final boolean toStoppedState) { return executor.submit(() -> { playerLock.lock(); releaseWifiLockIfNecessary(); @@ -816,7 +817,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { } final boolean hasNext = nextMedia != null; - executor.submit(() -> callback.onPostPlayback(currentMedia, !wasSkipped, hasNext)); + executor.submit(() -> callback.onPostPlayback(currentMedia, hasEnded, wasSkipped, hasNext)); } else if (isPlaying) { callback.onPlaybackPause(currentMedia, currentMedia.getPosition()); } @@ -883,7 +884,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { mp -> genericOnCompletion(); private void genericOnCompletion() { - endPlayback(false, true, true); + endPlayback(true, false, true, true); } private final MediaPlayer.OnBufferingUpdateListener audioBufferingUpdateListener = diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java index 33aec0ee0..ee6c46130 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java @@ -685,8 +685,9 @@ public class PlaybackService extends MediaBrowserServiceCompat { } @Override - public void onPostPlayback(@NonNull Playable media, boolean ended, boolean playingNext) { - PlaybackService.this.onPostPlayback(media, ended, playingNext); + public void onPostPlayback(@NonNull Playable media, boolean ended, boolean skipped, + boolean playingNext) { + PlaybackService.this.onPostPlayback(media, ended, skipped, playingNext); } @Override @@ -784,16 +785,20 @@ public class PlaybackService extends MediaBrowserServiceCompat { * Even though these tasks aren't supposed to be resource intensive, a good practice is to * usually call this method on a background thread. * - * @param playable the media object that was playing. It is assumed that its position property - * was updated before this method was called. - * @param ended if true, it signals that {@param playable} was played until its end. - * In such case, the position property of the media becomes irrelevant for most of - * the tasks (although it's still a good practice to keep it accurate). - * @param playingNext if true, it means another media object is being loaded in place of this one. + * @param playable the media object that was playing. It is assumed that its position + * property was updated before this method was called. + * @param ended if true, it signals that {@param playable} was played until its end. + * In such case, the position property of the media becomes irrelevant for + * most of the tasks (although it's still a good practice to keep it + * accurate). + * @param skipped if the user pressed a skip >| button. + * @param playingNext if true, it means another media object is being loaded in place of this + * one. * Instances when we'd set it to false would be when we're not following the * queue or when the queue has ended. */ - private void onPostPlayback(final Playable playable, boolean ended, boolean playingNext) { + private void onPostPlayback(final Playable playable, boolean ended, boolean skipped, + boolean playingNext) { if (playable == null) { Log.e(TAG, "Cannot do post-playback processing: media was null"); return; @@ -824,7 +829,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { if (item != null) { if (ended || smartMarkAsPlayed || - !UserPreferences.shouldSkipKeepEpisode()) { + (skipped && !UserPreferences.shouldSkipKeepEpisode())) { // only mark the item as played if we're not keeping it anyways DBWriter.markItemPlayed(item, FeedItem.PLAYED, ended); try { @@ -845,7 +850,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { } } - if (ended || playingNext) { + if (ended || skipped || playingNext) { DBWriter.addItemToPlaybackHistory(media); } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceMediaPlayer.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceMediaPlayer.java index aec059ca0..3b9a11610 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceMediaPlayer.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceMediaPlayer.java @@ -228,32 +228,35 @@ public abstract class PlaybackServiceMediaPlayer { protected abstract void setPlayable(Playable playable); public void skip() { - endPlayback(true, true, true); + endPlayback(false, true, true, true); } /** * Ends playback of current media (if any) and moves into INDETERMINATE state, unless * {@param toStoppedState} is set to true, in which case it moves into STOPPED state. * - * @see #endPlayback(boolean, boolean, boolean) + * @see #endPlayback(boolean, boolean, boolean, boolean) */ public Future stopPlayback(boolean toStoppedState) { - return endPlayback(true, false, toStoppedState); + return endPlayback(false, false, false, toStoppedState); } /** * Internal method that handles end of playback. * - * Currently, it has 4 use cases: + * Currently, it has 5 use cases: * * - * @param wasSkipped If true, we assume the current media's playback has ended, for + * @param hasEnded If true, we assume the current media's playback has ended, for * purposes of post playback processing. + * @param wasSkipped Whether the user chose to skip the episode (by pressing the skip + * button). * @param shouldContinue If true, the media player should try to load, and possibly play, * the next item, based on the user preferences and whether such item * exists. @@ -264,7 +267,8 @@ public abstract class PlaybackServiceMediaPlayer { * * @return a Future, just for the purpose of tracking its execution. */ - protected abstract Future endPlayback(boolean wasSkipped, boolean shouldContinue, boolean toStoppedState); + protected abstract Future endPlayback(boolean hasEnded, boolean wasSkipped, + boolean shouldContinue, boolean toStoppedState); /** * @return {@code true} if the WifiLock feature should be used, {@code false} otherwise. @@ -346,7 +350,7 @@ public abstract class PlaybackServiceMediaPlayer { boolean onMediaPlayerError(Object inObj, int what, int extra); - void onPostPlayback(@NonNull Playable media, boolean ended, boolean playingNext); + void onPostPlayback(@NonNull Playable media, boolean ended, boolean skipped, boolean playingNext); void onPlaybackStart(@NonNull Playable playable, int position); diff --git a/core/src/play/java/de/danoeh/antennapod/core/service/playback/RemotePSMP.java b/core/src/play/java/de/danoeh/antennapod/core/service/playback/RemotePSMP.java index ea95ea894..79e058d0c 100644 --- a/core/src/play/java/de/danoeh/antennapod/core/service/playback/RemotePSMP.java +++ b/core/src/play/java/de/danoeh/antennapod/core/service/playback/RemotePSMP.java @@ -123,7 +123,8 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { } if (playbackEnded) { // This is an unconventional thing to occur... - endPlayback(true, true, true); + Log.w(TAG, "Somehow, Chromecast went from playing directly to standby mode"); + endPlayback(false, false, true, true); } } @@ -239,7 +240,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { if (position >= 0) { oldMedia.setPosition(position); } - callback.onPostPlayback(oldMedia, false, false); + callback.onPostPlayback(oldMedia, false, false, false); } // onPlaybackEnded pretty much takes care of updating the UI return; @@ -261,13 +262,13 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { if (mediaChanged && currentMedia != null) { media = currentMedia; } - endPlayback(false, true, true); + endPlayback(true, false, true, true); return; case MediaStatus.IDLE_REASON_ERROR: Log.w(TAG, "Got an error status from the Chromecast. Skipping, if possible, to the next episode..."); callback.onMediaPlayerInfo(CAST_ERROR_PRIORITY_HIGH, R.string.cast_failed_media_error_skipping); - endPlayback(true, true, true); + endPlayback(false, false, true, true); return; } break; @@ -282,7 +283,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { if (mediaChanged) { callback.onMediaChanged(true); if (oldMedia != null) { - callback.onPostPlayback(oldMedia, false, currentMedia != null); + callback.onPostPlayback(oldMedia, false, false, currentMedia != null); } } } @@ -334,7 +335,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { } if (!media.getIdentifier().equals(playable.getIdentifier())) { final Playable oldMedia = media; - callback.onPostPlayback(oldMedia, false, true); + callback.onPostPlayback(oldMedia, false, false, true); } setPlayerStatus(PlayerStatus.INDETERMINATE, null); @@ -599,7 +600,8 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { } @Override - protected Future endPlayback(boolean wasSkipped, boolean shouldContinue, boolean toStoppedState) { + protected Future endPlayback(boolean hasEnded, boolean wasSkipped, boolean shouldContinue, + boolean toStoppedState) { Log.d(TAG, "endPlayback() called"); boolean isPlaying = playerStatus == PlayerStatus.PLAYING; if (playerStatus != PlayerStatus.INDETERMINATE) { @@ -647,7 +649,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer { } if (shouldPostProcess) { // Otherwise we rely on the chromecast callback to tell us the playback has stopped. - callback.onPostPlayback(currentMedia, !wasSkipped, nextMedia != null); + callback.onPostPlayback(currentMedia, hasEnded, wasSkipped, nextMedia != null); } } else if (isPlaying) { callback.onPlaybackPause(currentMedia,