music: redocument

Redocument the music module.

Much of it's documentation has drifted from reality as changes were
made, this commit completely redoes the documentation in order to
fix that.

app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt

@@ -31,7 +31,6 @@ import org.oxycblt.auxio.detail.recycler.ArtistDetailAdapter
 import org.oxycblt.auxio.detail.recycler.DetailAdapter
 import org.oxycblt.auxio.list.Item
 import org.oxycblt.auxio.list.ListFragment
-import org.oxycblt.auxio.list.selection.SelectionToolbarOverlay
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Music

app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt

@@ -37,7 +37,6 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.MusicStore
-import org.oxycblt.auxio.music.ReleaseType
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.Sort
 import org.oxycblt.auxio.music.storage.MimeType
@@ -53,23 +52,6 @@ import org.oxycblt.auxio.util.*
  */
 class DetailViewModel(application: Application) :
     AndroidViewModel(application), MusicStore.Callback {
-    /**
-     * A simpler mapping of [ReleaseType] used for grouping and sorting songs.
-     * @param headerTitleRes The title string resource to use for a header created
-     * out of an instance of this enum.
-     */
-    private enum class ReleaseTypeGrouping(@StringRes val headerTitleRes: Int) {
-        ALBUMS(R.string.lbl_albums),
-        EPS(R.string.lbl_eps),
-        SINGLES(R.string.lbl_singles),
-        COMPILATIONS(R.string.lbl_compilations),
-        SOUNDTRACKS(R.string.lbl_soundtracks),
-        MIXES(R.string.lbl_mixes),
-        MIXTAPES(R.string.lbl_mixtapes),
-        LIVE(R.string.lbl_live_group),
-        REMIXES(R.string.lbl_remix_group),
-    }
-
     private val musicStore = MusicStore.getInstance()
     private val settings = Settings(application)
 
@@ -183,7 +165,8 @@ class DetailViewModel(application: Application) :
         }
 
         // If we are showing any item right now, we will need to refresh it (and any information
-        // related to it) with the new library in order to keep it fresh.
+        // related to it) with the new library in order to prevent stale items from showing up
+        // in the UI.
 
         val song = currentSong.value
         if (song != null) {
@@ -232,7 +215,7 @@ class DetailViewModel(application: Application) :
     /**
      * Set a new [currentAlbum] from it's [Music.UID]. If the [Music.UID] differs, [currentAlbum]
      * and [albumList] will be updated to align with the new [Album].
-     * @param uid The UID of the [Album] to update to. Must be valid.
+     * @param uid The [Music.UID] of the [Album] to update [currentAlbum] to. Must be valid.
      */
     fun setAlbumUid(uid: Music.UID) {
         if (_currentAlbum.value?.uid == uid) {
@@ -246,7 +229,7 @@ class DetailViewModel(application: Application) :
     /**
      * Set a new [currentArtist] from it's [Music.UID]. If the [Music.UID] differs, [currentArtist]
      * and [artistList] will be updated to align with the new [Artist].
-     * @param uid The UID of the [Album] to update to. Must be valid.
+     * @param uid The [Music.UID] of the [Artist] to update [currentArtist] to. Must be valid.
      */
     fun setArtistUid(uid: Music.UID) {
         if (_currentArtist.value?.uid == uid) {
@@ -260,7 +243,7 @@ class DetailViewModel(application: Application) :
     /**
      * Set a new [currentGenre] from it's [Music.UID]. If the [Music.UID] differs, [currentGenre]
      * and [genreList] will be updated to align with the new album.
-     * @param uid The UID of the [Album] to update to. Must be valid.
+     * @param uid The [Music.UID] of the [Genre] to update [currentGenre] to. Must be valid.
      */
     fun setGenreUid(uid: Music.UID) {
         if (_currentGenre.value?.uid == uid)  {
@@ -405,21 +388,21 @@ class DetailViewModel(application: Application) :
 
         val byReleaseGroup =
             albums.groupBy {
-                // Remap the complicated ReleaseType data structure into an easier
+                // Remap the complicated Album.Type data structure into an easier
-                // "ReleaseTypeGrouping" enum that will automatically group and sort
+                // "AlbumGrouping" enum that will automatically group and sort
                 // the artist's albums.
-                when (it.releaseType.refinement) {
+                when (it.type.refinement) {
-                    ReleaseType.Refinement.LIVE -> ReleaseTypeGrouping.LIVE
+                    Album.Type.Refinement.LIVE -> AlbumGrouping.LIVE
-                    ReleaseType.Refinement.REMIX -> ReleaseTypeGrouping.REMIXES
+                    Album.Type.Refinement.REMIX -> AlbumGrouping.REMIXES
                     null ->
-                        when (it.releaseType) {
+                        when (it.type) {
-                            is ReleaseType.Album -> ReleaseTypeGrouping.ALBUMS
+                            is Album.Type.Album -> AlbumGrouping.ALBUMS
-                            is ReleaseType.EP -> ReleaseTypeGrouping.EPS
+                            is Album.Type.EP -> AlbumGrouping.EPS
-                            is ReleaseType.Single -> ReleaseTypeGrouping.SINGLES
+                            is Album.Type.Single -> AlbumGrouping.SINGLES
-                            is ReleaseType.Compilation -> ReleaseTypeGrouping.COMPILATIONS
+                            is Album.Type.Compilation -> AlbumGrouping.COMPILATIONS
-                            is ReleaseType.Soundtrack -> ReleaseTypeGrouping.SOUNDTRACKS
+                            is Album.Type.Soundtrack -> AlbumGrouping.SOUNDTRACKS
-                            is ReleaseType.Mix -> ReleaseTypeGrouping.MIXES
+                            is Album.Type.Mix -> AlbumGrouping.MIXES
-                            is ReleaseType.Mixtape -> ReleaseTypeGrouping.MIXTAPES
+                            is Album.Type.Mixtape -> AlbumGrouping.MIXTAPES
                         }
                 }
             }
@@ -455,4 +438,21 @@ class DetailViewModel(application: Application) :
         data.addAll(genreSort.songs(genre.songs))
         _genreData.value = data
     }
+
+    /**
+     * A simpler mapping of [Album.Type] used for grouping and sorting songs.
+     * @param headerTitleRes The title string resource to use for a header created
+     * out of an instance of this enum.
+     */
+    private enum class AlbumGrouping(@StringRes val headerTitleRes: Int) {
+        ALBUMS(R.string.lbl_albums),
+        EPS(R.string.lbl_eps),
+        SINGLES(R.string.lbl_singles),
+        COMPILATIONS(R.string.lbl_compilations),
+        SOUNDTRACKS(R.string.lbl_soundtracks),
+        MIXES(R.string.lbl_mixes),
+        MIXTAPES(R.string.lbl_mixtapes),
+        LIVE(R.string.lbl_live_group),
+        REMIXES(R.string.lbl_remix_group),
+    }
 }

app/src/main/java/org/oxycblt/auxio/detail/recycler/AlbumDetailAdapter.kt

@@ -126,7 +126,7 @@ private class AlbumDetailViewHolder private constructor(private val binding: Ite
         binding.detailCover.bind(album)
 
         // The type text depends on the release type (Album, EP, Single, etc.)
-        binding.detailType.text = binding.context.getString(album.releaseType.stringRes)
+        binding.detailType.text = binding.context.getString(album.type.stringRes)
 
         binding.detailName.text = album.resolveName(binding.context)
 
@@ -173,7 +173,7 @@ private class AlbumDetailViewHolder private constructor(private val binding: Ite
                         oldItem.date == newItem.date &&
                         oldItem.songs.size == newItem.songs.size &&
                         oldItem.durationMs == newItem.durationMs &&
-                        oldItem.releaseType == newItem.releaseType
+                        oldItem.type == newItem.type
             }
     }
 }

app/src/main/java/org/oxycblt/auxio/detail/recycler/DetailAdapter.kt

@@ -44,28 +44,6 @@ abstract class DetailAdapter(
     private val callback: Listener,
     itemCallback: DiffUtil.ItemCallback<Item>
 ) : SelectionIndicatorAdapter<RecyclerView.ViewHolder>(), AuxioRecyclerView.SpanSizeLookup {
-    /** An extended [ExtendedListListener] for [DetailAdapter] implementations. */
-    interface Listener : ExtendedListListener {
-        // TODO: Split off into sub-listeners if a collapsing toolbar is implemented.
-        /**
-         * Called when the play button in a detail header is pressed, requesting that the current
-         * item should be played.
-         */
-        fun onPlay()
-
-        /**
-         * Called when the shuffle button in a detail header is pressed, requesting that the current
-         * item should be shuffled
-         */
-        fun onShuffle()
-
-        /**
-         * Called when the button in a [SortHeader] item is pressed, requesting that the sort menu
-         * should be opened.
-         */
-        fun onOpenSortMenu(anchor: View)
-    }
-
     // Safe to leak this since the callback will not fire during initialization
     @Suppress("LeakingThis") protected val differ = AsyncListDiffer(this, itemCallback)
 
@@ -111,6 +89,30 @@ abstract class DetailAdapter(
         differ.submitList(newList)
     }
 
+    /**
+     * An extended [ExtendedListListener] for [DetailAdapter] implementations.
+     */
+    interface Listener : ExtendedListListener {
+        // TODO: Split off into sub-listeners if a collapsing toolbar is implemented.
+        /**
+         * Called when the play button in a detail header is pressed, requesting that the current
+         * item should be played.
+         */
+        fun onPlay()
+
+        /**
+         * Called when the shuffle button in a detail header is pressed, requesting that the current
+         * item should be shuffled
+         */
+        fun onShuffle()
+
+        /**
+         * Called when the button in a [SortHeader] item is pressed, requesting that the sort menu
+         * should be opened.
+         */
+        fun onOpenSortMenu(anchor: View)
+    }
+
     companion object {
         /** A comparator that can be used with DiffUtil. */
         val DIFF_CALLBACK =

app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt

@@ -76,7 +76,7 @@ class HomeFragment :
     // lifecycleObject builds this in the creation step, so doing this is okay.
     private val storagePermissionLauncher: ActivityResultLauncher<String> by lifecycleObject {
         registerForActivityResult(ActivityResultContracts.RequestPermission()) {
-            musicModel.reindex(true)
+            musicModel.refresh()
         }
     }
 
@@ -365,29 +365,29 @@ class HomeFragment :
                     logD("Updating UI to Response.Err state")
                     binding.homeIndexingStatus.text = context.getString(R.string.err_index_failed)
 
-                    // Configure the indexing button to act as a rescan trigger.
+                    // Configure the action to act as a reload trigger.
                     binding.homeIndexingAction.apply {
                         visibility = View.VISIBLE
                         text = context.getString(R.string.lbl_retry)
-                        setOnClickListener { musicModel.reindex(true) }
+                        setOnClickListener { musicModel.refresh() }
                     }
                 }
                 is Indexer.Response.NoMusic -> {
                     logD("Updating UI to Response.NoMusic state")
                     binding.homeIndexingStatus.text = context.getString(R.string.err_no_music)
 
-                    // Configure the indexing button to act as a rescan trigger.
+                    // Configure the action to act as a reload trigger.
                     binding.homeIndexingAction.apply {
                         visibility = View.VISIBLE
                         text = context.getString(R.string.lbl_retry)
-                        setOnClickListener { musicModel.reindex(true) }
+                        setOnClickListener { musicModel.refresh() }
                     }
                 }
                 is Indexer.Response.NoPerms -> {
                     logD("Updating UI to Response.NoPerms state")
                     binding.homeIndexingStatus.text = context.getString(R.string.err_no_perms)
 
-                    // Configure the indexing button to act as a permission launcher.
+                    // Configure the action to act as a permission launcher.
                     binding.homeIndexingAction.apply {
                         visibility = View.VISIBLE
                         text = context.getString(R.string.lbl_grant)

app/src/main/java/org/oxycblt/auxio/home/tabs/Tab.kt

@@ -17,10 +17,6 @@
  
 package org.oxycblt.auxio.home.tabs
 
-import org.oxycblt.auxio.home.tabs.Tab.Companion.fromSequence
-import org.oxycblt.auxio.home.tabs.Tab.Companion.toSequence
-import org.oxycblt.auxio.home.tabs.Tab.Invisible
-import org.oxycblt.auxio.home.tabs.Tab.Visible
 import org.oxycblt.auxio.music.MusicMode
 import org.oxycblt.auxio.util.logE
 
@@ -76,11 +72,11 @@ sealed class Tab(open val mode: MusicMode) {
             arrayOf(MusicMode.SONGS, MusicMode.ALBUMS, MusicMode.ARTISTS, MusicMode.GENRES)
 
         /**
-         * Convert an array of tabs into it's integer representation.
+         * Convert an array of [Tab]s into it's integer representation.
-         * @param tabs The array of tabs to convert
+         * @param tabs The array of [Tab]s to convert
-         * @return An integer representation of the tab array
+         * @return An integer representation of the [Tab] array
          */
-        fun toSequence(tabs: Array<Tab>): Int {
+        fun toIntCode(tabs: Array<Tab>): Int {
             // Like when deserializing, make sure there are no duplicate tabs for whatever reason.
             val distinct = tabs.distinctBy { it.mode }
 
@@ -102,11 +98,11 @@ sealed class Tab(open val mode: MusicMode) {
         }
 
         /**
-         * Convert a tab integer representation into an array of tabs.
+         * Convert a [Tab] integer representation into it's corresponding array of [Tab]s.
-         * @param sequence The integer representation of the tabs.
+         * @param sequence The integer representation of the [Tab]s.
-         * @return An array of tabs corresponding to the sequence.
+         * @return An array of [Tab]s corresponding to the sequence.
          */
-        fun fromSequence(sequence: Int): Array<Tab>? {
+        fun fromIntCode(sequence: Int): Array<Tab>? {
             val tabs = mutableListOf<Tab>()
 
             // Try to parse a mode for each chunk in the sequence.

app/src/main/java/org/oxycblt/auxio/home/tabs/TabAdapter.kt

@@ -69,10 +69,9 @@ class TabAdapter(private val listener: Listener) : RecyclerView.Adapter<TabViewH
      * Immediately update the tab array. This should be used when initializing the list.
      * @param newTabs The new array of tabs to show.
      */
-    @Suppress("NotifyDatasetChanged")
     fun submitTabs(newTabs: Array<Tab>) {
         tabs = newTabs
-        notifyDataSetChanged()
+        @Suppress("NotifyDatasetChanged") notifyDataSetChanged()
     }
 
     /**

app/src/main/java/org/oxycblt/auxio/home/tabs/TabCustomizeDialog.kt

@@ -59,7 +59,7 @@ class TabCustomizeDialog : ViewBindingDialogFragment<DialogTabsBinding>(), TabAd
         var tabs = settings.libTabs
         // Try to restore a pending tab configuration that was saved prior.
         if (savedInstanceState != null) {
-            val savedTabs = Tab.fromSequence(savedInstanceState.getInt(KEY_TABS))
+            val savedTabs = Tab.fromIntCode(savedInstanceState.getInt(KEY_TABS))
             if (savedTabs != null) {
                 tabs = savedTabs
             }
@@ -76,7 +76,7 @@ class TabCustomizeDialog : ViewBindingDialogFragment<DialogTabsBinding>(), TabAd
     override fun onSaveInstanceState(outState: Bundle) {
         super.onSaveInstanceState(outState)
         // Save any pending tab configurations to restore from when this dialog is re-created.
-        outState.putInt(KEY_TABS, Tab.toSequence(tabAdapter.tabs))
+        outState.putInt(KEY_TABS, Tab.toIntCode(tabAdapter.tabs))
     }
 
     override fun onDestroyBinding(binding: DialogTabsBinding) {

app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt

@@ -114,7 +114,6 @@ class BitmapProvider(private val context: Context) {
                                 }
                             }
                         })
-
         currentRequest = Request(context.imageLoader.enqueue(request.build()), target)
     }
 

app/src/main/java/org/oxycblt/auxio/image/CoverMode.kt

@@ -39,6 +39,7 @@ enum class CoverMode {
 
     /**
      * The integer representation of this instance.
+     * @see fromIntCode
      */
     val intCode: Int
         get() =
@@ -50,9 +51,10 @@ enum class CoverMode {
 
     companion object {
         /**
-         * Convert a [CoverMode], integer representation into an instance.
+         * Convert a [CoverMode] integer representation into an instance.
          * @param intCode An integer representation of a [CoverMode]
          * @return The corresponding [CoverMode], or null if the [CoverMode] is invalid.
+         * @see intCode
          */
         fun fromIntCode(intCode: Int) =
             when (intCode) {

app/src/main/java/org/oxycblt/auxio/image/ImageGroup.kt

@@ -55,17 +55,12 @@ class ImageGroup
 @JvmOverloads
 constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr: Int = 0) :
     FrameLayout(context, attrs, defStyleAttr) {
-    // Most attributes are simply handled by StyledImageView.
     private val innerImageView: StyledImageView
-    // The custom view is populated when the layout inflates.
     private var customView: View? = null
-    // PlaybackIndicatorView overlays on top of the StyledImageView and custom view.
     private val playbackIndicatorView: PlaybackIndicatorView
-    // The selection indicator view overlays all previous views.
     private val selectionIndicatorView: ImageView
-    // Animator to handle selection visibility animations
+
     private var fadeAnimator: ValueAnimator? = null
-    // Keep track of our corner radius so that we can apply the same attributes to the custom view.
     private val cornerRadius: Float
 
     init {
@@ -73,6 +68,8 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
         // then throw an error if you do because of duplicate attribute names.
         @SuppressLint("CustomViewStyleable")
         val styledAttrs = context.obtainStyledAttributes(attrs, R.styleable.StyledImageView)
+        // Keep track of our corner radius so that we can apply the same attributes to the custom
+        // view.
         cornerRadius = styledAttrs.getDimension(R.styleable.StyledImageView_cornerRadius, 0f)
         styledAttrs.recycle()
 
@@ -87,6 +84,8 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
                 setBackgroundResource(R.drawable.ui_selection_badge_bg)
             }
 
+        // The inner StyledImageView should be at the bottom and hidden by any other elements
+        // if they become visible.
         addView(innerImageView)
     }
 
@@ -95,8 +94,8 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
         // Due to innerImageView, the max child count is actually 2 and not 1.
         check(childCount < 3) { "Only one custom view is allowed" }
 
-        // Get the second inflated child, if it exists, and then customize it to
+        // Get the second inflated child, making sure we customize it to align with
-        // act like the other components in this view.
+        // the rest of this view.
         customView =
             getChildAt(1)?.apply {
                 background =
@@ -106,8 +105,9 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
                     }
             }
 
-        // Add the other two views to complete the layering.
+        // Playback indicator should sit above the inner StyledImageView and custom view/
         addView(playbackIndicatorView)
+        // Selction indicator should never be obscured, so place it at the top.
         addView(
             selectionIndicatorView,
             LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT).apply {

app/src/main/java/org/oxycblt/auxio/image/extractor/SquareFrameTransform.kt

@@ -24,8 +24,8 @@ import coil.transform.Transformation
 import kotlin.math.min
 
 /**
- * A transformation that performs a center crop-style transformation on an image, however unlike the
+ * A transformation that performs a center crop-style transformation on an image. Allowing this
- * actual ScaleType, this isn't affected by any hacks we do with ImageView itself.
+ * behavior to be intrinsic without any view configuration.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class SquareFrameTransform : Transformation {
@@ -38,20 +38,21 @@ class SquareFrameTransform : Transformation {
         val dstSize = min(input.width, input.height)
         val x = (input.width - dstSize) / 2
         val y = (input.height - dstSize) / 2
+        val dst = Bitmap.createBitmap(input, x, y, dstSize, dstSize)
 
         val desiredWidth = size.width.pxOrElse { dstSize }
         val desiredHeight = size.height.pxOrElse { dstSize }
-
-        val dst = Bitmap.createBitmap(input, x, y, dstSize, dstSize)
-
         if (dstSize != desiredWidth || dstSize != desiredHeight) {
+            // Image is not the desired size, upscale it.
             return Bitmap.createScaledBitmap(dst, desiredWidth, desiredHeight, true)
         }
-
         return dst
     }
 
     companion object {
+        /**
+         * A shared instance that can be re-used.
+         */
         val INSTANCE = SquareFrameTransform()
     }
 }

app/src/main/java/org/oxycblt/auxio/list/recycler/ViewHolders.kt

@@ -125,7 +125,7 @@ class AlbumViewHolder private constructor(private val binding: ItemParentBinding
                 override fun areContentsTheSame(oldItem: Album, newItem: Album) =
                     oldItem.rawName == newItem.rawName &&
                         oldItem.areArtistContentsTheSame(newItem) &&
-                        oldItem.releaseType == newItem.releaseType
+                        oldItem.type == newItem.type
             }
     }
 }

app/src/main/java/org/oxycblt/auxio/music/MusicMode.kt

@@ -19,12 +19,35 @@ package org.oxycblt.auxio.music
 
 import org.oxycblt.auxio.IntegerTable
 
+/**
+ * Represents a data configuration corresponding to a specific type of [Music],
+ * @author Alexander Capehart (OxygenCobalt)
+ */
 enum class MusicMode {
+    /**
+     * Configure with respect to [Song] instances.
+     */
     SONGS,
+
+    /**
+     * Configure with respect to [Album] instances.
+     */
     ALBUMS,
+
+    /**
+     * Configure with respect to [Artist] instances.
+     */
     ARTISTS,
+
+    /**
+     * Configure with respect to [Genre] instances.
+     */
     GENRES;
 
+    /**
+     * The integer representation of this instance.
+     * @see fromIntCode
+     */
     val intCode: Int
         get() =
             when (this) {
@@ -35,8 +58,14 @@ enum class MusicMode {
             }
 
     companion object {
-        fun fromInt(value: Int) =
+        /**
-            when (value) {
+         * Convert a [MusicMode] integer representation into an instance.
+         * @param intCode An integer representation of a [MusicMode]
+         * @return The corresponding [MusicMode], or null if the [MusicMode] is invalid.
+         * @see intCode
+         */
+        fun fromIntCode(intCode: Int) =
+            when (intCode) {
                 IntegerTable.MUSIC_MODE_SONGS -> SONGS
                 IntegerTable.MUSIC_MODE_ALBUMS -> ALBUMS
                 IntegerTable.MUSIC_MODE_ARTISTS -> ARTISTS

app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt

@@ -26,102 +26,141 @@ import org.oxycblt.auxio.music.storage.useQuery
 import org.oxycblt.auxio.util.contentResolverSafe
 
 /**
- * The main storage for music items.
+ * A repository granting access to the music library..
  *
- * Whereas other apps load music from MediaStore as it is shown, Auxio does not do that, as it
+ * This can be used to obtain certain music items, or await changes to the music library.
- * cripples any kind of advanced metadata functionality. Instead, Auxio loads all music into a
+ * It is generally recommended to use this over Indexer to keep track of the library state,
- * in-memory relational data-structure called [Library]. This costs more memory-wise, but is also
+ * as the interface will be less volatile.
- * much more sensible.
- *
- * The only other, memory-efficient option is to create our own hybrid database that leverages both
- * a typical DB and a mem-cache, like Vinyl. But why would we do that when I've encountered no real
- * issues with the current system?
- *
- * [Library] may not be available at all times, so leveraging [Callback] is recommended. Consumers
- * should also be aware that [Library] may change while they are running, and design their work
- * accordingly.
  *
  * @author Alexander Capehart (OxygenCobalt)
  */
 class MusicStore private constructor() {
     private val callbacks = mutableListOf<Callback>()
 
+    /**
+     * The current [Library]. May be null if a [Library] has not been successfully loaded yet.
+     * This can change, so it's highly recommended to not access this directly and instead
+     * rely on [Callback].
+     */
     var library: Library? = null
-        private set
+        set(value) {
+            field = value
+            for (callback in callbacks) {
+                callback.onLibraryChanged(library)
+            }
+        }
 
-    /** Add a callback to this instance. Make sure to remove it when done. */
+    /**
+     * Add a [Callback] to this instance. This can be used to receive changes in the music
+     * library. Will invoke all [Callback] methods to initialize the instance with the
+     * current state.
+     * @param callback The [Callback] to add.
+     * @see Callback
+     */
     @Synchronized
     fun addCallback(callback: Callback) {
         callback.onLibraryChanged(library)
         callbacks.add(callback)
     }
 
-    /** Remove a callback from this instance. */
+    /**
+     * Remove a [Callback] from this instance, preventing it from recieving any further
+     * updates.
+     * @param callback The [Callback] to remove. Does nothing if the [Callback] was never
+     * added in the first place.
+     * @see Callback
+     */
     @Synchronized
     fun removeCallback(callback: Callback) {
         callbacks.remove(callback)
     }
 
-    /** Update the library in this instance. This is only meant for use by the internal indexer. */
+    /**
-    @Synchronized
+     * A library of [Music] instances.
-    fun updateLibrary(newLibrary: Library?) {
+     * @param songs All [Song]s loaded from the device.
-        library = newLibrary
+     * @param albums All [Album]s that could be created.
-        for (callback in callbacks) {
+     * @param artists All [Artist]s that could be created.
-            callback.onLibraryChanged(library)
+     * @param genres All [Genre]s that could be created.
-        }
+     */
-    }
-
-    /** Represents a library of music owned by [MusicStore]. */
     data class Library(
-        val genres: List<Genre>,
+        val songs: List<Song>,
-        val artists: List<Artist>,
         val albums: List<Album>,
-        val songs: List<Song>
+        val artists: List<Artist>,
+        val genres: List<Genre>,
     ) {
         private val uidMap = HashMap<Music.UID, Music>()
 
         init {
+            // The data passed to Library initially are complete, but are still volitaile.
+            // Finalize them to ensure they are well-formed. Initialize the UID map in the
+            // same loop for efficiency.
             for (song in songs) {
+                song._finalize()
                 uidMap[song.uid] = song
             }
 
             for (album in albums) {
+                album._finalize()
                 uidMap[album.uid] = album
             }
 
             for (artist in artists) {
+                artist._finalize()
                 uidMap[artist.uid] = artist
             }
 
             for (genre in genres) {
+                genre._finalize()
                 uidMap[genre.uid] = genre
             }
         }
 
         /**
-         * Find a music [T] by its [uid]. If the music does not exist, or if the music is not [T],
+         * Finds a [Music] item [T] in the library by it's [Music.UID].
-         * null will be returned.
+         * @param uid The [Music.UID] to search for.
+         * @return The [T] corresponding to the given [Music.UID], or null if nothing could be
+         * found or the [Music.UID] did not correspond to a [T].
          */
         @Suppress("UNCHECKED_CAST") fun <T : Music> find(uid: Music.UID) = uidMap[uid] as? T
 
-        /** Sanitize an old item to find the corresponding item in a new library. */
+        /**
+         * Convert a [Song] from an another library into a [Song] in this [Library].
+         * @param song The [Song] to convert.
+         * @return The analogous [Song] in this [Library], or null if it does not exist.
+         */
         fun sanitize(song: Song) = find<Song>(song.uid)
 
-        /** Sanitize an old item to find the corresponding item in a new library. */
+        /**
+         * Convert a [Album] from an another library into a [Album] in this [Library].
+         * @param album The [Album] to convert.
+         * @return The analogous [Album] in this [Library], or null if it does not exist.
+         */
         fun sanitize(album: Album) = find<Album>(album.uid)
 
-        /** Sanitize an old item to find the corresponding item in a new library. */
+        /**
+         * Convert a [Artist] from an another library into a [Artist] in this [Library].
+         * @param artist The [Artist] to convert.
+         * @return The analogous [Artist] in this [Library], or null if it does not exist.
+         */
         fun sanitize(artist: Artist) = find<Artist>(artist.uid)
 
-        /** Sanitize an old item to find the corresponding item in a new library. */
+        /**
+         * Convert a [Genre] from an another library into a [Genre] in this [Library].
+         * @param song The [Genre] to convert.
+         * @return The analogous [Genre] in this [Library], or null if it does not exist.
+         */
         fun sanitize(genre: Genre) = find<Genre>(genre.uid)
 
-        /** Find a song for a [uri]. */
+        /**
+         * Find a [Song] instance corresponding to the given Intent.ACTION_VIEW [Uri].
+         * @param context [Context] required to analyze the [Uri].
+         * @param uri [Uri] to search for.
+         * @return A [Song] corresponding to the given [Uri], or null if one could not be found.
+         */
         fun findSongForUri(context: Context, uri: Uri) =
             context.contentResolverSafe.useQuery(
                 uri, arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)) { cursor ->
                 cursor.moveToFirst()
-
                 // We are weirdly limited to DISPLAY_NAME and SIZE when trying to locate a
                 // song. Do what we can to hopefully find the song the user wanted to open.
                 val displayName =
@@ -131,18 +170,26 @@ class MusicStore private constructor() {
             }
     }
 
-    /** A callback for awaiting the loading of music. */
+    /**
+     * A callback for changes in the music library.
+     */
     interface Callback {
+        /**
+         * Called when the current [Library] has changed.
+         * @param library The new [Library], or null if no [Library] has been loaded yet.
+         */
         fun onLibraryChanged(library: Library?)
     }
 
     companion object {
         @Volatile private var INSTANCE: MusicStore? = null
 
-        /** Get the process-level instance of [MusicStore] */
+        /**
+         * Get a singleton instance.
+         * @return The (possibly newly-created) singleton instance.
+         */
         fun getInstance(): MusicStore {
             val currentInstance = INSTANCE
-
             if (currentInstance != null) {
                 return currentInstance
             }

app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt

@@ -24,18 +24,24 @@ import org.oxycblt.auxio.music.system.Indexer
 import org.oxycblt.auxio.util.logD
 
 /**
- * A ViewModel representing the current indexing state.
+ * A [ViewModel] providing data specific to the music loading process.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class MusicViewModel : ViewModel(), Indexer.Callback {
     private val indexer = Indexer.getInstance()
 
     private val _indexerState = MutableStateFlow<Indexer.State?>(null)
-    /** The current music indexing state. */
+    /**
+     * The current music loading state, or null if no loading is going on.
+     * @see Indexer.State
+     */
     val indexerState: StateFlow<Indexer.State?> = _indexerState
 
     private val _statistics = MutableStateFlow<Statistics?>(null)
-    /** The current statistics of the music library. */
+    /**
+     * Statistics about the last completed music load.
+     * @see Statistics
+     */
     val statistics: StateFlow<Statistics?>
         get() = _statistics
 
@@ -43,16 +49,14 @@ class MusicViewModel : ViewModel(), Indexer.Callback {
         indexer.registerCallback(this)
     }
 
-    /** Re-index the music library while using the cache. */
+    override fun onCleared() {
-    fun reindex(ignoreCache: Boolean) {
+        indexer.unregisterCallback(this)
-        indexer.requestReindex(ignoreCache)
     }
 
     override fun onIndexerStateChanged(state: Indexer.State?) {
-        logD("New state: $state")
         _indexerState.value = state
-
         if (state is Indexer.State.Complete && state.response is Indexer.Response.Ok) {
+            // New state is a completed library, update the statistics values.
             val library = state.response.library
             _statistics.value =
                 Statistics(
@@ -64,21 +68,33 @@ class MusicViewModel : ViewModel(), Indexer.Callback {
         }
     }
 
-    override fun onCleared() {
+    /**
-        indexer.unregisterCallback(this)
+     * Requests that the music library should be re-loaded while leveraging the cache.
+     */
+    fun refresh() {
+        indexer.requestReindex(true)
     }
 
-    /** Non-manipulated statistics about the music library. */
+    /**
+     * Requests that the music library should be re-loaded while ignoring the cache.
+     */
+    fun rescan() {
+        indexer.requestReindex(false)
+    }
+
+    /**
+     * Non-manipulated statistics bound the last successful music load.
+     * @param songs The amount of [Song]s that were loaded.
+     * @param albums The amount of [Album]s that were created.
+     * @param artists The amount of [Artist]s that were created.
+     * @param genres The amount of [Genre]s that were created.
+     * @param durationMs The total duration of all songs in the library, in milliseconds.
+     */
     data class Statistics(
-        /** The amount of songs. */
         val songs: Int,
-        /** The amount of albums. */
         val albums: Int,
-        /** The amount of artists. */
         val artists: Int,
-        /** The amount of genres. */
         val genres: Int,
-        /** The total duration of the music library. */
         val durationMs: Long
     )
 }

app/src/main/java/org/oxycblt/auxio/music/Sort.kt

@@ -24,90 +24,166 @@ import org.oxycblt.auxio.R
 import org.oxycblt.auxio.music.Sort.Mode
 
 /**
- * Represents the sort modes used in Auxio.
+ * A sorting method.
  *
- * Sorting can be done by Name, Artist, Album, and others. Sorting of names is always
+ * This can be used not only to sort items, but also represent a sorting mode within the UI.
- * case-insensitive and article-aware. Certain datatypes may only support a subset of sorts since
- * certain sorts cannot be easily applied to them (For Example, [Mode.ByArtist] and [Mode.ByDate] or
- * [Mode.ByAlbum]).
- *
- * Internally, sorts are saved as an integer in the following format
- *
- * 0b(SORT INT)A
- *
- * Where SORT INT is the corresponding integer value of this specific sort and A is a bit
- * representing whether this sort is ascending or descending.
  *
+ * @param mode A [Mode] dictating how to sort the list.
+ * @param isAscending Whether to sort in ascending or descending order.
  * @author Alexander Capehart (OxygenCobalt)
  */
 data class Sort(val mode: Mode, val isAscending: Boolean) {
-    fun withAscending(new: Boolean) = Sort(mode, new)
+    /**
-    fun withMode(new: Mode) = Sort(new, isAscending)
+     * Create a new [Sort] with the same [mode], but different [isAscending] value.
+     * @param isAscending Whether the new sort should be in ascending order or not.
+     * @return A new sort with the same mode, but with the new [isAscending] value applied.
+     */
+    fun withAscending(isAscending: Boolean) = Sort(mode, isAscending)
 
-    val intCode: Int
+    /**
-        get() = mode.intCode.shl(1) or if (isAscending) 1 else 0
+     * Create a new [Sort] with the same [isAscending] value, but different [mode] value.
+     * @param mode Tbe new mode to use for the Sort.
+     * @return A new sort with the same [isAscending] value, but with the new [mode] applied.
+     */
+    fun withMode(mode: Mode) = Sort(mode, isAscending)
 
+    /**
+     * Sort a list of [Song]s.
+     * @param songs The list of [Song]s.
+     * @return A new list of [Song]s sorted by this [Sort]'s configuration.
+     */
     fun songs(songs: Collection<Song>): List<Song> {
         val mutable = songs.toMutableList()
         songsInPlace(mutable)
         return mutable
     }
 
+    /**
+     * Sort a list of [Album]s.
+     * @param albums The list of [Album]s.
+     * @return A new list of [Album]s sorted by this [Sort]'s configuration.
+     */
     fun albums(albums: Collection<Album>): List<Album> {
         val mutable = albums.toMutableList()
         albumsInPlace(mutable)
         return mutable
     }
 
+    /**
+     * Sort a list of [Artist]s.
+     * @param artists The list of [Artist]s.
+     * @return A new list of [Artist]s sorted by this [Sort]'s configuration.
+     */
     fun artists(artists: Collection<Artist>): List<Artist> {
         val mutable = artists.toMutableList()
         artistsInPlace(mutable)
         return mutable
     }
 
+    /**
+     * Sort a list of [Genre]s.
+     * @param genres The list of [Genre]s.
+     * @return A new list of [Genre]s sorted by this [Sort]'s configuration.
+     */
     fun genres(genres: Collection<Genre>): List<Genre> {
         val mutable = genres.toMutableList()
         genresInPlace(mutable)
         return mutable
     }
 
+    /**
+     * Sort a *mutable* list of [Song]s in-place using this [Sort]'s configuration.
+     * @param songs The [Song]s to sort.
+     */
     fun songsInPlace(songs: MutableList<Song>) {
         songs.sortWith(mode.getSongComparator(isAscending))
     }
 
+    /**
+     * Sort a *mutable* list of [Album]s in-place using this [Sort]'s configuration.
+     * @param albums The [Album]s to sort.
+     */
     private fun albumsInPlace(albums: MutableList<Album>) {
         albums.sortWith(mode.getAlbumComparator(isAscending))
     }
 
+    /**
+     * Sort a *mutable* list of [Artist]s in-place using this [Sort]'s configuration.
+     * @param artists The [Album]s to sort.
+     */
     private fun artistsInPlace(artists: MutableList<Artist>) {
         artists.sortWith(mode.getArtistComparator(isAscending))
     }
 
+    /**
+     * Sort a *mutable* list of [Genre]s in-place using this [Sort]'s configuration.
+     * @param genres The [Genre]s to sort.
+     */
     private fun genresInPlace(genres: MutableList<Genre>) {
         genres.sortWith(mode.getGenreComparator(isAscending))
     }
 
+    /**
+     * The integer representation of this instance.
+     * @see fromIntCode
+     */
+    val intCode: Int
+        // Sort's integer representation is formatted as AMMMM, where A is a bitflag
+        // representing on if the mode is ascending or descending, and M is the integer
+        // representation of the sort mode.
+        get() = mode.intCode.shl(1) or if (isAscending) 1 else 0
+
     sealed class Mode {
+        /**
+         * The integer representation of this sort mode.
+         */
         abstract val intCode: Int
+
+        /**
+         * The item ID of this sort mode in menu resources.
+         */
         abstract val itemId: Int
 
-        open fun getSongComparator(ascending: Boolean): Comparator<Song> {
+        /**
+         * Get a [Comparator] that sorts [Song]s according to this [Mode].
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @return A [Comparator] that can be used to sort a [Song] list according to this [Mode].
+         */
+        open fun getSongComparator(isAscending: Boolean): Comparator<Song> {
             throw UnsupportedOperationException()
         }
 
-        open fun getAlbumComparator(ascending: Boolean): Comparator<Album> {
+        /**
+         * Get  a [Comparator] that sorts [Album]s according to this [Mode].
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @return A [Comparator] that can be used to sort a [Album] list according to this [Mode].
+         */
+        open fun getAlbumComparator(isAscending: Boolean): Comparator<Album> {
             throw UnsupportedOperationException()
         }
 
-        open fun getArtistComparator(ascending: Boolean): Comparator<Artist> {
+        /**
+         * Return a [Comparator] that sorts [Artist]s according to this [Mode].
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @return A [Comparator] that can be used to sort a [Artist] list according to this [Mode].
+         */
+        open fun getArtistComparator(isAscending: Boolean): Comparator<Artist> {
             throw UnsupportedOperationException()
         }
 
-        open fun getGenreComparator(ascending: Boolean): Comparator<Genre> {
+        /**
+         * Return a [Comparator] that sorts [Genre]s according to this [Mode].
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @return A [Comparator] that can be used to sort a [Genre] list according to this [Mode].
+         */
+        open fun getGenreComparator(isAscending: Boolean): Comparator<Genre> {
             throw UnsupportedOperationException()
         }
 
-        /** Sort by the names of an item */
+        /**
+         * Sort by the item's name.
+         * @see Music.collationKey
+         */
         object ByName : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_NAME
@@ -115,20 +191,23 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_name
 
-            override fun getSongComparator(ascending: Boolean) =
+            override fun getSongComparator(isAscending: Boolean) =
-                compareByDynamic(ascending, BasicComparator.SONG)
+                compareByDynamic(isAscending, BasicComparator.SONG)
 
-            override fun getAlbumComparator(ascending: Boolean) =
+            override fun getAlbumComparator(isAscending: Boolean) =
-                compareByDynamic(ascending, BasicComparator.ALBUM)
+                compareByDynamic(isAscending, BasicComparator.ALBUM)
 
-            override fun getArtistComparator(ascending: Boolean) =
+            override fun getArtistComparator(isAscending: Boolean) =
-                compareByDynamic(ascending, BasicComparator.ARTIST)
+                compareByDynamic(isAscending, BasicComparator.ARTIST)
 
-            override fun getGenreComparator(ascending: Boolean) =
+            override fun getGenreComparator(isAscending: Boolean) =
-                compareByDynamic(ascending, BasicComparator.GENRE)
+                compareByDynamic(isAscending, BasicComparator.GENRE)
         }
 
-        /** Sort by the album of an item, only supported by [Song] */
+        /**
+         * Sort by the [Album] of an item. Only available for [Song]s.
+         * @see Album.collationKey
+         */
         object ByAlbum : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_ALBUM
@@ -136,15 +215,18 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_album
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending, BasicComparator.ALBUM) { it.album },
+                    compareByDynamic(isAscending, BasicComparator.ALBUM) { it.album },
                     compareBy(NullableComparator.INT) { it.disc },
                     compareBy(NullableComparator.INT) { it.track },
                     compareBy(BasicComparator.SONG))
         }
 
-        /** Sort by the artist of an item, only supported by [Album] and [Song] */
+        /**
+         * Sort by the [Artist] name of an item. Only available for [Song] and [Album].
+         * @see Artist.collationKey
+         */
         object ByArtist : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_ARTIST
@@ -152,23 +234,27 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_artist
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending, ListComparator.ARTISTS) { it.artists },
+                    compareByDynamic(isAscending, ListComparator.ARTISTS) { it.artists },
                     compareByDescending(NullableComparator.DATE) { it.album.date },
                     compareByDescending(BasicComparator.ALBUM) { it.album },
                     compareBy(NullableComparator.INT) { it.disc },
                     compareBy(NullableComparator.INT) { it.track },
                     compareBy(BasicComparator.SONG))
 
-            override fun getAlbumComparator(ascending: Boolean): Comparator<Album> =
+            override fun getAlbumComparator(isAscending: Boolean): Comparator<Album> =
                 MultiComparator(
-                    compareByDynamic(ascending, ListComparator.ARTISTS) { it.artists },
+                    compareByDynamic(isAscending, ListComparator.ARTISTS) { it.artists },
                     compareByDescending(NullableComparator.DATE) { it.date },
                     compareBy(BasicComparator.ALBUM))
         }
 
-        /** Sort by the date of an item, only supported by [Album] and [Song] */
+        /**
+         * Sort by the [Date] of an item. Only available for [Song] and [Album].
+         * @see Song.date
+         * @see Album.date
+         */
         object ByDate : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_YEAR
@@ -176,21 +262,23 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_year
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending, NullableComparator.DATE) { it.album.date },
+                    compareByDynamic(isAscending, NullableComparator.DATE) { it.album.date },
                     compareByDescending(BasicComparator.ALBUM) { it.album },
                     compareBy(NullableComparator.INT) { it.disc },
                     compareBy(NullableComparator.INT) { it.track },
                     compareBy(BasicComparator.SONG))
 
-            override fun getAlbumComparator(ascending: Boolean): Comparator<Album> =
+            override fun getAlbumComparator(isAscending: Boolean): Comparator<Album> =
                 MultiComparator(
-                    compareByDynamic(ascending, NullableComparator.DATE) { it.date },
+                    compareByDynamic(isAscending, NullableComparator.DATE) { it.date },
                     compareBy(BasicComparator.ALBUM))
         }
 
-        /** Sort by the duration of the item. Supports all items. */
+        /**
+         * Sort by the duration of an item.
+         */
         object ByDuration : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_DURATION
@@ -198,25 +286,28 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_duration
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.durationMs }, compareBy(BasicComparator.SONG))
+                    compareByDynamic(isAscending) { it.durationMs }, compareBy(BasicComparator.SONG))
 
-            override fun getAlbumComparator(ascending: Boolean): Comparator<Album> =
+            override fun getAlbumComparator(isAscending: Boolean): Comparator<Album> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.durationMs }, compareBy(BasicComparator.ALBUM))
+                    compareByDynamic(isAscending) { it.durationMs }, compareBy(BasicComparator.ALBUM))
 
-            override fun getArtistComparator(ascending: Boolean): Comparator<Artist> =
+            override fun getArtistComparator(isAscending: Boolean): Comparator<Artist> =
                 MultiComparator(
-                    compareByDynamic(ascending, NullableComparator.LONG) { it.durationMs },
+                    compareByDynamic(isAscending, NullableComparator.LONG) { it.durationMs },
                     compareBy(BasicComparator.ARTIST))
 
-            override fun getGenreComparator(ascending: Boolean): Comparator<Genre> =
+            override fun getGenreComparator(isAscending: Boolean): Comparator<Genre> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.durationMs }, compareBy(BasicComparator.GENRE))
+                    compareByDynamic(isAscending) { it.durationMs }, compareBy(BasicComparator.GENRE))
         }
 
-        /** Sort by the amount of songs. Only applicable to music parents. */
+        /**
+         * Sort by the amount of songs an item contains. Only available for [MusicParent]s.
+         * @see MusicParent.songs
+         */
         object ByCount : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_COUNT
@@ -224,21 +315,24 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_count
 
-            override fun getAlbumComparator(ascending: Boolean): Comparator<Album> =
+            override fun getAlbumComparator(isAscending: Boolean): Comparator<Album> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.songs.size }, compareBy(BasicComparator.ALBUM))
+                    compareByDynamic(isAscending) { it.songs.size }, compareBy(BasicComparator.ALBUM))
 
-            override fun getArtistComparator(ascending: Boolean): Comparator<Artist> =
+            override fun getArtistComparator(isAscending: Boolean): Comparator<Artist> =
                 MultiComparator(
-                    compareByDynamic(ascending, NullableComparator.INT) { it.songs.size },
+                    compareByDynamic(isAscending, NullableComparator.INT) { it.songs.size },
                     compareBy(BasicComparator.ARTIST))
 
-            override fun getGenreComparator(ascending: Boolean): Comparator<Genre> =
+            override fun getGenreComparator(isAscending: Boolean): Comparator<Genre> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.songs.size }, compareBy(BasicComparator.GENRE))
+                    compareByDynamic(isAscending) { it.songs.size }, compareBy(BasicComparator.GENRE))
         }
 
