Rewrite and unbreak sndio backend

[ghost]

The title says it all 😉. I'm hoping this can be merged. Seems to work well together with my lowly iPad 1.

The development branch doesn't currently build on FreeBSD, so this includes a small fix: FreeBSD doesn't have ETIME so I replaced it with ETIMEDOUT. Nothing checks for ETIME so I hope this is fine.

Thank you!

[mikebrady]

This looks interesting, thanks. I'll look at it over the weekend and hopefully pull it in!

[mikebrady]

You've put a lot of work into this!

[mikebrady]

Hi Tobias. I'm just trying this out now on a FreeBSD machine. It looks good.

[mikebrady]

Hi Tobias. I have a few small suggestions; I'll make them later on. Many thanks for this.

[mikebrady]

Hi Tobias. You have done a great deal of work, and I was wondering why synchronisation was just absent.

It turns out that a small bug in your code and a couple of assumptions in Shairport Sync were causing the problem. Anyhow, I'm listening to Shairport Sync on a FreeBSD box using your back end and having good synchronization.

I wrote up a guide, with the necessary workarounds, at FREEBSD.md.

I'd be interested in your experiences.

[ghost]

Thank you Mike!

It works well for me with the workaround you previously proposed i.e. setting

drift_tolerance_in_seconds = 0.005;
resync_threshold_in_seconds = 0.0;

I still have to set them after commit 4d169d7 however, so it doesn't fix the problem.

IIUC, the problem is that sndio will only start reporting delays (i.e. call onmove_cb) after the device buffer is full (or near full). To quote sio_write(3):

the device will wait for playback data to be provided (using the
sio_write() function).  Once enough data is queued to ensure that play
buffers will not underrun, actual playback is started automatically.

[...]

There's a phase during which sio_write() only queues data; once
there's enough data, actual playback starts.

But shairport-sync will try to resync (i.e. flush) after resyncthreshold seconds which it will always hit with the default value. Is it ok for backends to set config.resyncthreshold? If so how about setting the resyncthreshold based on the device's buffer size to allow it to actually fill before trying to resync:

diff --git a/audio_sndio.c b/audio_sndio.c
index 3434324..1c3862c 100644
--- a/audio_sndio.c
+++ b/audio_sndio.c
@@ -57,8 +57,6 @@ static struct sio_hdl *hdl;
 static int framesize;
 static size_t played;
 static size_t written;
-int64_t time_of_last_onmove_cb;
-int at_least_one_onmove_cb_seen;
 
 struct sndio_formats {
   const char *name;
@@ -159,8 +157,6 @@ static int init(int argc, char **argv) {
     die("sndio: cannot open audio device");
 
   written = played = 0;
-  time_of_last_onmove_cb = 0;
-  at_least_one_onmove_cb_seen = 0;
 
   for (i = 0; i < sizeof(formats) / sizeof(formats[0]); i++) {
     if (formats[i].fmt == config.output_format) {
@@ -189,6 +185,7 @@ static int init(int argc, char **argv) {
   config.output_rate = par.rate;
   config.audio_backend_buffer_desired_length = 1.0 * par.bufsz / par.rate;
   config.audio_backend_latency_offset = 0;
+  config.resyncthreshold = 1.0 * par.bufsz / par.rate;
 
   sio_onmove(hdl, onmove_cb, NULL);
 
@@ -207,15 +204,13 @@ static void start(int sample_rate, int sample_format) {
   if (!sio_start(hdl))
     die("sndio: unable to start");
   written = played = 0;
-  time_of_last_onmove_cb = 0;
-  at_least_one_onmove_cb_seen = 0;
   pthread_mutex_unlock(&sndio_mutex);
 }
 
 static void play(short buf[], int frames) {
   if (frames > 0) {
     pthread_mutex_lock(&sndio_mutex);
-    written += sio_write(hdl, buf, frames * framesize);
+    written += sio_write(hdl, buf, frames * framesize) / framesize;
     pthread_mutex_unlock(&sndio_mutex);
   }
 }
@@ -230,25 +225,13 @@ static void stop() {
 
 static void onmove_cb(void *arg, int delta) {
   pthread_mutex_lock(&sndio_mutex);
-  time_of_last_onmove_cb = get_absolute_time_in_fp();
-  at_least_one_onmove_cb_seen = 1;
   played += delta;
   pthread_mutex_unlock(&sndio_mutex);
 }
 
 static int delay(long *_delay) {
   pthread_mutex_lock(&sndio_mutex);
-  size_t estimated_extra_frames_output = 0;
-  if (at_least_one_onmove_cb_seen) { // when output starts, the onmove_cb callback will be made
-    // calculate the difference in time between now and when the last callback occoured,
-    // and use it to estimate the frames that would have been output
-    uint64_t time_difference = get_absolute_time_in_fp() - time_of_last_onmove_cb;
-    uint64_t frame_difference = time_difference * 44100;
-    uint64_t frame_difference_big_integer = frame_difference>>32;
-    estimated_extra_frames_output = frame_difference_big_integer;
-    // debug(1,"Frames played to last cb: %d, estimated to current time: %d.",played,estimated_extra_frames_output);
-  }
-  *_delay = (written / framesize) - (played+estimated_extra_frames_output);
+  *_delay = written - played;
   pthread_mutex_unlock(&sndio_mutex);
   return 0;
 }

[mikebrady]

Thanks for this. I'm going to have to think carefully about it. Right now, on two different real (non-virtual) FreeBSD boxes it works very well indeed on standard settings – in fact, I'd say it's better than on ALSA. On a virtual machine, I continue to get the problem you refer to above. I understand what you're saying, but Shairport Sync has two seconds to allow output to start up and stabilise, and it should be able to adjust to the phenomenon you refer to, so I'm going to be digging further into it.

[mikebrady]

Shairport Sync does not apply the resync threshold to the period of silence that is sent before audio starts, so (I think) your observation doesn't apply. What should happen is that everything should be nice and stable by the time the audio is coming out.