music: do settings-dependent parsing in song

Do settings-reliant parsing within the song class.

This way, song UIDs or future caching will not be unexpectedly broken
by changes in music settings.

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]

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() {
+class Song constructor(raw: Raw, settings: Settings) : Music() {
-    override val uid: UID
+    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<Artist> = mutableListOf()
 
-    private val artistName = raw.artistNames?.joinToString()
+    private val artistName = raw.artistNames.parseMultiValue(settings)
-    private val albumArtistName = raw.albumArtistNames?.joinToString()
+        .joinToString().ifEmpty { null }
-    private val artistSortName = raw.artistSortNames?.joinToString()
+
-    private val albumArtistSortName = raw.albumArtistSortNames?.joinToString()
+    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.parseReleaseType(settings),
-            releaseType = raw.albumReleaseType,
             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 albumReleaseType: List<String> = listOf(),
-        var artistNames: List<String>? = null,
+        var artistNames: List<String> = listOf(),
-        var artistSortNames: List<String>? = null,
+        var artistSortNames: List<String> = listOf(),
-        var albumArtistNames: List<String>? = null,
+        var albumArtistNames: List<String> = listOf(),
-        var albumArtistSortNames: List<String>? = null,
+        var albumArtistSortNames: List<String> = listOf(),
-        var genreNames: List<String>? = null
+        var genreNames: List<String> = listOf()
     )
 }
 
@@ -357,7 +365,13 @@ class Song constructor(raw: Raw) : Music() {
  * @author OxygenCobalt
  */
 class Album constructor(raw: Raw, override val songs: List<Song>) : 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<Song>) : 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<Song>) : 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<Song>) : MusicParent(
     }
 
     init {
-        uid =
+        var earliestDate: Date? = null
-            UID.hashed(MusicMode.ALBUMS) {
+        var totalDuration: Long = 0
-                update(rawName)
+        var earliestDateAdded: Long = 0
-                update(_rawArtist.name)
-                update(date)
-            }
 
+        // 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<Album>
 ) : 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<Song>()
+    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<Song>) : 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<Song>) : 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<String>) {
+    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<String>): ReleaseType {
+        fun parse(types: List<String>): ReleaseType? {
-            val primary = types[0]
+            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

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<Long, List<String>>()
+    private val genreNamesMap = mutableMapOf<Long, String>()
 
     private val _volumes = mutableListOf<StorageVolume>()
     protected val volumes: List<StorageVolume>
@@ -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)
+                val name = genreCursor.getStringOrNull(nameIndex) ?: continue
-                    .parseId3GenreNames(settings)
 
                 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.
+                        // Android is smart enough to separate genres.
-                        genreNamesMap[cursor.getLong(songIdIndex)] = names
+                        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 <unknown>, which makes absolutely no sense given how other fields default
         // to null if they are not present. If this field is <unknown>, null it so that
         // it's easier to handle later.
-        raw.artistNames =
+        val artist = cursor.getString(artistIndex)
-            cursor.getString(artistIndex).run {
+        if (artist != MediaStore.UNKNOWN_STRING) {
-                if (this != MediaStore.UNKNOWN_STRING) {
+            raw.artistNames = listOf(artist)
-                    // While we can't natively parse multi-value tags from MediaStore itself, we
+        }
-                    // can still parse by user-defined separators.
-                    maybeParseSeparators(settings)
-                } else {
-                    null
-                }
-            }
 
         // The album artist field is nullable and never has placeholder values.
-        raw.albumArtistNames =
+        cursor.getStringOrNull(albumArtistIndex)?.let { raw.albumArtistNames = listOf(it) }
-            cursor.getStringOrNull(albumArtistIndex)?.maybeParseSeparators(settings)
 
         // 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 {

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<Task?> = 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["TPE1"]?.let { raw.artistNames = it }
-        tags["TSOP"]?.let { raw.artistSortNames = it.parseMultiValue(settings) }
+        tags["TSOP"]?.let { raw.artistSortNames = it }
 
         // (Sort) Album artist
-        tags["TPE2"]?.let { raw.albumArtistNames = it.parseMultiValue(settings) }
+        tags["TPE2"]?.let { raw.albumArtistNames = it }
-        tags["TSO2"]?.let { raw.albumArtistSortNames = it.parseMultiValue(settings) }
+        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["ARTIST"]?.let { raw.artistNames = it }
-        tags["ARTISTSORT"]?.let { raw.artistSortNames = it.parseMultiValue(settings) }
+        tags["ARTISTSORT"]?.let { raw.artistSortNames = it }
 
         // (Sort) Album artist
-        tags["ALBUMARTIST"]?.let { raw.albumArtistNames = it.parseMultiValue(settings) }
+        tags["ALBUMARTIST"]?.let { raw.albumArtistNames = it }
-        tags["ALBUMARTISTSORT"]?.let { raw.albumArtistSortNames = it.parseMultiValue(settings) }
+        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 }
     }
 
     /**

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)

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<Song> {
+    private suspend fun buildSongs(metadataLayer: MetadataLayer, settings: Settings): List<Song> {
         logD("Starting indexing process")
 
         val start = System.currentTimeMillis()
@@ -257,7 +258,7 @@ class Indexer {
         val rawSongs = mutableListOf<Song.Raw>()
 
         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
+            albums.add(Album(entry.key, 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))
         }
 
         logD("Successfully built ${albums.size} albums")