From 594fa3597ec8af6749eda4a369eb27bd7acc35b4 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sun, 22 May 2022 11:55:17 -0600 Subject: [PATCH] playback: add delayed action system Replace the hodge-podge framework of state restoration and URI playing with a single "delayed action" system. Auxio's initialization routine is a total cluster----. This is mostly because it involves multiple asynchronous operations such as music loading, state restore, and service starting which tend to make it highly prone to race conditions and other insanity. In particular, the way Auxio would attempt to restore playback and handle file opening was a spaghetti pile of bad API boundaries and dubious UI code. This has not changed. I want to move this routine to the service, but it's lifecycle is also sh------ed to such an extent where that would be nearly impossible. Instead, this commit introduces a new "delayed action" system that bites the bullet and allowes PlaybackViewModel to accept a context and an action in return for initializing playback...eventually. I tried my best to eliminate as much memory leaks as I physically could here, but could only go so far. Still though, even this insane system is better than the UI-level LiveData shenanigans I did previously, and actually works compared to the broken android components that google keeps wanting you to use. --- .../java/org/oxycblt/auxio/MainActivity.kt | 20 ++- .../java/org/oxycblt/auxio/MainFragment.kt | 5 +- .../auxio/playback/PlaybackViewModel.kt | 115 ++++++++++-------- .../playback/state/PlaybackStateManager.kt | 9 +- .../playback/system/MediaSessionComponent.kt | 7 ++ .../auxio/playback/system/PlaybackService.kt | 10 +- .../auxio/settings/pref/M3SwitchPreference.kt | 4 +- 7 files changed, 98 insertions(+), 72 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt index dac057bdf..a4dd62445 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt @@ -18,6 +18,7 @@ package org.oxycblt.auxio import android.content.Intent +import android.net.Uri import android.os.Build import android.os.Bundle import android.view.View @@ -68,13 +69,24 @@ class MainActivity : AppCompatActivity() { startService(Intent(this, PlaybackService::class.java)) - // onNewIntent doesn't automatically call on startup, so call it here. - onNewIntent(intent) + // If we have a file URI already, open it. Otherwise, restore the playback state. + val action = + retrieveViewUri(intent)?.let { PlaybackViewModel.DelayedAction.Open(it) } + ?: PlaybackViewModel.DelayedAction.RestoreState + + playbackModel.performAction(this, action) } override fun onNewIntent(intent: Intent?) { super.onNewIntent(intent) + val uri = retrieveViewUri(intent) + if (uri != null) { + playbackModel.performAction(this, PlaybackViewModel.DelayedAction.Open(uri)) + } + } + + private fun retrieveViewUri(intent: Intent?): Uri? { // If this intent is a valid view intent that has not been used already, give it // to PlaybackViewModel to be used later. if (intent != null) { @@ -84,9 +96,11 @@ class MainActivity : AppCompatActivity() { if (action == Intent.ACTION_VIEW && !isConsumed) { // Mark the intent as used so this does not fire again intent.putExtra(KEY_INTENT_USED, true) - intent.data?.let { fileUri -> playbackModel.play(fileUri, this) } + return intent.data } } + + return null } private fun setupTheme() { diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt index 34395910e..69aeb59e5 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt @@ -117,9 +117,6 @@ class MainFragment : ViewBindingFragment() { // Handle the loader response. when (response) { - // Ok, start restoring playback now - is MusicStore.Response.Ok -> playbackModel.setupPlayback(requireContext()) - // Error, show the error to the user is MusicStore.Response.Err -> { logD("Received Response.Err") @@ -147,7 +144,7 @@ class MainFragment : ViewBindingFragment() { show() } } - null -> {} + else -> {} } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt index f81678d91..fa541e66b 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -34,7 +34,6 @@ import org.oxycblt.auxio.playback.state.PlaybackMode import org.oxycblt.auxio.playback.state.PlaybackStateManager import org.oxycblt.auxio.playback.state.RepeatMode import org.oxycblt.auxio.settings.SettingsManager -import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logE import org.oxycblt.auxio.util.unlikelyToBeNull @@ -46,12 +45,12 @@ import org.oxycblt.auxio.util.unlikelyToBeNull * class.** * @author OxygenCobalt */ -class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { +class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback, MusicStore.Callback { private val musicStore = MusicStore.getInstance() private val settingsManager = SettingsManager.getInstance() private val playbackManager = PlaybackStateManager.getInstance() - private var intentUri: Uri? = null + private var pendingDelayedAction: DelayedActionImpl? = null private val _song = MutableLiveData() /** The current song. */ @@ -82,6 +81,7 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { get() = _nextUp init { + musicStore.addCallback(this) playbackManager.addCallback(this) // If the PlaybackViewModel was cleared [Signified by PlaybackStateManager still being @@ -89,12 +89,53 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { // ViewModel state. If it isn't, then wait for MainFragment to give the command to restore // PlaybackStateManager. if (playbackManager.isInitialized) { - restorePlaybackState() + onNewPlayback(playbackManager.index, playbackManager.queue, playbackManager.parent) + onPositionChanged(playbackManager.positionMs) + onPlayingChanged(playbackManager.isPlaying) + onShuffledChanged(playbackManager.isShuffled) + onRepeatChanged(playbackManager.repeatMode) } } // --- PLAYING FUNCTIONS --- + /** + * Perform the given [DelayedAction]. + * + * A "delayed action" is a class of playback actions that must have music present to function, + * usually alongside a context too. Examples include: + * - Opening files + * - Restoring the playback state + * - Future app shortcuts + * + * We would normally want to put this kind of functionality into PlaybackService, but it's + * lifecycle makes that more or less impossible. + */ + fun performAction(context: Context, action: DelayedAction) { + val library = musicStore.library + val actionImpl = DelayedActionImpl(context.applicationContext, action) + if (library != null) { + performActionImpl(actionImpl, library) + } else { + pendingDelayedAction = actionImpl + } + } + + private fun performActionImpl(action: DelayedActionImpl, library: MusicStore.Library) { + when (action.inner) { + is DelayedAction.RestoreState -> { + if (!playbackManager.isInitialized) { + viewModelScope.launch { playbackManager.restoreState(action.context) } + } + } + is DelayedAction.Open -> { + library + .findSongForUri(action.inner.uri, action.context.contentResolver) + ?.let(::play) + } + } + } + /** * Play a [song] with the [mode] specified. [mode] will default to the preferred song playback * mode of the user if not specified. @@ -142,26 +183,6 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { playbackManager.play(genre, shuffled) } - /** - * Play using a file [uri]. This will not play instantly during the initial startup sequence. - */ - fun play(uri: Uri, context: Context) { - // Check if everything is already running to run the URI play - if (playbackManager.isInitialized && musicStore.library != null) { - playUriImpl(uri, context) - } else { - logD("Cant play this URI right now, waiting") - intentUri = uri - } - } - - /** Play with a file URI. This is called after [play] once its deemed safe to do so. */ - private fun playUriImpl(uri: Uri, context: Context) { - logD("Playing with uri $uri") - val library = musicStore.library ?: return - library.findSongForUri(uri, context.contentResolver)?.let { song -> play(song) } - } - /** Shuffle all songs */ fun shuffleAll() { playbackManager.shuffleAll() @@ -265,42 +286,20 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { } } - /** - * Restore playback on startup. This can do one of two things: - * - Play a file intent that was given by MainActivity in [play] - * - Restore the last playback state if there is no active file intent. - */ - fun setupPlayback(context: Context) { - val intentUri = intentUri - - if (intentUri != null) { - playUriImpl(intentUri, context) - // Remove the uri after finishing the calls so that this does not fire again. - this.intentUri = null - } else if (!playbackManager.isInitialized) { - // Otherwise just restore - viewModelScope.launch { playbackManager.restoreState(context) } - } + /** An action delayed until the complete load of the music library. */ + sealed class DelayedAction { + object RestoreState : DelayedAction() + data class Open(val uri: Uri) : DelayedAction() } - /** - * Attempt to restore the current playback state from an existing [PlaybackStateManager] - * instance. - */ - private fun restorePlaybackState() { - logD("Attempting to restore playback state") - - onNewPlayback(playbackManager.index, playbackManager.queue, playbackManager.parent) - onPositionChanged(playbackManager.positionMs) - onPlayingChanged(playbackManager.isPlaying) - onShuffledChanged(playbackManager.isShuffled) - onRepeatChanged(playbackManager.repeatMode) - } + private data class DelayedActionImpl(val context: Context, val inner: DelayedAction) // --- OVERRIDES --- override fun onCleared() { + musicStore.removeCallback(this) playbackManager.removeCallback(this) + pendingDelayedAction = null } override fun onIndexMoved(index: Int) { @@ -333,4 +332,14 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { override fun onRepeatChanged(repeatMode: RepeatMode) { _repeatMode.value = repeatMode } + + override fun onMusicUpdate(response: MusicStore.Response) { + if (response is MusicStore.Response.Ok) { + val action = pendingDelayedAction + if (action != null) { + performActionImpl(action, response.library) + pendingDelayedAction = null + } + } + } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt index 82e28cf80..303a75bd7 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt @@ -44,9 +44,6 @@ import org.oxycblt.auxio.util.logD * * TODO: Add a controller role and move song loading/seeking to that * - * TODO: Make PlaybackViewModel pass "delayed actions" to this and then await the service to start - * it??? - * * TODO: Bug test app behavior when playback stops */ class PlaybackStateManager private constructor() { @@ -125,6 +122,7 @@ class PlaybackStateManager private constructor() { } applyNewQueue(library, settingsManager.keepShuffle && isShuffled, song) + seekTo(0) notifyNewPlayback() notifyShuffledChanged() isPlaying = true @@ -139,6 +137,7 @@ class PlaybackStateManager private constructor() { val library = musicStore.library ?: return this.parent = parent applyNewQueue(library, shuffled, null) + seekTo(0) notifyNewPlayback() notifyShuffledChanged() isPlaying = true @@ -150,6 +149,7 @@ class PlaybackStateManager private constructor() { val library = musicStore.library ?: return parent = null applyNewQueue(library, true, null) + seekTo(0) notifyNewPlayback() notifyShuffledChanged() isPlaying = true @@ -182,6 +182,7 @@ class PlaybackStateManager private constructor() { private fun goto(idx: Int, play: Boolean) { index = idx + seekTo(0) notifyIndexMoved() isPlaying = play } @@ -261,8 +262,6 @@ class PlaybackStateManager private constructor() { newIndex = keep?.let(newQueue::indexOf) ?: 0 } - logD("$newIndex $newQueue") - _queue = newQueue index = newIndex isShuffled = shuffled diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt index fbf983a6d..a455289bf 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt @@ -54,6 +54,13 @@ class MediaSessionComponent(private val context: Context, private val player: Pl playbackManager.addCallback(this) settingsManager.addCallback(this) mediaSession.setCallback(this) + + if (playbackManager.isInitialized) { + updateMediaMetadata(playbackManager.song) + invalidateSessionState() + onRepeatChanged(playbackManager.repeatMode) + onShuffledChanged(playbackManager.isShuffled) + } } fun release() { diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt index 5002c4dfa..833ef24f0 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt @@ -263,7 +263,6 @@ class PlaybackService : } logD("Loading ${song.rawName}") - player.seekTo(0) player.setMediaItem(MediaItem.fromUri(song.uri)) player.prepare() notificationComponent.updateMetadata(song, playbackManager.parent) @@ -287,6 +286,7 @@ class PlaybackService : } override fun onSeek(positionMs: Long) { + logD("Seeking to ${positionMs}ms") player.seekTo(positionMs) } @@ -374,14 +374,12 @@ class PlaybackService : when (intent.action) { // --- SYSTEM EVENTS --- - // Android has four different ways of handling audio plug events for some reason: + // Android has three different ways of handling audio plug events for some reason: // 1. ACTION_HEADSET_PLUG, which only works with wired headsets - // 2. ACTION_SCO_AUDIO_STATE_UPDATED, which only works with pausing from a plug - // event and I'm not even sure if it's needed - // 3. ACTION_ACL_CONNECTED, which allows headset autoplay but also requires + // 2. ACTION_ACL_CONNECTED, which allows headset autoplay but also requires // granting the BLUETOOTH/BLUETOOTH_CONNECT permissions, which is more or less // a non-starter since both require me to display a permission prompt - // 4. Some weird internal framework thing that also handles bluetooth headsets??? + // 3. Some weird internal framework thing that also handles bluetooth headsets??? // // They should have just stopped at ACTION_HEADSET_PLUG. AudioManager.ACTION_HEADSET_PLUG -> { diff --git a/app/src/main/java/org/oxycblt/auxio/settings/pref/M3SwitchPreference.kt b/app/src/main/java/org/oxycblt/auxio/settings/pref/M3SwitchPreference.kt index 49544b217..6ddc0d22a 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/pref/M3SwitchPreference.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/pref/M3SwitchPreference.kt @@ -29,7 +29,9 @@ import org.oxycblt.auxio.util.getDrawableSafe /** * A [SwitchPreferenceCompat] that emulates the M3 switches until the design team actually bothers - * to add them to MDC TODO: Remove this once MaterialSwitch is stabilized. + * to add them to MDC + * + * TODO: Remove this once MaterialSwitch is stabilized. */ class M3SwitchPreference @JvmOverloads