diff --git a/CHANGELOG.md b/CHANGELOG.md index df41bdb4b..933cb04ed 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt index f801fd30b..3bda40104 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt @@ -123,7 +123,7 @@ class HomeFragment : ViewBindingFragment(), 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(), 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(), 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,15 +316,30 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI } } } - null -> { - binding.homeLoadingStatus.textSafe = getString(R.string.lbl_loading) - binding.homeLoadingAction.visibility = View.INVISIBLE - binding.homeLoadingProgress.visibility = View.VISIBLE - } } } } + 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 + } + } + private fun handleNavigation(item: Music?) { // Note: You will want to add a post call to this if you want to re-introduce a collapsing // toolbar. 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 90ca071df..03dd92bc8 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/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.onMusicUpdate(newResponse) + + callback.onLoadStateChanged(null) + 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) } diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt index dd6b552b4..8f18525b2 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt +++ b/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(null) - val response: StateFlow = _response + private val _loadState = MutableStateFlow(null) + val loadState: StateFlow = _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) - _response.value = result + musicStore.load(context, this@MusicViewModel) 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 + } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt index 84d0e1acf..4abfcac4e 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt +++ b/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 { + override fun loadSongs( + context: Context, + cursor: Cursor, + callback: MusicStore.LoadCallback + ): Collection { // 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), - Executors.newSingleThreadExecutor()) + AudioCallback(audio) { + 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, - private val taskIndex: Int + private val onComplete: (Song) -> Unit, ) : FutureCallback { 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()) - runningTasks[taskIndex] = null + onComplete(audio.toSong()) } override fun onFailure(t: Throwable) { logW("Unable to extract metadata for ${audio.title}") logW(t.stackTraceToString()) - dest.add(audio.toSong()) - runningTasks[taskIndex] = null + onComplete(audio.toSong()) } } @@ -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() } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt index 0745214f0..a78e80179 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt +++ b/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 { - val queryStart = System.currentTimeMillis() + private fun buildSongs( + context: Context, + backend: Backend, + callback: MusicStore.LoadCallback + ): List { + val start = System.currentTimeMillis() + var songs = backend.query(context).use { cursor -> - val loadStart = System.currentTimeMillis() - logD("Successfully queried media database in ${loadStart - queryStart}ms") + logD( + "Successfully queried media database " + + "in ${System.currentTimeMillis() - start}ms") - val songs = backend.loadSongs(context, cursor) - logD("Successfully loaded songs in ${System.currentTimeMillis() - loadStart}ms") - - songs + backend.loadSongs(context, cursor, callback) } // 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 + fun loadSongs( + context: Context, + cursor: Cursor, + callback: MusicStore.LoadCallback + ): Collection } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt index 717227f77..8115b0bec 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt +++ b/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 { + override fun loadSongs( + context: Context, + cursor: Cursor, + callback: MusicStore.LoadCallback + ): Collection { + // 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