integrate onPlaybackStart/Pause callbacks into setPlayerStatus()

4 files changed, 50 insertions, 46 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 97d607af6..2567cd6f5 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
@@ -223,7 +223,6 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
                 }
                 mediaPlayer.start();
 
-                callback.onPlaybackStart(media, INVALID_TIME);
                 setPlayerStatus(PlayerStatus.PLAYING, media);
                 pausedBecauseOfTransientAudiofocusLoss = false;
             } else {
@@ -253,8 +252,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
             if (playerStatus == PlayerStatus.PLAYING) {
                 Log.d(TAG, "Pausing playback.");
                 mediaPlayer.pause();
-                callback.onPlaybackPause(media, getPosition());
-                setPlayerStatus(PlayerStatus.PAUSED, media);
+                setPlayerStatus(PlayerStatus.PAUSED, media, getPosition());
 
                 if (abandonFocus) {
                     audioManager.abandonAudioFocus(audioFocusChangeListener);
@@ -373,8 +371,6 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
         if (playerStatus == PlayerStatus.PLAYING
                 || playerStatus == PlayerStatus.PAUSED
                 || playerStatus == PlayerStatus.PREPARED) {
-            statusBeforeSeeking = playerStatus;
-            setPlayerStatus(PlayerStatus.SEEKING, media);
             if(seekLatch != null && seekLatch.getCount() > 0) {
                 try {
                     seekLatch.await(3, TimeUnit.SECONDS);
@@ -383,9 +379,8 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
                 }
             }
             seekLatch = new CountDownLatch(1);
-            if (statusBeforeSeeking == PlayerStatus.PLAYING) {
-                callback.onPlaybackPause(media, getPosition());
-            }
+            statusBeforeSeeking = playerStatus;
+            setPlayerStatus(PlayerStatus.SEEKING, media, getPosition());
             mediaPlayer.seekTo(t);
             try {
                 seekLatch.await(3, TimeUnit.SECONDS);
@@ -934,10 +929,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer {
                 callback.onPlaybackStart(media, getPosition());
             }
             if (playerStatus == PlayerStatus.SEEKING) {
-                if (statusBeforeSeeking == PlayerStatus.PLAYING) {
-                    callback.onPlaybackStart(media, getPosition());
-                }
-                setPlayerStatus(statusBeforeSeeking, media);
+                setPlayerStatus(statusBeforeSeeking, media, getPosition());
             }
             playerLock.unlock();
         });
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 4a8907858..71d0abf07 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
@@ -594,6 +594,7 @@ public class PlaybackService extends MediaBrowserServiceCompat {
                     break;
 
                 case PLAYING:
+                    writePlayerStatusPlaybackPreferences();
                     setupNotification(newInfo);
                     started = true;
                     break;
@@ -675,7 +676,6 @@ public class PlaybackService extends MediaBrowserServiceCompat {
         @Override
         public void onPlaybackStart(@NonNull Playable playable, int position) {
             taskManager.startWidgetUpdater();
-            writePlayerStatusPlaybackPreferences();
             if (position != PlaybackServiceMediaPlayer.INVALID_TIME) {
                 playable.setPosition(position);
             }
@@ -684,12 +684,14 @@ public class PlaybackService extends MediaBrowserServiceCompat {
         }
 
         @Override
-        public void onPlaybackPause(@NonNull Playable playable, int position) {
+        public void onPlaybackPause(Playable playable, int position) {
             taskManager.cancelPositionSaver();
-            saveCurrentPosition(position == PlaybackServiceMediaPlayer.INVALID_TIME,
+            saveCurrentPosition(position == PlaybackServiceMediaPlayer.INVALID_TIME || playable == null,
                     playable, position);
             taskManager.cancelWidgetUpdater();
-            playable.onPlaybackPause(getApplicationContext());
+            if (playable != null) {
+                playable.onPlaybackPause(getApplicationContext());
+            }
         }
 
         @Override
@@ -1578,12 +1580,7 @@ public class PlaybackService extends MediaBrowserServiceCompat {
         public void onPause() {
             Log.d(TAG, "onPause()");
             if (getStatus() == PlayerStatus.PLAYING) {
-                pause(false, true);
-            }
-            if (UserPreferences.isPersistNotify()) {
-                pause(false, true);
-            } else {
-                pause(true, true);
+                pause(!UserPreferences.isPersistNotify(), true);
             }
         }
 
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 c7127ddb7..5940a463a 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
@@ -267,23 +267,41 @@ public abstract class PlaybackServiceMediaPlayer {
      * <p/>
      * This method will notify the callback about the change of the player status (even if the new status is the same
      * as the old one).
+     * <p/>
+     * It will also call {@link PSMPCallback#onPlaybackPause(Playable, int)} or {@link PSMPCallback#onPlaybackStart(Playable, int)}
+     * depending on the status change.
      *
      * @param newStatus The new PlayerStatus. This must not be null.
      * @param newMedia  The new playable object of the PSMP object. This can be null.
+     * @param position  The position to be set to the current Playable object in case playback started or paused.
+     *                  Will be ignored if given the value of {@link #INVALID_TIME}.
      */
-    protected final synchronized void setPlayerStatus(@NonNull PlayerStatus newStatus, Playable newMedia) {
+    protected final synchronized void setPlayerStatus(@NonNull PlayerStatus newStatus, Playable newMedia, int position) {
         Log.d(TAG, this.getClass().getSimpleName() + ": Setting player status to " + newStatus);
 
+        PlayerStatus oldStatus = playerStatus;
+
         this.playerStatus = newStatus;
         setPlayable(newMedia);
 
-        if (playerStatus != null) {
-            Log.d(TAG, "playerStatus: " + playerStatus.toString());
+        if (newMedia != null && newStatus != PlayerStatus.INDETERMINATE) {
+            if (oldStatus == PlayerStatus.PLAYING && newStatus != PlayerStatus.PLAYING) {
+                callback.onPlaybackPause(newMedia, position);
+            } else if (oldStatus != PlayerStatus.PLAYING && newStatus == PlayerStatus.PLAYING) {
+                callback.onPlaybackStart(newMedia, position);
+            }
         }
 
         callback.statusChanged(new PSMPInfo(playerStatus, getPlayable()));
     }
 
+    /**
+     * @see #setPlayerStatus(PlayerStatus, Playable, int)
+     */
+    protected final void setPlayerStatus(@NonNull PlayerStatus newStatus, Playable newMedia) {
+        setPlayerStatus(newStatus, newMedia, INVALID_TIME);
+    }
+
     public interface PSMPCallback {
         void statusChanged(PSMPInfo newInfo);
 
@@ -305,7 +323,7 @@ public abstract class PlaybackServiceMediaPlayer {
 
         void onPlaybackStart(@NonNull Playable playable, int position);
 
-        void onPlaybackPause(@NonNull Playable playable, int position);
+        void onPlaybackPause(Playable playable, int position);
 
         Playable getNextInQueue(Playable currentMedia);
 
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 8982e85f3..ee6cb653c 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
@@ -197,37 +197,34 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
             remoteState = state;
         }
 
+        if (mediaChanged && stateChanged && oldState == MediaStatus.PLAYER_STATE_PLAYING &&
+                state != MediaStatus.PLAYER_STATE_IDLE) {
+            callback.onPlaybackPause(null, INVALID_TIME);
+            // We don't want setPlayerStatus to handle the onPlaybackPause callback
+            setPlayerStatus(PlayerStatus.INDETERMINATE, currentMedia);
+        }
+
         setBuffering(state == MediaStatus.PLAYER_STATE_BUFFERING);
 
         switch (state) {
             case MediaStatus.PLAYER_STATE_PLAYING:
                 if (!stateChanged) {
+                    //These steps are necessary because they won't be performed by setPlayerStatus()
                     if (position >= 0) {
                         currentMedia.setPosition(position);
                     }
                     currentMedia.onPlaybackStart();
-                } else {
-                    callback.onPlaybackStart(currentMedia, position);
                 }
-                setPlayerStatus(PlayerStatus.PLAYING, currentMedia);
+                setPlayerStatus(PlayerStatus.PLAYING, currentMedia, position);
                 break;
             case MediaStatus.PLAYER_STATE_PAUSED:
-                if (!mediaChanged &&
-                        oldState == MediaStatus.PLAYER_STATE_PLAYING) {
-                    callback.onPlaybackPause(currentMedia, position);
-                }
-                setPlayerStatus(PlayerStatus.PAUSED, currentMedia);
+                setPlayerStatus(PlayerStatus.PAUSED, currentMedia, position);
                 break;
             case MediaStatus.PLAYER_STATE_BUFFERING:
-                if (!mediaChanged && currentMedia != null &&
-                        oldState == MediaStatus.PLAYER_STATE_PLAYING) {
-                    // position could already refer to the new position after seeking
-                    // so our best guess is the last one reported
-                    callback.onPlaybackPause(currentMedia, position);
-                }
                 setPlayerStatus((mediaChanged || playerStatus == PlayerStatus.PREPARING) ?
                         PlayerStatus.PREPARING : PlayerStatus.SEEKING,
-                        currentMedia);
+                        currentMedia,
+                        currentMedia != null ? currentMedia.getPosition() : INVALID_TIME);
                 break;
             case MediaStatus.PLAYER_STATE_IDLE:
                 int reason = status.getIdleReason();
@@ -247,8 +244,9 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
                     case MediaStatus.IDLE_REASON_INTERRUPTED:
                         // Means that a request to load a different media was sent
                         // Not sure if currentMedia already reflects the to be loaded one
-                        if (oldState == MediaStatus.PLAYER_STATE_PLAYING && oldMedia != null) {
-                            callback.onPlaybackPause(oldMedia, oldMedia.getPosition());
+                        if (mediaChanged && oldState == MediaStatus.PLAYER_STATE_PLAYING) {
+                            callback.onPlaybackPause(null, INVALID_TIME);
+                            setPlayerStatus(PlayerStatus.INDETERMINATE, currentMedia);
                         }
                         setPlayerStatus(PlayerStatus.PREPARING, currentMedia);
                         break;
@@ -272,10 +270,9 @@ public class RemotePSMP extends PlaybackServiceMediaPlayer {
                 }
                 break;
             case MediaStatus.PLAYER_STATE_UNKNOWN:
-                if (oldState == MediaStatus.PLAYER_STATE_PLAYING && oldMedia != null) {
-                    callback.onPlaybackPause(oldMedia, position);
+                if (playerStatus != PlayerStatus.INDETERMINATE || media != currentMedia) {
+                    setPlayerStatus(PlayerStatus.INDETERMINATE, currentMedia);
                 }
-                setPlayerStatus(PlayerStatus.INDETERMINATE, currentMedia);
                 break;
             default:
                 Log.wtf(TAG, "Remote media state undetermined!");