-        /** Sort by the disc, and then track number of an item. Only supported by [Song]. */
+        /**
+         * Sort by the disc number of an item. Only available for [Song]s.
+         * @see Song.disc
+         */
         object ByDisc : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_DISC
@@ -246,16 +340,16 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_disc
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending, NullableComparator.INT) { it.disc },
+                    compareByDynamic(isAscending, NullableComparator.INT) { it.disc },
                     compareBy(NullableComparator.INT) { it.track },
                     compareBy(BasicComparator.SONG))
         }
 
         /**
-         * Sort by the disc, and then track number of an item. Only supported by [Song]. Do not use
+         * Sort by the track number of an item. Only available for [Song]s.
-         * this in a main sorting view, as it is not assigned to a particular item ID
+         * @see Song.track
          */
         object ByTrack : Mode() {
             override val intCode: Int
@@ -264,14 +358,18 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_track
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
                     compareBy(NullableComparator.INT) { it.disc },
-                    compareByDynamic(ascending, NullableComparator.INT) { it.track },
+                    compareByDynamic(isAscending, NullableComparator.INT) { it.track },
                     compareBy(BasicComparator.SONG))
         }
 
-        /** Sort by the time the item was added. Only supported by [Song] */
+        /**
+         * Sort by the date an item was added. Only supported by [Song]s and [Album]s.
+         * @see Song.dateAdded
+         * @see Album.date
+         */
         object ByDateAdded : Mode() {
             override val intCode: Int
                 get() = IntegerTable.SORT_BY_DATE_ADDED
@@ -279,52 +377,81 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             override val itemId: Int
                 get() = R.id.option_sort_date_added
 
-            override fun getSongComparator(ascending: Boolean): Comparator<Song> =
+            override fun getSongComparator(isAscending: Boolean): Comparator<Song> =
                 MultiComparator(
-                    compareByDynamic(ascending) { it.dateAdded }, compareBy(BasicComparator.SONG))
+                    compareByDynamic(isAscending) { it.dateAdded }, compareBy(BasicComparator.SONG))
 
-            override fun getAlbumComparator(ascending: Boolean): Comparator<Album> =
+            override fun getAlbumComparator(isAscending: Boolean): Comparator<Album> =
                 MultiComparator(
-                    compareByDynamic(ascending) { album -> album.songs.minOf { it.dateAdded } },
+                    compareByDynamic(isAscending) { album -> album.dateAdded },
                     compareBy(BasicComparator.ALBUM))
         }
 
-        protected inline fun <T : Music, K> compareByDynamic(
+        /**
-            ascending: Boolean,
+         * Utility function to create a [Comparator] in a dynamic way determined by [isAscending].
-            comparator: Comparator<in K>,
+         * @param isAscending Whether to sort in ascending or descending order.
-            crossinline selector: (T) -> K
+         * @see compareBy
-        ) =
+         * @see compareByDescending
-            if (ascending) {
+         */
-                compareBy(comparator, selector)
-            } else {
-                compareByDescending(comparator, selector)
-            }
-
-        protected fun <T : Music> compareByDynamic(
-            ascending: Boolean,
-            comparator: Comparator<in T>
-        ): Comparator<T> = compareByDynamic(ascending, comparator) { it }
-
         protected inline fun <T : Music, K : Comparable<K>> compareByDynamic(
-            ascending: Boolean,
+            isAscending: Boolean,
             crossinline selector: (T) -> K
         ) =
-            if (ascending) {
+            if (isAscending) {
                 compareBy(selector)
             } else {
                 compareByDescending(selector)
             }
 
+        /**
+         * Utility function to create a [Comparator] in a dynamic way determined by [isAscending]
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @param comparator A [Comparator] to wrap.
+         * @return A new [Comparator] with the specified configuration.
+         * @see compareBy
+         * @see compareByDescending
+         */
+        protected fun <T : Music> compareByDynamic(
+            isAscending: Boolean,
+            comparator: Comparator<in T>
+        ): Comparator<T> = compareByDynamic(isAscending, comparator) { it }
+
+        /**
+         * Utility function to create a [Comparator] a dynamic way determined by [isAscending]
+         * @param isAscending Whether to sort in ascending or descending order.
+         * @param comparator A [Comparator] to wrap.
+         * @param selector Called to obtain a specific attribute to sort by.
+         * @return A new [Comparator] with the specified configuration.
+         * @see compareBy
+         * @see compareByDescending
+         */
+        protected inline fun <T : Music, K> compareByDynamic(
+            isAscending: Boolean,
+            comparator: Comparator<in K>,
+            crossinline selector: (T) -> K
+        ) =
+            if (isAscending) {
+                compareBy(comparator, selector)
+            } else {
+                compareByDescending(comparator, selector)
+            }
+
+        /**
+         * Utility function to create a [Comparator] that sorts in ascending order based on
+         * the given [Comparator], with a selector based on the item itself.
+         * @param comparator The [Comparator] to wrap.
+         * @return A new [Comparator] with the specified configuration.
+         * @see compareBy
+         */
         protected fun <T : Music> compareBy(comparator: Comparator<T>): Comparator<T> =
             compareBy(comparator) { it }
 
         /**
-         * Chains the given comparators together to form one comparator.
+         * A [Comparator] that chains several other [Comparator]s together to form one
-         *
+         * comparison.
-         * Sorts often need to compare multiple things at once across several hierarchies, with this
+         * @param comparators The [Comparator]s to chain. These will be iterated through
-         * class doing such in a more efficient manner than resorting at multiple intervals or
+         * in order during a comparison, with the first non-equal result becoming the
-         * grouping items up. Comparators are checked from first to last, with the first comparator
+         * result.
-         * that returns a non-equal result being propagated upwards.
          */
         private class MultiComparator<T>(vararg comparators: Comparator<T>) : Comparator<T> {
             private val _comparators = comparators
@@ -341,6 +468,10 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             }
         }
 
