music: improve indexing progress

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

@@ -58,7 +58,6 @@ import org.oxycblt.auxio.home.tabs.Tab
 import org.oxycblt.auxio.list.ListViewModel
 import org.oxycblt.auxio.list.SelectionFragment
 import org.oxycblt.auxio.list.menu.Menu
-import org.oxycblt.auxio.music.IndexingProgress
 import org.oxycblt.auxio.music.IndexingState
 import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.MusicType
@@ -70,6 +69,7 @@ import org.oxycblt.auxio.music.Playlist
 import org.oxycblt.auxio.music.PlaylistDecision
 import org.oxycblt.auxio.music.PlaylistMessage
 import org.oxycblt.auxio.music.external.M3U
+import org.oxycblt.auxio.music.stack.IndexingProgress
 import org.oxycblt.auxio.playback.PlaybackDecision
 import org.oxycblt.auxio.playback.PlaybackViewModel
 import org.oxycblt.auxio.util.collect
@@ -384,19 +384,12 @@ class HomeFragment :
         binding.homeIndexingActions.visibility = View.INVISIBLE
 
         binding.homeIndexingStatus.setText(R.string.lng_indexing)
-        when (progress) {
-            is IndexingProgress.Indeterminate -> {
-                // In a query/initialization state, show a generic loading status.
-                binding.homeIndexingProgress.isIndeterminate = true
-            }
-            is IndexingProgress.Songs -> {
-                // Actively loading songs, show the current progress.
-                binding.homeIndexingProgress.apply {
-                    isIndeterminate = false
-                    max = progress.total
-                    this.progress = progress.current
-                }
-            }
+
+        // Actively loading songs, show the current progress.
+        binding.homeIndexingProgress.apply {
+            isIndeterminate = false
+            max = progress.explored
+            this.progress = progress.interpreted
         }
     }
 

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

@@ -19,6 +19,7 @@
 package org.oxycblt.auxio.music
 
 import android.os.Build
+import org.oxycblt.auxio.music.stack.IndexingProgress
 
 /** Version-aware permission identifier for reading audio files. */
 val PERMISSION_READ_AUDIO =
@@ -50,24 +51,6 @@ sealed interface IndexingState {
     data class Completed(val error: Exception?) : IndexingState
 }
 
-/**
- * Represents the current progress of music loading.
- *
- * @author Alexander Capehart (OxygenCobalt)
- */
-sealed interface IndexingProgress {
-    /** Other work is being done that does not have a defined progress. */
-    data object Indeterminate : IndexingProgress
-
-    /**
-     * Songs are currently being loaded.
-     *
-     * @param current The current amount of songs loaded.
-     * @param total The projected total amount of songs.
-     */
-    data class Songs(val current: Int, val total: Int) : IndexingProgress
-}
-
 /**
  * Thrown by the music loader when [PERMISSION_READ_AUDIO] was not granted.
  *

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

@@ -32,6 +32,7 @@ import kotlinx.coroutines.yield
 import org.oxycblt.auxio.music.info.Name
 import org.oxycblt.auxio.music.metadata.Separators
 import org.oxycblt.auxio.music.stack.Indexer
+import org.oxycblt.auxio.music.stack.IndexingProgress
 import org.oxycblt.auxio.music.stack.interpret.Interpretation
 import org.oxycblt.auxio.music.stack.interpret.model.MutableLibrary
 import timber.log.Timber as L
@@ -363,26 +364,8 @@ constructor(private val indexer: Indexer, private val musicSettings: MusicSettin
                 Name.Known.SimpleFactory
             }
 
-        var explored = 0
-        var loaded = 0
         val newLibrary =
-            indexer.run(listOf(), Interpretation(nameFactory, separators)) {
-                when (it) {
-                    is Indexer.Event.Discovered -> {
-                        explored = it.amount
-                        emitIndexingProgress(IndexingProgress.Songs(loaded, explored))
-                    }
-                    is Indexer.Event.Extracted -> {
-                        loaded = it.amount
-                        emitIndexingProgress(IndexingProgress.Songs(loaded, explored))
-                    }
-                    is Indexer.Event.Interpret -> {
-                        if (explored == loaded) {
-                            emitIndexingProgress(IndexingProgress.Indeterminate)
-                        }
-                    }
-                }
-            }
+            indexer.run(listOf(), Interpretation(nameFactory, separators), ::emitIndexingProgress)
 
         // We want to make sure that all reads and writes are synchronized due to the sheer
         // amount of consumers of MusicRepository.

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

@@ -25,7 +25,7 @@ import org.oxycblt.auxio.BuildConfig
 import org.oxycblt.auxio.ForegroundServiceNotification
 import org.oxycblt.auxio.IntegerTable
 import org.oxycblt.auxio.R
-import org.oxycblt.auxio.music.IndexingProgress
+import org.oxycblt.auxio.music.stack.IndexingProgress
 import org.oxycblt.auxio.util.newMainPendingIntent
 import timber.log.Timber as L
 
@@ -61,33 +61,19 @@ class IndexingNotification(private val context: Context) :
      * @return true if the notification updated, false otherwise
      */
     fun updateIndexingState(progress: IndexingProgress): Boolean {
-        when (progress) {
-            is IndexingProgress.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.
-                L.d("Updating state to $progress")
-                lastUpdateTime = -1
-                setContentText(context.getString(R.string.lng_indexing))
-                setProgress(0, 0, true)
-                return true
-            }
-            is IndexingProgress.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.
-                val now = SystemClock.elapsedRealtime()
-                if (lastUpdateTime > -1 && (now - lastUpdateTime) < 1500) {
-                    return false
-                }
-                lastUpdateTime = SystemClock.elapsedRealtime()
-                L.d("Updating state to $progress")
-                setContentText(
-                    context.getString(R.string.fmt_indexing, progress.current, progress.total))
-                setProgress(progress.total, progress.current, false)
-                return true
-            }
+        // Determinate state, show an active progress meter. Since these updates arrive
+        // highly rapidly, only update every 1.5 seconds to prevent notification rate
+        // limiting.
+        val now = SystemClock.elapsedRealtime()
+        if (lastUpdateTime > -1 && (now - lastUpdateTime) < 1500) {
+            return false
         }
