Fixed an issue causing the service to shutdown while music is playing..

- Fixed an issue where playback wouldn't start if the queue wasn't yet loaded - If play() is called while the queue is currently reloading, the service shutdown isn't cancelled (because nothing is playing). When play() eventually is called after the reload is complete, we are now cancelling shutdown. - releaseServiceUIAndStop() now checks whether any music is currently playing, or if music is scheduled to be playing in the near future (like, when a phone call ends), and won't shutdown under these circumstances. Resolves #358 Possible fix for #343
timusus · Dec 6, 2018 · 2b9111b · 2b9111b
1 parent 72585e3
commit 2b9111b
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 12 deletions.
diff --git a/app/src/main/java/com/simplecity/amp_library/playback/MusicService.java b/app/src/main/java/com/simplecity/amp_library/playback/MusicService.java
@@ -151,7 +151,7 @@ public void onCreate() {
         AnalyticsManager.dropBreadcrumb(TAG, "onCreate(), scheduling delayed shutdown");
         scheduleDelayedShutdown();
 
-        reloadQueue();
+        playbackManager.reloadQueue();
     }
 
     @Override
@@ -432,6 +432,11 @@ public String commandToAction(@NonNull String command) {
      */
     void releaseServiceUiAndStop() {
 
+        // If we're currently playing, or we're going to be playing in the near future, don't shutdown.
+        if (isPlaying() || playbackManager.willResumePlayback()) {
+            return;
+        }
+
         AnalyticsManager.dropBreadcrumb(TAG, "releaseServiceUiAndStop()");
 
         playbackManager.release();
@@ -476,7 +481,7 @@ public void onReceive(Context context, Intent intent) {
                         closeExternalStorageFiles();
                     } else if (Intent.ACTION_MEDIA_MOUNTED.equals(action)) {
                         queueManager.queueIsSaveable = true;
-                        reloadQueue();
+                        playbackManager.reloadQueue();
                     }
                 }
             };
@@ -729,10 +734,6 @@ public void clearQueue() {
         playbackManager.clearQueue();
     }
 
-    void reloadQueue() {
-        playbackManager.reloadQueue(false);
-    }
-
     public List<QueueItem> getQueue() {
         return queueManager.getCurrentPlaylist();
     }

diff --git a/app/src/main/java/com/simplecity/amp_library/playback/PlaybackManager.java b/app/src/main/java/com/simplecity/amp_library/playback/PlaybackManager.java
@@ -50,6 +50,8 @@ public class PlaybackManager implements Playback.Callbacks {
 
     private MusicService.Callbacks musicServiceCallbacks;
 
+    private boolean playOnQueueReload = false;
+
     @NonNull
     Playback playback;
 
@@ -96,15 +98,17 @@ void clearQueue() {
         queueManager.clearQueue();
     }
 
-    void reloadQueue(boolean playWhenReady) {
+    void reloadQueue() {
         if (ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
             return;
         }
 
         disposables.add(queueManager.reloadQueue(() -> {
-            load(true, playWhenReady, PlaybackSettingsManager.INSTANCE.getSeekPosition());
-            return Unit.INSTANCE;
-        }));
+                    load(true, playOnQueueReload, PlaybackSettingsManager.INSTANCE.getSeekPosition());
+                    playOnQueueReload = false;
+                    return Unit.INSTANCE;
+                })
+        );
     }
 
     void removeQueueItems(List<QueueItem> queueItems) {
@@ -257,6 +261,9 @@ private void loadAttempt(int attempt, Boolean playWhenReady, long seekPosition,
     private void load(@NonNull Song song, Boolean playWhenReady, long seekPosition, @Nullable Function1<Boolean, Unit> completion) {
         playback.load(song, playWhenReady, seekPosition, success -> {
             if (success) {
+                if (playWhenReady) {
+                    musicServiceCallbacks.cancelShutdown();
+                }
                 if (completion != null) {
                     completion.invoke(true);
                 }
@@ -484,8 +491,7 @@ public void play() {
         } else if (queueManager.getCurrentPlaylist().isEmpty()) {
             // This is mostly so that if you press 'play' on a bluetooth headset without ever having played anything before, it will still play something.
             if (queueManager.queueReloading) {
-                // Todo: This doesn't make sense. If our queue is already reloading we just want to play once it's finished. Not reload the queue all over again.
-                reloadQueue(true);
+                playOnQueueReload = true;
             } else {
                 playAutoShuffleList();
             }

diff --git a/app/src/main/java/com/simplecity/amp_library/playback/QueueManager.java b/app/src/main/java/com/simplecity/amp_library/playback/QueueManager.java
@@ -12,7 +12,9 @@
 import com.simplecity.amp_library.utils.DataManager;
 import com.simplecity.amp_library.utils.LogUtils;
 import com.simplecity.amp_library.utils.SettingsManager;
+import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.Disposable;
+import io.reactivex.schedulers.Schedulers;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -383,6 +385,8 @@ Disposable reloadQueue(@NonNull Function0<Unit> onComplete) {
         return DataManager.getInstance().getSongsRelay()
                 .first(Collections.emptyList())
                 .map(QueueItemKt::toQueueItems)
+                .subscribeOn(Schedulers.io())
+                .observeOn(AndroidSchedulers.mainThread())
                 .subscribe((UnsafeConsumer<List<QueueItem>>) queueItems -> {
                     String queueList = PlaybackSettingsManager.INSTANCE.getQueueList();
                     if (queueList != null) {