From 3ab425839c3e75900336053199ac2e8ccc09052e Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sun, 26 Sep 2021 15:33:31 -0600 Subject: [PATCH] music: refactor music loading Refactor music loading to be based off of songs entirely. This reduces efficency but enables some nices fixes, notably: 1. Album artists now have basic support [You won't be able to see specific artists, but they won't be fragmented anymore] 2. Samsung devices probably shouldn't get confused about artist names anymore, like in #40 This should hopefully be the last time I need to refactor this horrible system. Thank god. --- .../org/oxycblt/auxio/coil/AlbumArtFetcher.kt | 8 +- .../org/oxycblt/auxio/coil/MosaicFetcher.kt | 7 +- .../org/oxycblt/auxio/home/HomeFragment.kt | 12 +- .../org/oxycblt/auxio/home/HomeViewModel.kt | 26 +- .../java/org/oxycblt/auxio/music/Models.kt | 49 ++-- .../org/oxycblt/auxio/music/MusicLoader.kt | 262 +++++++++--------- .../oxycblt/auxio/playback/state/LoopMode.kt | 12 +- .../auxio/playback/state/PlaybackMode.kt | 15 +- .../java/org/oxycblt/auxio/ui/DiffCallback.kt | 3 + .../java/org/oxycblt/auxio/ui/DisplayMode.kt | 10 + 10 files changed, 202 insertions(+), 202 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/coil/AlbumArtFetcher.kt b/app/src/main/java/org/oxycblt/auxio/coil/AlbumArtFetcher.kt index 6a6d20682..f2274edaa 100644 --- a/app/src/main/java/org/oxycblt/auxio/coil/AlbumArtFetcher.kt +++ b/app/src/main/java/org/oxycblt/auxio/coil/AlbumArtFetcher.kt @@ -30,6 +30,7 @@ import coil.size.Size 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 java.io.ByteArrayInputStream @@ -56,14 +57,15 @@ class AlbumArtFetcher(private val context: Context) : Fetcher { } private fun loadMediaStoreCovers(data: Album): SourceResult { - val stream = context.contentResolver.openInputStream(data.coverUri) + val uri = data.id.toAlbumArtURI() + val stream = context.contentResolver.openInputStream(uri) if (stream != null) { // Don't close the stream here as it will cause an error later from an attempted read. // This stream still seems to close itself at some point, so its fine. return SourceResult( source = stream.source().buffer(), - mimeType = context.contentResolver.getType(data.coverUri), + mimeType = context.contentResolver.getType(uri), dataSource = DataSource.DISK ) } @@ -97,5 +99,5 @@ class AlbumArtFetcher(private val context: Context) : Fetcher { return loadMediaStoreCovers(data) } - override fun key(data: Album) = data.coverUri.toString() + override fun key(data: Album) = data.id.toString() } diff --git a/app/src/main/java/org/oxycblt/auxio/coil/MosaicFetcher.kt b/app/src/main/java/org/oxycblt/auxio/coil/MosaicFetcher.kt index 6fa008e26..f5af35442 100644 --- a/app/src/main/java/org/oxycblt/auxio/coil/MosaicFetcher.kt +++ b/app/src/main/java/org/oxycblt/auxio/coil/MosaicFetcher.kt @@ -38,6 +38,7 @@ import org.oxycblt.auxio.music.Album import org.oxycblt.auxio.music.Artist import org.oxycblt.auxio.music.Genre import org.oxycblt.auxio.music.Parent +import org.oxycblt.auxio.music.toAlbumArtURI import java.io.Closeable import java.io.InputStream @@ -57,11 +58,11 @@ class MosaicFetcher(private val context: Context) : Fetcher { when (data) { is Artist -> data.albums.forEachIndexed { index, album -> - if (index < 4) { uris.add(album.coverUri) } + if (index < 4) { uris.add(album.id.toAlbumArtURI()) } } - is Genre -> data.songs.groupBy { it.album.coverUri }.keys.forEachIndexed { index, uri -> - if (index < 4) { uris.add(uri) } + is Genre -> data.songs.groupBy { it.album.id }.keys.forEachIndexed { index, id -> + if (index < 4) { uris.add(id.toAlbumArtURI()) } } else -> {} diff --git a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt index dd319e5ea..b64b5cbce 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt @@ -163,13 +163,13 @@ class HomeFragment : Fragment() { homeModel.curTab.observe(viewLifecycleOwner) { tab -> binding.homeAppbar.liftOnScrollTargetViewId = when (requireNotNull(tab)) { DisplayMode.SHOW_SONGS -> { - updateSortMenu(sortItem, homeModel.songSortMode) + updateSortMenu(sortItem, tab) R.id.home_song_list } DisplayMode.SHOW_ALBUMS -> { - updateSortMenu(sortItem, homeModel.albumSortMode) { id -> + updateSortMenu(sortItem, tab) { id -> id != R.id.option_sort_album } @@ -177,7 +177,7 @@ class HomeFragment : Fragment() { } DisplayMode.SHOW_ARTISTS -> { - updateSortMenu(sortItem, homeModel.artistSortMode) { id -> + updateSortMenu(sortItem, tab) { id -> id == R.id.option_sort_asc || id == R.id.option_sort_dsc } @@ -185,7 +185,7 @@ class HomeFragment : Fragment() { } DisplayMode.SHOW_GENRES -> { - updateSortMenu(sortItem, homeModel.genreSortMode) { id -> + updateSortMenu(sortItem, tab) { id -> id == R.id.option_sort_asc || id == R.id.option_sort_dsc } @@ -228,9 +228,11 @@ class HomeFragment : Fragment() { private fun updateSortMenu( item: MenuItem, - toHighlight: SortMode, + displayMode: DisplayMode, isVisible: (Int) -> Boolean = { true } ) { + val toHighlight = homeModel.getSortForDisplay(displayMode) + for (option in item.subMenu) { if (option.itemId == toHighlight.itemId) { option.isChecked = true diff --git a/app/src/main/java/org/oxycblt/auxio/home/HomeViewModel.kt b/app/src/main/java/org/oxycblt/auxio/home/HomeViewModel.kt index a2a3ab54a..11f4df009 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/HomeViewModel.kt @@ -26,6 +26,7 @@ import org.oxycblt.auxio.music.Artist import org.oxycblt.auxio.music.Genre import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.Song +import org.oxycblt.auxio.settings.SettingsManager import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.SortMode @@ -56,19 +57,13 @@ class HomeViewModel : ViewModel() { private val mCurTab = MutableLiveData(mTabs.value!![0]) val curTab: LiveData = mCurTab - var genreSortMode = SortMode.ASCENDING - private set - - var artistSortMode = SortMode.ASCENDING - private set - - var albumSortMode = SortMode.ASCENDING - private set - - var songSortMode = SortMode.ASCENDING - private set + private var genreSortMode = SortMode.ASCENDING + private var artistSortMode = SortMode.ASCENDING + private var albumSortMode = SortMode.ASCENDING + private var songSortMode = SortMode.ASCENDING private val musicStore = MusicStore.getInstance() + private val settingsManager = SettingsManager.getInstance() init { mSongs.value = songSortMode.sortSongs(musicStore.songs) @@ -86,6 +81,15 @@ class HomeViewModel : ViewModel() { mCurTab.value = mode } + fun getSortForDisplay(displayMode: DisplayMode): SortMode { + return when (displayMode) { + DisplayMode.SHOW_GENRES -> genreSortMode + DisplayMode.SHOW_ARTISTS -> artistSortMode + DisplayMode.SHOW_ALBUMS -> albumSortMode + DisplayMode.SHOW_SONGS -> songSortMode + } + } + /** * Update the currently displayed item's [SortMode]. */ 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 225f28938..9cf8f5531 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Models.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Models.kt @@ -18,7 +18,6 @@ package org.oxycblt.auxio.music -import android.net.Uri import android.view.View import androidx.annotation.DrawableRes import androidx.annotation.StringRes @@ -87,7 +86,10 @@ data class Song( override val id: Long, override val name: String, val fileName: String, + val albumName: String, val albumId: Long, + val artistName: String, + val year: Int, val track: Int, val duration: Long ) : BaseModel(), Hashable { @@ -97,8 +99,8 @@ data class Song( val genre: Genre? get() = mGenre val album: Album get() = requireNotNull(mAlbum) - val seconds = duration / 1000 - val formattedDuration = seconds.toDuration() + val seconds: Long get() = duration / 1000 + val formattedDuration: String get() = (duration / 1000).toDuration() override val hash: Int get() { var result = name.hashCode() @@ -108,22 +110,17 @@ data class Song( } fun linkAlbum(album: Album) { - if (mAlbum == null) { - mAlbum = album - } + mAlbum = album } fun linkGenre(genre: Genre) { - if (mGenre == null) { - mGenre = genre - } + mGenre = genre } } /** * The data object for an album. Inherits [Parent]. * @property artistName The name of the parent artist. Do not use this outside of creating the artist from albums - * @property coverUri The [Uri] for the album's cover. **Load this using Coil.** * @property year The year this album was released. 0 if there is none in the metadata. * @property artist The Album's parent [Artist]. use this instead of [artistName] * @property songs The Album's child [Song]s. @@ -133,15 +130,18 @@ data class Album( override val id: Long, override val name: String, val artistName: String, - val coverUri: Uri, - val year: Int + val year: Int, + val songs: List ) : Parent() { + init { + songs.forEach { song -> + song.linkAlbum(this) + } + } + private var mArtist: Artist? = null val artist: Artist get() = requireNotNull(mArtist) - private val mSongs = mutableListOf() - val songs: List get() = mSongs - val totalDuration: String get() = songs.sumOf { it.seconds }.toDuration() @@ -155,13 +155,6 @@ data class Album( fun linkArtist(artist: Artist) { mArtist = artist } - - fun linkSongs(songs: List) { - for (song in songs) { - song.linkAlbum(this) - mSongs.add(song) - } - } } /** @@ -175,6 +168,12 @@ data class Artist( override val name: String, val albums: List ) : Parent() { + init { + albums.forEach { album -> + album.linkArtist(this) + } + } + val genre: Genre? by lazy { // Get the genre that corresponds to the most songs in this artist, which would be // the most "Prominent" genre. @@ -186,12 +185,6 @@ data class Artist( } override val hash = name.hashCode() - - init { - albums.forEach { album -> - album.linkArtist(this) - } - } } /** 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 30881a46c..905f314fc 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicLoader.kt @@ -20,18 +20,74 @@ package org.oxycblt.auxio.music import android.annotation.SuppressLint import android.content.Context -import android.net.Uri -import android.provider.MediaStore -import android.provider.MediaStore.Audio.Albums import android.provider.MediaStore.Audio.Genres import android.provider.MediaStore.Audio.Media import androidx.core.database.getStringOrNull import org.oxycblt.auxio.R import org.oxycblt.auxio.excluded.ExcludedDatabase +import org.oxycblt.auxio.ui.SortMode import org.oxycblt.auxio.util.logD /** - * Class that loads/constructs [Genre]s, [Artist]s, [Album]s, and [Song] objects from the filesystem + * This class does pretty much all the black magic required to get a remotely sensible music + * indexing system while still optimizing for time. I would recommend you leave this file now + * before you lose your sanity trying to understand the hoops I had to jump through for this + * system, but if you really want to stay, here's a debrief on why this code is so awful. + * + * MediaStore is not a good API. It is not even a bad API. Calling it a bad API is an insult to + * other bad android APIs, like CoordinatorLayout or InputMethodManager. No. MediaStore is a + * crime against humanity and probably a way to summon Zalgo if you tried hard enough. + * + * You think that if you wanted to query a song's genre from a media database, you could just + * put "genre" in the query and it would return it, right? But not with MediaStore! No, that's + * to straightfoward for this platform. So instead, you have to query for each genre, query all + * the songs in each genre, and then iterate through those songs to link every song with their + * genre. This is not documented anywhere in MediaStore's documentation, and the O(mom im scared) + * algorithm you have to run to get it working single-handedly DOUBLES Auxio's loading times. At + * no point have the devs considered that this column is absolutely busted, and instead focused on + * adding infuriat- I mean nice proprietary extensions to MediaStore for their own Google Play Music, + * and we all know how great that worked out! + * + * It's not even ergonomics that makes this API bad. It's base implementation is completely borked + * as well. Did you know that MediaStore doesn't accept dates that aren't from ID3v2.3 MP3 files? + * I sure didn't, until I decided to upgrade my music collection to ID3v2.4 and Xiph only to see + * that their metadata parser has a brain aneurysm the moment it stumbles upon a dreaded TRDC or + * DATE tag. Once again, this is because internally android uses an ancient in-house metadata + * parser to get everything indexed, and so far they have not bothered to modernize this parser + * or even switch it to something more powerful like Taglib, not even in Android 12. ID3v2.4 is + * 21 years old. It can drink now. All my what. + * + * Not to mention all the other infuriating quirks. Album artists can't be accessed from the albums + * table, so we have to go for the less efficent "make a big query on all the songs lol" method + * so that songs don't end up fragmented across artists. Pretty much every OEM has added some + * extension or quirk to MediaStore that I cannot determine, with some OEMs (COUGHSAMSUNGCOUGH) + * crippling the normal tables so that you're railroaded into their ad-infested music app. + * The way I do blacklisting relies on a deprecated method, and the supposedly "modern" method + * is SLOWER and causes even more problems since I have to manage databases across version + * boundaries. Sometimes music will have a deformed clone that I can't filter out, sometimes + * Genre's will just break for no reason, sometimes this plate of spaghetti just completely breaks + * down and is unable to get any metadata. Everything is broken in it's own special unique way and + * I absolutely hate it. + * + * Is there anything we can do about it? No. Google has routinely shut down issues that beg 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, especially for such obscure things + * as indexing music. As a result, some players like Vanilla and VLC just hack their own pidgin + * version of MediaStore from their own 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 me 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. + * + * 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. + * Because go **** yourself for wanting to listen to music you own. Be a good consoomer and listen + * to your AlgoMix MusikStream™. + * + * I hate this platform so much. + * * @author OxygenCobalt */ class MusicLoader(private val context: Context) { @@ -53,12 +109,11 @@ class MusicLoader(private val context: Context) { buildSelector() loadGenres() - loadAlbums() loadSongs() - - linkAlbums() - buildArtists() linkGenres() + + buildAlbums() + buildArtists() } @Suppress("DEPRECATION") @@ -95,7 +150,8 @@ class MusicLoader(private val context: Context) { while (cursor.moveToNext()) { val id = cursor.getLong(idIndex) - val name = cursor.getStringOrNull(nameIndex) ?: continue // No non-broken genre would be missing a name + // No non-broken genre would be missing a name. + val name = cursor.getStringOrNull(nameIndex) ?: continue genres.add(Genre(id, name)) } @@ -104,53 +160,6 @@ class MusicLoader(private val context: Context) { logD("Genre search finished with ${genres.size} genres found.") } - private fun loadAlbums() { - logD("Starting album search...") - - val albumCursor = resolver.query( - Albums.EXTERNAL_CONTENT_URI, - arrayOf( - Albums._ID, // 0 - Albums.ALBUM, // 1 - Albums.ARTIST, // 2 - Albums.LAST_YEAR, // 4 - ), - null, null, - Albums.DEFAULT_SORT_ORDER - ) - - val albumPlaceholder = context.getString(R.string.def_album) - val artistPlaceholder = context.getString(R.string.def_artist) - - albumCursor?.use { cursor -> - val idIndex = cursor.getColumnIndexOrThrow(Albums._ID) - val nameIndex = cursor.getColumnIndexOrThrow(Albums.ALBUM) - val artistNameIndex = cursor.getColumnIndexOrThrow(Albums.ARTIST) - val yearIndex = cursor.getColumnIndexOrThrow(Albums.LAST_YEAR) - - while (cursor.moveToNext()) { - val id = cursor.getLong(idIndex) - val name = cursor.getString(nameIndex) ?: albumPlaceholder - var artistName = cursor.getString(artistNameIndex) ?: artistPlaceholder - val year = cursor.getInt(yearIndex) - val coverUri = id.toAlbumArtURI() - - // Correct any artist names to a nicer "Unknown Artist" label - if (artistName == MediaStore.UNKNOWN_STRING) { - artistName = artistPlaceholder - } - - albums.add(Album(id, name, artistName, coverUri, year)) - } - } - - albums = albums.distinctBy { - it.name to it.artistName to it.year - }.toMutableList() - - logD("Album search finished with ${albums.size} albums found") - } - @SuppressLint("InlinedApi") private fun loadSongs() { logD("Starting song search...") @@ -159,11 +168,15 @@ class MusicLoader(private val context: Context) { Media.EXTERNAL_CONTENT_URI, arrayOf( Media._ID, // 0 - Media.DISPLAY_NAME, // 1 - Media.TITLE, // 2 - Media.ALBUM_ID, // 3 - Media.TRACK, // 4 - Media.DURATION, // 5 + Media.TITLE, // 1 + Media.DISPLAY_NAME, // 2 + Media.ALBUM, // 3 + Media.ALBUM_ID, // 4 + Media.ARTIST, // 5 + Media.ALBUM_ARTIST, // 6 + Media.YEAR, // 7 + Media.TRACK, // 8 + Media.DURATION, // 9 ), selector, args, Media.DEFAULT_SORT_ORDER @@ -173,7 +186,11 @@ class MusicLoader(private val context: Context) { val idIndex = cursor.getColumnIndexOrThrow(Media._ID) val titleIndex = cursor.getColumnIndexOrThrow(Media.TITLE) val fileIndex = cursor.getColumnIndexOrThrow(Media.DISPLAY_NAME) - val albumIndex = cursor.getColumnIndexOrThrow(Media.ALBUM_ID) + val albumIndex = cursor.getColumnIndexOrThrow(Media.ALBUM) + val albumIdIndex = cursor.getColumnIndexOrThrow(Media.ALBUM_ID) + val artistIndex = cursor.getColumnIndexOrThrow(Media.ARTIST) + val albumArtistIndex = cursor.getColumnIndexOrThrow(Media.ALBUM_ARTIST) + val yearIndex = cursor.getColumnIndexOrThrow(Media.YEAR) val trackIndex = cursor.getColumnIndexOrThrow(Media.TRACK) val durationIndex = cursor.getColumnIndexOrThrow(Media.DURATION) @@ -181,111 +198,89 @@ class MusicLoader(private val context: Context) { val id = cursor.getLong(idIndex) val fileName = cursor.getString(fileIndex) val title = cursor.getString(titleIndex) ?: fileName - val albumId = cursor.getLong(albumIndex) + val album = cursor.getString(albumIndex) + val albumId = cursor.getLong(albumIdIndex) + + // MediaStore does not have support for artists in the album field, so we have to + // detect it on a song-by-song basis. This is another massive bottleneck in the music + // loader since we have to do a massive query to get what we want, but theres not + // a lot I can do that doesn't degrade UX. + val artist = cursor.getStringOrNull(albumArtistIndex) + ?: cursor.getString(artistIndex) + + val year = cursor.getInt(yearIndex) val track = cursor.getInt(trackIndex) val duration = cursor.getLong(durationIndex) - songs.add(Song(id, title, fileName, albumId, track, duration)) + songs.add( + Song( + id, title, fileName, album, albumId, artist, year, track, duration + ) + ) } } songs = songs.distinctBy { - it.name to it.albumId to it.track to it.duration + it.name to it.albumId to it.artistName to it.track to it.duration }.toMutableList() logD("Song search finished with ${songs.size} found") } - private fun linkAlbums() { + private fun buildAlbums() { logD("Linking albums") // Group up songs by their album ids and then link them with their albums val songsByAlbum = songs.groupBy { it.albumId } - val unknownAlbum = Album( - id = -1, - name = context.getString(R.string.def_album), - artistName = context.getString(R.string.def_artist), - coverUri = Uri.EMPTY, - year = 0 - ) songsByAlbum.forEach { entry -> - (albums.find { it.id == entry.key } ?: unknownAlbum).linkSongs(entry.value) + // Rely on the first song in this list for album information. + // Note: This might result in a bad year being used for an album if an album's songs + // have multiple years. This is fixable but is currently omitted for speed. + val song = entry.value[0] + + albums.add( + Album( + id = entry.key, + name = song.albumName, + artistName = song.artistName, + songs = entry.value, + year = song.year + ) + ) } albums.removeAll { it.songs.isEmpty() } + albums = SortMode.ASCENDING.sortAlbums(albums).toMutableList() - // If something goes horribly wrong and somehow songs are still not linked up by the - // album id, just throw them into an unknown album. - if (unknownAlbum.songs.isNotEmpty()) { - albums.add(unknownAlbum) - } + logD("Songs successfully linked into ${albums.size} albums") } private fun buildArtists() { logD("Linking artists") - // Group albums up by their artist name, should not result in any null-artist issues val albumsByArtist = albums.groupBy { it.artistName } albumsByArtist.forEach { entry -> + // Because of our hacky album artist system, MediaStore artist IDs are unreliable. + // Therefore we just use the hashCode of the artist name as our ID and move on. artists.add( - // IDs are incremented from the minimum int value so that they remain unique. Artist( - id = (Int.MIN_VALUE + artists.size).toLong(), + id = entry.key.hashCode().toLong(), name = entry.key, albums = entry.value ) ) } + artists = SortMode.ASCENDING.sortModels(artists).toMutableList() + logD("Albums successfully linked into ${artists.size} artists") } private fun linkGenres() { logD("Linking genres") - /* - * Okay, I'm going to go on a bit of a tangent here because this bit of code infuriates me. - * - * In any reasonable platform, you think that you could just query a media database for the - * "genre" field and get the genre for a song, right? But not with android! No. Thats too - * normal for this dysfunctional SDK that was dropped on it's head as a baby. Instead, we - * have to iterate through EACH GENRE, QUERY THE MEDIA DATABASE FOR THE SONGS OF THAT GENRE, - * AND THEN ITERATE THROUGH THAT LIST TO LINK EVERY SONG WITH THE GENRE. This O(mom im scared) - * algorithm single-handedly DOUBLES the amount of time it takes Auxio to load music, but - * apparently this is the only way you can have a remotely sensible genre system on this - * busted OS. Why is it this way? Nobody knows! Now this quirk is immortalized and has to - * be replicated in all future versions of this API, because god forbid you break some - * app that's probably older than some of the people reading this by now. - * - * Another fun fact, did you know that you can only get the date from ID3v2.3 MPEG files? - * I sure didn't, until I decided to update my music collection to ID3v2.4 and Xiph only - * to see that android apparently has a brain aneurysm the moment it sees a dreaded TDRC - * or DATE tag. This bug is similarly baked into the platform and hasnt been fixed, even - * in android TWELVE. ID3v2.4 has existed for TWENTY-ONE YEARS. IT CAN DRINK NOW. At least - * you could replace your ancient dinosaur parser with Taglib or something, but again, - * google cant bear even slighting the gods of backwards compat. - * - * Is there anything we can do about this system? No. Google's issue tracker, in classic - * google fashion, requires a google account to even view. Even if I were to set aside my - * Torvalds-esqe convictions and make an account, MediaStore is such an obscure part of the - * platform that it basically receives no attention compared to the advertising APIs or the - * nineteenth UI rework, and its not like the many music player developers are going to band - * together to beg google to take this API behind the woodshed and shoot it. So instead, - * players like VLC and Vanilla just hack their own pidgin version of MediaStore off of - * their own media parsers, but even this becomes increasingly impossible as google - * continues to kill the filesystem ala iOS. In the future MediaStore could be the only - * system we have, which is also the day greenland melts and birthdays stop happening - * forever. I'm pretty sure that at this point nothing is going to happen, google will - * continue to neglect MediaStore, and all the people who just want to listen to their music - * collections on their phone will continue to get screwed. But hey, at least some dev at - * google got a cushy managerial position where they can tweet about politics all day for - * shipping the brand new androidx.FooBarBlasterView, yay! - * - * I hate this platform so much. - */ - genres.forEach { genre -> val songCursor = resolver.query( Genres.Members.getContentUri("external", genre.id), @@ -306,22 +301,21 @@ class MusicLoader(private val context: Context) { } } - // Any songs without genres will be thrown into an unknown genre - val songsWithoutGenres = songs.filter { it.genre == null } + // Songs that don't have a genre will be thrown into an unknown genre. - if (songsWithoutGenres.isNotEmpty()) { - val unknownGenre = Genre( - id = -2, - name = context.getString(R.string.def_genre) - ) + val unknownGenre = Genre( + id = -2, + name = context.getString(R.string.def_genre) + ) - songsWithoutGenres.forEach { song -> + songs.forEach { song -> + if (song.genre == null) { unknownGenre.linkSong(song) } - - genres.add(unknownGenre) } - genres.removeAll { it.songs.isEmpty() } + if (unknownGenre.songs.isEmpty()) { + genres.add(unknownGenre) + } } } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/LoopMode.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/LoopMode.kt index 317c10485..25a626028 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/LoopMode.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/LoopMode.kt @@ -41,17 +41,13 @@ enum class LoopMode { * @return The int constant for this mode */ fun toInt(): Int { - return when (this) { - NONE -> CONST_NONE - ALL -> CONST_ALL - TRACK -> CONST_TRACK - } + return CONST_NONE + ordinal } companion object { - const val CONST_NONE = 0xA100 - const val CONST_ALL = 0xA101 - const val CONST_TRACK = 0xA102 + private const val CONST_NONE = 0xA100 + private const val CONST_ALL = 0xA101 + private const val CONST_TRACK = 0xA102 /** * Convert an int [constant] into a LoopMode, or null if it isnt valid. diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackMode.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackMode.kt index 4068dfb73..46aadbb00 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackMode.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackMode.kt @@ -37,19 +37,14 @@ enum class PlaybackMode { * @return The constant for this mode, */ fun toInt(): Int { - return when (this) { - IN_ARTIST -> CONST_IN_ARTIST - IN_GENRE -> CONST_IN_GENRE - IN_ALBUM -> CONST_IN_ALBUM - ALL_SONGS -> CONST_ALL_SONGS - } + return CONST_IN_ARTIST + ordinal } companion object { - const val CONST_IN_GENRE = 0xA103 - const val CONST_IN_ARTIST = 0xA104 - const val CONST_IN_ALBUM = 0xA105 - const val CONST_ALL_SONGS = 0xA106 + private const val CONST_IN_GENRE = 0xA103 + private const val CONST_IN_ARTIST = 0xA104 + private const val CONST_IN_ALBUM = 0xA105 + private const val CONST_ALL_SONGS = 0xA106 /** * Get a [PlaybackMode] for an int [constant] diff --git a/app/src/main/java/org/oxycblt/auxio/ui/DiffCallback.kt b/app/src/main/java/org/oxycblt/auxio/ui/DiffCallback.kt index a1aa55ddb..768a3a63d 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/DiffCallback.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/DiffCallback.kt @@ -32,6 +32,9 @@ class DiffCallback : DiffUtil.ItemCallback() { } override fun areItemsTheSame(oldItem: T, newItem: T): Boolean { + // Prevent ID collisions from occurring between datatypes. + if (oldItem.javaClass != newItem.javaClass) return false + return oldItem.id == newItem.id } } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/DisplayMode.kt b/app/src/main/java/org/oxycblt/auxio/ui/DisplayMode.kt index 01710cd60..b10a0f039 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/DisplayMode.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/DisplayMode.kt @@ -35,6 +35,11 @@ enum class DisplayMode { private const val CONST_SHOW_ALBUMS = 0xA10A private const val CONST_SHOW_SONGS = 0xA10B + /** + * Convert this enum into an integer for filtering. + * In this context, a null value means to filter nothing. + * @return An integer constant for that display mode, or a constant for a null [DisplayMode] + */ fun toFilterInt(value: DisplayMode?): Int { return when (value) { SHOW_SONGS -> CONST_SHOW_SONGS @@ -45,6 +50,11 @@ enum class DisplayMode { } } + /** + * Convert a filtering integer to a [DisplayMode]. + * In this context, a null value means to filter nothing. + * @return A [DisplayMode] for this constant (including null) + */ fun fromFilterInt(value: Int): DisplayMode? { return when (value) { CONST_SHOW_SONGS -> SHOW_SONGS