From caaeaca494e0e74510de9b3c9dc3973df2f13bb9 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sun, 29 May 2022 20:04:20 -0600 Subject: [PATCH] home: rework loading screen Move the loading screen into the home view. Previously, we would use a Snackbar to track the music loading state. This ended up being a pretty stupid and buggy idea, and would get even worse with the new music loader changes. Instead, we re-implement the loading screen into the home view to generally be more sane and extendable compared to previously. --- app/build.gradle | 1 - .../java/org/oxycblt/auxio/MainFragment.kt | 48 ----------- .../org/oxycblt/auxio/home/HomeFragment.kt | 83 +++++++++++++++++-- .../org/oxycblt/auxio/music/MusicStore.kt | 10 +-- .../auxio/music/indexer/ExoPlayerBackend.kt | 10 +-- .../oxycblt/auxio/music/indexer/Indexer.kt | 19 +++-- .../auxio/music/indexer/MediaStoreBackend.kt | 37 +++------ .../playback/state/PlaybackStateManager.kt | 4 +- app/src/main/res/layout/fragment_home.xml | 59 +++++++++++-- app/src/main/res/values/strings.xml | 1 + 10 files changed, 164 insertions(+), 108 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index aed9f1737..5a71b3877 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -35,7 +35,6 @@ android { buildTypes { debug { - debuggable true applicationIdSuffix = ".debug" versionNameSuffix = "-DEBUG" } diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt index 13495cea6..1cc498b27 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt @@ -17,28 +17,23 @@ package org.oxycblt.auxio -import android.Manifest import android.os.Build import android.os.Bundle import android.view.LayoutInflater import android.view.View import androidx.activity.OnBackPressedCallback -import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts import androidx.fragment.app.activityViewModels import androidx.navigation.findNavController import androidx.navigation.fragment.findNavController -import com.google.android.material.snackbar.Snackbar import org.oxycblt.auxio.databinding.FragmentMainBinding import org.oxycblt.auxio.music.Music -import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.MusicViewModel import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.ui.MainNavigationAction import org.oxycblt.auxio.ui.NavigationViewModel import org.oxycblt.auxio.ui.ViewBindingFragment -import org.oxycblt.auxio.util.logD /** * A wrapper around the home fragment that shows the playback fragment and controls the more @@ -86,11 +81,6 @@ class MainFragment : ViewBindingFragment() { // TODO: Move this to a service [automatic rescanning] musicModel.loadMusic(requireContext()) - // Handle the music loader response. - musicModel.loaderResponse.observe(viewLifecycleOwner) { response -> - handleLoaderResponse(response, permLauncher) - } - navModel.mainNavigationAction.observe(viewLifecycleOwner, ::handleMainNavigation) navModel.exploreNavigationItem.observe(viewLifecycleOwner, ::handleExploreNavigation) @@ -107,44 +97,6 @@ class MainFragment : ViewBindingFragment() { callback?.isEnabled = false } - private fun handleLoaderResponse( - response: MusicStore.Response?, - permLauncher: ActivityResultLauncher - ) { - val binding = requireBinding() - - // Handle any error cases, showing a useful message. - when (response) { - is MusicStore.Response.Err -> { - logD("Received Response.Err") - Snackbar.make(binding.root, R.string.err_load_failed, Snackbar.LENGTH_INDEFINITE) - .apply { - setAction(R.string.lbl_retry) { musicModel.reloadMusic(context) } - show() - } - } - is MusicStore.Response.NoMusic -> { - logD("Received Response.NoMusic") - Snackbar.make(binding.root, R.string.err_no_music, Snackbar.LENGTH_INDEFINITE) - .apply { - setAction(R.string.lbl_retry) { musicModel.reloadMusic(context) } - show() - } - } - is MusicStore.Response.NoPerms -> { - logD("Received Response.NoPerms") - Snackbar.make(binding.root, R.string.err_no_perms, Snackbar.LENGTH_INDEFINITE) - .apply { - setAction(R.string.lbl_grant) { - permLauncher.launch(Manifest.permission.READ_EXTERNAL_STORAGE) - } - show() - } - } - else -> {} - } - } - private fun handleMainNavigation(action: MainNavigationAction?) { if (action == null) return 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 553fddd41..353144fea 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt @@ -17,12 +17,17 @@ package org.oxycblt.auxio.home +import android.Manifest import android.os.Bundle import android.view.LayoutInflater import android.view.MenuItem +import android.view.View +import androidx.activity.result.ActivityResultLauncher +import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.widget.Toolbar import androidx.core.view.isVisible import androidx.core.view.iterator +import androidx.core.view.updatePadding import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.navigation.fragment.findNavController @@ -51,6 +56,8 @@ import org.oxycblt.auxio.ui.ViewBindingFragment import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logE import org.oxycblt.auxio.util.logTraceOrThrow +import org.oxycblt.auxio.util.systemBarInsetsCompat +import org.oxycblt.auxio.util.textSafe import org.oxycblt.auxio.util.unlikelyToBeNull /** @@ -69,6 +76,12 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI override fun onBindingCreated(binding: FragmentHomeBinding, savedInstanceState: Bundle?) { val sortItem: MenuItem + // Build the permission launcher here as you can only do it in onCreateView/onCreate + val permLauncher = + registerForActivityResult(ActivityResultContracts.RequestPermission()) { + musicModel.reloadMusic(requireContext()) + } + binding.homeToolbar.apply { sortItem = menu.findItem(R.id.submenu_sorting) setOnMenuItemClickListener(this@HomeFragment) @@ -76,6 +89,11 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI updateTabConfiguration() + binding.homeLoadingContainer.setOnApplyWindowInsetsListener { v, insets -> + v.updatePadding(bottom = insets.systemBarInsetsCompat.bottom) + insets + } + binding.homePager.apply { adapter = HomePagerAdapter() @@ -104,7 +122,10 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI homeModel.currentTab.observe(viewLifecycleOwner) { tab -> updateCurrentTab(sortItem, tab) } homeModel.recreateTabs.observe(viewLifecycleOwner, ::handleRecreateTabs) - musicModel.loaderResponse.observe(viewLifecycleOwner, ::handleLoaderResponse) + musicModel.loaderResponse.observe(viewLifecycleOwner) { response -> + handleLoaderResponse(response, permLauncher) + } + navModel.exploreNavigationItem.observe(viewLifecycleOwner, ::handleNavigation) } @@ -236,15 +257,61 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI } } - private fun handleLoaderResponse(response: MusicStore.Response?) { + private fun handleLoaderResponse( + response: MusicStore.Response?, + permLauncher: ActivityResultLauncher + ) { val binding = requireBinding() - when (response) { - is MusicStore.Response.Ok -> binding.homeFab.show() - // While loading or during an error, make sure we keep the shuffle fab hidden so - // that any kind of playback is impossible. PlaybackStateManager also relies on this - // invariant, so please don't change it. - else -> binding.homeFab.hide() + if (response is MusicStore.Response.Ok) { + binding.homeFab.show() + binding.homeLoadingContainer.visibility = View.INVISIBLE + binding.homePager.visibility = View.VISIBLE + } else { + binding.homeFab.hide() + binding.homePager.visibility = View.INVISIBLE + binding.homeLoadingContainer.visibility = View.VISIBLE + + logD("Received non-ok response $response") + + when (response) { + is MusicStore.Response.Ok -> error("Unreachable") + is MusicStore.Response.Err -> { + logD("Received Response.Err") + binding.homeLoadingProgress.visibility = View.INVISIBLE + binding.homeLoadingStatus.textSafe = getString(R.string.err_load_failed) + binding.homeLoadingAction.apply { + visibility = View.VISIBLE + text = getString(R.string.lbl_retry) + setOnClickListener { musicModel.reloadMusic(requireContext()) } + } + } + is MusicStore.Response.NoMusic -> { + binding.homeLoadingProgress.visibility = View.INVISIBLE + binding.homeLoadingStatus.textSafe = getString(R.string.err_no_music) + binding.homeLoadingAction.apply { + visibility = View.VISIBLE + text = getString(R.string.lbl_retry) + setOnClickListener { musicModel.reloadMusic(requireContext()) } + } + } + is MusicStore.Response.NoPerms -> { + binding.homeLoadingProgress.visibility = View.INVISIBLE + binding.homeLoadingStatus.textSafe = getString(R.string.err_no_perms) + binding.homeLoadingAction.apply { + visibility = View.VISIBLE + text = getString(R.string.lbl_grant) + setOnClickListener { + permLauncher.launch(Manifest.permission.READ_EXTERNAL_STORAGE) + } + } + } + null -> { + binding.homeLoadingStatus.textSafe = getString(R.string.lbl_loading) + binding.homeLoadingAction.visibility = View.INVISIBLE + binding.homeLoadingProgress.visibility = View.VISIBLE + } + } } } 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 74b3d3a81..f406d9fc1 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -53,17 +53,13 @@ class MusicStore private constructor() { private val callbacks = mutableListOf() - /** - * Add a callback to this instance. Make sure to remove it when done. - */ + /** Add a callback to this instance. Make sure to remove it when done. */ fun addCallback(callback: Callback) { response?.let(callback::onMusicUpdate) callbacks.add(callback) } - /** - * Remove a callback from this instance. - */ + /** Remove a callback from this instance. */ fun removeCallback(callback: Callback) { callbacks.remove(callback) } @@ -78,7 +74,7 @@ class MusicStore private constructor() { return newResponse } - private fun loadImpl(context: Context): Response { + private suspend fun loadImpl(context: Context): Response { val notGranted = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) == PackageManager.PERMISSION_DENIED 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 922524c8c..e3b0b6756 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 @@ -62,8 +62,10 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { val songs = ConcurrentLinkedQueue() while (cursor.moveToNext()) { + // Note: This call to buildAudio does not populate the genre field. This is + // because indexing genres is quite slow with MediaStore, and so keeping the + // field blank on unsupported ExoPlayer formats ends up being preferable. val audio = inner.buildAudio(context, cursor) - val audioUri = requireNotNull(audio.id) { "Malformed audio: No id" }.audioUri while (true) { @@ -182,11 +184,7 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { } companion object { - /** - * The amount of tasks this backend can run efficiently at one time. Eight was chosen here - * as higher values made little difference in speed, and lower values generally caused - * bottlenecks. - */ + /** The amount of tasks this backend can run efficiently at once. */ private const val TASK_CAPACITY = 8 } } 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 542d49e73..0f3928597 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 @@ -74,11 +74,11 @@ object Indexer { // Sanity check: Ensure that all songs are linked up to albums/artists/genres. for (song in songs) { if (song._isMissingAlbum || song._isMissingArtist || song._isMissingGenre) { - throw IllegalStateException( - "Found malformed song: ${song.rawName} [" + - "album: ${!song._isMissingAlbum} " + - "artist: ${!song._isMissingArtist} " + - "genre: ${!song._isMissingGenre}]") + error( + "Found unlinked song: ${song.rawName} [" + + "missing album: ${song._isMissingAlbum} " + + "missing artist: ${song._isMissingArtist} " + + "missing genre: ${song._isMissingGenre}]") } } @@ -99,8 +99,10 @@ object Indexer { backend.query(context).use { cursor -> val loadStart = System.currentTimeMillis() logD("Successfully queried media database in ${loadStart - queryStart}ms") + val songs = backend.loadSongs(context, cursor) logD("Successfully loaded songs in ${System.currentTimeMillis() - loadStart}ms") + songs } @@ -181,7 +183,6 @@ object Indexer { for (entry in albumsByArtist) { // The first album will suffice for template metadata. val templateAlbum = entry.value[0] - artists.add(Artist(rawName = templateAlbum._artistGroupingName, albums = entry.value)) } @@ -213,4 +214,10 @@ object Indexer { /** Create a list of songs from the [Cursor] queried in [query]. */ fun loadSongs(context: Context, cursor: Cursor): Collection } + + sealed class Event { + object Query : Event() + object LoadSongs : Event() + object BuildLibrary : Event() + } } 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 9470352f7..7e6a388a1 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 @@ -135,7 +135,7 @@ abstract class MediaStoreBackend : Indexer.Backend { // The audio is not actually complete at this point, as we cannot obtain a genre // through a song query. Instead, we have to do the hack where we iterate through - // every genre and assign it's name to each component song. + // every genre and assign it's name to audios that match it's child ID. context.contentResolverSafe.useQuery( MediaStore.Audio.Genres.EXTERNAL_CONTENT_URI, @@ -149,35 +149,24 @@ abstract class MediaStoreBackend : Indexer.Backend { // anyway, so we skip genres that have them. val id = genreCursor.getLong(idIndex) val name = genreCursor.getStringOrNull(nameIndex) ?: continue - linkGenreAudios(context, id, name, audios) + + context.contentResolverSafe.useQuery( + MediaStore.Audio.Genres.Members.getContentUri(VOLUME_EXTERNAL, id), + arrayOf(MediaStore.Audio.Genres.Members._ID)) { cursor -> + val songIdIndex = + cursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.Members._ID) + + while (cursor.moveToNext()) { + val songId = cursor.getLong(songIdIndex) + audios.find { it.id == songId }?.let { song -> song.genre = name } + } + } } } return audios.map { it.toSong() } } - /** - * Links up the given genre data ([genreId] and [genreName]) to the child audios connected to - * [genreId]. - */ - private fun linkGenreAudios( - context: Context, - genreId: Long, - genreName: String, - audios: List