diff --git a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt index 2a628996b..0d93d08c2 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt +++ b/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 } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/Indexing.kt b/app/src/main/java/org/oxycblt/auxio/music/Indexing.kt index d4e582660..33ab1315c 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Indexing.kt +++ b/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. * diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt index 231da75bf..328e03b27 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt +++ b/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. diff --git a/app/src/main/java/org/oxycblt/auxio/music/service/IndexerNotifications.kt b/app/src/main/java/org/oxycblt/auxio/music/service/IndexerNotifications.kt index aafe8ab8b..68fddfc14 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/service/IndexerNotifications.kt +++ b/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 } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/Indexer.kt index 8c3262dce..37bfbb216 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/Indexer.kt +++ b/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, 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, 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)) + } } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Explorer.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Explorer.kt index 3c2142256..28f3c9f4c 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Explorer.kt +++ b/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, eventHandler: suspend (Indexer.Event) -> Unit): Files + fun explore(uris: List, onExplored: suspend () -> Unit): Files } data class Files(val audios: Flow, val playlists: Flow) @@ -58,14 +57,14 @@ constructor( private val storedPlaylists: StoredPlaylists ) : Explorer { @OptIn(ExperimentalCoroutinesApi::class) - override fun explore(uris: List, eventHandler: suspend (Indexer.Event) -> Unit): Files { + override fun explore(uris: List, 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 Flow.split(n: Int): Array> { + private fun Flow.split(n: Int): Flow> { 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() } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Files.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Files.kt index 4861a8126..7f4991ceb 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/Files.kt +++ b/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, diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/fs/DeviceFiles.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/explore/fs/DeviceFiles.kt index 41f290475..c306909f9 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/explore/fs/DeviceFiles.kt +++ b/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): Flow = - 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 = 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, diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/Interpreter.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/Interpreter.kt index a3271fe21..281832d67 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/Interpreter.kt +++ b/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, playlistFiles: Flow, 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, playlistFiles: Flow, 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) : diff --git a/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/model/DeviceMusicImpl.kt b/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/model/DeviceMusicImpl.kt index dd3d0e30b..969641104 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/stack/interpret/model/DeviceMusicImpl.kt +++ b/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() - private val albums = mutableSetOf() - private val albumMap = mutableMapOf() - override lateinit var explicitAlbums: Set - override lateinit var implicitAlbums: Set + override var explicitAlbums = mutableSetOf() + override var implicitAlbums = mutableSetOf() - override lateinit var genres: List + override val genres: List 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 - } }