From 2b9111b6160f877fe39ddee7ca3bace0e1f05f8b Mon Sep 17 00:00:00 2001 From: Tim Malseed Date: Thu, 6 Dec 2018 19:15:38 +1100 Subject: [PATCH] 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 --- .../amp_library/playback/MusicService.java | 13 +++++++------ .../amp_library/playback/PlaybackManager.java | 18 ++++++++++++------ .../amp_library/playback/QueueManager.java | 4 ++++ 3 files changed, 23 insertions(+), 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 index d09add9a3..d2fdb8ac6 100644 --- 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 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 index c618dc8b9..6242c0dbd 100644 --- 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 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 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 index 42bef3466..2e6127dbd 100644 --- 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 onComplete) { return DataManager.getInstance().getSongsRelay() .first(Collections.emptyList()) .map(QueueItemKt::toQueueItems) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) .subscribe((UnsafeConsumer>) queueItems -> { String queueList = PlaybackSettingsManager.INSTANCE.getQueueList(); if (queueList != null) {