music: add loading progress indicator

Add an indicator to gague the current music loading progress.

This is actually a lot harder to implement than it might seem, not only
due to UI state issues, but also due to the fact that MusicStore needs
to keep it's state sane across a myriad of possible events that could
occur while loading music. This system seems like a good stopgap until
a full service-backed implementation can be created.

CHANGELOG.md

@@ -4,10 +4,12 @@
 
 #### What's Improved
 - Loading UI is now more clear and easy-to-use
+- Made album/artist/genre grouping order consistent (May change genre images)
 
 #### What's Fixed
 - Fixed crash when seeking to the end of a track as the track changed to a track with a lower duration 
 - Fixed regression where GadgetBridge media controls would no longer work
+- Fixed bug where music would be incorrectly reloaded on a hot restart
 - Fixed issue where the album/artist/genre would not be correctly restored
 - Fixed issue where items would not highlight properly in the detail UI
 

app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt

@@ -123,7 +123,7 @@ class HomeFragment : ViewBindingFragment<FragmentHomeBinding>(), Toolbar.OnMenuI
         launch { homeModel.isFastScrolling.collect(::updateFastScrolling) }
         launch { homeModel.currentTab.collect(::updateCurrentTab) }
         launch { homeModel.recreateTabs.collect(::handleRecreateTabs) }
-        launch { musicModel.response.collect(::handleLoaderResponse) }
+        launch { musicModel.loadState.collect(::handleLoadEvent) }
         launch { navModel.exploreNavigationItem.collect(::handleNavigation) }
     }
 
@@ -178,7 +178,8 @@ class HomeFragment : ViewBindingFragment<FragmentHomeBinding>(), Toolbar.OnMenuI
 
         // Make sure an update here doesn't mess up the FAB state when it comes to the
         // loader response.
-        if (musicModel.response.value !is MusicStore.Response.Ok) {
+        val state = musicModel.loadState.value
+        if (!(state is MusicStore.LoadState.Complete && state.response is MusicStore.Response.Ok)) {
             return
         }
 
@@ -256,9 +257,17 @@ class HomeFragment : ViewBindingFragment<FragmentHomeBinding>(), Toolbar.OnMenuI
         }
     }
 
-    private fun handleLoaderResponse(response: MusicStore.Response?) {
+    private fun handleLoadEvent(state: MusicStore.LoadState?) {
         val binding = requireBinding()
 
+        if (state is MusicStore.LoadState.Complete) {
+            handleLoaderResponse(binding, state.response)
+        } else {
+            handleLoadEvent(binding, state)
+        }
+    }
+
+    private fun handleLoaderResponse(binding: FragmentHomeBinding, response: MusicStore.Response) {
         if (response is MusicStore.Response.Ok) {
             binding.homeFab.show()
             binding.homeLoadingContainer.visibility = View.INVISIBLE
@@ -307,12 +316,27 @@ class HomeFragment : ViewBindingFragment<FragmentHomeBinding>(), Toolbar.OnMenuI
                         }
                     }
                 }
-                null -> {
+            }
-                    binding.homeLoadingStatus.textSafe = getString(R.string.lbl_loading)
+        }
-                    binding.homeLoadingAction.visibility = View.INVISIBLE
+    }
+
+    private fun handleLoadEvent(binding: FragmentHomeBinding, event: MusicStore.LoadState?) {
+        binding.homePager.visibility = View.INVISIBLE
+        binding.homeLoadingContainer.visibility = View.VISIBLE
         binding.homeLoadingProgress.visibility = View.VISIBLE
+        binding.homeLoadingAction.visibility = View.INVISIBLE
+
+        if (event is MusicStore.LoadState.Indexing) {
+            binding.homeLoadingStatus.textSafe =
+                getString(R.string.fmt_indexing, event.current, event.total)
+            binding.homeLoadingProgress.apply {
+                isIndeterminate = false
+                max = event.total
+                progress = event.current
             }
-            }
+        } else {
+            binding.homeLoadingStatus.textSafe = getString(R.string.lbl_loading)
+            binding.homeLoadingProgress.isIndeterminate = true
         }
     }
 

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

