music: improve indexer callback impl

Improve the indexer callback system to be more coherent and efficient.

This delegates the old Callback role to a new singular Callback and
Controller roles. IndexerService also handles the loading process
more gracefully, reducing the amount of time music loads take.

app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt

@@ -25,7 +25,6 @@ import coil.request.Disposable
 import coil.request.ImageRequest
 import coil.size.Size
 import org.oxycblt.auxio.music.Song
-import org.oxycblt.auxio.util.logD
 
 /**
  * A utility to provide bitmaps in a manner less prone to race conditions.
@@ -54,7 +53,6 @@ class BitmapProvider(private val context: Context) {
         // Increment the generation value so that previous requests are invalidated.
         // This is a second safeguard to mitigate instruction-by-instruction race conditions.
         val generation = synchronized(this) { ++currentGeneration }
-        logD("new generation for ${song.rawName}: $generation $currentGeneration")
 
         currentRequest?.run { disposable.dispose() }
         currentRequest = null

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

@@ -23,14 +23,14 @@ import android.content.pm.PackageManager
 import android.database.Cursor
 import android.os.Build
 import androidx.core.content.ContextCompat
-import kotlinx.coroutines.Dispatchers
+import org.oxycblt.auxio.BuildConfig
-import kotlinx.coroutines.withContext
 import org.oxycblt.auxio.music.backend.Api21MediaStoreBackend
 import org.oxycblt.auxio.music.backend.Api29MediaStoreBackend
 import org.oxycblt.auxio.music.backend.Api30MediaStoreBackend
 import org.oxycblt.auxio.ui.Sort
 import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.logE
+import org.oxycblt.auxio.util.logW
 
 /**
  * Auxio's media indexer.
@@ -58,7 +58,8 @@ class Indexer {
     private var indexingState: Indexing? = null
 
     private var currentGeneration = 0L
-    private val callbacks = mutableListOf<Callback>()
+    private var controller: Controller? = null
+    private var callback: Callback? = null
 
     /**
      * Whether this instance is in an indeterminate state or not, where nothing has been previously
@@ -67,19 +68,50 @@ class Indexer {
     val isIndeterminate: Boolean
         get() = lastResponse == null && indexingState == null
 
-    fun addCallback(callback: Callback) {
+    /** Register a [Controller] with this instance. */
+    fun registerController(controller: Controller) {
+        if (BuildConfig.DEBUG && this.controller != null) {
+            logW("Controller is already registered")
+            return
+        }
+
+        synchronized(this) { this.controller = controller }
+    }
+
+    /** Unregister a [Controller] with this instance. */
+    fun unregisterController(controller: Controller) {
+        if (BuildConfig.DEBUG && this.controller !== controller) {
+            logW("Given controller did not match current controller")
+            return
+        }
+
+        synchronized(this) { this.controller = null }
+    }
+
+    fun registerCallback(callback: Callback) {
+        if (BuildConfig.DEBUG && this.callback != null) {
+            logW("Callback is already registered")
+            return
+        }
+
         val currentState =
             indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) }
 
         callback.onIndexerStateChanged(currentState)
-        callbacks.add(callback)
+
+        this.callback = callback
     }
 
