From 60b637e1cebb89cbc6a44b40a681c79f8dec0126 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Mon, 11 Jul 2022 11:29:34 -0600 Subject: [PATCH] all: cleanup Semi-major code cleanup. --- app/build.gradle | 1 - .../java/org/oxycblt/auxio/MainActivity.kt | 2 - .../oxycblt/auxio/detail/DetailViewModel.kt | 11 +- .../org/oxycblt/auxio/home/HomeFragment.kt | 12 +- .../home/fastscroll/FastScrollRecyclerView.kt | 10 +- .../org/oxycblt/auxio/image/BitmapProvider.kt | 16 ++- .../org/oxycblt/auxio/music/MusicStore.kt | 4 +- ...{IndexerViewModel.kt => MusicViewModel.kt} | 21 ++-- .../oxycblt/auxio/music/StorageFramework.kt | 7 +- .../org/oxycblt/auxio/music/system/Indexer.kt | 114 +++++++++--------- .../auxio/playback/PlaybackPanelFragment.kt | 8 +- .../auxio/playback/PlaybackViewModel.kt | 5 +- .../replaygain/ReplayGainAudioProcessor.kt | 6 +- .../playback/state/PlaybackStateManager.kt | 3 + .../playback/system/MediaSessionComponent.kt | 43 ++++--- .../auxio/playback/system/PlaybackService.kt | 5 +- .../oxycblt/auxio/settings/SettingsCompat.kt | 12 +- .../auxio/settings/SettingsListFragment.kt | 17 +-- .../auxio/ui/system/ForegroundManager.kt | 8 +- .../auxio/ui/system/ServiceNotification.kt | 4 +- .../org/oxycblt/auxio/util/PrimitiveUtil.kt | 22 ++-- .../oxycblt/auxio/widgets/WidgetComponent.kt | 6 +- app/src/main/res/drawable/ic_pause_32.xml | 11 -- app/src/main/res/drawable/ic_play_32.xml | 11 -- .../main/res/drawable/ui_accent_circle.xml | 2 +- .../layout-land/fragment_playback_panel.xml | 8 +- .../fragment_playback_panel.xml | 20 +-- .../fragment_playback_panel.xml | 32 ++--- .../fragment_playback_panel.xml | 8 +- app/src/main/res/layout/dialog_music_dirs.xml | 16 +-- app/src/main/res/layout/dialog_pre_amp.xml | 12 +- .../main/res/layout/dialog_song_detail.xml | 4 +- .../res/layout/fragment_playback_panel.xml | 8 +- app/src/main/res/layout/item_music_dir.xml | 4 +- app/src/main/res/layout/item_tab.xml | 2 +- app/src/main/res/layout/widget_large.xml | 4 +- app/src/main/res/layout/widget_medium.xml | 4 +- app/src/main/res/layout/widget_small.xml | 2 +- app/src/main/res/layout/widget_wide.xml | 2 +- app/src/main/res/values/dimens.xml | 7 +- app/src/main/res/values/styles_android.xml | 4 - app/src/main/res/values/styles_ui.xml | 94 ++++++++------- info/ARCHITECTURE.md | 3 +- 43 files changed, 288 insertions(+), 307 deletions(-) rename app/src/main/java/org/oxycblt/auxio/music/{IndexerViewModel.kt => MusicViewModel.kt} (65%) delete mode 100644 app/src/main/res/drawable/ic_pause_32.xml delete mode 100644 app/src/main/res/drawable/ic_play_32.xml diff --git a/app/build.gradle b/app/build.gradle index e3ae63e60..ae25a0181 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -82,7 +82,6 @@ dependencies { implementation "androidx.navigation:navigation-fragment-ktx:$navigation_version" // Media - // TODO: Dumpster this for Media3 implementation "androidx.media:media:1.6.0" // Preferences diff --git a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt index d52799f1d..17402d30e 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt @@ -44,8 +44,6 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat * * TODO: Add multi-select * - * TODO: Bug test runtime rescanning - * * @author OxygenCobalt */ class MainActivity : AppCompatActivity() { diff --git a/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt b/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt index 842ac6d99..b88cbdb25 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt @@ -39,7 +39,7 @@ import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.recycler.Header import org.oxycblt.auxio.ui.recycler.Item -import org.oxycblt.auxio.util.GenerationGuard +import org.oxycblt.auxio.util.TaskGuard import org.oxycblt.auxio.util.application import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logW @@ -113,7 +113,7 @@ class DetailViewModel(application: Application) : currentGenre.value?.let(::refreshGenreData) } - private val songGuard = GenerationGuard() + private val songGuard = TaskGuard() fun setSongId(id: Long) { if (_currentSong.value?.run { song.id } == id) return @@ -123,6 +123,7 @@ class DetailViewModel(application: Application) : } fun clearSong() { + songGuard.newHandle() _currentSong.value = null } @@ -161,9 +162,9 @@ class DetailViewModel(application: Application) : private fun generateDetailSong(song: Song) { _currentSong.value = DetailSong(song, null) viewModelScope.launch(Dispatchers.IO) { - val generation = songGuard.newHandle() + val handle = songGuard.newHandle() val info = generateDetailSongInfo(song) - songGuard.yield(generation) + songGuard.yield(handle) _currentSong.value = DetailSong(song, info) } } @@ -220,7 +221,7 @@ class DetailViewModel(application: Application) : // To create a good user experience regarding disc numbers, we intersperse // items that show the disc number throughout the album's songs. In the case - // that the album does not have distinct disc numbers, we omit the header. + // that the album does not have distinct disc numbers, we omit such a header val songs = albumSort.songs(album.songs) val byDisc = songs.groupBy { it.disc ?: 1 } if (byDisc.size > 1) { 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 6d877e668..31ff83809 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/HomeFragment.kt @@ -46,8 +46,8 @@ import org.oxycblt.auxio.home.list.SongListFragment import org.oxycblt.auxio.music.Album import org.oxycblt.auxio.music.Artist import org.oxycblt.auxio.music.Genre -import org.oxycblt.auxio.music.IndexerViewModel import org.oxycblt.auxio.music.Music +import org.oxycblt.auxio.music.MusicViewModel import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.system.Indexer import org.oxycblt.auxio.playback.PlaybackViewModel @@ -75,7 +75,7 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI private val playbackModel: PlaybackViewModel by androidActivityViewModels() private val navModel: NavigationViewModel by activityViewModels() private val homeModel: HomeViewModel by androidActivityViewModels() - private val indexerModel: IndexerViewModel by activityViewModels() + private val indexerModel: MusicViewModel by activityViewModels() // lifecycleObject builds this in the creation step, so doing this is okay. private val storagePermissionLauncher: ActivityResultLauncher by lifecycleObject { @@ -135,10 +135,10 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI // --- VIEWMODEL SETUP --- - collectImmediately(homeModel.songs, homeModel.isFastScrolling, ::updateFab) collect(homeModel.recreateTabs, ::handleRecreateTabs) collectImmediately(homeModel.currentTab, ::updateCurrentTab) - collectImmediately(indexerModel.state, ::handleIndexerState) + collectImmediately(indexerModel.libraryExists, homeModel.isFastScrolling, ::updateFab) + collectImmediately(indexerModel.indexerState, ::handleIndexerState) collect(navModel.exploreNavigationItem, ::handleNavigation) } @@ -328,9 +328,9 @@ class HomeFragment : ViewBindingFragment(), Toolbar.OnMenuI } } - private fun updateFab(songs: List, isFastScrolling: Boolean) { + private fun updateFab(hasLoaded: Boolean, isFastScrolling: Boolean) { val binding = requireBinding() - if (isFastScrolling || songs.isEmpty()) { + if (!hasLoaded || isFastScrolling) { binding.homeFab.hide() } else { binding.homeFab.show() diff --git a/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt b/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt index af6feecec..ce22496e1 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt @@ -36,7 +36,6 @@ import kotlin.math.abs import org.oxycblt.auxio.R import org.oxycblt.auxio.ui.recycler.EdgeRecyclerView import org.oxycblt.auxio.util.canScroll -import org.oxycblt.auxio.util.clamp import org.oxycblt.auxio.util.getDimenOffsetSafe import org.oxycblt.auxio.util.getDimenSizeSafe import org.oxycblt.auxio.util.getDrawableSafe @@ -260,9 +259,10 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr val thumbAnchorY = thumbView.paddingTop val popupTop = - (thumbTop + thumbAnchorY - popupAnchorY).clamp( - thumbPadding.top + popupLayoutParams.topMargin, - height - thumbPadding.bottom - popupLayoutParams.bottomMargin - popupHeight) + (thumbTop + thumbAnchorY - popupAnchorY) + .coerceAtLeast(thumbPadding.top + popupLayoutParams.topMargin) + .coerceAtMost( + height - thumbPadding.bottom - popupLayoutParams.bottomMargin - popupHeight) popupView.layout(popupLeft, popupTop, popupLeft + popupWidth, popupTop + popupHeight) } @@ -358,7 +358,7 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr } private fun scrollToThumbOffset(thumbOffset: Int) { - val clampedThumbOffset = thumbOffset.clamp(0, thumbOffsetRange) + val clampedThumbOffset = thumbOffset.coerceAtLeast(0).coerceAtMost(thumbOffsetRange) val scrollOffset = (scrollOffsetRange.toLong() * clampedThumbOffset / thumbOffsetRange).toInt() - diff --git a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt index 937bf15ad..d76a2dc0f 100644 --- a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt +++ b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt @@ -25,22 +25,20 @@ import coil.request.Disposable import coil.request.ImageRequest import coil.size.Size import org.oxycblt.auxio.music.Song -import org.oxycblt.auxio.util.GenerationGuard +import org.oxycblt.auxio.util.TaskGuard /** * A utility to provide bitmaps in a manner less prone to race conditions. * * Pretty much each service component needs to load bitmaps of some kind, but doing a blind image - * request with some target callbacks could result in overlapping requests causing unrelated - * updates. This class (to an extent) resolves this by keeping track of the current request and - * disposing of it every time a new request is created. This greatly reduces the surface for race - * conditions. + * request with some target callbacks could result in overlapping requests causing incorrect + * updates. This class (to an extent) resolves this by adding several guards * * @author OxygenCobalt */ class BitmapProvider(private val context: Context) { private var currentRequest: Request? = null - private var guard = GenerationGuard() + private var guard = TaskGuard() /** If this provider is currently attempting to load something. */ val isBusy: Boolean @@ -52,7 +50,7 @@ class BitmapProvider(private val context: Context) { */ @Synchronized fun load(song: Song, target: Target) { - val generation = guard.newHandle() + val handle = guard.newHandle() currentRequest?.run { disposable.dispose() } currentRequest = null @@ -64,12 +62,12 @@ class BitmapProvider(private val context: Context) { .size(Size.ORIGINAL) .target( onSuccess = { - if (guard.check(generation)) { + if (guard.check(handle)) { target.onCompleted(it.toBitmap()) } }, onError = { - if (guard.check(generation)) { + if (guard.check(handle)) { target.onCompleted(null) } }) 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 ddeec19fd..b0819d742 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicStore.kt @@ -34,7 +34,9 @@ import org.oxycblt.auxio.util.contentResolverSafe * a typical DB and a mem-cache, like Vinyl. But why would we do that when I've encountered no real * issues with the current system. * - * [Library] may not be available at all times, so leveraging [Callback] is recommended. + * [Library] may not be available at all times, so leveraging [Callback] is recommended. Consumers + * should also be aware that [Library] may change while they are running, and design their work + * accordingly. * * @author OxygenCobalt */ diff --git a/app/src/main/java/org/oxycblt/auxio/music/IndexerViewModel.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt similarity index 65% rename from app/src/main/java/org/oxycblt/auxio/music/IndexerViewModel.kt rename to app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt index 809006c79..be68bb4b3 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/IndexerViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt @@ -23,16 +23,19 @@ import kotlinx.coroutines.flow.StateFlow import org.oxycblt.auxio.music.system.Indexer /** - * A ViewModel representing the current music indexing state. + * A ViewModel representing the current indexing state. * @author OxygenCobalt - * - * TODO: Indeterminate state for Home + Settings */ -class IndexerViewModel : ViewModel(), Indexer.Callback { +class MusicViewModel : ViewModel(), Indexer.Callback { private val indexer = Indexer.getInstance() - private val _state = MutableStateFlow(null) - val state: StateFlow = _state + private val _indexerState = MutableStateFlow(null) + /** The current music indexing state. */ + val indexerState: StateFlow = _indexerState + + private val _libraryExists = MutableStateFlow(false) + /** Whether a music library has successfully been loaded. */ + val libraryExists: StateFlow = _libraryExists init { indexer.registerCallback(this) @@ -43,7 +46,11 @@ class IndexerViewModel : ViewModel(), Indexer.Callback { } override fun onIndexerStateChanged(state: Indexer.State?) { - _state.value = state + _indexerState.value = state + + if (state is Indexer.State.Complete && state.response is Indexer.Response.Ok) { + _libraryExists.value = true + } } override fun onCleared() { diff --git a/app/src/main/java/org/oxycblt/auxio/music/StorageFramework.kt b/app/src/main/java/org/oxycblt/auxio/music/StorageFramework.kt index 0540eb4fb..aaad8cfa5 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/StorageFramework.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/StorageFramework.kt @@ -51,15 +51,14 @@ data class Directory(val volume: StorageVolume, val relativePath: String) { context.getString(R.string.fmt_path, volume.getDescriptionCompat(context), relativePath) /** Converts this dir into an opaque document URI in the form of VOLUME:PATH. */ - fun toDocumentUri(): String? { + fun toDocumentUri() = // "primary" actually corresponds to the internal storage, not the primary volume. // Removable storage is represented with the UUID. - return if (volume.isInternalCompat) { + if (volume.isInternalCompat) { "${DOCUMENT_URI_PRIMARY_NAME}:${relativePath}" } else { - "${(volume.uuidCompat ?: return null).uppercase()}:${relativePath}" + volume.uuidCompat?.let { uuid -> "${uuid}:${relativePath}" } } - } companion object { private const val DOCUMENT_URI_PRIMARY_NAME = "primary" diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt index 41dc994f7..ccdbf93a9 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt @@ -33,7 +33,7 @@ import org.oxycblt.auxio.music.MusicStore import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.ui.Sort -import org.oxycblt.auxio.util.GenerationGuard +import org.oxycblt.auxio.util.TaskGuard import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logE import org.oxycblt.auxio.util.logW @@ -64,7 +64,7 @@ class Indexer { private var lastResponse: Response? = null private var indexingState: Indexing? = null - private var guard = GenerationGuard() + private var guard = TaskGuard() private var controller: Controller? = null private var callback: Callback? = null @@ -133,21 +133,21 @@ class Indexer { suspend fun index(context: Context) { requireBackgroundThread() - val generation = guard.newHandle() + val handle = guard.newHandle() val notGranted = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) == PackageManager.PERMISSION_DENIED if (notGranted) { - emitCompletion(Response.NoPerms, generation) + emitCompletion(Response.NoPerms, handle) return } val response = try { val start = System.currentTimeMillis() - val library = indexImpl(context, generation) + val library = indexImpl(context, handle) if (library != null) { logD( "Music indexing completed successfully in " + @@ -163,7 +163,7 @@ class Indexer { Response.Err(e) } - emitCompletion(response, generation) + emitCompletion(response, handle) } /** @@ -178,63 +178,22 @@ class Indexer { /** * "Cancel" the last job by making it unable to send further state updates. This will cause the - * worker operating the job for that specific generation to cancel as soon as it tries to send a + * worker operating the job for that specific handle to cancel as soon as it tries to send a * state update. */ @Synchronized fun cancelLast() { logD("Cancelling last job") - val generation = guard.newHandle() - emitIndexing(null, generation) - } - - @Synchronized - private fun emitIndexing(indexing: Indexing?, generation: Long) { - guard.yield(generation) - - if (indexing == indexingState) { - // Ignore redundant states used when the backends just want to check for - // a cancellation - return - } - - indexingState = indexing - - // If we have canceled the loading process, we want to revert to a previous completion - // whenever possible to prevent state inconsistency. - val state = - indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) } - - controller?.onIndexerStateChanged(state) - callback?.onIndexerStateChanged(state) - } - - private suspend fun emitCompletion(response: Response, generation: Long) { - guard.yield(generation) - - // Swap to the Main thread so that downstream callbacks don't crash from being on - // a background thread. Does not occur in emitIndexing due to efficiency reasons. - withContext(Dispatchers.Main) { - synchronized(this) { - // Do not check for redundancy here, as we actually need to notify a switch - // from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete. - lastResponse = response - indexingState = null - - val state = State.Complete(response) - - controller?.onIndexerStateChanged(state) - callback?.onIndexerStateChanged(state) - } - } + val handle = guard.newHandle() + emitIndexing(null, handle) } /** - * Run the proper music loading process. [generation] must be a truthful value of the generation - * calling this function. + * Run the proper music loading process. [handle] must be a truthful handle of the task calling + * this function. */ - private fun indexImpl(context: Context, generation: Long): MusicStore.Library? { - emitIndexing(Indexing.Indeterminate, generation) + private fun indexImpl(context: Context, handle: Long): MusicStore.Library? { + emitIndexing(Indexing.Indeterminate, handle) // Since we have different needs for each version, we determine a "Backend" to use // when loading music and then leverage that to create the initial song list. @@ -256,7 +215,7 @@ class Indexer { mediaStoreBackend } - val songs = buildSongs(context, backend, generation) + val songs = buildSongs(context, backend, handle) if (songs.isEmpty()) { return null } @@ -290,7 +249,7 @@ class Indexer { * [buildGenres] functions must be called with the returned list so that all songs are properly * linked up. */ - private fun buildSongs(context: Context, backend: Backend, generation: Long): List { + private fun buildSongs(context: Context, backend: Backend, handle: Long): List { val start = System.currentTimeMillis() var songs = @@ -299,7 +258,7 @@ class Indexer { "Successfully queried media database " + "in ${System.currentTimeMillis() - start}ms") - backend.buildSongs(context, cursor) { emitIndexing(it, generation) } + backend.buildSongs(context, cursor) { emitIndexing(it, handle) } } // Deduplicate songs to prevent (most) deformed music clones @@ -403,6 +362,47 @@ class Indexer { return genres } + @Synchronized + private fun emitIndexing(indexing: Indexing?, handle: Long) { + guard.yield(handle) + + if (indexing == indexingState) { + // Ignore redundant states used when the backends just want to check for + // a cancellation + return + } + + indexingState = indexing + + // If we have canceled the loading process, we want to revert to a previous completion + // whenever possible to prevent state inconsistency. + val state = + indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) } + + controller?.onIndexerStateChanged(state) + callback?.onIndexerStateChanged(state) + } + + private suspend fun emitCompletion(response: Response, handle: Long) { + guard.yield(handle) + + // Swap to the Main thread so that downstream callbacks don't crash from being on + // a background thread. Does not occur in emitIndexing due to efficiency reasons. + withContext(Dispatchers.Main) { + synchronized(this) { + // Do not check for redundancy here, as we actually need to notify a switch + // from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete. + lastResponse = response + indexingState = null + + val state = State.Complete(response) + + controller?.onIndexerStateChanged(state) + callback?.onIndexerStateChanged(state) + } + } + } + /** Represents the current indexer state. */ sealed class State { data class Indexing(val indexing: Indexer.Indexing) : State() diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt index 5e1e678df..72ed9ecc3 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt @@ -102,13 +102,7 @@ class PlaybackPanelFragment : binding.playbackRepeat.setOnClickListener { playbackModel.incrementRepeatMode() } binding.playbackSkipPrev.setOnClickListener { playbackModel.prev() } - - binding.playbackPlayPause.apply { - // Abuse the play/pause FAB (see style definition for more info) - post { binding.playbackPlayPause.stateListAnimator = null } - setOnClickListener { playbackModel.invertPlaying() } - } - + binding.playbackPlayPause.setOnClickListener { playbackModel.invertPlaying() } binding.playbackSkipNext.setOnClickListener { playbackModel.next() } binding.playbackShuffle.setOnClickListener { playbackModel.invertShuffled() } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt index 3f749e7d5..b66400016 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -284,9 +284,10 @@ class PlaybackViewModel(application: Application) : /** * Force restore the last [PlaybackStateManager] saved state, regardless of if a library exists - * or not. + * or not. [onDone] will be called with true if it was successfully done, or false if there was + * no state or if a library was not present. */ - fun restorePlaybackState(onDone: (Boolean) -> Unit) { + fun tryRestorePlaybackState(onDone: (Boolean) -> Unit) { viewModelScope.launch { val restored = playbackManager.restoreState(PlaybackStateDatabase.getInstance(application)) diff --git a/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt b/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt index 1bfad4b7c..3ffa77036 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt @@ -29,7 +29,6 @@ import kotlin.math.pow import org.oxycblt.auxio.music.Album import org.oxycblt.auxio.playback.state.PlaybackStateManager import org.oxycblt.auxio.settings.Settings -import org.oxycblt.auxio.util.clamp import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.unlikelyToBeNull @@ -42,6 +41,8 @@ import org.oxycblt.auxio.util.unlikelyToBeNull * when the active track changes. * * @author OxygenCobalt + * + * TODO: Convert a low-level audio processor capable of handling any kind of PCM data. */ class ReplayGainAudioProcessor(context: Context) : BaseAudioProcessor() { private data class Gain(val track: Float, val album: Float) @@ -235,7 +236,8 @@ class ReplayGainAudioProcessor(context: Context) : BaseAudioProcessor() { sample = (sample * volume) .toInt() - .clamp(Short.MIN_VALUE.toInt(), Short.MAX_VALUE.toInt()) + .coerceAtLeast(Short.MIN_VALUE.toInt()) + .coerceAtMost(Short.MAX_VALUE.toInt()) .toShort() buffer.putLeShort(sample) } 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 910d21f47..c2ada51d2 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 @@ -400,6 +400,9 @@ class PlaybackStateManager private constructor() { logD("Sanitizing state") + // While we could just save and reload the state, we instead sanitize the state + // at runtime for better efficiency (and to sidestep a co-routine on behalf of the caller). + val oldSongId = song?.id val oldPosition = positionMs diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt index b5551b2f7..a6753305c 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt @@ -44,8 +44,9 @@ import org.oxycblt.auxio.util.logD * * @author OxygenCobalt * - * TODO: Update textual metadata first, then cover metadata later. Janky, yes, but also resolves - * some coherency issues. + * TODO: Queue functionality + * + * TODO: Remove the player callback once smooth seeking is implemented */ class MediaSessionComponent( private val context: Context, @@ -60,10 +61,12 @@ class MediaSessionComponent( fun onPostNotification(notification: NotificationComponent?) } - val mediaSession = MediaSessionCompat(context, context.packageName).apply { isActive = true } + private val mediaSession = + MediaSessionCompat(context, context.packageName).apply { isActive = true } private val playbackManager = PlaybackStateManager.getInstance() private val settings = Settings(context, this) + private val notification = NotificationComponent(context, mediaSession.sessionToken) private val provider = BitmapProvider(context) @@ -138,10 +141,18 @@ class MediaSessionComponent( builder.putString(MediaMetadataCompat.METADATA_KEY_DATE, song.album.year.toString()) } - // Normally, android expects one to provide a URI to the metadata instance instead of - // a full blown bitmap. In practice, this is not ideal in the slightest, as we cannot - // provide any user customization or quality of life improvements with a flat URI. - // Instead, we load a full size bitmap and use it within it's respective fields. + // Cover loading is a mess. Android expects you to provide a clean, easy URI for it to + // leverage, but Auxio cannot do that as quality-of-life features like scaling or + // 1:1 cropping could not be used + // + // Thus, we have two options to handle album art: + // 1. Load the bitmap, then post the notification + // 2. Post the notification with text metadata, then post it with the bitmap when it's + // loaded. + // + // Neither of these are good, but 1 is the only one that will work on all versions + // without the notification being eaten by rate-limiting. + provider.load( song, object : BitmapProvider.Target { @@ -262,10 +273,16 @@ class MediaSessionComponent( private fun invalidateSessionState() { logD("Updating media session playback state") - // Position updates arrive faster when you upload a state that is different, as it - // forces the system to re-poll the position. - // FIXME: For some reason however, positions just DON'T UPDATE AT ALL when you - // change from FROM THE APP ONLY WHEN THE PLAYER IS PAUSED. + // There are two unfixable issues with this code: + // 1. If the position is changed while paused (from the app), the position just won't + // update unless I re-post the notification. However, I cannot do such without being + // rate-limited. I cannot believe android rate-limits media notifications when they + // have to be updated as often as they need to. + // 2. Due to metadata updates being delayed but playback remaining ongoing, the position + // will be wonky until we can upload a duration. Again, this ties back to how I must + // aggressively batch notification updates to prevent rate-limiting. + // Android 13 seems to resolve these, but I'm still stuck with these issues below that + // version. // TODO: Add the custom actions for Android 13 val state = PlaybackStateCompat.Builder() @@ -278,10 +295,6 @@ class MediaSessionComponent( .build()) .setBufferedPosition(player.bufferedPosition) - state.setState(PlaybackStateCompat.STATE_NONE, player.bufferedPosition, 1.0f) - - mediaSession.setPlaybackState(state.build()) - val playerState = if (playbackManager.isPlaying) { PlaybackStateCompat.STATE_PLAYING 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 add539b49..8892b4220 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 @@ -68,8 +68,6 @@ import org.oxycblt.auxio.widgets.WidgetProvider * * TODO: Android Auto * - * TODO: Get MediaSessionConnector (or the media3 equivalent) working or die trying - * * @author OxygenCobalt */ class PlaybackService : @@ -248,7 +246,7 @@ class PlaybackService : override fun loadSong(song: Song?) { if (song == null) { - // Clear if there's nothing to play. + // Stop the foreground state if there's nothing to play. logD("Nothing playing, stopping playback") player.stop() stopAndSave() @@ -281,6 +279,7 @@ class PlaybackService : } if (hasPlayed) { + logD("Updating notification") if (!foregroundManager.tryStartForeground(notification)) { notification.post() } diff --git a/app/src/main/java/org/oxycblt/auxio/settings/SettingsCompat.kt b/app/src/main/java/org/oxycblt/auxio/settings/SettingsCompat.kt index 0747919c8..a40bf6f24 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/SettingsCompat.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/SettingsCompat.kt @@ -70,19 +70,21 @@ fun handleAccentCompat(context: Context, prefs: SharedPreferences): Accent { * was a dumb idea, as the choice of a full-blown database for a few paths was overkill, version * boundaries were not respected, and the data format limited us to grokking DATA. * - * In 2.4.0, Auxio switched to a system based on SharedPreferences, also switching from a flat - * path-based excluded system to a volume-based excluded system at the same time. These are both - * rolled into this conversion. + * In 2.4.0, Auxio switched to a system based on SharedPreferences, also switching from a path-based + * excluded system to a volume-based excluded system at the same time. These are both rolled into + * this conversion. */ fun handleExcludedCompat(context: Context, storageManager: StorageManager): List { Log.d("Auxio.SettingsCompat", "Migrating old excluded database") - val db = LegacyExcludedDatabase(context) + // /storage/emulated/0 (the old path prefix) should correspond to primary *emulated* storage. val primaryVolume = storageManager.storageVolumesCompat.find { it.isInternalCompat } ?: return emptyList() + val primaryDirectory = (primaryVolume.directoryCompat ?: return emptyList()) + File.separatorChar - return db.readPaths().map { path -> + + return LegacyExcludedDatabase(context).readPaths().map { path -> val relativePath = path.removePrefix(primaryDirectory) Log.d("Auxio.SettingsCompat", "Migrate $path -> $relativePath") Directory(primaryVolume, relativePath) diff --git a/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt b/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt index 0153ea1fe..096f75866 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt @@ -31,7 +31,7 @@ import androidx.recyclerview.widget.RecyclerView import coil.Coil import org.oxycblt.auxio.R import org.oxycblt.auxio.home.tabs.TabCustomizeDialog -import org.oxycblt.auxio.music.IndexerViewModel +import org.oxycblt.auxio.music.MusicViewModel import org.oxycblt.auxio.music.dirs.MusicDirsDialog import org.oxycblt.auxio.playback.PlaybackViewModel import org.oxycblt.auxio.playback.replaygain.PreAmpCustomizeDialog @@ -51,21 +51,17 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat * @author OxygenCobalt * * TODO: Add option to not restore state - * - * TODO: Disable playback state options when music is loading - * - * TODO: Indicate music loading in the "reload music" option??? */ @Suppress("UNUSED") class SettingsListFragment : PreferenceFragmentCompat() { private val playbackModel: PlaybackViewModel by androidActivityViewModels() - private val indexerModel: IndexerViewModel by activityViewModels() + private val indexerModel: MusicViewModel by activityViewModels() override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) preferenceManager.onDisplayPreferenceDialogListener = this - preferenceScreen.children.forEach(::recursivelyHandlePreference) + preferenceScreen.children.forEach(::setupPreference) // Make the RecycleView edge-to-edge capable view.findViewById(androidx.preference.R.id.recycler_view).apply { @@ -125,7 +121,7 @@ class SettingsListFragment : PreferenceFragmentCompat() { playbackModel.savePlaybackState { context?.showToast(R.string.lbl_state_saved) } } getString(R.string.set_key_restore_state) -> - playbackModel.restorePlaybackState { restored -> + playbackModel.tryRestorePlaybackState { restored -> if (restored) { context?.showToast(R.string.lbl_state_restored) } else { @@ -141,15 +137,14 @@ class SettingsListFragment : PreferenceFragmentCompat() { return true } - /** Recursively handle a preference, doing any specific actions on it. */ - private fun recursivelyHandlePreference(preference: Preference) { + private fun setupPreference(preference: Preference) { val settings = Settings(requireContext()) if (!preference.isVisible) return if (preference is PreferenceCategory) { for (child in preference.children) { - recursivelyHandlePreference(child) + setupPreference(child) } } diff --git a/app/src/main/java/org/oxycblt/auxio/ui/system/ForegroundManager.kt b/app/src/main/java/org/oxycblt/auxio/ui/system/ForegroundManager.kt index 685525fc4..e6e9153b1 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/system/ForegroundManager.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/system/ForegroundManager.kt @@ -33,8 +33,8 @@ class ForegroundManager(private val service: Service) { } /** - * Try to enter a foreground state. Returns false if already in foreground, returns true - * if state was entered. + * Try to enter a foreground state. Returns false if already in foreground, returns true if + * state was entered. */ fun tryStartForeground(notification: ServiceNotification): Boolean { if (isForeground) { @@ -50,8 +50,8 @@ class ForegroundManager(private val service: Service) { } /** - * Try to stop a foreground state. Returns false if already in backend, returns true - * if state was stopped. + * Try to stop a foreground state. Returns false if already in backend, returns true if state + * was stopped. */ fun tryStopForeground(): Boolean { if (!isForeground) { diff --git a/app/src/main/java/org/oxycblt/auxio/ui/system/ServiceNotification.kt b/app/src/main/java/org/oxycblt/auxio/ui/system/ServiceNotification.kt index 6c51e86ae..8955b246e 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/system/ServiceNotification.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/system/ServiceNotification.kt @@ -26,7 +26,8 @@ import androidx.core.app.NotificationCompat import org.oxycblt.auxio.util.getSystemServiceSafe /** - * Wrapper around [NotificationCompat.Builder] that automates parts of the notification setup. + * Wrapper around [NotificationCompat.Builder] that automates parts of the notification setup, under + * the assumption that the notification will be used in a service. * @author OxygenCobalt */ abstract class ServiceNotification(context: Context, info: ChannelInfo) : @@ -35,7 +36,6 @@ abstract class ServiceNotification(context: Context, info: ChannelInfo) : init { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - val channel = NotificationChannel(info.id, context.getString(info.nameRes), info.importance) 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 d06a3b1ce..046d54cad 100644 --- a/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt @@ -19,7 +19,6 @@ package org.oxycblt.auxio.util import android.os.Looper import android.text.format.DateUtils -import androidx.core.math.MathUtils import java.lang.reflect.Field import java.lang.reflect.Method import java.util.concurrent.CancellationException @@ -37,16 +36,12 @@ fun requireBackgroundThread() { * Sanitizes a nullable value that is not likely to be null. On debug builds, requireNotNull is * used, while on release builds, the unsafe assertion operator [!!] ]is used */ -fun unlikelyToBeNull(value: T?): T { - return if (BuildConfig.DEBUG) { +fun unlikelyToBeNull(value: T?) = + if (BuildConfig.DEBUG) { requireNotNull(value) } else { value!! } -} - -/** Shortcut to clamp an integer between [min] and [max] */ -fun Int.clamp(min: Int, max: Int): Int = MathUtils.clamp(this, min, max) /** * Convert a [Long] of seconds into a string duration. @@ -80,22 +75,21 @@ fun lazyReflectedMethod(clazz: KClass<*>, method: String) = lazy { } /** - * A generation-based abstraction that allows cheap cooperative multi-threading in shared object - * contexts. Every new task should call [newHandle], while every running task should call [check] or - * [yield] depending on the context. + * An abstraction that allows cheap cooperative multi-threading in shared object contexts. Every new + * task should call [newHandle], while every running task should call [check] or [yield] depending + * on the context. * * @author OxygenCobalt */ -class GenerationGuard { +class TaskGuard { private var currentHandle = 0L /** - * Returns a new handle to the calling task while invalidating the generations of the previous - * task. + * Returns a new handle to the calling task while invalidating the handle of the previous task. */ @Synchronized fun newHandle() = ++currentHandle - /** Check if the given [handle] is still the one represented by this class. */ + /** Check if the given [handle] is still the one stored by this class. */ @Synchronized fun check(handle: Long) = handle == currentHandle /** diff --git a/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt b/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt index 1e0bde6aa..43f3f7a86 100644 --- a/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt +++ b/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt @@ -21,7 +21,6 @@ import android.content.Context import android.graphics.Bitmap import android.os.Build import coil.request.ImageRequest -import coil.size.Size import coil.transform.RoundedCornersTransformation import org.oxycblt.auxio.R import org.oxycblt.auxio.image.BitmapProvider @@ -112,10 +111,7 @@ class WidgetComponent(private val context: Context) : // bitmap on very large screens. .size(minOf(metrics.widthPixels, metrics.heightPixels, 1024)) } else { - this@WidgetComponent.logD("Doing API 21 cover load") - // Note: Explicitly use the "original" size as without it the scaling logic - // in coil breaks down and results in an error. - builder.size(Size.ORIGINAL) + builder } } diff --git a/app/src/main/res/drawable/ic_pause_32.xml b/app/src/main/res/drawable/ic_pause_32.xml deleted file mode 100644 index ebca0bee5..000000000 --- a/app/src/main/res/drawable/ic_pause_32.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - diff --git a/app/src/main/res/drawable/ic_play_32.xml b/app/src/main/res/drawable/ic_play_32.xml deleted file mode 100644 index 3116666c3..000000000 --- a/app/src/main/res/drawable/ic_play_32.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - diff --git a/app/src/main/res/drawable/ui_accent_circle.xml b/app/src/main/res/drawable/ui_accent_circle.xml index 8d867e3c4..6c7a11916 100644 --- a/app/src/main/res/drawable/ui_accent_circle.xml +++ b/app/src/main/res/drawable/ui_accent_circle.xml @@ -3,7 +3,7 @@ android:color="?attr/colorControlHighlight"> - + diff --git a/app/src/main/res/layout-land/fragment_playback_panel.xml b/app/src/main/res/layout-land/fragment_playback_panel.xml index 8b7e74a40..c0b51c474 100644 --- a/app/src/main/res/layout-land/fragment_playback_panel.xml +++ b/app/src/main/res/layout-land/fragment_playback_panel.xml @@ -127,18 +127,18 @@ app:layout_constraintStart_toEndOf="@+id/playback_repeat" app:layout_constraintTop_toTopOf="@+id/playback_play_pause" /> - + tools:icon="@drawable/ic_play_24" /> - + tools:icon="@drawable/ic_pause_24" /> - + tools:icon="@drawable/ic_pause_24" /> - + tools:icon="@drawable/ic_play_24" /> diff --git a/app/src/main/res/layout/dialog_pre_amp.xml b/app/src/main/res/layout/dialog_pre_amp.xml index 07b6458a2..715987d77 100644 --- a/app/src/main/res/layout/dialog_pre_amp.xml +++ b/app/src/main/res/layout/dialog_pre_amp.xml @@ -14,7 +14,7 @@ android:id="@+id/with_tags_header" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:layout_marginStart="@dimen/spacing_mid_large" + android:layout_marginStart="@dimen/spacing_large" android:layout_marginTop="@dimen/spacing_medium" android:text="@string/set_pre_amp_with" android:textAppearance="@style/TextAppearance.Auxio.TitleMediumLowEmphasis" @@ -38,7 +38,7 @@ android:id="@+id/with_tags_ticker" android:layout_width="0dp" android:layout_height="wrap_content" - android:layout_marginEnd="@dimen/spacing_mid_large" + android:layout_marginEnd="@dimen/spacing_large" android:gravity="center" android:minWidth="@dimen/size_pre_amp_ticker" android:textAppearance="@style/TextAppearance.Auxio.LabelMedium" @@ -51,7 +51,7 @@ android:id="@+id/without_tags_header" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:layout_marginStart="@dimen/spacing_mid_large" + android:layout_marginStart="@dimen/spacing_large" android:layout_marginTop="@dimen/spacing_medium" android:text="@string/set_pre_amp_without" android:textAppearance="@style/TextAppearance.Auxio.TitleMediumLowEmphasis" @@ -75,7 +75,7 @@ android:id="@+id/without_tags_ticker" android:layout_width="0dp" android:layout_height="wrap_content" - android:layout_marginEnd="@dimen/spacing_mid_large" + android:layout_marginEnd="@dimen/spacing_large" android:gravity="center" android:minWidth="@dimen/size_pre_amp_ticker" android:textAppearance="@style/TextAppearance.Auxio.LabelMedium" @@ -88,9 +88,9 @@ android:id="@+id/pre_amp_notice" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_marginStart="@dimen/spacing_mid_large" + android:layout_marginStart="@dimen/spacing_large" android:layout_marginTop="@dimen/spacing_medium" - android:layout_marginEnd="@dimen/spacing_mid_large" + android:layout_marginEnd="@dimen/spacing_large" android:text="@string/set_pre_amp_warning" android:textAlignment="viewStart" android:textAppearance="@style/TextAppearance.Auxio.BodySmall" diff --git a/app/src/main/res/layout/dialog_song_detail.xml b/app/src/main/res/layout/dialog_song_detail.xml index da7c2e892..0d74afb27 100644 --- a/app/src/main/res/layout/dialog_song_detail.xml +++ b/app/src/main/res/layout/dialog_song_detail.xml @@ -15,8 +15,8 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:orientation="vertical" - android:paddingStart="@dimen/spacing_mid_large" - android:paddingEnd="@dimen/spacing_mid_large" + android:paddingStart="@dimen/spacing_large" + android:paddingEnd="@dimen/spacing_large" android:showDividers="middle" android:visibility="gone" tools:visibility="visible"> diff --git a/app/src/main/res/layout/fragment_playback_panel.xml b/app/src/main/res/layout/fragment_playback_panel.xml index b729287da..65e429ffb 100644 --- a/app/src/main/res/layout/fragment_playback_panel.xml +++ b/app/src/main/res/layout/fragment_playback_panel.xml @@ -110,18 +110,18 @@ app:layout_constraintStart_toEndOf="@+id/playback_repeat" app:layout_constraintTop_toTopOf="@+id/playback_play_pause" /> - + tools:icon="@drawable/ic_play_24" /> + android:padding="@dimen/spacing_mid_medium"> + android:padding="@dimen/spacing_mid_medium"> 4dp 8dp - 12dp + 12dp 16dp - 24dp - 32dp + 20dp + 24dp + 32dp -4dp -8dp diff --git a/app/src/main/res/values/styles_android.xml b/app/src/main/res/values/styles_android.xml index 71d3e5951..076967a77 100644 --- a/app/src/main/res/values/styles_android.xml +++ b/app/src/main/res/values/styles_android.xml @@ -2,10 +2,6 @@ - - - - - - - + + + + + +