@@ -25,6 +25,7 @@ import android.provider.OpenableColumns
 import androidx.core.content.ContextCompat
 import java.lang.Exception
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.withContext
 import org.oxycblt.auxio.music.indexer.Indexer
 import org.oxycblt.auxio.music.indexer.useQuery
@@ -65,16 +66,22 @@ class MusicStore private constructor() {
     }
 
     /** Load/Sort the entire music library. Should always be ran on a coroutine. */
-    suspend fun load(context: Context): Response {
+    suspend fun load(context: Context, callback: LoadCallback): Response {
         logD("Starting initial music load")
-        val newResponse = withContext(Dispatchers.IO) { loadImpl(context) }.also { response = it }
+
-        for (callback in callbacks) {
+        callback.onLoadStateChanged(null)
-            callback.onMusicUpdate(newResponse)
+        val newResponse =
+            withContext(Dispatchers.IO) { loadImpl(context, callback) }.also { response = it }
+
+        callback.onLoadStateChanged(LoadState.Complete(newResponse))
+        for (responseCallbacks in callbacks) {
+            responseCallbacks.onMusicUpdate(newResponse)
         }
+
         return newResponse
     }
 
-    private fun loadImpl(context: Context): Response {
+    private fun loadImpl(context: Context, callback: LoadCallback): Response {
         val notGranted =
             ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) ==
                 PackageManager.PERMISSION_DENIED
@@ -86,10 +93,11 @@ class MusicStore private constructor() {
         val response =
             try {
                 val start = System.currentTimeMillis()
-                val library = Indexer.index(context)
+                val library = Indexer.index(context, callback)
                 if (library != null) {
                     logD(
-                        "Music load completed successfully in ${System.currentTimeMillis() - start}ms")
+                        "Music load completed successfully in " +
+                            "${System.currentTimeMillis() - start}ms")
                     Response.Ok(library)
                 } else {
                     logE("No music found")
@@ -131,6 +139,25 @@ class MusicStore private constructor() {
             }
     }
 
+    /** Represents the current state of the loading process. */
+    sealed class LoadState {
+        data class Indexing(val current: Int, val total: Int) : LoadState()
+        data class Complete(val response: Response) : LoadState()
+    }
+
+    /**
+     * A callback for events that occur during the loading process. This is used by [load] in order
+     * to have a separate callback interface that is more efficient for rapid-fire updates.
+     */
+    interface LoadCallback {
+        /**
+         * Called when the state of the loading process changes. A value of null represents the
+         * beginning of a loading process.
+         */
+        fun onLoadStateChanged(state: LoadState?)
+    }
+
+    /** Represents the possible outcomes of a loading process. */
     sealed class Response {
         data class Ok(val library: Library) : Response()
         data class Err(val throwable: Throwable) : Response()
@@ -138,6 +165,7 @@ class MusicStore private constructor() {
         object NoPerms : Response()
     }
 
+    /** A callback for awaiting the loading of music. */
     interface Callback {
         fun onMusicUpdate(response: Response)
     }

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

@@ -26,11 +26,11 @@ import kotlinx.coroutines.launch
 import org.oxycblt.auxio.util.logD
 
 /** A [ViewModel] that represents the current music indexing state. */
-class MusicViewModel : ViewModel() {
+class MusicViewModel : ViewModel(), MusicStore.LoadCallback {
     private val musicStore = MusicStore.getInstance()
 
-    private val _response = MutableStateFlow<MusicStore.Response?>(null)
+    private val _loadState = MutableStateFlow<MusicStore.LoadState?>(null)
-    val response: StateFlow<MusicStore.Response?> = _response
+    val loadState: StateFlow<MusicStore.LoadState?> = _loadState
 
     private var isBusy = false
 
@@ -39,17 +39,16 @@ class MusicViewModel : ViewModel() {
      * navigated to and because SnackBars will have the best UX here.
      */
     fun loadMusic(context: Context) {
-        if (_response.value != null || isBusy) {
+        if (_loadState.value != null || isBusy) {
             logD("Loader is busy/already completed, not reloading")
             return
         }
 
         isBusy = true
-        _response.value = null
+        _loadState.value = null
 
         viewModelScope.launch {
-            val result = musicStore.load(context)
+            musicStore.load(context, this@MusicViewModel)
-            _response.value = result
             isBusy = false
         }
     }
@@ -60,7 +59,11 @@ class MusicViewModel : ViewModel() {
      */
     fun reloadMusic(context: Context) {
         logD("Reloading music library")
-        _response.value = null
+        _loadState.value = null
         loadMusic(context)
     }
+
+    override fun onLoadStateChanged(state: MusicStore.LoadState?) {
+        _loadState.value = state
+    }
 }

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

@@ -30,6 +30,7 @@ import com.google.common.util.concurrent.Futures
 import java.util.concurrent.ConcurrentLinkedQueue
 import java.util.concurrent.Executors
 import java.util.concurrent.Future
+import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.util.logW
 
@@ -57,7 +58,11 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend {
     // MediaStore.
     override fun query(context: Context) = inner.query(context)
 
-    override fun loadSongs(context: Context, cursor: Cursor): Collection<Song> {
+    override fun loadSongs(
+        context: Context,
+        cursor: Cursor,
+        callback: MusicStore.LoadCallback
+    ): Collection<Song> {
         // Metadata retrieval with ExoPlayer is asynchronous, so a callback may at any point
         // add a completed song to the list. To prevent a crash in that case, we use the
         // concurrent counterpart to a typical mutable list.
@@ -81,8 +86,13 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend {
 
                     Futures.addCallback(
                         task,
-                        AudioCallback(audio, songs, index),
+                        AudioCallback(audio) {
-                        Executors.newSingleThreadExecutor())
+                            runningTasks[index] = null
+                            songs.add(it)
+                            callback.onLoadStateChanged(
+                                MusicStore.LoadState.Indexing(songs.size, cursor.count))
+                        },
+                        CALLBACK_EXECUTOR)
 
                     runningTasks[index] = task
 
@@ -100,8 +110,7 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend {
 
     private inner class AudioCallback(
         private val audio: MediaStoreBackend.Audio,
-        private val dest: ConcurrentLinkedQueue<Song>,
+        private val onComplete: (Song) -> Unit,
-        private val taskIndex: Int
     ) : FutureCallback<TrackGroupArray> {
         override fun onSuccess(result: TrackGroupArray) {
             val metadata = result[0].getFormat(0).metadata
@@ -111,15 +120,13 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend {
                 logW("No metadata was found for ${audio.title}")
             }
 
-            dest.add(audio.toSong())
+            onComplete(audio.toSong())
-            runningTasks[taskIndex] = null
         }
 
         override fun onFailure(t: Throwable) {
             logW("Unable to extract metadata for ${audio.title}")
             logW(t.stackTraceToString())
-            dest.add(audio.toSong())
+            onComplete(audio.toSong())
-            runningTasks[taskIndex] = null
         }
     }
 
@@ -213,5 +220,7 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend {
     companion object {
         /** The amount of tasks this backend can run efficiently at once. */
         private const val TASK_CAPACITY = 8
+
+        private val CALLBACK_EXECUTOR = Executors.newSingleThreadExecutor()
     }
 }

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

@@ -46,7 +46,7 @@ import org.oxycblt.auxio.util.logD
  * @author OxygenCobalt
  */
 object Indexer {
-    fun index(context: Context): MusicStore.Library? {
+    fun index(context: Context, callback: MusicStore.LoadCallback): MusicStore.Library? {
         // Establish the backend to use when initially loading songs.
         val mediaStoreBackend =
             when {
@@ -62,7 +62,7 @@ object Indexer {
         //     mediaStoreBackend
         // }
 
-        val songs = buildSongs(context, mediaStoreBackend)
+        val songs = buildSongs(context, mediaStoreBackend, callback)
         if (songs.isEmpty()) {
             return null
         }
@@ -95,17 +95,20 @@ object Indexer {
      * [buildGenres] functions must be called with the returned list so that all songs are properly
      * linked up.
      */
-    private fun buildSongs(context: Context, backend: Backend): List<Song> {
+    private fun buildSongs(
-        val queryStart = System.currentTimeMillis()
+        context: Context,
+        backend: Backend,
+        callback: MusicStore.LoadCallback
+    ): List<Song> {
+        val start = System.currentTimeMillis()
+
         var songs =
             backend.query(context).use { cursor ->
-                val loadStart = System.currentTimeMillis()
+                logD(
-                logD("Successfully queried media database in ${loadStart - queryStart}ms")
+                    "Successfully queried media database " +
+                        "in ${System.currentTimeMillis() - start}ms")
 
-                val songs = backend.loadSongs(context, cursor)
+                backend.loadSongs(context, cursor, callback)
-                logD("Successfully loaded songs in ${System.currentTimeMillis() - loadStart}ms")
-
-                songs
             }
 
         // Deduplicate songs to prevent (most) deformed music clones
@@ -126,7 +129,7 @@ object Indexer {
         // Ensure that sorting order is consistent so that grouping is also consistent.
         Sort.ByName(true).songsInPlace(songs)
 
-        logD("Successfully loaded ${songs.size} songs")
+        logD("Successfully loaded ${songs.size} songs in ${System.currentTimeMillis() - start}ms")
 
         return songs
     }
@@ -223,6 +226,10 @@ object Indexer {
         fun query(context: Context): Cursor
 
         /** Create a list of songs from the [Cursor] queried in [query]. */
-        fun loadSongs(context: Context, cursor: Cursor): Collection<Song>
+        fun loadSongs(
+            context: Context,
+            cursor: Cursor,
+            callback: MusicStore.LoadCallback
+        ): Collection<Song>
     }
 }

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

@@ -26,6 +26,7 @@ import android.provider.MediaStore
 import androidx.annotation.RequiresApi
 import androidx.core.database.getIntOrNull
 import androidx.core.database.getStringOrNull
+import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.excluded.ExcludedDatabase
 import org.oxycblt.auxio.util.contentResolverSafe
@@ -127,7 +128,15 @@ abstract class MediaStoreBackend : Indexer.Backend {
                 args.toTypedArray())) { "Content resolver failure: No Cursor returned" }
     }
 
-    override fun loadSongs(context: Context, cursor: Cursor): Collection<Song> {
+    override fun loadSongs(
+        context: Context,
+        cursor: Cursor,
+        callback: MusicStore.LoadCallback
+    ): Collection<Song> {
+        // Note: We do not actually update the callback with an Indexing state, 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))

app/src/main/java/org/oxycblt/auxio/search/SearchFragment.kt

@@ -109,7 +109,7 @@ class SearchFragment :
         // --- VIEWMODEL SETUP ---
 
         launch { searchModel.searchResults.collect(::updateResults) }
-        launch { musicModel.response.collect(::handleLoaderResponse) }
+        launch { musicModel.loadState.collect(::handleLoadState) }
         launch { navModel.exploreNavigationItem.collect(::handleNavigation) }
     }
 
@@ -171,8 +171,8 @@ class SearchFragment :
         requireImm().hide()
     }
 
-    private fun handleLoaderResponse(response: MusicStore.Response?) {
+    private fun handleLoadState(state: MusicStore.LoadState?) {
-        if (response is MusicStore.Response.Ok) {
+        if (state is MusicStore.LoadState.Complete && state.response is MusicStore.Response.Ok) {
             searchModel.refresh(requireContext())
         }
     }

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

@@ -187,6 +187,8 @@
     <string name="fmt_db_pos">+%.1f dB</string>
     <string name="fmt_db_neg">-%.1f dB</string>
 
+    <string name="fmt_indexing">Loading your music library… (%1$d/%2$d)</string>
+
     <string name="fmt_songs_loaded">Songs loaded: %d</string>
     <string name="fmt_albums_loaded">Albums loaded: %d</string>
     <string name="fmt_artists_loaded">Artists loaded: %d</string>