From f52fa7f338ae0a4b62a6bde6952888b83f7b84ed Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sun, 29 May 2022 10:09:49 -0600 Subject: [PATCH] music: cleanup implementation Clean up the indexer implementation even further. --- .../java/org/oxycblt/auxio/music/Music.kt | 257 +---------------- .../org/oxycblt/auxio/music/MusicStore.kt | 6 + .../auxio/music/indexer/ExoPlayerBackend.kt | 53 ++-- .../oxycblt/auxio/music/indexer/Indexer.kt | 3 +- .../auxio/music/indexer/IndexerUtil.kt | 272 +++++++++++++++++- .../auxio/music/indexer/MediaStoreBackend.kt | 33 ++- .../oxycblt/auxio/playback/StyledSeekBar.kt | 2 + .../playback/state/PlaybackStateManager.kt | 6 +- .../org/oxycblt/auxio/util/PrimitiveUtil.kt | 3 - 9 files changed, 333 insertions(+), 302 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index 518e9ae2e..658c13042 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -22,8 +22,9 @@ package org.oxycblt.auxio.music import android.content.Context import android.net.Uri import android.provider.MediaStore -import androidx.core.text.isDigitsOnly import org.oxycblt.auxio.R +import org.oxycblt.auxio.music.indexer.id3v2GenreName +import org.oxycblt.auxio.music.indexer.withoutArticle import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.util.unlikelyToBeNull @@ -250,258 +251,8 @@ data class Genre(override val rawName: String?, override val songs: List) get() = (rawName ?: MediaStore.UNKNOWN_STRING).hashCode().toLong() override val sortName: String? - get() = rawName?.genreNameCompat + get() = rawName?.id3v2GenreName override fun resolveName(context: Context) = - rawName?.genreNameCompat ?: context.getString(R.string.def_genre) + rawName?.id3v2GenreName ?: context.getString(R.string.def_genre) } - -/** - * Slice a string so that any preceding articles like The/A(n) are truncated. This is hilariously - * anglo-centric, but its mostly for MediaStore compat and hopefully shouldn't run with other - * languages. - */ -private val String.withoutArticle: String - get() { - if (length > 5 && startsWith("the ", ignoreCase = true)) { - return slice(4..lastIndex) - } - - if (length > 4 && startsWith("an ", ignoreCase = true)) { - return slice(3..lastIndex) - } - - if (length > 3 && startsWith("a ", ignoreCase = true)) { - return slice(2..lastIndex) - } - - return this - } - -/** - * Decodes the genre name from an ID3(v2) constant. See [genreConstantTable] for the genre constant - * map that Auxio uses. - */ -private val String.genreNameCompat: String - get() { - if (isDigitsOnly()) { - // ID3v1, just parse as an integer - return genreConstantTable.getOrNull(toInt()) ?: this - } - - if (startsWith('(') && endsWith(')')) { - // ID3v2.3/ID3v2.4, parse out the parentheses and get the integer - // Any genres formatted as "(CHARS)" will be ignored. - val genreInt = substring(1 until lastIndex).toIntOrNull() - if (genreInt != null) { - return genreConstantTable.getOrNull(genreInt) ?: this - } - } - - // Current name is fine. - return this - } - -/** - * A complete table of all the constant genre values for ID3(v2), including non-standard extensions. - */ -private val genreConstantTable = - arrayOf( - // ID3 Standard - "Blues", - "Classic Rock", - "Country", - "Dance", - "Disco", - "Funk", - "Grunge", - "Hip-Hop", - "Jazz", - "Metal", - "New Age", - "Oldies", - "Other", - "Pop", - "R&B", - "Rap", - "Reggae", - "Rock", - "Techno", - "Industrial", - "Alternative", - "Ska", - "Death Metal", - "Pranks", - "Soundtrack", - "Euro-Techno", - "Ambient", - "Trip-Hop", - "Vocal", - "Jazz+Funk", - "Fusion", - "Trance", - "Classical", - "Instrumental", - "Acid", - "House", - "Game", - "Sound Clip", - "Gospel", - "Noise", - "AlternRock", - "Bass", - "Soul", - "Punk", - "Space", - "Meditative", - "Instrumental Pop", - "Instrumental Rock", - "Ethnic", - "Gothic", - "Darkwave", - "Techno-Industrial", - "Electronic", - "Pop-Folk", - "Eurodance", - "Dream", - "Southern Rock", - "Comedy", - "Cult", - "Gangsta", - "Top 40", - "Christian Rap", - "Pop/Funk", - "Jungle", - "Native American", - "Cabaret", - "New Wave", - "Psychadelic", - "Rave", - "Showtunes", - "Trailer", - "Lo-Fi", - "Tribal", - "Acid Punk", - "Acid Jazz", - "Polka", - "Retro", - "Musical", - "Rock & Roll", - "Hard Rock", - - // Winamp extensions, more or less a de-facto standard - "Folk", - "Folk-Rock", - "National Folk", - "Swing", - "Fast Fusion", - "Bebob", - "Latin", - "Revival", - "Celtic", - "Bluegrass", - "Avantgarde", - "Gothic Rock", - "Progressive Rock", - "Psychedelic Rock", - "Symphonic Rock", - "Slow Rock", - "Big Band", - "Chorus", - "Easy Listening", - "Acoustic", - "Humour", - "Speech", - "Chanson", - "Opera", - "Chamber Music", - "Sonata", - "Symphony", - "Booty Bass", - "Primus", - "Porn Groove", - "Satire", - "Slow Jam", - "Club", - "Tango", - "Samba", - "Folklore", - "Ballad", - "Power Ballad", - "Rhythmic Soul", - "Freestyle", - "Duet", - "Punk Rock", - "Drum Solo", - "A capella", - "Euro-House", - "Dance Hall", - "Goa", - "Drum & Bass", - "Club-House", - "Hardcore", - "Terror", - "Indie", - "Britpop", - "Negerpunk", - "Polsk Punk", - "Beat", - "Christian Gangsta", - "Heavy Metal", - "Black Metal", - "Crossover", - "Contemporary Christian", - "Christian Rock", - "Merengue", - "Salsa", - "Thrash Metal", - "Anime", - "JPop", - "Synthpop", - - // Winamp 5.6+ extensions, also used by EasyTAG. - // I only include this because post-rock is a based genre and deserves a slot. - "Abstract", - "Art Rock", - "Baroque", - "Bhangra", - "Big Beat", - "Breakbeat", - "Chillout", - "Downtempo", - "Dub", - "EBM", - "Eclectic", - "Electro", - "Electroclash", - "Emo", - "Experimental", - "Garage", - "Global", - "IDM", - "Illbient", - "Industro-Goth", - "Jam Band", - "Krautrock", - "Leftfield", - "Lounge", - "Math Rock", - "New Romantic", - "Nu-Breakz", - "Post-Punk", - "Post-Rock", - "Psytrance", - "Shoegaze", - "Space Rock", - "Trop Rock", - "World Music", - "Neoclassical", - "Audiobook", - "Audio Theatre", - "Neue Deutsche Welle", - "Podcast", - "Indie Rock", - "G-Funk", - "Dubstep", - "Garage Rock", - "Psybient") 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 92a942413..74b3d3a81 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -53,11 +53,17 @@ class MusicStore private constructor() { private val callbacks = mutableListOf() + /** + * Add a callback to this instance. Make sure to remove it when done. + */ fun addCallback(callback: Callback) { response?.let(callback::onMusicUpdate) callbacks.add(callback) } + /** + * Remove a callback from this instance. + */ fun removeCallback(callback: Callback) { callbacks.remove(callback) } diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt index 3a0232bb9..922524c8c 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt @@ -17,10 +17,8 @@ package org.oxycblt.auxio.music.indexer -import android.content.ContentUris import android.content.Context import android.database.Cursor -import android.provider.MediaStore import com.google.android.exoplayer2.MediaItem import com.google.android.exoplayer2.MetadataRetriever import com.google.android.exoplayer2.metadata.Metadata @@ -29,7 +27,6 @@ import com.google.android.exoplayer2.metadata.vorbis.VorbisComment import com.google.android.exoplayer2.source.TrackGroupArray import com.google.common.util.concurrent.FutureCallback import com.google.common.util.concurrent.Futures -import java.nio.charset.StandardCharsets import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.Executors import java.util.concurrent.Future @@ -39,11 +36,20 @@ import org.oxycblt.auxio.util.logW /** * A [Indexer.Backend] that leverages ExoPlayer's metadata retrieval system to index metadata. * + * Normally, leveraging ExoPlayer's metadata system would be a terrible idea, as it is horrifically + * slow. However, if we parallelize it, we can get similar throughput to other metadata extractors, + * which is nice as it means we don't have to bundle a redundant metadata library like JAudioTagger. + * + * Now, ExoPlayer's metadata API is not the best. It's opaque, undocumented, and prone to weird + * pitfalls given ExoPlayer's cozy relationship with native code. However, this backend should do + * enough to eliminate such issues. + * * TODO: This class is currently not used, as there are a number of technical improvements that must - * be made first before it is practical. + * be made first before it can be integrated. * * @author OxygenCobalt */ +@Suppress("UNUSED") class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { private val runningTasks: Array?> = arrayOfNulls(TASK_CAPACITY) @@ -58,21 +64,21 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { while (cursor.moveToNext()) { val audio = inner.buildAudio(context, cursor) - val audioUri = - ContentUris.withAppendedId( - MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, requireNotNull(audio.id)) + val audioUri = requireNotNull(audio.id) { "Malformed audio: No id" }.audioUri while (true) { // Spin until a task slot opens up, then start trying to parse metadata. - val idx = runningTasks.indexOfFirst { it == null } - if (idx != -1) { + val index = runningTasks.indexOfFirst { it == null } + if (index != -1) { val task = MetadataRetriever.retrieveMetadata(context, MediaItem.fromUri(audioUri)) Futures.addCallback( - task, AudioCallback(audio, songs, idx), Executors.newSingleThreadExecutor()) + task, + AudioCallback(audio, songs, index), + Executors.newSingleThreadExecutor()) - runningTasks[idx] = task + runningTasks[index] = task break } @@ -89,7 +95,7 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { private inner class AudioCallback( private val audio: MediaStoreBackend.Audio, private val dest: ConcurrentLinkedQueue, - private val taskIdx: Int + private val taskIndex: Int ) : FutureCallback { override fun onSuccess(result: TrackGroupArray) { val metadata = result[0].getFormat(0).metadata @@ -100,30 +106,33 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { } dest.add(audio.toSong()) - runningTasks[taskIdx] = null + runningTasks[taskIndex] = null } override fun onFailure(t: Throwable) { logW("Unable to extract metadata for ${audio.title}") logW(t.stackTraceToString()) dest.add(audio.toSong()) - runningTasks[taskIdx] = null + runningTasks[taskIndex] = null } } private fun completeAudio(audio: MediaStoreBackend.Audio, metadata: Metadata) { for (i in 0 until metadata.length()) { + // We only support two formats as it stands: + // - ID3v2 text frames + // - Vorbis comments + // This should be enough to cover the vast, vast majority of audio formats. + // It is also assumed that a file only has either ID3v2 text frames or vorbis + // comments. when (val tag = metadata.get(i)) { - // ID3v2 text information frame. is TextInformationFrame -> if (tag.value.isNotEmpty()) { - handleId3TextFrame(tag.id, tag.value.sanitize(), audio) + handleId3v2TextFrame(tag.id.sanitize(), tag.value.sanitize(), audio) } - // Vorbis comment. It is assumed that a Metadata can only have vorbis - // comments/pictures or ID3 frames, never both. is VorbisComment -> if (tag.value.isNotEmpty()) { - handleVorbisComment(tag.key, tag.value.sanitize(), audio) + handleVorbisComment(tag.key.sanitize(), tag.value.sanitize(), audio) } } } @@ -138,12 +147,12 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { * * This function mitigates it by first encoding the string as UTF-8 bytes (replacing malformed * characters with the replacement in the process), and then re-interpreting it as a new string, - * which hopefully fixes encoding sanity while also copying the string out of dodgy native + * which hopefully fixes encoding insanity while also copying the string out of dodgy native * memory. */ - private fun String.sanitize() = String(encodeToByteArray(), StandardCharsets.UTF_8) + private fun String.sanitize() = String(encodeToByteArray()) - private fun handleId3TextFrame(id: String, value: String, audio: MediaStoreBackend.Audio) { + private fun handleId3v2TextFrame(id: String, value: String, audio: MediaStoreBackend.Audio) { // It's assumed that duplicate frames are eliminated by ExoPlayer's metadata parser. when (id) { "TIT2" -> audio.title = value // Title diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt index 9940d97f9..542d49e73 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt @@ -66,6 +66,7 @@ object Indexer { if (songs.isEmpty()) return null val buildStart = System.currentTimeMillis() + val albums = buildAlbums(songs) val artists = buildArtists(albums) val genres = buildGenres(songs) @@ -206,7 +207,7 @@ object Indexer { /** Represents a backend that metadata can be extracted from. */ interface Backend { - /** Query the media database for an initial cursor. */ + /** Query the media database for a basic cursor. */ fun query(context: Context): Cursor /** Create a list of songs from the [Cursor] queried in [query]. */ diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt index be540d2c2..f8fc87e21 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/IndexerUtil.kt @@ -18,8 +18,11 @@ package org.oxycblt.auxio.music.indexer import android.content.ContentResolver +import android.content.ContentUris import android.database.Cursor import android.net.Uri +import android.provider.MediaStore +import androidx.core.text.isDigitsOnly /** Shortcut for making a [ContentResolver] query with less superfluous arguments. */ fun ContentResolver.queryCursor( @@ -38,6 +41,12 @@ fun ContentResolver.useQuery( block: (Cursor) -> R ): R? = queryCursor(uri, projection, selector, args)?.use(block) +/** Converts a [Long] Audio ID into a URI to that particular audio file. */ +val Long.audioUri: Uri + get() = + ContentUris.withAppendedId( + MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, requireNotNull(this)) + /** * Parse out the number field from an NN/TT string that is typically found in DISC_NUMBER and * CD_TRACK_NUMBER. @@ -45,6 +54,265 @@ fun ContentResolver.useQuery( val String.no: Int? get() = split('/', limit = 2).getOrNull(0)?.toIntOrNull() -/** Parse out the year field from a (presumably) ISO-8601-like date. */ +/** + * Parse out the year field from a (presumably) ISO-8601-like date. This differs across tag formats + * and has no real consistent, but it's assumed that most will format granular dates as YYYY-MM-DD + * (...) and thus we can parse the year out by splitting at the first -. + */ val String.iso8601year: Int? - get() = split(":", limit = 2).getOrNull(0)?.toIntOrNull() + get() = split('-', limit = 2).getOrNull(0)?.toIntOrNull() + +/** + * Slice a string so that any preceding articles like The/A(n) are truncated. This is hilariously + * anglo-centric, but it's also a bit of an expected feature in music players, so we implement it + * anyway. + */ +val String.withoutArticle: String + get() { + if (length > 5 && startsWith("the ", ignoreCase = true)) { + return slice(4..lastIndex) + } + + if (length > 4 && startsWith("an ", ignoreCase = true)) { + return slice(3..lastIndex) + } + + if (length > 3 && startsWith("a ", ignoreCase = true)) { + return slice(2..lastIndex) + } + + return this + } + +/** + * Decodes the genre name from an ID3(v2) constant. See [genreConstantTable] for the genre constant + * map that Auxio uses. + */ +val String.id3v2GenreName: String + get() { + if (isDigitsOnly()) { + // ID3v1, just parse as an integer + return genreConstantTable.getOrNull(toInt()) ?: this + } + + if (startsWith('(') && endsWith(')')) { + // ID3v2.3/ID3v2.4, parse out the parentheses and get the integer + // Any genres formatted as "(CHARS)" will be ignored. + + // TODO: Technically, the spec for genres is far more complex here. Perhaps we + // should copy mutagen's implementation? + // https://github.com/quodlibet/mutagen/blob/master/mutagen/id3/_frames.py + + val genreInt = substring(1 until lastIndex).toIntOrNull() + if (genreInt != null) { + return genreConstantTable.getOrNull(genreInt) ?: this + } + } + + // Current name is fine. + return this + } + +/** + * A complete table of all the constant genre values for ID3(v2), including non-standard extensions. + */ +private val genreConstantTable = + arrayOf( + // ID3 Standard + "Blues", + "Classic Rock", + "Country", + "Dance", + "Disco", + "Funk", + "Grunge", + "Hip-Hop", + "Jazz", + "Metal", + "New Age", + "Oldies", + "Other", + "Pop", + "R&B", + "Rap", + "Reggae", + "Rock", + "Techno", + "Industrial", + "Alternative", + "Ska", + "Death Metal", + "Pranks", + "Soundtrack", + "Euro-Techno", + "Ambient", + "Trip-Hop", + "Vocal", + "Jazz+Funk", + "Fusion", + "Trance", + "Classical", + "Instrumental", + "Acid", + "House", + "Game", + "Sound Clip", + "Gospel", + "Noise", + "AlternRock", + "Bass", + "Soul", + "Punk", + "Space", + "Meditative", + "Instrumental Pop", + "Instrumental Rock", + "Ethnic", + "Gothic", + "Darkwave", + "Techno-Industrial", + "Electronic", + "Pop-Folk", + "Eurodance", + "Dream", + "Southern Rock", + "Comedy", + "Cult", + "Gangsta", + "Top 40", + "Christian Rap", + "Pop/Funk", + "Jungle", + "Native American", + "Cabaret", + "New Wave", + "Psychadelic", + "Rave", + "Showtunes", + "Trailer", + "Lo-Fi", + "Tribal", + "Acid Punk", + "Acid Jazz", + "Polka", + "Retro", + "Musical", + "Rock & Roll", + "Hard Rock", + + // Winamp extensions, more or less a de-facto standard + "Folk", + "Folk-Rock", + "National Folk", + "Swing", + "Fast Fusion", + "Bebob", + "Latin", + "Revival", + "Celtic", + "Bluegrass", + "Avantgarde", + "Gothic Rock", + "Progressive Rock", + "Psychedelic Rock", + "Symphonic Rock", + "Slow Rock", + "Big Band", + "Chorus", + "Easy Listening", + "Acoustic", + "Humour", + "Speech", + "Chanson", + "Opera", + "Chamber Music", + "Sonata", + "Symphony", + "Booty Bass", + "Primus", + "Porn Groove", + "Satire", + "Slow Jam", + "Club", + "Tango", + "Samba", + "Folklore", + "Ballad", + "Power Ballad", + "Rhythmic Soul", + "Freestyle", + "Duet", + "Punk Rock", + "Drum Solo", + "A capella", + "Euro-House", + "Dance Hall", + "Goa", + "Drum & Bass", + "Club-House", + "Hardcore", + "Terror", + "Indie", + "Britpop", + "Negerpunk", + "Polsk Punk", + "Beat", + "Christian Gangsta", + "Heavy Metal", + "Black Metal", + "Crossover", + "Contemporary Christian", + "Christian Rock", + "Merengue", + "Salsa", + "Thrash Metal", + "Anime", + "JPop", + "Synthpop", + + // Winamp 5.6+ extensions, also used by EasyTAG. + // I only include this because post-rock is a based genre and deserves a slot. + "Abstract", + "Art Rock", + "Baroque", + "Bhangra", + "Big Beat", + "Breakbeat", + "Chillout", + "Downtempo", + "Dub", + "EBM", + "Eclectic", + "Electro", + "Electroclash", + "Emo", + "Experimental", + "Garage", + "Global", + "IDM", + "Illbient", + "Industro-Goth", + "Jam Band", + "Krautrock", + "Leftfield", + "Lounge", + "Math Rock", + "New Romantic", + "Nu-Breakz", + "Post-Punk", + "Post-Rock", + "Psytrance", + "Shoegaze", + "Space Rock", + "Trop Rock", + "World Music", + "Neoclassical", + "Audiobook", + "Audio Theatre", + "Neue Deutsche Welle", + "Podcast", + "Indie Rock", + "G-Funk", + "Dubstep", + "Garage Rock", + "Psybient") diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt index b5bb9307b..9470352f7 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/MediaStoreBackend.kt @@ -73,14 +73,12 @@ import org.oxycblt.auxio.util.contentResolverSafe * Is there anything we can do about it? No. Google has routinely shut down issues that begged * google to fix glaring issues with MediaStore or to just take the API behind the woodshed and * shoot it. Largely because they have zero incentive to improve it given how "obscure" local music - * listening is. As a result, some players like Vanilla and VLC just hack their own - * pseudo-MediaStore implementation from their own (better) parsers, but this is both infeasible for - * Auxio due to how incredibly slow it is to get a file handle from the android sandbox AND how much - * harder it is to manage a database of your own media that mirrors the filesystem perfectly. And - * even if I set aside those crippling issues and changed my indexer to that, it would face the even - * larger problem of how google keeps trying to kill the filesystem and force you into their - * ContentResolver API. In the future MediaStore could be the only system we have, which is also the - * day that greenland melts and birthdays stop happening forever. + * listening is. As a result, Auxio exposes an option to use an internal parser based on ExoPlayer + * that at least tries to correct the insane metadata that this API returns, but not only is that + * system horrifically slow and bug-prone, it also faces the even larger issue of how google keeps + * trying to kill the filesystem and force you into their ContentResolver API. In the future + * MediaStore could be the only system we have, which is also the day that greenland melts and + * birthdays stop happening forever. * * I'm pretty sure nothing is going to happen and MediaStore will continue to be neglected and * probably deprecated eventually for a "new" API that just coincidentally excludes music indexing. @@ -181,7 +179,7 @@ abstract class MediaStoreBackend : Indexer.Backend { } /** - * The projection to use when querying media. Add version-specific columns here in our + * The projection to use when querying media. Add version-specific columns here in an * implementation. */ open val projection: Array @@ -230,8 +228,10 @@ abstract class MediaStoreBackend : Indexer.Backend { audio.album = cursor.getStringOrNull(albumIndex) audio.albumId = cursor.getLong(albumIdIndex) - // If the artist field is , make it null. This makes handling the - // insanity of the artist field easier later on. + // Android does not make a non-existent artist tag null, it instead fills it in + // as , which makes absolutely no sense given how other fields default + // to null if they are not present. If this field is , null it so that + // it's easier to handle later. audio.artist = cursor.getStringOrNull(artistIndex)?.run { if (this != MediaStore.UNKNOWN_STRING) { @@ -267,21 +267,20 @@ abstract class MediaStoreBackend : Indexer.Backend { ) { fun toSong(): Song = Song( + // Assert that the fields that should exist are present. I can't confirm that + // every device provides these fields, but it seems likely that they do. rawName = requireNotNull(title) { "Malformed audio: No title" }, fileName = requireNotNull(displayName) { "Malformed audio: No file name" }, - uri = - ContentUris.withAppendedId( - MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, - requireNotNull(id) { "Malformed audio: No song id" }), + uri = requireNotNull(id) { "Malformed audio: No id" }.audioUri, durationMs = requireNotNull(duration) { "Malformed audio: No duration" }, track = track, disc = disc, _year = year, - _albumName = requireNotNull(album) { "Malformed song: No album name" }, + _albumName = requireNotNull(album) { "Malformed audio: No album name" }, _albumCoverUri = ContentUris.withAppendedId( EXTERNAL_ALBUM_ART_URI, - requireNotNull(albumId) { "Malformed song: No album id" }), + requireNotNull(albumId) { "Malformed audio: No album id" }), _artistName = artist, _albumArtistName = albumArtist, _genreName = genre) diff --git a/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt b/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt index 560621799..d3e1554c7 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt @@ -40,6 +40,8 @@ import org.oxycblt.auxio.util.textSafe * Instead, we wrap it in a safe class that hopefully implements enough safety to not crash the app * or result in blatantly janky behavior. Mostly. * + * TODO: Add smooth seeking + * * @author OxygenCobalt */ class StyledSeekBar diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt index fd8e02ccb..c5dbbcee4 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt @@ -71,8 +71,7 @@ class PlaybackStateManager private constructor() { notifyPlayingChanged() } /** The current playback progress */ - var positionMs = 0L - private set + private var positionMs = 0L /** The current [RepeatMode] */ var repeatMode = RepeatMode.NONE set(value) { @@ -92,8 +91,7 @@ class PlaybackStateManager private constructor() { private val callbacks = mutableListOf() /** - * Add a [PlaybackStateManager.Callback] to this instance. Make sure to remove the callback with - * [removeCallback] when done. + * Add a callback to this instance. Make sure to remove it when done. */ fun addCallback(callback: Callback) { if (isInitialized) { diff --git a/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt b/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt index 6e1158e1a..c8e0ea91a 100644 --- a/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt @@ -44,9 +44,6 @@ fun unlikelyToBeNull(value: T?): T { /** Shortcut to clamp an integer between [min] and [max] */ fun Int.clamp(min: Int, max: Int): Int = MathUtils.clamp(this, min, max) -/** Shortcut to clamp an integer between [min] and [max] */ -fun Long.clamp(min: Long, max: Long): Long = MathUtils.clamp(this, min, max) - /** * Convert a [Long] of seconds into a string duration. * @param isElapsed Whether this duration is represents elapsed time. If this is false, then --:--