From 72a7aa6744e4e754f6a33ebf26a2f70fb3656433 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sat, 20 Mar 2021 16:14:36 -0600 Subject: [PATCH] Fix song restore issue Fix an issue where the playback restore process would only search the list of songs for the currently playing song, running the risk of picking the wrong song with the same name. --- .../oxycblt/auxio/database/DatabaseUtils.kt | 12 -------- .../oxycblt/auxio/database/PlaybackState.kt | 4 ++- .../auxio/database/PlaybackStateDatabase.kt | 6 +++- .../org/oxycblt/auxio/music/MusicStore.kt | 29 ++++++++++++++----- .../auxio/playback/PlaybackViewModel.kt | 2 +- .../playback/state/PlaybackStateManager.kt | 9 ++---- info/ARCHITECTURE.md | 3 +- info/FORMATS.md | 2 +- 8 files changed, 36 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/database/DatabaseUtils.kt b/app/src/main/java/org/oxycblt/auxio/database/DatabaseUtils.kt index 75a4bcfda..413785abe 100644 --- a/app/src/main/java/org/oxycblt/auxio/database/DatabaseUtils.kt +++ b/app/src/main/java/org/oxycblt/auxio/database/DatabaseUtils.kt @@ -29,18 +29,6 @@ fun SQLiteDatabase.execute(commands: SQLiteDatabase.() -> Unit): Boolean { return success } -/** - * Shortcut for running a query on this database and then running [block] with the cursor returned. - * Will not run if the cursor is null. - */ -fun SQLiteDatabase.queryUse( - tableName: String, - columns: Array?, - selection: String?, - vararg args: String, - block: (Cursor) -> R -) = query(tableName, columns, selection, args, null, null, null, null)?.use(block) - /** * Shortcut for querying all items in a database and running [block] with the cursor returned. * Will not run if the cursor is null. diff --git a/app/src/main/java/org/oxycblt/auxio/database/PlaybackState.kt b/app/src/main/java/org/oxycblt/auxio/database/PlaybackState.kt index 9685580a9..fa5a4cc22 100644 --- a/app/src/main/java/org/oxycblt/auxio/database/PlaybackState.kt +++ b/app/src/main/java/org/oxycblt/auxio/database/PlaybackState.kt @@ -15,6 +15,7 @@ package org.oxycblt.auxio.database data class PlaybackState( val id: Long = 0L, val songName: String = "", + val songAlbumName: String = "", val position: Long, val parentName: String = "", val index: Int, @@ -25,7 +26,8 @@ data class PlaybackState( ) { companion object { const val COLUMN_ID = "state_id" - const val COLUMN_SONG_NAME = "song_name" + const val COLUMN_SONG_NAME = "cur_song_name" + const val COLUMN_SONG_ALBUM_NAME = "cur_song_album" const val COLUMN_POSITION = "position" const val COLUMN_PARENT_NAME = "parent_name" const val COLUMN_INDEX = "state_index" diff --git a/app/src/main/java/org/oxycblt/auxio/database/PlaybackStateDatabase.kt b/app/src/main/java/org/oxycblt/auxio/database/PlaybackStateDatabase.kt index 6a9d22b82..21d48b446 100644 --- a/app/src/main/java/org/oxycblt/auxio/database/PlaybackStateDatabase.kt +++ b/app/src/main/java/org/oxycblt/auxio/database/PlaybackStateDatabase.kt @@ -53,6 +53,7 @@ class PlaybackStateDatabase(context: Context) : private fun constructStateTable(command: StringBuilder): StringBuilder { command.append("${PlaybackState.COLUMN_ID} LONG PRIMARY KEY,") .append("${PlaybackState.COLUMN_SONG_NAME} TEXT NOT NULL,") + .append("${PlaybackState.COLUMN_SONG_ALBUM_NAME} TEXT NOT NULL,") .append("${PlaybackState.COLUMN_POSITION} LONG NOT NULL,") .append("${PlaybackState.COLUMN_PARENT_NAME} TEXT NOT NULL,") .append("${PlaybackState.COLUMN_INDEX} INTEGER NOT NULL,") @@ -89,9 +90,10 @@ class PlaybackStateDatabase(context: Context) : this@PlaybackStateDatabase.logD("Wiped state db.") - val stateData = ContentValues(9).apply { + val stateData = ContentValues(10).apply { put(PlaybackState.COLUMN_ID, state.id) put(PlaybackState.COLUMN_SONG_NAME, state.songName) + put(PlaybackState.COLUMN_SONG_ALBUM_NAME, state.songAlbumName) put(PlaybackState.COLUMN_POSITION, state.position) put(PlaybackState.COLUMN_PARENT_NAME, state.parentName) put(PlaybackState.COLUMN_INDEX, state.index) @@ -120,6 +122,7 @@ class PlaybackStateDatabase(context: Context) : if (cursor.count == 0) return@queryAll val songIndex = cursor.getColumnIndexOrThrow(PlaybackState.COLUMN_SONG_NAME) + val albumIndex = cursor.getColumnIndexOrThrow(PlaybackState.COLUMN_SONG_ALBUM_NAME) val posIndex = cursor.getColumnIndexOrThrow(PlaybackState.COLUMN_POSITION) val parentIndex = cursor.getColumnIndexOrThrow(PlaybackState.COLUMN_PARENT_NAME) val indexIndex = cursor.getColumnIndexOrThrow(PlaybackState.COLUMN_INDEX) @@ -134,6 +137,7 @@ class PlaybackStateDatabase(context: Context) : state = PlaybackState( songName = cursor.getStringOrNull(songIndex) ?: "", + songAlbumName = cursor.getStringOrNull(albumIndex) ?: "", position = cursor.getLong(posIndex), parentName = cursor.getStringOrNull(parentIndex) ?: "", index = cursor.getInt(indexIndex), 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 5a4a15fdf..401a0853e 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -75,17 +75,30 @@ class MusicStore private constructor() { } /** - * Get the song for a file [uri]. + * Find a song from this instance in a safe manner. + * Using a normal search of the songs list runs the risk of getting the *wrong* song with + * the same name, so the album name is used to fix the above problem. + * @param name The name of the song + * @param albumName The name of the song's album. + * @return The song requested, null if there isnt one. + */ + fun findSong(name: String, albumName: String): Song? { + return albums.find { it.name == albumName }?.songs?.find { it.name == name } + } + + /** + * Find a song for a [uri], this is similar to [findSong], but with some kind of content uri. * @return The corresponding [Song] for this [uri], null if there isnt one. */ - fun getSongForUri(uri: Uri, resolver: ContentResolver): Song? { - resolver.query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null) - ?.use { cursor -> - cursor.moveToFirst() - val fileName = cursor.getString(cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)) + fun findSongForUri(uri: Uri, resolver: ContentResolver): Song? { + val cur = resolver.query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null) - return songs.find { it.fileName == fileName } - } + cur?.use { cursor -> + cursor.moveToFirst() + val fileName = cursor.getString(cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)) + + return songs.find { it.fileName == fileName } + } return null } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt index 0b6e3386c..1b9086691 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -183,7 +183,7 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback { private fun playWithUriInternal(uri: Uri, context: Context) { logD("Playing with uri $uri") - musicStore.getSongForUri(uri, context.contentResolver)?.let { song -> + musicStore.findSongForUri(uri, context.contentResolver)?.let { song -> playSong(song) } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt index 439d3fcf7..ca5a482a9 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt @@ -652,6 +652,7 @@ class PlaybackStateManager private constructor() { private fun packToPlaybackState(): PlaybackState { return PlaybackState( songName = mSong?.name ?: "", + songAlbumName = mSong?.album?.name ?: "", position = mPosition, parentName = mParent?.name ?: "", index = mIndex, @@ -674,7 +675,7 @@ class PlaybackStateManager private constructor() { mIndex = playbackState.index // Then set up the current state - mSong = musicStore.songs.find { it.name == playbackState.songName } + mSong = musicStore.findSong(playbackState.songName, playbackState.songAlbumName) mLoopMode = LoopMode.fromInt(playbackState.loopMode) ?: LoopMode.NONE mIsShuffling = playbackState.isShuffling mIsInUserQueue = playbackState.inUserQueue @@ -709,12 +710,8 @@ class PlaybackStateManager private constructor() { * @param queueItems The list of [QueueItem]s to unpack. */ private fun unpackQueue(queueItems: List) { - // When unpacking, first traverse albums and then traverse album songs to reduce - // the amount of comparisons in large queues. queueItems.forEach { item -> - val album = musicStore.albums.find { it.name == item.albumName } - - album?.songs?.find { it.name == item.songName }?.let { song -> + musicStore.findSong(item.songName, item.albumName)?.let { song -> if (item.isUserQueue) { mUserQueue.add(song) } else { diff --git a/info/ARCHITECTURE.md b/info/ARCHITECTURE.md index 4b7a1f6ac..b174e8d00 100644 --- a/info/ARCHITECTURE.md +++ b/info/ARCHITECTURE.md @@ -50,9 +50,10 @@ org.oxycblt.auxio # Main UI's and logging utilities │ └──.viewholders # Shared ViewHolders and ViewHolder utilities ├──.search # Search UI ├──.settings # Settings UI and systems +│ ├──.blacklist # Excluded Directories/Systems │ └──.ui # Contains UI's related to the settings view, such as the about screen ├──.songs # Songs UI -├──.ui # Shared user interface utilities +└──.ui # Shared user interface utilities ``` #### `.coil` diff --git a/info/FORMATS.md b/info/FORMATS.md index b0c18b10d..cba8446e9 100644 --- a/info/FORMATS.md +++ b/info/FORMATS.md @@ -14,7 +14,7 @@ Here are the music formats that Auxio supports, as per the [Supported ExoPlayer | MP3 | ✅ | | | MKA | ✅ | | | OGG | ✅ | Containing Vorbis, Opus, and FLAC | -| WAV | ✅ | | +| WAV | ✅ | | | MPEG | ✅ | | | AAC | ✅ | | | FLAC | ❌ | Auxio must be patched with the [FLAC Extension](https://github.com/google/ExoPlayer/tree/release-v2/extensions/flac) |