+        lastUpdateTime = SystemClock.elapsedRealtime()
+        L.d("Updating state to $progress")
+        setContentText(
+            context.getString(R.string.fmt_indexing, progress.interpreted, progress.explored))
+        setProgress(progress.explored, progress.interpreted, false)
+        return true
     }
 }
 

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

@@ -24,7 +24,6 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.flow.buffer
 import kotlinx.coroutines.flow.flowOn
-import org.oxycblt.auxio.music.stack.Indexer.Event
 import org.oxycblt.auxio.music.stack.explore.Explorer
 import org.oxycblt.auxio.music.stack.interpret.Interpretation
 import org.oxycblt.auxio.music.stack.interpret.Interpreter
@@ -34,31 +33,38 @@ interface Indexer {
     suspend fun run(
         uris: List<Uri>,
         interpretation: Interpretation,
-        eventHandler: suspend (Event) -> Unit = {}
+        onProgress: suspend (IndexingProgress) -> Unit = {}
     ): MutableLibrary
-
-    sealed interface Event {
-        data class Discovered(
-            val amount: Int,
-        ) : Event
-
-        data class Extracted(val amount: Int) : Event
-
-        data class Interpret(val amount: Int) : Event
-    }
 }
 
+/**
+ * Represents the current progress of music loading.
+ *
+ * @author Alexander Capehart (OxygenCobalt)
+ */
+data class IndexingProgress(val interpreted: Int, val explored: Int)
+
 class IndexerImpl
 @Inject
 constructor(private val explorer: Explorer, private val interpreter: Interpreter) : Indexer {
     override suspend fun run(
         uris: List<Uri>,
         interpretation: Interpretation,
-        eventHandler: suspend (Event) -> Unit
+        onProgress: suspend (IndexingProgress) -> Unit
     ) = coroutineScope {
-        val files = explorer.explore(uris, eventHandler)
+        var interpreted = 0
+        var explored = 0
+        val files =
+            explorer.explore(uris) {
+                explored++
+                onProgress(IndexingProgress(interpreted, explored))
+            }
         val audioFiles = files.audios.flowOn(Dispatchers.IO).buffer()
         val playlistFiles = files.playlists.flowOn(Dispatchers.IO).buffer()
-        interpreter.interpret(audioFiles, playlistFiles, interpretation, eventHandler)
+
+        interpreter.interpret(audioFiles, playlistFiles, interpretation) {
+            interpreted++
+            onProgress(IndexingProgress(interpreted, explored))
+        }
     }
 }

app/src/main/java/org/oxycblt/auxio/music/stack/explore/Explorer.kt

