music: rework library update process [#176]

Update the library update process to be on a co-routine, updating callbacks on the main thread. For some insane reason, the Main dispatcher used normally when loading music just disappears sometimes. This leads to unpleasent crashes as callbacks expect to be called on the app thread, not any background threads. Fix this by forcing the Main dispatcher during the update process. This requires the music update process to also run on a background thread, albeit that will be useful for automatic rescanning late ron.
2022-07-01 08:54:42 -06:00 · 2022-07-01 08:54:42 -06:00 · e883336b04
commit e883336b04
parent 090a1713dd
18 changed files with 48 additions and 38 deletions
--- a/app/src/main/java/org/oxycblt/auxio/music/Indexer.kt
+++ b/app/src/main/java/org/oxycblt/auxio/music/Indexer.kt
@ -361,6 +361,12 @@ class Indexer {
        object NoPerms : Response()
    }

+    /**
+     * A callback to use when the indexing state changes.
+     *
+     * This callback is low-level and not guaranteed to be single-thread. For that,
+     * [MusicStore.Callback] is recommended instead.
+     */
    interface Callback {
        /**
         * Called when the current state of the Indexer changed.
--- a/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt
+++ b/app/src/main/java/org/oxycblt/auxio/music/IndexerService.kt
@ -54,6 +54,7 @@ class IndexerService : Service(), Indexer.Callback {

    private val serviceJob = Job()
    private val indexScope = CoroutineScope(serviceJob + Dispatchers.Main)
+    private val updateScope = CoroutineScope(serviceJob + Dispatchers.Main)

    private var isForeground = false
    private lateinit var notification: IndexerNotification
@ -92,20 +93,21 @@ class IndexerService : Service(), Indexer.Callback {
                if (state.response is Indexer.Response.Ok &&
                    state.response.library != musicStore.library) {
                    logD("Applying new library")
+
                    // Load was completed successfully, so apply the new library if we
-                    // have not already.
-                    musicStore.library = state.response.library
+                    // have not already. Only when we are done updating the library will
+                    // the service stop it's foreground state.
+                    updateScope.launch {
+                        musicStore.updateLibrary(state.response.library)
+                        stopForegroundSession()
+                    }
+                } else {
+                    // On errors, while we would want to show a notification that displays the
+                    // error, in practice that comes into conflict with the upcoming Android 13
+                    // notification permission, and there is no point implementing permission
+                    // on-boarding for such when it will only be used for this.
+                    stopForegroundSession()
                }
-
-                // On errors, while we would want to show a notification that displays the
-                // error, in practice that comes into conflict with the upcoming Android 13
-                // notification permission, and there is no point implementing permission
-                // on-boarding for such when it will only be used for this.
-
-                // Note that we don't stop the service here, as (in the future)
-                // this service will be used to reload music and observe the music
-                // database.
-                stopForegroundSession()
            }
            is Indexer.State.Indexing -> {
                // When loading, we want to enter the foreground state so that android does
--- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt
+++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt
@ -20,6 +20,8 @@ package org.oxycblt.auxio.music
 import android.content.Context
 import android.net.Uri
 import android.provider.OpenableColumns
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import org.oxycblt.auxio.util.contentResolverSafe

 /**
@ -33,14 +35,7 @@ class MusicStore private constructor() {
    private val callbacks = mutableListOf<Callback>()

    var library: Library? = null
-        set(value) {
-            synchronized(this) {
-                field = value
-                for (callback in callbacks) {
-                    callback.onLibraryChanged(library)
-                }
-            }
-        }
+        private set

    /** Add a callback to this instance. Make sure to remove it when done. */
    fun addCallback(callback: Callback) {
@ -53,6 +48,19 @@ class MusicStore private constructor() {
        callbacks.remove(callback)
    }

+    suspend fun updateLibrary(newLibrary: Library?) {
+        // Ensure we are on the main thread when updating the library, as callbacks expect to
+        // run in an stable app thread.
+        withContext(Dispatchers.Main) {
+            synchronized(this) {
+                library = newLibrary
+                for (callback in callbacks) {
+                    callback.onLibraryChanged(library)
+                }
+            }
+        }
+    }
+
    /** Represents a library of music owned by [MusicStore]. */
    data class Library(
        val genres: List<Genre>,
@ -80,7 +88,6 @@ class MusicStore private constructor() {
                songs.find { it.path.name == displayName }
            }

-        /** "Sanitize" a music object from a previous library iteration. */
        fun sanitize(song: Song) = songs.find { it.id == song.id }
        fun sanitize(album: Album) = albums.find { it.id == album.id }
        fun sanitize(artist: Artist) = artists.find { it.id == artist.id }
--- a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt
@ -143,7 +143,6 @@ class PlaybackService :
        // --- PLAYBACKSTATEMANAGER SETUP ---

        settings = Settings(this, this)
-
        playbackManager.registerController(this)

        logD("Service created")
--- a/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt
+++ b/app/src/main/java/org/oxycblt/auxio/ui/Sort.kt
@ -28,7 +28,7 @@ import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.Song

 /**
- * A data class representing the sort modes used in Auxio.
+ * Represents the sort modes used in Auxio.
 *
 * Sorting can be done by Name, Artist, Album, and others. Sorting of names is always
 * case-insensitive and article-aware. Certain datatypes may only support a subset of sorts since
--- a/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentGridLayoutManager.kt
+++ b/app/src/main/java/org/oxycblt/auxio/ui/accent/AccentGridLayoutManager.kt
@ -37,7 +37,7 @@ class AccentGridLayoutManager(
 ) : GridLayoutManager(context, attrs, defStyleAttr, defStyleRes) {
    // We use 72dp here since that's the rough size of the accent item.
    // This will need to be modified if this is used beyond the accent dialog.
-    private var columnWidth = context.pxOfDp(64f)
+    private var columnWidth = context.pxOfDp(56f)

    private var lastWidth = -1
    private var lastHeight = -1
--- a/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt
+++ b/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt
@ -100,7 +100,7 @@ private fun RemoteViews.applyCover(
            R.id.widget_cover,
            context.getString(R.string.desc_album_cover, state.song.album.resolveName(context)))
    } else {
-        setImageViewResource(R.id.widget_cover, R.drawable.ic_remote_default_cover)
+        setImageViewResource(R.id.widget_cover, R.drawable.ic_remote_default_cover_24)
        setContentDescription(R.id.widget_cover, context.getString(R.string.desc_no_cover))
    }

--- a/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt
+++ b/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt
@ -61,8 +61,7 @@ class WidgetComponent(private val context: Context) :
     */
    fun update() {
        // TODO: Rework margins/button layout to do the magic that other button bars do
-        // TODO: Try to change the error icon again
-        // TODO:
+        // TODO: Respond to rounded covers

        // Updating Auxio's widget is unlike the rest of Auxio for a few reasons:
        // 1. We can't use the typical primitives like ViewModels
--- a/app/src/main/res/drawable/ic_queue_24.xml
+++ b/app/src/main/res/drawable/ic_queue_24.xml
--- a/app/src/main/res/drawable/ic_remote_default_cover_24.xml
+++ b/app/src/main/res/drawable/ic_remote_default_cover_24.xml
--- a/app/src/main/res/layout/item_accent.xml
+++ b/app/src/main/res/layout/item_accent.xml
@ -4,7 +4,7 @@
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
-    android:padding="@dimen/spacing_small"
+    android:padding="@dimen/spacing_tiny"
    android:theme="@style/ThemeOverlay.Accent">

    <com.google.android.material.button.MaterialButton
@ -13,7 +13,6 @@
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_gravity="center"
-        android:background="@drawable/ui_accent_circle"
        app:icon="@drawable/ic_check_24"
        app:iconTint="?attr/colorSurface"
        tools:backgroundTint="?attr/colorPrimary"
--- a/app/src/main/res/layout/widget_large.xml
+++ b/app/src/main/res/layout/widget_large.xml
@ -36,7 +36,7 @@
        android:layout_alignTop="@id/widget_aspect_ratio"
        android:layout_alignEnd="@id/widget_aspect_ratio"
        android:layout_alignBottom="@id/widget_aspect_ratio"
-        android:src="@drawable/ic_remote_default_cover"
+        android:src="@drawable/ic_remote_default_cover_24"
        tools:ignore="ContentDescription" />

    <android.widget.LinearLayout
--- a/app/src/main/res/layout/widget_medium.xml
+++ b/app/src/main/res/layout/widget_medium.xml
@ -36,7 +36,7 @@
        android:layout_alignTop="@id/widget_aspect_ratio"
        android:layout_alignEnd="@id/widget_aspect_ratio"
        android:layout_alignBottom="@id/widget_aspect_ratio"
-        android:src="@drawable/ic_remote_default_cover"
+        android:src="@drawable/ic_remote_default_cover_24"
        tools:ignore="ContentDescription" />

    <android.widget.LinearLayout
--- a/app/src/main/res/layout/widget_small.xml
+++ b/app/src/main/res/layout/widget_small.xml
@ -44,7 +44,7 @@
        android:layout_alignTop="@id/widget_aspect_ratio"
        android:layout_alignEnd="@id/widget_aspect_ratio"
        android:layout_alignBottom="@id/widget_aspect_ratio"
-        android:src="@drawable/ic_remote_default_cover"
+        android:src="@drawable/ic_remote_default_cover_24"
        tools:ignore="ContentDescription" />

    <android.widget.LinearLayout
--- a/app/src/main/res/layout/widget_thin.xml
+++ b/app/src/main/res/layout/widget_thin.xml
@ -49,7 +49,7 @@
            android:layout_alignTop="@id/widget_aspect_ratio"
            android:layout_alignEnd="@id/widget_aspect_ratio"
            android:layout_alignBottom="@id/widget_aspect_ratio"
-            android:src="@drawable/ic_remote_default_cover"
+            android:src="@drawable/ic_remote_default_cover_24"
            tools:ignore="ContentDescription" />

    </android.widget.RelativeLayout>
--- a/app/src/main/res/layout/widget_wide.xml
+++ b/app/src/main/res/layout/widget_wide.xml
@ -37,7 +37,7 @@
        android:layout_alignTop="@id/widget_aspect_ratio"
        android:layout_alignEnd="@id/widget_aspect_ratio"
        android:layout_alignBottom="@id/widget_aspect_ratio"
-        android:src="@drawable/ic_remote_default_cover"
+        android:src="@drawable/ic_remote_default_cover_24"
        tools:ignore="ContentDescription" />

    <android.widget.LinearLayout
--- a/app/src/main/res/menu/menu_playback.xml
+++ b/app/src/main/res/menu/menu_playback.xml
@ -3,7 +3,7 @@
    xmlns:app="http://schemas.android.com/apk/res-auto">
    <item
        android:id="@+id/action_queue"
-        android:icon="@drawable/ic_queue"
+        android:icon="@drawable/ic_queue_24"
        android:title="@string/lbl_queue"
        app:showAsAction="ifRoom" />
    <item
--- a/app/src/main/res/values/dimens.xml
+++ b/app/src/main/res/values/dimens.xml
@ -31,10 +31,8 @@

    <dimen name="size_pre_amp_ticker">56dp</dimen>

-    <dimen name="text_size_ext_label_larger">16sp</dimen>
-    <dimen name="text_size_ext_title_mid_large">18sp</dimen>
    <dimen name="text_size_track_number_min">12sp</dimen>
-    <dimen name="text_size_track_number_max">20sp</dimen>
+    <dimen name="text_size_track_number_max">22sp</dimen>
    <dimen name="text_size_track_number_step">2sp</dimen>

    <!-- Misc -->