Merge pull request #2182 from domingos86/fix-episodes-deleted

fix unskipped episodes being removed from queue
This commit is contained in:
Martin Fietz 2016-11-12 09:10:57 +01:00 committed by GitHub
commit ddef76a9cf
4 changed files with 46 additions and 34 deletions

View File

@ -146,7 +146,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
if (!media.getIdentifier().equals(playable.getIdentifier())) { if (!media.getIdentifier().equals(playable.getIdentifier())) {
final Playable oldMedia = media; final Playable oldMedia = media;
executor.submit(() -> callback.onPostPlayback(oldMedia, false, true)); executor.submit(() -> callback.onPostPlayback(oldMedia, false, false, true));
} }
setPlayerStatus(PlayerStatus.INDETERMINATE, null); setPlayerStatus(PlayerStatus.INDETERMINATE, null);
@ -758,7 +758,8 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
@Override @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(() -> { return executor.submit(() -> {
playerLock.lock(); playerLock.lock();
releaseWifiLockIfNecessary(); releaseWifiLockIfNecessary();
@ -816,7 +817,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
} }
final boolean hasNext = nextMedia != null; final boolean hasNext = nextMedia != null;
executor.submit(() -> callback.onPostPlayback(currentMedia, !wasSkipped, hasNext)); executor.submit(() -> callback.onPostPlayback(currentMedia, hasEnded, wasSkipped, hasNext));
} else if (isPlaying) { } else if (isPlaying) {
callback.onPlaybackPause(currentMedia, currentMedia.getPosition()); callback.onPlaybackPause(currentMedia, currentMedia.getPosition());
} }
@ -883,7 +884,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
mp -> genericOnCompletion(); mp -> genericOnCompletion();
private void genericOnCompletion() { private void genericOnCompletion() {
endPlayback(false, true, true); endPlayback(true, false, true, true);
} }
private final MediaPlayer.OnBufferingUpdateListener audioBufferingUpdateListener = private final MediaPlayer.OnBufferingUpdateListener audioBufferingUpdateListener =

View File

