music: dont make path parsing redundant

Don't do API 21 path parsing on API 29.

There's no need yet, so it's better to skip the step and make the
loading process more efficient.

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 #### What's New
 - Excluded directories has been revampled into "Music folders"
-    - Folders on external drives can now be excluded on Android Q+ [#134]
+    - Folders on external drives can now be excluded [#134]
     - Added new "Include" option to restrict indexing to a particular folder [#154]
 - Added a new view for song properties (Such as Bitrate)
 - The playback bar now has a new design, with an improved progress indicator and a
@@ -29,6 +29,7 @@
 - Moved music loading to a foreground service
 - Phased out `ImageButton` for `MaterialButton`
 - Unified icon sizing
+- Properly handle volumes throughout the entire music loading process
 - Added original date support to ExoPlayer parser (Not exposed in app)
 
 ## v2.3.1

app/src/main/java/org/oxycblt/auxio/music/backend/MediaStoreBackend.kt

@@ -123,13 +123,12 @@ abstract class MediaStoreBackend : Indexer.Backend {
     private var albumArtistIndex = -1
     private var dataIndex = -1
 
-    private val _volumes = mutableListOf<StorageVolume>()
+    protected val volumes = mutableListOf<StorageVolume>()
-    protected val volumes = _volumes
 
     override fun query(context: Context): Cursor {
         val settingsManager = SettingsManager.getInstance()
         val storageManager = context.getSystemServiceSafe(StorageManager::class)
-        _volumes.addAll(storageManager.storageVolumesCompat)
+        volumes.addAll(storageManager.storageVolumesCompat)
         val dirs = settingsManager.getMusicDirs(context, storageManager)
 
         val args = mutableListOf<String>()
@@ -137,13 +136,14 @@ abstract class MediaStoreBackend : Indexer.Backend {
 
         if (dirs.dirs.isNotEmpty()) {
             // We have directories we need to exclude, extend the selector with new arguments
-            selector += if (dirs.shouldInclude) {
+            selector +=
-                logD("Need to select folders (Include)")
+                if (dirs.shouldInclude) {
-                " AND ("
+                    logD("Need to select folders (Include)")
-            } else {
+                    " AND ("
-                logD("Need to select folders (Exclude)")
+                } else {
-                " AND NOT ("
+                    logD("Need to select folders (Exclude)")
-            }
+                    " AND NOT ("
+                }
 
             // Since selector arguments are contained within a single parentheses, we need to
             // do a bunch of stuff.
@@ -264,7 +264,7 @@ abstract class MediaStoreBackend : Indexer.Backend {
         // from the android system. Once again though, OEM issues get in our way and
         // this field isn't available on some platforms. In that case, we have to rely
         // on DATA to get a reasonable file name.
-        audio.displayName = cursor.getStringOrNull(displayNameIndex)
+        // audio.displayName = cursor.getStringOrNull(displayNameIndex)
 
         audio.duration = cursor.getLong(durationIndex)
         audio.year = cursor.getIntOrNull(yearIndex)
@@ -386,9 +386,54 @@ abstract class MediaStoreBackend : Indexer.Backend {
          * The base selector that works across all versions of android. Does not exclude
          * directories.
          */
-        @JvmStatic
+        private const val BASE_SELECTOR =
-        protected val BASE_SELECTOR =
+            "${MediaStore.Audio.Media.IS_MUSIC}=1 " + "AND NOT ${MediaStore.Audio.Media.SIZE}=0"
-            "${MediaStore.Audio.Media.IS_MUSIC}=1 AND NOT ${MediaStore.Audio.Media.SIZE}=0"
+    }
+}
+
+/**
+ * Implements shared aspects of both [Api21MediaStoreBackend] and [Api29MediaStoreBackend]. Done so
+ * that each impl can avoid redundant jobs regarding path parsing.
+ * @author OxygenCobalt
+ */
+abstract class BaseApi21MediaStoreBackend : MediaStoreBackend() {
+    private var trackIndex = -1
+    private var dataIndex = -1
+
+    override val projection: Array<String>
+        get() =
+            super.projection +
+                arrayOf(MediaStore.Audio.AudioColumns.TRACK, MediaStore.Audio.AudioColumns.DATA)
+
+    override fun buildAudio(context: Context, cursor: Cursor): Audio {
+        val audio = super.buildAudio(context, cursor)
+
+        // Initialize our indices if we have not already.
+        if (trackIndex == -1) {
+            trackIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.TRACK)
+            dataIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.DATA)
+        }
+
+        // TRACK is formatted as DTTT where D is the disc number and T is the track number.
+        // Except on Android 10. For some reason it's bugged on that version.
+        val rawTrack = cursor.getIntOrNull(trackIndex)
+        if (rawTrack != null) {
+            audio.track = rawTrack % 1000
+
+            // A disc number of 0 means that there is no disc.
+            val disc = rawTrack / 1000
+            if (disc > 0) {
+                audio.disc = disc
+            }
+        }
+
+        // Fill in DISPLAY_NAME with data if not present
+        val data = cursor.getStringOrNull(dataIndex)
+        if (data != null && audio.displayName == null) {
+            audio.displayName = data.substringAfterLast(File.separatorChar, "").ifEmpty { null }
+        }
+
+        return audio
     }
 }
 
@@ -396,8 +441,7 @@ abstract class MediaStoreBackend : Indexer.Backend {
  * A [MediaStoreBackend] that completes the music loading process in a way compatible from
  * @author OxygenCobalt
  */
-open class Api21MediaStoreBackend : MediaStoreBackend() {
+class Api21MediaStoreBackend : BaseApi21MediaStoreBackend() {
-    private var trackIndex = -1
     private var dataIndex = -1
 
     override val projection: Array<String>
@@ -417,35 +461,15 @@ open class Api21MediaStoreBackend : MediaStoreBackend() {
         val audio = super.buildAudio(context, cursor)
 
         // Initialize our indices if we have not already.
-        if (trackIndex == -1) {
+        if (dataIndex == -1) {
-            trackIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.TRACK)
             dataIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.DATA)
         }
 
-        // TRACK is formatted as DTTT where D is the disc number and T is the track number.
-        // Except on Android 10. For some reason it's bugged on that version.
-
-        val rawTrack = cursor.getIntOrNull(trackIndex)
-        if (rawTrack != null) {
-            audio.track = rawTrack % 1000
-
-            // A disc number of 0 means that there is no disc.
-            val disc = rawTrack / 1000
-            if (disc > 0) {
-                audio.disc = disc
-            }
-        }
-
         val data = cursor.getStringOrNull(dataIndex)
         if (data != null) {
-            if (audio.displayName == null) {
-                audio.displayName = data.substringAfterLast(File.separatorChar, "").ifEmpty { null }
-            }
-
-            val rawPath = data.substringBeforeLast(File.separatorChar)
-
             // Find the volume that transforms the DATA field into a relative path. This is
             // the volume and relative path we will use.
+            val rawPath = data.substringBeforeLast(File.separatorChar)
             for (volume in volumes) {
                 val volumePath = volume.directoryCompat ?: continue
                 val strippedPath = rawPath.removePrefix(volumePath)
@@ -466,7 +490,7 @@ open class Api21MediaStoreBackend : MediaStoreBackend() {
  * @author OxygenCobalt
  */
 @RequiresApi(Build.VERSION_CODES.Q)
-open class Api29MediaStoreBackend : Api21MediaStoreBackend() {
+open class Api29MediaStoreBackend : BaseApi21MediaStoreBackend() {
     private var volumeIndex = -1
     private var relativePathIndex = -1
 
@@ -483,7 +507,8 @@ open class Api29MediaStoreBackend : Api21MediaStoreBackend() {
                 "AND ${MediaStore.Audio.AudioColumns.RELATIVE_PATH} LIKE ?)"
 
     override fun addDirToSelectorArgs(dir: Directory, args: MutableList<String>): Boolean {
-        // Leverage the volume field when selecting our directories.
+        // Leverage the volume field when selecting our directories. It's a little too
+        // expensive to include this alongside the data checks, so we assume that
         args.add(dir.volume.mediaStoreVolumeNameCompat ?: return false)
         args.add("${dir.relativePath}%")
         return true
@@ -505,7 +530,8 @@ open class Api29MediaStoreBackend : Api21MediaStoreBackend() {
         // I have no idea how well this works in practice, so we still leverage
         // the API 21 path grokking in the case that these fields are not sane.
         if (volumeName != null && relativePath != null) {
-            // Iterating through the volume list is easier t
+            // Iterating through the volume list is cheaper than creating a map,
+            // interestingly enough.
             val volume = volumes.find { it.mediaStoreVolumeNameCompat == volumeName }
             if (volume != null) {
                 audio.dir = Directory(volume, relativePath.removeSuffix(File.separator))

app/src/main/res/values/donottranslate.xml

@@ -7,14 +7,10 @@
     <string name="fmt_two" translatable="false">%1$s • %2$s</string>
     <string name="fmt_three" translatable="false">%1$s • %2$s • %3$s</string>
     <string name="fmt_number" translatable="false">%d</string>
+    <string name="fmt_path">%1$s/%2$s</string>
 
     <!-- Codec Namespace | Format names -->
     <string name="cdc_vorbis">Vorbis</string>
     <string name="cdc_opus">Opus</string>
     <string name="cdc_wav">Microsoft WAVE</string>
-
-    <!-- Note: These are stopgap measures until we make the path code rely on components! -->
-    <string name="fmt_path">%1$s/%2$s</string>
-    <string name="fmt_primary_path">Internal:%s</string>
-    <string name="fmt_secondary_path">SDCARD:%s</string>
 </resources>