diff --git a/app/src/main/java/org/oxycblt/auxio/coil/AuxioFetcher.kt b/app/src/main/java/org/oxycblt/auxio/coil/AuxioFetcher.kt index 6c52f9b87..910e4c43c 100644 --- a/app/src/main/java/org/oxycblt/auxio/coil/AuxioFetcher.kt +++ b/app/src/main/java/org/oxycblt/auxio/coil/AuxioFetcher.kt @@ -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) diff --git a/app/src/main/java/org/oxycblt/auxio/music/Models.kt b/app/src/main/java/org/oxycblt/auxio/music/Models.kt index e30d0addc..6d5f8b7e5 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Models.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Models.kt @@ -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 + /** The URI for the cover art corresponding to this album. */ + val albumCoverUri: Uri, + /** The songs of this album. */ + val songs: List, + /** 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 ) : 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() - val songs: List 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() + val songs: List 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 diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt index 29f2d7902..47d25b07d 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt @@ -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): List { - // 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() - 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): List { val artists = mutableListOf() - 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) { // 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) } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt index 0ca70a191..22addc747 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -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 } } /** diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicUtils.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicUtils.kt index 8bf54b984..a20a83ca2 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicUtils.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicUtils.kt @@ -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 diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt index e7558cb95..5ed2a2b35 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateDatabase.kt @@ -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) diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt index 97febe5f8..15b76ed4f 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt @@ -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) diff --git a/info/ARCHITECTURE.md b/info/ARCHITECTURE.md index 5cedeea46..c38364d23 100644 --- a/info/ARCHITECTURE.md +++ b/info/ARCHITECTURE.md @@ -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**.