From a217bde713d58e501ee177e86ed3c72ec57b9be1 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Thu, 7 Jul 2022 15:07:23 -0600 Subject: [PATCH] playback: save state during sanitize Save, but do not read the playback state when sanitizing. Turns out there is an edge-case where we want to save the state. Still keep the runtime sanitization, as that greatly reduces the time it takes to rescan the library. --- .../org/oxycblt/auxio/music/IndexerService.kt | 8 +- .../org/oxycblt/auxio/music/MusicStore.kt | 5 + .../playback/state/PlaybackStateDatabase.kt | 1 + .../playback/state/PlaybackStateManager.kt | 112 +++++++++--------- 4 files changed, 69 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt b/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt index 2c4dccfb7..0791755c6 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.launch import org.oxycblt.auxio.IntegerTable import org.oxycblt.auxio.R +import org.oxycblt.auxio.playback.state.PlaybackStateDatabase import org.oxycblt.auxio.playback.state.PlaybackStateManager import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.util.logD @@ -117,10 +118,9 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback { // Wipe possibly-invalidated album covers imageLoader.memoryCache?.clear() - // Clear invalid models from PlaybackStateManager. Shared objects - // shouldn't be plugged into the callback system of other shared - // objects, so we must update it here. - playbackManager.sanitize(newLibrary) + // Clear invalid models from PlaybackStateManager. + playbackManager.sanitize( + PlaybackStateDatabase.getInstance(this@IndexerService), newLibrary) } musicStore.updateLibrary(newLibrary) diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt index b98f2f493..9580bea5b 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -83,10 +83,15 @@ class MusicStore private constructor() { songs.find { it.path.name == displayName } } + /** Sanitize an old item to find the corresponding item in a new library. */ fun sanitize(song: Song) = songs.find { it.id == song.id } + /** Sanitize an old item to find the corresponding item in a new library. */ fun sanitize(songs: List) = songs.mapNotNull { sanitize(it) } + /** Sanitize an old item to find the corresponding item in a new library. */ fun sanitize(album: Album) = albums.find { it.id == album.id } + /** Sanitize an old item to find the corresponding item in a new library. */ fun sanitize(artist: Artist) = artists.find { it.id == artist.id } + /** Sanitize an old item to find the corresponding item in a new library. */ fun sanitize(genre: Genre) = genres.find { it.id == genre.id } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt index eaf9c7337..1fed5f1f8 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt @@ -107,6 +107,7 @@ class PlaybackStateDatabase private constructor(context: Context) : val rawState = readRawState() ?: return null val queue = readQueue(library) + // Correct the index to match up with a possibly shortened queue (file removals/changes) var actualIndex = rawState.index while (queue.getOrNull(actualIndex)?.id != rawState.songId && actualIndex > -1) { actualIndex-- 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 91acab17e..bbb7dec33 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 @@ -30,7 +30,6 @@ import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logW -import org.oxycblt.auxio.util.unlikelyToBeNull /** * Master class (and possible god object) for the playback state. @@ -364,10 +363,27 @@ class PlaybackStateManager private constructor() { val state = withContext(Dispatchers.IO) { database.read(library) } synchronized(this) { - val exists = state != null - if (exists) { - applyStateImpl(unlikelyToBeNull(state)) - } + val exists = + if (state != null) { + // Continuing playback while also possibly doing drastic state updates is + // a bad idea, so pause. + isPlaying = false + + index = state.index + parent = state.parent + _queue = state.queue.toMutableList() + repeatMode = state.repeatMode + isShuffled = state.isShuffled + + notifyNewPlayback() + seekTo(state.positionMs) + notifyRepeatModeChanged() + notifyShuffledChanged() + + true + } else { + false + } isInitialized = true @@ -384,40 +400,47 @@ class PlaybackStateManager private constructor() { withContext(Dispatchers.IO) { database.write(state) } } - @Synchronized - fun sanitize(newLibrary: MusicStore.Library) { - if (!isInitialized) { - logD("Not initialized, no need to sanitize") - return - } - - logD("Sanitizing state") - - val oldSongId = song?.id - val oldPosition = positionMs - - parent = - parent?.let { - when (it) { - is Album -> newLibrary.sanitize(it) - is Artist -> newLibrary.sanitize(it) - is Genre -> newLibrary.sanitize(it) + /** Sanitize the state with [newLibrary]. Writes the state to [database] */ + suspend fun sanitize(database: PlaybackStateDatabase, newLibrary: MusicStore.Library) { + val state = + synchronized(this) { + if (!isInitialized) { + logD("Not initialized, no need to sanitize") + return } + + logD("Sanitizing state") + + val oldSongId = song?.id + val oldPosition = positionMs + + parent = + parent?.let { + when (it) { + is Album -> newLibrary.sanitize(it) + is Artist -> newLibrary.sanitize(it) + is Genre -> newLibrary.sanitize(it) + } + } + + _queue = newLibrary.sanitize(_queue).toMutableList() + + while (song?.id != oldSongId && index > -1) { + index-- + } + + // Continuing playback while also possibly doing drastic state updates is + // a bad idea, so pause. + isPlaying = false + notifyNewPlayback() + + // Controller may have reloaded the media item, re-seek to the previous position + seekTo(oldPosition) + + makeStateImpl() } - _queue = newLibrary.sanitize(_queue).toMutableList() - - while (song?.id != oldSongId && index > -1) { - index-- - } - - // Continuing playback while also possibly doing drastic state updates is - // a bad idea, so pause. - isPlaying = false - notifyNewPlayback() - - // Controller may have reloaded the media item, re-seek to the previous position - seekTo(oldPosition) + withContext(Dispatchers.IO) { database.write(state) } } private fun makeStateImpl() = @@ -429,23 +452,6 @@ class PlaybackStateManager private constructor() { isShuffled = isShuffled, repeatMode = repeatMode) - private fun applyStateImpl(state: PlaybackStateDatabase.SavedState) { - // Continuing playback while also possibly doing drastic state updates is - // a bad idea, so pause. - isPlaying = false - - index = state.index - parent = state.parent - _queue = state.queue.toMutableList() - repeatMode = state.repeatMode - isShuffled = state.isShuffled - - notifyNewPlayback() - seekTo(state.positionMs) - notifyRepeatModeChanged() - notifyShuffledChanged() - } - // --- CALLBACKS --- private fun notifyIndexMoved() {