From 3f0a532a2df3d639739ba2079e517f10c8866bd1 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Thu, 5 Jan 2023 09:20:21 -0700 Subject: [PATCH] music: allow editing past queue items Allow past and currently playing queue items to be edited, instead of just future queue items. This was a somewhat requested feature that was impossible with the prior queue system. With some fixes, the new queue system can now be used to do this. This even works with edge cases like removing the currently playing song. Albeit, it's likely that more bug fixes and testing will be needed. Resolves #223. --- CHANGELOG.md | 6 ++ .../auxio/playback/PlaybackViewModel.kt | 15 ++-- .../auxio/playback/queue/QueueAdapter.kt | 37 +++------- .../auxio/playback/queue/QueueDragCallback.kt | 18 +---- .../auxio/playback/queue/QueueViewModel.kt | 15 ++-- .../playback/state/PlaybackStateManager.kt | 62 ++++++++++------- .../org/oxycblt/auxio/playback/state/Queue.kt | 68 +++++++++++++++---- .../playback/system/MediaSessionComponent.kt | 17 +++-- .../oxycblt/auxio/widgets/WidgetComponent.kt | 2 +- 9 files changed, 145 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f14fdece8..173367fb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,16 @@ ## dev +#### What's Improved +- Added ability to edit previously played or currently playing items in the queue + #### What's Fixed - Fixed crash that would occur in music folders dialog when user does not have a working file manager +#### What's Changed +- Implemented new queue system + ## 3.0.1 #### What's New 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 0ae5f2765..6a453726b 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt @@ -98,16 +98,23 @@ class PlaybackViewModel(application: Application) : } override fun onIndexMoved(queue: Queue) { - _song.value = playbackManager.queue.currentSong + _song.value = queue.currentSong } - override fun onQueueReworked(queue: Queue) { + override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) { + // Other types of queue changes preserve the current song. + if (change == Queue.ChangeResult.SONG) { + _song.value = queue.currentSong + } + } + + override fun onQueueReordered(queue: Queue) { _isShuffled.value = queue.isShuffled } override fun onNewPlayback(queue: Queue, parent: MusicParent?) { - _song.value = playbackManager.queue.currentSong - _parent.value = playbackManager.parent + _song.value = queue.currentSong + _parent.value = parent _isShuffled.value = queue.isShuffled } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt index cb4ca8a58..d195b26e9 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueAdapter.kt @@ -31,10 +31,7 @@ import org.oxycblt.auxio.list.recycler.PlayingIndicatorAdapter import org.oxycblt.auxio.list.recycler.SongViewHolder import org.oxycblt.auxio.list.recycler.SyncListDiffer import org.oxycblt.auxio.music.Song -import org.oxycblt.auxio.util.context -import org.oxycblt.auxio.util.getAttrColorCompat -import org.oxycblt.auxio.util.getDimen -import org.oxycblt.auxio.util.inflater +import org.oxycblt.auxio.util.* /** * A [RecyclerView.Adapter] that shows an editable list of queue items. @@ -96,30 +93,19 @@ class QueueAdapter(private val listener: EditableListListener) : * @param isPlaying Whether playback is ongoing or paused. */ fun setPosition(index: Int, isPlaying: Boolean) { - var updatedIndex = false + logD("Updating index") + val lastIndex = currentIndex + currentIndex = index - if (index != currentIndex) { - val lastIndex = currentIndex - currentIndex = index - updatedIndex = true - - // Have to update not only the currently playing item, but also all items marked - // as playing. - if (currentIndex < lastIndex) { - notifyItemRangeChanged(0, lastIndex + 1, PAYLOAD_UPDATE_POSITION) - } else { - notifyItemRangeChanged(0, currentIndex + 1, PAYLOAD_UPDATE_POSITION) - } + // Have to update not only the currently playing item, but also all items marked + // as playing. + if (currentIndex < lastIndex) { + notifyItemRangeChanged(0, lastIndex + 1, PAYLOAD_UPDATE_POSITION) + } else { + notifyItemRangeChanged(0, currentIndex + 1, PAYLOAD_UPDATE_POSITION) } - if (this.isPlaying != isPlaying) { - this.isPlaying = isPlaying - // Don't need to do anything if we've already sent an update from changing the - // index. - if (!updatedIndex) { - notifyItemChanged(index, PAYLOAD_UPDATE_POSITION) - } - } + this.isPlaying = isPlaying } private companion object { @@ -158,7 +144,6 @@ class QueueSongViewHolder private constructor(private val binding: ItemQueueSong binding.songAlbumCover.isEnabled = value binding.songName.isEnabled = value binding.songInfo.isEnabled = value - binding.songDragHandle.isEnabled = value } init { diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt index 1fb220c9b..dc9eb13ec 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueDragCallback.kt @@ -35,21 +35,9 @@ import org.oxycblt.auxio.util.logD class QueueDragCallback(private val playbackModel: QueueViewModel) : ItemTouchHelper.Callback() { private var shouldLift = true - override fun getMovementFlags( - recyclerView: RecyclerView, - viewHolder: RecyclerView.ViewHolder - ): Int { - val queueHolder = viewHolder as QueueSongViewHolder - return if (queueHolder.isFuture) { - makeFlag( - ItemTouchHelper.ACTION_STATE_DRAG, ItemTouchHelper.UP or ItemTouchHelper.DOWN) or - makeFlag(ItemTouchHelper.ACTION_STATE_SWIPE, ItemTouchHelper.START) - } else { - // Avoid allowing any touch actions for already-played queue items, as the playback - // system does not currently allow for this. - 0 - } - } + override fun getMovementFlags(recyclerView: RecyclerView, viewHolder: RecyclerView.ViewHolder) = + makeFlag(ItemTouchHelper.ACTION_STATE_DRAG, ItemTouchHelper.UP or ItemTouchHelper.DOWN) or + makeFlag(ItemTouchHelper.ACTION_STATE_SWIPE, ItemTouchHelper.START) override fun onChildDraw( c: Canvas, diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueViewModel.kt index 44c227759..2141a47bd 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueViewModel.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueViewModel.kt @@ -24,6 +24,7 @@ import org.oxycblt.auxio.music.MusicParent import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.playback.state.PlaybackStateManager import org.oxycblt.auxio.playback.state.Queue +import org.oxycblt.auxio.util.logD /** * A [ViewModel] that manages the current queue state and allows navigation through the queue. @@ -79,9 +80,7 @@ class QueueViewModel : ViewModel(), PlaybackStateManager.Listener { * @return true if the items were moved, false otherwise. */ fun moveQueueDataItems(adapterFrom: Int, adapterTo: Int): Boolean { - if (adapterFrom <= playbackManager.queue.index || - adapterTo <= playbackManager.queue.index) { - // Invalid input. Nothing to do. + if (adapterFrom !in queue.value.indices || adapterTo !in queue.value.indices) { return false } playbackManager.moveQueueItem(adapterFrom, adapterTo) @@ -105,14 +104,18 @@ class QueueViewModel : ViewModel(), PlaybackStateManager.Listener { _index.value = queue.index } - override fun onQueueChanged(queue: Queue) { - // Queue changed trivially due to item move -> Diff queue, stay at current index. + override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) { + // Queue changed trivially due to item mo -> Diff queue, stay at current index. replaceQueue = false scrollTo = null _queue.value = queue.resolve() + if (change != Queue.ChangeResult.MAPPING) { + // Index changed, make sure it remains updated without actually scrolling to it. + _index.value = queue.index + } } - override fun onQueueReworked(queue: Queue) { + override fun onQueueReordered(queue: Queue) { // Queue changed completely -> Replace queue, update index replaceQueue = true scrollTo = queue.index 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 11f710456..c5a588226 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 @@ -226,11 +226,7 @@ class PlaybackStateManager private constructor() { * Add a [Song] to the top of the queue. * @param song The [Song] to add. */ - @Synchronized - fun playNext(song: Song) { - queue.playNext(listOf(song)) - notifyQueueChanged() - } + @Synchronized fun playNext(song: Song) = playNext(listOf(song)) /** * Add [Song]s to the top of the queue. @@ -238,19 +234,22 @@ class PlaybackStateManager private constructor() { */ @Synchronized fun playNext(songs: List) { - queue.playNext(songs) - notifyQueueChanged() + val internalPlayer = internalPlayer ?: return + when (queue.playNext(songs)) { + Queue.ChangeResult.MAPPING -> notifyQueueChanged(Queue.ChangeResult.MAPPING) + Queue.ChangeResult.SONG -> { + internalPlayer.loadSong(queue.currentSong, true) + notifyNewPlayback() + } + Queue.ChangeResult.INDEX -> error("Unreachable") + } } /** * Add a [Song] to the end of the queue. * @param song The [Song] to add. */ - @Synchronized - fun addToQueue(song: Song) { - queue.addToQueue(listOf(song)) - notifyQueueChanged() - } + @Synchronized fun addToQueue(song: Song) = addToQueue(listOf(song)) /** * Add [Song]s to the end of the queue. @@ -258,8 +257,15 @@ class PlaybackStateManager private constructor() { */ @Synchronized fun addToQueue(songs: List) { - queue.addToQueue(songs) - notifyQueueChanged() + val internalPlayer = internalPlayer ?: return + when (queue.addToQueue(songs)) { + Queue.ChangeResult.MAPPING -> notifyQueueChanged(Queue.ChangeResult.MAPPING) + Queue.ChangeResult.SONG -> { + internalPlayer.loadSong(queue.currentSong, true) + notifyNewPlayback() + } + Queue.ChangeResult.INDEX -> error("Unreachable") + } } /** @@ -270,8 +276,7 @@ class PlaybackStateManager private constructor() { @Synchronized fun moveQueueItem(src: Int, dst: Int) { logD("Moving item $src to position $dst") - queue.move(src, dst) - notifyQueueChanged() + notifyQueueChanged(queue.move(src, dst)) } /** @@ -280,9 +285,13 @@ class PlaybackStateManager private constructor() { */ @Synchronized fun removeQueueItem(at: Int) { + val internalPlayer = internalPlayer ?: return logD("Removing item at $at") - queue.remove(at) - notifyQueueChanged() + val change = queue.remove(at) + if (change == Queue.ChangeResult.SONG) { + internalPlayer.loadSong(queue.currentSong, playerState.isPlaying) + } + notifyQueueChanged(change) } /** @@ -292,7 +301,7 @@ class PlaybackStateManager private constructor() { @Synchronized fun reorder(shuffled: Boolean) { queue.reorder(shuffled) - notifyQueueReworked() + notifyQueueReordered() } // --- INTERNAL PLAYER FUNCTIONS --- @@ -532,15 +541,15 @@ class PlaybackStateManager private constructor() { } } - private fun notifyQueueChanged() { + private fun notifyQueueChanged(change: Queue.ChangeResult) { for (callback in listeners) { - callback.onQueueChanged(queue) + callback.onQueueChanged(queue, change) } } - private fun notifyQueueReworked() { + private fun notifyQueueReordered() { for (callback in listeners) { - callback.onQueueReworked(queue) + callback.onQueueReordered(queue) } } @@ -575,17 +584,18 @@ class PlaybackStateManager private constructor() { fun onIndexMoved(queue: Queue) {} /** - * Called when the [Queue] changed in a trivial manner, such as a move. + * Called when the [Queue] changed in a manner outlined by the given [Queue.ChangeResult]. * @param queue The new [Queue]. + * @param change The type of [Queue.ChangeResult] that occurred. */ - fun onQueueChanged(queue: Queue) {} + fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) {} /** * Called when the [Queue] has changed in a non-trivial manner (such as re-shuffling), but * the currently playing [Song] has not. * @param queue The new [Queue]. */ - fun onQueueReworked(queue: Queue) {} + fun onQueueReordered(queue: Queue) {} /** * Called when a new playback configuration was created. diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt index 8cb285d69..0c1551b9c 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt @@ -115,12 +115,14 @@ class Queue { /** * Add [Song]s to the top of the queue. Will start playback if nothing is playing. * @param songs The [Song]s to add. + * @return [ChangeResult.MAPPING] if added to an existing queue, or [ChangeResult.SONG] if there + * was no prior playback and these enqueued [Song]s start new playback. */ - fun playNext(songs: List) { + fun playNext(songs: List): ChangeResult { if (orderedMapping.isEmpty()) { // No playback, start playing these songs. start(songs[0], songs, false) - return + return ChangeResult.SONG } val heapIndices = songs.map(::addSongToHeap) @@ -134,17 +136,20 @@ class Queue { // Add the new song in front of the current index in the ordered mapping. orderedMapping.addAll(index, heapIndices) } + return ChangeResult.MAPPING } /** * Add [Song]s to the end of the queue. Will start playback if nothing is playing. * @param songs The [Song]s to add. + * @return [ChangeResult.MAPPING] if added to an existing queue, or [ChangeResult.SONG] if there + * was no prior playback and these enqueued [Song]s start new playback. */ - fun addToQueue(songs: List) { + fun addToQueue(songs: List): ChangeResult { if (orderedMapping.isEmpty()) { // No playback, start playing these songs. start(songs[0], songs, false) - return + return ChangeResult.SONG } val heapIndices = songs.map(::addSongToHeap) @@ -153,14 +158,18 @@ class Queue { if (shuffledMapping.isNotEmpty()) { shuffledMapping.addAll(heapIndices) } + return ChangeResult.MAPPING } /** * Move a [Song] at the given position to a new position. * @param src The position of the [Song] to move. * @param dst The destination position of the [Song]. + * @return [ChangeResult.MAPPING] if the move occurred after the current index, + * [ChangeResult.INDEX] if the move occurred before or at the current index, requiring it to be + * mutated. */ - fun move(src: Int, dst: Int) { + fun move(src: Int, dst: Int): ChangeResult { if (shuffledMapping.isNotEmpty()) { // Move songs only in the shuffled mapping. There is no sane analogous form of // this for the ordered mapping. @@ -170,21 +179,31 @@ class Queue { orderedMapping.add(dst, orderedMapping.removeAt(src)) } - if (index in (src + 1) until dst) { + return when (index) { + // Moving the currently playing song. + src -> { + index = dst + ChangeResult.INDEX + } // Index was ahead of moved song but not ahead of it's destination position. // This makes it functionally a removal, so update the index to preserve consistency. - index -= 1 - } else if (index == src) { - // Moving the currently playing song. - index = dst + in (src + 1)..dst -> { + index -= 1 + ChangeResult.INDEX + } + // Nothing to do. + else -> ChangeResult.MAPPING } } /** * Remove a [Song] at the given position. * @param at The position of the [Song] to remove. + * @return [ChangeResult.MAPPING] if the removed [Song] was after the current index, + * [ChangeResult.INDEX] if the removed [Song] was before the current index, and + * [ChangeResult.SONG] if the currently playing [Song] was removed. */ - fun remove(at: Int) { + fun remove(at: Int): ChangeResult { if (shuffledMapping.isNotEmpty()) { // Remove the specified index in the shuffled mapping and the analogous song in the // ordered mapping. @@ -199,9 +218,16 @@ class Queue { // of the player to be completely invalidated. It's generally easier to not remove the // song and retain player state consistency. - if (index > at) { + return when { + // We just removed the currently playing song. + index == at -> ChangeResult.SONG // Index was ahead of removed song, shift back to preserve consistency. - index -= 1 + index > at -> { + index -= 1 + ChangeResult.INDEX + } + // Nothing to do + else -> ChangeResult.MAPPING } } @@ -232,4 +258,20 @@ class Queue { heap.add(song) return heap.lastIndex } + + /** + * Represents the possible changes that can occur during certain queue mutation events. The + * precise meanings of these differ somewhat depending on the type of mutation done. + */ + enum class ChangeResult { + /** Only the mapping has changed. */ + MAPPING, + /** The mapping has changed, and the index also changed to align with it. */ + INDEX, + /** + * The current song has changed, possibly alongside the mapping and index depending on the + * context. + */ + SONG + } } 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 3833bc188..b86aff5f9 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 @@ -94,15 +94,24 @@ class MediaSessionComponent(private val context: Context, private val listener: // --- PLAYBACKSTATEMANAGER OVERRIDES --- override fun onIndexMoved(queue: Queue) { - updateMediaMetadata(playbackManager.queue.currentSong, playbackManager.parent) + updateMediaMetadata(queue.currentSong, playbackManager.parent) invalidateSessionState() } - override fun onQueueChanged(queue: Queue) { + override fun onQueueChanged(queue: Queue, change: Queue.ChangeResult) { updateQueue(queue) + when (change) { + // Nothing special to do with mapping changes. + Queue.ChangeResult.MAPPING -> {} + // Index changed, ensure playback state's index changes. + Queue.ChangeResult.INDEX -> invalidateSessionState() + // Song changed, ensure metadata changes. + Queue.ChangeResult.SONG -> + updateMediaMetadata(queue.currentSong, playbackManager.parent) + } } - override fun onQueueReworked(queue: Queue) { + override fun onQueueReordered(queue: Queue) { updateQueue(queue) invalidateSessionState() mediaSession.setShuffleMode( @@ -115,7 +124,7 @@ class MediaSessionComponent(private val context: Context, private val listener: } override fun onNewPlayback(queue: Queue, parent: MusicParent?) { - updateMediaMetadata(playbackManager.queue.currentSong, parent) + updateMediaMetadata(queue.currentSong, parent) updateQueue(queue) invalidateSessionState() } 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 42b99d989..6b11f4488 100644 --- a/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt +++ b/app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt @@ -117,7 +117,7 @@ class WidgetComponent(private val context: Context) : // Hook all the major song-changing updates + the major player state updates // to updating the "Now Playing" widget. override fun onIndexMoved(queue: Queue) = update() - override fun onQueueReworked(queue: Queue) = update() + override fun onQueueReordered(queue: Queue) = update() override fun onNewPlayback(queue: Queue, parent: MusicParent?) = update() override fun onStateChanged(state: InternalPlayer.State) = update() override fun onRepeatChanged(repeatMode: RepeatMode) = update()