You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 an album/artist/playlist
Check that the current and next song in the queue are not adjacent in the unshuffled playlist.
Swap the current and next song in the queue
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:
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D shuffle order = 6, 4, 1, 2, 10, 5, 9, 8, 0, 3, 7
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D old timeline = 0. Intro, 1. VCR, 2. Crystalised, 3. Islands, 4. Heart Skipped A Beat, 5. Fantasy, 6. Shelter, 7. Basic Space, 8. Infinity, 9. Night Time, 10. Stars
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D current index = 6. Shelter
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D next item = 4. Heart Skipped A Beat
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D from = 1, to = 0
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D trueFrom = 4, trueTo = 6
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D new timeline = 0. Intro, 1. VCR, 2. Crystalised, 3. Islands, 4. Fantasy, 5. Shelter, 6. Heart Skipped A Beat, 7. Basic Space, 8. Infinity, 9. Night Time, 10. Stars
...
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D new current index = 5. Shelter
22260-22260 ExoPlaybackStateHolder org.oxycblt.auxio.debug D new next item = 9. Night Time
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.
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:
Thank you for the extremely detailed report @unrenowned. I actually made this change because:
Media3's interface requires a more standard "move" operation rather than my swap behavior.
I thought the swapping was actually a no-op since I forgot about this index shifting edge case. I'm pretty sure this is why I was swapping in the first place to more or less hack around it.
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: