diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ac9a1159..3bf8a7a6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,12 +3,14 @@ ## dev #### What's New +- Added support for songs with multiple genres - Reworked music hashing to be even more reliable (Will wipe playback state) #### What's Improved - Sorting now takes accented characters into account - Added support for compilation sub-release-types like (DJ) Mix - Album dates now start from the earliest date instead of latest date +- Reshuffling the queue will no longer drop any songs you have added/removed #### What's Fixed - Fixed issue where the scroll popup would not display correctly in landscape mode [#230] diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index 47ed7a980..c06778399 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -41,8 +41,6 @@ import java.util.UUID import kotlin.math.max import kotlin.math.min -// TODO: Make empty parents a hard error - // --- MUSIC MODELS --- /** [Item] variant that represents a music item. */ @@ -55,20 +53,31 @@ sealed class Music : Item { /** The raw sorting name of this item. Null if not present. */ abstract val rawSortName: String? + /** + * A key used by the sorting system that takes into account the sort tags of this item, + * any (english) articles that prefix the names, and collation rules. + */ + abstract val collationKey: CollationKey? + /** * Resolve a name from it's raw form to a form suitable to be shown in a UI. * Null values will be resolved into their string form with this function. */ abstract fun resolveName(context: Context): String - /** - * A key used by the sorting system that takes into account the sort tags of this item, - * any (english) articles that prefix the names, and collation rules. - */ - val collationKey: CollationKey? by lazy { - // Ideally, we would generate this on creation, but this is an abstract class, which - // requires us to generate it lazily instead. + // Equality is based on UIDs, as some items (Especially artists) can have identical + // properties (Name) yet non-identical UIDs due to MusicBrainz tags + override fun hashCode() = uid.hashCode() + + override fun equals(other: Any?) = + other is Music && javaClass == other.javaClass && uid == other.uid + + /** + * Workaround to allow for easy collation key generation in the initializer without + * base-class initialization issues or slow lazy initialization. + */ + protected fun makeCollationKeyImpl(): CollationKey? { val sortName = (rawSortName ?: rawName)?.run { when { length > 5 && startsWith("the ", ignoreCase = true) -> substring(4) @@ -78,16 +87,15 @@ sealed class Music : Item { } } - COLLATOR.getCollationKey(sortName) + return COLLATOR.getCollationKey(sortName) } - // Equality is based on UIDs, as some items (Especially artists) can have identical - // properties (Name) yet non-identical UIDs due to MusicBrainz tags - - override fun hashCode() = uid.hashCode() - - override fun equals(other: Any?) = - other is Music && javaClass == other.javaClass && uid == other.uid + /** + * Called when the library has been linked and validation/construction steps dependent + * on linked items should run. It's also used to do last-step initialization of fields + * that require any parent values that would not be present during startup. + */ + abstract fun _finalize() /** * A unique identifier for a piece of music. @@ -192,6 +200,10 @@ sealed class Music : Item { sealed class MusicParent : Music() { /** The songs that this parent owns. */ abstract val songs: List + + override fun _finalize() { + check(songs.isNotEmpty()) { "Invalid parent: No songs" } + } } /** @@ -218,6 +230,8 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { override val rawSortName = raw.sortName + override val collationKey = makeCollationKeyImpl() + override fun resolveName(context: Context) = rawName /** The track number of this song in it's album.. */ @@ -279,15 +293,6 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { private val albumArtistSortName = raw.albumArtistSortNames.parseMultiValue(settings) .joinToString().ifEmpty { null } - /** - * The raw artist name for this song in particular. First uses the artist tag, and then falls - * back to the album artist tag (i.e parent artist name). Null if name is unknown. - */ - val individualArtistRawName: String? - // Note: This is a getter since it relies on a parent value that will not be initialized - // yet on creation. - get() = artistName ?: album.artist.rawName - /** * Resolve the artist name for this song in particular. First uses the artist tag, and then * falls back to the album artist tag (i.e parent artist name) @@ -295,6 +300,14 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { fun resolveIndividualArtistName(context: Context) = artistName ?: album.artist.resolveName(context) + fun areArtistContentsTheSame(other: Song): Boolean { + if (other.artistName != null && artistName != null) { + return other.artistName == artistName + } + + return album.artist.rawName == other.album.artist.rawName + } + private val _genres: MutableList = mutableListOf() /** @@ -331,9 +344,9 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { _genres.add(genre) } - fun _validate() { - (checkNotNull(_album) { "Malformed song: album is null" })._validate() - check(_genres.isNotEmpty()) { "Malformed song: genres are empty" } + override fun _finalize() { + (checkNotNull(_album) { "Malformed song: Album is null" }) + check(_genres.isNotEmpty()) { "Malformed song: No genres" } } class Raw @@ -381,6 +394,8 @@ class Album constructor(raw: Raw, override val songs: List) : MusicParent( override val rawSortName = raw.sortName + override val collationKey = makeCollationKeyImpl() + override fun resolveName(context: Context) = rawName /** The earliest date this album was released. */ @@ -415,7 +430,8 @@ class Album constructor(raw: Raw, override val songs: List) : MusicParent( _artist = artist } - fun _validate() { + override fun _finalize() { + super._finalize() checkNotNull(_artist) { "Invalid album: Artist is null " } } @@ -478,6 +494,8 @@ constructor( override val rawSortName = raw.sortName + override val collationKey = makeCollationKeyImpl() + override fun resolveName(context: Context) = rawName ?: context.getString(R.string.def_artist) private val _songs = mutableListOf() @@ -525,6 +543,8 @@ class Genre constructor(raw: Raw, override val songs: List) : MusicParent( // Sort tags don't make sense on genres override val rawSortName = rawName + override val collationKey = makeCollationKeyImpl() + override fun resolveName(context: Context) = rawName ?: context.getString(R.string.def_genre) /** The total duration of the songs in this genre, in millis. */ diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt index fa2062997..1b0644598 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -34,7 +34,7 @@ import org.oxycblt.auxio.util.contentResolverSafe * * 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. + * 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 diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreExtractor.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreExtractor.kt index 36d79f38b..46bef4155 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreExtractor.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreExtractor.kt @@ -100,7 +100,7 @@ import java.io.File * music loading process. * @author OxygenCobalt */ -abstract class MediaStoreLayer(private val context: Context, private val cacheLayer: CacheDatabase) { +abstract class MediaStoreExtractor(private val context: Context, private val cacheDatabase: CacheDatabase) { private var cursor: Cursor? = null private var idIndex = -1 @@ -129,7 +129,7 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa logD("Initializing") val start = System.currentTimeMillis() - cacheLayer.init() + cacheDatabase.init() val storageManager = context.getSystemServiceCompat(StorageManager::class) _volumes.addAll(storageManager.storageVolumesCompat) @@ -238,7 +238,7 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa cursor?.close() cursor = null - cacheLayer.finalize(rawSongs) + cacheDatabase.finalize(rawSongs) } /** @@ -259,7 +259,7 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa raw.dateAdded = cursor.getLong(dateAddedIndex) raw.dateModified = cursor.getLong(dateAddedIndex) - if (cacheLayer.maybePopulateCachedRaw(raw)) { + if (cacheDatabase.maybePopulateCachedRaw(raw)) { // We found a valid cache entry, no need to extract metadata. logD("Found cached raw: ${raw.name}") return true @@ -368,12 +368,12 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa // speed, we only want to add redundancy on known issues, not with possible issues. /** - * A [MediaStoreLayer] that completes the music loading process in a way compatible from + * A [MediaStoreExtractor] that completes the music loading process in a way compatible from * API 21 onwards to API 29. * @author OxygenCobalt */ -class Api21MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : - MediaStoreLayer(context, cacheLayer) { +class Api21MediaStoreExtractor(context: Context, cacheDatabase: CacheDatabase) : + MediaStoreExtractor(context, cacheDatabase) { private var trackIndex = -1 private var dataIndex = -1 @@ -433,13 +433,13 @@ class Api21MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : } /** - * A [MediaStoreLayer] that selects directories and builds paths using the modern volume fields + * A [MediaStoreExtractor] that selects directories and builds paths using the modern volume fields * available from API 29 onwards. * @author OxygenCobalt */ @RequiresApi(Build.VERSION_CODES.Q) -open class BaseApi29MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : - MediaStoreLayer(context, cacheLayer) { +open class BaseApi29MediaStoreExtractor(context: Context, cacheDatabase: CacheDatabase) : + MediaStoreExtractor(context, cacheDatabase) { private var volumeIndex = -1 private var relativePathIndex = -1 @@ -489,13 +489,13 @@ open class BaseApi29MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) } /** - * A [MediaStoreLayer] that completes the music loading process in a way compatible with at least + * A [MediaStoreExtractor] that completes the music loading process in a way compatible with at least * API 29. * @author OxygenCobalt */ @RequiresApi(Build.VERSION_CODES.Q) -open class Api29MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : - BaseApi29MediaStoreLayer(context, cacheLayer) { +open class Api29MediaStoreExtractor(context: Context, cacheDatabase: CacheDatabase) : + BaseApi29MediaStoreExtractor(context, cacheDatabase) { private var trackIndex = -1 override fun init(): Cursor { @@ -521,13 +521,13 @@ open class Api29MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : } /** - * A [MediaStoreLayer] that completes the music loading process in a way compatible with at least + * A [MediaStoreExtractor] that completes the music loading process in a way compatible with at least * API 30. * @author OxygenCobalt */ @RequiresApi(Build.VERSION_CODES.R) -class Api30MediaStoreLayer(context: Context, cacheLayer: CacheDatabase) : - BaseApi29MediaStoreLayer(context, cacheLayer) { +class Api30MediaStoreExtractor(context: Context, cacheDatabase: CacheDatabase) : + BaseApi29MediaStoreExtractor(context, cacheDatabase) { private var trackIndex: Int = -1 private var discIndex: Int = -1 diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt index b95fcc0fc..6e61913b8 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt @@ -43,19 +43,19 @@ import org.oxycblt.auxio.util.logW * * @author OxygenCobalt */ -class MetadataLayer(private val context: Context, private val mediaStoreLayer: MediaStoreLayer) { +class MetadataExtractor(private val context: Context, private val mediaStoreExtractor: MediaStoreExtractor) { private val taskPool: Array = arrayOfNulls(TASK_CAPACITY) /** Initialize the sub-layers that this layer relies on. */ - fun init() = mediaStoreLayer.init().count + fun init() = mediaStoreExtractor.init().count /** Finalize the sub-layers that this layer relies on. */ - fun finalize(rawSongs: List) = mediaStoreLayer.finalize(rawSongs) + fun finalize(rawSongs: List) = mediaStoreExtractor.finalize(rawSongs) suspend fun parse(emit: suspend (Song.Raw) -> Unit) { while (true) { val raw = Song.Raw() - if (mediaStoreLayer.populateRawSong(raw) ?: break) { + if (mediaStoreExtractor.populateRawSong(raw) ?: break) { // No need to extract metadata that was successfully restored from the cache emit(raw) continue diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt index 0e3b82edc..acc36d14d 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt @@ -33,11 +33,11 @@ import org.oxycblt.auxio.music.Genre import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Sort -import org.oxycblt.auxio.music.extractor.Api21MediaStoreLayer -import org.oxycblt.auxio.music.extractor.Api29MediaStoreLayer -import org.oxycblt.auxio.music.extractor.Api30MediaStoreLayer +import org.oxycblt.auxio.music.extractor.Api21MediaStoreExtractor +import org.oxycblt.auxio.music.extractor.Api29MediaStoreExtractor +import org.oxycblt.auxio.music.extractor.Api30MediaStoreExtractor import org.oxycblt.auxio.music.extractor.CacheDatabase -import org.oxycblt.auxio.music.extractor.MetadataLayer +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 @@ -197,25 +197,25 @@ class Indexer { private suspend fun indexImpl(context: Context): MusicStore.Library? { emitIndexing(Indexing.Indeterminate) - // Create the chain of layers. Each layer builds on the previous layer and + // 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 // your compile times by 3x. Isn't that nice. - val cacheLayer = CacheDatabase() + val cacheDatabase = CacheDatabase() - val mediaStoreLayer = + val mediaStoreExtractor = when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> - Api30MediaStoreLayer(context, cacheLayer) + Api30MediaStoreExtractor(context, cacheDatabase) Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> - Api29MediaStoreLayer(context, cacheLayer) - else -> Api21MediaStoreLayer(context, cacheLayer) + Api29MediaStoreExtractor(context, cacheDatabase) + else -> Api21MediaStoreExtractor(context, cacheDatabase) } - val metadataLayer = MetadataLayer(context, mediaStoreLayer) + val metadataExtractor = MetadataExtractor(context, mediaStoreExtractor) - val songs = buildSongs(metadataLayer, Settings(context)) + val songs = buildSongs(metadataExtractor, Settings(context)) if (songs.isEmpty()) { return null } @@ -226,9 +226,21 @@ class Indexer { val artists = buildArtists(albums) val genres = buildGenres(songs) - // Sanity check: Ensure that all songs are linked up to albums/artists/genres. + // Make sure we finalize all the items now that they are fully built. for (song in songs) { - song._validate() + 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") @@ -237,19 +249,19 @@ class Indexer { } /** - * Does the initial query over the song database using [metadataLayer]. The songs returned by + * Does the initial query over the song database using [metadataExtractor]. The songs returned by * this function are **not** well-formed. The companion [buildAlbums], [buildArtists], and * [buildGenres] functions must be called with the returned list so that all songs are properly * linked up. */ - private suspend fun buildSongs(metadataLayer: MetadataLayer, settings: Settings): List { + private suspend fun buildSongs(metadataExtractor: MetadataExtractor, settings: Settings): List { logD("Starting indexing process") val start = System.currentTimeMillis() // Initialize the extractor chain. This also nets us the projected total // that we can show when loading. - val total = metadataLayer.init() + val total = metadataExtractor.init() yield() // Note: We use a set here so we can eliminate effective duplicates of @@ -257,7 +269,7 @@ class Indexer { val songs = mutableSetOf() val rawSongs = mutableListOf() - metadataLayer.parse { rawSong -> + metadataExtractor.parse { rawSong -> songs.add(Song(rawSong, settings)) rawSongs.add(rawSong) @@ -266,7 +278,7 @@ class Indexer { emitIndexing(Indexing.Songs(songs.size, total)) } - metadataLayer.finalize(rawSongs) + metadataExtractor.finalize(rawSongs) val sorted = Sort(Sort.Mode.ByName, true).songs(songs) diff --git a/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt b/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt index 5eb802480..409216af1 100644 --- a/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/search/SearchViewModel.kt @@ -144,7 +144,6 @@ class SearchViewModel(application: Application) : private fun List.filterSongsBy(value: String) = baseFilterBy(value) { - logD(it.rawSortName) it.rawSortName?.contains(value, ignoreCase = true) == true || it.path.name.contains(value) } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt b/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt index 0d354f27b..3cd73b9a8 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/recycler/ViewHolders.kt @@ -64,7 +64,7 @@ class SongViewHolder private constructor(private val binding: ItemSongBinding) : object : SimpleItemCallback() { override fun areContentsTheSame(oldItem: Song, newItem: Song) = oldItem.rawName == newItem.rawName && - oldItem.individualArtistRawName == oldItem.individualArtistRawName + oldItem.areArtistContentsTheSame(newItem) } } }