music: new fuzzy grouper

New fuzzy grouper that:
1. Does not eagerly group by MBID unless fully tagged
2. Does not eagerly group by artist by default

CHANGELOG.md

@@ -10,6 +10,8 @@
 - Added GitHub/email feedback forms to about page
 
 #### What's Improved
+- Album grouping no longer done with artist in mind by default
+- MusicBrainz IDs will no longer split albums/artists in less tagged libraries
 - M3U playlist file name is now proposed if one cannot be found within the file
 - Sorting songs by date now uses songs date first, before the earliest album date
 - Added working layouts for small split-screen form factors

app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt

@@ -21,12 +21,14 @@ package org.oxycblt.auxio.music.device
 import android.content.Context
 import android.net.Uri
 import android.provider.OpenableColumns
+import java.util.UUID
 import javax.inject.Inject
 import kotlinx.coroutines.channels.Channel
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.Music
+import org.oxycblt.auxio.music.MusicParent
 import org.oxycblt.auxio.music.MusicRepository
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.fs.Path
@@ -51,10 +53,13 @@ import timber.log.Timber as L
 interface DeviceLibrary {
     /** All [Song]s in this [DeviceLibrary]. */
     val songs: Collection<Song>
+
     /** All [Album]s in this [DeviceLibrary]. */
     val albums: Collection<Album>
+
     /** All [Artist]s in this [DeviceLibrary]. */
     val artists: Collection<Artist>
+
     /** All [Genre]s in this [DeviceLibrary]. */
     val genres: Collection<Genre>
 
@@ -134,11 +139,9 @@ class DeviceLibraryFactoryImpl @Inject constructor() : DeviceLibrary.Factory {
         nameFactory: Name.Known.Factory
     ): DeviceLibraryImpl {
         val songGrouping = mutableMapOf<Music.UID, SongImpl>()
-        val albumGrouping = mutableMapOf<RawAlbum.Key, Grouping<RawAlbum, SongImpl>>()
+        val albumGrouping = mutableMapOf<String?, MutableMap<UUID?, Grouping<RawAlbum, SongImpl>>>()
-        val artistGrouping = mutableMapOf<RawArtist.Key, Grouping<RawArtist, Music>>()
+        val artistGrouping = mutableMapOf<String?, MutableMap<UUID?, Grouping<RawArtist, Music>>>()
-        val genreGrouping = mutableMapOf<RawGenre.Key, Grouping<RawGenre, SongImpl>>()
+        val genreGrouping = mutableMapOf<String?, Grouping<RawGenre, SongImpl>>()
-
-        // TODO: Use comparators here
 
         // All music information is grouped as it is indexed by other components.
         rawSongs.forEachWithTimeout { rawSong ->
@@ -159,62 +162,22 @@ class DeviceLibraryFactoryImpl @Inject constructor() : DeviceLibrary.Factory {
             songGrouping[song.uid] = song
 
             // Group the new song into an album.
-            val albumKey = song.rawAlbum.key
+            appendToMusicBrainzIdTree(song, song.rawAlbum, albumGrouping) { old, new ->
-            val albumBody = albumGrouping[albumKey]
+                compareSongTracks(old, new)
-            if (albumBody != null) {
-                albumBody.music.add(song)
-                val prioritized = albumBody.raw.src
-                // Since albums are grouped fuzzily, we pick the song with the earliest track to
-                // use for album information to ensure consistent metadata and UIDs. Fall back to
-                // the name otherwise.
-                val higherPriority =
-                    song.track != null &&
-                        (prioritized.track == null ||
-                            song.track < prioritized.track ||
-                            (song.track == prioritized.track && song.name < prioritized.name))
-
-                if (higherPriority) {
-                    albumBody.raw = PrioritizedRaw(song.rawAlbum, song)
-                }
-            } else {
-                // Need to initialize this grouping.
-                albumGrouping[albumKey] =
-                    Grouping(PrioritizedRaw(song.rawAlbum, song), mutableSetOf(song))
             }
-
             // Group the song into each of it's artists.
             for (rawArtist in song.rawArtists) {
-                val artistKey = rawArtist.key
+                appendToMusicBrainzIdTree(song, rawArtist, artistGrouping) { old, new ->
-                val artistBody = artistGrouping[artistKey]
+                    // Artist information from earlier dates is prioritized, as it is less likely to
-                if (artistBody != null) {
+                    // change with the addition of new tracks. Fall back to the name otherwise.
-                    // Since artists are not guaranteed to have songs, song artist information is
+                    check(old is SongImpl) // This should always be the case.
-                    // de-prioritized compared to album artist information.
+                    compareSongDates(old, new)
-                    artistBody.music.add(song)
-                } else {
-                    // Need to initialize this grouping.
-                    artistGrouping[artistKey] =
-                        Grouping(PrioritizedRaw(rawArtist, song), mutableSetOf(song))
                 }
             }
 
             // Group the song into each of it's genres.
             for (rawGenre in song.rawGenres) {
-                val genreKey = rawGenre.key
+                appendToNameTree(song, rawGenre, genreGrouping) { old, new -> new.name < old.name }
-                val genreBody = genreGrouping[genreKey]
-                if (genreBody != null) {
-                    genreBody.music.add(song)
-                    // Genre information from higher songs in ascending alphabetical order are
-                    // prioritized.
-                    val prioritized = genreBody.raw.src
-                    val higherPriority = song.name < prioritized.name
-                    if (higherPriority) {
-                        genreBody.raw = PrioritizedRaw(rawGenre, song)
-                    }
-                } else {
-                    // Need to initialize this grouping.
-                    genreGrouping[genreKey] =
-                        Grouping(PrioritizedRaw(rawGenre, song), mutableSetOf(song))
-                }
             }
 
             processedSongs.sendWithTimeout(rawSong)
@@ -222,47 +185,154 @@ class DeviceLibraryFactoryImpl @Inject constructor() : DeviceLibrary.Factory {
 
         // Now that all songs are processed, also process albums and group them into their
         // respective artists.
-        val albums = albumGrouping.values.mapTo(mutableSetOf()) { AlbumImpl(it, nameFactory) }
+        pruneMusicBrainzIdTree(albumGrouping) { old, new ->
+            compareSongTracks(old, new)
+        }
+        val albums = flattenMusicBrainzIdTree(albumGrouping) { AlbumImpl(it, nameFactory) }
         for (album in albums) {
             for (rawArtist in album.rawArtists) {
-                val key = RawArtist.Key(rawArtist)
+                appendToMusicBrainzIdTree(album, rawArtist, artistGrouping) { old, new ->
-                val body = artistGrouping[key]
+                    when (old) {
-                if (body != null) {
-                    body.music.add(album)
-                    when (val prioritized = body.raw.src) {
                         // Immediately replace any songs that initially held the priority position.
-                        is SongImpl -> body.raw = PrioritizedRaw(rawArtist, album)
+                        is SongImpl -> true
                         is AlbumImpl -> {
-                            // Album artist information from earlier dates is prioritized, as it is
+                            compareAlbumDates(old, new)
-                            // less likely to change with the addition of new tracks. Fall back to
-                            // the name otherwise.
-                            val prioritize =
-                                album.dates != null &&
-                                    (prioritized.dates == null ||
-                                        album.dates < prioritized.dates ||
-                                        (album.dates == prioritized.dates &&
-                                            album.name < prioritized.name))
-
-                            if (prioritize) {
-                                body.raw = PrioritizedRaw(rawArtist, album)
-                            }
                         }
                         else -> throw IllegalStateException()
                     }
-                } else {
-                    // Need to initialize this grouping.
-                    artistGrouping[key] =
-                        Grouping(PrioritizedRaw(rawArtist, album), mutableSetOf(album))
                 }
             }
         }
 
         // Artists and genres do not need to be grouped and can be processed immediately.
-        val artists = artistGrouping.values.mapTo(mutableSetOf()) { ArtistImpl(it, nameFactory) }
+        pruneMusicBrainzIdTree(artistGrouping) { old, new ->
-        val genres = genreGrouping.values.mapTo(mutableSetOf()) { GenreImpl(it, nameFactory) }
+            when {
+                // Immediately replace any songs that initially held the priority position.
+                old is SongImpl && new is AlbumImpl -> true
+                old is AlbumImpl && new is SongImpl -> false
+                old is SongImpl && new is SongImpl -> {
+                    compareSongDates(old, new)
+                }
+                old is AlbumImpl && new is AlbumImpl -> {
+                   compareAlbumDates(old, new)
+                }
+                else -> throw IllegalStateException()
+            }
+        }
+        val artists = flattenMusicBrainzIdTree(artistGrouping) { ArtistImpl(it, nameFactory) }
+        val genres = flattenNameTree(genreGrouping) { GenreImpl(it, nameFactory) }
 
         return DeviceLibraryImpl(songGrouping.values.toSet(), albums, artists, genres)
     }
+
+    private inline fun <R : NameGroupable, O : Music, N : O> appendToNameTree(
+        music: N,
+        raw: R,
+        tree: MutableMap<String?, Grouping<R, O>>,
+        prioritize: (old: O, new: N) -> Boolean,
+    ) {
+        val nameKey = raw.name?.lowercase()
+        val body = tree[nameKey]
+        if (body != null) {
+            body.music.add(music)
+            if (prioritize(body.raw.src, music)) {
+                body.raw = PrioritizedRaw(raw, music)
+            }
+        } else {
+            // Need to initialize this grouping.
+            tree[nameKey] = Grouping(PrioritizedRaw(raw, music), mutableSetOf(music))
+        }
+    }
+
+    private inline fun <R : NameGroupable, O : Music, P : MusicParent> flattenNameTree(
+        tree: MutableMap<String?, Grouping<R, O>>,
+        map: (Grouping<R, O>) -> P
+    ): Set<P> = tree.values.mapTo(mutableSetOf()) { map(it) }
+
+    private inline fun <R : MusicBrainzGroupable, O : Music, N : O> appendToMusicBrainzIdTree(
+        music: N,
+        raw: R,
+        tree: MutableMap<String?, MutableMap<UUID?, Grouping<R, O>>>,
+        prioritize: (old: O, new: N) -> Boolean,
+    ) {
+        val nameKey = raw.name?.lowercase()
+        val musicBrainzIdGroups = tree[nameKey]
+        if (musicBrainzIdGroups != null) {
+            val body = musicBrainzIdGroups[raw.musicBrainzId]
+            if (body != null) {
+                body.music.add(music)
+                if (prioritize(body.raw.src, music)) {
+                    body.raw = PrioritizedRaw(raw, music)
+                }
+            } else {
+                // Need to initialize this grouping.
+                musicBrainzIdGroups[raw.musicBrainzId] =
+                    Grouping(PrioritizedRaw(raw, music), mutableSetOf(music))
+            }
+        } else {
+            // Need to initialize this grouping.
+            tree[nameKey] =
+                mutableMapOf(
+                    raw.musicBrainzId to Grouping(PrioritizedRaw(raw, music), mutableSetOf(music)))
+        }
+    }
+
+    private inline fun <R, M : Music> pruneMusicBrainzIdTree(
+        tree: MutableMap<String?, MutableMap<UUID?, Grouping<R, M>>>,
+        prioritize: (old: M, new: M) -> Boolean
+    ) {
+        for ((_, musicBrainzIdGroups) in tree) {
+            var nullGroup = musicBrainzIdGroups[null]
+            if (nullGroup == null) {
+                // Full MusicBrainz ID tagging. Nothing to do.
+                continue
+            }
+            // Only partial MusicBrainz ID tagging. For the sake of basic sanity, just
+            // collapse all of them into the null group.
+            // TODO: More advanced heuristics eventually (tm)
+            musicBrainzIdGroups
+                .filter { it.key != null }
+                .forEach {
+                    val (_, group) = it
+                    nullGroup.music.addAll(group.music)
+                    if (prioritize(group.raw.src, nullGroup.raw.src)) {
+                        nullGroup.raw = group.raw
+                    }
+                    musicBrainzIdGroups.remove(it.key)
+                }
+        }
+    }
+
+    private inline fun <R, M : Music, T : MusicParent> flattenMusicBrainzIdTree(
+        tree: MutableMap<String?, MutableMap<UUID?, Grouping<R, M>>>,
+        map: (Grouping<R, M>) -> T
+    ): Set<T> {
+        val result = mutableSetOf<T>()
+        for ((_, musicBrainzIdGroups) in tree) {
+            for (group in musicBrainzIdGroups.values) {
+                result += map(group)
+            }
+        }
+        return result
+    }
+
+    private fun compareSongTracks(old: SongImpl, new: SongImpl) =
+        new.track != null &&
+            (old.track == null ||
+                new.track < old.track ||
+                (new.track == old.track && new.name < old.name))
+
+    private fun compareAlbumDates(old: AlbumImpl, new: AlbumImpl) =
+        new.dates != null &&
+            (old.dates == null ||
+                new.dates < old.dates ||
+                (new.dates == old.dates && new.name < old.name))
+
+    private fun compareSongDates(old: SongImpl, new: SongImpl) =
+        new.date != null &&
+            (old.date == null ||
+                new.date < old.date ||
+                (new.date == old.date && new.name < old.name))
 }
 
 // TODO: Avoid redundant data creation

app/src/main/java/org/oxycblt/auxio/music/device/DeviceMusicImpl.kt

@@ -160,7 +160,14 @@ class SongImpl(
                         name,
                         artistSortNames.getOrNull(i))
                 }
-                .distinctBy { it.key }
+                // Some songs have the same artist listed multiple times (sometimes with different
+                // casing!),
+                // so we need to deduplicate lest finalization reordering fails.
+                // Since MBID data can wind up clobbered later in the grouper, we can't really
+                // use it to deduplicate. That means that a hypothetical track with two artists
+                // of the same name but different MBIDs will be grouped wrong. That is a bridge
+                // I will cross when I get to it.
+                .distinctBy { it.name?.lowercase() }
 
         val albumArtistMusicBrainzIds = separators.split(rawSong.albumArtistMusicBrainzIds)
         val albumArtistNames = separators.split(rawSong.albumArtistNames)
@@ -173,7 +180,7 @@ class SongImpl(
                         name,
                         albumArtistSortNames.getOrNull(i))
                 }
-                .distinctBy { it.key }
+                .distinctBy { it.name?.lowercase() }
 
         rawAlbum =
             RawAlbum(
@@ -199,7 +206,10 @@ class SongImpl(
         val genreNames =
             (rawSong.genreNames.parseId3GenreNames() ?: separators.split(rawSong.genreNames))
         rawGenres =
-            genreNames.map { RawGenre(it) }.distinctBy { it.key }.ifEmpty { listOf(RawGenre()) }
+            genreNames
+                .map { RawGenre(it) }
+                .distinctBy { it.name?.lowercase() }
+                .ifEmpty { listOf(RawGenre()) }
 
         hashCode = 31 * hashCode + rawSong.hashCode()
         hashCode = 31 * hashCode + nameFactory.hashCode()
@@ -507,7 +517,7 @@ class ArtistImpl(
      * @return The index of the [Artist]'s [RawArtist] within the list.
      */
     fun getOriginalPositionIn(rawArtists: List<RawArtist>) =
-        rawArtists.indexOfFirst { it.key == rawArtist.key }
+        rawArtists.indexOfFirst { it.name?.lowercase() == rawArtist.name?.lowercase() }
 
     /**
      * Perform final validation and organization on this instance.
@@ -600,7 +610,7 @@ class GenreImpl(
      * @return The index of the [Genre]'s [RawGenre] within the list.
      */
     fun getOriginalPositionIn(rawGenres: List<RawGenre>) =
-        rawGenres.indexOfFirst { it.key == rawGenre.key }
+        rawGenres.indexOfFirst { it.name?.lowercase() == rawGenre.name?.lowercase() }
 
     /**
      * Perform final validation and organization on this instance.

app/src/main/java/org/oxycblt/auxio/music/device/RawMusic.kt

@@ -107,49 +107,16 @@ data class RawAlbum(
      */
     val mediaStoreId: Long,
     /** @see Music.uid */
-    val musicBrainzId: UUID?,
+    override val musicBrainzId: UUID?,
     /** @see Music.name */
-    val name: String,
+    override val name: String,
     /** @see Music.name */
     val sortName: String?,
     /** @see Album.releaseType */
     val releaseType: ReleaseType?,
     /** @see RawArtist.name */
     val rawArtists: List<RawArtist>
-) {
+) : MusicBrainzGroupable
-    val key = Key(this)
-
-    /**
-     * Allows [RawAlbum]s to be compared by "fundamental" information that is unlikely to change on
-     * an item-by-item
-     */
-    data class Key(private val inner: RawAlbum) {
-        // Albums are grouped as follows:
-        // - If we have a MusicBrainz ID, only group by it. This allows different Albums with the
-        // same name to be differentiated, which is common in large libraries.
-        // - If we do not have a MusicBrainz ID, compare by the lowercase album name and lowercase
-        // artist name. This allows for case-insensitive artist/album grouping, which can be common
-        // for albums/artists that have different naming (ex. "RAMMSTEIN" vs. "Rammstein").
-        private val artistKeys = inner.rawArtists.map { it.key }
-
-        // Cache the hash-code for HashMap efficiency.
-        private val hashCode =
-            inner.musicBrainzId?.hashCode()
-                ?: (31 * inner.name.lowercase().hashCode() + artistKeys.hashCode())
-
-        override fun hashCode() = hashCode
-
-        override fun equals(other: Any?) =
-            other is Key &&
-                when {
-                    inner.musicBrainzId != null && other.inner.musicBrainzId != null ->
-                        inner.musicBrainzId == other.inner.musicBrainzId
-                    inner.musicBrainzId == null && other.inner.musicBrainzId == null ->
-                        inner.name.equals(other.inner.name, true) && artistKeys == other.artistKeys
-                    else -> false
-                }
-    }
-}
 
 /**
  * Raw information about an [ArtistImpl] obtained from the component [SongImpl] and [AlbumImpl]
@@ -159,49 +126,12 @@ data class RawAlbum(
  */
 data class RawArtist(
     /** @see Music.UID */
-    val musicBrainzId: UUID? = null,
+    override val musicBrainzId: UUID? = null,
     /** @see Music.name */
-    val name: String? = null,
+    override val name: String? = null,
     /** @see Music.name */
     val sortName: String? = null
-) {
+) : MusicBrainzGroupable
-    val key = Key(this)
-
-    /**
-     * Allows [RawArtist]s to be compared by "fundamental" information that is unlikely to change on
-     * an item-by-item
-     */
-    data class Key(private val inner: RawArtist) {
-        // Artists are grouped as follows:
-        // - If we have a MusicBrainz ID, only group by it. This allows different Artists with the
-        // same name to be differentiated, which is common in large libraries.
-        // - If we do not have a MusicBrainz ID, compare by the lowercase name. This allows artist
-        // grouping to be case-insensitive.
-
-        // Cache the hashCode for HashMap efficiency.
-        val hashCode = inner.musicBrainzId?.hashCode() ?: inner.name?.lowercase().hashCode()
-
-        // Compare names and MusicBrainz IDs in order to differentiate artists with the
-        // same name in large libraries.
-
-        override fun hashCode() = hashCode
-
-        override fun equals(other: Any?) =
-            other is Key &&
-                when {
-                    inner.musicBrainzId != null && other.inner.musicBrainzId != null ->
-                        inner.musicBrainzId == other.inner.musicBrainzId
-                    inner.musicBrainzId == null && other.inner.musicBrainzId == null ->
-                        when {
-                            inner.name != null && other.inner.name != null ->
-                                inner.name.equals(other.inner.name, true)
-                            inner.name == null && other.inner.name == null -> true
-                            else -> false
-                        }
-                    else -> false
-                }
-    }
-}
 
 /**
  * Raw information about a [GenreImpl] obtained from the component [SongImpl] instances.
@@ -210,32 +140,15 @@ data class RawArtist(
  */
 data class RawGenre(
     /** @see Music.name */
-    val name: String? = null
+    override val name: String? = null
-) {
+) : NameGroupable
-    val key = Key(this)
 
-    /**
+interface NameGroupable {
-     * Allows [RawGenre]s to be compared by "fundamental" information that is unlikely to change on
+    val name: String?
-     * an item-by-item
+}
-     */
-    data class Key(private val inner: RawGenre) {
-        // Cache the hashCode for HashMap efficiency.
-        private val hashCode = inner.name?.lowercase().hashCode()
 
-        // Only group by the lowercase genre name. This allows Genre grouping to be
+interface MusicBrainzGroupable : NameGroupable {
-        // case-insensitive, which may be helpful in some libraries with different ways of
+    val musicBrainzId: UUID?
-        // formatting genres.
-        override fun hashCode() = hashCode
-
-        override fun equals(other: Any?) =
-            other is Key &&
-                when {
-                    inner.name != null && other.inner.name != null ->
-                        inner.name.equals(other.inner.name, true)
-                    inner.name == null && other.inner.name == null -> true
-                    else -> false
-                }
-    }
 }
 
 /**