rbackhouse / MaximumMPD

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

mpd playlist wiped at MaximumMPD connection #68

Closed macmpi closed 3 years ago

macmpi commented 3 years ago

Hi,

Just noticed some strange behaviour when Maximum MPD first connects to my mpd server while it is playing a playlist. Playback just stops when accessing UI views and playqueue is wiped: I assume this may be due to some erroneous __playlist_to_restore.m3u initialization or something... Here is /var/log/messages when Maximum MPD connects:

Apr 13 14:34:52 tiny-radiocd daemon.notice mpd: player: played "http://direct.franceinter.fr/live/franceinter-midfi.mp3"
Apr 13 14:34:52 tiny-radiocd daemon.err mpd: exception: Failed to open '/var/lib/mpd/playlists/__playlist_to_restore.m3u': No such file or directory
Apr 13 14:34:52 tiny-radiocd daemon.err mpd: exception: No such playlist

If I reload my playlist with UI, then all is fine.

macmpi commented 3 years ago

my bad, again a write access problem on /var/lib/mpd/playlists

macmpi commented 3 years ago

re-opening as it happens also with:

tiny-radiocd:~# ls -la /var/lib/mpd/
total 4
drwxr-xr-x    4 root     root           100 Jan  1  1970 .
drwxr-xr-x   10 root     root           200 Jan  1  1970 ..
drwxr-xr-x    4 mpd      audio           80 Jan  1  1970 music
drwxr-xr-x    2 mpd      audio           80 Jan  1  1970 playlists
-rw-r--r--    1 mpd      audio          471 Apr  1 19:21 tag_cache
rbackhouse commented 3 years ago

Can you provide some more detail on how the playlist was added. I'm assuming that there was something already in the queue before and you used "Play Now" ?

macmpi commented 3 years ago

My server loads a default playlist at startup, and starts playing it, without any user intervention. I may then use MaximumMPD to adjust playback, load other playlists, Play Now other files, etc...

rbackhouse commented 3 years ago

Are you recreating a new server on the same host and port that would start out with a fresh set of mpd directories ? If you have that connection defined in MaximumMPD that pointed to older instance I could see why this scenario would happen

macmpi commented 3 years ago

Yes could be. I frequently setup again the server for debug purpose. Have no idea if Maximum MPD remembers it in some way (IP address may change) or not. I just add-it again from discovery screen.

rbackhouse commented 3 years ago

If you have started to play a song via "Play Now" and the queue already contains songs MaximumMPD stores some state around that previous queue and uses a temporary playlist to save and restore after the "Play Now" song has finished. The state is associated with the connections host and port. If you recreate that MPD instance then the state stored in MaximumMPD could be out of sync. That is why it's trying to restore the temporary playlist and gets an error that it's not found.

macmpi commented 3 years ago

Does this mean if a server does not have a fixed IP, and across reboots changes IP address, the issue may unexpectedly pop-up if it gets back a previously used address under-which ¨Play know¨ had been used before? You may want to use some sort of logic (like flag based on mpd running time if avail) to invalidate stored state information if mpd server has been restarted.

rbackhouse commented 3 years ago

Here's how the current code works

1) You select to play a song via "Play Now" 2) That song along with the current state for "repeat" and "consume" are saved to app storage with a key made up from the current connections host/ipaddr and port 3) If you have songs already in the queue they are saved to a playlist called "__playlist_to_restore"

On status changes the current song playing is compared to the autoplay song. If they are different it is assumed the "Now Play" song has finished or has been removed. 1) The saved values for "repeat" and "consume" are restored 2) The queue is cleared 3) if there is a the playlist called "__playlist_to_restore" it is reloaded to the queue and then deleted 4) The app storage for the connection is deleted.

The reason for the use of app storage is so that if MaximumMPD is shutdown before the "Now Playing" song has completed it can still clean up correctly when reconnected.

You are correct on the fact that a newly assigned ip would mess up the state.

Instead of using the ip and port for the key I should be able to use the name. For discovered connections that would be the one configured in mpd for zeroconf_name and for connections added manually it would be what is entered there.

I think that should handle the potential ip address changes across reboots

macmpi commented 3 years ago

Another thing I was thinking about (not tested): how would that play-out if several (concurrent) mpc clients are accessing the server (clients being MaximumMPD on other idevices or any other mpc clients)?...

How about constructing the updated queue right-on (inserting the play-now file in queue, and add-up the previouly remaining of the queue to follow-up): this would avoid the __playlist_to_restore thingy, and be consistent whatever mpc client comes next.

The remaining issue is about status restore (consume/repeat) after ¨play now" file is played (until oneshot consume is implemented on mpd side)...

rbackhouse commented 3 years ago

It would only be one MaximumMPD instance that initiated the "Play Now" that would be accessing this save state.

Doing this via a simple insert in the queue would certainly be much simpler but as you say there still need to be some way to restore previous consume repeat state.

MPD does have a Sticker API that does give some level of state storage on the MPD server however its not turned on by default.

macmpi commented 3 years ago

Doing this via a simple insert in the queue would certainly be much simpler

yes and might have fewer adverse side-effect in case client & server get out of sync: worst case only consume/repeat status might be lost, but not the actual queue. When a server is restarted, one can safely assume it restarts with its own default status set.

rbackhouse commented 3 years ago

I tried out doing inserts into the current queue using addid, playid and then deleteid after the autoplay song has completed or the current song changes. It appears to work well.

Additionally I changed the key of state saving to use the connection name (either configured zeroconf or entered name) along with the port instead of the ipaddr. I think that should cover reboot ipaddr changes

rbackhouse commented 3 years ago

Available in Version 4.8