music: clean implementation

Clean up the music implementation heavily, removing redundant code and
splitting off some code into utilities.

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?,
+    val _genreName: String?
-    /** Internal field. Do not use. */
-    val _mediaStoreGenreName: 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()
+            var result =
-            result = 31 * result + _mediaStoreAlbumName.lowercase().hashCode()
+                (_artistGroupingName?.lowercase() ?: MediaStore.UNKNOWN_STRING).hashCode().toLong()
+            result = 31 * result + _albumName.lowercase().hashCode()
             return result
         }
 
     /** Internal field. Do not use. */
-    val _artistGroupingName: String
+    val _artistGroupingName: String?
-        get() = _mediaStoreAlbumArtistName ?: _mediaStoreArtistName ?: MediaStore.UNKNOWN_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<Song>,
     /** 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

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
+            return context.contentResolverSafe.useQuery(
-                .query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)
+                uri, arrayOf(OpenableColumns.DISPLAY_NAME)) { cursor ->
-                ?.use { cursor ->
+                cursor.moveToFirst()
-                    cursor.moveToFirst()
-                    val fileName =
-                        cursor.getString(cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME))
 
-                    return songs.find { it.fileName == fileName }
+                val displayName =
-                }
+                    cursor.getString(cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME))
 
-            return null
+                songs.find { it.fileName == displayName }
+            }
         }
     }
 

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()
 

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._albumName to
-                    it._mediaStoreArtistName to
+                    it._artistName to
-                    it._mediaStoreAlbumArtistName to
+                    it._albumArtistName to
-                    it._mediaStoreGenreName 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,
+                    rawName = templateSong._albumName,
-                    albumYear,
+                    year = templateSong._year,
-                    albumCoverUri,
+                    albumCoverUri = templateSong._albumCoverUri,
-                    albumSongs,
+                    _artistGroupingName = templateSong._artistGroupingName,
-                    artistName,
+                    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
+     * Build genres and link them to their particular songs.
-     * make dozens of useless queries just to link genres up.
      */
     private fun buildGenres(songs: List<Song>): List<Genre> {
         val genres = mutableListOf<Genre>()
-        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<Song>
     }
 }

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
+ * 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 module now
+ * 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(),
+                args.toTypedArray())) { "Content resolver failure: No Cursor returned" }
-                null)) { "Content resolver failure: No Cursor returned" }
     }
 
     override fun loadSongs(context: Context, cursor: Cursor): Collection<Song> {
@@ -129,56 +137,63 @@ abstract class MediaStoreBackend : Indexer.Backend {
             audios.add(buildAudio(context, cursor))
         }
 
-        context.contentResolverSafe
+        // The audio is not actually complete at this point, as we cannot obtain a genre
-            .query(
+        // through a song query. Instead, we have to do the hack where we iterate through
-                MediaStore.Audio.Genres.EXTERNAL_CONTENT_URI,
+        // every genre and assign it's name to each component song.
-                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)
 
-                while (genreCursor.moveToNext()) {
+        context.contentResolverSafe.useQuery(
-                    // Genre names can be a normal name, an ID3v2 constant, or null. Normal names
+            MediaStore.Audio.Genres.EXTERNAL_CONTENT_URI,
-                    // are resolved as usual, but null values don't make sense and are often junk
+            arrayOf(MediaStore.Audio.Genres._ID, MediaStore.Audio.Genres.NAME)) { genreCursor ->
-                    // anyway, so we skip genres that have them.
+            val idIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres._ID)
-                    val id = genreCursor.getLong(idIndex)
+            val nameIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.NAME)
-                    val name = genreCursor.getStringOrNull(nameIndex) ?: continue
+
-                    linkGenreAudios(context, id, name, audios)
+            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<Audio>
     ) {
-        context.contentResolverSafe
+        context.contentResolverSafe.useQuery(
-            .query(
+            MediaStore.Audio.Genres.Members.getContentUri(VOLUME_EXTERNAL, genreId),
-                MediaStore.Audio.Genres.Members.getContentUri("external", genreId),
+            arrayOf(MediaStore.Audio.Genres.Members._ID)) { cursor ->
-                arrayOf(MediaStore.Audio.Genres.Members._ID),
+            val idIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.Members._ID)
-                null,
-                null,
-                null)
-            ?.use { cursor ->
-                val idIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.Members._ID)
 
-                while (cursor.moveToNext()) {
+            while (cursor.moveToNext()) {
-                    val id = cursor.getLong(idIndex)
+                val id = cursor.getLong(idIndex)
-                    audios.find { it.id == id }?.let { song -> song.genre = genreName }
+                audios.find { it.id == id }?.let { song -> song.genre = genreName }
-                }
             }
+        }
     }
 
