Pato05 / just_audio_media_kit

A platform interface for just_audio which wraps media_kit
https://pub.dev/packages/just_audio_media_kit
The Unlicense
9 stars 11 forks source link

Fix playback stopping after each track #4

Closed Chaphasilor closed 8 months ago

Chaphasilor commented 8 months ago

So I've managed to fix the issue described in #3 by never returning ProcessingStateMessage.completed. That probably breaks stopping, but that's not as much of a problem in our case.
I haven't had time to look into in properly, but I'm guessing the check for when ProcessingStateMessage.completed should be returned instead of ProcessingStateMessage.ready isn't quiet correct, so I'm opening this draft PR to see if you might have an idea where to look for a proper fix?

Pato05 commented 8 months ago

This is a rather odd issue, as I'm using this library in my music app as well, and I have not encountered any issues with playing other items in queue, though I'm playing files from a remote URI instead of a local one, so that might be the reason why it works in my case and doesn't in yours...

Pato05 commented 8 months ago

I have tried to replicate the issue, but failed to do so, as my example is working (you can find it on https://github.com/Pato05/just_audio_media_kit_example).

Please try checking if your code might be doing something wrong, or, otherwise, please try to make a reproducible example so that I can check it, thus test the library against it to find a fix.

ProcessingStateMessage.completed, even if sent, shouldn't trigger anything in just_audio by itself, as the underlying player would still be allowed to continue playback in the next queue item (and it should).

Also, maybe try running your application in debug mode, and providing logs so that we can analyze those and check whether is there anything wrong.

Chaphasilor commented 8 months ago

I did some initial testing (Windows, with media_kit_libs_windows_audio):

What is probably the culprit is this section:

_player.processingStateStream.listen((event) {
  if (event == ProcessingState.completed) {
    stop();
  }
});

(https://github.com/jmshrv/finamp/blob/ac09018ff7db3dabce38bf60e315df9fc67c83e4/lib/services/music_player_background_task.dart#L196-L200)

The question is: why is the behavior different from regular just_audio? We've been running that code for over a year on Android and iOS and never had such an issue with it. It seems like just_audio is not sending out that state when just_audio_media_kit is.

Chaphasilor commented 8 months ago

Expanding on this, I think there's a fundamental semantic difference between just_audio's ProcessingState.completed and media-kit's Player.stream.completed:

The just_audio documentation (https://pub.dev/documentation/just_audio/latest/just_audio/ProcessingState.html) mentions:

completed → const ProcessingState
The player has reached the end of the audio.

"audio" in this case seems to refer to an AudioSource, at least that is what all the other ProcessingStates are based upon. And since ConcatenatingAudioSource is an AudioSource (see "Implementers at https://pub.dev/documentation/just_audio/latest/just_audio/AudioSource-class.html), this should mean that ProcessingState.completed should only be dispatched when the end of the ConcatenatingAudioSource has been reached (and loop mode is off).

media-kit on the other hand offers the following documentation (based on doc comments in player_stream.dart):

/// Currently opened [Media]s.
final Stream<Playlist> playlist;

...

/// Whether end of currently playing [Media] has been reached.
final Stream<bool> completed;

Since the playlist (probably the equivalent of a ConcatenatingAudioSource) is described as a set of multiple Medias, the Player.stream.completed stream seems to fire an event every time a single track has finished playing. At least that would explain the behavior we're seeing.

A better approach might be to use the Player.stream.playlist stream, comparing the Playlists.index to the length of Playlist.medias, and then checking for the Player.stream.completed event only if the last index is playing, all while also respecting the PlaylistMode.
I've implemented this with the latest commit, and it seems to be working.

Pato05 commented 8 months ago

Very interesting.. I wanna thank you for the time you invested in investigating this further, and for your findings. I'll take a better look at this later.

Pato05 commented 8 months ago

Could you also squash your commits into one, and change the commit message to something more specific like "set ProcessingStateMessage.completed only on Playlist end"? Otherwise I could make a Co-Authored commit and close this PR.

Chaphasilor commented 8 months ago

I created the PR with github.dev, and there's not full git integration. I could clone it and do the squash, or you can just use the "Squash and merge" option to merge the PR?
Otherwise, feel free to co-author :)

Pato05 commented 8 months ago

Merged manually.