diff --git a/CHANGELOG.md b/CHANGELOG.md index f6ffec5cc..1ac9a1159 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ #### 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 #### 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 b22766bcb..aa47ddddc 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -26,6 +26,9 @@ import kotlinx.parcelize.Parcelize import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.R import org.oxycblt.auxio.music.Date.Companion.from +import org.oxycblt.auxio.music.extractor.parseMultiValue +import org.oxycblt.auxio.music.extractor.parseReleaseType +import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.ui.recycler.Item import org.oxycblt.auxio.util.inRangeOrNull import org.oxycblt.auxio.util.nonZeroOrNull @@ -192,8 +195,21 @@ sealed class MusicParent : Music() { * A song. * @author OxygenCobalt */ -class Song constructor(raw: Raw) : Music() { - override val uid: UID +class Song constructor(raw: Raw, settings: Settings) : Music() { + override val uid = UID.hashed(MusicMode.SONGS) { + // Song UIDs are based on the raw data without parsing so that they remain + // consistent across music setting changes. Parents are not held up to the + // same standard since grouping is directly linked to settings. + update(raw.name) + update(raw.albumName) + update(raw.date) + + update(raw.artistNames) + update(raw.albumArtistNames) + + update(raw.track) + update(raw.disc) + } override val rawName = requireNotNull(raw.name) { "Invalid raw: No title" } @@ -201,6 +217,15 @@ class Song constructor(raw: Raw) : Music() { override fun resolveName(context: Context) = rawName + /** The track number of this song in it's album.. */ + val track = raw.track + + /** The disc number of this song in it's album. */ + val disc = raw.disc + + /** The date of this song. May differ from the album date. */ + val date = raw.date + /** The URI pointing towards this audio file. */ val uri = requireNotNull(raw.mediaStoreId) { "Invalid raw: No id" }.audioUri @@ -230,12 +255,6 @@ class Song constructor(raw: Raw) : Music() { /** The date this audio file was added, as a unix epoch timestamp. */ val dateAdded = requireNotNull(raw.dateAdded) { "Invalid raw: No date added" } - /** The track number of this song in it's album.. */ - val track = raw.track - - /** The disc number of this song in it's album. */ - val disc = raw.disc - private var _album: Album? = null /** The album of this song. */ @@ -245,10 +264,17 @@ class Song constructor(raw: Raw) : Music() { // TODO: Multi-artist support // private val _artists: MutableList = mutableListOf() - private val artistName = raw.artistNames?.joinToString() - private val albumArtistName = raw.albumArtistNames?.joinToString() - private val artistSortName = raw.artistSortNames?.joinToString() - private val albumArtistSortName = raw.albumArtistSortNames?.joinToString() + private val artistName = raw.artistNames.parseMultiValue(settings) + .joinToString().ifEmpty { null } + + private val albumArtistName = raw.albumArtistNames.parseMultiValue(settings) + .joinToString().ifEmpty { null } + + private val artistSortName = raw.artistSortNames.parseMultiValue(settings) + .joinToString().ifEmpty { null } + + 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 @@ -282,8 +308,7 @@ class Song constructor(raw: Raw) : Music() { mediaStoreId = requireNotNull(raw.albumMediaStoreId) { "Invalid raw: No album id" }, name = requireNotNull(raw.albumName) { "Invalid raw: No album name" }, sortName = raw.albumSortName, - date = raw.date, - releaseType = raw.albumReleaseType, + releaseType = raw.albumReleaseType.parseReleaseType(settings), rawArtist = if (albumArtistName != null) { Artist.Raw(albumArtistName, albumArtistSortName) @@ -292,7 +317,7 @@ class Song constructor(raw: Raw) : Music() { } ) - val _rawGenres = raw.genreNames?.map { Genre.Raw(it) } ?: listOf(Genre.Raw(null)) + val _rawGenres = raw.genreNames.map { Genre.Raw(it) }.ifEmpty { listOf(Genre.Raw(null)) } fun _link(album: Album) { _album = album @@ -307,23 +332,6 @@ class Song constructor(raw: Raw) : Music() { check(_genres.isNotEmpty()) { "Malformed song: genres are empty" } } - init { - // Generally, we calculate UIDs at the end since everything will definitely be initialized - // by now. - uid = - UID.hashed(MusicMode.SONGS) { - update(rawName) - update(_rawAlbum.name) - update(_rawAlbum.date) - - update(artistName) - update(albumArtistName) - - update(track) - update(disc) - } - } - class Raw constructor( var mediaStoreId: Long? = null, @@ -343,12 +351,12 @@ class Song constructor(raw: Raw) : Music() { var albumMediaStoreId: Long? = null, var albumName: String? = null, var albumSortName: String? = null, - var albumReleaseType: ReleaseType? = null, - var artistNames: List? = null, - var artistSortNames: List? = null, - var albumArtistNames: List? = null, - var albumArtistSortNames: List? = null, - var genreNames: List? = null + var albumReleaseType: List = listOf(), + var artistNames: List = listOf(), + var artistSortNames: List = listOf(), + var albumArtistNames: List = listOf(), + var albumArtistSortNames: List = listOf(), + var genreNames: List = listOf() ) } @@ -357,7 +365,13 @@ class Song constructor(raw: Raw) : Music() { * @author OxygenCobalt */ class Album constructor(raw: Raw, override val songs: List) : MusicParent() { - override val uid: UID + override val uid = UID.hashed(MusicMode.ALBUMS) { + // Hash based on only names despite the presence of a date to increase stability. + // I don't know if there is any situation where an artist will have two albums with + // the exact same name, but if there is, I would love to know. + update(raw.name) + update(raw.rawArtist.name) + } override val rawName = raw.name @@ -366,7 +380,7 @@ class Album constructor(raw: Raw, override val songs: List) : MusicParent( override fun resolveName(context: Context) = rawName /** The latest date this album was released. */ - val date = raw.date + val date: Date? /** The release type of this album, such as "EP". Defaults to "Album". */ val releaseType = raw.releaseType ?: ReleaseType.Album(null) @@ -377,11 +391,11 @@ class Album constructor(raw: Raw, override val songs: List) : MusicParent( */ val coverUri = raw.mediaStoreId.albumCoverUri - /** The earliest date a song in this album was added. */ - val dateAdded = songs.minOf { it.dateAdded } - /** The total duration of songs in this album, in millis. */ - val durationMs = songs.sumOf { it.durationMs } + val durationMs: Long + + /** The earliest date a song in this album was added. */ + val dateAdded: Long private var _artist: Artist? = null @@ -402,23 +416,37 @@ class Album constructor(raw: Raw, override val songs: List) : MusicParent( } init { - uid = - UID.hashed(MusicMode.ALBUMS) { - update(rawName) - update(_rawArtist.name) - update(date) - } + var earliestDate: Date? = null + var totalDuration: Long = 0 + var earliestDateAdded: Long = 0 + // Do linking and value generation in the same loop to save time for (song in songs) { song._link(this) + + if (song.date != null) { + if (earliestDate == null || song.date < earliestDate) { + earliestDate = song.date + } + } + + if (song.dateAdded < earliestDateAdded) { + earliestDateAdded = song.dateAdded + } + + totalDuration += song.durationMs + } + + date = earliestDate + durationMs = totalDuration + dateAdded = earliestDateAdded } class Raw( val mediaStoreId: Long, val name: String, val sortName: String?, - val date: Date?, val releaseType: ReleaseType?, val rawArtist: Artist.Raw ) { @@ -441,7 +469,7 @@ constructor( /** The albums of this artist. */ val albums: List ) : MusicParent() { - override val uid: UID + override val uid = UID.hashed(MusicMode.ARTISTS) { update(raw.name) } override val rawName = raw.name @@ -449,17 +477,22 @@ constructor( override fun resolveName(context: Context) = rawName ?: context.getString(R.string.def_artist) - override val songs = albums.flatMap { it.songs } + private val _songs = mutableListOf() + override val songs = _songs /** The total duration of songs in this artist, in millis. */ - val durationMs = songs.sumOf { it.durationMs } + val durationMs: Long init { - uid = UID.hashed(MusicMode.ARTISTS) { update(rawName) } + var totalDuration = 0L for (album in albums) { album._link(this) + _songs.addAll(album.songs) + totalDuration += album.durationMs } + + durationMs = totalDuration } class Raw(val name: String?, val sortName: String?) { @@ -482,7 +515,7 @@ constructor( * @author OxygenCobalt */ class Genre constructor(raw: Raw, override val songs: List) : MusicParent() { - override val uid: UID + override val uid = UID.hashed(MusicMode.GENRES) { update(raw.name) } override val rawName = raw.name @@ -492,14 +525,17 @@ class Genre constructor(raw: Raw, override val songs: List) : MusicParent( override fun resolveName(context: Context) = rawName ?: context.getString(R.string.def_genre) /** The total duration of the songs in this genre, in millis. */ - val durationMs = songs.sumOf { it.durationMs } + val durationMs: Long init { - uid = UID.hashed(MusicMode.GENRES) { update(rawName) } + val totalDuration = 0L for (song in songs) { song._link(this) + durationMs += song.durationMs } + + durationMs = totalDuration } class Raw(val name: String?) { @@ -531,6 +567,11 @@ fun MessageDigest.update(date: Date?) { update(date.toString().toByteArray()) } +/** Update the digest using a list of strings. */ +fun MessageDigest.update(strings: List) { + strings.forEach(::update) +} + // Note: All methods regarding integer byte-mucking must be little-endian /** @@ -807,8 +848,8 @@ sealed class ReleaseType { // Note: The parsing code is extremely clever in order to reduce duplication. It's // better just to read the specification behind release types than follow this code. - fun parse(types: List): ReleaseType { - val primary = types[0] + fun parse(types: List): 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 diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreLayer.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreLayer.kt index 6e8fb70e8..0fc3814cb 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreLayer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/MediaStoreLayer.kt @@ -26,7 +26,6 @@ import android.provider.MediaStore import androidx.annotation.RequiresApi import androidx.core.database.getIntOrNull import androidx.core.database.getStringOrNull -import org.oxycblt.auxio.music.Date import org.oxycblt.auxio.music.Directory import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.directoryCompat @@ -121,7 +120,7 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa private var albumArtistIndex = -1 private val settings = Settings(context) - private val genreNamesMap = mutableMapOf>() + private val genreNamesMap = mutableMapOf() private val _volumes = mutableListOf() protected val volumes: List @@ -212,11 +211,8 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa val nameIndex = genreCursor.getColumnIndexOrThrow(MediaStore.Audio.Genres.NAME) while (genreCursor.moveToNext()) { - // We can't assume what format these genres are derived from, so we just have - // to assume ID3v2 rules. val id = genreCursor.getLong(idIndex) - val names = (genreCursor.getStringOrNull(nameIndex) ?: continue) - .parseId3GenreNames(settings) + val name = genreCursor.getStringOrNull(nameIndex) ?: continue context.contentResolverSafe.useQuery( MediaStore.Audio.Genres.Members.getContentUri(VOLUME_EXTERNAL, id), @@ -227,8 +223,8 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa while (cursor.moveToNext()) { // Assume that a song can't inhabit multiple genre entries, as I doubt - // Android is smart enough to separate genres into separators. - genreNamesMap[cursor.getLong(songIdIndex)] = names + // Android is smart enough to separate genres. + genreNamesMap[cursor.getLong(songIdIndex)] = name } } } @@ -319,7 +315,7 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa raw.displayName = cursor.getStringOrNull(displayNameIndex) raw.durationMs = cursor.getLong(durationIndex) - raw.date = cursor.getIntOrNull(yearIndex)?.let(Date::from) + 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 @@ -332,23 +328,16 @@ abstract class MediaStoreLayer(private val context: Context, private val cacheLa // as , which makes absolutely no sense given how other fields default // to null if they are not present. If this field is , null it so that // it's easier to handle later. - raw.artistNames = - cursor.getString(artistIndex).run { - if (this != MediaStore.UNKNOWN_STRING) { - // While we can't natively parse multi-value tags from MediaStore itself, we - // can still parse by user-defined separators. - maybeParseSeparators(settings) - } else { - null - } - } + val artist = cursor.getString(artistIndex) + if (artist != MediaStore.UNKNOWN_STRING) { + raw.artistNames = listOf(artist) + } // The album artist field is nullable and never has placeholder values. - raw.albumArtistNames = - cursor.getStringOrNull(albumArtistIndex)?.maybeParseSeparators(settings) + cursor.getStringOrNull(albumArtistIndex)?.let { raw.albumArtistNames = listOf(it) } // Get the genre value we had to query for in initialization - raw.genreNames = genreNamesMap[raw.mediaStoreId] + genreNamesMap[raw.mediaStoreId]?.let { raw.genreNames = listOf(it) } } companion object { diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataLayer.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataLayer.kt index 18496b517..399f93eca 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataLayer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataLayer.kt @@ -27,7 +27,6 @@ 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.audioUri -import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logW @@ -45,7 +44,6 @@ import org.oxycblt.auxio.util.logW * @author OxygenCobalt */ class MetadataLayer(private val context: Context, private val mediaStoreLayer: MediaStoreLayer) { - private val settings = Settings(context) private val taskPool: Array = arrayOfNulls(TASK_CAPACITY) /** Initialize the sub-layers that this layer relies on. */ @@ -75,11 +73,11 @@ class MetadataLayer(private val context: Context, private val mediaStoreLayer: M val finishedRaw = task.get() if (finishedRaw != null) { emit(finishedRaw) - taskPool[i] = Task(context, settings, raw) + taskPool[i] = Task(context, raw) break@spin } } else { - taskPool[i] = Task(context, settings, raw) + taskPool[i] = Task(context, raw) break@spin } } @@ -112,7 +110,7 @@ class MetadataLayer(private val context: Context, private val mediaStoreLayer: M * Wraps an ExoPlayer metadata retrieval task in a safe abstraction. Access is done with [get]. * @author OxygenCobalt */ -class Task(context: Context, private val settings: Settings, private val raw: Song.Raw) { +class Task(context: Context, private val raw: Song.Raw) { private val future = MetadataRetriever.retrieveMetadata( context, @@ -226,18 +224,18 @@ class Task(context: Context, private val settings: Settings, private val raw: So tags["TSOA"]?.let { raw.albumSortName = it[0] } // (Sort) Artist - tags["TPE1"]?.let { raw.artistNames = it.parseMultiValue(settings) } - tags["TSOP"]?.let { raw.artistSortNames = it.parseMultiValue(settings) } + tags["TPE1"]?.let { raw.artistNames = it } + tags["TSOP"]?.let { raw.artistSortNames = it } // (Sort) Album artist - tags["TPE2"]?.let { raw.albumArtistNames = it.parseMultiValue(settings) } - tags["TSO2"]?.let { raw.albumArtistSortNames = it.parseMultiValue(settings) } + tags["TPE2"]?.let { raw.albumArtistNames = it } + tags["TSO2"]?.let { raw.albumArtistSortNames = it } // Genre, with the weird ID3 rules. - tags["TCON"]?.let { raw.genreNames = it.parseId3GenreNames(settings) } + tags["TCON"]?.let { raw.genreNames = it } // Release type (GRP1 is sometimes used for this, so fall back to it) - (tags["TXXX:MusicBrainz Album Type"] ?: tags["GRP1"])?.parseReleaseType(settings)?.let { + (tags["TXXX:MusicBrainz Album Type"] ?: tags["GRP1"])?.let { raw.albumReleaseType = it } } @@ -297,18 +295,18 @@ class Task(context: Context, private val settings: Settings, private val raw: So tags["ALBUMSORT"]?.let { raw.albumSortName = it[0] } // (Sort) Artist - tags["ARTIST"]?.let { raw.artistNames = it.parseMultiValue(settings) } - tags["ARTISTSORT"]?.let { raw.artistSortNames = it.parseMultiValue(settings) } + tags["ARTIST"]?.let { raw.artistNames = it } + tags["ARTISTSORT"]?.let { raw.artistSortNames = it } // (Sort) Album artist - tags["ALBUMARTIST"]?.let { raw.albumArtistNames = it.parseMultiValue(settings) } - tags["ALBUMARTISTSORT"]?.let { raw.albumArtistSortNames = it.parseMultiValue(settings) } + tags["ALBUMARTIST"]?.let { raw.albumArtistNames = it } + tags["ALBUMARTISTSORT"]?.let { raw.albumArtistSortNames = it } // Genre, no ID3 rules here - tags["GENRE"]?.let { raw.genreNames = it.parseMultiValue(settings) } + tags["GENRE"]?.let { raw.genreNames = it } // Release type - tags["RELEASETYPE"]?.parseReleaseType(settings)?.let { raw.albumReleaseType = it } + tags["RELEASETYPE"]?.let { raw.albumReleaseType = it } } /** diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/ParsingUtil.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/ParsingUtil.kt index 3adc3da70..78d8c0ed9 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/ParsingUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/ParsingUtil.kt @@ -42,8 +42,11 @@ fun Int.unpackDiscNo() = div(1000).nonZeroOrNull() */ fun String.parsePositionNum() = split('/', limit = 2)[0].toIntOrNull()?.nonZeroOrNull() +/** Transform an int year into a [Date] */ +fun Int.toDate() = Date.from(this) + /** Parse a plain year from the field into a [Date]. */ -fun String.parseYear() = toIntOrNull()?.let(Date::from) +fun String.parseYear() = toIntOrNull()?.toDate() /** Parse an ISO-8601 time-stamp from this field into a [Date]. */ fun String.parseTimestamp() = Date.from(this) 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 e97c28388..6676ae09f 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 @@ -38,6 +38,7 @@ import org.oxycblt.auxio.music.extractor.Api29MediaStoreLayer import org.oxycblt.auxio.music.extractor.Api30MediaStoreLayer import org.oxycblt.auxio.music.extractor.CacheLayer import org.oxycblt.auxio.music.extractor.MetadataLayer +import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logE import org.oxycblt.auxio.util.logW @@ -214,7 +215,7 @@ class Indexer { val metadataLayer = MetadataLayer(context, mediaStoreLayer) - val songs = buildSongs(metadataLayer) + val songs = buildSongs(metadataLayer, Settings(context)) if (songs.isEmpty()) { return null } @@ -241,7 +242,7 @@ class Indexer { * [buildGenres] functions must be called with the returned list so that all songs are properly * linked up. */ - private suspend fun buildSongs(metadataLayer: MetadataLayer): List { + private suspend fun buildSongs(metadataLayer: MetadataLayer, settings: Settings): List { logD("Starting indexing process") val start = System.currentTimeMillis() @@ -257,7 +258,7 @@ class Indexer { val rawSongs = mutableListOf() metadataLayer.parse { rawSong -> - songs.add(Song(rawSong)) + songs.add(Song(rawSong, settings)) rawSongs.add(rawSong) // Check if we got cancelled after every song addition. @@ -293,15 +294,7 @@ class Indexer { val songsByAlbum = songs.groupBy { it._rawAlbum } for (entry in songsByAlbum) { - val albumSongs = entry.value - - // Use the song with the latest year as our metadata song. - // This allows us to replicate the LAST_YEAR field, which is useful as it means that - // weird years like "0" wont show up if there are alternatives. - val templateSong = - albumSongs.maxWith(compareBy(Sort.Mode.NullableComparator.DATE) { entry.key.date }) - - albums.add(Album(templateSong._rawAlbum, albumSongs)) + albums.add(Album(entry.key, entry.value)) } logD("Successfully built ${albums.size} albums")