From 182b08ab655435ef6752bb52bf0dd568920b5498 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Thu, 2 Jun 2022 09:20:31 -0600 Subject: [PATCH] playback: fix state restore regression Fix a state restore issue that would cause the parent to restore incorrectly. At some point, I accidentally used the index for the PlaybackMode field when restoring the playbackState. This resulted in the playback mode effectively reverting to ALL_SONGS and causing a number of subtle issues. --- CHANGELOG.md | 1 + .../java/org/oxycblt/auxio/IntegerTable.kt | 3 + .../auxio/music/indexer/ExoPlayerBackend.kt | 7 +- .../oxycblt/auxio/music/indexer/Indexer.kt | 4 +- .../auxio/music/indexer/IndexerUtil.kt | 2 +- .../auxio/music/indexer/MediaStoreBackend.kt | 18 +++-- .../playback/state/PlaybackStateDatabase.kt | 3 +- .../oxycblt/auxio/settings/AboutFragment.kt | 58 ++++++++------- .../oxycblt/auxio/settings/SettingsManager.kt | 72 +++++++++---------- app/src/main/res/navigation/nav_explore.xml | 7 ++ prebuild.py | 2 +- 11 files changed, 98 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfc7f9bd8..027cedebc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ #### 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 issue where the album/artist/genre would not be correctly restored #### Dev/Meta - Switched from `LiveData` to `StateFlow` diff --git a/app/src/main/java/org/oxycblt/auxio/IntegerTable.kt b/app/src/main/java/org/oxycblt/auxio/IntegerTable.kt index 863b256fd..b2bcd46a0 100644 --- a/app/src/main/java/org/oxycblt/auxio/IntegerTable.kt +++ b/app/src/main/java/org/oxycblt/auxio/IntegerTable.kt @@ -84,6 +84,9 @@ object IntegerTable { /** DisplayMode.SHOW_SONGS */ const val DISPLAY_MODE_SHOW_SONGS = 0xA10B + // Note: Sort integer codes are non-contiguous due to significant amounts of time + // passing between the additions of new sort modes. + /** Sort.ByName */ const val SORT_BY_NAME = 0xA10C /** Sort.ByArtist */ 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 dbb7ff169..84d0e1acf 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 @@ -53,6 +53,8 @@ import org.oxycblt.auxio.util.logW class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { private val runningTasks: Array?> = arrayOfNulls(TASK_CAPACITY) + // No need to implement our own query logic, as this backend is still reliant on + // MediaStore. override fun query(context: Context) = inner.query(context) override fun loadSongs(context: Context, cursor: Cursor): Collection { @@ -69,7 +71,9 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { val audioUri = requireNotNull(audio.id) { "Malformed audio: No id" }.audioUri while (true) { - // Spin until a task slot opens up, then start trying to parse metadata. + // In order to properly parallelize ExoPlayer's parser, we have an array containing + // a few slots for ongoing futures. As soon as a finished task opens up their slot, + // we begin loading metadata for this audio. val index = runningTasks.indexOfFirst { it == null } if (index != -1) { val task = @@ -82,7 +86,6 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { runningTasks[index] = task - break } } 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 8dfb0a4b4..0745214f0 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 @@ -63,7 +63,9 @@ object Indexer { // } val songs = buildSongs(context, mediaStoreBackend) - if (songs.isEmpty()) return null + if (songs.isEmpty()) { + return null + } val buildStart = System.currentTimeMillis() diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt index f8fc87e21..28ae66970 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt @@ -56,7 +56,7 @@ val String.no: Int? /** * Parse out the year field from a (presumably) ISO-8601-like date. This differs across tag formats - * and has no real consistent, but it's assumed that most will format granular dates as YYYY-MM-DD + * and has no real consistency, but it's assumed that most will format granular dates as YYYY-MM-DD * (...) and thus we can parse the year out by splitting at the first -. */ val String.iso8601year: Int? 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 7e6a388a1..717227f77 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 @@ -63,12 +63,12 @@ import org.oxycblt.auxio.util.contentResolverSafe * table, so we have to go for the less efficient "make a big query on all the songs lol" method so * that songs don't end up fragmented across artists. Pretty much every OEM has added some extension * or quirk to MediaStore that I cannot reproduce, with some OEMs (COUGHSAMSUNGCOUGH) crippling the - * normal tables so that you're railroaded into their music app. The way I do blacklisting relies on - * a semi-deprecated method, and the supposedly "modern" method is SLOWER and causes even more - * problems since I have to manage databases across version boundaries. Sometimes music will have a - * deformed clone that I can't filter out, sometimes Genres will just break for no reason, and - * sometimes tags encoded in UTF-8 will be interpreted as anything from UTF-16 to Latin-1 to *Shift - * JIS* WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY + * normal tables so that you're railroaded into their music app. I have to use a semi-deprecated + * field to work with file paths, and the supposedly "modern" method is SLOWER and causes even more + * problems since some devices just don't expose those fields for some insane reason. Sometimes + * music will have a deformed clone that I can't filter out, sometimes Genres will just break for + * no reason, and sometimes tags encoded in UTF-8 will be interpreted as anything from UTF-16 to + * Latin-1 to *Shift JIS* WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY WHY * * Is there anything we can do about it? No. Google has routinely shut down issues that begged * google to fix glaring issues with MediaStore or to just take the API behind the woodshed and @@ -214,7 +214,10 @@ abstract class MediaStoreBackend : Indexer.Backend { audio.duration = cursor.getLong(durationIndex) audio.year = cursor.getIntOrNull(yearIndex) - audio.album = cursor.getStringOrNull(albumIndex) + // A non-existent album name should theoretically be the name of the folder it contained + // in, but in practice it is more often "0" (as in /storage/emulated/0), even when it the + // file is not actually in the root internal storage directory. + audio.album = cursor.getString(albumIndex) audio.albumId = cursor.getLong(albumIdIndex) // Android does not make a non-existent artist tag null, it instead fills it in @@ -230,6 +233,7 @@ abstract class MediaStoreBackend : Indexer.Backend { } } + // The album artist field is nullable and never has placeholder values. audio.albumArtist = cursor.getStringOrNull(albumArtistIndex) return audio diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt index d9fe4795e..3749812ae 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt @@ -155,7 +155,8 @@ class PlaybackStateDatabase(context: Context) : isShuffled = cursor.getInt(shuffleIndex) == 1, songId = cursor.getLong(songIdIndex), parentId = cursor.getLongOrNull(parentIdIndex), - playbackMode = PlaybackMode.fromInt(playbackModeIndex) ?: PlaybackMode.ALL_SONGS) + playbackMode = PlaybackMode.fromInt(cursor.getInt(playbackModeIndex)) + ?: PlaybackMode.ALL_SONGS) } } diff --git a/app/src/main/java/org/oxycblt/auxio/settings/AboutFragment.kt b/app/src/main/java/org/oxycblt/auxio/settings/AboutFragment.kt index ee404e42f..679435750 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/AboutFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/AboutFragment.kt @@ -28,11 +28,14 @@ import androidx.core.view.updatePadding import androidx.fragment.app.activityViewModels import androidx.navigation.fragment.findNavController import com.google.android.material.bottomsheet.BottomSheetDialogFragment -import kotlinx.coroutines.flow.collect import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.R import org.oxycblt.auxio.databinding.FragmentAboutBinding import org.oxycblt.auxio.home.HomeViewModel +import org.oxycblt.auxio.music.Album +import org.oxycblt.auxio.music.Artist +import org.oxycblt.auxio.music.Genre +import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.ui.ViewBindingFragment import org.oxycblt.auxio.util.formatDuration import org.oxycblt.auxio.util.launch @@ -63,38 +66,33 @@ class AboutFragment : ViewBindingFragment() { binding.aboutFaq.setOnClickListener { openLinkInBrowser(LINK_FAQ) } binding.aboutLicenses.setOnClickListener { openLinkInBrowser(LINK_LICENSES) } - launch { - homeModel.songs.collect { songs -> - binding.aboutSongCount.textSafe = getString(R.string.fmt_songs_loaded, songs.size) - binding.aboutTotalDuration.textSafe = - getString( - R.string.fmt_total_duration, - getString( - R.string.fmt_total_duration, - songs.sumOf { it.durationSecs }.formatDuration(false))) - } - } + launch { homeModel.songs.collect(::updateSongCount) } + launch { homeModel.albums.collect(::updateAlbumCount) } + launch { homeModel.artists.collect(::updateArtistCount) } + launch { homeModel.genres.collect(::updateGenreCount) } + } - launch { - homeModel.albums.collect { albums -> - binding.aboutAlbumCount.textSafe = - getString(R.string.fmt_albums_loaded, albums.size) - } - } + private fun updateSongCount(songs: List) { + val binding = requireBinding() + binding.aboutSongCount.textSafe = getString(R.string.fmt_songs_loaded, songs.size) + binding.aboutTotalDuration.textSafe = + getString( + R.string.fmt_total_duration, songs.sumOf { it.durationSecs }.formatDuration(false)) + } - launch { - homeModel.artists.collect { artists -> - binding.aboutArtistCount.textSafe = - getString(R.string.fmt_artists_loaded, artists.size) - } - } + private fun updateAlbumCount(albums: List) { + requireBinding().aboutAlbumCount.textSafe = + getString(R.string.fmt_albums_loaded, albums.size) + } - launch { - homeModel.genres.collect { genres -> - binding.aboutGenreCount.textSafe = - getString(R.string.fmt_genres_loaded, genres.size) - } - } + private fun updateArtistCount(artists: List) { + requireBinding().aboutArtistCount.textSafe = + getString(R.string.fmt_artists_loaded, artists.size) + } + + private fun updateGenreCount(genres: List) { + requireBinding().aboutGenreCount.textSafe = + getString(R.string.fmt_genres_loaded, genres.size) } /** Go through the process of opening a [link] in a browser. */ diff --git a/app/src/main/java/org/oxycblt/auxio/settings/SettingsManager.kt b/app/src/main/java/org/oxycblt/auxio/settings/SettingsManager.kt index db5aec157..a7d82ec00 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/SettingsManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/SettingsManager.kt @@ -38,27 +38,27 @@ import org.oxycblt.auxio.util.unlikelyToBeNull class SettingsManager private constructor(context: Context) : SharedPreferences.OnSharedPreferenceChangeListener { - private val prefs = PreferenceManager.getDefaultSharedPreferences(context) + private val inner = PreferenceManager.getDefaultSharedPreferences(context) init { - prefs.registerOnSharedPreferenceChangeListener(this) + inner.registerOnSharedPreferenceChangeListener(this) } // --- VALUES --- /** The current theme */ val theme: Int - get() = prefs.getInt(KEY_THEME, AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM) + get() = inner.getInt(KEY_THEME, AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM) /** Whether the dark theme should be black or not */ val useBlackTheme: Boolean - get() = prefs.getBoolean(KEY_BLACK_THEME, false) + get() = inner.getBoolean(KEY_BLACK_THEME, false) /** The current accent. */ var accent: Accent - get() = handleAccentCompat(prefs) + get() = handleAccentCompat(inner) set(value) { - prefs.edit { + inner.edit { putInt(KEY_ACCENT, value.index) apply() } @@ -69,15 +69,15 @@ class SettingsManager private constructor(context: Context) : * true if shuffle. */ val useAltNotifAction: Boolean - get() = prefs.getBoolean(KEY_USE_ALT_NOTIFICATION_ACTION, false) + get() = inner.getBoolean(KEY_USE_ALT_NOTIFICATION_ACTION, false) /** The current library tabs preferred by the user. */ var libTabs: Array get() = - Tab.fromSequence(prefs.getInt(KEY_LIB_TABS, Tab.SEQUENCE_DEFAULT)) + Tab.fromSequence(inner.getInt(KEY_LIB_TABS, Tab.SEQUENCE_DEFAULT)) ?: unlikelyToBeNull(Tab.fromSequence(Tab.SEQUENCE_DEFAULT)) set(value) { - prefs.edit { + inner.edit { putInt(KEY_LIB_TABS, Tab.toSequence(value)) apply() } @@ -85,33 +85,33 @@ class SettingsManager private constructor(context: Context) : /** Whether to load embedded covers */ val showCovers: Boolean - get() = prefs.getBoolean(KEY_SHOW_COVERS, true) + get() = inner.getBoolean(KEY_SHOW_COVERS, true) /** Whether to ignore MediaStore covers */ val useQualityCovers: Boolean - get() = prefs.getBoolean(KEY_QUALITY_COVERS, false) + get() = inner.getBoolean(KEY_QUALITY_COVERS, false) /** Whether to round album covers */ val roundCovers: Boolean - get() = prefs.getBoolean(KEY_ROUND_COVERS, false) + get() = inner.getBoolean(KEY_ROUND_COVERS, false) /** Whether to resume playback when a headset is connected (may not work well in all cases) */ val headsetAutoplay: Boolean - get() = prefs.getBoolean(KEY_HEADSET_AUTOPLAY, false) + get() = inner.getBoolean(KEY_HEADSET_AUTOPLAY, false) /** The current ReplayGain configuration */ val replayGainMode: ReplayGainMode get() = - ReplayGainMode.fromIntCode(prefs.getInt(KEY_REPLAY_GAIN, Int.MIN_VALUE)) + ReplayGainMode.fromIntCode(inner.getInt(KEY_REPLAY_GAIN, Int.MIN_VALUE)) ?: ReplayGainMode.OFF /** The current ReplayGain pre-amp configuration */ var replayGainPreAmp: ReplayGainPreAmp get() = ReplayGainPreAmp( - prefs.getFloat(KEY_PRE_AMP_WITH, 0f), prefs.getFloat(KEY_PRE_AMP_WITHOUT, 0f)) + inner.getFloat(KEY_PRE_AMP_WITH, 0f), inner.getFloat(KEY_PRE_AMP_WITHOUT, 0f)) set(value) { - prefs.edit { + inner.edit { putFloat(KEY_PRE_AMP_WITH, value.with) putFloat(KEY_PRE_AMP_WITHOUT, value.without) apply() @@ -121,29 +121,29 @@ class SettingsManager private constructor(context: Context) : /** What queue to create when a song is selected (ex. From All Songs or Search) */ val songPlaybackMode: PlaybackMode get() = - PlaybackMode.fromInt(prefs.getInt(KEY_SONG_PLAYBACK_MODE, Int.MIN_VALUE)) + PlaybackMode.fromInt(inner.getInt(KEY_SONG_PLAYBACK_MODE, Int.MIN_VALUE)) ?: PlaybackMode.ALL_SONGS /** Whether shuffle should stay on when a new song is selected. */ val keepShuffle: Boolean - get() = prefs.getBoolean(KEY_KEEP_SHUFFLE, true) + get() = inner.getBoolean(KEY_KEEP_SHUFFLE, true) /** Whether to rewind when the back button is pressed. */ val rewindWithPrev: Boolean - get() = prefs.getBoolean(KEY_PREV_REWIND, true) + get() = inner.getBoolean(KEY_PREV_REWIND, true) /** * Whether [org.oxycblt.auxio.playback.state.RepeatMode.TRACK] should pause when the track * repeats */ val pauseOnRepeat: Boolean - get() = prefs.getBoolean(KEY_PAUSE_ON_REPEAT, false) + get() = inner.getBoolean(KEY_PAUSE_ON_REPEAT, false) /** The current filter mode of the search tab */ var searchFilterMode: DisplayMode? - get() = DisplayMode.fromInt(prefs.getInt(KEY_SEARCH_FILTER_MODE, Int.MIN_VALUE)) + get() = DisplayMode.fromInt(inner.getInt(KEY_SEARCH_FILTER_MODE, Int.MIN_VALUE)) set(value) { - prefs.edit { + inner.edit { putInt(KEY_SEARCH_FILTER_MODE, value?.intCode ?: Int.MIN_VALUE) apply() } @@ -152,9 +152,9 @@ class SettingsManager private constructor(context: Context) : /** The song sort mode on HomeFragment */ var libSongSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_LIB_SONGS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) + Sort.fromIntCode(inner.getInt(KEY_LIB_SONGS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) set(value) { - prefs.edit { + inner.edit { putInt(KEY_LIB_SONGS_SORT, value.intCode) apply() } @@ -163,9 +163,9 @@ class SettingsManager private constructor(context: Context) : /** The album sort mode on HomeFragment */ var libAlbumSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_LIB_ALBUMS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) + Sort.fromIntCode(inner.getInt(KEY_LIB_ALBUMS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) set(value) { - prefs.edit { + inner.edit { putInt(KEY_LIB_ALBUMS_SORT, value.intCode) apply() } @@ -174,9 +174,9 @@ class SettingsManager private constructor(context: Context) : /** The artist sort mode on HomeFragment */ var libArtistSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_LIB_ARTISTS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) + Sort.fromIntCode(inner.getInt(KEY_LIB_ARTISTS_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) set(value) { - prefs.edit { + inner.edit { putInt(KEY_LIB_ARTISTS_SORT, value.intCode) apply() } @@ -185,9 +185,9 @@ class SettingsManager private constructor(context: Context) : /** The genre sort mode on HomeFragment */ var libGenreSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_LIB_GENRES_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) + Sort.fromIntCode(inner.getInt(KEY_LIB_GENRES_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) set(value) { - prefs.edit { + inner.edit { putInt(KEY_LIB_GENRES_SORT, value.intCode) apply() } @@ -197,7 +197,7 @@ class SettingsManager private constructor(context: Context) : var detailAlbumSort: Sort get() { var sort = - Sort.fromIntCode(prefs.getInt(KEY_DETAIL_ALBUM_SORT, Int.MIN_VALUE)) + Sort.fromIntCode(inner.getInt(KEY_DETAIL_ALBUM_SORT, Int.MIN_VALUE)) ?: Sort.ByDisc(true) // Correct legacy album sort modes to Disc @@ -208,7 +208,7 @@ class SettingsManager private constructor(context: Context) : return sort } set(value) { - prefs.edit { + inner.edit { putInt(KEY_DETAIL_ALBUM_SORT, value.intCode) apply() } @@ -217,10 +217,10 @@ class SettingsManager private constructor(context: Context) : /** The detail artist sort mode */ var detailArtistSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_DETAIL_ARTIST_SORT, Int.MIN_VALUE)) + Sort.fromIntCode(inner.getInt(KEY_DETAIL_ARTIST_SORT, Int.MIN_VALUE)) ?: Sort.ByYear(false) set(value) { - prefs.edit { + inner.edit { putInt(KEY_DETAIL_ARTIST_SORT, value.intCode) apply() } @@ -229,10 +229,10 @@ class SettingsManager private constructor(context: Context) : /** The detail genre sort mode */ var detailGenreSort: Sort get() = - Sort.fromIntCode(prefs.getInt(KEY_DETAIL_GENRE_SORT, Int.MIN_VALUE)) + Sort.fromIntCode(inner.getInt(KEY_DETAIL_GENRE_SORT, Int.MIN_VALUE)) ?: Sort.ByName(true) set(value) { - prefs.edit { + inner.edit { putInt(KEY_DETAIL_GENRE_SORT, value.intCode) apply() } diff --git a/app/src/main/res/navigation/nav_explore.xml b/app/src/main/res/navigation/nav_explore.xml index aa1ca9425..8482d5170 100644 --- a/app/src/main/res/navigation/nav_explore.xml +++ b/app/src/main/res/navigation/nav_explore.xml @@ -4,6 +4,13 @@ xmlns:tools="http://schemas.android.com/tools" app:startDestination="@id/home_fragment"> + +