ryanheise / just_audio

Audio Player
1.05k stars 675 forks source link

AudioPlayer.play can start playback at unexpected positions within a ConcatenatingAudioSource #591

Closed agersant closed 2 years ago

agersant commented 2 years ago

Which API doesn't behave as documented, and how does it misbehave? When using ConcatenatingAudioSource to manage a playlist, it is possible for AudioPlayer.play() to have unexpected behavior.

Minimal reproduction project https://github.com/agersant/just_audio/tree/just-audio-issue-590 (make sure to grab the just-audio-issue-590 branch)

To Reproduce (i.e. user steps, not code) Steps to reproduce the behavior:

  1. Launch the example app under just_audio_background/example in the branch listed above
  2. Press the 'Queue songs' button to queue 3 audio sources
  3. After playback has started, press the Clear Playlist button. Observe that playback stops (✅)
  4. Press the 'Queue songs' button again. Observe that playback starts on the second item in the playlist instead of the first one (❌).

Error messages None

Expected behavior After step 4, I would expect the first item in the playlist to be playing.

Screenshots N/A

Desktop (please complete the following information):

Smartphone (please complete the following information):

Flutter SDK version

[√] Flutter (Channel stable, 2.5.3, on Microsoft Windows [Version 10.0.19042.1288], locale en-US)
[!] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    X cmdline-tools component is missing
      Run `path/to/sdkmanager --install "cmdline-tools;latest"`
      See https://developer.android.com/studio/command-line for more details.
    X Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/windows#android-setup for more details.
[√] Chrome - develop for the web
[√] Android Studio (version 2020.3)
[√] VS Code (version 1.62.2)
[√] Connected device (3 available)

Additional context In the provided reproduction example, replacing the implementation of clearPlaylist with the following also leads to strange behavior when following the same repro steps (third song in the playlist plays):

void clearPlaylist() async {
  _playlist.clear();
}
fupingren commented 2 years ago

encountered same bug

serraojoao commented 2 years ago

Hi, @agersant did you found a work around for this problem?

ryanheise commented 2 years ago

Although this bug should be fixed, you might be able to workaround it by manually calling seek to reset to the right position and index.

agersant commented 2 years ago

I just tried adding the following call at the end of clearPlaylist in the minimal repro example: _player.seek(Duration(), index: 0);

This did not seem to make a difference, so no known workaround at this time. @serraojoao

ryanheise commented 2 years ago

I have pushed a fix on the fix/insert_skip branch. Can you test it and let me know if it works?

agersant commented 2 years ago

I can confirm this fixes the issue in the minimal reproduction. I could no longer get it to play the wrong source, even when inserting new ones during playback or letting them finish. 🥳

There is one minor issue remaining. When following the repro steps, the highlighted audio source can be seen transitioning from the previously active index to the now active first one. In other words, sequenceStateStream.currentIndex doesn't update as quickly as sequenceStateStream.sequence. I have noticed a similar issue in the past where calling move on a ConcatenatingAudioSource emits a transient state update where the currentIndex and the sequence don't line up.

Regardless, I'm very happy to see the core issue fixed. Thank you Ryan!

ryanheise commented 2 years ago

I've gone ahead and released the fix, but still do let me know how it goes for you and whether we can close the issue.

agersant commented 2 years ago

👍 I can report back after this makes it into a release (and into my real app).

ryanheise commented 2 years ago

after this makes it into a release

Sorry, I meant to say I have released it.

agersant commented 2 years ago

I can confirm this fixed the issue for me in https://github.com/agersant/polaris-android/tree/flutter

The state inconsistency I mentioned in my previous message is also a non-issue for me due to how the app UI is organized. Closing this one out, thanks again!

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs, or use StackOverflow if you need help with just_audio.