search

volumio / Volumio2

Volumio 2 - Audiophile Music Player
http://volumio.org
Other
1.37k stars 315 forks source link

Websocket sends too many pushState during song change #1824

Open gilphilbert opened 5 years ago

gilphilbert commented 5 years ago

When changing songs (skipping) on songs in current queue (I've been using MPD, I haven't checked other interfaces) the CommandRouter seems to send multiple state updates to the websocket plugin. This is my log from a simple websocket "skip" request:

stop status for the current track (position 9), not repeated

{"status":"stop","position":9,"title":"As Soon As The Tide Comes In","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/10. As Soon As The Tide Comes In - Del Amitri.flac","trackType":"flac","seek":0,"duration":264,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}

stop status for the new track (position 10), which is repeated (same data three times)

{"status":"stop","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":0,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}
{"status":"stop","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":0,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}
{"status":"stop","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":0,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}

Play state for the new track (position 10), not repeated:

{"status":"play","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":250,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}

Play state for the new track (position 10), which is repeated (same data twice). The only difference between the state above and two below are 46 milliseconds of seek time.

{"status":"play","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":296,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}
{"status":"play","position":10,"title":"Behind The Fool","artist":"Del Amitri","album":"Change Everything","albumart":"/albumart?cacheid=983&web=Del%20Amitri/Change%20Everything/extralarge&path=%2Fmnt%2FNAS%2Fmusichd%2FDel%20Amitri%2FChange%20Everything&metadata=false","uri":"music-library/NAS/musichd/Del Amitri/Change Everything/11. Behind The Fool - Del Amitri.flac","trackType":"flac","seek":296,"duration":198,"samplerate":"44.1 kHz","bitdepth":"24 bit","channels":6,"random":null,"repeat":null,"repeatSingle":false,"consume":false,"volume":100,"disableVolumeControl":true,"mute":false,"stream":"flac","updatedb":false,"volatile":false,"service":"mpd"}

That's seven pushState emits for a single skip event, four of which are duplicates. Due to the non-blocking nature of websockets, these often arrive on different threads making it extremely difficult to determine duplicates due to race conditions. From the perspective of my apps (and I don't claim to represent everyone) the only notification I'm interested in is the last one.

It would be far more friendly to clients if Volumio simply sent a single pushState with the play state for the new track - the stopping of the old and new tracks as they're loaded don't add anything that I'm aware of. If the track failed to play, the state would be "stop" (or the next track in the queue).

I'll dig through the code myself but wanted to log this in case anyone knows this code well and can fix it more quickly than I can!

Phill

lukeffo commented 1 year ago

Hello Team.

I'm facing the same issue with volumio3

Any hint / tip to avoid this behaviour?

Thanks in advance;

Luca