@ -685,8 +685,9 @@ public class PlaybackService extends MediaBrowserServiceCompat {
} }
@Override @Override
public void onPostPlayback(@NonNull Playable media, boolean ended, boolean playingNext) { public void onPostPlayback(@NonNull Playable media, boolean ended, boolean skipped,
PlaybackService.this.onPostPlayback(media, ended, playingNext); boolean playingNext) {
PlaybackService.this.onPostPlayback(media, ended, skipped, playingNext);
} }
@Override @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 * Even though these tasks aren't supposed to be resource intensive, a good practice is to
* usually call this method on a background thread. * usually call this method on a background thread.
* *
* @param playable the media object that was playing. It is assumed that its position property * @param playable the media object that was playing. It is assumed that its position
* was updated before this method was called. * property was updated before this method was called.
* @param ended if true, it signals that {@param playable} was played until its end. * @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 * In such case, the position property of the media becomes irrelevant for
* the tasks (although it's still a good practice to keep it accurate). * most of the tasks (although it's still a good practice to keep it
* @param playingNext if true, it means another media object is being loaded in place of this one. * 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 * Instances when we'd set it to false would be when we're not following the
* queue or when the queue has ended. * 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) { if (playable == null) {
Log.e(TAG, "Cannot do post-playback processing: media was null"); Log.e(TAG, "Cannot do post-playback processing: media was null");
return; return;
@ -824,7 +829,7 @@ public class PlaybackService extends MediaBrowserServiceCompat {
if (item != null) { if (item != null) {
if (ended || smartMarkAsPlayed || if (ended || smartMarkAsPlayed ||
!UserPreferences.shouldSkipKeepEpisode()) { (skipped && !UserPreferences.shouldSkipKeepEpisode())) {
// only mark the item as played if we're not keeping it anyways // only mark the item as played if we're not keeping it anyways
DBWriter.markItemPlayed(item, FeedItem.PLAYED, ended); DBWriter.markItemPlayed(item, FeedItem.PLAYED, ended);
try { try {
@ -845,7 +850,7 @@ public class PlaybackService extends MediaBrowserServiceCompat {
} }
} }
if (ended || playingNext) { if (ended || skipped || playingNext) {
DBWriter.addItemToPlaybackHistory(media); DBWriter.addItemToPlaybackHistory(media);
} }
} }

View File

@ -228,32 +228,35 @@ public abstract class PlaybackServiceMediaPlayer {
protected abstract void setPlayable(Playable playable); protected abstract void setPlayable(Playable playable);
public void skip() { 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 * 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. * {@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) { public Future<?> stopPlayback(boolean toStoppedState) {
return endPlayback(true, false, toStoppedState); return endPlayback(false, false, false, toStoppedState);
} }
/** /**
* Internal method that handles end of playback. * Internal method that handles end of playback.
* *
* Currently, it has 4 use cases: * Currently, it has 5 use cases:
* <ul> * <ul>
* <li>Media playback has completed: call with (false, true, true)</li> * <li>Media playback has completed: call with (true, false, true, true)</li>
* <li>User asks to skip to next episode: call with (true, true, true)</li> * <li>User asks to skip to next episode: call with (false, true, true, true)</li>
* <li>Stopping the media player: call with (true, false, true)</li> * <li>Skipping to next episode due to playback error: call with (false, false, true, true)</li>
* <li>We want to change the media player implementation: call with (true, false, false)</li> * <li>Stopping the media player: call with (false, false, false, true)</li>
* <li>We want to change the media player implementation: call with (false, false, false, false)</li>
* </ul> * </ul>
* *
* @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. * 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, * @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 * the next item, based on the user preferences and whether such item
* exists. * exists.
@ -264,7 +267,8 @@ public abstract class PlaybackServiceMediaPlayer {
* *
* @return a Future, just for the purpose of tracking its execution. * @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. * @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); 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); void onPlaybackStart(@NonNull Playable playable, int position);

View File

@ -123,7 +123,8 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
} }
if (playbackEnded) { if (playbackEnded) {
// This is an unconventional thing to occur... // 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) { if (position >= 0) {
oldMedia.setPosition(position); oldMedia.setPosition(position);
} }
callback.onPostPlayback(oldMedia, false, false); callback.onPostPlayback(oldMedia, false, false, false);
} }
// onPlaybackEnded pretty much takes care of updating the UI // onPlaybackEnded pretty much takes care of updating the UI
return; return;
@ -261,13 +262,13 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
if (mediaChanged && currentMedia != null) { if (mediaChanged && currentMedia != null) {
media = currentMedia; media = currentMedia;
} }
endPlayback(false, true, true); endPlayback(true, false, true, true);
return; return;
case MediaStatus.IDLE_REASON_ERROR: case MediaStatus.IDLE_REASON_ERROR:
Log.w(TAG, "Got an error status from the Chromecast. Skipping, if possible, to the next episode..."); Log.w(TAG, "Got an error status from the Chromecast. Skipping, if possible, to the next episode...");
callback.onMediaPlayerInfo(CAST_ERROR_PRIORITY_HIGH, callback.onMediaPlayerInfo(CAST_ERROR_PRIORITY_HIGH,
R.string.cast_failed_media_error_skipping); R.string.cast_failed_media_error_skipping);
endPlayback(true, true, true); endPlayback(false, false, true, true);
return; return;
} }
break; break;
@ -282,7 +283,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
if (mediaChanged) { if (mediaChanged) {
callback.onMediaChanged(true); callback.onMediaChanged(true);
if (oldMedia != null) { 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())) { if (!media.getIdentifier().equals(playable.getIdentifier())) {
final Playable oldMedia = media; final Playable oldMedia = media;
callback.onPostPlayback(oldMedia, false, true); callback.onPostPlayback(oldMedia, false, false, true);
} }
setPlayerStatus(PlayerStatus.INDETERMINATE, null); setPlayerStatus(PlayerStatus.INDETERMINATE, null);
@ -599,7 +600,8 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
} }
@Override @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"); Log.d(TAG, "endPlayback() called");
boolean isPlaying = playerStatus == PlayerStatus.PLAYING; boolean isPlaying = playerStatus == PlayerStatus.PLAYING;
if (playerStatus != PlayerStatus.INDETERMINATE) { if (playerStatus != PlayerStatus.INDETERMINATE) {
@ -647,7 +649,7 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
} }
if (shouldPostProcess) { if (shouldPostProcess) {
// Otherwise we rely on the chromecast callback to tell us the playback has stopped. // 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) { } else if (isPlaying) {
callback.onPlaybackPause(currentMedia, callback.onPlaybackPause(currentMedia,