-    fun removeCallback(callback: Callback) {
+    fun unregisterCallback(callback: Callback) {
-        callbacks.remove(callback)
+        if (BuildConfig.DEBUG && this.callback !== callback) {
+            logW("Given controller did not match current controller")
+            return
+        }
+
+        this.callback = null
     }
 
-    suspend fun index(context: Context) {
+    fun index(context: Context) {
         val generation = synchronized(this) { ++currentGeneration }
 
         val notGranted =
@@ -94,7 +126,7 @@ class Indexer {
         val response =
             try {
                 val start = System.currentTimeMillis()
-                val library = withContext(Dispatchers.IO) { indexImpl(context, generation) }
+                val library = indexImpl(context, generation)
                 if (library != null) {
                     logD(
                         "Music indexing completed successfully in " +
@@ -119,9 +151,7 @@ class Indexer {
      */
     fun requestReindex() {
         logD("Requesting reindex")
-        for (callback in callbacks) {
+        controller?.onStartIndexing()
-            callback.onRequestReindex()
-        }
     }
 
     /**
@@ -152,9 +182,8 @@ class Indexer {
                 indexingState?.let { State.Indexing(it) }
                     ?: lastResponse?.let { State.Complete(it) }
 
-            for (callback in callbacks) {
+            controller?.onIndexerStateChanged(state)
-                callback.onIndexerStateChanged(state)
+            callback?.onIndexerStateChanged(state)
-            }
         }
     }
 
@@ -168,9 +197,8 @@ class Indexer {
             indexingState = null
 
             val state = State.Complete(response)
-            for (callback in callbacks) {
+            controller?.onIndexerStateChanged(state)
-                callback.onIndexerStateChanged(state)
+            callback?.onIndexerStateChanged(state)
-            }
         }
     }
 
@@ -377,12 +405,10 @@ class Indexer {
          * canceled for one reason or another.
          */
         fun onIndexerStateChanged(state: State?)
+    }
 
-        /**
+    interface Controller : Callback {
-         * Called when some piece of code that cannot index music requests a reindex. Callbacks that
+        fun onStartIndexing()
-         * can index music should begin reindexing at this call.
-         */
-        fun onRequestReindex() {}
     }
 
     /** Represents a backend that metadata can be extracted from. */

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

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2022 Auxio Project
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+ 
+package org.oxycblt.auxio.music
+
+import android.app.NotificationChannel
+import android.app.NotificationManager
+import android.content.Context
+import android.os.Build
+import androidx.core.app.NotificationCompat
+import org.oxycblt.auxio.BuildConfig
+import org.oxycblt.auxio.IntegerTable
+import org.oxycblt.auxio.R
+import org.oxycblt.auxio.util.getSystemServiceSafe
+import org.oxycblt.auxio.util.logD
+import org.oxycblt.auxio.util.newMainPendingIntent
+
+class IndexerNotification(private val context: Context) :
+    NotificationCompat.Builder(context, CHANNEL_ID) {
+    private val notificationManager = context.getSystemServiceSafe(NotificationManager::class)
+
+    init {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            val channel =
+                NotificationChannel(
+                    CHANNEL_ID,
+                    context.getString(R.string.info_indexer_channel_name),
+                    NotificationManager.IMPORTANCE_LOW)
+
+            notificationManager.createNotificationChannel(channel)
+        }
+
+        setSmallIcon(R.drawable.ic_indexer_24)
+        setCategory(NotificationCompat.CATEGORY_PROGRESS)
+        setShowWhen(false)
+        setSilent(true)
+        setContentIntent(context.newMainPendingIntent())
+        setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
+        setContentTitle(context.getString(R.string.info_indexer_channel_name))
+        setContentText(context.getString(R.string.lbl_indexing))
+        setProgress(0, 0, true)
+    }
+
+    fun renotify() {
+        notificationManager.notify(IntegerTable.INDEXER_NOTIFICATION_CODE, build())
+    }
+
+    fun updateIndexingState(indexing: Indexer.Indexing): Boolean {
+        when (indexing) {
+            is Indexer.Indexing.Indeterminate -> {
+                logD("Updating state to $indexing")
+                setContentText(context.getString(R.string.lbl_indexing))
+                setProgress(0, 0, true)
+                return true
+            }
+            is Indexer.Indexing.Songs -> {
+                // Only update the notification every 50 songs to prevent excessive updates.
+                if (indexing.current % 50 == 0) {
+                    logD("Updating state to $indexing")
+                    setContentText(
+                        context.getString(R.string.fmt_indexing, indexing.current, indexing.total))
+                    setProgress(indexing.total, indexing.current, false)
+                    return true
+                }
+            }
+        }
+
+        return false
+    }
+
+    companion object {
+        const val CHANNEL_ID = BuildConfig.APPLICATION_ID + ".channel.INDEXER"
+    }
+}

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

@@ -17,25 +17,17 @@
  
 package org.oxycblt.auxio.music
 
-import android.app.NotificationChannel
-import android.app.NotificationManager
 import android.app.Service
-import android.content.Context
 import android.content.Intent
-import android.os.Build
 import android.os.IBinder
-import androidx.core.app.NotificationCompat
 import androidx.core.app.ServiceCompat
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
-import org.oxycblt.auxio.BuildConfig
+import kotlinx.coroutines.withContext
 import org.oxycblt.auxio.IntegerTable
-import org.oxycblt.auxio.R
-import org.oxycblt.auxio.util.getSystemServiceSafe
 import org.oxycblt.auxio.util.logD
-import org.oxycblt.auxio.util.newMainPendingIntent
 
 /**
  * A [Service] that handles the music loading process.
@@ -48,13 +40,13 @@ import org.oxycblt.auxio.util.newMainPendingIntent
  *
  * @author OxygenCobalt
  */
-class IndexerService : Service(), Indexer.Callback {
+class IndexerService : Service(), Indexer.Controller {
     private val indexer = Indexer.getInstance()
     private val musicStore = MusicStore.getInstance()
 
     private val serviceJob = Job()
-    private val indexScope = CoroutineScope(serviceJob + Dispatchers.Main)
+    private val indexScope = CoroutineScope(serviceJob + Dispatchers.IO)
-    private val updateScope = CoroutineScope(serviceJob + Dispatchers.Main)
+    private val updateScope = CoroutineScope(serviceJob + Dispatchers.IO)
 
     private var isForeground = false
     private lateinit var notification: IndexerNotification
@@ -64,10 +56,10 @@ class IndexerService : Service(), Indexer.Callback {
 
         notification = IndexerNotification(this)
 
-        indexer.addCallback(this)
+        indexer.registerController(this)
         if (musicStore.library == null && indexer.isIndeterminate) {
             logD("No library present and no previous response, indexing music now")
-            onRequestReindex()
+            onStartIndexing()
         }
 
         logD("Service created.")
@@ -83,10 +75,14 @@ class IndexerService : Service(), Indexer.Callback {
         // cancelLast actually stops foreground for us as it updates the loading state to
         // null or completed.
         indexer.cancelLast()
-        indexer.removeCallback(this)
+        indexer.unregisterController(this)
         serviceJob.cancel()
     }
 
+    override fun onStartIndexing() {
+        indexScope.launch { indexer.index(this@IndexerService) }
+    }
+
     override fun onIndexerStateChanged(state: Indexer.State?) {
         when (state) {
             is Indexer.State.Complete -> {
@@ -98,7 +94,12 @@ class IndexerService : Service(), Indexer.Callback {
                     // 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)
+                        // TODO: Update PlaybackStateManager here 
+
+                        withContext(Dispatchers.Main) {
+                            musicStore.updateLibrary(state.response.library)
+                        }
+
                         stopForegroundSession()
                     }
                 } else {
@@ -133,10 +134,6 @@ class IndexerService : Service(), Indexer.Callback {
         }
     }
 
-    override fun onRequestReindex() {
-        indexScope.launch { indexer.index(this@IndexerService) }
-    }
-
     private fun stopForegroundSession() {
         if (isForeground) {
             ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE)
@@ -144,61 +141,3 @@ class IndexerService : Service(), Indexer.Callback {
         }
     }
 }
-
-private class IndexerNotification(private val context: Context) :
-    NotificationCompat.Builder(context, CHANNEL_ID) {
-    private val notificationManager = context.getSystemServiceSafe(NotificationManager::class)
-
-    init {
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            val channel =
-                NotificationChannel(
-                    CHANNEL_ID,
-                    context.getString(R.string.info_indexer_channel_name),
-                    NotificationManager.IMPORTANCE_LOW)
-
-            notificationManager.createNotificationChannel(channel)
-        }
-
-        setSmallIcon(R.drawable.ic_indexer_24)
-        setCategory(NotificationCompat.CATEGORY_PROGRESS)
-        setShowWhen(false)
-        setSilent(true)
-        setContentIntent(context.newMainPendingIntent())
-        setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
-        setContentTitle(context.getString(R.string.info_indexer_channel_name))
-        setContentText(context.getString(R.string.lbl_indexing))
-        setProgress(0, 0, true)
-    }
-
-    fun renotify() {
-        notificationManager.notify(IntegerTable.INDEXER_NOTIFICATION_CODE, build())
-    }
-
-    fun updateIndexingState(indexing: Indexer.Indexing): Boolean {
-        when (indexing) {
-            is Indexer.Indexing.Indeterminate -> {
-                logD("Updating state to $indexing")
-                setContentText(context.getString(R.string.lbl_indexing))
-                setProgress(0, 0, true)
-                return true
-            }
-            is Indexer.Indexing.Songs -> {
-                // Only update the notification every 50 songs to prevent excessive updates.
-                if (indexing.current % 50 == 0) {
-                    logD("Updating state to $indexing")
-                    setContentText(
-                        context.getString(R.string.fmt_indexing, indexing.current, indexing.total))
-                    setProgress(indexing.total, indexing.current, false)
-                    return true
-                }
-            }
-        }
-
-        return false
-    }
-
-    companion object {
-        const val CHANNEL_ID = BuildConfig.APPLICATION_ID + ".channel.INDEXER"
-    }
-}

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

@@ -32,7 +32,7 @@ class IndexerViewModel : ViewModel(), Indexer.Callback {
     val state: StateFlow<Indexer.State?> = _state
 
     init {
-        indexer.addCallback(this)
+        indexer.registerCallback(this)
     }
 
     fun reindex() {
@@ -44,6 +44,6 @@ class IndexerViewModel : ViewModel(), Indexer.Callback {
     }
 
     override fun onCleared() {
-        indexer.removeCallback(this)
+        indexer.unregisterCallback(this)
     }
 }

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

@@ -20,8 +20,6 @@ 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
 
 /**
@@ -48,15 +46,11 @@ class MusicStore private constructor() {
         callbacks.remove(callback)
     }
 
-    suspend fun updateLibrary(newLibrary: Library?) {
+    fun updateLibrary(newLibrary: Library?) {
-        // Ensure we are on the main thread when updating the library, as callbacks expect to
+        synchronized(this) {
-        // run in an stable app thread.
+            library = newLibrary
-        withContext(Dispatchers.Main) {
+            for (callback in callbacks) {
-            synchronized(this) {
+                callback.onLibraryChanged(library)
-                library = newLibrary
-                for (callback in callbacks) {
-                    callback.onLibraryChanged(library)
-                }
             }
         }
     }

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

@@ -105,12 +105,12 @@ class PlaybackStateManager private constructor() {
         }
     }
 
-    /** Remove a [PlaybackStateManager.Callback] bound to this instance. */
+    /** Remove a [Callback] bound to this instance. */
     fun removeCallback(callback: Callback) {
         callbacks.remove(callback)
     }
 
-    /** Register a [PlaybackStateManager.Controller] with this instance. */
+    /** Register a [Controller] with this instance. */
     fun registerController(controller: Controller) {
         if (BuildConfig.DEBUG && this.controller != null) {
             logW("Controller is already registered")
@@ -129,7 +129,7 @@ class PlaybackStateManager private constructor() {
         }
     }
 
-    /** Unregister a [PlaybackStateManager.Controller] with this instance. */
+    /** Unregister a [Controller] with this instance. */
     fun unregisterController(controller: Controller) {
         if (BuildConfig.DEBUG && this.controller !== controller) {
             logW("Given controller did not match current controller")

app/src/main/res/layout/widget_large.xml

@@ -81,7 +81,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_repeat"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_change_repeat"
@@ -94,7 +94,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_skip_prev"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_skip_prev"
@@ -122,7 +122,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_skip_next"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_skip_next"
@@ -135,7 +135,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_shuffle"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_shuffle"

app/src/main/res/layout/widget_medium.xml

@@ -79,7 +79,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_skip_prev"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_skip_prev"
@@ -107,7 +107,7 @@
 
             <android.widget.ImageButton
                 android:id="@+id/widget_skip_next"
-                style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+                style="@style/Widget.Auxio.MaterialButton.AppWidget"
                 android:layout_width="@dimen/size_btn"
                 android:layout_height="@dimen/size_btn"
                 android:contentDescription="@string/desc_skip_next"

app/src/main/res/layout/widget_small.xml

@@ -68,7 +68,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_skip_prev"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_skip_prev"
@@ -94,7 +94,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_skip_next"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_skip_next"

app/src/main/res/layout/widget_wide.xml

@@ -55,7 +55,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_repeat"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_change_repeat"
@@ -68,7 +68,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_skip_prev"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_skip_prev"
@@ -95,7 +95,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_skip_next"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_skip_next"
@@ -108,7 +108,7 @@
 
         <android.widget.ImageButton
             android:id="@+id/widget_shuffle"
-            style="@style/Widget.Auxio.PlaybackButton.AppWidget"
+            style="@style/Widget.Auxio.MaterialButton.AppWidget"
             android:layout_width="@dimen/size_btn"
             android:layout_height="@dimen/size_btn"
             android:contentDescription="@string/desc_shuffle"

app/src/main/res/values/styles_android.xml

@@ -49,7 +49,7 @@
     </style>
 
     <!-- A variant of PlaybackButton that plays along with AppWidget restrictions. -->
-    <style name="Widget.Auxio.PlaybackButton.AppWidget" parent="Widget.AppCompat.Button.Borderless">
+    <style name="Widget.Auxio.MaterialButton.AppWidget" parent="Widget.AppCompat.Button.Borderless">
         <item name="android:minHeight">@dimen/size_btn</item>
         <item name="android:background">@drawable/ui_remote_ripple</item>
     </style>