-
-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rearranging a shuffled queue moves songs to unexpected positions #770
Comments
Thank you for the extremely detailed report @unrenowned. I actually made this change because:
The underlying timeline is altered when the shuffled queue is re-arranged, and mutating/replacing the shuffle order itself is a huge performance hit since it triggers a full reload of the player state. I need to find a way to avoid re-introducing the swap behavior since it can no longer work when external apps do a direct move but Auxio internally is only expecting swaps. |
LOL, I figured out this issue months ago and thought it was my problem: androidx/media#954 Yeah. It legit does not update the shuffle order when you move it, I have to do it. I wouldn't know how to patch this in myself since ExoPlayer internals are so nightmarishly difficult to use. |
Re-added old swap code. I don't care if larger moves break it, Android Auto probably doesn't make those anyway. Will wait for them to do androidx/media#1381 |
Describe the Bug/Crash
N.B. This currently only affects the latest commit on the dev branch, but I had some questions about the code in the latest release too so thought I'd mention both in this bug.
When shuffle is enabled, rearranging the queue causes songs to jump to unexpected positions. To reproduce:
shuffle-rearrange-bug.mp4
Describe the intended behavior
The next song should move so it sits behind the current song.
What android version do you use?
Android 13
What device model do you use?
Pixel 2 (Android Emulator -- sorry, I don't have a spare device to test on at the moment!)
Logs
I added some extra debug logs to the code to try and work out what's going on. Here are those logs for the video above:
You can see that Auxio is moving the next song in the shuffle order (Heart Skipped A Beat) to the index of the current song (Shelter), shifting it one index to the left. Because the shuffle order stays constant, that bumps the current song to a completely different position in the shuffle order.
Duplicates
Root Cause
It looks like the code in
ExoPlaybackStateHolder.move()
changed when you broke up theplayback
package. Originally it swapped the positions of the songs, now it just moves the song in thefrom
index to theto
index.However, I don't think reverting to this will work in all cases either. If
abs(to - from) > 1
, you'll get other unexpected results as the songs swap position in the queue/shuffle order and skip over the songs in between them. In practice this doesn't seem to happen unless you drag a song to a different position very quickly, but I thought it was worth mentioning in case this is a problem.I think the simplest solution would be to keep the current code for unshuffled queues, but when shuffle is enabled just rearrange the shuffle order and leave the actual timeline alone. I can write a PR for this if you'd like, but I wanted to check first as I'm not sure if/how you want the underlying timeline to be affected when someone rearranges a shuffled queue.
The text was updated successfully, but these errors were encountered: