music: refactor music grouping

Completely refactor the way music is grouped up into artists and
albums.

The issue with previous attempts at implementing better artist
management was the reliance on MediaStore IDs in many parts of the
program. Dumpster this by merging the hash and ID values into a
single field that is garunteed to be unique enough. This allows songs
to be adequately grouped into case-insensitive artists while also
deduplicating albums that may have been split my MediaStore due to
heterogeneous formats.

Resolves #66.
This commit is contained in:
OxygenCobalt 2022-02-04 06:45:18 -07:00
parent 6e00fd1129
commit b121b6428d
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
8 changed files with 201 additions and 158 deletions

View file

@ -25,8 +25,6 @@ import kotlinx.coroutines.withContext
import okio.buffer
import okio.source
import org.oxycblt.auxio.music.Album
import org.oxycblt.auxio.music.toAlbumArtURI
import org.oxycblt.auxio.music.toURI
import org.oxycblt.auxio.settings.SettingsManager
import org.oxycblt.auxio.util.logD
import java.io.ByteArrayInputStream
@ -63,7 +61,7 @@ abstract class AuxioFetcher : Fetcher {
@Suppress("BlockingMethodInNonBlockingContext")
private suspend fun fetchMediaStoreCovers(context: Context, data: Album): InputStream? {
val uri = data.id.toAlbumArtURI()
val uri = data.albumCoverUri
// Eliminate any chance that this blocking call might mess up the cancellation process
return withContext(Dispatchers.IO) {
@ -114,7 +112,7 @@ abstract class AuxioFetcher : Fetcher {
extractor.use { ext ->
// This call is time-consuming but it also doesn't seem to hold up the main thread,
// so it's probably fine not to wrap it.
ext.setDataSource(context, album.songs[0].id.toURI())
ext.setDataSource(context, album.songs[0].uri)
// Get the embedded picture from MediaMetadataRetriever, which will return a full
// ByteArray of the cover without any compression artifacts.
@ -126,7 +124,7 @@ abstract class AuxioFetcher : Fetcher {
}
private suspend fun fetchExoplayerCover(context: Context, album: Album): InputStream? {
val uri = album.songs[0].id.toURI()
val uri = album.songs[0].uri
val future = MetadataRetriever.retrieveMetadata(
context, MediaItem.fromUri(uri)

View file

@ -18,6 +18,9 @@
package org.oxycblt.auxio.music
import android.content.ContentUris
import android.net.Uri
import android.provider.MediaStore
import android.view.View
import androidx.annotation.DrawableRes
import androidx.annotation.StringRes
@ -26,7 +29,7 @@ import androidx.annotation.StringRes
/**
* The base data object for all music.
* @property id The ID that is assigned to this object
* @property id A unique ID for this object. ***THIS IS NOT A MEDIASTORE ID!**
*/
sealed class BaseModel {
abstract val id: Long
@ -35,11 +38,9 @@ sealed class BaseModel {
/**
* A [BaseModel] variant that represents a music item.
* @property name The raw name of this track
* @property hash A stable, unique-ish hash for this item. Used for database work.
*/
sealed class Music : BaseModel() {
abstract val name: String
abstract val hash: Long
}
/**
@ -56,48 +57,67 @@ sealed class MusicParent : Music() {
* The data object for a song. Inherits [BaseModel].
*/
data class Song(
override val id: Long,
override val name: String,
/** The file name of this song, excluding the full path. */
val fileName: String,
val albumName: String,
val albumId: Long,
val artistName: String?,
val albumArtistName: String?,
val year: Int,
/** The total duration of this song, in millis. */
val duration: Long,
/** The track number of this song. */
val track: Int,
val duration: Long
/** Internal field. Do not use. */
val _mediaStoreId: Long,
/** Internal field. Do not use. */
val _mediaStoreArtistName: String?,
/** Internal field. Do not use. */
val _mediaStoreAlbumArtistName: String?,
/** Internal field. Do not use. */
val _mediaStoreAlbumId: Long,
/** Internal field. Do not use. */
val _mediaStoreAlbumName: String,
/** Internal field. Do not use. */
val _mediaStoreYear: Int
) : Music() {
private var mAlbum: Album? = null
private var mGenre: Genre? = null
val genre: Genre? get() = mGenre
val album: Album get() = requireNotNull(mAlbum)
val seconds: Long get() = duration / 1000
val formattedDuration: String get() = seconds.toDuration(false)
override val hash: Long get() {
override val id: Long get() {
var result = name.hashCode().toLong()
result = 31 * result + albumName.hashCode()
result = 31 * result + artistName.hashCode()
result = 31 * result + album.name.hashCode()
result = 31 * result + album.artist.name.hashCode()
result = 31 * result + track
result = 31 * result + duration.hashCode()
return result
}
/** The URI for this song. */
val uri: Uri get() = ContentUris.withAppendedId(
MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, _mediaStoreId
)
/** The duration of this song, in seconds (rounded down) */
val seconds: Long get() = duration / 1000
/** The seconds of this song, but as a duration. */
val formattedDuration: String get() = seconds.toDuration(false)
private var mAlbum: Album? = null
/** The album of this song. */
val album: Album get() = requireNotNull(mAlbum)
var mGenre: Genre? = null
/** The genre of this song. May be null due to MediaStore insanity. */
val genre: Genre? get() = mGenre
/** An album name resolved to this song in particular. */
val resolvedAlbumName: String get() =
album.resolvedName
/** An artist name resolved to this song in particular. */
val resolvedArtistName: String get() =
artistName ?: album.artist.resolvedName
_mediaStoreArtistName ?: album.artist.resolvedName
fun linkAlbum(album: Album) {
/** Internal method. Do not use. */
fun mediaStoreLinkAlbum(album: Album) {
mAlbum = album
}
fun linkGenre(genre: Genre) {
/** Internal method. Do not use. */
fun mediaStoreLinkGenre(genre: Genre) {
mGenre = genre
}
}
@ -106,38 +126,46 @@ data class Song(
* The data object for an album. Inherits [MusicParent].
*/
data class Album(
override val id: Long,
override val name: String,
val artistName: String,
/** The latest year of the songs in this album. */
val year: Int,
val songs: List<Song>
/** The URI for the cover art corresponding to this album. */
val albumCoverUri: Uri,
/** The songs of this album. */
val songs: List<Song>,
/** Internal field. Do not use. */
val _mediaStoreArtistName: String,
) : MusicParent() {
init {
songs.forEach { song ->
song.linkAlbum(this)
song.mediaStoreLinkAlbum(this)
}
}
private var mArtist: Artist? = null
val artist: Artist get() = requireNotNull(mArtist)
val totalDuration: String get() =
songs.sumOf { it.seconds }.toDuration(false)
override val resolvedName: String
get() = name
val resolvedArtistName: String get() =
artist.resolvedName
override val hash: Long get() {
override val id: Long get() {
var result = name.hashCode().toLong()
result = 31 * result + artistName.hashCode()
result = 31 * result + artist.name.hashCode()
result = 31 * result + year
return result
}
fun linkArtist(artist: Artist) {
override val resolvedName: String
get() = name
/** The formatted total duration of this album */
val totalDuration: String get() =
songs.sumOf { it.seconds }.toDuration(false)
private var mArtist: Artist? = null
/** The parent artist of this album. */
val artist: Artist get() = requireNotNull(mArtist)
/** The artist name, resolved to this album in particular. */
val resolvedArtistName: String get() =
artist.resolvedName
/** Internal method. Do not use. */
fun mediaStoreLinkArtist(artist: Artist) {
mArtist = artist
}
}
@ -147,41 +175,46 @@ data class Album(
* album artist or artist field, not the individual performers of an artist.
*/
data class Artist(
override val id: Long,
override val name: String,
override val resolvedName: String,
/** The albums of this artist. */
val albums: List<Album>
) : MusicParent() {
init {
albums.forEach { album ->
album.linkArtist(this)
album.mediaStoreLinkArtist(this)
}
}
override val id = name.hashCode().toLong()
/** The songs of this artist. */
val songs = albums.flatMap { it.songs }
override val hash = name.hashCode().toLong()
}
/**
* The data object for a genre. Inherits [MusicParent]
*/
data class Genre(
override val id: Long,
override val name: String,
override val resolvedName: String
override val resolvedName: String,
/** Internal field. Do not use. */
val _mediaStoreId: Long
) : MusicParent() {
private val mSongs = mutableListOf<Song>()
val songs: List<Song> get() = mSongs
override val id = name.hashCode().toLong()
/** The formatted total duration of this genre */
val totalDuration: String get() =
songs.sumOf { it.seconds }.toDuration(false)
private val mSongs = mutableListOf<Song>()
val songs: List<Song> get() = mSongs
/** Internal method. Do not use. */
fun linkSong(song: Song) {
mSongs.add(song)
song.linkGenre(this)
song.mediaStoreLinkGenre(this)
}
override val hash = name.hashCode().toLong()
}
/**
@ -189,6 +222,7 @@ data class Genre(
*/
data class Header(
override val id: Long,
/** The string resource used for the header. */
@StringRes val string: Int
) : BaseModel()
@ -198,12 +232,16 @@ data class Header(
*/
data class ActionHeader(
override val id: Long,
/** The string resource used for the header. */
@StringRes val string: Int,
/** The icon resource used for the header action. */
@DrawableRes val icon: Int,
/** The string resource used for the header action's content description. */
@StringRes val desc: Int,
/** A callback for when this item is clicked. */
val onClick: (View) -> Unit,
) : BaseModel() {
// JVM can't into comparing lambdas, so we override equals/hashCode and exclude them.
// All lambdas are not equal to each-other, so we override equals/hashCode and exclude them.
override fun equals(other: Any?): Boolean {
if (this === other) return true

View file

@ -1,10 +1,14 @@
package org.oxycblt.auxio.music
import android.content.ContentUris
import android.content.Context
import android.net.Uri
import android.provider.MediaStore
import androidx.core.database.getStringOrNull
import org.oxycblt.auxio.R
import org.oxycblt.auxio.excluded.ExcludedDatabase
import org.oxycblt.auxio.util.logE
import java.lang.Exception
/**
* This class acts as the base for most the black magic required to get a remotely sensible music
@ -84,6 +88,16 @@ class MusicLoader {
val artists = buildArtists(context, albums)
val genres = readGenres(context, songs)
// Sanity check: Ensure that all songs are well-formed.
for (song in songs) {
try {
song.album.artist
} catch (e: Exception) {
logE("Found malformed song: ${song.name}")
throw e
}
}
return Library(
genres,
artists,
@ -156,109 +170,112 @@ class MusicLoader {
songs.add(
Song(
id,
title,
fileName,
album,
albumId,
duration,
track,
id,
artist,
albumArtist,
albumId,
album,
year,
track,
duration
)
)
}
}
songs = songs.distinctBy {
it.name to it.albumName to it.artistName to it.track to it.duration
it.name to it._mediaStoreAlbumName to it._mediaStoreArtistName to it._mediaStoreAlbumArtistName to it.track to it.duration
}.toMutableList()
return songs
}
private fun buildAlbums(songs: List<Song>): List<Album> {
// Group up songs by their album ids and then link them with their albums
// 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:
// 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"
// 2. Sometimes MediaStore will split album IDs up if the songs differ in format. This
// ensures that all songs are unified under a single album.
// This does come with some costs, it's far slower than using the album ID itself, and it
// may result in an unrelated album art being selected depending on the song chosen as
// the template, but it seems to work pretty well.
val albums = mutableListOf<Album>()
val songsByAlbum = songs.groupBy { it.albumId }
val songsByAlbum = songs.groupBy { song ->
val albumName = song._mediaStoreAlbumName
val artistName = song.resolveAlbumArtistName()
Pair(albumName.lowercase(), artistName.lowercase())
}
for (entry in songsByAlbum) {
val albumId = entry.key
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 = requireNotNull(albumSongs.maxByOrNull { it.year })
val albumName = templateSong.albumName
val templateSong = requireNotNull(albumSongs.maxByOrNull { it._mediaStoreYear })
val albumName = templateSong._mediaStoreAlbumName
val albumYear = templateSong._mediaStoreYear
val albumCoverUri = ContentUris.withAppendedId(
Uri.parse("content://media/external/audio/albumart"),
templateSong._mediaStoreAlbumId
)
val artistName = templateSong.resolveAlbumArtistName()
// 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.
val artistName = templateSong.albumArtistName
?: templateSong.artistName ?: MediaStore.UNKNOWN_STRING
val albumYear = templateSong.year
// Search for duplicate albums first. This serves two purposes:
// 1. It collapses differently styled artists [ex. Rammstein vs. RAMMSTEIN] into
// a single grouped artist
// 2. It also unifies albums that exist across several file formats [excluding
// when the titles are mangled by MediaStore insanity]
val previousAlbumIndex = albums.indexOfFirst { album ->
album.name.lowercase() == albumName.lowercase() &&
album.artistName.lowercase() == artistName.lowercase()
}
if (previousAlbumIndex > -1) {
val previousAlbum = albums[previousAlbumIndex]
albums[previousAlbumIndex] = Album(
previousAlbum.id,
previousAlbum.name,
previousAlbum.artistName,
previousAlbum.year,
previousAlbum.songs + albumSongs
albums.add(
Album(
albumName,
albumYear,
albumCoverUri,
albumSongs,
artistName,
)
} else {
albums.add(
Album(
albumId,
albumName,
artistName,
albumYear,
albumSongs
)
)
}
)
}
albums.removeAll { it.songs.isEmpty() }
return albums
}
private fun buildArtists(context: Context, albums: List<Album>): List<Artist> {
val artists = mutableListOf<Artist>()
val albumsByArtist = albums.groupBy { it.artistName }
val albumsByArtist = albums.groupBy { it._mediaStoreArtistName }
for (entry in albumsByArtist) {
val name = entry.key
val resolvedName = when (name) {
val artistName = entry.key
val resolvedName = when (artistName) {
MediaStore.UNKNOWN_STRING -> context.getString(R.string.def_artist)
else -> name
else -> artistName
}
val artistAlbums = entry.value
// Due to the black magic we do to get a good artist field, the ID is unreliable.
// Take a hash of the artist name instead.
artists.add(
Artist(
name.hashCode().toLong(),
name,
resolvedName,
artistAlbums
val previousArtistIndex = artists.indexOfFirst { artist ->
artist.name.lowercase() == artistName.lowercase()
}
if (previousArtistIndex > -1) {
val previousArtist = artists[previousArtistIndex]
artists[previousArtistIndex] = Artist(
previousArtist.name,
previousArtist.resolvedName,
previousArtist.albums + artistAlbums
)
)
} else {
artists.add(
Artist(
artistName,
resolvedName,
artistAlbums
)
)
}
}
return artists
@ -291,7 +308,7 @@ class MusicLoader {
else -> name.getGenreNameCompat() ?: name
}
val genre = Genre(id, name, resolvedName)
val genre = Genre(name, resolvedName, id)
linkGenre(context, genre, songs)
genres.add(genre)
@ -300,9 +317,9 @@ class MusicLoader {
// Songs that don't have a genre will be thrown into an unknown genre.
val unknownGenre = Genre(
id = Long.MIN_VALUE,
name = MediaStore.UNKNOWN_STRING,
resolvedName = context.getString(R.string.def_genre)
resolvedName = context.getString(R.string.def_genre),
Long.MIN_VALUE
)
songs.forEach { song ->
@ -321,7 +338,7 @@ class MusicLoader {
private fun linkGenre(context: Context, genre: Genre, songs: List<Song>) {
// Don't even bother blacklisting here as useless iterations are less expensive than IO
val songCursor = context.contentResolver.query(
MediaStore.Audio.Genres.Members.getContentUri("external", genre.id),
MediaStore.Audio.Genres.Members.getContentUri("external", genre._mediaStoreId),
arrayOf(MediaStore.Audio.Genres.Members._ID),
null, null, null
)
@ -332,7 +349,7 @@ class MusicLoader {
while (cursor.moveToNext()) {
val id = cursor.getLong(idIndex)
songs.find { it.id == id }?.let { song ->
songs.find { it._mediaStoreId == id }?.let { song ->
genre.linkSong(song)
}
}

View file

@ -88,10 +88,10 @@ class MusicStore private constructor() {
}
/**
* Find a song in a faster manner using a hash for its album as well.
* Find a song in a faster manner using an ID for its album as well.
*/
fun findSongFast(songHash: Long, albumHash: Long): Song? {
return albums.find { it.hash == albumHash }?.songs?.find { it.hash == songHash }
fun findSongFast(songId: Long, albumId: Long): Song? {
return albums.find { it.id == albumId }?.songs?.find { it.id == songId }
}
/**

View file

@ -18,10 +18,7 @@
package org.oxycblt.auxio.music
import android.content.ContentUris
import android.content.Context
import android.net.Uri
import android.provider.MediaStore
import android.text.format.DateUtils
import android.widget.TextView
import androidx.core.text.isDigitsOnly
@ -94,20 +91,6 @@ fun String.getGenreNameCompat(): String? {
return null
}
/**
* Convert an id to its corresponding URI
*/
fun Long.toURI(): Uri {
return ContentUris.withAppendedId(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, this)
}
/**
* Get the URI for an album's cover art, corresponds to MediaStore.
*/
fun Long.toAlbumArtURI(): Uri {
return ContentUris.withAppendedId(Uri.parse("content://media/external/audio/albumart"), this)
}
/**
* Convert a [Long] of seconds into a string duration.
* @param isElapsed Whether this duration is represents elapsed time. If this is false, then

View file

@ -116,9 +116,9 @@ class PlaybackStateDatabase(context: Context) :
val stateData = ContentValues(10).apply {
put(StateColumns.COLUMN_ID, 0)
put(StateColumns.COLUMN_SONG_HASH, state.song?.hash)
put(StateColumns.COLUMN_SONG_HASH, state.song?.id)
put(StateColumns.COLUMN_POSITION, state.position)
put(StateColumns.COLUMN_PARENT_HASH, state.parent?.hash)
put(StateColumns.COLUMN_PARENT_HASH, state.parent?.id)
put(StateColumns.COLUMN_QUEUE_INDEX, state.queueIndex)
put(StateColumns.COLUMN_PLAYBACK_MODE, state.playbackMode.toInt())
put(StateColumns.COLUMN_IS_SHUFFLING, state.isShuffling)
@ -154,17 +154,17 @@ class PlaybackStateDatabase(context: Context) :
cursor.moveToFirst()
val song = cursor.getLongOrNull(songIndex)?.let { hash ->
musicStore.songs.find { it.hash == hash }
val song = cursor.getLongOrNull(songIndex)?.let { id ->
musicStore.songs.find { it.id == id }
}
val mode = PlaybackMode.fromInt(cursor.getInt(modeIndex)) ?: PlaybackMode.ALL_SONGS
val parent = cursor.getLongOrNull(parentIndex)?.let { hash ->
val parent = cursor.getLongOrNull(parentIndex)?.let { id ->
when (mode) {
PlaybackMode.IN_GENRE -> musicStore.genres.find { it.hash == hash }
PlaybackMode.IN_ARTIST -> musicStore.artists.find { it.hash == hash }
PlaybackMode.IN_ALBUM -> musicStore.albums.find { it.hash == hash }
PlaybackMode.IN_GENRE -> musicStore.genres.find { it.id == id }
PlaybackMode.IN_ARTIST -> musicStore.artists.find { it.id == id }
PlaybackMode.IN_ALBUM -> musicStore.albums.find { it.id == id }
PlaybackMode.ALL_SONGS -> null
}
}
@ -216,8 +216,8 @@ class PlaybackStateDatabase(context: Context) :
val itemData = ContentValues(4).apply {
put(QueueColumns.ID, idStart + i)
put(QueueColumns.SONG_HASH, song.hash)
put(QueueColumns.ALBUM_HASH, song.album.hash)
put(QueueColumns.SONG_HASH, song.id)
put(QueueColumns.ALBUM_HASH, song.album.id)
}
insert(TABLE_NAME_QUEUE, null, itemData)

View file

@ -54,7 +54,6 @@ import kotlinx.coroutines.launch
import org.oxycblt.auxio.BuildConfig
import org.oxycblt.auxio.music.MusicParent
import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.music.toURI
import org.oxycblt.auxio.playback.state.LoopMode
import org.oxycblt.auxio.playback.state.PlaybackStateManager
import org.oxycblt.auxio.settings.SettingsManager
@ -261,7 +260,7 @@ class PlaybackService : Service(), Player.Listener, PlaybackStateManager.Callbac
override fun onSongUpdate(song: Song?) {
if (song != null) {
player.setMediaItem(MediaItem.fromUri(song.id.toURI()))
player.setMediaItem(MediaItem.fromUri(song.uri))
player.prepare()
notification.setMetadata(song, ::startForegroundOrNotify)

View file

@ -104,10 +104,18 @@ to a name that can be used in UIs.
while `ActionHeader` corresponds to an action with a dedicated icon, such as with sorting.
Other data types represent a specific UI configuration or state:
- Sealed classes like `Sort` and `HeaderString` contain data with them that can be modified.
- Enums like `DisplayMode` and `LoopMode` only contain static data, such as a string resource.
Things to keep in mind while working with music data:
- `id` is not derived from the `MediaStore` ID of the music data. It is actually a hash of the unique fields of the music data.
Attempting to use it as a `MediaStore` ID will result in errors.
- Any field beginning with `_mediaStore` is off-limits. These fields are meant for use within MusicLoader and generally provide
poor UX to the user.
- Generally, `name` is used when saving music data to storage, while `resolvedName` is used when displaying music data to the user.
- For `Song` instances in particular, prefer `resolvedAlbumName` and `resolvedArtistName` over `album.resolvedName` and `album.artist.resolvedName`
- For `Album` instances in particular, prefer `resolvedArtistName` over `artist.resolvedName`
#### Music Access
All music on a system is asynchronously loaded into the shared object `MusicStore`. Because of this, **`MusicStore` may not be available at all times**.