@@ -36,7 +36,6 @@ import kotlinx.coroutines.flow.merge
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.flow.withIndex
-import org.oxycblt.auxio.music.stack.Indexer
 import org.oxycblt.auxio.music.stack.explore.cache.CacheResult
 import org.oxycblt.auxio.music.stack.explore.cache.TagCache
 import org.oxycblt.auxio.music.stack.explore.extractor.TagExtractor
@@ -44,7 +43,7 @@ import org.oxycblt.auxio.music.stack.explore.fs.DeviceFiles
 import org.oxycblt.auxio.music.stack.explore.playlists.StoredPlaylists
 
 interface Explorer {
-    fun explore(uris: List<Uri>, eventHandler: suspend (Indexer.Event) -> Unit): Files
+    fun explore(uris: List<Uri>, onExplored: suspend () -> Unit): Files
 }
 
 data class Files(val audios: Flow<AudioFile>, val playlists: Flow<PlaylistFile>)
@@ -58,14 +57,14 @@ constructor(
     private val storedPlaylists: StoredPlaylists
 ) : Explorer {
     @OptIn(ExperimentalCoroutinesApi::class)
-    override fun explore(uris: List<Uri>, eventHandler: suspend (Indexer.Event) -> Unit): Files {
+    override fun explore(uris: List<Uri>, onExplored: suspend () -> Unit): Files {
         var discovered = 0
         val deviceFiles =
             deviceFiles
                 .explore(uris.asFlow())
                 .onEach {
                     discovered++
-                    eventHandler(Indexer.Event.Discovered(discovered))
+                    onExplored()
                 }
                 .flowOn(Dispatchers.IO)
                 .buffer()
@@ -75,15 +74,9 @@ constructor(
             uncachedDeviceFiles
                 .split(8)
                 .map { tagExtractor.extract(it).flowOn(Dispatchers.IO).buffer() }
-                .asFlow()
                 .flattenMerge()
         val writtenAudioFiles = tagCache.write(extractedAudioFiles).flowOn(Dispatchers.IO).buffer()
-        var loaded = 0
-        val audioFiles =
-            merge(cachedAudioFiles, writtenAudioFiles).onEach {
-                loaded++
-                eventHandler(Indexer.Event.Extracted(loaded))
-            }
+        val audioFiles = merge(cachedAudioFiles, writtenAudioFiles)
         val playlistFiles = storedPlaylists.read()
         return Files(audioFiles, playlistFiles)
     }
@@ -96,11 +89,11 @@ constructor(
         return files to songs
     }
 
-    private fun <T> Flow<T>.split(n: Int): Array<Flow<T>> {
+    private fun <T> Flow<T>.split(n: Int): Flow<Flow<T>> {
         val indexed = withIndex()
         val shared =
             indexed.shareIn(
                 CoroutineScope(Dispatchers.Main), SharingStarted.WhileSubscribed(), replay = 0)
-        return Array(n) { shared.filter { it.index % n == 0 }.map { it.value } }
+        return Array(n) { shared.filter { it.index % n == 0 }.map { it.value } }.asFlow()
     }
 }

app/src/main/java/org/oxycblt/auxio/music/stack/explore/Files.kt

@@ -23,7 +23,6 @@ import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.info.Date
 import org.oxycblt.auxio.music.stack.explore.fs.Path
-import org.oxycblt.auxio.music.stack.interpret.model.SongImpl
 
 data class DeviceFile(
     val uri: Uri,
@@ -33,11 +32,6 @@ data class DeviceFile(
     val lastModified: Long
 )
 
-/**
- * Raw information about a [SongImpl] obtained from the filesystem/Extractor instances.
- *
- * @author Alexander Capehart (OxygenCobalt)
- */
 data class AudioFile(
     val deviceFile: DeviceFile,
     val durationMs: Long,

app/src/main/java/org/oxycblt/auxio/music/stack/explore/fs/DeviceFiles.kt

@@ -42,21 +42,19 @@ class DeviceFilesImpl
 @Inject
 constructor(
     @ApplicationContext private val context: Context,
-    private val volumeManager: VolumeManager
+    @Inject private val documentPathFactory: DocumentPathFactory
 ) : DeviceFiles {
     private val contentResolver = context.contentResolverSafe
 
     override fun explore(uris: Flow<Uri>): Flow<DeviceFile> =
-        uris.flatMapMerge { rootUri -> exploreImpl(contentResolver, rootUri, Components.nil()) }
+        uris.flatMapMerge { rootUri -> exploreImpl(contentResolver, rootUri,
+            requireNotNull(documentPathFactory.unpackDocumentTreeUri(rootUri))) }
 
     private fun exploreImpl(
         contentResolver: ContentResolver,
         uri: Uri,
-        relativePath: Components
+        relativePath: Path
     ): Flow<DeviceFile> = flow {
-        // TODO: Temporary to maintain path api parity
-        // Figure out what we actually want to do to paths now in saf world.
-        val external = volumeManager.getInternalVolume()
         contentResolver.useQuery(uri, PROJECTION) { cursor ->
             val childUriIndex =
                 cursor.getColumnIndexOrThrow(DocumentsContract.Document.COLUMN_DOCUMENT_ID)
@@ -73,7 +71,7 @@ constructor(
                 val childId = cursor.getString(childUriIndex)
                 val childUri = DocumentsContract.buildDocumentUriUsingTree(uri, childId)
                 val displayName = cursor.getString(displayNameIndex)
-                val path = relativePath.child(displayName)
+                val path = relativePath.file(displayName)
                 val mimeType = cursor.getString(mimeTypeIndex)
                 if (mimeType == DocumentsContract.Document.MIME_TYPE_DIR) {
                     // This does NOT block the current coroutine. Instead, we will
@@ -85,7 +83,7 @@ constructor(
                     // rather than just being a glorified async.
                     val lastModified = cursor.getLong(lastModifiedIndex)
                     val size = cursor.getLong(sizeIndex)
-                    emit(DeviceFile(childUri, mimeType, Path(external, path), size, lastModified))
+                    emit(DeviceFile(childUri, mimeType, path, size, lastModified))
                 }
             }
             // Hypothetically, we could just emitAll as we recurse into a new directory,

app/src/main/java/org/oxycblt/auxio/music/stack/interpret/Interpreter.kt

@@ -21,13 +21,12 @@ package org.oxycblt.auxio.music.stack.interpret
 import javax.inject.Inject
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.asFlow
 import kotlinx.coroutines.flow.buffer
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.toList
 import org.oxycblt.auxio.music.Music
-import org.oxycblt.auxio.music.stack.Indexer
 import org.oxycblt.auxio.music.stack.explore.AudioFile
 import org.oxycblt.auxio.music.stack.explore.PlaylistFile
 import org.oxycblt.auxio.music.stack.interpret.linker.AlbumLinker
@@ -50,7 +49,7 @@ interface Interpreter {
         audioFiles: Flow<AudioFile>,
         playlistFiles: Flow<PlaylistFile>,
         interpretation: Interpretation,
-        eventHandler: suspend (Indexer.Event) -> Unit
+        onInterpret: suspend () -> Unit
     ): MutableLibrary
 }
 
@@ -59,7 +58,7 @@ class InterpreterImpl @Inject constructor(private val preparer: Preparer) : Inte
         audioFiles: Flow<AudioFile>,
         playlistFiles: Flow<PlaylistFile>,
         interpretation: Interpretation,
-        eventHandler: suspend (Indexer.Event) -> Unit
+        onInterpret: suspend () -> Unit
     ): MutableLibrary {
         val preSongs =
             preparer.prepare(audioFiles, interpretation).flowOn(Dispatchers.Main).buffer()
@@ -69,24 +68,19 @@ class InterpreterImpl @Inject constructor(private val preparer: Preparer) : Inte
 
         val artistLinker = ArtistLinker()
         val artistLinkedSongs =
-            artistLinker.register(genreLinkedSongs).flowOn(Dispatchers.Main).buffer()
+            artistLinker.register(genreLinkedSongs).flowOn(Dispatchers.Main).toList()
         // This is intentional. Song and album instances are dependent on artist
         // data, so we need to ensure that all of the linked artist data is resolved
         // before we go any further.
         val genres = genreLinker.resolve()
         val artists = artistLinker.resolve()
 
-        var interpreted = 0
         val albumLinker = AlbumLinker()
         val albumLinkedSongs =
             albumLinker
-                .register(artistLinkedSongs)
-                .flowOn(Dispatchers.Main)
-                .onEach {
-                    interpreted++
-                    eventHandler(Indexer.Event.Interpret(interpreted))
-                }
+                .register(artistLinkedSongs.asFlow())
                 .map { LinkedSongImpl(it) }
+                .flowOn(Dispatchers.Main)
                 .toList()
         val albums = albumLinker.resolve()
 
@@ -96,17 +90,13 @@ class InterpreterImpl @Inject constructor(private val preparer: Preparer) : Inte
                 val uid = it.preSong.computeUid()
                 val other = uidMap[uid]
                 if (other == null) {
-                    SongImpl(it)
+                    SongImpl(it).also { onInterpret() }
                 } else {
                     L.d("Song @ $uid already exists at ${other.path}, ignoring")
                     null
                 }
             }
-        return LibraryImpl(
-            songs,
-            albums.onEach { it.finalize() },
-            artists.onEach { it.finalize() },
-            genres.onEach { it.finalize() })
+        return LibraryImpl(songs, albums, artists, genres)
     }
 
     private data class LinkedSongImpl(private val albumLinkedSong: AlbumLinker.LinkedSong) :

app/src/main/java/org/oxycblt/auxio/music/stack/interpret/model/DeviceMusicImpl.kt

@@ -122,15 +122,6 @@ class AlbumImpl(linkedAlbum: LinkedAlbum) : Album {
         }
         hashCode = 31 * hashCode + song.hashCode()
     }
-
-    /**
-     * Perform final validation and organization on this instance.
-     *
-     * @return This instance upcasted to [Album].
-     */
-    fun finalize(): Album {
-        return this
-    }
 }
 
 /**
@@ -147,12 +138,15 @@ class ArtistImpl(private val preArtist: PreArtist) : Artist {
 
     override val songs = mutableSetOf<Song>()
 
-    private val albums = mutableSetOf<Album>()
-    private val albumMap = mutableMapOf<Album, Boolean>()
-    override lateinit var explicitAlbums: Set<Album>
-    override lateinit var implicitAlbums: Set<Album>
+    override var explicitAlbums = mutableSetOf<Album>()
+    override var implicitAlbums = mutableSetOf<Album>()
 
-    override lateinit var genres: List<Genre>
+    override val genres: List<Genre> by lazy {
+        // TODO: Not sure how to integrate this into music loading.
+        Sort(Sort.Mode.ByName, Sort.Direction.ASCENDING)
+            .genres(songs.flatMapTo(mutableSetOf()) { it.genres })
+            .sortedByDescending { genre -> songs.count { it.genres.contains(genre) } }
+    }
 
     override var durationMs = 0L
     override lateinit var cover: ParentCover
@@ -176,40 +170,15 @@ class ArtistImpl(private val preArtist: PreArtist) : Artist {
     fun link(song: SongImpl) {
         songs.add(song)
         durationMs += song.durationMs
-        if (albumMap[song.album] == null) {
-            albumMap[song.album] = false
+        if (!explicitAlbums.contains(song.album)) {
+            implicitAlbums.add(song.album)
         }
         hashCode = 31 * hashCode + song.hashCode()
     }
 
     fun link(album: AlbumImpl) {
-        albums.add(album)
-        albumMap[album] = true
-    }
-
-    /**
-     * Perform final validation and organization on this instance.
-     *
-     * @return This instance upcasted to [Artist].
-     */
-    fun finalize(): Artist {
-        // There are valid artist configurations:
-        // 1. No songs, no implicit albums, some explicit albums
-        // 2. Some songs, no implicit albums, some explicit albums
-        // 3. Some songs, some implicit albums, no implicit albums
-        // 4. Some songs, some implicit albums, some explicit albums
-        // I'm pretty sure the latter check could be reduced to just explicitAlbums.isNotEmpty,
-        // but I can't be 100% certain.
-        check(songs.isNotEmpty() || (implicitAlbums.size + explicitAlbums.size) > 0) {
-            "Malformed artist $name: Empty"
-        }
-        explicitAlbums = albums.filterTo(mutableSetOf()) { albumMap[it] == true }
-        implicitAlbums = albums.filterNotTo(mutableSetOf()) { albumMap[it] == true }
-        genres =
-            Sort(Sort.Mode.ByName, Sort.Direction.ASCENDING)
-                .genres(songs.flatMapTo(mutableSetOf()) { it.genres })
-                .sortedByDescending { genre -> songs.count { it.genres.contains(genre) } }
-        return this
+        explicitAlbums.add(album)
+        implicitAlbums.remove(album)
     }
 }
 
@@ -242,13 +211,4 @@ class GenreImpl(private val preGenre: PreGenre) : Genre {
         durationMs += song.durationMs
         hashCode = 31 * hashCode + song.hashCode()
     }
-
-    /**
-     * Perform final validation and organization on this instance.
-     *
-     * @return This instance upcasted to [Genre].
-     */
-    fun finalize(): Genre {
-        return this
-    }
 }