music: modify model configuration

Do a couple things to the music models:
1. Make the genre field non-nullable. This is because I beleive I've
largely eliminated the genre bugs in previous versions and future ones
can be caught with a crash screen I plan to add.
2. Make the initial album grouping process use hashCode instead of a
pair of names. This just helps with loading speed in general, albeit I
am slightly worried that it may result in improper grouping if some
edge case appears.
This commit is contained in:
OxygenCobalt 2022-02-08 06:53:53 -07:00
parent f4217a337a
commit 04bec3161f
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
2 changed files with 42 additions and 34 deletions

View file

@ -65,17 +65,17 @@ data class Song(
/** The track number of this song. */ /** The track number of this song. */
val track: Int, val track: Int,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreId: Long, val internalMediaStoreId: Long,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreArtistName: String?, val internalMediaStoreArtistName: String?,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreAlbumArtistName: String?, val internalMediaStoreAlbumArtistName: String?,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreAlbumId: Long, val internalMediaStoreAlbumId: Long,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreAlbumName: String, val internalMediaStoreAlbumName: String,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreYear: Int val internalMediaStoreYear: Int
) : Music() { ) : Music() {
override val id: Long get() { override val id: Long get() {
var result = name.hashCode().toLong() var result = name.hashCode().toLong()
@ -88,7 +88,7 @@ data class Song(
/** The URI for this song. */ /** The URI for this song. */
val uri: Uri get() = ContentUris.withAppendedId( val uri: Uri get() = ContentUris.withAppendedId(
MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, _mediaStoreId MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, internalMediaStoreId
) )
/** The duration of this song, in seconds (rounded down) */ /** The duration of this song, in seconds (rounded down) */
val seconds: Long get() = duration / 1000 val seconds: Long get() = duration / 1000
@ -100,8 +100,8 @@ data class Song(
val album: Album get() = requireNotNull(mAlbum) val album: Album get() = requireNotNull(mAlbum)
var mGenre: Genre? = null var mGenre: Genre? = null
/** The genre of this song. May be null due to MediaStore insanity. */ /** The genre of this song. Will be an "unknown genre" if the song does not have any. */
val genre: Genre? get() = mGenre val genre: Genre get() = requireNotNull(mGenre)
/** An album name resolved to this song in particular. */ /** An album name resolved to this song in particular. */
val resolvedAlbumName: String get() = val resolvedAlbumName: String get() =
@ -109,15 +109,29 @@ data class Song(
/** An artist name resolved to this song in particular. */ /** An artist name resolved to this song in particular. */
val resolvedArtistName: String get() = val resolvedArtistName: String get() =
_mediaStoreArtistName ?: album.artist.resolvedName internalMediaStoreArtistName ?: album.artist.resolvedName
/** Internal field. Do not use. */
val internalGroupingId: Int get() {
var result = internalGroupingArtistName.lowercase().hashCode()
result = 31 * result + internalMediaStoreAlbumName.lowercase().hashCode()
return result
}
/** Internal field. Do not use. */
val internalGroupingArtistName: String get() = internalMediaStoreAlbumArtistName
?: internalMediaStoreArtistName ?: MediaStore.UNKNOWN_STRING
/** Internal field. Do not use. **/
val internalMissingGenre: Boolean get() = mGenre == null
/** Internal method. Do not use. */ /** Internal method. Do not use. */
fun mediaStoreLinkAlbum(album: Album) { fun internalLinkAlbum(album: Album) {
mAlbum = album mAlbum = album
} }
/** Internal method. Do not use. */ /** Internal method. Do not use. */
fun mediaStoreLinkGenre(genre: Genre) { fun internalLinkGenre(genre: Genre) {
mGenre = genre mGenre = genre
} }
} }
@ -134,11 +148,11 @@ data class Album(
/** The songs of this album. */ /** The songs of this album. */
val songs: List<Song>, val songs: List<Song>,
/** Internal field. Do not use. */ /** Internal field. Do not use. */
val _mediaStoreArtistName: String, val internalGroupingArtistName: String,
) : MusicParent() { ) : MusicParent() {
init { init {
for (song in songs) { for (song in songs) {
song.mediaStoreLinkAlbum(this) song.internalLinkAlbum(this)
} }
} }
@ -165,7 +179,7 @@ data class Album(
artist.resolvedName artist.resolvedName
/** Internal method. Do not use. */ /** Internal method. Do not use. */
fun mediaStoreLinkArtist(artist: Artist) { fun internalLinkArtist(artist: Artist) {
mArtist = artist mArtist = artist
} }
} }
@ -182,7 +196,7 @@ data class Artist(
) : MusicParent() { ) : MusicParent() {
init { init {
for (album in albums) { for (album in albums) {
album.mediaStoreLinkArtist(this) album.internalLinkArtist(this)
} }
} }
@ -202,7 +216,7 @@ data class Genre(
) : MusicParent() { ) : MusicParent() {
init { init {
for (song in songs) { for (song in songs) {
song.mediaStoreLinkGenre(this) song.internalLinkGenre(this)
} }
} }

View file

@ -92,6 +92,7 @@ class MusicLoader {
for (song in songs) { for (song in songs) {
try { try {
song.album.artist song.album.artist
song.genre
} catch (e: Exception) { } catch (e: Exception) {
logE("Found malformed song: ${song.name}") logE("Found malformed song: ${song.name}")
throw e throw e
@ -187,18 +188,13 @@ class MusicLoader {
} }
songs = songs.distinctBy { songs = songs.distinctBy {
it.name to it._mediaStoreAlbumName to it._mediaStoreArtistName to it._mediaStoreAlbumArtistName to it.track to it.duration it.name to it.internalMediaStoreAlbumName to it.internalMediaStoreArtistName to it.internalMediaStoreAlbumArtistName to it.track to it.duration
}.toMutableList() }.toMutableList()
return songs return songs
} }
private fun buildAlbums(songs: List<Song>): List<Album> { private fun buildAlbums(songs: List<Song>): List<Album> {
// When assigning an artist to an album, use the album artist first, then the
// normal artist, and then the internal representation of an unknown artist name.
fun Song.resolveAlbumArtistName() = _mediaStoreAlbumArtistName ?: _mediaStoreArtistName
?: MediaStore.UNKNOWN_STRING
// Group up songs by their lowercase artist and album name. This serves two purposes: // Group up songs by their lowercase artist and album name. This serves two purposes:
// 1. Sometimes artist names can be styled differently, e.g "Rammstein" vs. "RAMMSTEIN". // 1. Sometimes artist names can be styled differently, e.g "Rammstein" vs. "RAMMSTEIN".
// This makes sure both of those are resolved into a single artist called "Rammstein" // This makes sure both of those are resolved into a single artist called "Rammstein"
@ -209,9 +205,7 @@ class MusicLoader {
// the template, but it seems to work pretty well. // the template, but it seems to work pretty well.
val albums = mutableListOf<Album>() val albums = mutableListOf<Album>()
val songsByAlbum = songs.groupBy { song -> val songsByAlbum = songs.groupBy { song ->
val albumName = song._mediaStoreAlbumName song.internalGroupingId
val artistName = song.resolveAlbumArtistName()
Pair(albumName.lowercase(), artistName.lowercase())
} }
for (entry in songsByAlbum) { for (entry in songsByAlbum) {
@ -220,14 +214,14 @@ class MusicLoader {
// Use the song with the latest year as our metadata song. // 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 // 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. // weird years like "0" wont show up if there are alternatives.
val templateSong = requireNotNull(albumSongs.maxByOrNull { it._mediaStoreYear }) val templateSong = requireNotNull(albumSongs.maxByOrNull { it.internalMediaStoreYear })
val albumName = templateSong._mediaStoreAlbumName val albumName = templateSong.internalMediaStoreAlbumName
val albumYear = templateSong._mediaStoreYear val albumYear = templateSong.internalMediaStoreYear
val albumCoverUri = ContentUris.withAppendedId( val albumCoverUri = ContentUris.withAppendedId(
Uri.parse("content://media/external/audio/albumart"), Uri.parse("content://media/external/audio/albumart"),
templateSong._mediaStoreAlbumId templateSong.internalMediaStoreAlbumId
) )
val artistName = templateSong.resolveAlbumArtistName() val artistName = templateSong.internalGroupingArtistName
albums.add( albums.add(
Album( Album(
@ -245,7 +239,7 @@ class MusicLoader {
private fun buildArtists(context: Context, albums: List<Album>): List<Artist> { private fun buildArtists(context: Context, albums: List<Album>): List<Artist> {
val artists = mutableListOf<Artist>() val artists = mutableListOf<Artist>()
val albumsByArtist = albums.groupBy { it._mediaStoreArtistName } val albumsByArtist = albums.groupBy { it.internalGroupingArtistName }
for (entry in albumsByArtist) { for (entry in albumsByArtist) {
val artistName = entry.key val artistName = entry.key
@ -318,7 +312,7 @@ class MusicLoader {
} }
} }
val songsWithoutGenres = songs.filter { it.genre == null } val songsWithoutGenres = songs.filter { it.internalMissingGenre }
if (songsWithoutGenres.isNotEmpty()) { if (songsWithoutGenres.isNotEmpty()) {
// Songs that don't have a genre will be thrown into an unknown genre. // Songs that don't have a genre will be thrown into an unknown genre.
@ -350,7 +344,7 @@ class MusicLoader {
while (cursor.moveToNext()) { while (cursor.moveToNext()) {
val id = cursor.getLong(idIndex) val id = cursor.getLong(idIndex)
songs.find { it._mediaStoreId == id }?.let { song -> songs.find { it.internalMediaStoreId == id }?.let { song ->
genreSongs.add(song) genreSongs.add(song)
} }
} }