+    /**
+     * The projection to use when querying media. Add version-specific columns here in our
+     * implementation.
+     */
     open val projection: Array<String>
-        get() = PROJECTION
+        get() = BASE_PROJECTION
 
+    /**
+     * Build an [Audio] based on the current cursor values. Each implementation should try to obtain
+     * an upstream [Audio] first, and then populate it with version-specific fields outlined in
+     * [projection].
+     */
     open fun buildAudio(context: Context, cursor: Cursor): Audio {
         // Initialize our cursor indices if we haven't already.
         if (idIndex == -1) {
@@ -254,18 +269,22 @@ abstract class MediaStoreBackend : Indexer.Backend {
     ) {
         fun toSong(): Song =
             Song(
-                requireNotNull(title) { "Malformed song: No title" },
+                rawName = requireNotNull(title) { "Malformed audio: No title" },
-                requireNotNull(displayName) { "Malformed song: No file name" },
+                fileName = requireNotNull(displayName) { "Malformed audio: No file name" },
-                requireNotNull(duration) { "Malformed song: No duration" },
+                uri = ContentUris.withAppendedId(
-                track,
+                    MediaStore.Audio.Media.EXTERNAL_CONTENT_URI,
-                disc,
+                    requireNotNull(id) { "Malformed audio: No song id" }),
-                requireNotNull(id) { "Malformed song: No song id" },
+                durationMs = requireNotNull(duration) { "Malformed audio: No duration" },
-                year,
+                track = track,
-                requireNotNull(album) { "Malformed song: No album name" },
+                disc = disc,
-                requireNotNull(albumId) { "Malformed song: No album id" },
+                _year = year,
-                artist,
+                _albumName = requireNotNull(album) { "Malformed song: No album name" },
-                albumArtist,
+                _albumCoverUri = ContentUris.withAppendedId(
-                genre)
+                    EXTERNAL_ALBUM_ART_URI,
+                    requireNotNull(albumId) { "Malformed song: No album id" }),
+                _artistName = artist,
+                _albumArtistName = albumArtist,
+                _genreName = genre)
     }
 
     companion object {
@@ -278,7 +297,23 @@ abstract class MediaStoreBackend : Indexer.Backend {
         @Suppress("InlinedApi")
         private const val AUDIO_COLUMN_ALBUM_ARTIST = MediaStore.Audio.AudioColumns.ALBUM_ARTIST
 
-        private val PROJECTION =
+        /**
+         * External has existed since at least API 21, but no constant existed for it until API 29.
+         * This constant is safe to use.
+         */
+        @Suppress("InlinedApi") private const val VOLUME_EXTERNAL = MediaStore.VOLUME_EXTERNAL
+
+        /**
+         * For some reason the album art URI namespace does not have a member in [MediaStore], but
+         * it still works since at least API 21.
+         */
+        private val EXTERNAL_ALBUM_ART_URI = Uri.parse("content://media/external/audio/albumart")
+
+        /**
+         * The basic projection that works across all versions of android. Is incomplete, hence why
+         * sub-implementations should be used instead.
+         */
+        private val BASE_PROJECTION =
             arrayOf(
                 MediaStore.Audio.AudioColumns._ID,
                 MediaStore.Audio.AudioColumns.TITLE,
@@ -293,6 +328,10 @@ abstract class MediaStoreBackend : Indexer.Backend {
     }
 }
 
+/**
+ * A [MediaStoreBackend] that completes the music loading process in a way compatible from
+ * @author OxygenCobalt
+ */
 class Api21MediaStoreBackend : MediaStoreBackend() {
     private var trackIndex = -1
 
@@ -325,6 +364,11 @@ class Api21MediaStoreBackend : MediaStoreBackend() {
     }
 }
 
+/**
+ * A [MediaStoreBackend] that completes the music loading process in a way compatible with at least
+ * API 30.
+ * @author OxygenCobalt
+ */
 @RequiresApi(Build.VERSION_CODES.R)
 class Api30MediaStoreBackend : MediaStoreBackend() {
     private var trackIndex: Int = -1
@@ -350,16 +394,16 @@ class Api30MediaStoreBackend : MediaStoreBackend() {
         // N is the number and T is the total. Parse the number while leaving out the
         // total, as we have no use for it.
 
-        cursor
+        cursor.getStringOrNull(trackIndex)?.no?.let { audio.track = it }
-            .getStringOrNull(trackIndex)
+        cursor.getStringOrNull(discIndex)?.no?.let { audio.disc = it }
-            ?.split('/', limit = 2)
-            ?.getOrNull(0)
-            ?.toIntOrNull()
-            ?.let { audio.track = it }
-        cursor.getStringOrNull(discIndex)?.split('/', limit = 2)?.getOrNull(0)?.toIntOrNull()?.let {
-            audio.disc = it
-        }
 
         return audio
     }
+
+    /**
+     * Parse out the number field from an NN/TT string that is typically found in DISC_NUMBER and
+     * CD_TRACK_NUMBER.
+     */
+    private val String.no: Int?
+        get() = split('/', limit = 2).getOrNull(0)?.toIntOrNull()
 }

app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt

@@ -73,6 +73,10 @@ constructor(
             // to jump around).
             if (value <= durationSecs && !isActivated) {
                 binding.seekBarSlider.value = value.toFloat()
+
+                // We would want to keep this in the callback, but the callback only fires when
+                // a value changes completely, and sometimes that does not happen with this view.
+                binding.seekBarPosition.textSafe = value.formatDuration(true)
             }
         }
 

app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt

@@ -133,10 +133,15 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() {
             val value: String
 
             when (entry) {
+                // ID3v2 text information frame, usually these are formatted in lowercase
+                // (like "replaygain_track_gain"), but can also be uppercase. Make sure that
+                // capitalization is consistent before continuing.
                 is TextInformationFrame -> {
                     key = entry.description?.uppercase()
                     value = entry.value
                 }
+                // Vorbis comment. These are nearly always uppercase, so a check for such is
+                // skipped.
                 is VorbisComment -> {
                     key = entry.key
                     value = entry.value
@@ -183,6 +188,7 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() {
                 albumGain += tag.value / 256f
                 found = true
             }
+
         return if (found) {
             Gain(trackGain, albumGain)
         } else {

app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt

@@ -17,6 +17,7 @@
  
 package org.oxycblt.auxio.util
 
+import android.content.ContentResolver
 import android.content.Context
 import android.content.res.ColorStateList
 import android.database.Cursor
@@ -24,6 +25,7 @@ import android.database.sqlite.SQLiteDatabase
 import android.graphics.Insets
 import android.graphics.Rect
 import android.graphics.drawable.Drawable
+import android.net.Uri
 import android.os.Build
 import android.view.View
 import android.view.WindowInsets
@@ -154,6 +156,23 @@ fun Fragment.requireAttached() = check(!isDetached) { "Fragment is detached from
 fun <R> SQLiteDatabase.queryAll(tableName: String, block: (Cursor) -> R) =
     query(tableName, null, null, null, null, null, null)?.use(block)
 
+/** Shortcut for making a [ContentResolver] query with less superfluous arguments. */
+fun ContentResolver.queryCursor(
+    uri: Uri,
+    projection: Array<out String>,
+    selector: String? = null,
+    args: Array<String>? = null
+) = query(uri, projection, selector, args, null)
+
+/** Shortcut for making a [ContentResolver] query and using the particular cursor with [use]. */
+fun <R> ContentResolver.useQuery(
+    uri: Uri,
+    projection: Array<out String>,
+    selector: String? = null,
+    args: Array<String>? = null,
+    block: (Cursor) -> R
+): R? = queryCursor(uri, projection, selector, args)?.use(block)
+
 /**
  * Resolve system bar insets in a version-aware manner. This can be used to apply padding to a view
  * that properly follows all the frustrating changes that were made between Android 8-11.