I’ve been developing a plugin for the Phish.in website (source code) and using the YouTube music service plugin as my model for development. While working through getting the playback methods to work, I think I’ve discovered 2 bugs related to the "increasePlaybackTimer" method and the “prefech” method this will call from a music service.
I’ve tested and this affects both my Phish.in plugin and the published YouTube plugin. I think it is the source of the problems that people have reported with the queue advancing oddly and/or stopping altogether. I can reliably reproduce both these bugs by creating a playlist that has both local tracks played by MPD and http tracks played from either YouTube or Phish.in.
(I apologize for these lengthy explanations.)
#1 Timer Overrun Bug
Because both the YouTube plugin and my Phish.in plugin stream tracks via http, MPD can take up to 3-4 seconds to buffer then play a track. This will often happen when seeking. However, the timer will continue to run while the track is buffering. This will push the “remainingTime” variable into negative values.
var remainingTime=this.currentSongDuration-this.currentSeek;The result could be that the “increasePlaybackTimer” method’s 1st if statement and 2nd if statement might run more than once on the same track. First, when the remainingTime variable is still positive but <5000 and <=500 respectively and then both will run any time remainingTime is less than 0 but it hasn’t advanced to the next track yet.
I’ve seen this happen as many as 3 times after a long buffer during seek. The result is that the UI display could advance way past the next track and multiple tracks could be added to the MPD queue. The next track(s) will play but playback will stop after the MPD queue is exhausted and using any playback buttons might cause errors that stop play or crash Volumio.
The fix seems to be pretty easy. I’ve changed line 434 to
if(remainingTime>=0 && remainingTime<5000 && this.askedForPrefetch==false)and line 456 to
if(remainingTime>=0 && remainingTime<=500 && this.prefetchDone==true && this.simulateStopStartDone==false)The extra parameter in the if statements will prevent these blocks from running twice unintentionally if the timer overruns due to buffering.
#1 prefetchDone variable reset Bug
The next bug is a little difficult to understand but I’ve gone through it on my plugin playback thread. The short version is that the prefetchDone variable gets set to “true” in the 1st if statement but only gets reset to “false” if the stateService.status is “stop” (here, here, and here – all 3 inside the conditional “else if (stateService.status === ‘stop’)”).
This is fine if you have consecutive tracks all playing from the same stateService. For example, if consecutive tracks are from MPD, the 1st if statement will run the MPD music service’s “prefetch” method and then the 2nd if will advance the UI track at the same time that MPD queues that song. prefetchDone is mostly irrelevant because the “prefetch” method is run each time.
However, if you have 2 consecutive tracks from different services AFTER you’ve already had 2 consecutive tracks from the same service, things can go wrong. When the 2 consecutive tracks from the same service run, prefetchDone is set to true. BUT if the service is never stopped, prefetchDone is never reset to false (this might be complicated by the way the YouTube & Phish.in plugins use MPD but announce themselves as a different music service). This means when the next track is from another service, the 1st if statement in the “increasePlaybackTimer” method will NOT run prefetch, but with prefetchDone still set to true the 2nd if statement will run when it is not supposed to. This will cause the UI to skip forward 2 tracks and stop since nothing is queued in MPD.
The fix again should be easy. Just set:
this.prefetchDone=false;somewhere after the 2nd if statement is run. It could go at the end of that statement, maybe after line 474. It could also go in the “startPlaybackTimer” method, since that is run in the 2nd if statement.
Whew! Hope that made sense!
If this seems like fixes that won’t cause any problems, I could submit them. I’m not a programmer and this is the first time I’m using GitHub, but I’d be happy to do it if someone could walk me through it. I think I fork the Volumio2 project, make the changes, and then submit a PR to merge the changes back in? Let me know.
Thanks,
Noah