rbackhouse / MaximumMPD

React Native based MPD Client for iOS and Android
MIT License
46 stars 5 forks source link

Direct file play: error on second one if soonafter #76

Open macmpi opened 2 years ago

macmpi commented 2 years ago

Hi,

I just noticed a small issue with direct file playback (swipe-right on a file): initial file will play fine, but it I play another file right-away, then that file won't play and I get the following error on screen:

Capture d’écran   2021-12-07 à 16 52 30

After a little while (few seconds in-between), I can play again another file with swipe-right.

It seems there might be a sync/refresh issue between server & client. You may notice that with short audio files in particular.

macmpi commented 2 years ago

Also noticed that repeat status is changed (i.e. not restored) after play single file. If it was ON before, it becomes OFF after.

rbackhouse commented 2 years ago

The current code should not allow the playing of another "Play Now" song" while one is playing so I'm surprised you were able to just wait a few seconds to do that. If this were very short audio files though I guess there could be a small enough window for that to happen.

I've revisited the code and cleaned up the synchronization of all the required MPD commands. This will allow new "Play Now" songs to be added while one is running. It should also ensure that the repeat and consume values are restored correctly.

It should be in the next release I'me working on now.

macmpi commented 2 years ago

Thanks for the quick fix: works a treat! Beside repeat & consume, I assume random & single are left untouched too (did not check)?

rbackhouse commented 2 years ago

Yes, only repeat & consume are saved and restored

macmpi commented 2 years ago

Hi, There still seem there are some erratic commands sequence management happening. I can reproduce it with a mpd server having a playlist of internet radio streams (repeat on, random off, single off, consume off), and then playing short files (about 1 second) with the play single interface (swipe-right play).

It often works fine (single file will just interrupts radio stream, which will come back right after), but in some cases (several files played one after the other), server sequence becomes erratic...for instance it could come back to another radio stream (some prev/next command might run in wrong order?) Any thought?

rbackhouse commented 2 years ago

It sounds like I will have to attempt to recreate. To recap the scenario :

1) Add multiple entries to the queue 2) Start playing the queue 3) Perform a Play Now on a very short file of around 1 sec multiple times

Sound about right ?

macmpi commented 2 years ago

Yes indeed. Thanks Note, entries to queue are very long (infinite) in case of radio streams, so no chance they go to next without explicit mpc command interventions.

macmpi commented 2 years ago

Hi. Did you have a chance to replicate the issue, or do you need some more info? I noticed the volume fix in the meantime, which was indeed another issue I encountered in the test scenario.

rbackhouse commented 2 years ago

I've tried to recreate with my Mac Mini based MPD server with no luck. I'm wondering if the capabilities of the MPD hosting machine has something to do with it. If I recall you are running on a PiZero. Perhaps that has something to do with it. I have an old Pi series 2 that i might be able to try and recreate on

macmpi commented 2 years ago

Thanks for feedback. Actually my server app can be installed on any platform, and I also made some tests on x86 PCs. There may be a specific issue when using bluetooth A2DP headset (I'm investigating that part), but I got the following error on iphone screen with PC server basic audio card, if it can help figure-out what happens:

error
rbackhouse commented 2 years ago

There are 2 scenarios where the "deleteid" command is use 1) When an autoplay song has completed and is removed from the queue 2) When a new autoplay song is added and there is a current autoplay one already playing. The current one is first deleted before adding the new one.

As much as possible I use MPD command lists to ensure the commands are executed sequentially. I guess it's still possible that a timing issue could be at play here

macmpi commented 2 years ago

Yes possibly. Do/can you protect those playlist manipulations & command queuing from eventual other concurrent MPD clients (may not be other Maximum MPD instances) which may intervene at any time and send their own mpc commands in-between? I guess such things can generate tricky situations...

rbackhouse commented 2 years ago

I don't think there is a way to do that apart from using the command lists. I'm not sure if MPD ensures other client commands are blocked while a list is being processed though

macmpi commented 2 years ago
1. When an autoplay song has completed and is removed from the queue

Wouldn't temporarily setting consume on (and restoring after) achieve the same thing? Though it might be tricky to restore consume mode right after play as there is no callback system... (or are command lists just doing that really?... Is the later one executing only when previous one is completed?)

While at this, have you checked the "One-shot single mode" available since MPD 0.21 as MAFA quotes it: Added support for single oneshot MPD player mode. Oneshot means single mode will only apply to the currently playing track and will reset automatically when MPD moves to the next track.

Was also thinking about this usecase if in random mode initially (not my case): does it need to be done setting prioid?

rbackhouse commented 2 years ago

Wouldn't temporarily setting consume on (and restoring after) achieve the same thing? Though it might be tricky to restore consume mode right after play as there is no callback system... (or are command lists just doing that really?... Is the later one executing only when previous one is completed?)

Yes it should however i would expect the deletion from the queue to be performed indirectly, whereas the deleteid command is directly executed.

command_list are executed in order and the list stopped if one of the commands fails "If a command fails, no more commands are executed and the appropriate ACK error is returned."

While at this, have you checked the "One-shot single mode" available since MPD 0.21 as MAFA quotes it: Added support for single oneshot MPD player mode. Oneshot means single mode will only apply to the currently playing track and will reset automatically when MPD moves to the next track.

I have not yet looked at "One-shot single mode"

macmpi commented 2 years ago

seems like MPD will provide a "one-shot" consume mode with v0.24: it may ease things... (relevant merge)