music: more hypothetical fixes

Fix a bunch of more hypothetical issues that could occur with runtime
rescanning. It's still global mutable concurrent state though. Sigh.

CHANGELOG.md

@@ -8,7 +8,7 @@
 - Added a shuffle shortcut
 - Widgets now have a more sleek and consistent button layout
 - "Rounded album covers" is now "Round mode"
-- You can now customize what occurs when a song is played from an album/artist/genre [#164]
+- Added option to customize what occurs when a song is played from an album/artist/genre [#164]
 
 #### What's Improved
 - Made "timeline" elements (like playback controls) always left-to-right

app/src/main/java/org/oxycblt/auxio/music/Indexer.kt

@@ -167,10 +167,9 @@ class Indexer {
     }
 
     /**
-     * "Cancel" the last job by making it unable to send further state updates. This should be
-     * called if an object that called [index] is about to be destroyed and thus will have it's task
-     * canceled, in which it would be useful for any ongoing loading process to not accidentally
-     * corrupt the current state.
+     * "Cancel" the last job by making it unable to send further state updates. This will cause the
+     * worker operating the job for that specific generation to cancel as soon as it tries to send a
+     * state update.
      */
     @Synchronized
     fun cancelLast() {
@@ -182,11 +181,17 @@ class Indexer {
     @Synchronized
     private fun emitIndexing(indexing: Indexing?, generation: Long) {
         if (currentGeneration != generation) {
-            // Not the running task anymore, cancel this co-routine
-            // We do this instead of using yield since it is *far* cheaper.
+            // Not the running task anymore, cancel this co-routine. This allows a yield-like
+            // behavior to be implemented in a far cheaper manner for each backend.
             throw CancellationException()
         }
 
+        if (indexing == indexingState) {
+            // Ignore redundant states used when the backends just want to check for
+            // a cancellation
+            return
+        }
+
         indexingState = indexing
 
         // If we have canceled the loading process, we want to revert to a previous completion
@@ -201,11 +206,14 @@ class Indexer {
     @Synchronized
     private fun emitCompletion(response: Response, generation: Long) {
         if (currentGeneration != generation) {
-            // Not the running task anymore, cancel this co-routine
-            // We do this instead of using yield since it is *far* cheaper.
+            // Not the running task anymore, cancel this co-routine. This allows a yield-like
+            // behavior to be implemented in a far cheaper manner for each backend.
             throw CancellationException()
         }
 
+        // Do not check for redundancy here, as we actually need to notify a switch
+        // from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
+
         lastResponse = response
         indexingState = null
 
@@ -284,9 +292,7 @@ class Indexer {
                     "Successfully queried media database " +
                         "in ${System.currentTimeMillis() - start}ms")
 
-                backend.buildSongs(context, cursor) { indexing ->
-                    emitIndexing(indexing, generation)
-                }
+                backend.buildSongs(context, cursor) { emitIndexing(it, generation) }
             }
 
         // Deduplicate songs to prevent (most) deformed music clones

app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt

@@ -112,17 +112,15 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
                     // Load was completed successfully. However, we still need to do some
                     // extra work to update the app's state.
                     updateScope.launch {
-                        // Invalidate the image cache, as there may be some covers that are
-                        // no longer valid.
-                        imageLoader.memoryCache?.clear()
-
                         if (musicStore.library != null) {
-                            // This is a new library, so we need to make sure the playback state
-                            // is coherent. This seems a bit muddly, but PlaybackService (or any
-                            // other playback component capable of handling this) may not be
-                            // capable of long-running background work as the library is being
-                            // updated. The initialization steps on the other hand are firmly in
-                            // the place of the playback module.
+                            // This is a new library to replace a pre-existing one.
+
+                            // 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 is the only component
+                            // capable of doing long-running background work as it stands.
                             playbackManager.sanitize(
                                 PlaybackStateDatabase.getInstance(this@IndexerService), newLibrary)
                         }

app/src/main/java/org/oxycblt/auxio/music/backend/ExoPlayerBackend.kt

@@ -242,7 +242,6 @@ class Task(context: Context, private val audio: MediaStoreBackend.Audio) {
     }
 
     private fun populateVorbis(tags: Map<String, String>) {
-        logD(tags)
         // Title
         tags["TITLE"]?.let { audio.title = it }
 

app/src/main/java/org/oxycblt/auxio/music/backend/MediaStoreBackend.kt

@@ -177,13 +177,13 @@ abstract class MediaStoreBackend : Indexer.Backend {
         cursor: Cursor,
         emitIndexing: (Indexer.Indexing) -> Unit
     ): List<Song> {
-        // Note: We do not actually update the callback with a current/total value, this is because
-        // loading music from MediaStore tends to be quite fast, with the only bottlenecks being
-        // with genre loading and querying the media database itself. As a result, a progress bar
-        // is not really that applicable.
         val audios = mutableListOf<Audio>()
         while (cursor.moveToNext()) {
             audios.add(buildAudio(context, cursor))
+            if (cursor.position % 50 == 0) {
+                // Only check for a cancellation every 50 songs or so (~20ms).
+                emitIndexing(Indexer.Indexing.Indeterminate)
+            }
         }
 
         // The audio is not actually complete at this point, as we cannot obtain a genre
@@ -212,9 +212,18 @@ abstract class MediaStoreBackend : Indexer.Backend {
                     while (cursor.moveToNext()) {
                         val songId = cursor.getLong(songIdIndex)
                         audios.find { it.id == songId }?.let { song -> song.genre = name }
+
+                        if (cursor.position % 50 == 0) {
+                            // Only check for a cancellation every 50 songs or so (~20ms).
+                            emitIndexing(Indexer.Indexing.Indeterminate)
+                        }
                     }
                 }
             }
+
+            // Check for a cancellation every time we finish a genre too, in the case that
+            // the genre has <50 songs.
+            emitIndexing(Indexer.Indexing.Indeterminate)
         }
 
         return audios.map { it.toSong() }

app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt

@@ -384,11 +384,7 @@ class PlaybackStateManager private constructor() {
         // 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.
         logD("Sanitizing state")
-        val state =
-            synchronized(this) {
-                isPlaying = false
-                makeStateImpl()
-            }
+        val state = synchronized(this) { makeStateImpl() }
 
         val sanitizedState =
             withContext(Dispatchers.IO) {
@@ -413,6 +409,10 @@ class PlaybackStateManager private constructor() {
             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()