From e0a05ef486e1f41abbeca7183cff4171a0ca97ca Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Thu, 7 Jul 2022 11:58:25 -0600 Subject: [PATCH] playback: make sanitization runtime Do not save the playback state when sanitizing. After some thought, there is no situation where re-saving the playback state is desirable. A previously saved state will be consistent with a sanitized state, and there is no need to save when the service is active. Thus, save on speed and reduce insane race conditions by just sanitizing the current runtime state and not saving at all. --- .github/CONTRIBUTING.md | 4 +- .../auxio/music/IndexerNotification.kt | 1 + .../org/oxycblt/auxio/music/IndexerService.kt | 15 ++---- .../org/oxycblt/auxio/music/MusicStore.kt | 1 + .../playback/state/PlaybackStateManager.kt | 50 ++++++++++++------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 1f708053e..32e9ccd9a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -32,7 +32,7 @@ If you have the knowledge, you can also implement the feature yourself and creat Its also recommended that you read about [Auxio's Architecture](../info/ARCHITECTURE.md) as well to make changes better and more efficient. ## Translations -I don't really see the use in weblate, so currently you should see the [Translations Megathread]](https://github.com/OxygenCobalt/Auxio/issues/3) to see how to propose translations. +Go to Auxio's weblate project [here](https://hosted.weblate.org/engage/auxio/). ## Code Contributions If you have knowledge of Android/Kotlin, feel free to to contribute to the project. @@ -44,4 +44,4 @@ If you have knowledge of Android/Kotlin, feel free to to contribute to the proje - Please ***FULLY TEST*** your changes before creating a PR. Untested code will not be merged. - Java code will **NOT** be accepted. Kotlin only. - Keep your code up the date with the upstream and continue to maintain it after you create the PR. This makes it less of a hassle to merge. -- Make sure you have read about the [Accepted Additions and Requests](../info/ADDITIONS.md) before working on your addition. \ No newline at end of file +- Make sure you have read about the [Accepted Additions and Requests](../info/ADDITIONS.md) before working on your addition. diff --git a/app/src/main/java/org/oxycblt/auxio/music/IndexerNotification.kt b/app/src/main/java/org/oxycblt/auxio/music/IndexerNotification.kt index 8078fedd0..3df87c034 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/IndexerNotification.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/IndexerNotification.kt @@ -29,6 +29,7 @@ import org.oxycblt.auxio.util.getSystemServiceSafe import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.newMainPendingIntent +/** The notification responsible for showing the indexer state. */ class IndexerNotification(private val context: Context) : NotificationCompat.Builder(context, CHANNEL_ID) { private val notificationManager = context.getSystemServiceSafe(NotificationManager::class) 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 32df42a61..2c4dccfb7 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt @@ -28,7 +28,6 @@ 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 @@ -118,16 +117,10 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback { // Wipe possibly-invalidated album covers imageLoader.memoryCache?.clear() - // PlaybackStateManager needs to be updated. We would do this in the - // playback module, but this service could be the only component capable - // of doing this at a particular point. Note that while it's certain - // that PlaybackStateManager is initialized by now, it's best to be safe - // and check first. - if (playbackManager.isInitialized) { - playbackManager.sanitize( - PlaybackStateDatabase.getInstance(this@IndexerService), - newLibrary) - } + // 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) } 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 f9832d59d..b98f2f493 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -84,6 +84,7 @@ class MusicStore private constructor() { } fun sanitize(song: Song) = songs.find { it.id == song.id } + fun sanitize(songs: List) = songs.mapNotNull { sanitize(it) } fun sanitize(album: Album) = albums.find { it.id == album.id } fun sanitize(artist: Artist) = artists.find { it.id == artist.id } fun sanitize(genre: Genre) = genres.find { it.id == genre.id } 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 87eea7eb0..91acab17e 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 @@ -384,24 +384,40 @@ class PlaybackStateManager private constructor() { withContext(Dispatchers.IO) { database.write(state) } } - suspend fun sanitize(database: PlaybackStateDatabase, newLibrary: MusicStore.Library) { - // Since we need to sanitize the state and re-save it for consistency, take the - // easy way out and just write a new state and restore from it. Don't really care. - // TODO: Do we even need to save here? Doesn't seem like it's required for - logD("Sanitizing state") - val state = synchronized(this) { makeStateImpl() } - - val sanitizedState = - withContext(Dispatchers.IO) { - database.write(state) - database.read(newLibrary) - } - - synchronized(this) { - if (sanitizedState != null) { - applyStateImpl(sanitizedState) - } + @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) + } + } + + _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) } private fun makeStateImpl() =