Play next crashes the app with a single-song queue

SunBreaker1 commented 2 months ago

Describe the Bug/Crash

Select and play a song
exit album and select another song, choose play next
app crashes
Re-open app
select and play song
exit album and select another song, choose add to queue
exit album and select another song, select play next
app doesn't crash

https://github.com/OxygenCobalt/Auxio/assets/85519676/3b3255c0-1434-4132-b4af-20945ec598ed

Describe the intended behavior

App should not crash when selecting a song to "play next"

What android version do you use?

Android 13

What device model do you use?

Samsung S23+ One UI 5.1

Bug report

Due to my concerns with security, id rather not submit a system bug report given the OS says it can contain sensitive and personal info info. Hope this doesn't hinder the bug investigation and fixing.

Duplicates

[X] I have checked the Troubleshooting page.
[X] I have checked this issue for duplicates.
[X] I have checked that this issue occurs on the lastest version.
[X] I agree to the Contribution Guidelines.

OxygenCobalt commented 2 months ago

Could reproduce with a single-song queue. This seems like another thing I forgot to test with gapless playback. Thanks for reporting this.

unrenowned commented 1 month ago

I stumbled onto this bug while looking into #733. Not sure if you're already working on this @OxygenCobalt, but thought I'd mention it here anyway.

This bug doesn't just affect single-song queues. It occurs whenever you try to use "Play next" on the last song of a queue. I've put the (truncated) stracktrace you get when the crash happens below:

FATAL EXCEPTION: main
Process: org.oxycblt.auxio.debug, PID: 11379
java.lang.IllegalArgumentException
    at androidx.media3.common.util.Assertions.checkArgument(Assertions.java:40)
    at androidx.media3.exoplayer.ExoPlayerImpl.addMediaSources(ExoPlayerImpl.java:661)
    at androidx.media3.exoplayer.ExoPlayerImpl.addMediaItems(ExoPlayerImpl.java:637)
    at org.oxycblt.auxio.playback.system.PlaybackService.playNext(PlaybackService.kt:380)
...

From this, I think I can see the cause. When you run getNextMediaItemIndex() at the end of a playlist, you get back C.INDEX_UNSET (-1). This value fails the argument check in ExoPlayerImpl.addMediaSources():

checkArgument(index >= 0)

I'm pretty sure you can fix this with a check in PlaybackService.playNext(), something like this maybe?

val nextIndex = player.nextMediaItemIndex
if (nextIndex == C.INDEX_UNSET) {
    player.addMediaItems(songs.map { it.toMediaItem() })
} else {
    player.addMediaItems(nextIndex, songs.map { it.toMediaItem() })
}

If you want I can make a PR for this: just let me know which branch you want me to merge into as I think dev is a little behind on some shuffle order hotfixes.

OxygenCobalt commented 1 month ago

You're probably right about this @unrenowned. Feel free to branch off hotfixes and make a patch PR, I'm super busy right now.

OxygenCobalt commented 1 month ago

Resolved in #748. Will arrive in a hotfix release eventually.

OxygenCobalt / Auxio