Play a couple seconds of silence before going idle?

[airdrummingfool]

I'm posting this as an issue before I PR because I'm not sure if this is something snapcast should concern itself with.

I use snapcast to listen to Spotify via librespot. In between songs, librespot stops outputting while it loads the next song (see plietar/librespot#18), which means snapserver goes idle for a time (for me, less than 1 second). This idle time is enough to cause both my macOS and Linux (RPi) clients to lose their sync. On Linux, audio returns pretty much instantly, and full sync (no sleep log statements) is re-established within 3 seconds. On macOS, audio normally returns almost instantly, but it takes some time (anywhere from 3-30 seconds) for the small sleep adjustments to settle down and gain full sync.

In practice, 98% of the time I cannot tell that the clients desync between songs. However, every once in a while, one of my macOS clients will go wonky while recovering, and the first ~20-30 seconds of the song is unintelligible, if not unbearable.

My thought is to have snapserver send up to 2(?) seconds of silence when input dries up, before going idle and allowing the clients to desync. In my testing, this eliminates the desync issue on both macOS and Linux during song changes and when skipping between songs.

Right now my implementation only affects inputs read via processStream.cpp and derivatives (i.e. not when reading from a fifo), but it can easily be copied over to pipeStream.cpp (there might be a better place to implement it). It would be easy to hide behind a compiler flag or a command-line argument if necessary.

Again, I don't even know if this is something that should be implemented in snapcast (vs addressing it in individual sources, such as the librespot issue referenced above). If you do think this is something that would improve snapcast, I'll create a PR so we can go over implementation details.

[airdrummingfool]

@badaix any thoughts on this? I'm interested in seeing if you think this is a good idea.

[badaix]

@airdrummingfool sorry for the seven month delay :)
This sounds actually very good. I will have a deeper look later today and get back to you. We could integrate this into the 0.12 version.

[badaix]

I like it. Maybe it could be another base parameter "silence_ms" (or preferably a better name, maybe "dryout_ms"), encoded in PcmStream's URI, e.g.

 	if (uri_.query.find("silence_ms") != uri_.query.end())
		silenceMs_ = cpt::stoul(uri_.query["silence_ms"]);

with a default value of 2000, if not set

[christf]

can this be closed, now that the PR is merged?

[airdrummingfool]

Yep, thanks for the ping.