From 87bdf50d3987912b9a5f8ba75b87e2e392fcc0de Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sat, 28 May 2022 14:49:18 -0600 Subject: [PATCH] music: clean implementation Clean up the music implementation heavily, removing redundant code and splitting off some code into utilities. --- .../java/org/oxycblt/auxio/music/Music.kt | 39 ++--- .../org/oxycblt/auxio/music/MusicStore.kt | 17 +- .../org/oxycblt/auxio/music/MusicViewModel.kt | 3 + .../oxycblt/auxio/music/indexer/Indexer.kt | 54 +++--- .../auxio/music/indexer/MediaStoreBackend.kt | 162 +++++++++++------- .../oxycblt/auxio/playback/StyledSeekBar.kt | 4 + .../replaygain/ReplayGainAudioProcessor.kt | 6 + .../org/oxycblt/auxio/util/FrameworkUtil.kt | 19 ++ 8 files changed, 181 insertions(+), 123 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index 470344db4..518e9ae2e 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -19,7 +19,6 @@ package org.oxycblt.auxio.music -import android.content.ContentUris import android.content.Context import android.net.Uri import android.provider.MediaStore @@ -63,6 +62,8 @@ data class Song( override val rawName: String, /** The file name of this song, excluding the full path. */ val fileName: String, + /** The URI linking to this song's file. */ + val uri: Uri, /** The total duration of this song, in millis. */ val durationMs: Long, /** The track number of this song, null if there isn't any. */ @@ -70,19 +71,17 @@ data class Song( /** The disc number of this song, null if there isn't any. */ val disc: Int?, /** Internal field. Do not use. */ - val _mediaStoreId: Long, + val _year: Int?, /** Internal field. Do not use. */ - val _mediaStoreYear: Int?, + val _albumName: String, /** Internal field. Do not use. */ - val _mediaStoreAlbumName: String, + val _albumCoverUri: Uri, /** Internal field. Do not use. */ - val _mediaStoreAlbumId: Long, + val _artistName: String?, /** Internal field. Do not use. */ - val _mediaStoreArtistName: String?, + val _albumArtistName: String?, /** Internal field. Do not use. */ - val _mediaStoreAlbumArtistName: String?, - /** Internal field. Do not use. */ - val _mediaStoreGenreName: String? + val _genreName: String? ) : Music() { override val id: Long get() { @@ -100,11 +99,6 @@ data class Song( override fun resolveName(context: Context) = rawName - /** The URI for this song. */ - val uri: Uri - get() = - ContentUris.withAppendedId(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, _mediaStoreId) - /** The duration of this song, in seconds (rounded down) */ val durationSecs: Long get() = durationMs / 1000 @@ -124,26 +118,27 @@ data class Song( * back to the album artist tag (i.e parent artist name). Null if name is unknown. */ val individualRawArtistName: String? - get() = _mediaStoreArtistName ?: album.artist.rawName + get() = _artistName ?: album.artist.rawName /** * Resolve the artist name for this song in particular. First uses the artist tag, and then * falls back to the album artist tag (i.e parent artist name) */ fun resolveIndividualArtistName(context: Context) = - _mediaStoreArtistName ?: album.artist.resolveName(context) + _artistName ?: album.artist.resolveName(context) /** Internal field. Do not use. */ val _albumGroupingId: Long get() { - var result = _artistGroupingName.lowercase().hashCode().toLong() - result = 31 * result + _mediaStoreAlbumName.lowercase().hashCode() + var result = + (_artistGroupingName?.lowercase() ?: MediaStore.UNKNOWN_STRING).hashCode().toLong() + result = 31 * result + _albumName.lowercase().hashCode() return result } /** Internal field. Do not use. */ - val _artistGroupingName: String - get() = _mediaStoreAlbumArtistName ?: _mediaStoreArtistName ?: MediaStore.UNKNOWN_STRING + val _artistGroupingName: String? + get() = _albumArtistName ?: _artistName /** Internal field. Do not use. */ val _isMissingAlbum: Boolean @@ -176,7 +171,7 @@ data class Album( /** The songs of this album. */ override val songs: List, /** Internal field. Do not use. */ - val _artistGroupingName: String, + val _artistGroupingName: String?, ) : MusicParent() { init { for (song in songs) { @@ -204,7 +199,7 @@ data class Album( /** Internal field. Do not use. */ val _artistGroupingId: Long - get() = _artistGroupingName.lowercase().hashCode().toLong() + get() = (_artistGroupingName?.lowercase() ?: MediaStore.UNKNOWN_STRING).hashCode().toLong() /** Internal field. Do not use. */ val _isMissingArtist: Boolean 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 3a5b292cb..9ad64be8e 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -30,6 +30,7 @@ import org.oxycblt.auxio.music.indexer.Indexer import org.oxycblt.auxio.util.contentResolverSafe import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logE +import org.oxycblt.auxio.util.useQuery /** * The main storage for music items. Getting an instance of this object is more complicated as it @@ -117,17 +118,15 @@ class MusicStore private constructor() { * @return The corresponding [Song] for this [uri], null if there isn't one. */ fun findSongForUri(context: Context, uri: Uri): Song? { - context.contentResolverSafe - .query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null) - ?.use { cursor -> - cursor.moveToFirst() - val fileName = - cursor.getString(cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)) + return context.contentResolverSafe.useQuery( + uri, arrayOf(OpenableColumns.DISPLAY_NAME)) { cursor -> + cursor.moveToFirst() - return songs.find { it.fileName == fileName } - } + val displayName = + cursor.getString(cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)) - return null + songs.find { it.fileName == displayName } + } } } 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 1b7a08704..7c1374dd8 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt @@ -25,6 +25,9 @@ import androidx.lifecycle.viewModelScope import kotlinx.coroutines.launch import org.oxycblt.auxio.util.logD +/** + * A [ViewModel] that represents the current music indexing state. + */ class MusicViewModel : ViewModel(), MusicStore.Callback { private val musicStore = MusicStore.getInstance() 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 899b2d20c..c3a4894b2 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 @@ -17,10 +17,8 @@ package org.oxycblt.auxio.music.indexer -import android.content.ContentUris import android.content.Context import android.database.Cursor -import android.net.Uri import android.os.Build import android.provider.MediaStore import org.oxycblt.auxio.music.Album @@ -62,7 +60,7 @@ object Indexer { } /** - * Does the initial query over the song database, including excluded directory checks. The songs + * Does the initial query over the song database using [backend]. The songs * returned by this function are **not** well-formed. The companion [buildAlbums], * [buildArtists], and [buildGenres] functions must be called with the returned list so that all * songs are properly linked up. @@ -74,10 +72,10 @@ object Indexer { songs = songs.distinctBy { it.rawName to - it._mediaStoreAlbumName to - it._mediaStoreArtistName to - it._mediaStoreAlbumArtistName to - it._mediaStoreGenreName to + it._albumName to + it._artistName to + it._albumArtistName to + it._genreName to it.track to it.disc to it.durationMs @@ -122,22 +120,13 @@ object Indexer { } } - val albumName = templateSong._mediaStoreAlbumName - val albumYear = templateSong._mediaStoreYear - val albumCoverUri = - ContentUris.withAppendedId( - Uri.parse("content://media/external/audio/albumart"), - templateSong._mediaStoreAlbumId) - val artistName = templateSong._artistGroupingName - albums.add( Album( - albumName, - albumYear, - albumCoverUri, - albumSongs, - artistName, - )) + rawName = templateSong._albumName, + year = templateSong._year, + albumCoverUri = templateSong._albumCoverUri, + _artistGroupingName = templateSong._artistGroupingName, + songs = entry.value)) } logD("Successfully built ${albums.size} albums") @@ -154,16 +143,13 @@ object Indexer { val albumsByArtist = albums.groupBy { it._artistGroupingId } for (entry in albumsByArtist) { + // The first album will suffice for template metadata. val templateAlbum = entry.value[0] - val artistName = - if (templateAlbum._artistGroupingName != MediaStore.UNKNOWN_STRING) { - templateAlbum._artistGroupingName - } else { - null - } - val artistAlbums = entry.value - artists.add(Artist(artistName, artistAlbums)) + artists.add(Artist( + rawName = templateAlbum._artistGroupingName, + albums = entry.value + )) } logD("Successfully built ${artists.size} artists") @@ -172,16 +158,15 @@ object Indexer { } /** - * Read all genres and link them up to the given songs. This is the code that requires me to - * make dozens of useless queries just to link genres up. + * Build genres and link them to their particular songs. */ private fun buildGenres(songs: List): List { val genres = mutableListOf() - val songsByGenre = songs.groupBy { it._mediaStoreGenreName?.hashCode() } + val songsByGenre = songs.groupBy { it._genreName?.hashCode() } for (entry in songsByGenre) { val templateSong = entry.value[0] - genres.add(Genre(templateSong._mediaStoreGenreName, entry.value)) + genres.add(Genre(rawName = templateSong._genreName, songs = entry.value)) } logD("Successfully built ${genres.size} genres") @@ -190,7 +175,10 @@ object Indexer { } interface Backend { + /** Query the media database for an initial cursor. */ fun query(context: Context): Cursor + + /** Create a list of songs from the [Cursor] queried in [query]. */ fun loadSongs(context: Context, cursor: Cursor): 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 683e6fab0..98edeaf36 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 @@ -17,8 +17,10 @@ package org.oxycblt.auxio.music.indexer +import android.content.ContentUris import android.content.Context import android.database.Cursor +import android.net.Uri import android.os.Build import android.provider.MediaStore import androidx.annotation.RequiresApi @@ -27,10 +29,12 @@ import androidx.core.database.getStringOrNull import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.excluded.ExcludedDatabase import org.oxycblt.auxio.util.contentResolverSafe +import org.oxycblt.auxio.util.queryCursor +import org.oxycblt.auxio.util.useQuery /* - * This class acts as the base for most the black magic required to get a remotely sensible music - * indexing system while still optimizing for time. I would recommend you leave this module now + * This file acts as the base for most the black magic required to get a remotely sensible music + * indexing system while still optimizing for time. I would recommend you leave this file now * before you lose your sanity trying to understand the hoops I had to jump through for this system, * but if you really want to stay, here's a debrief on why this code is so awful. * @@ -88,6 +92,11 @@ import org.oxycblt.auxio.util.contentResolverSafe * I wish I was born in the neolithic. */ +/** + * Represents a [Indexer.Backend] that loads music from the media database ([MediaStore]). This is + * not a fully-featured class by itself, and it's API-specific derivatives should be used instead. + * @author OxygenCobalt + */ abstract class MediaStoreBackend : Indexer.Backend { private var idIndex = -1 private var titleIndex = -1 @@ -115,12 +124,11 @@ abstract class MediaStoreBackend : Indexer.Backend { } return requireNotNull( - context.contentResolverSafe.query( + context.contentResolverSafe.queryCursor( MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, projection, selector, - args.toTypedArray(), - null)) { "Content resolver failure: No Cursor returned" } + args.toTypedArray())) { "Content resolver failure: No Cursor returned" } } override fun loadSongs(context: Context, cursor: Cursor): Collection { @@ -129,56 +137,63 @@ abstract class MediaStoreBackend : Indexer.Backend { audios.add(buildAudio(context, cursor)) } - context.contentResolverSafe - .query( - MediaStore.Audio.Genres.EXTERNAL_CONTENT_URI, - arrayOf(MediaStore.Audio.Genres._ID, MediaStore.Audio.Genres.NAME), - null, - null, - null) - ?.use { genreCursor -> - val idIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres._ID) - val nameIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.NAME) + // 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. - while (genreCursor.moveToNext()) { - // Genre names can be a normal name, an ID3v2 constant, or null. Normal names - // are resolved as usual, but null values don't make sense and are often junk - // 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.EXTERNAL_CONTENT_URI, + arrayOf(MediaStore.Audio.Genres._ID, MediaStore.Audio.Genres.NAME)) { genreCursor -> + val idIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres._ID) + val nameIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.NAME) + + while (genreCursor.moveToNext()) { + // Genre names can be a normal name, an ID3v2 constant, or null. Normal names + // are resolved as usual, but null values don't make sense and are often junk + // anyway, so we skip genres that have them. + val id = genreCursor.getLong(idIndex) + val name = genreCursor.getStringOrNull(nameIndex) ?: continue + linkGenreAudios(context, id, name, audios) } + } 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