+        /**
+         * Wraps a [Comparator], extending it to compare two lists.
+         * @param inner The [Comparator] to use.
+         */
         private class ListComparator<T>(private val inner: Comparator<T>) : Comparator<List<T>> {
             override fun compare(a: List<T>, b: List<T>): Int {
                 for (i in 0 until max(a.size, b.size)) {
@@ -363,10 +494,19 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             }
 
             companion object {
+                /**
+                 * A shared instance configured for [Artist]s that can be re-used.
+                 */
                 val ARTISTS: Comparator<List<Artist>> = ListComparator(BasicComparator.ARTIST)
             }
         }
 
+        /**
+         * A [Comparator] that compares abstract [Music] values. Internally, this is similar
+         * to [NullableComparator], however comparing [Music.collationKey] instead of [Comparable].
+         * @see NullableComparator
+         * @see Music.collationKey
+         */
         private class BasicComparator<T : Music> private constructor() : Comparator<T> {
             override fun compare(a: T, b: T): Int {
                 val aKey = a.collationKey
@@ -380,13 +520,29 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
             }
 
             companion object {
+                /**
+                 * A shared instance configured for [Song]s that can be re-used.
+                 */
                 val SONG: Comparator<Song> = BasicComparator()
+                /**
+                 * A shared instance configured for [Album]s that can be re-used.
+                 */
                 val ALBUM: Comparator<Album> = BasicComparator()
+                /**
+                 * A shared instance configured for [Artist]s that can be re-used.
+                 */
                 val ARTIST: Comparator<Artist> = BasicComparator()
+                /**
+                 * A shared instance configured for [Genre]s that can be re-used.
+                 */
                 val GENRE: Comparator<Genre> = BasicComparator()
             }
         }
 
+        /**
+         * A [Comparator] that compares two possibly null values. Values will be considered
+         * lesser if they are null, and greater if they are non-null.
+         */
         private class NullableComparator<T : Comparable<T>> private constructor() : Comparator<T?> {
             override fun compare(a: T?, b: T?) =
                 when {
@@ -397,13 +553,48 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
                 }
 
             companion object {
+                /**
+                 * A shared instance configured for [Int]s that can be re-used.
+                 */
                 val INT = NullableComparator<Int>()
+                /**
+                 * A shared instance configured for [Long]s that can be re-used.
+                 */
                 val LONG = NullableComparator<Long>()
+                /**
+                 * A shared instance configured for [Date]s that can be re-used.
+                 */
                 val DATE = NullableComparator<Date>()
             }
         }
 
         companion object {
+            /**
+             * Convert a [Mode] integer representation into an instance.
+             * @param intCode An integer representation of a [Mode]
+             * @return The corresponding [Mode], or null if the [Mode] is invalid.
+             * @see intCode
+             */
+            fun fromIntCode(intCode: Int) =
+                when (intCode) {
+                    ByName.intCode -> ByName
+                    ByArtist.intCode -> ByArtist
+                    ByAlbum.intCode -> ByAlbum
+                    ByDate.intCode -> ByDate
+                    ByDuration.intCode -> ByDuration
+                    ByCount.intCode -> ByCount
+                    ByDisc.intCode -> ByDisc
+                    ByTrack.intCode -> ByTrack
+                    ByDateAdded.intCode -> ByDateAdded
+                    else -> null
+                }
+
+            /**
+             * Convert a menu item ID into a [Mode].
+             * @param itemId The menu resource ID to convert
+             * @return A [Mode] corresponding to the given ID, or null if the ID is invalid.
+             * @see itemId
+             */
             fun fromItemId(@IdRes itemId: Int) =
                 when (itemId) {
                     ByName.itemId -> ByName
@@ -421,28 +612,18 @@ data class Sort(val mode: Mode, val isAscending: Boolean) {
     }
 
     companion object {
-
         /**
-         * Convert a sort's integer representation into a [Sort] instance.
+         * Convert a [Sort] integer representation into an instance.
-         *
+         * @param intCode An integer representation of a [Sort]
-         * @return A [Sort] instance, null if the data is malformed.
+         * @return The corresponding [Sort], or null if the [Sort] is invalid.
+         * @see intCode
          */
-        fun fromIntCode(value: Int): Sort? {
+        fun fromIntCode(intCode: Int): Sort? {
-            val isAscending = (value and 1) == 1
+            // Sort's integer representation is formatted as AMMMM, where A is a bitflag
-            val mode =
+            // representing on if the mode is ascending or descending, and M is the integer
-                when (value.shr(1)) {
+            // representation of the sort mode.
-                    Mode.ByName.intCode -> Mode.ByName
+            val isAscending = (intCode and 1) == 1
-                    Mode.ByArtist.intCode -> Mode.ByArtist
+            val mode = Mode.fromIntCode(intCode.shr(1)) ?: return null
-                    Mode.ByAlbum.intCode -> Mode.ByAlbum
-                    Mode.ByDate.intCode -> Mode.ByDate
-                    Mode.ByDuration.intCode -> Mode.ByDuration
-                    Mode.ByCount.intCode -> Mode.ByCount
-                    Mode.ByDisc.intCode -> Mode.ByDisc
-                    Mode.ByTrack.intCode -> Mode.ByTrack
-                    Mode.ByDateAdded.intCode -> Mode.ByDateAdded
-                    else -> return null
-                }
-
             return Sort(mode, isAscending)
         }
     }

app/src/main/java/org/oxycblt/auxio/music/Tags.kt

@@ -1,320 +0,0 @@
-/*
- * Copyright (c) 2022 Auxio Project
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- */
- 
-package org.oxycblt.auxio.music
-
-import android.content.Context
-import java.text.SimpleDateFormat
-import kotlin.math.max
-import kotlin.math.min
-import org.oxycblt.auxio.BuildConfig
-import org.oxycblt.auxio.R
-import org.oxycblt.auxio.util.inRangeOrNull
-import org.oxycblt.auxio.util.logE
-import org.oxycblt.auxio.util.nonZeroOrNull
-
-/**
- * An ISO-8601/RFC 3339 Date.
- *
- * Unlike a typical Date within the standard library, this class just represents the ID3v2/Vorbis
- * date format, which is largely assumed to be a subset of ISO-8601. No validation outside of format
- * validation is done, and any date calculation is (fallibly) performed when displayed in the UI.
- *
- * The reasoning behind Date is that Auxio cannot trust any kind of metadata date to actually make
- * sense in a calendar, due to bad tagging, locale-specific issues, or simply from the limited
- * nature of tag formats. Thus, it's better to use an analogous data structure that will not mangle
- * or reject valid-ish dates.
- *
- * Date instances are immutable and their implementation is hidden. To instantiate one, use [from].
- * The string representation of a Date is RFC 3339, with granular position depending on the presence
- * of particular tokens.
- *
- * @author Alexander Capehart (OxygenCobalt)
- */
-class Date private constructor(private val tokens: List<Int>) : Comparable<Date> {
-    init {
-        if (BuildConfig.DEBUG) {
-            // Last-ditch sanity check to catch format bugs that might slip through
-            check(tokens.size in 1..6) { "There must be 1-6 date tokens" }
-            check(tokens.slice(0..min(tokens.lastIndex, 2)).all { it > 0 }) {
-                "All date tokens must be non-zero "
-            }
-            check(tokens.slice(1..tokens.lastIndex).all { it < 100 }) {
-                "All non-year tokens must be two digits"
-            }
-        }
-    }
-
-    private val year = tokens[0]
-
-    private val month = tokens.getOrNull(1)
-
-    private val day = tokens.getOrNull(2)
-
-    private val hour = tokens.getOrNull(3)
-
-    private val minute = tokens.getOrNull(4)
-
-    private val second = tokens.getOrNull(5)
-
-    /**
-     * Resolve this date into a string. This could result in a year string formatted as "YYYY", or a
-     * month and year string formatted as "MMM YYYY" depending on the situation.
-     */
-    fun resolveDate(context: Context) =
-        try {
-            resolveFullDate(context)
-        } catch (e: Exception) {
-            logE("Failed to format a full date")
-            logE(e.stackTraceToString())
-            resolveYear(context)
-        }
-
-    private fun resolveFullDate(context: Context): String {
-        return if (month != null) {
-            // Parse out from an ISO-ish format
-            val format = (SimpleDateFormat.getDateInstance() as SimpleDateFormat)
-            format.applyPattern("yyyy-MM")
-            val date = format.parse("$year-$month") ?: return resolveYear(context)
-
-            // Reformat as a readable month and year
-            format.applyPattern("MMM yyyy")
-            format.format(date)
-        } else {
-            resolveYear(context)
-        }
-    }
-
-    /** Resolve the year field in a way suitable for the UI. */
-    private fun resolveYear(context: Context) = context.getString(R.string.fmt_number, year)
-
-    override fun hashCode() = tokens.hashCode()
-
-    override fun equals(other: Any?) = other is Date && tokens == other.tokens
-
-    override fun compareTo(other: Date): Int {
-        for (i in 0 until max(tokens.size, other.tokens.size)) {
-            val ai = tokens.getOrNull(i)
-            val bi = other.tokens.getOrNull(i)
-            when {
-                ai != null && bi != null -> {
-                    val result = ai.compareTo(bi)
-                    if (result != 0) {
-                        return result
-                    }
-                }
-                ai == null && bi != null -> return -1 // a < b
-                ai == null && bi == null -> return 0 // a = b
-                else -> return 1 // a < b
-            }
-        }
-
-        return 0
-    }
-
-    override fun toString() = StringBuilder().appendDate().toString()
-
-    private fun StringBuilder.appendDate(): StringBuilder {
-        append(year.toFixedString(4))
-        append("-${(month ?: return this).toFixedString(2)}")
-        append("-${(day ?: return this).toFixedString(2)}")
-        append("T${(hour ?: return this).toFixedString(2)}")
-        append(":${(minute ?: return this.append('Z')).toFixedString(2)}")
-        append(":${(second ?: return this.append('Z')).toFixedString(2)}")
-        return this.append('Z')
-    }
-
-    private fun Int.toFixedString(len: Int) = toString().padStart(len, '0').substring(0 until len)
-
-    companion object {
-        private val ISO8601_REGEX =
-            Regex(
-                """^(\d{4,})([-.](\d{2})([-.](\d{2})([T ](\d{2})([:.](\d{2})([:.](\d{2})(Z)?)?)?)?)?)?$""")
-
-        fun from(year: Int) = fromTokens(listOf(year))
-
-        fun from(year: Int, month: Int, day: Int) = fromTokens(listOf(year, month, day))
-
-        fun from(year: Int, month: Int, day: Int, hour: Int, minute: Int) =
-            fromTokens(listOf(year, month, day, hour, minute))
-
-        fun from(timestamp: String): Date? {
-            val groups =
-                (ISO8601_REGEX.matchEntire(timestamp) ?: return null)
-                    .groupValues
-                    .mapIndexedNotNull { index, s -> if (index % 2 != 0) s.toIntOrNull() else null }
-
-            return fromTokens(groups)
-        }
-
-        private fun fromTokens(tokens: List<Int>): Date? {
-            val out = mutableListOf<Int>()
-            validateTokens(tokens, out)
-            if (out.isEmpty()) {
-                return null
-            }
-
-            return Date(out)
-        }
-
-        private fun validateTokens(src: List<Int>, dst: MutableList<Int>) {
-            dst.add(src.getOrNull(0)?.nonZeroOrNull() ?: return)
-            dst.add(src.getOrNull(1)?.inRangeOrNull(1..12) ?: return)
-            dst.add(src.getOrNull(2)?.inRangeOrNull(1..31) ?: return)
-            dst.add(src.getOrNull(3)?.inRangeOrNull(0..23) ?: return)
-            dst.add(src.getOrNull(4)?.inRangeOrNull(0..59) ?: return)
-            dst.add(src.getOrNull(5)?.inRangeOrNull(0..59) ?: return)
-        }
-    }
-}
-
-/**
- * Represents the type of release a particular album is.
- *
- * This can be used to differentiate between album sub-types like Singles, EPs, Compilations, and
- * others. Internally, it operates on a reduced version of the MusicBrainz release type
- * specification. It can be extended if there is demand.
- *
- * @author Alexander Capehart (OxygenCobalt)
- */
-sealed class ReleaseType {
-    abstract val refinement: Refinement?
-    abstract val stringRes: Int
-
-    data class Album(override val refinement: Refinement?) : ReleaseType() {
-        override val stringRes: Int
-            get() =
-                when (refinement) {
-                    null -> R.string.lbl_album
-                    Refinement.LIVE -> R.string.lbl_album_live
-                    Refinement.REMIX -> R.string.lbl_album_remix
-                }
-    }
-
-    data class EP(override val refinement: Refinement?) : ReleaseType() {
-        override val stringRes: Int
-            get() =
-                when (refinement) {
-                    null -> R.string.lbl_ep
-                    Refinement.LIVE -> R.string.lbl_ep_live
-                    Refinement.REMIX -> R.string.lbl_ep_remix
-                }
-    }
-
-    data class Single(override val refinement: Refinement?) : ReleaseType() {
-        override val stringRes: Int
-            get() =
-                when (refinement) {
-                    null -> R.string.lbl_single
-                    Refinement.LIVE -> R.string.lbl_single_live
-                    Refinement.REMIX -> R.string.lbl_single_remix
-                }
-    }
-
-    data class Compilation(override val refinement: Refinement?) : ReleaseType() {
-        override val stringRes: Int
-            get() =
-                when (refinement) {
-                    null -> R.string.lbl_compilation
-                    Refinement.LIVE -> R.string.lbl_compilation_live
-                    Refinement.REMIX -> R.string.lbl_compilation_remix
-                }
-    }
-
-    object Soundtrack : ReleaseType() {
-        override val refinement: Refinement?
-            get() = null
-
-        override val stringRes: Int
-            get() = R.string.lbl_soundtrack
-    }
-
-    object Mix : ReleaseType() {
-        override val refinement: Refinement?
-            get() = null
-
-        override val stringRes: Int
-            get() = R.string.lbl_mix
-    }
-
-    object Mixtape : ReleaseType() {
-        override val refinement: Refinement?
-            get() = null
-
-        override val stringRes: Int
-            get() = R.string.lbl_mixtape
-    }
-
-    /**
-     * Roughly analogous to the MusicBrainz "live" and "remix" secondary types. Unlike the main
-     * types, these only modify an existing, primary type.
-     */
-    enum class Refinement {
-        LIVE,
-        REMIX
-    }
-
-    companion object {
-        // Note: The parsing code is extremely clever in order to reduce duplication. It's
-        // better just to read the specification behind release types than to follow this code.
-
-        fun parse(types: List<String>): ReleaseType? {
-            val primary = types.getOrNull(0) ?: return null
-
-            // Primary types should be the first one in sequence. The spec makes no mention of
-            // whether primary types are a pre-requisite for secondary types, so we assume that
-            // it isn't. There are technically two other types, but those are unrelated to music
-            // and thus we don't support them.
-            return when {
-                primary.equals("album", true) -> types.parseSecondaryTypes(1) { Album(it) }
-                primary.equals("ep", true) -> types.parseSecondaryTypes(1) { EP(it) }
-                primary.equals("single", true) -> types.parseSecondaryTypes(1) { Single(it) }
-                else -> types.parseSecondaryTypes(0) { Album(it) }
-            }
-        }
-
-        private inline fun List<String>.parseSecondaryTypes(
-            secondaryIdx: Int,
-            convertRefinement: (Refinement?) -> ReleaseType
-        ): ReleaseType {
-            val secondary = getOrNull(secondaryIdx)
-
-            return if (secondary.equals("compilation", true)) {
-                // Secondary type is a compilation, actually parse the third type
-                // and put that into a compilation if needed.
-                parseSecondaryTypeImpl(getOrNull(secondaryIdx + 1)) { Compilation(it) }
-            } else {
-                // Secondary type is a plain value, use the original values given.
-                parseSecondaryTypeImpl(secondary, convertRefinement)
-            }
-        }
-
-        private inline fun parseSecondaryTypeImpl(
-            type: String?,
-            convertRefinement: (Refinement?) -> ReleaseType
-        ) =
-            when {
-                // Parse all the types that have no children
-                type.equals("soundtrack", true) -> Soundtrack
-                type.equals("mixtape/street", true) -> Mixtape
-                type.equals("dj-mix", true) -> Mix
-                type.equals("live", true) -> convertRefinement(Refinement.LIVE)
-                type.equals("remix", true) -> convertRefinement(Refinement.REMIX)
-                else -> convertRefinement(null)
-            }
-    }
-}

app/src/main/java/org/oxycblt/auxio/music/extractor/CacheExtractor.kt

@@ -32,22 +32,75 @@ import org.oxycblt.auxio.util.queryAll
 import org.oxycblt.auxio.util.requireBackgroundThread
 
 /**
- * The extractor that caches music metadata for faster use later. The cache is only responsible for
+ * Defines an Extractor that can load cached music. This is the first step in the music extraction
- * storing "intrinsic" data, as in information derived from the file format and not information from
+ * process and is an optimization to avoid the slow [MediaStoreExtractor] and [MetadataExtractor]
- * the media database or file system. The exceptions are the database ID and modification times for
+ * extraction process.
- * files, as these are required for the cache to function well.
  * @author Alexander Capehart (OxygenCobalt)
  */
-class CacheExtractor(private val context: Context, private val noop: Boolean) {
+interface CacheExtractor {
-    private var cacheMap: Map<Long, Song.Raw>? = null
+    /**
-    private var shouldWriteCache = noop
+     * Initialize the Extractor by reading the cache data into memory.
+     */
+    fun init()
 
-    fun init() {
+    /**
-        if (noop) {
+     * Finalize the Extractor by writing the newly-loaded [Song.Raw] back into the cache,
-            return
+     * alongside freeing up memory.
+     * @param rawSongs The songs to write into the cache.
+     */
+    fun finalize(rawSongs: List<Song.Raw>)
+
+    /**
+     * Use the cache to populate the given [Song.Raw].
+     * @param rawSong The [Song.Raw] to attempt to populate. Note that this [Song.Raw] will
+     * only contain the bare minimum information required to load a cache entry.
+     * @return An [ExtractionResult] representing the result of the operation.
+     * [ExtractionResult.PARSED] is not returned.
+     */
+    fun populate(rawSong: Song.Raw): ExtractionResult
+}
+
+/**
+ * A [CacheExtractor] only capable of writing to the cache. This can be used to load music
+ * with without the cache if the user desires.
+ * @param context [Context] required to read the cache database.
+ * @see CacheExtractor
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+open class WriteOnlyCacheExtractor(private val context: Context) : CacheExtractor {
+    override fun init() {
+        // Nothing to do.
     }
 
+    override fun finalize(rawSongs: List<Song.Raw>) {
         try {
+            // Still write out whatever data was extracted.
+            CacheDatabase.getInstance(context).write(rawSongs)
+        } catch (e: Exception) {
+            logE("Unable to save cache database.")
+            logE(e.stackTraceToString())
+        }
+    }
+
+    override fun populate(rawSong: Song.Raw) =
+        // Nothing to do.
+        ExtractionResult.NONE
+}
+
+/**
+ * A [CacheExtractor] that supports reading from and writing to the cache.
+ * @param context [Context] required to load
+ * @see CacheExtractor
+ * @author Alexander Capehart
+ */
+class ReadWriteCacheExtractor(private val context: Context) : WriteOnlyCacheExtractor(context) {
+    private var cacheMap: Map<Long, Song.Raw>? = null
+    private var invalidate = false
+
+    override fun init() {
+        try {
+            // Faster to load the whole database into memory than do a query on each
+            // populate call.
             cacheMap = CacheDatabase.getInstance(context).read()
         } catch (e: Exception) {
             logE("Unable to load cache database.")
@@ -55,34 +108,32 @@ class CacheExtractor(private val context: Context, private val noop: Boolean) {
         }
     }
 
-    /** Write a list of newly-indexed raw songs to the database. */
+    override fun finalize(rawSongs: List<Song.Raw>) {
-    fun finalize(rawSongs: List<Song.Raw>) {
         cacheMap = null
-
+        // Same some time by not re-writing the cache if we were able to create the entire
-        if (shouldWriteCache) {
+        // library from it. If there is even just one song we could not populate from the
-            // If the entire library could not be loaded from the cache, we need to re-write it
+        // cache, then we will re-write it.
-            // with the new library.
+        if (invalidate) {
             logD("Cache was invalidated during loading, rewriting")
-            try {
+            super.finalize(rawSongs)
-                CacheDatabase.getInstance(context).write(rawSongs)
-            } catch (e: Exception) {
-                logE("Unable to save cache database.")
-                logE(e.stackTraceToString())
-            }
         }
     }
 
-    /**
+    override fun populate(rawSong: Song.Raw): ExtractionResult {
-     * Maybe copy a cached raw song into this instance, assuming that it has not changed since it
+        val map = requireNotNull(cacheMap) {
-     * was last saved. Returns true if a song was loaded.
+            "Must initialize this extractor before populating a raw song."
-     */
+        }
-    fun populateFromCache(rawSong: Song.Raw): Boolean {
-        val map = cacheMap ?: return false
 
+        // For a cached raw song to be used, it must exist within the cache and have matching
+        // addition and modification timestamps. Technically the addition timestamp doesn't
+        // exist, but to safeguard against possible OEM-specific timestamp incoherence, we
+        // check for it anyway.
         val cachedRawSong = map[rawSong.mediaStoreId]
         if (cachedRawSong != null &&
             cachedRawSong.dateAdded == rawSong.dateAdded &&
             cachedRawSong.dateModified == rawSong.dateModified) {
+            // No built-in "copy from" method for data classes, just have to assign
+            // the data ourselves.
             rawSong.musicBrainzId = cachedRawSong.musicBrainzId
             rawSong.name = cachedRawSong.name
             rawSong.sortName = cachedRawSong.sortName
@@ -98,7 +149,7 @@ class CacheExtractor(private val context: Context, private val noop: Boolean) {
             rawSong.albumMusicBrainzId = cachedRawSong.albumMusicBrainzId
             rawSong.albumName = cachedRawSong.albumName
             rawSong.albumSortName = cachedRawSong.albumSortName
-            rawSong.albumReleaseTypes = cachedRawSong.albumReleaseTypes
+            rawSong.albumTypes = cachedRawSong.albumTypes
 
             rawSong.artistMusicBrainzIds = cachedRawSong.artistMusicBrainzIds
             rawSong.artistNames = cachedRawSong.artistNames
@@ -110,17 +161,27 @@ class CacheExtractor(private val context: Context, private val noop: Boolean) {
 
             rawSong.genreNames = cachedRawSong.genreNames
 
-            return true
+            return ExtractionResult.CACHED
         }
 
-        shouldWriteCache = true
+        // We could not populate this song. This means our cache is stale and should be
-        return false
+        // re-written with newly-loaded music.
+        invalidate = true
+        return ExtractionResult.NONE
     }
 }
 
+/**
+ * Internal [Song.Raw] cache database.
+ * @author Alexander Capehart (OxygenCobalt)
+ * @see [CacheExtractor]
+ */
 private class CacheDatabase(context: Context) :
     SQLiteOpenHelper(context, File(context.cacheDir, DB_NAME).absolutePath, null, DB_VERSION) {
     override fun onCreate(db: SQLiteDatabase) {
+        // Map the cacheable raw song fields to database fields. Cache-able in this context
+        // means information independent of the file-system, excluding IDs and timestamps required
+        // to retrieve items from the cache.
         val command =
             StringBuilder()
                 .append("CREATE TABLE IF NOT EXISTS $TABLE_RAW_SONGS(")
@@ -139,7 +200,7 @@ private class CacheDatabase(context: Context) :
                 .append("${Columns.ALBUM_MUSIC_BRAINZ_ID} STRING,")
                 .append("${Columns.ALBUM_NAME} STRING NOT NULL,")
                 .append("${Columns.ALBUM_SORT_NAME} STRING,")
-                .append("${Columns.ALBUM_RELEASE_TYPES} STRING,")
+                .append("${Columns.ALBUM_TYPES} STRING,")
                 .append("${Columns.ARTIST_MUSIC_BRAINZ_IDS} STRING,")
                 .append("${Columns.ARTIST_NAMES} STRING,")
                 .append("${Columns.ARTIST_SORT_NAMES} STRING,")
@@ -156,6 +217,7 @@ private class CacheDatabase(context: Context) :
     override fun onDowngrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) = nuke(db)
 
     private fun nuke(db: SQLiteDatabase) {
+        // No cost to nuking this database, only causes higher loading times.
         logD("Nuking database")
         db.apply {
             execSQL("DROP TABLE IF EXISTS $TABLE_RAW_SONGS")
@@ -163,13 +225,21 @@ private class CacheDatabase(context: Context) :
         }
     }
 
+    /**
+     * Read out this database into memory.
+     * @return A mapping between the MediaStore IDs of the cache entries and a [Song.Raw] containing
+     * the cacheable data for the entry. Note that any filesystem-dependent information
+     * (excluding IDs and timestamps) is not cached.
+     */
     fun read(): Map<Long, Song.Raw> {
         requireBackgroundThread()
-
+        val start = System.currentTimeMillis()
         val map = mutableMapOf<Long, Song.Raw>()
-
         readableDatabase.queryAll(TABLE_RAW_SONGS) { cursor ->
-            if (cursor.count == 0) return@queryAll
+            if (cursor.count == 0) {
+                // Nothing to do.
+                return@queryAll
+            }
 
             val idIndex = cursor.getColumnIndexOrThrow(Columns.MEDIA_STORE_ID)
             val dateAddedIndex = cursor.getColumnIndexOrThrow(Columns.DATE_ADDED)
@@ -191,7 +261,7 @@ private class CacheDatabase(context: Context) :
                 cursor.getColumnIndexOrThrow(Columns.ALBUM_MUSIC_BRAINZ_ID)
             val albumNameIndex = cursor.getColumnIndexOrThrow(Columns.ALBUM_NAME)
             val albumSortNameIndex = cursor.getColumnIndexOrThrow(Columns.ALBUM_SORT_NAME)
-            val albumReleaseTypesIndex = cursor.getColumnIndexOrThrow(Columns.ALBUM_RELEASE_TYPES)
+            val albumReleaseTypesIndex = cursor.getColumnIndexOrThrow(Columns.ALBUM_TYPES)
 
             val artistMusicBrainzIdsIndex =
                 cursor.getColumnIndexOrThrow(Columns.ARTIST_MUSIC_BRAINZ_IDS)
@@ -229,40 +299,48 @@ private class CacheDatabase(context: Context) :
                 raw.albumMusicBrainzId = cursor.getStringOrNull(albumMusicBrainzIdIndex)
                 raw.albumName = cursor.getString(albumNameIndex)
                 raw.albumSortName = cursor.getStringOrNull(albumSortNameIndex)
-                cursor.getStringOrNull(albumReleaseTypesIndex)?.parseMultiValue()?.let {
+                cursor.getStringOrNull(albumReleaseTypesIndex)?.parseSQLMultiValue()?.let {
-                    raw.albumReleaseTypes = it
+                    raw.albumTypes = it
                 }
 
                 cursor.getStringOrNull(artistMusicBrainzIdsIndex)?.let {
-                    raw.artistMusicBrainzIds = it.parseMultiValue()
+                    raw.artistMusicBrainzIds = it.parseSQLMultiValue()
                 }
                 cursor.getStringOrNull(artistNamesIndex)?.let {
-                    raw.artistNames = it.parseMultiValue()
+                    raw.artistNames = it.parseSQLMultiValue()
                 }
                 cursor.getStringOrNull(artistSortNamesIndex)?.let {
-                    raw.artistSortNames = it.parseMultiValue()
+                    raw.artistSortNames = it.parseSQLMultiValue()
                 }
 
                 cursor.getStringOrNull(albumArtistMusicBrainzIdsIndex)?.let {
-                    raw.albumArtistMusicBrainzIds = it.parseMultiValue()
+                    raw.albumArtistMusicBrainzIds = it.parseSQLMultiValue()
                 }
                 cursor.getStringOrNull(albumArtistNamesIndex)?.let {
-                    raw.albumArtistNames = it.parseMultiValue()
+                    raw.albumArtistNames = it.parseSQLMultiValue()
                 }
                 cursor.getStringOrNull(albumArtistSortNamesIndex)?.let {
-                    raw.albumArtistSortNames = it.parseMultiValue()
+                    raw.albumArtistSortNames = it.parseSQLMultiValue()
                 }
 
-                cursor.getStringOrNull(genresIndex)?.let { raw.genreNames = it.parseMultiValue() }
+                cursor.getStringOrNull(genresIndex)?.let { raw.genreNames = it.parseSQLMultiValue() }
 
                 map[id] = raw
             }
         }
 
+        logD("Read cache in ${System.currentTimeMillis() - start}ms")
+
         return map
     }
 
+    /**
+     * Write a new list of [Song.Raw] to this database.
+     * @param rawSongs The new [Song.Raw] instances to cache. Note that any filesystem-dependent
+     * information (excluding IDs and timestamps) is not cached.
+     */
     fun write(rawSongs: List<Song.Raw>) {
+        val start = System.currentTimeMillis()
         var position = 0
         val database = writableDatabase
         database.transaction { delete(TABLE_RAW_SONGS, null, null) }
@@ -299,24 +377,24 @@ private class CacheDatabase(context: Context) :
                             put(Columns.ALBUM_NAME, rawSong.albumName)
                             put(Columns.ALBUM_SORT_NAME, rawSong.albumSortName)
                             put(
-                                Columns.ALBUM_RELEASE_TYPES,
+                                Columns.ALBUM_TYPES,
-                                rawSong.albumReleaseTypes.toMultiValue())
+                                rawSong.albumTypes.toSQLMultiValue())
 
                             put(
                                 Columns.ARTIST_MUSIC_BRAINZ_IDS,
-                                rawSong.artistMusicBrainzIds.toMultiValue())
+                                rawSong.artistMusicBrainzIds.toSQLMultiValue())
-                            put(Columns.ARTIST_NAMES, rawSong.artistNames.toMultiValue())
+                            put(Columns.ARTIST_NAMES, rawSong.artistNames.toSQLMultiValue())
-                            put(Columns.ARTIST_SORT_NAMES, rawSong.artistSortNames.toMultiValue())
+                            put(Columns.ARTIST_SORT_NAMES, rawSong.artistSortNames.toSQLMultiValue())
 
                             put(
                                 Columns.ALBUM_ARTIST_MUSIC_BRAINZ_IDS,
-                                rawSong.albumArtistMusicBrainzIds.toMultiValue())
+                                rawSong.albumArtistMusicBrainzIds.toSQLMultiValue())
-                            put(Columns.ALBUM_ARTIST_NAMES, rawSong.albumArtistNames.toMultiValue())
+                            put(Columns.ALBUM_ARTIST_NAMES, rawSong.albumArtistNames.toSQLMultiValue())
                             put(
                                 Columns.ALBUM_ARTIST_SORT_NAMES,
-                                rawSong.albumArtistSortNames.toMultiValue())
+                                rawSong.albumArtistSortNames.toSQLMultiValue())
 
-                            put(Columns.GENRE_NAMES, rawSong.genreNames.toMultiValue())
+                            put(Columns.GENRE_NAMES, rawSong.genreNames.toSQLMultiValue())
                         }
 
                     insert(TABLE_RAW_SONGS, null, itemData)
@@ -329,62 +407,160 @@ private class CacheDatabase(context: Context) :
 
             logD("Wrote batch of raw songs. Position is now at $position")
         }
+
+        logD("Wrote cache in ${System.currentTimeMillis() - start}ms")
     }
 
     // SQLite does not natively support multiple values, so we have to serialize multi-value
     // tags with separators. Not ideal, but nothing we can do.
 
-    private fun List<String>.toMultiValue() =
+    /**
+     * Transforms the multi-string list into a SQL-safe multi-string value.
+     * @return A single string containing all values within the multi-string list, delimited
+     * by a ";". Pre-existing ";" characters will be escaped.
+     */
+    private fun List<String>.toSQLMultiValue() =
         if (isNotEmpty()) {
             joinToString(";") { it.replace(";", "\\;") }
         } else {
             null
         }
 
-    private fun String.parseMultiValue() = splitEscaped { it == ';' }
+    /**
+     * Transforms the SQL-safe multi-string value into a multi-string list.
+     * @return A list of strings corresponding to the delimited values present within the
+     * original string. Escaped delimiters are converted back into their normal forms.
+     */
+    private fun String.parseSQLMultiValue() = splitEscaped { it == ';' }
 
+    /**
+     * Defines the columns used in this database.
+     */
     private object Columns {
+        /**
+         * @see Song.Raw.mediaStoreId
+         */
         const val MEDIA_STORE_ID = "msid"
+        /**
+         * @see Song.Raw.dateAdded
+         */
         const val DATE_ADDED = "date_added"
+        /**
+         * @see Song.Raw.dateModified
+         */
         const val DATE_MODIFIED = "date_modified"
 
+        /**
+         * @see Song.Raw.size
+         */
         const val SIZE = "size"
+        /**
+         * @see Song.Raw.durationMs
+         */
         const val DURATION = "duration"
+        /**
+         * @see Song.Raw.formatMimeType
+         */
         const val FORMAT_MIME_TYPE = "fmt_mime"
 
+        /**
+         * @see Song.Raw.musicBrainzId
+         */
         const val MUSIC_BRAINZ_ID = "mbid"
+        /**
+         * @see Song.Raw.name
+         */
         const val NAME = "name"
+        /**
+         * @see Song.Raw.sortName
+         */
         const val SORT_NAME = "sort_name"
 
+        /**
+         * @see Song.Raw.track
+         */
         const val TRACK = "track"
+        /**
+         * @see Song.Raw.disc
+         */
         const val DISC = "disc"
+        /**
+         * @see [Song.Raw.date
+         */
         const val DATE = "date"
 
+        /**
+         * @see [Song.Raw.albumMusicBrainzId
+         */
         const val ALBUM_MUSIC_BRAINZ_ID = "album_mbid"
+        /**
+         * @see Song.Raw.albumName
+         */
         const val ALBUM_NAME = "album"
+        /**
+         * @see Song.Raw.albumSortName
+         */
         const val ALBUM_SORT_NAME = "album_sort"
-        const val ALBUM_RELEASE_TYPES = "album_types"
+        /**
+         * @see Song.Raw.albumReleaseTypes
+         */
+        const val ALBUM_TYPES = "album_types"
 
+        /**
+         * @see Song.Raw.artistMusicBrainzIds
+         */
         const val ARTIST_MUSIC_BRAINZ_IDS = "artists_mbid"
+        /**
+         * @see Song.Raw.artistNames
+         */
         const val ARTIST_NAMES = "artists"
+        /**
+         * @see Song.Raw.artistSortNames
+         */
         const val ARTIST_SORT_NAMES = "artists_sort"
 
+        /**
+         * @see Song.Raw.albumArtistMusicBrainzIds
+         */
         const val ALBUM_ARTIST_MUSIC_BRAINZ_IDS = "album_artists_mbid"
+        /**
+         * @see Song.Raw.albumArtistNames
+         */
         const val ALBUM_ARTIST_NAMES = "album_artists"
+        /**
+         * @see Song.Raw.albumArtistSortNames
+         */
         const val ALBUM_ARTIST_SORT_NAMES = "album_artists_sort"
 
+        /**
+         * @see Song.Raw.genreNames
+         */
         const val GENRE_NAMES = "genres"
     }
 
     companion object {
+        /**
+         * The file name of the database.
+         */
         const val DB_NAME = "auxio_music_cache.db"
+
+        /**
+         * The current version of the database. Increment whenever a breaking change is made
+         * to the schema. When incremented, the database will be wiped.
+         */
         const val DB_VERSION = 1
 
+        /**
+         * The table containing the cached [Song.Raw] instances.
+         */
         const val TABLE_RAW_SONGS = "raw_songs"
 
         @Volatile private var INSTANCE: CacheDatabase? = null
 
-        /** Get/Instantiate the single instance of [CacheDatabase]. */
+        /**
+         * Get a singleton instance.
+         * @return The (possibly newly-created) singleton instance.
+         */
         fun getInstance(context: Context): CacheDatabase {
             val currentInstance = INSTANCE
 

app/src/main/java/org/oxycblt/auxio/music/extractor/ExtractionResult.kt

@@ -0,0 +1,22 @@
+package org.oxycblt.auxio.music.extractor
+
+/**
+ * Represents the result of an extraction operation.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+enum class ExtractionResult {
+    /**
+     * A raw song was successfully extracted from the cache.
+     */
+    CACHED,
+
+    /**
+     * A raw song was successfully extracted from parsing it's file.
+     */
+    PARSED,
+
+    /**
+     * A raw song could not be parsed.
+     */
+    NONE
+}

app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreExtractor.kt

@@ -31,7 +31,7 @@ import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.storage.Directory
 import org.oxycblt.auxio.music.storage.directoryCompat
 import org.oxycblt.auxio.music.storage.mediaStoreVolumeNameCompat
-import org.oxycblt.auxio.music.storage.queryCursor
+import org.oxycblt.auxio.music.storage.safeQuery
 import org.oxycblt.auxio.music.storage.storageVolumesCompat
 import org.oxycblt.auxio.music.storage.useQuery
 import org.oxycblt.auxio.settings.Settings
@@ -40,16 +40,19 @@ import org.oxycblt.auxio.util.getSystemServiceCompat
 import org.oxycblt.auxio.util.logD
 
 /**
- * The layer that loads music from the MediaStore database. This is an intermediate step in the
+ * The layer that loads music from the [MediaStore] database. This is an intermediate step in the
- * music loading process.
+ * music extraction process and primarily intended for redundancy for files not natively
+ * supported by [MetadataExtractor]. Solely relying on this is not recommended, as it often
+ * produces bad metadata.
+ * @param context [Context] required to query the media database.
+ * @param cacheExtractor [CacheExtractor] implementation for cache optimizations.
  * @author Alexander Capehart (OxygenCobalt)
  */
 abstract class MediaStoreExtractor(
     private val context: Context,
-    private val cacheDatabase: CacheExtractor
+    private val cacheExtractor: CacheExtractor
 ) {
     private var cursor: Cursor? = null
-
     private var idIndex = -1
     private var titleIndex = -1
     private var displayNameIndex = -1
@@ -63,52 +66,59 @@ abstract class MediaStoreExtractor(
     private var albumIdIndex = -1
     private var artistIndex = -1
     private var albumArtistIndex = -1
-
-    private val settings = Settings(context)
     private val genreNamesMap = mutableMapOf<Long, String>()
 
-    private val _volumes = mutableListOf<StorageVolume>()
+    /**
-    protected val volumes: List<StorageVolume>
+     * The [StorageVolume]s currently scanned by [MediaStore]. This should be used to transform
-        get() = _volumes
+     * path information from the database into volume-aware paths.
+     */
+    protected var volumes = listOf<StorageVolume>()
+        private set
 
-    /** Initialize this instance by making a query over the media database. */
+    /**
+     * Initialize this instance. This involves setting up the required sub-extractors and
+     * querying the media database for music files.
+     * @return A [Cursor] of the music data returned from the database.
+     */
     open fun init(): Cursor {
-        logD("Initializing")
+        // Initialize sub-extractors for later use.
+        cacheExtractor.init()
+
         val start = System.currentTimeMillis()
-
+        val settings = Settings(context)
-        cacheDatabase.init()
-
         val storageManager = context.getSystemServiceCompat(StorageManager::class)
-        _volumes.addAll(storageManager.storageVolumesCompat)
+        // Set up the volume list for concrete implementations to use.
-        val dirs = settings.getMusicDirs(storageManager)
+        volumes = storageManager.storageVolumesCompat
 
         val args = mutableListOf<String>()
         var selector = BASE_SELECTOR
 
+        // Filter out music that is not music, if enabled.
         if (settings.excludeNonMusic) {
             selector += " AND ${MediaStore.Audio.AudioColumns.IS_MUSIC}=1"
         }
 
+        // Set up the projection to follow the music directory configuration.
+        val dirs = settings.getMusicDirs(storageManager)
         if (dirs.dirs.isNotEmpty()) {
-            // Need to select for directories. The path query is the same, only difference is
+            selector += " AND "
-            // the presence of a NOT.
+            if (!dirs.shouldInclude) {
-            selector +=
+                // Without a NOT, the query will be restricted to the specified paths, resulting
-                if (dirs.shouldInclude) {
+                // in the "Include" mode. With a NOT, the specified paths will not be included,
-                    logD("Need to select dirs (Include)")
+                // resulting in the "Exclude" mode.
-                    " AND ("
+                selector += "NOT "
-                } else {
-                    logD("Need to select dirs (Exclude)")
-                    " AND NOT ("
             }
+            selector += " ("
 
-            // Each impl adds the directories that they want selected.
+            // Specifying the paths to filter is version-specific, delegate to the concrete
+            // implementations.
             for (i in dirs.dirs.indices) {
-                if (addDirToSelectorArgs(dirs.dirs[i], args)) {
+                if (addDirToSelector(dirs.dirs[i], args)) {
                     selector +=
                         if (i < dirs.dirs.lastIndex) {
-                            "$dirSelector OR "
+                            "$dirSelectorTemplate OR "
                         } else {
-                            dirSelector
+                            dirSelectorTemplate
                         }
                 }
             }
@@ -116,17 +126,16 @@ abstract class MediaStoreExtractor(
             selector += ')'
         }
 
+        // Now we can actually query MediaStore.
         logD("Starting song query [proj: ${projection.toList()}, selector: $selector, args: $args]")
-
+        val cursor = context.contentResolverSafe.safeQuery(
-        val cursor =
-            requireNotNull(
-                    context.contentResolverSafe.queryCursor(
                         MediaStore.Audio.Media.EXTERNAL_CONTENT_URI,
                         projection,
                         selector,
-                        args.toTypedArray())) { "Content resolver failure: No Cursor returned" }
+                        args.toTypedArray()).also { cursor = it }
-                .also { cursor = it }
+        logD("Song query succeeded [Projected total: ${cursor.count}]")
 
+        // Set up cursor indices for later use.
         idIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns._ID)
         titleIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.TITLE)
         displayNameIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.DISPLAY_NAME)
@@ -142,15 +151,13 @@ abstract class MediaStoreExtractor(
         artistIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.ARTIST)
         albumArtistIndex = cursor.getColumnIndexOrThrow(AUDIO_COLUMN_ALBUM_ARTIST)
 
-        logD("Song query succeeded [Projected total: ${cursor.count}]")
-
         logD("Assembling genre map")
 
-        // Since we can't obtain the genre tag from a song query, we must construct
+        // Since we can't obtain the genre tag from a song query, we must construct our own
-        // our own equivalent from genre database queries. Theoretically, this isn't
+        // equivalent from genre database queries. Theoretically, this isn't needed since
-        // needed since MetadataLayer will fill this in for us, but I'd imagine there
+        // MetadataLayer will fill this in for us, but I'd imagine there are some obscure
-        // are some obscure formats where genre support is only really covered by this,
+        // formats where genre support is only really covered by this, so we are forced to
-        // so we are forced to bite the O(n^2) complexity here.
+        // bite the O(n^2) complexity here.
         context.contentResolverSafe.useQuery(
             MediaStore.Audio.Genres.EXTERNAL_CONTENT_URI,
             arrayOf(MediaStore.Audio.Genres._ID, MediaStore.Audio.Genres.NAME)) { genreCursor ->
@@ -169,7 +176,7 @@ abstract class MediaStoreExtractor(
 
                     while (cursor.moveToNext()) {
                         // Assume that a song can't inhabit multiple genre entries, as I doubt
-                        // Android is smart enough to separate genres.
+                        // MediaStore is actually aware that songs can have multiple genres.
                         genreNamesMap[cursor.getLong(songIdIndex)] = name
                     }
                 }
@@ -183,42 +190,43 @@ abstract class MediaStoreExtractor(
 
     /** Finalize this instance by closing the cursor and finalizing the cache. */
     fun finalize(rawSongs: List<Song.Raw>) {
+        // Free the cursor (and it's resources)
         cursor?.close()
         cursor = null
 
-        cacheDatabase.finalize(rawSongs)
+        // Finalize sub-extractors
+        cacheExtractor.finalize(rawSongs)
     }
 
     /**
-     * Populate a [raw] with whatever the next value in the cursor is.
+     * Populate a [Song.Raw] with the next [Cursor] value provided by [MediaStore].
-     *
+     * @param raw The [Song.Raw] to populate.
-     * This returns true if the song could be restored from cache, false if metadata had to be
+     * @return An [ExtractionResult] signifying the result of the operation. Will return
-     * re-extracted, and null if the cursor is exhausted.
+     * [ExtractionResult.CACHED] if [CacheExtractor] returned it.
      */
-    fun populateRawSong(raw: Song.Raw): Boolean? {
+    fun populate(raw: Song.Raw): ExtractionResult {
         val cursor = requireNotNull(cursor) { "MediaStoreLayer is not properly initialized" }
+        // Move to the next cursor, stopping if we have exhausted it.
         if (!cursor.moveToNext()) {
             logD("Cursor is exhausted")
-            return null
+            return ExtractionResult.NONE
         }
 
-        // Populate the minimum required fields to maybe obtain a cache entry.
+        // Populate the minimum required columns to maybe obtain a cache entry.
         populateFileData(cursor, raw)
-
+        if (cacheExtractor.populate(raw) == ExtractionResult.CACHED) {
-        if (cacheDatabase.populateFromCache(raw)) {
+            // We found a valid cache entry, no need to fully read the entry.
-            // We found a valid cache entry, no need to extract metadata.
+            return ExtractionResult.CACHED
-            return true
         }
 
+        // Could not load entry from cache, we have to read the rest of the metadata.
         populateMetadata(cursor, raw)
-
+        return ExtractionResult.PARSED
-        // We had to freshly make this raw, return false
-        return false
     }
 
     /**
-     * The projection to use when querying media. Add version-specific columns here in an
+     * The database columns available to all android versions supported by Auxio.
-     * implementation.
+     * Concrete implementations can extend this projection to add version-specific columns.
      */
     protected open val projection: Array<String>
         get() =
@@ -238,78 +246,101 @@ abstract class MediaStoreExtractor(
                 MediaStore.Audio.AudioColumns.ARTIST,
                 AUDIO_COLUMN_ALBUM_ARTIST)
 
-    protected abstract val dirSelector: String
+    /**
-    protected abstract fun addDirToSelectorArgs(dir: Directory, args: MutableList<String>): Boolean
+     * The companion template to add to the projection's selector whenever arguments are added
+     * by [addDirToSelector].
+     * @see addDirToSelector
+     */
+    protected abstract val dirSelectorTemplate: String
 
     /**
-     * Populate the "file data" of the cursor, or data that is required to access a cache entry or
+     * Add a [Directory] to the given list of projection selector arguments.
-     * makes no sense to cache. This includes database IDs, modification dates,
+     * @param dir The [Directory] to add.
+     * @param args The destination list to append selector arguments to that are analogous
+     * to the given [Directory].
+     * @return true if the [Directory] was added, false otherwise.
+     * @see dirSelectorTemplate
+     */
+    protected abstract fun addDirToSelector(dir: Directory, args: MutableList<String>): Boolean
+
+    /**
+     * Populate a [Song.Raw] with the "File Data" of the given [MediaStore] [Cursor], which is the
+     * data that cannot be cached. This includes any information not intrinsic to the file and
+     * instead dependent on the file-system, which could change without invalidating the cache
+     * due to volume additions or removals.
+     * @param cursor The [Cursor] to read from.
+     * @param raw The [Song.Raw] to populate.
+     * @see populateMetadata
      */
     protected open fun populateFileData(cursor: Cursor, raw: Song.Raw) {
         raw.mediaStoreId = cursor.getLong(idIndex)
         raw.dateAdded = cursor.getLong(dateAddedIndex)
         raw.dateModified = cursor.getLong(dateAddedIndex)
-
+        // Try to use the DISPLAY_NAME column to obtain a (probably sane) file name
-        // Try to use the DISPLAY_NAME field to obtain a (probably sane) file name
         // from the android system.
         raw.fileName = cursor.getStringOrNull(displayNameIndex)
         raw.extensionMimeType = cursor.getString(mimeTypeIndex)
-
         raw.albumMediaStoreId = cursor.getLong(albumIdIndex)
     }
 
-    /** Extract cursor metadata into [raw]. */
+    /**
+     * Populate a [Song.Raw] with the Metadata of the given [MediaStore] [Cursor], which is the
+     * data about a [Song.Raw] that can be cached. This includes any information intrinsic to
+     * the file or it's file format, such as music tags.
+     * @param cursor The [Cursor] to read from.
+     * @param raw The [Song.Raw] to populate.
+     * @see populateFileData
+     */
     protected open fun populateMetadata(cursor: Cursor, raw: Song.Raw) {
+        // Song title
         raw.name = cursor.getString(titleIndex)
-
+        // Size (in bytes)
         raw.size = cursor.getLong(sizeIndex)
+        // Duration (in milliseconds)
         raw.durationMs = cursor.getLong(durationIndex)
-
+        // MediaStore only exposes the year value of a file. This is actually worse than it
+        // seems, as it means that it will not read ID3v2 TDRC tags or Vorbis DATE comments.
+        // This is one of the major weaknesses of using MediaStore, hence the redundancy layers.
         raw.date = cursor.getIntOrNull(yearIndex)?.toDate()
-
         // 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. We can't do anything to
         // fix this, really.
         raw.albumName = cursor.getString(albumIndex)
-
         // Android does not make a non-existent artist tag null, it instead fills it in
-        // as <unknown>, which makes absolutely no sense given how other fields default
+        // as <unknown>, which makes absolutely no sense given how other columns default
-        // to null if they are not present. If this field is <unknown>, null it so that
+        // to null if they are not present. If this column is such, null it so that
         // it's easier to handle later.
         val artist = cursor.getString(artistIndex)
         if (artist != MediaStore.UNKNOWN_STRING) {
             raw.artistNames = listOf(artist)
         }
-
+        // The album artist column is nullable and never has placeholder values.
-        // The album artist field is nullable and never has placeholder values.
         cursor.getStringOrNull(albumArtistIndex)?.let { raw.albumArtistNames = listOf(it) }
-
         // Get the genre value we had to query for in initialization
         genreNamesMap[raw.mediaStoreId]?.let { raw.genreNames = listOf(it) }
     }
 
     companion object {
         /**
-         * The album_artist MediaStore field has existed since at least API 21, but until API 30 it
+         * The base selector that works across all versions of android. Does not exclude
-         * was a proprietary extension for Google Play Music and was not documented. Since this
+         * directories.
-         * field probably works on all versions Auxio supports, we suppress the warning about using
+         */
-         * a possibly-unsupported constant.
+        private const val BASE_SELECTOR = "NOT ${MediaStore.Audio.Media.SIZE}=0"
+
+        /**
+         * The album artist of a song. This column has existed since at least API 21, but until API
+         * 30 it was an undocumented extension for Google Play Music. This column will work on all
+         * versions that Auxio supports.
          */
         @Suppress("InlinedApi")
         private const val AUDIO_COLUMN_ALBUM_ARTIST = MediaStore.Audio.AudioColumns.ALBUM_ARTIST
 
         /**
-         * External has existed since at least API 21, but no constant existed for it until API 29.
+         * The external volume. This naming has existed since API 21, but no constant existed
-         * This constant is safe to use.
+         * for it until API 29. This will work on all versions that Auxio supports.
          */
         @Suppress("InlinedApi") private const val VOLUME_EXTERNAL = MediaStore.VOLUME_EXTERNAL
-
-        /**
-         * The base selector that works across all versions of android. Does not exclude
-         * directories.
-         */
-        private const val BASE_SELECTOR = "NOT ${MediaStore.Audio.Media.SIZE}=0"
     }
 }
 
@@ -318,16 +349,19 @@ abstract class MediaStoreExtractor(
 
 /**
  * A [MediaStoreExtractor] that completes the music loading process in a way compatible from API 21
- * onwards to API 29.
+ * onwards to API 28.
+ * @param context [Context] required to query the media database.
+ * @param cacheExtractor [CacheExtractor] implementation for cache optimizations.
  * @author Alexander Capehart (OxygenCobalt)
  */
-class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor) :
+class Api21MediaStoreExtractor(context: Context, cacheExtractor: CacheExtractor) :
-    MediaStoreExtractor(context, cacheDatabase) {
+    MediaStoreExtractor(context, cacheExtractor) {
     private var trackIndex = -1
     private var dataIndex = -1
 
     override fun init(): Cursor {
         val cursor = super.init()
+        // Set up cursor indices for later use.
         trackIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.TRACK)
         dataIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.DATA)
         return cursor
@@ -336,13 +370,20 @@ class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor)
     override val projection: Array<String>
         get() =
             super.projection +
-                arrayOf(MediaStore.Audio.AudioColumns.TRACK, MediaStore.Audio.AudioColumns.DATA)
+                arrayOf(MediaStore.Audio.AudioColumns.TRACK,
+                    // Below API 29, we are restricted to the absolute path (Called DATA by
+                    // MedaStore) when working with audio files.
+                    MediaStore.Audio.AudioColumns.DATA)
 
-    override val dirSelector: String
+    // The selector should be configured to convert the given directories instances to their
+    // absolute paths and then compare them to DATA.
+
+    override val dirSelectorTemplate: String
         get() = "${MediaStore.Audio.Media.DATA} LIKE ?"
 
-    override fun addDirToSelectorArgs(dir: Directory, args: MutableList<String>): Boolean {
+    override fun addDirToSelector(dir: Directory, args: MutableList<String>): Boolean {
-        // Generate an equivalent DATA value from the volume directory and the relative path.
+        // "%" signifies to accept any DATA value that begins with the Directory's path,
+        // thus recursively filtering all files in the directory.
         args.add("${dir.volume.directoryCompat ?: return false}/${dir.relativePath}%")
         return true
     }
@@ -350,19 +391,18 @@ class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor)
     override fun populateFileData(cursor: Cursor, raw: Song.Raw) {
         super.populateFileData(cursor, raw)
 
-        // DATA is equivalent to the absolute path of the file.
         val data = cursor.getString(dataIndex)
 
         // On some OEM devices below API 29, DISPLAY_NAME may not be present. I assume
-        // that this only applies to below API 29, as beyond API 29, this field not being
+        // that this only applies to below API 29, as beyond API 29, this column not being
         // present would completely break the scoped storage system. Fill it in with DATA
         // if it's not available.
         if (raw.fileName == null) {
             raw.fileName = data.substringAfterLast(File.separatorChar, "").ifEmpty { null }
         }
 
-        // Find the volume that transforms the DATA field into a relative path. This is
+        // Find the volume that transforms the DATA column into a relative path. This is
-        // the volume and relative path we will use.
+        // the Directory we will use.
         val rawPath = data.substringBeforeLast(File.separatorChar)
         for (volume in volumes) {
             val volumePath = volume.directoryCompat ?: continue
@@ -376,7 +416,8 @@ class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor)
 
     override fun populateMetadata(cursor: Cursor, raw: Song.Raw) {
         super.populateMetadata(cursor, raw)
-
+        // See unpackTrackNo/unpackDiscNo for an explanation
+        // of how this column is set up.
         val rawTrack = cursor.getIntOrNull(trackIndex)
         if (rawTrack != null) {
             rawTrack.unpackTrackNo()?.let { raw.track = it }
@@ -386,23 +427,23 @@ class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor)
 }
 
 /**
- * A [MediaStoreExtractor] that selects directories and builds paths using the modern volume fields
+ * A [MediaStoreExtractor] that implements common behavior supported from API 29 onwards.
- * available from API 29 onwards.
+ * @param context [Context] required to query the media database.
+ * @param cacheExtractor [CacheExtractor] implementation for cache optimizations.
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.Q)
-open class BaseApi29MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor) :
+open class BaseApi29MediaStoreExtractor(context: Context, cacheExtractor: CacheExtractor) :
-    MediaStoreExtractor(context, cacheDatabase) {
+    MediaStoreExtractor(context, cacheExtractor) {
     private var volumeIndex = -1
     private var relativePathIndex = -1
 
     override fun init(): Cursor {
         val cursor = super.init()
-
+        // Set up cursor indices for later use.
         volumeIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.VOLUME_NAME)
         relativePathIndex =
             cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.RELATIVE_PATH)
-
         return cursor
     }
 
@@ -410,29 +451,36 @@ open class BaseApi29MediaStoreExtractor(context: Context, cacheDatabase: CacheEx
         get() =
             super.projection +
                 arrayOf(
+                    // After API 29, we now have access to the volume name and relative
+                    // path, which simplifies working with Paths significantly.
                     MediaStore.Audio.AudioColumns.VOLUME_NAME,
                     MediaStore.Audio.AudioColumns.RELATIVE_PATH)
 
-    override val dirSelector: String
+    // The selector should be configured to compare both the volume name and relative path
+    // of the given directories, albeit with some conversion to the analogous MediaStore
+    // column values.
+
+    override val dirSelectorTemplate: String
         get() =
             "(${MediaStore.Audio.AudioColumns.VOLUME_NAME} LIKE ? " +
                 "AND ${MediaStore.Audio.AudioColumns.RELATIVE_PATH} LIKE ?)"
 
-    override fun addDirToSelectorArgs(dir: Directory, args: MutableList<String>): Boolean {
+    override fun addDirToSelector(dir: Directory, args: MutableList<String>): Boolean {
-        // Leverage new the volume field when selecting our directories.
+        // MediaStore uses a different naming scheme for it's volume column convert this
+        // directory's volume to it.
         args.add(dir.volume.mediaStoreVolumeNameCompat ?: return false)
+        // "%" signifies to accept any DATA value that begins with the Directory's path,
+        // thus recursively filtering all files in the directory.
         args.add("${dir.relativePath}%")
         return true
     }
 
     override fun populateFileData(cursor: Cursor, raw: Song.Raw) {
         super.populateFileData(cursor, raw)
-
+        // Find the StorageVolume whose MediaStore name corresponds to this song.
+        // This is combined with the plain relative path column to create the directory.
         val volumeName = cursor.getString(volumeIndex)
         val relativePath = cursor.getString(relativePathIndex)
-
-        // Find the StorageVolume whose MediaStore name corresponds to this song.
-        // This is what we use for the Directory's volume.
         val volume = volumes.find { it.mediaStoreVolumeNameCompat == volumeName }
         if (volume != null) {
             raw.directory = Directory.from(volume, relativePath)
@@ -442,12 +490,14 @@ open class BaseApi29MediaStoreExtractor(context: Context, cacheDatabase: CacheEx
 
 /**
  * A [MediaStoreExtractor] that completes the music loading process in a way compatible with at
- * least API 29.
+ * API 29.
+ * @param context [Context] required to query the media database.
+ * @param cacheExtractor [CacheExtractor] implementation for cache functionality.
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.Q)
-open class Api29MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor) :
+open class Api29MediaStoreExtractor(context: Context, cacheExtractor: CacheExtractor) :
-    BaseApi29MediaStoreExtractor(context, cacheDatabase) {
+    BaseApi29MediaStoreExtractor(context, cacheExtractor) {
     private var trackIndex = -1
 
     override fun init(): Cursor {
@@ -461,9 +511,9 @@ open class Api29MediaStoreExtractor(context: Context, cacheDatabase: CacheExtrac
 
     override fun populateMetadata(cursor: Cursor, raw: Song.Raw) {
         super.populateMetadata(cursor, raw)
-
+        // This backend is volume-aware, but does not support the modern track columns.
-        // This backend is volume-aware, but does not support the modern track fields.
+        // Use the old column instead. See unpackTrackNo/unpackDiscNo for an explanation
-        // Use the old field instead.
+        // of how this column is set up.
         val rawTrack = cursor.getIntOrNull(trackIndex)
         if (rawTrack != null) {
             rawTrack.unpackTrackNo()?.let { raw.track = it }
@@ -473,13 +523,15 @@ open class Api29MediaStoreExtractor(context: Context, cacheDatabase: CacheExtrac
 }
 
 /**
- * A [MediaStoreExtractor] that completes the music loading process in a way compatible with at
+ * A [MediaStoreExtractor] that completes the music loading process in a way compatible from
- * least API 30.
+ * API 30 onwards.
+ * @param context [Context] required to query the media database.
+ * @param cacheExtractor [CacheExtractor] implementation for cache optimizations.
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.R)
-class Api30MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor) :
+class Api30MediaStoreExtractor(context: Context, cacheExtractor: CacheExtractor) :
-    BaseApi29MediaStoreExtractor(context, cacheDatabase) {
+    BaseApi29MediaStoreExtractor(context, cacheExtractor) {
     private var trackIndex: Int = -1
     private var discIndex: Int = -1
 
@@ -494,15 +546,16 @@ class Api30MediaStoreExtractor(context: Context, cacheDatabase: CacheExtractor)
         get() =
             super.projection +
                 arrayOf(
+                    // API 30 grant us access to the superior CD_TRACK_NUMBER and DISC_NUMBER
+                    // fields, which take the place of TRACK.
                     MediaStore.Audio.AudioColumns.CD_TRACK_NUMBER,
                     MediaStore.Audio.AudioColumns.DISC_NUMBER)
 
     override fun populateMetadata(cursor: Cursor, raw: Song.Raw) {
         super.populateMetadata(cursor, raw)
-
         // Both CD_TRACK_NUMBER and DISC_NUMBER tend to be formatted as they are in
         // the tag itself, which is to say that it is formatted as NN/TT tracks, where
-        // N is the number and T is the total. Parse the number while leaving out the
+        // N is the number and T is the total. Parse the number while ignoring the
         // total, as we have no use for it.
         cursor.getStringOrNull(trackIndex)?.parsePositionNum()?.let { raw.track = it }
         cursor.getStringOrNull(discIndex)?.parsePositionNum()?.let { raw.disc = it }

app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt

@@ -26,50 +26,62 @@ import com.google.android.exoplayer2.metadata.id3.TextInformationFrame
 import com.google.android.exoplayer2.metadata.vorbis.VorbisComment
 import org.oxycblt.auxio.music.Date
 import org.oxycblt.auxio.music.Song
-import org.oxycblt.auxio.music.storage.audioUri
+import org.oxycblt.auxio.music.storage.toAudioUri
 import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.logW
 
 /**
- * The layer that leverages ExoPlayer's metadata retrieval system to index metadata.
+ * The extractor that leverages ExoPlayer's [MetadataRetriever] API to parse metadata. This is the
- *
+ * last step in the music extraction process and is mostly responsible for papering over the
- * Normally, ExoPlayer's metadata system is quite slow. However, if we parallelize it, we can get
+ * bad metadata that [MediaStoreExtractor] produces.
- * similar throughput to other metadata extractors, which is nice as it means we don't have to
- * bundle a redundant metadata library like JAudioTagger.
- *
- * Now, ExoPlayer's metadata API is not the best. It's opaque, undocumented, and prone to weird
- * pitfalls given ExoPlayer's cozy relationship with native code. However, this backend should do
- * enough to eliminate such issues.
- *
- * TODO: Fix failing ID3v2 multi-value tests in fork (Implies parsing problem)
  *
+ * @param context [Context] required for reading audio files.
+ * @param mediaStoreExtractor [MediaStoreExtractor] implementation for cache optimizations and
+ * redundancy.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class MetadataExtractor(
     private val context: Context,
     private val mediaStoreExtractor: MediaStoreExtractor
 ) {
+    // We can parallelize MetadataRetriever Futures to work around it's speed issues,
+    // producing similar throughput's to other kinds of manual metadata extraction.
     private val taskPool: Array<Task?> = arrayOfNulls(TASK_CAPACITY)
 
-    /** Initialize the sub-layers that this layer relies on. */
+    /**
+     * Initialize this extractor. This actually initializes the sub-extractors that this instance
+     * relies on.
+     * @return The amount of music that is expected to be loaded.
+     */
     fun init() = mediaStoreExtractor.init().count
 
-    /** Finalize the sub-layers that this layer relies on. */
+    /**
+     * Finalize this extractor with the newly parsed [Song.Raw]. This actually finalizes the
+     * sub-extractors that this instance relies on.
+     */
     fun finalize(rawSongs: List<Song.Raw>) = mediaStoreExtractor.finalize(rawSongs)
 
+    /**
+     * Parse all [Song.Raw] instances queued by the sub-extractors. This will first delegate
+     * to the sub-extractors before parsing the metadata itself.
+     * @param emit A callback that will be invoked with every new [Song.Raw] instance when
+     * they are successfully loaded.
+     */
     suspend fun parse(emit: suspend (Song.Raw) -> Unit) {
         while (true) {
             val raw = Song.Raw()
-            if (mediaStoreExtractor.populateRawSong(raw) ?: break) {
+            when (mediaStoreExtractor.populate(raw)) {
-                // No need to extract metadata that was successfully restored from the cache
+                ExtractionResult.NONE -> break
+                ExtractionResult.PARSED -> {}
+                ExtractionResult.CACHED -> {
+                    // Avoid running the expensive parsing process on songs we can already
+                    // restore from the cache.
                     emit(raw)
                     continue
                 }
+            }
 
-            // Spin until there is an open slot we can insert a task in. Note that we do
+            // Spin until there is an open slot we can insert a task in.
-            // not add callbacks to our new tasks, as Future callbacks run on a different
-            // executor and thus will crash the app if an error occurs instead of bubbling
-            // back up to Indexer.
             spin@ while (true) {
                 for (i in taskPool.indices) {
                     val task = taskPool[i]
@@ -106,24 +118,32 @@ class MetadataExtractor(
     }
 
     companion object {
-        /** The amount of tasks this backend can run efficiently at once. */
+        /**
+         * The amount of [Task]s this instance can return
+         */
         private const val TASK_CAPACITY = 8
     }
 }
 
 /**
- * Wraps an ExoPlayer metadata retrieval task in a safe abstraction. Access is done with [get].
+ * Wraps a [MetadataExtractor] future and processes it into a [Song.Raw] when completed.
+ * TODO: Re-unify with MetadataExtractor.
+ * @param context [Context] required to open the audio file.
+ * @param raw [Song.Raw] to process.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class Task(context: Context, private val raw: Song.Raw) {
+    // Note that we do not leverage future callbacks. This is because errors in the
+    // (highly fallible) extraction process will not bubble up to Indexer when a
+    // callback is used, instead crashing the app entirely.
     private val future =
         MetadataRetriever.retrieveMetadata(
             context,
-            MediaItem.fromUri(requireNotNull(raw.mediaStoreId) { "Invalid raw: No id" }.audioUri))
+            MediaItem.fromUri(requireNotNull(raw.mediaStoreId) { "Invalid raw: No id" }.toAudioUri()))
 
     /**
-     * Get the song that this task is trying to complete. If the task is still busy, this will
+     * Try to get a completed song from this [Task], if it has finished processing.
-     * return null. Otherwise, it will return a song.
+     * @return A [Song.Raw] instance if processing has completed, null otherwise.
      */
     fun get(): Song.Raw? {
         if (!future.isDone) {
@@ -145,11 +165,12 @@ class Task(context: Context, private val raw: Song.Raw) {
         }
 
         // Populate the format mime type if we have one.
+        // TODO: Check if this is even useful or not.
         format.sampleMimeType?.let { raw.formatMimeType = it }
 
         val metadata = format.metadata
         if (metadata != null) {
-            completeRawSong(metadata)
+            populateWithMetadata(metadata)
         } else {
             logD("No metadata could be extracted for ${raw.name}")
         }
@@ -157,7 +178,11 @@ class Task(context: Context, private val raw: Song.Raw) {
         return raw
     }
 
-    private fun completeRawSong(metadata: Metadata) {
+    /**
+     * Complete this instance's [Song.Raw] with the newly extracted [Metadata].
+     * @param metadata The [Metadata] to complete the [Song.Raw] with.
+     */
+    private fun populateWithMetadata(metadata: Metadata) {
         val id3v2Tags = mutableMapOf<String, List<String>>()
         val vorbisTags = mutableMapOf<String, MutableList<String>>()
 
@@ -167,6 +192,8 @@ class Task(context: Context, private val raw: Song.Raw) {
         for (i in 0 until metadata.length()) {
             when (val tag = metadata[i]) {
                 is TextInformationFrame -> {
+                    // Map TXXX frames differently so we can specifically index by their
+                    // descriptions.
                     val id = tag.description?.let { "TXXX:${it.sanitize()}" } ?: tag.id.sanitize()
                     val values = tag.values.map { it.sanitize() }
                     if (values.isNotEmpty() && values.all { it.isNotEmpty() }) {
@@ -185,28 +212,33 @@ class Task(context: Context, private val raw: Song.Raw) {
         }
 
         when {
-            vorbisTags.isEmpty() -> populateId3v2(id3v2Tags)
+            vorbisTags.isEmpty() -> populateWithId3v2(id3v2Tags)
-            id3v2Tags.isEmpty() -> populateVorbis(vorbisTags)
+            id3v2Tags.isEmpty() -> populateWithVorbis(vorbisTags)
             else -> {
                 // Some formats (like FLAC) can contain both ID3v2 and Vorbis, so we apply
                 // them both with priority given to vorbis.
-                populateId3v2(id3v2Tags)
+                populateWithId3v2(id3v2Tags)
-                populateVorbis(vorbisTags)
+                populateWithVorbis(vorbisTags)
             }
         }
     }
 
-    private fun populateId3v2(tags: Map<String, List<String>>) {
+    /**
+     * Complete this instance's [Song.Raw] with ID3v2 Text Identification Frames.
+     * @param textFrames A mapping between ID3v2 Text Identification Frame IDs and one or more
+     * values.
+     */
+    private fun populateWithId3v2(textFrames: Map<String, List<String>>) {
         // Song
-        tags["TXXX:MusicBrainz Release Track Id"]?.let { raw.musicBrainzId = it[0] }
+        textFrames["TXXX:MusicBrainz Release Track Id"]?.let { raw.musicBrainzId = it[0] }
-        tags["TIT2"]?.let { raw.name = it[0] }
+        textFrames["TIT2"]?.let { raw.name = it[0] }
-        tags["TSOT"]?.let { raw.sortName = it[0] }
+        textFrames["TSOT"]?.let { raw.sortName = it[0] }
 
-        // Track, as NN/TT
+        // Track. Only parse out the track number and ignore the total tracks value.
-        tags["TRCK"]?.run { get(0).parsePositionNum() }?.let { raw.track = it }
+        textFrames["TRCK"]?.run { get(0).parsePositionNum() }?.let { raw.track = it }
 
-        // Disc, as NN/TT
+        // Disc. Only parse out the disc number and ignore the total discs value.
-        tags["TPOS"]?.run { get(0).parsePositionNum() }?.let { raw.disc = it }
+        textFrames["TPOS"]?.run { get(0).parsePositionNum() }?.let { raw.disc = it }
 
         // Dates are somewhat complicated, as not only did their semantics change from a flat year
         // value in ID3v2.3 to a full ISO-8601 date in ID3v2.4, but there are also a variety of
@@ -217,103 +249,127 @@ class Task(context: Context, private val raw: Song.Raw) {
         // 3. ID3v2.4 Release Date, as it is the second most common date type
         // 4. ID3v2.3 Original Date, as it is like #1
         // 5. ID3v2.3 Release Year, as it is the most common date type
-        (tags["TDOR"]?.run { get(0).parseTimestamp() }
+        (textFrames["TDOR"]?.run { get(0).parseTimestamp() }
-                ?: tags["TDRC"]?.run { get(0).parseTimestamp() }
+                ?: textFrames["TDRC"]?.run { get(0).parseTimestamp() }
-                    ?: tags["TDRL"]?.run { get(0).parseTimestamp() } ?: parseId3v23Date(tags))
+                    ?: textFrames["TDRL"]?.run { get(0).parseTimestamp() } ?: parseId3v23Date(textFrames))
             ?.let { raw.date = it }
 
         // Album
-        tags["TXXX:MusicBrainz Album Id"]?.let { raw.albumMusicBrainzId = it[0] }
+        textFrames["TXXX:MusicBrainz Album Id"]?.let { raw.albumMusicBrainzId = it[0] }
-        tags["TALB"]?.let { raw.albumName = it[0] }
+        textFrames["TALB"]?.let { raw.albumName = it[0] }
-        tags["TSOA"]?.let { raw.albumSortName = it[0] }
+        textFrames["TSOA"]?.let { raw.albumSortName = it[0] }
-        (tags["TXXX:MusicBrainz Album Type"] ?: tags["GRP1"])?.let { raw.albumReleaseTypes = it }
+        (textFrames["TXXX:MusicBrainz Album Type"] ?: textFrames["GRP1"])?.let { raw.albumTypes = it }
 
         // Artist
-        tags["TXXX:MusicBrainz Artist Id"]?.let { raw.artistMusicBrainzIds = it }
+        textFrames["TXXX:MusicBrainz Artist Id"]?.let { raw.artistMusicBrainzIds = it }
-        tags["TPE1"]?.let { raw.artistNames = it }
+        textFrames["TPE1"]?.let { raw.artistNames = it }
-        tags["TSOP"]?.let { raw.artistSortNames = it }
+        textFrames["TSOP"]?.let { raw.artistSortNames = it }
 
         // Album artist
-        tags["TXXX:MusicBrainz Album Artist Id"]?.let { raw.albumArtistMusicBrainzIds = it }
+        textFrames["TXXX:MusicBrainz Album Artist Id"]?.let { raw.albumArtistMusicBrainzIds = it }
-        tags["TPE2"]?.let { raw.albumArtistNames = it }
+        textFrames["TPE2"]?.let { raw.albumArtistNames = it }
-        tags["TSO2"]?.let { raw.albumArtistSortNames = it }
+        textFrames["TSO2"]?.let { raw.albumArtistSortNames = it }
 
         // Genre
-        tags["TCON"]?.let { raw.genreNames = it }
+        textFrames["TCON"]?.let { raw.genreNames = it }
     }
 
-    private fun parseId3v23Date(tags: Map<String, List<String>>): Date? {
+    /**
-        val year =
+     * Parses the ID3v2.3 timestamp specification into a [Date] from the given Text Identification
-            tags["TORY"]?.run { get(0).toIntOrNull() }
+     * Frames.
-                ?: tags["TYER"]?.run { get(0).toIntOrNull() } ?: return null
+     * @param textFrames A mapping between ID3v2 Text Identification Frame IDs and one or more
-
+     * values.
+     * @retrn A [Date]  of a year value from TORY/TYER, a month and day value from TDAT,
+     * and a hour/minute value from TIME. No second value is included. The latter two fields may
+     * not be included in they cannot be parsed. Will be null if a year value could not be parsed.
+     */
+    private fun parseId3v23Date(textFrames: Map<String, List<String>>): Date? {
         // Assume that TDAT/TIME can refer to TYER or TORY depending on if TORY
         // is present.
+        val year =
+            textFrames["TORY"]?.run { get(0).toIntOrNull() }
+                ?: textFrames["TYER"]?.run { get(0).toIntOrNull() } ?: return null
 
-        val tdat = tags["TDAT"]
+        val tdat = textFrames["TDAT"]
         return if (tdat != null && tdat[0].length == 4 && tdat[0].isDigitsOnly()) {
+            // TDAT frames consist of a 4-digit string where the first two digits are
+            // the month and the last two digits are the day.
             val mm = tdat[0].substring(0..1).toInt()
             val dd = tdat[0].substring(2..3).toInt()
 
-            val time = tags["TIME"]
+            val time = textFrames["TIME"]
             if (time != null && time[0].length == 4 && time[0].isDigitsOnly()) {
+                // TIME frames consist of a 4-digit string where the first two digits are
+                // the hour and the last two digits are the minutes. No second value is
+                // possible.
                 val hh = time[0].substring(0..1).toInt()
                 val mi = time[0].substring(2..3).toInt()
+                // Able to return a full date.
                 Date.from(year, mm, dd, hh, mi)
             } else {
+                // Unable to parse time, just return a date
                 Date.from(year, mm, dd)
             }
         } else {
+            // Unable to parse month/day, just return a year
             return Date.from(year)
         }
     }
 
-    private fun populateVorbis(tags: Map<String, List<String>>) {
+    /**
+     * Complete this instance's [Song.Raw] with Vorbis comments.
+     * @param comments A mapping between vorbis comment names and one or more vorbis comment
+     * values.
+     */
+    private fun populateWithVorbis(comments: Map<String, List<String>>) {
         // Song
-        tags["MUSICBRAINZ_RELEASETRACKID"]?.let { raw.musicBrainzId = it[0] }
+        comments["MUSICBRAINZ_RELEASETRACKID"]?.let { raw.musicBrainzId = it[0] }
-        tags["TITLE"]?.let { raw.name = it[0] }
+        comments["TITLE"]?.let { raw.name = it[0] }
-        tags["TITLESORT"]?.let { raw.sortName = it[0] }
+        comments["TITLESORT"]?.let { raw.sortName = it[0] }
 
-        // Track
+        // Track. The total tracks value is in a different comment, so we can just
-        tags["TRACKNUMBER"]?.run { get(0).parsePositionNum() }?.let { raw.track = it }
+        // convert the entirety of this comment into a number.
+        comments["TRACKNUMBER"]?.run { get(0).toIntOrNull() }?.let { raw.track = it }
 
-        // Disc
+        // Disc. The total discs value is in a different comment, so we can just
-        tags["DISCNUMBER"]?.run { get(0).parsePositionNum() }?.let { raw.disc = it }
+        // convert the entirety of this comment into a number.
+        comments["DISCNUMBER"]?.run { get(0).toIntOrNull() }?.let { raw.disc = it }
 
         // Vorbis dates are less complicated, but there are still several types
         // Our hierarchy for dates is as such:
         // 1. Original Date, as it solves the "Released in X, Remastered in Y" issue
         // 2. Date, as it is the most common date type
         // 3. Year, as old vorbis tags tended to use this (I know this because it's the only
-        // tag that android supports, so it must be 15 years old or more!)
+        // date tag that android supports, so it must be 15 years old or more!)
-        (tags["ORIGINALDATE"]?.run { get(0).parseTimestamp() }
+        (comments["ORIGINALDATE"]?.run { get(0).parseTimestamp() }
-                ?: tags["DATE"]?.run { get(0).parseTimestamp() }
+                ?: comments["DATE"]?.run { get(0).parseTimestamp() }
-                    ?: tags["YEAR"]?.run { get(0).parseYear() })
+                    ?: comments["YEAR"]?.run { get(0).parseYear() })
             ?.let { raw.date = it }
 
         // Album
-        tags["MUSICBRAINZ_ALBUMID"]?.let { raw.albumMusicBrainzId = it[0] }
+        comments["MUSICBRAINZ_ALBUMID"]?.let { raw.albumMusicBrainzId = it[0] }
-        tags["ALBUM"]?.let { raw.albumName = it[0] }
+        comments["ALBUM"]?.let { raw.albumName = it[0] }
-        tags["ALBUMSORT"]?.let { raw.albumSortName = it[0] }
+        comments["ALBUMSORT"]?.let { raw.albumSortName = it[0] }
-        tags["RELEASETYPE"]?.let { raw.albumReleaseTypes = it }
+        comments["RELEASETYPE"]?.let { raw.albumTypes = it }
 
         // Artist
-        tags["MUSICBRAINZ_ARTISTID"]?.let { raw.artistMusicBrainzIds = it }
+        comments["MUSICBRAINZ_ARTISTID"]?.let { raw.artistMusicBrainzIds = it }
-        tags["ARTIST"]?.let { raw.artistNames = it }
+        comments["ARTIST"]?.let { raw.artistNames = it }
-        tags["ARTISTSORT"]?.let { raw.artistSortNames = it }
+        comments["ARTISTSORT"]?.let { raw.artistSortNames = it }
 
         // Album artist
-        tags["MUSICBRAINZ_ALBUMARTISTID"]?.let { raw.albumArtistMusicBrainzIds = it }
+        comments["MUSICBRAINZ_ALBUMARTISTID"]?.let { raw.albumArtistMusicBrainzIds = it }
-        tags["ALBUMARTIST"]?.let { raw.albumArtistNames = it }
+        comments["ALBUMARTIST"]?.let { raw.albumArtistNames = it }
-        tags["ALBUMARTISTSORT"]?.let { raw.albumArtistSortNames = it }
+        comments["ALBUMARTISTSORT"]?.let { raw.albumArtistSortNames = it }
 
         // Genre
-        tags["GENRE"]?.let { raw.genreNames = it }
+        comments["GENRE"]?.let { raw.genreNames = it }
     }
 
     /**
-     * Copies and sanitizes this string under the assumption that it is UTF-8. This should launder
+     * Copies and sanitizes a possibly native/non-UTF-8 string.
-     * away any weird UTF-8 issues that ExoPlayer may cause.
+     * @return A new string allocated in a memory-safe manner with any UTF-8 errors
+     * replaced with the Unicode replacement byte sequence.
      */
     private fun String.sanitize() = String(encodeToByteArray())
 }

app/src/main/java/org/oxycblt/auxio/music/extractor/ParsingUtil.kt

@@ -24,43 +24,73 @@ import org.oxycblt.auxio.settings.Settings
 import org.oxycblt.auxio.util.nonZeroOrNull
 
 /**
- * Parse out the track number field as if the given Int is formatted as DTTT, where D Is the disc
+ * Unpack the track number from a combined track + disc [Int] field.
- * and T is the track number. Values of zero will be ignored under the assumption that they are
+ * These fields appear within MediaStore's TRACK column, and combine the track and disc value
- * invalid.
+ * into a single field where the disc number is the 4th+ digit.
+ * @return The track number extracted from the combined integer value, or null if the value
+ * was zero.
  */
 fun Int.unpackTrackNo() = mod(1000).nonZeroOrNull()
 
 /**
- * Parse out the disc number field as if the given Int is formatted as DTTT, where D Is the disc and
+ * Unpack the disc number from a combined track + disc [Int] field.
- * T is the track number. Values of zero will be ignored under the assumption that they are invalid.
+ * These fields appear within MediaStore's TRACK column, and combine the track and disc value
+ * into a single field where the disc number is the 4th+ digit.
+ * @return The disc number extracted from the combined integer field, or null if the value
+ * was zero.
  */
 fun Int.unpackDiscNo() = div(1000).nonZeroOrNull()
 
 /**
- * Parse out the number field from an NN/TT string that is typically found in DISC_NUMBER and
+ * Parse the number out of a combined number + total position [String] field.
- * CD_TRACK_NUMBER. Values of zero will be ignored under the assumption that they are invalid.
+ * These fields often appear in ID3v2 files, and consist of a number and an (optional) total
+ * value delimited by a /.
+ * @return The number value extracted from the string field, or null if the value could not be
+ * parsed or if the value was zero.
  */
 fun String.parsePositionNum() = split('/', limit = 2)[0].toIntOrNull()?.nonZeroOrNull()
 
-/** Transform an int year into a [Date] */
+/**
+ * Transform an [Int] year field into a [Date].
+ * @return A [Date] consisting of the year value, or null if the value was zero.
+ * @see Date.from
+ */
 fun Int.toDate() = Date.from(this)
 
-/** Parse a plain year from the field into a [Date]. */
+/**
+ * Parse an integer year field from a [String] and transform it into a [Date].
+ * @return A [Date] consisting of the year value, or null if the value could not
+ * be parsed or if the value was zero.
+ * @see Date.from
+ */
 fun String.parseYear() = toIntOrNull()?.toDate()
 
-/** Parse an ISO-8601 time-stamp from this field into a [Date]. */
+/**
+ * Parse an ISO-8601 timestamp [String] into a [Date].
+ * @return A [Date] consisting of the year value plus one or more refinement values
+ * (ex. month, day), or null if the timestamp was not valid.
+ */
 fun String.parseTimestamp() = Date.from(this)
 
-/** Split a string by [selector], also handling escaping. */
+/**
+ * Split a [String] by the given selector, automatically handling escaped characters
+ * that satisfy the selector.
+ * @param selector A block that determines if the string should be split at a given
+ * character.
+ * @return One or more [String]s split by the selector.
+ */
 inline fun String.splitEscaped(selector: (Char) -> Boolean): MutableList<String> {
     val split = mutableListOf<String>()
     var currentString = ""
     var i = 0
+
     while (i < length) {
         val a = get(i)
         val b = getOrNull(i + 1)
 
         if (selector(a)) {
+            // Non-escaped separator, split the string here, making sure any stray whitespace
+            // is removed.
             split.add(currentString.trim())
             currentString = ""
             i++
@@ -68,15 +98,19 @@ inline fun String.splitEscaped(selector: (Char) -> Boolean): MutableList<String>
         }
 
         if (b != null && a == '\\' && selector(b)) {
+            // Is an escaped character, add the non-escaped variant and skip two
+            // characters to move on to the next one.
             currentString += b
             i += 2
         } else {
+            // Non-escaped, increment normally.
             currentString += a
             i++
         }
     }
 
     if (currentString.isNotEmpty()) {
+        // Had an in-progress split string we should add.
         split.add(currentString.trim())
     }
 
@@ -84,30 +118,36 @@ inline fun String.splitEscaped(selector: (Char) -> Boolean): MutableList<String>
 }
 
 /**
- * Fully parse a multi-value tag.
+ * Parse a multi-value tag based on the user configuration. If the value is already composed of
- *
+ * more than one value, nothing is done. Otherwise, this function will attempt to split it based
- * If there is only one string in the tag, and if enabled, it will be parsed for any multi-value
+ * on the user's separator preferences.
- * separators desired. Escaped separators will be ignored and replaced with their correct character.
+ * @param settings [Settings] required to obtain user separator configuration.
- *
+ * @return A new list of one or more [String]s.
- * Alternatively, if there are several tags already, it will be returned without modification.
  */
 fun List<String>.parseMultiValue(settings: Settings) =
     if (size == 1) {
         get(0).maybeParseSeparators(settings)
     } else {
+        // Nothing to do.
         this
     }
 
 /**
- * Maybe a single tag into multi values with the user-preferred separators. If not enabled, the
+ * Attempt to parse a string by the user's separator preferences.
- * plain string will be returned.
+ * @param settings [Settings] required to obtain user separator configuration.
+ * @return A list of one or more [String]s that were split up by the user-defined separators.
  */
 fun String.maybeParseSeparators(settings: Settings): List<String> {
-    // Get the separators the user desires. If null, we don't parse any.
+    // Get the separators the user desires. If null, there's nothing to do.
     val separators = settings.separators ?: return listOf(this)
     return splitEscaped { separators.contains(it) }
 }
 
+/**
+ * Convert a [String] to a [UUID].
+ * @return A [UUID] converted from the [String] value, or null if the value was not valid.
+ * @see UUID.fromString
+ */
 fun String.toUuidOrNull(): UUID? =
     try {
         UUID.fromString(this)
@@ -116,21 +156,32 @@ fun String.toUuidOrNull(): UUID? =
     }
 
 /**
- * Parse a multi-value genre name using ID3v2 rules. If there is one value, the ID3v2.3 rules will
+ * Parse a multi-value genre name using ID3 rules. This will convert any ID3v1 integer
- * be used, followed by separator parsing. Otherwise, each value will be iterated through, and
+ * representations of genre fields into their named counterparts, and split up singular
- * numeric values transformed into string values.
+ * ID3v2-style integer genre fields into one or more genres.
+ * @param settings [Settings] required to obtain user separator configuration.
+ * @return A list of one or more genre names..
  */
 fun List<String>.parseId3GenreNames(settings: Settings) =
     if (size == 1) {
         get(0).parseId3GenreNames(settings)
     } else {
+        // Nothing to split, just map any ID3v1 genres to their name counterparts.
         map { it.parseId3v1Genre() ?: it }
     }
 
-/** Parse a single genre name using ID3v2.3 rules. */
+/**
+ * Parse a single ID3v1/ID3v2 integer genre field into their named representations.
+ * @return A list of one or more genre names.
+ */
 fun String.parseId3GenreNames(settings: Settings) =
     parseId3v1Genre()?.let { listOf(it) } ?: parseId3v2Genre() ?: maybeParseSeparators(settings)
 
+/**
+ * Parse an ID3v1 integer genre field.
+ * @return A named genre if the field is a valid integer, "Cover" or "Remix" if the field is
+ * "CR"/"RX" respectively, and nothing if the field is not a valid ID3v1 integer genre.
+ */
 private fun String.parseId3v1Genre(): String? =
     when {
         // ID3v1 genres are a plain integer value without formatting, so in that case
@@ -145,8 +196,20 @@ private fun String.parseId3v1Genre(): String? =
         else -> null
     }
 
+/**
+ * A [Regex] that implements parsing for ID3v2's genre format.
+ * Derived from mutagen: https://github.com/quodlibet/mutagen
+ */
+private val ID3V2_GENRE_RE = Regex("((?:\\((\\d+|RX|CR)\\))*)(.+)?")
+
+/**
+ * Parse an ID3v2 integer genre field, which has support for multiple genre values and
+ * combined named/integer genres.
+ * @return A list of one or more genres, or null if the field is not a valid ID3v2
+ * integer genre.
+ */
 private fun String.parseId3v2Genre(): List<String>? {
-    val groups = (GENRE_RE.matchEntire(this) ?: return null).groupValues
+    val groups = (ID3V2_GENRE_RE.matchEntire(this) ?: return null).groupValues
     val genres = mutableSetOf<String>()
 
     // ID3v2.3 genres are far more complex and require string grokking to properly implement.
@@ -182,12 +245,9 @@ private fun String.parseId3v2Genre(): List<String>? {
     return genres.toList()
 }
 
-/** Regex that implements matching for ID3v2's genre format. */
-private val GENRE_RE = Regex("((?:\\((\\d+|RX|CR)\\))*)(.+)?")
-
 /**
- * A complete table of all the constant genre values for ID3(v2), including non-standard extensions.
+ * A table of the "conventional" mapping between ID3v1 integer genres and their named counterparts.
- * Note that we do not translate these, as that greatly increases technical complexity.
+ * Includes non-standard extensions.
  */
 private val GENRE_TABLE =
     arrayOf(
@@ -343,8 +403,8 @@ private val GENRE_TABLE =
         "JPop",
         "Synthpop",
 
-        // Winamp 5.6+ extensions, also used by EasyTAG.
+        // Winamp 5.6+ extensions, also used by EasyTAG. Not common, but post-rock is a good
-        // I only include this because post-rock is a based genre and deserves a slot.
+        // genre and should be included in the mapping.
         "Abstract",
         "Art Rock",
         "Baroque",
@@ -390,5 +450,5 @@ private val GENRE_TABLE =
         "Garage Rock",
         "Psybient",
 
-        // Auxio's extensions (Future garage is also based and deserves a slot)
+        // Auxio's extensions, added because Future Garage is also a good genre.
         "Future Garage")

app/src/main/java/org/oxycblt/auxio/music/extractor/SeparatorsDialog.kt

@@ -28,6 +28,12 @@ import org.oxycblt.auxio.settings.Settings
 import org.oxycblt.auxio.shared.ViewBindingDialogFragment
 import org.oxycblt.auxio.util.context
 
+/**
+ * A [ViewBindingDialogFragment] that allows the user to configure the separator characters
+ * used to split tags with multiple values.
+ * TODO: Add saved state for pending configurations.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
 class SeparatorsDialog : ViewBindingDialogFragment<DialogSeparatorsBinding>() {
     private val settings: Settings by lifecycleObject { binding -> Settings(binding.context) }
 
@@ -39,6 +45,9 @@ class SeparatorsDialog : ViewBindingDialogFragment<DialogSeparatorsBinding>() {
             .setTitle(R.string.set_separators)
             .setNegativeButton(R.string.lbl_cancel, null)
             .setPositiveButton(R.string.lbl_save) { _, _ ->
+                // Create the separator list based on the checked configuration of each
+                // view element. It's generally more stable to duplicate this code instead
+                // of use a mapping that could feasibly drift from the actual layout.
                 var separators = ""
                 val binding = requireBinding()
                 if (binding.separatorComma.isChecked) separators += SEPARATOR_COMMA
@@ -53,10 +62,15 @@ class SeparatorsDialog : ViewBindingDialogFragment<DialogSeparatorsBinding>() {
     override fun onBindingCreated(binding: DialogSeparatorsBinding, savedInstanceState: Bundle?) {
         for (child in binding.separatorGroup.children) {
             if (child is MaterialCheckBox) {
+                // Reset the CheckBox state so that we can ensure that state we load in
+                // from settings is not contaminated from the built-in CheckBox saved state.
                 child.isChecked = false
             }
         }
 
+        // More efficient to do one iteration through the separator list and initialize
+        // the corresponding CheckBox for each character instead of doing an iteration
+        // through the separator list for each CheckBox.
         settings.separators?.forEach {
             when (it) {
                 SEPARATOR_COMMA -> binding.separatorComma.isChecked = true
@@ -70,6 +84,7 @@ class SeparatorsDialog : ViewBindingDialogFragment<DialogSeparatorsBinding>() {
     }
 
     companion object {
+        // TODO: Move these to a more "Correct" location?
         private const val SEPARATOR_COMMA = ','
         private const val SEPARATOR_SEMICOLON = ';'
         private const val SEPARATOR_SLASH = '/'

app/src/main/java/org/oxycblt/auxio/music/picker/ArtistChoiceAdapter.kt

@@ -27,7 +27,11 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.util.context
 import org.oxycblt.auxio.util.inflater
 
-/** The adapter that displays a list of artist choices in the picker UI. */
+/**
+ * An adapter responsible for showing a list of [Artist] choices in [ArtistPickerDialog].
+ * @param listener A [BasicListListener] for list interactions.
+ * @author OxygenCobalt.
+ */
 class ArtistChoiceAdapter(private val listener: BasicListListener) :
     RecyclerView.Adapter<ArtistChoiceViewHolder>() {
     private var artists = listOf<Artist>()
@@ -40,28 +44,41 @@ class ArtistChoiceAdapter(private val listener: BasicListListener) :
     override fun onBindViewHolder(holder: ArtistChoiceViewHolder, position: Int) =
         holder.bind(artists[position], listener)
 
+    /**
+     * Immediately update the tab array. This should be used when initializing the list.
+     * @param newTabs The new array of tabs to show.
+     */
     fun submitList(newArtists: List<Artist>) {
         if (newArtists != artists) {
             artists = newArtists
-
             @Suppress("NotifyDataSetChanged") notifyDataSetChanged()
         }
     }
 }
 
 /**
- * The ViewHolder that displays a artist choice. Smaller than other parent items due to dialog
+ * A [DialogRecyclerView.ViewHolder] that displays a smaller variant of a typical
- * constraints.
+ * [Artist] item, for use with [ArtistChoiceAdapter]. Use [new] to instantiate a new instance.
  */
 class ArtistChoiceViewHolder(private val binding: ItemPickerChoiceBinding) :
     DialogRecyclerView.ViewHolder(binding.root) {
+    /**
+     * Bind new data to this instance.
+     * @param artist The new [Artist] to bind.
+     * @param listener A [BasicListListener] to bind interactions to.
+     */
     fun bind(artist: Artist, listener: BasicListListener) {
+        binding.root.setOnClickListener { listener.onClick(artist) }
         binding.pickerImage.bind(artist)
         binding.pickerName.text = artist.resolveName(binding.context)
-        binding.root.setOnClickListener { listener.onClick(artist) }
     }
 
     companion object {
+        /**
+         * Create a new instance.
+         * @param parent The parent to inflate this instance from.
+         * @return A new instance.
+         */
         fun new(parent: View) =
             ArtistChoiceViewHolder(ItemPickerChoiceBinding.inflate(parent.context.inflater))
     }

app/src/main/java/org/oxycblt/auxio/music/picker/ArtistNavigationPickerDialog.kt

@@ -26,12 +26,13 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.shared.NavigationViewModel
 
 /**
- * The [ArtistPickerDialog] for ambiguous artist navigation operations.
+ * An [ArtistPickerDialog] intended for when [Artist] navigation is ambiguous.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class ArtistNavigationPickerDialog : ArtistPickerDialog() {
     private val navModel: NavigationViewModel by activityViewModels()
-
+    // Information about what artists to display is initially within the navigation arguments
+    // as a list of UIDs, as that is the only safe way to parcel an artist.
     private val args: ArtistNavigationPickerDialogArgs by navArgs()
 
     override fun onBindingCreated(binding: DialogMusicPickerBinding, savedInstanceState: Bundle?) {
@@ -42,6 +43,7 @@ class ArtistNavigationPickerDialog : ArtistPickerDialog() {
     override fun onClick(item: Item) {
         super.onClick(item)
         check(item is Artist) { "Unexpected datatype: ${item::class.simpleName}" }
+        // User made a choice, navigate to it.
         navModel.exploreNavigateTo(item)
     }
 }

app/src/main/java/org/oxycblt/auxio/music/picker/ArtistPickerDialog.kt

@@ -30,9 +30,16 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.shared.ViewBindingDialogFragment
 import org.oxycblt.auxio.util.collectImmediately
 
+/**
+ * The base class for dialogs that implements common behavior across all [Artist] pickers.
+ * These are shown whenever what to do with an item's [Artist] is ambiguous, as there are
+ * multiple [Artist]'s to choose from.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
 abstract class ArtistPickerDialog : ViewBindingDialogFragment<DialogMusicPickerBinding>(), BasicListListener {
-    protected val pickerModel: MusicPickerViewModel by viewModels()
+    protected val pickerModel: PickerViewModel by viewModels()
-    private val artistAdapter = ArtistChoiceAdapter(this)
+    // Okay to leak this since the Listener will not be called until after full initialization.
+    private val artistAdapter = ArtistChoiceAdapter(@Suppress("LeakingThis") this)
 
     override fun onCreateBinding(inflater: LayoutInflater) =
         DialogMusicPickerBinding.inflate(inflater)
@@ -46,8 +53,12 @@ abstract class ArtistPickerDialog : ViewBindingDialogFragment<DialogMusicPickerB
 
         collectImmediately(pickerModel.currentArtists) { artists ->
             if (!artists.isNullOrEmpty()) {
+                // Make sure the artist choices align with the current music library.
+                // TODO: I really don't think it makes sense to do this. I'd imagine it would
+                //  be more productive to just exit this dialog rather than try to update it.
                 artistAdapter.submitList(artists)
             } else {
+                // Not showing any choices, navigate up.
                 findNavController().navigateUp()
             }
         }

app/src/main/java/org/oxycblt/auxio/music/picker/ArtistPlaybackPickerDialog.kt

@@ -26,12 +26,13 @@ import org.oxycblt.auxio.playback.PlaybackViewModel
 import org.oxycblt.auxio.util.androidActivityViewModels
 
 /**
- * The [ArtistPickerDialog] for ambiguous artist playback operations.
+ * An [ArtistPickerDialog] intended for when [Artist] playback is ambiguous.
  * @author Alexander Capehart (OxygenCobalt)
  */
 class ArtistPlaybackPickerDialog : ArtistPickerDialog() {
     private val playbackModel: PlaybackViewModel by androidActivityViewModels()
-
+    // Information about what artists to display is initially within the navigation arguments
+    // as a list of UIDs, as that is the only safe way to parcel an artist.
     private val args: ArtistPlaybackPickerDialogArgs by navArgs()
 
     override fun onBindingCreated(binding: DialogMusicPickerBinding, savedInstanceState: Bundle?) {
@@ -42,6 +43,7 @@ class ArtistPlaybackPickerDialog : ArtistPickerDialog() {
     override fun onClick(item: Item) {
         super.onClick(item)
         check(item is Artist) { "Unexpected datatype: ${item::class.simpleName}" }
+        // User made a choice, play the given song from that artist.
         pickerModel.currentSong.value?.let { song -> playbackModel.playFromArtist(song, item) }
     }
 }

app/src/main/java/org/oxycblt/auxio/music/picker/PickerViewModel.kt

@@ -26,30 +26,40 @@ import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.util.unlikelyToBeNull
 
-class MusicPickerViewModel : ViewModel(), MusicStore.Callback {
+/**
+ * a [ViewModel] that manages the current music picker state.
+ * TODO: This really shouldn't exist. Make it so that the dialogs just contain the music
+ * themselves and then exit if the library changes.
+ * TODO: While we are at it, let's go and add ClickableSpan too to reduce the extent of
+ * this dialog.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+class PickerViewModel : ViewModel(), MusicStore.Callback {
     private val musicStore = MusicStore.getInstance()
 
     private val _currentSong = MutableStateFlow<Song?>(null)
+    /**
+     * The current [Song] whose choices are being shown in the picker. Null if there is no [Song].
+     */
     val currentSong: StateFlow<Song?>
         get() = _currentSong
 
     private val _currentArtists = MutableStateFlow<List<Artist>?>(null)
+    /**
+     * The current [Artist] whose choices are being shown in the picker. Null/Empty if there is none.
+     */
     val currentArtists: StateFlow<List<Artist>?>
         get() = _currentArtists
 
-    fun setSongUid(uid: Music.UID) {
+    override fun onCleared() {
-        val library = unlikelyToBeNull(musicStore.library)
+        musicStore.removeCallback(this)
-        _currentSong.value = library.find(uid)
-        _currentArtists.value = _currentSong.value?.artists
-    }
-
-    fun setArtistUids(uids: Array<Music.UID>) {
-        val library = unlikelyToBeNull(musicStore.library)
-        _currentArtists.value = uids.mapNotNull { library.find<Artist>(it) }.ifEmpty { null }
     }
 
     override fun onLibraryChanged(library: MusicStore.Library?) {
         if (library != null) {
+            // If we are showing any item right now, we will need to refresh it (and any information
+            // related to it) with the new library in order to prevent stale items from appearing
+            // in the UI.
             val song = _currentSong.value
             val artists = _currentArtists.value
             if (song != null) {
@@ -60,4 +70,25 @@ class MusicPickerViewModel : ViewModel(), MusicStore.Callback {
             }
         }
     }
+
+    /**
+     * Set a new [currentSong] from it's [Music.UID].
+     * @param uid The [Music.UID] of the [Song] to update to.
+     */
+    fun setSongUid(uid: Music.UID) {
+        val library = unlikelyToBeNull(musicStore.library)
+        _currentSong.value = library.find(uid)
+        _currentArtists.value = _currentSong.value?.artists
+    }
+
+    /**
+     * Set a new [currentArtists] list from a list of [Music.UID]'s.
+     * @param uids The [Music.UID]s of the [Artist]s to [currentArtists] to.
+     */
+    fun setArtistUids(uids: Array<Music.UID>) {
+        val library = unlikelyToBeNull(musicStore.library)
+        // Map the UIDs to artist instances and filter out the ones that can't be found.
+        _currentArtists.value = uids.mapNotNull { library.find<Artist>(it) }.ifEmpty { null }
+    }
+
 }

app/src/main/java/org/oxycblt/auxio/music/storage/DirectoryAdapter.kt

@@ -26,11 +26,15 @@ import org.oxycblt.auxio.util.context
 import org.oxycblt.auxio.util.inflater
 
 /**
- * Adapter that shows the list of music folder and their "Clear" button.
+ * [RecyclerView.Adapter] that manages a list of [Directory] instances.
+ * @param listener [Listener] for list interactions.
  * @author Alexander Capehart (OxygenCobalt)
  */
-class MusicDirAdapter(private val listener: Listener) : RecyclerView.Adapter<MusicDirViewHolder>() {
+class DirectoryAdapter(private val listener: Listener) : RecyclerView.Adapter<MusicDirViewHolder>() {
     private val _dirs = mutableListOf<Directory>()
+    /**
+     * The current list of [Directory]s, may not line up with [MusicDirectories] due to removals.
+     */
     val dirs: List<Directory> = _dirs
 
     override fun getItemCount() = dirs.size
@@ -41,6 +45,10 @@ class MusicDirAdapter(private val listener: Listener) : RecyclerView.Adapter<Mus
     override fun onBindViewHolder(holder: MusicDirViewHolder, position: Int) =
         holder.bind(dirs[position], listener)
 
+    /**
+     * Add a [Directory] to the end of the list.
+     * @param dir The [Directory] to add.
+     */
     fun add(dir: Directory) {
         if (_dirs.contains(dir)) {
             return
@@ -50,27 +58,38 @@ class MusicDirAdapter(private val listener: Listener) : RecyclerView.Adapter<Mus
         notifyItemInserted(_dirs.lastIndex)
     }
 
+    /**
+     * Add a list of [Directory] instances to the end of the list.
+     * @param dirs The [Directory instances to add.
+     */
     fun addAll(dirs: List<Directory>) {
         val oldLastIndex = dirs.lastIndex
         _dirs.addAll(dirs)
         notifyItemRangeInserted(oldLastIndex, dirs.size)
     }
 
+    /**
+     * Remove a [Directory] from the list.
+     * @param dir The [Directory] to remove. Must exist in the list.
+     */
     fun remove(dir: Directory) {
         val idx = _dirs.indexOf(dir)
         _dirs.removeAt(idx)
         notifyItemRemoved(idx)
     }
 
+    /**
+     * A Listener for [DirectoryAdapter] interactions.
+     */
     interface Listener {
         fun onRemoveDirectory(dir: Directory)
     }
 }
 
-/** The viewholder for [MusicDirAdapter]. Not intended for use in other adapters. */
+/** The viewholder for [DirectoryAdapter]. Not intended for use in other adapters. */
 class MusicDirViewHolder private constructor(private val binding: ItemMusicDirBinding) :
     DialogRecyclerView.ViewHolder(binding.root) {
-    fun bind(item: Directory, listener: MusicDirAdapter.Listener) {
+    fun bind(item: Directory, listener: DirectoryAdapter.Listener) {
         binding.dirPath.text = item.resolveName(binding.context)
         binding.dirDelete.setOnClickListener { listener.onRemoveDirectory(item) }
     }

app/src/main/java/org/oxycblt/auxio/music/storage/MusicDirectories.kt

@@ -16,6 +16,3 @@
  */
  
 package org.oxycblt.auxio.music.storage
-
-/** Represents a the configuration for the "Folder Management" setting */
-data class MusicDirs(val dirs: List<Directory>, val shouldInclude: Boolean)

app/src/main/java/org/oxycblt/auxio/music/storage/MusicDirsDialog.kt

@@ -40,8 +40,8 @@ import org.oxycblt.auxio.util.showToast
  * @author Alexander Capehart (OxygenCobalt)
  */
 class MusicDirsDialog :
-    ViewBindingDialogFragment<DialogMusicDirsBinding>(), MusicDirAdapter.Listener {
+    ViewBindingDialogFragment<DialogMusicDirsBinding>(), DirectoryAdapter.Listener {
-    private val dirAdapter = MusicDirAdapter(this)
+    private val dirAdapter = DirectoryAdapter(this)
     private val settings: Settings by lifecycleObject { binding -> Settings(binding.context) }
     private val storageManager: StorageManager by lifecycleObject { binding ->
         binding.context.getSystemServiceCompat(StorageManager::class)
@@ -59,7 +59,7 @@ class MusicDirsDialog :
             .setPositiveButton(R.string.lbl_save) { _, _ ->
                 val dirs = settings.getMusicDirs(storageManager)
                 val newDirs =
-                    MusicDirs(
+                    MusicDirectories(
                         dirs = dirAdapter.dirs, shouldInclude = isUiModeInclude(requireBinding()))
                 if (dirs != newDirs) {
                     logD("Committing changes")
@@ -97,8 +97,8 @@ class MusicDirsDialog :
 
             if (pendingDirs != null) {
                 dirs =
-                    MusicDirs(
+                    MusicDirectories(
-                        pendingDirs.mapNotNull { Directory.fromDocumentUri(storageManager, it) },
+                        pendingDirs.mapNotNull { Directory.fromDocumentTreeUri(storageManager, it) },
                         savedInstanceState.getBoolean(KEY_PENDING_MODE))
             }
         }
@@ -162,7 +162,7 @@ class MusicDirsDialog :
         val treeUri = DocumentsContract.getTreeDocumentId(docUri)
 
         // Parsing handles the rest
-        return Directory.fromDocumentUri(storageManager, treeUri)
+        return Directory.fromDocumentTreeUri(storageManager, treeUri)
     }
 
     private fun updateMode() {

app/src/main/java/org/oxycblt/auxio/music/storage/Storage.kt

@@ -0,0 +1,200 @@
+package org.oxycblt.auxio.music.storage
+
+import android.content.Context
+import android.os.storage.StorageManager
+import android.os.storage.StorageVolume
+import android.webkit.MimeTypeMap
+import com.google.android.exoplayer2.util.MimeTypes
+import org.oxycblt.auxio.R
+import java.io.File
+
+
+/**
+ * A full absolute path to a file. Only intended for display purposes. For accessing files,
+ * URIs are preferred in all cases due to scoped storage limitations.
+ * @param name The name of the file.
+ * @param parent The parent [Directory] of the file.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+data class Path(val name: String, val parent: Directory)
+
+/**
+ * A volume-aware relative path to a directory.
+ * @param volume The [StorageVolume] that the [Directory] is contained in.
+ * @param relativePath The relative path from within the [StorageVolume] to the [Directory].
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+class Directory private constructor(val volume: StorageVolume, val relativePath: String) {
+    /**
+     * Resolve the [Directory] instance into a human-readable path name.
+     * @param context [Context] required to obtain volume descriptions.
+     * @return A human-readable path.
+     * @see StorageVolume.getDescription
+     */
+    fun resolveName(context: Context) =
+        context.getString(R.string.fmt_path, volume.getDescriptionCompat(context), relativePath)
+
+    /**
+     * Converts this [Directory] instance into an opaque document tree path.
+     * This is a huge violation of the document tree URI contract, but it's also the only
+     * one can sensibly work with these uris in the UI, and it doesn't exactly matter since
+     * we never write or read directory.
+     * @return A URI [String] abiding by the document tree specification, or null
+     * if the [Directory] is not valid.
+     */
+    fun toDocumentTreeUri() =
+        // Document tree URIs consist of a prefixed volume name followed by a relative path.
+        if (volume.isInternalCompat) {
+            // The primary storage has a volume prefix of "primary", regardless
+            // of if it's internal or not.
+            "$DOCUMENT_URI_PRIMARY_NAME:$relativePath"
+        } else {
+            // Removable storage has a volume prefix of it's UUID.
+            volume.uuidCompat?.let { uuid -> "$uuid:$relativePath" }
+        }
+
+    override fun hashCode(): Int {
+        var result = volume.hashCode()
+        result = 31 * result + relativePath.hashCode()
+        return result
+    }
+
+    override fun equals(other: Any?) =
+        other is Directory && other.volume == volume && other.relativePath == relativePath
+
+    companion object {
+        /**
+         * The name given to the internal volume when in a document tree URI.
+         */
+        private const val DOCUMENT_URI_PRIMARY_NAME = "primary"
+
+        /**
+         * Create a new directory instance from the given components.
+         * @param volume The [StorageVolume] that the [Directory] is contained in.
+         * @param relativePath The relative path from within the [StorageVolume] to the [Directory].
+         * Will be stripped of any trailing separators for a consistent internal representation.
+         * @return A new [Directory] created from the components.
+         */
+        fun from(volume: StorageVolume, relativePath: String) =
+            Directory(
+                volume, relativePath.removePrefix(File.separator).removeSuffix(File.separator))
+
+        /**
+         * Create a new directory from a document tree URI.
+         * This is a huge violation of the document tree URI contract, but it's also the only
+         * one can sensibly work with these uris in the UI, and it doesn't exactly matter since
+         * we never write or read directory.
+         * @param storageManager [StorageManager] in order to obtain the [StorageVolume] specified
+         * in the given URI.
+         * @param uri The URI string to parse into a [Directory].
+         * @return A new [Directory] parsed from the URI, or null if the URI is not valid.
+         */
+        fun fromDocumentTreeUri(storageManager: StorageManager, uri: String): Directory? {
+            // Document tree URIs consist of a prefixed volume name followed by a relative path,
+            // delimited with a colon.
+            val split = uri.split(File.pathSeparator, limit = 2)
+            val volume =
+                when (split[0]) {
+                    // The primary storage has a volume prefix of "primary", regardless
+                    // of if it's internal or not.
+                    DOCUMENT_URI_PRIMARY_NAME -> storageManager.primaryStorageVolumeCompat
+                    // Removable storage has a volume prefix of it's UUID, try to find it
+                    // within StorageManager's volume list.
+                    else -> storageManager.storageVolumesCompat.find { it.uuidCompat == split[0] }
+                }
+            val relativePath = split.getOrNull(1)
+            return from(volume ?: return null, relativePath ?: return null)
+        }
+    }
+}
+
+/**
+ * Represents the configuration for specific directories to filter to/from when loading music.
+ * TODO: Migrate to a combined "Include + Exclude" system that is more sensible.
+ * @param dirs A list of [Directory] instances. How these are interpreted depends on
+ * [shouldInclude].
+ * @param shouldInclude True if the library should only load from the [Directory] instances,
+ * false if the library should not load from the [Directory] instances.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+data class MusicDirectories(val dirs: List<Directory>, val shouldInclude: Boolean)
+
+/**
+ * A mime type of a file. Only intended for display.
+ * @param fromExtension The mime type obtained by analyzing the file extension.
+ * @param fromFormat The mime type obtained by analyzing the file format. Null if could
+ * not be obtained.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+data class MimeType(val fromExtension: String, val fromFormat: String?) {
+    /**
+     * Resolve the mime type into a human-readable format name, such as "Ogg Vorbis".
+     * @param context [Context] required to obtain human-readable strings.
+     * @return A human-readable name for this mime type. Will first try [fromFormat],
+     * then falling back to [fromExtension], then falling back to the extension name,
+     * and then finally a placeholder "No Format" string.
+     */
+    fun resolveName(context: Context): String {
+        // We try our best to produce a more readable name for the common audio formats.
+        val formatName =
+            when (fromFormat) {
+                // We start with the extracted mime types, as they are more consistent. Note that
+                // we do not include container formats at all with these names. It is only the
+                // inner codec that we bother with.
+                MimeTypes.AUDIO_MPEG,
+                MimeTypes.AUDIO_MPEG_L1,
+                MimeTypes.AUDIO_MPEG_L2 -> R.string.cdc_mp3
+                MimeTypes.AUDIO_AAC -> R.string.cdc_aac
+                MimeTypes.AUDIO_VORBIS -> R.string.cdc_vorbis
+                MimeTypes.AUDIO_OPUS -> R.string.cdc_opus
+                MimeTypes.AUDIO_FLAC -> R.string.cdc_flac
+                MimeTypes.AUDIO_WAV -> R.string.cdc_wav
+
+                // We don't give a name to more unpopular formats.
+
+                else -> -1
+            }
+
+        if (formatName > -1) {
+            return context.getString(formatName)
+        }
+
+        // Fall back to the file extension in the case that we have no mime type or
+        // a useless "audio/raw" mime type. Here:
+        // - We return names for container formats instead of the inner format, as we
+        // cannot parse the file.
+        // - We are at the mercy of the Android OS, hence we check for every possible mime
+        // type for a particular format according to Wikipedia.
+        val extensionName =
+            when (fromExtension) {
+                "audio/mpeg",
+                "audio/mp3" -> R.string.cdc_mp3
+                "audio/mp4",
+                "audio/mp4a-latm",
+                "audio/mpeg4-generic" -> R.string.cdc_mp4
+                "audio/aac",
+                "audio/aacp",
+                "audio/3gpp",
+                "audio/3gpp2" -> R.string.cdc_aac
+                "audio/ogg",
+                "application/ogg",
+                "application/x-ogg" -> R.string.cdc_ogg
+                "audio/flac" -> R.string.cdc_flac
+                "audio/wav",
+                "audio/x-wav",
+                "audio/wave",
+                "audio/vnd.wave" -> R.string.cdc_wav
+                "audio/x-matroska" -> R.string.cdc_mka
+                else -> -1
+            }
+
+        return if (extensionName > -1) {
+            context.getString(extensionName)
+        } else {
+            // Fall back to the extension if we can't find a special name for this format.
+            MimeTypeMap.getSingleton().getExtensionFromMimeType(fromExtension)?.uppercase()
+                // Fall back to a placeholder if even that fails.
+                ?: context.getString(R.string.def_codec)
+        }
+    }
+}

app/src/main/java/org/oxycblt/auxio/music/storage/StorageUtil.kt

@@ -28,186 +28,109 @@ import android.os.Environment
 import android.os.storage.StorageManager
 import android.os.storage.StorageVolume
 import android.provider.MediaStore
-import android.webkit.MimeTypeMap
-import com.google.android.exoplayer2.util.MimeTypes
-import java.io.File
 import java.lang.reflect.Method
-import org.oxycblt.auxio.R
 import org.oxycblt.auxio.util.lazyReflectedMethod
 
-/** A path to a file. [name] is the stripped file name, [parent] is the parent path. */
+// --- MEDIASTORE UTILITIES ---
-data class Path(val name: String, val parent: Directory)
 
 /**
- * A path to a directory. [volume] is the volume the directory resides in, and [relativePath] is the
+ * A shortcut for querying the [ContentResolver] database.
- * path from the volume's root to the directory itself.
+ * @param uri The [Uri] of content to retrieve.
+ * @param projection A list of SQL columns to query from the database.
+ * @param selector A SQL selection statement to filter results. Spaces where
+ * arguments should be filled in are represented with a "?".
+ * @param args The arguments used for the selector.
+ * @return A [Cursor] of the queried values, organized by the column projection.
+ * @throws IllegalStateException If the [ContentResolver] did not successfully return
+ * @see ContentResolver.query
+ * a queried [Cursor].
  */
-class Directory private constructor(val volume: StorageVolume, val relativePath: String) {
+fun ContentResolver.safeQuery(
-    fun resolveName(context: Context) =
-        context.getString(R.string.fmt_path, volume.getDescriptionCompat(context), relativePath)
-
-    /** Converts this dir into an opaque document URI in the form of VOLUME:PATH. */
-    fun toDocumentUri() =
-        // "primary" actually corresponds to the internal storage, not the primary volume.
-        // Removable storage is represented with the UUID.
-        if (volume.isInternalCompat) {
-            "$DOCUMENT_URI_PRIMARY_NAME:$relativePath"
-        } else {
-            volume.uuidCompat?.let { uuid -> "$uuid:$relativePath" }
-        }
-
-    override fun hashCode(): Int {
-        var result = volume.hashCode()
-        result = 31 * result + relativePath.hashCode()
-        return result
-    }
-
-    override fun equals(other: Any?) =
-        other is Directory && other.volume == volume && other.relativePath == relativePath
-
-    companion object {
-        private const val DOCUMENT_URI_PRIMARY_NAME = "primary"
-
-        fun from(volume: StorageVolume, relativePath: String) =
-            Directory(
-                volume, relativePath.removePrefix(File.separator).removeSuffix(File.separator))
-
-        /**
-         * Converts an opaque document uri in the form of VOLUME:PATH into a [Directory]. This is a
-         * flagrant violation of the API convention, but since we never really write to the URI I
-         * really doubt it matters.
-         */
-        fun fromDocumentUri(storageManager: StorageManager, uri: String): Directory? {
-            val split = uri.split(File.pathSeparator, limit = 2)
-
-            val volume =
-                when (split[0]) {
-                    DOCUMENT_URI_PRIMARY_NAME -> storageManager.primaryStorageVolumeCompat
-                    else -> storageManager.storageVolumesCompat.find { it.uuidCompat == split[0] }
-                }
-
-            val relativePath = split.getOrNull(1)
-
-            return from(volume ?: return null, relativePath ?: return null)
-        }
-    }
-}
-
-/**
- * Represents a mime type as it is loaded by Auxio. [fromExtension] is based on the file extension
- * should always exist, while [fromFormat] is based on the file itself and may not be available.
- * @author Alexander Capehart (OxygenCobalt)
- */
-data class MimeType(val fromExtension: String, val fromFormat: String?) {
-    fun resolveName(context: Context): String {
-        // We try our best to produce a more readable name for the common audio formats.
-        val formatName =
-            when (fromFormat) {
-                // We start with the extracted mime types, as they are more consistent. Note that
-                // we do not include container formats at all with these names. It is only the
-                // inner codec that we show.
-                MimeTypes.AUDIO_MPEG,
-                MimeTypes.AUDIO_MPEG_L1,
-                MimeTypes.AUDIO_MPEG_L2 -> R.string.cdc_mp3
-                MimeTypes.AUDIO_AAC -> R.string.cdc_aac
-                MimeTypes.AUDIO_VORBIS -> R.string.cdc_vorbis
-                MimeTypes.AUDIO_OPUS -> R.string.cdc_opus
-                MimeTypes.AUDIO_FLAC -> R.string.cdc_flac
-                MimeTypes.AUDIO_WAV -> R.string.cdc_wav
-
-                // We don't give a name to more unpopular formats.
-
-                else -> -1
-            }
-
-        if (formatName > -1) {
-            return context.getString(formatName)
-        }
-
-        // Fall back to the file extension in the case that we have no mime type or
-        // a useless "audio/raw" mime type. Here:
-        // - We return names for container formats instead of the inner format, as we
-        // cannot parse the file.
-        // - We are at the mercy of the Android OS, hence we check for every possible mime
-        // type for a particular format.
-        val extensionName =
-            when (fromExtension) {
-                "audio/mpeg",
-                "audio/mp3" -> R.string.cdc_mp3
-                "audio/mp4",
-                "audio/mp4a-latm",
-                "audio/mpeg4-generic" -> R.string.cdc_mp4
-                "audio/aac",
-                "audio/aacp",
-                "audio/3gpp",
-                "audio/3gpp2" -> R.string.cdc_aac
-                "audio/ogg",
-                "application/ogg",
-                "application/x-ogg" -> R.string.cdc_ogg
-                "audio/flac" -> R.string.cdc_flac
-                "audio/wav",
-                "audio/x-wav",
-                "audio/wave",
-                "audio/vnd.wave" -> R.string.cdc_wav
-                "audio/x-matroska" -> R.string.cdc_mka
-                else -> -1
-            }
-
-        return if (extensionName > -1) {
-            context.getString(extensionName)
-        } else {
-            // Fall back to the extension if we can't find a special name for this format.
-            MimeTypeMap.getSingleton().getExtensionFromMimeType(fromExtension)?.uppercase()
-                ?: context.getString(R.string.def_codec)
-        }
-    }
-}
-
-/** 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)
+) = requireNotNull(query(uri, projection, selector, args, null)) {
+    "ContentResolver query failed"
+}
 
-/** Shortcut for making a [ContentResolver] query and using the particular cursor with [use]. */
+/**
+ * A shortcut for [safeQuery] with [use] applied, automatically cleaning up the [Cursor]'s
+ * resources when no longer used.
+ * @param uri The [Uri] of content to retrieve.
+ * @param projection A list of SQL columns to query from the database.
+ * @param selector A SQL selection statement to filter results. Spaces where
+ * arguments should be filled in are represented with a "?".
+ * @param args The arguments used for the selector.
+ * @param block The block of code to run with the queried [Cursor]. Will not be ran if the
+ * [Cursor] is empty.
+ * @throws IllegalStateException If the [ContentResolver] did not successfully return
+ * @see ContentResolver.query
+ * a queried [Cursor].
+ */
 inline fun <reified R> ContentResolver.useQuery(
     uri: Uri,
     projection: Array<out String>,
     selector: String? = null,
     args: Array<String>? = null,
     block: (Cursor) -> R
-) = queryCursor(uri, projection, selector, args)?.use(block)
+) = safeQuery(uri, projection, selector, args).use(block)
 
 /**
- * For some reason the album cover URI namespace does not have a member in [MediaStore], but it
+ * Album art [MediaStore] database is not a built-in constant, have to define it ourselves.
- * still works since at least API 21.
  */
-private val EXTERNAL_ALBUM_ART_URI = Uri.parse("content://media/external/audio/albumart")
+private val EXTERNAL_COVERS_URI = Uri.parse("content://media/external/audio/albumart")
 
-/** Converts a [Long] Audio ID into a URI to that particular audio file. */
+/**
-val Long.audioUri: Uri
+ * Convert a [MediaStore] Song ID into a [Uri] to it's audio file.
-    get() = ContentUris.withAppendedId(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, this)
+ * @return An external storage audio file [Uri]. May not exist.
+ * @see ContentUris.withAppendedId
+ * @see MediaStore.Audio.Media.EXTERNAL_CONTENT_URI
+ */
+fun Long.toAudioUri() = ContentUris.withAppendedId(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, this)
 
-/** Converts a [Long] Album ID into a URI pointing to MediaStore-cached album art. */
+/**
-val Long.albumCoverUri: Uri
+ * Convert a [MediaStore] Album ID into a [Uri] to it's system-provided album cover. This cover
-    get() = ContentUris.withAppendedId(EXTERNAL_ALBUM_ART_URI, this)
+ * will be fast to load, but will be lower quality.
+ * @return An external storage image [Uri]. May not exist.
+ * @see ContentUris.withAppendedId
+ */
+fun Long.toCoverUri() = ContentUris.withAppendedId(EXTERNAL_COVERS_URI, this)
 
+// --- STORAGEMANAGER UTILITIES ---
+// Largely derived from Material Files: https://github.com/zhanghai/MaterialFiles
+
+/**
+ * Provides the analogous method to [StorageManager.getStorageVolumes] method that is usable from
+ * API 21 to API 23, in which the [StorageManager] API was hidden and differed greatly.
+ * @see StorageManager.getStorageVolumes
+ */
 @Suppress("NewApi")
 private val SM_API21_GET_VOLUME_LIST_METHOD: Method by
     lazyReflectedMethod(StorageManager::class, "getVolumeList")
 
+
+/**
+ * Provides the analogous method to [StorageVolume.getDirectory] method that is usable from
+ * API 21 to API 23, in which the [StorageVolume] API was hidden and differed greatly.
+ * @see StorageVolume.getDirectory
+ */
 @Suppress("NewApi")
 private val SV_API21_GET_PATH_METHOD: Method by lazyReflectedMethod(StorageVolume::class, "getPath")
 
-/** The "primary" storage volume containing the OS. May be an SD Card. */
+/**
+ * The [StorageVolume] considered the "primary" volume by the system, obtained in a
+ * version-compatible manner.
+ * @see StorageManager.getPrimaryStorageVolume
+ * @see StorageVolume.isPrimary
+ */
 val StorageManager.primaryStorageVolumeCompat: StorageVolume
     @Suppress("NewApi") get() = primaryStorageVolume
 
 /**
- * A list of recognized volumes, retrieved in a compatible manner. Note that these volumes may be
+ * The list of [StorageVolume]s currently recognized by [StorageManager], in a version-compatible
- * mounted or unmounted.
+ * manner.
+ * @see StorageManager.getStorageVolumes
  */
 val StorageManager.storageVolumesCompat: List<StorageVolume>
     get() =
@@ -218,14 +141,18 @@ val StorageManager.storageVolumesCompat: List<StorageVolume>
             (SM_API21_GET_VOLUME_LIST_METHOD.invoke(this) as Array<StorageVolume>).toList()
         }
 
-/** Returns the absolute path to a particular volume in a compatible manner. */
+/**
+ * The the absolute path to this [StorageVolume]'s directory within the file-system, in a
+ * version-compatible manner. Will be null if the [StorageVolume] cannot be read.
+ * @see StorageVolume.getDirectory
+ */
 val StorageVolume.directoryCompat: String?
     @SuppressLint("NewApi")
     get() =
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
             directory?.absolutePath
         } else {
-            // Replicate API: getPath if mounted, null if not
+            // Replicate API: Analogous method if mounted, null if not
             when (stateCompat) {
                 Environment.MEDIA_MOUNTED,
                 Environment.MEDIA_MOUNTED_READ_ONLY ->
@@ -234,37 +161,59 @@ val StorageVolume.directoryCompat: String?
             }
         }
 
-/** Get the readable description of the volume in a compatible manner. */
+/**
+ * Get the human-readable description of this volume, such as "Internal Shared Storage".
+ * @param context [Context] required to obtain human-readable string resources.
+ * @return A human-readable name for this volume.
+ */
 @SuppressLint("NewApi")
 fun StorageVolume.getDescriptionCompat(context: Context): String = getDescription(context)
 
-/** If this volume is the primary volume. May still be removable storage. */
+/**
+ * If this [StorageVolume] is considered the "Primary" volume where the Android System is
+ * kept. May still be a removable volume.
+ * @see StorageVolume.isPrimary
+ */
 val StorageVolume.isPrimaryCompat: Boolean
     @SuppressLint("NewApi") get() = isPrimary
 
-/** If this volume is emulated. */
+/**
+ * If this storage is "emulated", i.e intrinsic to the device, obtained in a version compatible
+ * manner.
+ * @see StorageVolume.isEmulated
+ */
 val StorageVolume.isEmulatedCompat: Boolean
     @SuppressLint("NewApi") get() = isEmulated
 
 /**
- * If this volume corresponds to "Internal shared storage", represented in document URIs as
+ * If this [StorageVolume] represents the "Internal Shared Storage" volume, also known as
- * "primary". These volumes are primary volumes, but are also non-removable and emulated.
+ * "primary" to [MediaStore] and Document [Uri]s, obtained in a version compatible manner.
  */
 val StorageVolume.isInternalCompat: Boolean
+    // Must contain the android system AND be an emulated drive, as non-emulated system
+    // volumes use their UUID instead of primary in MediaStore/Document URIs.
     get() = isPrimaryCompat && isEmulatedCompat
 
-/** Returns the UUID of the volume in a compatible manner. */
+/**
+ * The unique identifier for this [StorageVolume], obtained in a version compatible manner
+ * Can be null.
+ * @see StorageVolume.getUuid
+ */
 val StorageVolume.uuidCompat: String?
     @SuppressLint("NewApi") get() = uuid
 
-/** Returns the state of the volume in a compatible manner. */
+/**
+ * The current state of this [StorageVolume], such as "mounted" or "read-only", obtained in
+ * a version compatible manner.
+ * @see StorageVolume.getState
+ */
 val StorageVolume.stateCompat: String
     @SuppressLint("NewApi") get() = state
 
 /**
- * Returns the name of this volume as it is used in [MediaStore]. This will be
+ * Returns the name of this volume that can be used to interact with [MediaStore], in
- * [MediaStore.VOLUME_EXTERNAL_PRIMARY] if it is the primary volume, and the lowercase UUID of the
+ * a version compatible manner. Will be null if the volume is not scanned by [MediaStore].
- * volume otherwise.
+ * @see StorageVolume.getMediaStoreVolumeName
  */
 val StorageVolume.mediaStoreVolumeNameCompat: String?
     get() =
@@ -273,8 +222,8 @@ val StorageVolume.mediaStoreVolumeNameCompat: String?
         } else {
             // Replicate API: primary_external if primary storage, lowercase uuid otherwise
             if (isPrimaryCompat) {
-                @Suppress("NewApi") // Inlined constant
+                // "primary_external" is used in all versions that Auxio supports, is safe to use.
-                MediaStore.VOLUME_EXTERNAL_PRIMARY
+                @Suppress("NewApi") MediaStore.VOLUME_EXTERNAL_PRIMARY
             } else {
                 uuidCompat?.lowercase()
             }

app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt

@@ -34,56 +34,48 @@ import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.Sort
-import org.oxycblt.auxio.music.extractor.Api21MediaStoreExtractor
+import org.oxycblt.auxio.music.extractor.*
-import org.oxycblt.auxio.music.extractor.Api29MediaStoreExtractor
-import org.oxycblt.auxio.music.extractor.Api30MediaStoreExtractor
-import org.oxycblt.auxio.music.extractor.CacheExtractor
-import org.oxycblt.auxio.music.extractor.MetadataExtractor
 import org.oxycblt.auxio.settings.Settings
 import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.logE
 import org.oxycblt.auxio.util.logW
 
 /**
- * Auxio's media indexer.
+ * Core music loading state class.
  *
- * Auxio's media indexer is somewhat complicated, as it has grown to support a variety of use cases
+ * This class provides low-level access into the exact state of the music loading process.
- * (and hacky garbage) in order to produce the best possible experience. It is split into three
+ * **This class should not be used in most cases.** It is highly volatile and provides far
- * distinct steps:
+ * more information than is usually needed. Use [MusicStore] instead if you do not need to
- *
+ * work with the exact music loading state.
- * 1. Creating the chain of extractors to extract metadata with
- * 2. Running the chain process
- * 3. Using the songs to build the library, which primarily involves linking up all data objects
- * with their corresponding parents/children.
- *
- * This class in particular handles 3 primarily. For the code that handles 1 and 2, see the layer
- * implementations.
- *
- * This class also fulfills the role of maintaining the current music loading state, which seems
- * like a job for [MusicStore] but in practice is only really leveraged by the components that
- * directly work with music loading, making such redundant.
  *
  * @author Alexander Capehart (OxygenCobalt)
  */
-class Indexer {
+class Indexer private constructor() {
     private var lastResponse: Response? = null
     private var indexingState: Indexing? = null
-
     private var controller: Controller? = null
     private var callback: Callback? = null
 
     /**
-     * Whether this instance is in an indeterminate state or not, where nothing has been previously
+     * Whether this instance is currently loading music.
-     * loaded, yet no loading is going on.
+      */
+    val isIndexing: Boolean
+        get() = indexingState != null
+
+    /**
+     * Whether this instance has not completed a loading process and is not currently
+     * loading music. This often occurs early in an app's lifecycle, and consumers should
+     * try to avoid showing any state when this flag is true.
      */
     val isIndeterminate: Boolean
         get() = lastResponse == null && indexingState == null
 
-    /** Whether this instance is actively indexing or not. */
+    /**
-    val isIndexing: Boolean
+     * Register a [Controller] for this instance. This instance will handle any commands to start
-        get() = indexingState != null
+     * the music loading process. There can be only one [Controller] at a time. Will invoke all
-
+     * [Callback] methods to initialize the instance with the current state.
-    /** Register a [Controller] with this instance. */
+     * @param controller The [Controller] to register. Will do nothing if already registered.
+     */
     @Synchronized
     fun registerController(controller: Controller) {
         if (BuildConfig.DEBUG && this.controller != null) {
@@ -91,10 +83,19 @@ class Indexer {
             return
         }
 
+        // Initialize the controller with the current state.
+        val currentState =
+            indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) }
+        controller.onIndexerStateChanged(currentState)
         this.controller = controller
     }
 
-    /** Unregister a [Controller] with this instance. */
+    /**
+     * Unregister the [Controller] from this instance, prevent it from recieving any further
+     * commands.
+     * @param controller The [Controller] to unregister. Must be the current [Controller]. Does
+     * nothing if invoked by another [Controller] implementation.
+     */
     @Synchronized
     fun unregisterController(controller: Controller) {
         if (BuildConfig.DEBUG && this.controller !== controller) {
@@ -105,6 +106,12 @@ class Indexer {
         this.controller = null
     }
 
+    /**
+     * Register the [Callback] for this instance. This can be used to receive rapid-fire updates
+     * to the current music loading state. There can be only one [Callback] at a time.
+     * Will invoke all [Callback] methods to initialize the instance with the current state.
+     * @param callback The [Callback] to add.
+     */
     @Synchronized
     fun registerCallback(callback: Callback) {
         if (BuildConfig.DEBUG && this.callback != null) {
@@ -112,14 +119,20 @@ class Indexer {
             return
         }
 
+        // Initialize the callback with the current state.
         val currentState =
             indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) }
-
         callback.onIndexerStateChanged(currentState)
-
         this.callback = callback
     }
 
+    /**
+     * Unregister a [Callback] from this instance, preventing it from recieving any further
+     * updates.
+     * @param callback The [Callback] to unregister. Must be the current [Callback]. Does
+     * nothing if invoked by another [Callback] implementation.
+     * @see Callback
+     */
     @Synchronized
     fun unregisterCallback(callback: Callback) {
         if (BuildConfig.DEBUG && this.callback !== callback) {
@@ -131,16 +144,16 @@ class Indexer {
     }
 
     /**
-     * Start the indexing process. This should be done by [Controller] in a background thread. When
+     * Start the indexing process. This should be done from in the background from [Controller]'s
-     * complete, a new completion state will be pushed to each callback.
+     * context after a command has been received to start the process.
-     * @param withCache Whether to use the cache when loading.
+     * @param context [Context] required to load music.
+     * @param withCache Whether to use the cache or not when loading. If false, the cache will
+     * still be written, but no cache entries will be loaded into the new library.
      */
     suspend fun index(context: Context, withCache: Boolean) {
-        val notGranted =
+        if (ContextCompat.checkSelfPermission(context, PERMISSION_READ_AUDIO) ==
-            ContextCompat.checkSelfPermission(context, PERMISSION_READ_AUDIO) ==
+                PackageManager.PERMISSION_DENIED) {
-                PackageManager.PERMISSION_DENIED
+            // No permissions, signal that we can't do anything.
-
-        if (notGranted) {
             emitCompletion(Response.NoPerms)
             return
         }
@@ -150,19 +163,22 @@ class Indexer {
                 val start = System.currentTimeMillis()
                 val library = indexImpl(context, withCache)
                 if (library != null) {
+                    // Successfully loaded a library.
                     logD(
                         "Music indexing completed successfully in " +
                             "${System.currentTimeMillis() - start}ms")
                     Response.Ok(library)
                 } else {
+                    // Loaded a library, but it contained no music.
                     logE("No music found")
                     Response.NoMusic
                 }
             } catch (e: CancellationException) {
-                // Got cancelled, propagate upwards
+                // Got cancelled, propagate upwards to top-level co-routine.
                 logD("Loading routine was cancelled")
                 throw e
             } catch (e: Exception) {
+                // Music loading process failed due to something we have not handled.
                 logE("Music indexing failed")
                 logE(e.stackTraceToString())
                 Response.Err(e)
@@ -172,9 +188,11 @@ class Indexer {
     }
 
     /**
-     * Request that re-indexing should be done. This should be used by components that do not manage
+     * Request that the music library should be reloaded. This should be used by components that
-     * the indexing process to re-index music.
+     * do not manage the indexing process in order to signal that the [Controller] should call
-     * @param withCache Whether to use the cache when loading music.
+     * [index] eventually.
+     * @param withCache Whether to use the cache when loading music. Does nothing if there is no
+     * [Controller].
      */
     @Synchronized
     fun requestReindex(withCache: Boolean) {
@@ -183,25 +201,31 @@ class Indexer {
     }
 
     /**
-     * "Cancel" the last job by making it unable to send further state updates. This will cause the
+     * Reset the current loading state to signal that the instance is not loading. This should
-     * worker operating the job for that specific handle to cancel as soon as it tries to send a
+     * be called by [Controller] after it's indexing co-routine was cancelled.
-     * state update.
      */
     @Synchronized
-    fun cancelLast() {
+    fun reset() {
         logD("Cancelling last job")
         emitIndexing(null)
     }
 
-    /** Run the proper music loading process. */
+    /**
+     * Internal implementation of the music loading process.
+     * @param context [Context] required to load music.
+     * @param withCache Whether to use the cache or not when loading. If false, the cache will
+     * still be written, but no cache entries will be loaded into the new library.
+     * @return A newly-loaded [MusicStore.Library], or null if nothing was loaded.
+     */
     private suspend fun indexImpl(context: Context, withCache: Boolean): MusicStore.Library? {
         // Create the chain of extractors. Each extractor builds on the previous and
         // enables version-specific features in order to create the best possible music
-        // experience. This is technically dependency injection. Except it doesn't increase
+        // experience.
-        // your compile times by 3x. Isn't that nice.
+        val cacheDatabase = if (withCache) {
-
+            ReadWriteCacheExtractor(context)
-        val cacheDatabase = CacheExtractor(context, !withCache)
+        } else {
-
+            WriteOnlyCacheExtractor(context)
+        }
         val mediaStoreExtractor =
             when {
                 Build.VERSION.SDK_INT >= Build.VERSION_CODES.R ->
@@ -210,118 +234,101 @@ class Indexer {
                     Api29MediaStoreExtractor(context, cacheDatabase)
                 else -> Api21MediaStoreExtractor(context, cacheDatabase)
             }
-
         val metadataExtractor = MetadataExtractor(context, mediaStoreExtractor)
-
         val songs = buildSongs(metadataExtractor, Settings(context))
         if (songs.isEmpty()) {
+            // No songs, nothing else to do.
             return null
         }
 
+        // Build the rest of the music library from the song list. This is much more powerful
+        // and reliable compared to using MediaStore to obtain grouping information.
         val buildStart = System.currentTimeMillis()
-
         val albums = buildAlbums(songs)
         val artists = buildArtists(songs, albums)
         val genres = buildGenres(songs)
-
-        // Make sure we finalize all the items now that they are fully built.
-        for (song in songs) {
-            song._finalize()
-        }
-
-        for (album in albums) {
-            album._finalize()
-        }
-
-        for (artist in artists) {
-            artist._finalize()
-        }
-
-        for (genre in genres) {
-            genre._finalize()
-        }
-
         logD("Successfully built library in ${System.currentTimeMillis() - buildStart}ms")
-
+        return MusicStore.Library(songs, albums, artists, genres)
-        return MusicStore.Library(genres, artists, albums, songs)
     }
 
     /**
-     * Does the initial query over the song database using [metadataExtractor]. The songs returned
+     * Load a list of [Song]s from the device.
-     * by this function are **not** well-formed. The companion [buildAlbums], [buildArtists], and
+     * @param metadataExtractor The completed [MetadataExtractor] instance to use to load
-     * [buildGenres] functions must be called with the returned list so that all songs are properly
+     * [Song.Raw] instances.
-     * linked up.
+     * @param settings [Settings] required to create [Song] instances.
+     * @return A possibly empty list of [Song]s. These [Song]s will be incomplete and
+     * must be linked with parent [Album], [Artist], and [Genre] items in order to be usable.
      */
     private suspend fun buildSongs(
         metadataExtractor: MetadataExtractor,
         settings: Settings
     ): List<Song> {
         logD("Starting indexing process")
-
         val start = System.currentTimeMillis()
+        // Start initializing the extractors. Here, we will signal that we are loading music,
+        // but have no ETA on how far we are.
         emitIndexing(Indexing.Indeterminate)
-
-        // Initialize the extractor chain. This also nets us the projected total
-        // that we can show when loading.
         val total = metadataExtractor.init()
+        // Handle if we were canceled while initializing the extractors.
         yield()
 
-        // Note: We use a set here so we can eliminate effective duplicates of
+        // Note: We use a set here so we can eliminate song duplicates.
-        // songs (by UID) and sort to achieve consistent orderings
         val songs = mutableSetOf<Song>()
         val rawSongs = mutableListOf<Song.Raw>()
-
         metadataExtractor.parse { rawSong ->
             songs.add(Song(rawSong, settings))
             rawSongs.add(rawSong)
-
+            // Handle if we were cancelled while loading a song.
-            // Check if we got cancelled after every song addition.
             yield()
+            // Now we can signal a defined progress by showing how many songs we have
+            // loaded, and the projected amount of songs we found in the library
+            // (obtained by the extractors)
             emitIndexing(Indexing.Songs(songs.size, total))
         }
 
+        // Finalize the extractors with the songs we have no loaded. There is no ETA
+        // on this process, so go back to an indeterminate state.
         emitIndexing(Indexing.Indeterminate)
-
         metadataExtractor.finalize(rawSongs)
-
         logD("Successfully built ${songs.size} songs in ${System.currentTimeMillis() - start}ms")
-
         // Ensure that sorting order is consistent so that grouping is also consistent.
+        // Rolling this into the set is not an option, as songs with the same sort result
+        // would be lost.
         return Sort(Sort.Mode.ByName, true).songs(songs)
     }
 
     /**
-     * Group songs up into their respective albums. Instead of using the unreliable album or artist
+     * Build a list of [Album]s from the given [Song]s.
-     * databases, we instead group up songs by their *lowercase* artist and album name to create
+     * @param songs The [Song]s to build [Album]s from. These will be linked with their
-     * albums. This serves two purposes:
+     * respective [Album]s when created.
-     * 1. Sometimes artist names can be styled differently, e.g "Rammstein" vs. "RAMMSTEIN". This
+     * @return A non-empty list of [Album]s. These [Album]s will be incomplete and
-     * makes sure both of those are resolved into a single artist called "Rammstein"
+     * must be linked with parent [Artist] instances in order to be usable.
-     * 2. Sometimes MediaStore will split album IDs up if the songs differ in format. This ensures
-     * that all songs are unified under a single album.
-     *
-     * This does come with some costs, it's far slower than using the album ID itself, and it may
-     * result in an unrelated album cover being selected depending on the song chosen as the
-     * template, but it seems to work pretty well.
      */
     private fun buildAlbums(songs: List<Song>): List<Album> {
-        val albums = mutableListOf<Album>()
+        // Group songs by their singular raw album, then map the raw instances and their
+        // grouped songs to Album values. Album.Raw will handle the actual grouping rules.
         val songsByAlbum = songs.groupBy { it._rawAlbum }
-
+        val albums = songsByAlbum.map { Album(it.key, it.value) }
-        for (entry in songsByAlbum) {
-            albums.add(Album(entry.key, entry.value))
-        }
-
         logD("Successfully built ${albums.size} albums")
-
         return albums
     }
 
     /**
-     * Group up songs AND albums into artists. This process seems weird (because it is), but the
+     * Group up [Song]s and [Album]s into [Artist] instances. Both of these items are required
-     * purpose is that the actual artist information of albums and songs often differs, and so they
+     * as they group into [Artist] instances much differently, with [Song]s being grouped
-     * are linked in different ways.
+     * primarily by artist names, and [Album]s being grouped primarily by album artist names.
+     * @param songs The [Song]s to build [Artist]s from. One [Song] can result in
+     * the creation of one or more [Artist] instances. These will be linked with their
+     * respective [Artist]s when created.
+     * @param albums The [Album]s to build [Artist]s from. One [Album] can result in
+     * the creation of one or more [Artist] instances. These will be linked with their
+     * respective [Artist]s when created.
+     * @return A non-empty list of [Artist]s. These [Artist]s will consist of the combined
+     * groupings of [Song]s and [Album]s.
      */
     private fun buildArtists(songs: List<Song>, albums: List<Album>): List<Artist> {
+        // Add every raw artist credited to each Song/Album to the grouping. This way,
+        // different multi-artist combinations are not treated as different artists.
         val musicByArtist = mutableMapOf<Artist.Raw, MutableList<Music>>()
         for (song in songs) {
             for (rawArtist in song._rawArtists) {
@@ -335,19 +342,22 @@ class Indexer {
             }
         }
 
+        // Convert the combined mapping into artist instances.
         val artists = musicByArtist.map { Artist(it.key, it.value) }
-
         logD("Successfully built ${artists.size} artists")
-
         return artists
     }
 
     /**
-     * Group up songs into genres. This is a relatively simple step compared to the other library
+     * Group up [Song]s into [Genre] instances.
-     * steps, as there is no demand to deduplicate genres by a lowercase name.
+     * @param [songs] The [Song]s to build [Genre]s from. One [Song] can result in
+     * the creation of one or more [Genre] instances. These will be linked with their
+     * respective [Genre]s when created.
+     * @return A non-empty list of [Genre]s.
      */
     private fun buildGenres(songs: List<Song>): List<Genre> {
-        val genres = mutableListOf<Genre>()
+        // Add every raw genre credited to each Song to the grouping. This way,
+        // different multi-genre combinations are not treated as different genres.
         val songsByGenre = mutableMapOf<Genre.Raw, MutableList<Song>>()
         for (song in songs) {
             for (rawGenre in song._rawGenres) {
@@ -355,31 +365,40 @@ class Indexer {
             }
         }
 
-        for (entry in songsByGenre) {
+        // Convert the mapping into genre instances.
-            genres.add(Genre(entry.key, entry.value))
+        val genres = songsByGenre.map { Genre(it.key, it.value) }
-        }
-
         logD("Successfully built ${genres.size} genres")
-
         return genres
     }
 
+    /**
+     * Emit a new [State.Indexing] state. This can be used to signal the current state of the music
+     * loading process to external code. Assumes that the callee has already checked if they
+     * have not been canceled and thus have the ability to emit a new state.
+     * @param indexing The new [Indexing] state to emit, or null if no loading process is occurring.
+     */
     @Synchronized
     private fun emitIndexing(indexing: Indexing?) {
         indexingState = indexing
-
         // If we have canceled the loading process, we want to revert to a previous completion
         // whenever possible to prevent state inconsistency.
         val state =
             indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) }
-
         controller?.onIndexerStateChanged(state)
         callback?.onIndexerStateChanged(state)
     }
 
+    /**
+     * Emit a new [State.Complete] state. This can be used to signal the completion of the music
+     * loading process to external code. Will check if the callee has not been canceled and thus
+     * has the ability to emit a new state
+     * @param response The new [Response] to emit, representing the outcome of the music loading
+     * process.
+     */
     private suspend fun emitCompletion(response: Response) {
+        // Handle if this co-routine was canceled in the period between the last loading state
+        // and this completion state.
         yield()
-
         // Swap to the Main thread so that downstream callbacks don't crash from being on
         // a background thread. Does not occur in emitIndexing due to efficiency reasons.
         withContext(Dispatchers.Main) {
@@ -388,39 +407,85 @@ class Indexer {
                 // from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
                 lastResponse = response
                 indexingState = null
-
+                // Signal that the music loading process has been completed.
                 val state = State.Complete(response)
-
                 controller?.onIndexerStateChanged(state)
                 callback?.onIndexerStateChanged(state)
             }
         }
     }
 
-    /** Represents the current indexer state. */
+    /**
+     * Represents the current state of the music loading process.
+     */
     sealed class State {
+        /**
+         * Music loading is ongoing.
+         * @param indexing The current music loading progress..
+         * @see Indexer.Indexing
+         */
         data class Indexing(val indexing: Indexer.Indexing) : State()
+
+        /**
+         * Music loading has completed.
+         * @param response The outcome of the music loading process.
+         * @see Response
+         */
         data class Complete(val response: Response) : State()
     }
 
+    /**
+     * The current progress of the music loader. Usually encapsulated in a [State].
+     * @see State.Indexing
+     */
     sealed class Indexing {
+        /**
+         * Music loading is occurring, but no definite estimate can be put on the current
+         * progress.
+         */
         object Indeterminate : Indexing()
+
+        /**
+         * Music loading has a definite progress.
+         * @param current The current amount of songs that have been loaded.
+         * @param total The projected total amount of songs that will be loaded.
+         */
         class Songs(val current: Int, val total: Int) : Indexing()
     }
 
-    /** Represents the possible outcomes of a loading process. */
+    /**
+     * The possible outcomes of the music loading process.
+     */
     sealed class Response {
+        /**
+         * Music load was successful and produced a [MusicStore.Library].
+         * @param library The loaded [MusicStore.Library].
+         */
         data class Ok(val library: MusicStore.Library) : Response()
+
+        /**
+         * Music loading encountered an unexpected error.
+         * @param throwable The error thrown.
+         */
         data class Err(val throwable: Throwable) : Response()
+
+        /**
+         * Music loading occurred, but resulted in no music.
+         */
         object NoMusic : Response()
+
+        /**
+         * Music loading could not occur due to a lack of storage permissions.
+         */
         object NoPerms : Response()
     }
 
     /**
-     * A callback to use when the indexing state changes.
+     * A callback for rapid-fire changes in the music loading state.
      *
-     * This callback is low-level and not guaranteed to be single-thread. For that,
+     * This is only useful for code that absolutely must show the current loading process.
-     * [MusicStore.Callback] is recommended instead.
+     * Otherwise, [MusicStore.Callback] is highly recommended due to it's updates only
+     * consisting of the [MusicStore.Library].
      */
     interface Callback {
         /**
@@ -434,13 +499,29 @@ class Indexer {
         fun onIndexerStateChanged(state: State?)
     }
 
+    /**
+     * Context that runs the music loading process. Implementations should be capable of
+     * running the background for long periods of time without android killing the process.
+     */
     interface Controller : Callback {
+        /**
+         * Called when a new music loading process was requested. Implementations should
+         * forward this to [index].
+         * @param withCache Whether to use the cache or not when loading. If false, the cache should
+         * still be written, but no cache entries will be loaded into the new library.
+         * @see index
+         */
         fun onStartIndexing(withCache: Boolean)
     }
 
     companion object {
         @Volatile private var INSTANCE: Indexer? = null
 
+        /**
+         * A version-compatible identifier for the read external storage permission required
+         * by the system to load audio.
+         * TODO: Move elsewhere.
+         */
         val PERMISSION_READ_AUDIO =
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
                 // READ_EXTERNAL_STORAGE was superseded by READ_MEDIA_AUDIO in Android 13
@@ -449,10 +530,12 @@ class Indexer {
                 Manifest.permission.READ_EXTERNAL_STORAGE
             }
 
-        /** Get the process-level instance of [Indexer]. */
+        /**
+         * Get a singleton instance.
+         * @return The (possibly newly-created) singleton instance.
+         */
         fun getInstance(): Indexer {
             val currentInstance = INSTANCE
-
             if (currentInstance != null) {
                 return currentInstance
             }

app/src/main/java/org/oxycblt/auxio/music/system/IndexerNotifications.kt

@@ -27,7 +27,11 @@ import org.oxycblt.auxio.shared.ServiceNotification
 import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.newMainPendingIntent
 
-/** The notification responsible for showing the indexer state. */
+/**
+ * A dynamic [ServiceNotification] that shows the current music loading state.
+ * @param context [Context] required to create the notification.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
 class IndexingNotification(private val context: Context) :
     ServiceNotification(context, INDEXER_CHANNEL) {
     private var lastUpdateTime = -1L
@@ -47,9 +51,17 @@ class IndexingNotification(private val context: Context) :
     override val code: Int
         get() = IntegerTable.INDEXER_NOTIFICATION_CODE
 
+    /**
+     * Update this notification with the new music loading state.
+     * @param indexing The new music loading state to display in the notification.
+     * @return true if the notification updated, false otherwise
+     */
     fun updateIndexingState(indexing: Indexer.Indexing): Boolean {
         when (indexing) {
             is Indexer.Indexing.Indeterminate -> {
+                // Indeterminate state, use a vaguer description and in-determinate progress.
+                // These events are not very frequent, and thus we don't need to safeguard
+                // against rate limiting.
                 logD("Updating state to $indexing")
                 lastUpdateTime = -1
                 setContentText(context.getString(R.string.lng_indexing))
@@ -57,14 +69,15 @@ class IndexingNotification(private val context: Context) :
                 return true
             }
             is Indexer.Indexing.Songs -> {
+                // Determinate state, show an active progress meter. Since these updates arrive
+                // highly rapidly, only update every 1.5 seconds to prevent notification rate
+                // limiting.
+                // TODO: Can I port this to the playback notification somehow?
                 val now = SystemClock.elapsedRealtime()
                 if (lastUpdateTime > -1 && (now - lastUpdateTime) < 1500) {
                     return false
                 }
-
                 lastUpdateTime = SystemClock.elapsedRealtime()
-
-                // Only update the notification every 1.5s to prevent rate-limiting.
                 logD("Updating state to $indexing")
                 setContentText(
                     context.getString(R.string.fmt_indexing, indexing.current, indexing.total))
@@ -75,7 +88,11 @@ class IndexingNotification(private val context: Context) :
     }
 }
 
-/** The notification responsible for showing the indexer state. */
+/**
+ * A static [ServiceNotification] that signals to the user that the app is currently monitoring
+ * the music library for changes.
+ * @author Alexander Capehart (OxygenCobalt)
+ */
 class ObservingNotification(context: Context) : ServiceNotification(context, INDEXER_CHANNEL) {
     init {
         setSmallIcon(R.drawable.ic_indexer_24)
@@ -92,6 +109,9 @@ class ObservingNotification(context: Context) : ServiceNotification(context, IND
         get() = IntegerTable.INDEXER_NOTIFICATION_CODE
 }
 
+/**
+ * Shared channel that [IndexingNotification] and [ObservingNotification] post to.
+ */
 private val INDEXER_CHANNEL =
     ServiceNotification.ChannelInfo(
         id = BuildConfig.APPLICATION_ID + ".channel.INDEXER", nameRes = R.string.lbl_indexer)

app/src/main/java/org/oxycblt/auxio/music/system/IndexerService.kt

@@ -41,50 +41,50 @@ import org.oxycblt.auxio.util.getSystemServiceCompat
 import org.oxycblt.auxio.util.logD
 
 /**
- * A [Service] that handles the music loading process.
+ * A [Service] that manages the background music loading process.
  *
- * Loading music is actually somewhat time-consuming, to the point where it's likely better suited
+ * Loading music is a time-consuming process that would likely be killed by the system before
- * to a service that is less likely to be killed by the OS.
+ * it could complete if ran anywhere else. So, this [Service] manages the music loading process
+ * as an instance of [Indexer.Controller].
  *
- * You could probably do the same using WorkManager and the GooberQueue library or whatever, but the
+ * This [Service] also handles automatic rescanning, as that is a similarly long-running
- * boilerplate you skip is not worth the insanity of androidx.
+ * background operation that would be unsuitable elsewhere in the app.
+ *
+ * TODO: Unify with PlaybackService as part of the service independence project
  *
  * @author Alexander Capehart (OxygenCobalt)
  */
 class IndexerService : Service(), Indexer.Controller, Settings.Callback {
     private val indexer = Indexer.getInstance()
     private val musicStore = MusicStore.getInstance()
-
+    private val playbackManager = PlaybackStateManager.getInstance()
     private val serviceJob = Job()
     private val indexScope = CoroutineScope(serviceJob + Dispatchers.IO)
     private var currentIndexJob: Job? = null
-
-    private val playbackManager = PlaybackStateManager.getInstance()
-
     private lateinit var foregroundManager: ForegroundManager
     private lateinit var indexingNotification: IndexingNotification
     private lateinit var observingNotification: ObservingNotification
-
-    private lateinit var settings: Settings
     private lateinit var wakeLock: PowerManager.WakeLock
     private lateinit var indexerContentObserver: SystemContentObserver
+    private lateinit var settings: Settings
 
     override fun onCreate() {
         super.onCreate()
-
+        // Initialize the core service components first.
         foregroundManager = ForegroundManager(this)
         indexingNotification = IndexingNotification(this)
         observingNotification = ObservingNotification(this)
-
         wakeLock =
             getSystemServiceCompat(PowerManager::class)
                 .newWakeLock(
                     PowerManager.PARTIAL_WAKE_LOCK, BuildConfig.APPLICATION_ID + ":IndexerService")
-
+        // Initialize any callback-dependent components last as we wouldn't want a callback race
-        settings = Settings(this, this)
+        // condition to cause us to load music before we were fully initialize.
         indexerContentObserver = SystemContentObserver()
-
+        settings = Settings(this, this)
         indexer.registerController(this)
+        // An indeterminate indexer and a missing library implies we are extremely early
+        // in app initialization so start loading music.
         if (musicStore.library == null && indexer.isIndeterminate) {
             logD("No library present and no previous response, indexing music now")
             onStartIndexing(true)
@@ -99,29 +99,30 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
 
     override fun onDestroy() {
         super.onDestroy()
-
+        // De-initialize core service components first.
         foregroundManager.release()
         wakeLock.releaseSafe()
-
+        // Then cancel the callback-dependent components to ensure that stray reloading
-        // De-initialize the components first to prevent stray reloading events
+        // events will not occur.
-        settings.release()
         indexerContentObserver.release()
+        settings.release()
         indexer.unregisterController(this)
-
+        // Then cancel any remaining music loading jobs.
-        // Then cancel the other components.
-        indexer.cancelLast()
         serviceJob.cancel()
+        indexer.reset()
     }
 
     // --- CONTROLLER CALLBACKS ---
 
     override fun onStartIndexing(withCache: Boolean) {
         if (indexer.isIndexing) {
+            // Cancel the previous music loading job.
             currentIndexJob?.cancel()
-            indexer.cancelLast()
+            indexer.reset()
         }
-
+        // Start a new music loading job on a co-routine.
-        currentIndexJob = indexScope.launch { indexer.index(this@IndexerService, withCache) }
+        currentIndexJob = indexScope.launch {
+            indexer.index(this@IndexerService, withCache) }
     }
 
     override fun onIndexerStateChanged(state: Indexer.State?) {
@@ -130,28 +131,23 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
                 if (state.response is Indexer.Response.Ok &&
                     state.response.library != musicStore.library) {
                     logD("Applying new library")
-
                     val newLibrary = state.response.library
-
+                    // We only care if the newly-loaded library is going to replace a previously
+                    // loaded library.
                     if (musicStore.library != null) {
-                        // This is a new library to replace an existing one.
+                        // Wipe possibly-invalidated outdated covers
-
-                        // Wipe possibly-invalidated album covers
                         imageLoader.memoryCache?.clear()
-
                         // Clear invalid models from PlaybackStateManager. This is not connected
                         // to a callback as it is bad practice for a shared object to attach to
                         // the callback system of another.
                         playbackManager.sanitize(newLibrary)
                     }
-
+                    // Forward the new library to MusicStore to continue the update process.
-                    musicStore.updateLibrary(newLibrary)
+                    musicStore.library = newLibrary
                 }
-
                 // On errors, while we would want to show a notification that displays the
-                // error, in practice that comes into conflict with the upcoming Android 13
+                // error, that requires the Android 13 notification permission, which is not
-                // notification permission, and there is no point implementing permission
+                // handled right now.
-                // on-boarding for such when it will only be used for this.
                 updateIdleSession()
             }
             is Indexer.State.Indexing -> {
@@ -178,7 +174,6 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
             logD("Notification changed, re-posting notification")
             indexingNotification.post()
         }
-
         // Make sure we can keep the CPU on while loading music
         wakeLock.acquireSafe()
     }
@@ -191,27 +186,32 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
             // we can go foreground later.
             // 2. If a non-foreground service is killed, the app will probably still be alive,
             // and thus the music library will not be updated at all.
+            // TODO: Assuming I unify this with PlaybackService, it's possible that I won't need
+            //  this anymore.
             if (!foregroundManager.tryStartForeground(observingNotification)) {
                 observingNotification.post()
             }
         } else {
+            // Not observing and done loading, exit foreground.
             foregroundManager.tryStopForeground()
         }
-
         // Release our wake lock (if we were using it)
         wakeLock.releaseSafe()
     }
 
     private fun PowerManager.WakeLock.acquireSafe() {
+        // Avoid unnecessary acquire calls.
         if (!wakeLock.isHeld) {
             logD("Acquiring wake lock")
-
+            // Time out after a minute, which is the average music loading time for a medium-sized
-            // We always drop the wakelock eventually. Timeout is not needed.
+            // library. If this runs out, we will re-request the lock, and if music loading is
-            @Suppress("WakelockTimeout") acquire()
+            // shorter than the timeout, it will be released early.
+            acquire(WAKELOCK_TIMEOUT_MS)
         }
     }
 
     private fun PowerManager.WakeLock.releaseSafe() {
+        // Avoid unnecessary release calls.
         if (wakeLock.isHeld) {
             logD("Releasing wake lock")
             release()
@@ -222,11 +222,16 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
 
     override fun onSettingChanged(key: String) {
         when (key) {
+            // Hook changes in music settings to a new music loading event.
             getString(R.string.set_key_exclude_non_music),
             getString(R.string.set_key_music_dirs),
             getString(R.string.set_key_music_dirs_include),
             getString(R.string.set_key_separators) -> onStartIndexing(true)
             getString(R.string.set_key_observing) -> {
+                // Make sure we don't override the service state with the observing
+                // notification if we were actively loading when the automatic rescanning
+                // setting changed. In such a case, the state will still be updated when
+                // the music loading process ends.
                 if (!indexer.isIndexing) {
                     updateIdleSession()
                 }
@@ -234,23 +239,32 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
         }
     }
 
-    /** Internal content observer intended to work with the automatic reloading system. */
+    /**
-    private inner class SystemContentObserver(
+     * A [ContentObserver] that observes the [MediaStore] music database for changes, a behavior
-        private val handler: Handler = Handler(Looper.getMainLooper())
+     * known to the user as automatic rescanning. The active (and not passive) nature of observing
-    ) : ContentObserver(handler), Runnable {
+     * the database is what requires [IndexerService] to stay foreground when this is enabled.
+     */
+    private inner class SystemContentObserver : ContentObserver(Handler(Looper.getMainLooper())), Runnable {
+        private val handler = Handler(Looper.getMainLooper())
+
         init {
             contentResolverSafe.registerContentObserver(
                 MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, true, this)
         }
 
+        /**
+         * Release this instance, preventing it from further observing the database and cancelling
+         * any pending update events.
+         */
         fun release() {
+            handler.removeCallbacks(this)
             contentResolverSafe.unregisterContentObserver(this)
         }
 
         override fun onChange(selfChange: Boolean) {
             // Batch rapid-fire updates to the library into a single call to run after 500ms
             handler.removeCallbacks(this)
-            handler.postDelayed(this, REINDEX_DELAY)
+            handler.postDelayed(this, REINDEX_DELAY_MS)
         }
 
         override fun run() {
@@ -263,6 +277,16 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
     }
 
     companion object {
-        const val REINDEX_DELAY = 500L
+        /**
+         * The amount of time to hold the wake lock when loading music, in milliseconds.
+         * Equivalent to one minute.
+         */
+        private const val WAKELOCK_TIMEOUT_MS = 60 * 1000L
+
+        /**
+         * The amount of time to wait between a change in the music library and to start
+         * the music loading process, in milliseconds. Equivalent to half a second.
+         */
+        private const val REINDEX_DELAY_MS = 500L
     }
 }

app/src/main/java/org/oxycblt/auxio/search/SearchFragment.kt

@@ -32,7 +32,6 @@ import org.oxycblt.auxio.R
 import org.oxycblt.auxio.databinding.FragmentSearchBinding
 import org.oxycblt.auxio.list.Item
 import org.oxycblt.auxio.list.ListFragment
-import org.oxycblt.auxio.list.selection.SelectionToolbarOverlay
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre

app/src/main/java/org/oxycblt/auxio/settings/Settings.kt

@@ -31,7 +31,7 @@ import org.oxycblt.auxio.image.CoverMode
 import org.oxycblt.auxio.music.MusicMode
 import org.oxycblt.auxio.music.Sort
 import org.oxycblt.auxio.music.storage.Directory
-import org.oxycblt.auxio.music.storage.MusicDirs
+import org.oxycblt.auxio.music.storage.MusicDirectories
 import org.oxycblt.auxio.playback.ActionMode
 import org.oxycblt.auxio.playback.replaygain.ReplayGainMode
 import org.oxycblt.auxio.playback.replaygain.ReplayGainPreAmp
@@ -196,12 +196,12 @@ class Settings(private val context: Context, private val callback: Callback? = n
     /** The current library tabs preferred by the user. */
     var libTabs: Array<Tab>
         get() =
-            Tab.fromSequence(
+            Tab.fromIntCode(
                 inner.getInt(context.getString(R.string.set_key_lib_tabs), Tab.SEQUENCE_DEFAULT))
-                ?: unlikelyToBeNull(Tab.fromSequence(Tab.SEQUENCE_DEFAULT))
+                ?: unlikelyToBeNull(Tab.fromIntCode(Tab.SEQUENCE_DEFAULT))
         set(value) {
             inner.edit {
-                putInt(context.getString(R.string.set_key_lib_tabs), Tab.toSequence(value))
+                putInt(context.getString(R.string.set_key_lib_tabs), Tab.toIntCode(value))
                 apply()
             }
         }
@@ -256,7 +256,7 @@ class Settings(private val context: Context, private val callback: Callback? = n
     /** What queue to create when a song is selected from the library or search */
     val libPlaybackMode: MusicMode
         get() =
-            MusicMode.fromInt(
+            MusicMode.fromIntCode(
                 inner.getInt(
                     context.getString(R.string.set_key_library_song_playback_mode), Int.MIN_VALUE))
                 ?: MusicMode.SONGS
@@ -267,7 +267,7 @@ class Settings(private val context: Context, private val callback: Callback? = n
      */
     val detailPlaybackMode: MusicMode?
         get() =
-            MusicMode.fromInt(
+            MusicMode.fromIntCode(
                 inner.getInt(
                     context.getString(R.string.set_key_detail_song_playback_mode), Int.MIN_VALUE))
 
@@ -302,21 +302,21 @@ class Settings(private val context: Context, private val callback: Callback? = n
         get() = inner.getBoolean(context.getString(R.string.set_key_exclude_non_music), true)
 
     /** Get the list of directories that music should be hidden/loaded from. */
-    fun getMusicDirs(storageManager: StorageManager): MusicDirs {
+    fun getMusicDirs(storageManager: StorageManager): MusicDirectories {
         val dirs =
             (inner.getStringSet(context.getString(R.string.set_key_music_dirs), null) ?: emptySet())
-                .mapNotNull { Directory.fromDocumentUri(storageManager, it) }
+                .mapNotNull { Directory.fromDocumentTreeUri(storageManager, it) }
 
-        return MusicDirs(
+        return MusicDirectories(
             dirs, inner.getBoolean(context.getString(R.string.set_key_music_dirs_include), false))
     }
 
     /** Set the list of directories that music should be hidden/loaded from. */
-    fun setMusicDirs(musicDirs: MusicDirs) {
+    fun setMusicDirs(musicDirs: MusicDirectories) {
         inner.edit {
             putStringSet(
                 context.getString(R.string.set_key_music_dirs),
-                musicDirs.dirs.map(Directory::toDocumentUri).toSet())
+                musicDirs.dirs.map(Directory::toDocumentTreeUri).toSet())
             putBoolean(
                 context.getString(R.string.set_key_music_dirs_include), musicDirs.shouldInclude)
             apply()
@@ -339,7 +339,7 @@ class Settings(private val context: Context, private val callback: Callback? = n
     /** The current filter mode of the search tab */
     var searchFilterMode: MusicMode?
         get() =
-            MusicMode.fromInt(
+            MusicMode.fromIntCode(
                 inner.getInt(context.getString(R.string.set_key_search_filter), Int.MIN_VALUE))
         set(value) {
             inner.edit {

app/src/main/java/org/oxycblt/auxio/settings/prefs/PreferenceFragment.kt

@@ -139,8 +139,8 @@ class PreferenceFragment : PreferenceFragmentCompat() {
                         this.context?.showToast(R.string.err_did_not_restore)
                     }
                 }
-            context.getString(R.string.set_key_reindex) -> musicModel.reindex(true)
+            context.getString(R.string.set_key_reindex) -> musicModel.refresh()
-            context.getString(R.string.set_key_rescan) -> musicModel.reindex(false)
+            context.getString(R.string.set_key_rescan) -> musicModel.rescan()
             else -> return super.onPreferenceTreeClick(preference)
         }