Index | Thread | Search

From:
"Jonathan Armani" <jonathan@armani.tech>
Subject:
Re: libsndio: Add an interface to report underruns
To:
"Alexandre Ratchov" <alex@caoua.org>
Cc:
tech@openbsd.org
Date:
Wed, 31 Dec 2025 17:40:57 +0100

Download raw body.

Thread
Coucou Alexandre,

Questions / comments inline, I'm a bit rusty.

On Tue, Dec 30, 2025, at 10:36, Alexandre Ratchov wrote:
> On Mon, Dec 29, 2025 at 05:22:02PM +0100, Omar Polo wrote:
>> Alexandre Ratchov <alex@caoua.org> wrote:
>> > This diff adds the new sio_onxrun(3) function that can be used to
>> > register a callback function that will be called on buffer underrun.
>> > 
>> > There are cases where a program may use an external audio clock
>> > (ex. an RTP stream) and resample to make the local audio rate
>> > match the remote rate to keep the latency constant. To do so,
>> > the program must measure continuously the clock drift and calculate
>> > the resampling ratio. Upon underrun, such programs must restart
>> > the measurements, hence the need for this new interface.
>> > 
>> > The audio/sndiortp port will be the first user of this function,
>> > but other programs could benefit from this as well.
>> > 
>> > This function will be necessary to make sndiod combine multiple audio
>> > devices into a single one (ex. combine webcam's mic with the internal
>> > speaker).
>> 
>> (looking forward to that!)
>> 
>> > OK?
>> 
>> don't feel confident with the internals to give a proper ok, I can just
>> say that it reads fine to me.
>> 
>
> thanks
>
>> noticed one minor omission in the manpage, hence the reply:
>> 
>> > RCS file: /cvs/src/lib/libsndio/sio_open.3,v
>> > diff -u -p -u -p -r1.58 sio_open.3
>> > --- lib/libsndio/sio_open.3	13 Jun 2025 18:34:00 -0000	1.58
>> > +++ lib/libsndio/sio_open.3	27 Dec 2025 17:42:51 -0000
>> > @@ -588,6 +588,21 @@ might overrun; in this case recorded dat
>> >  Similarly if the application cannot provide data to play
>> >  fast enough, the play buffer underruns and silence is played
>> >  instead.
>> 
>> it's missing the sio_onxrun() prototype in the SYNOPSIS?
>>
>
> Outch! it was also missing in the DESCRIPTION. New diff below:
>
> Index: include/sndio.h
> ===================================================================
> RCS file: /cvs/src/include/sndio.h,v
> diff -u -p -u -p -r1.15 sndio.h
> --- include/sndio.h	24 May 2024 15:10:26 -0000	1.15
> +++ include/sndio.h	30 Dec 2025 09:33:58 -0000
> @@ -169,6 +169,7 @@ int sio_setpar(struct sio_hdl *, struct 
>  int sio_getpar(struct sio_hdl *, struct sio_par *);
>  int sio_getcap(struct sio_hdl *, struct sio_cap *);
>  void sio_onmove(struct sio_hdl *, void (*)(void *, int), void *);
> +void sio_onxrun(struct sio_hdl *, void (*)(void *), void *);
>  size_t sio_write(struct sio_hdl *, const void *, size_t);
>  size_t sio_read(struct sio_hdl *, void *, size_t);
>  int sio_start(struct sio_hdl *);
> Index: lib/libsndio/Symbols.map
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/Symbols.map,v
> diff -u -p -u -p -r1.3 Symbols.map
> --- lib/libsndio/Symbols.map	29 Apr 2022 08:30:48 -0000	1.3
> +++ lib/libsndio/Symbols.map	30 Dec 2025 09:33:58 -0000
> @@ -18,6 +18,7 @@
>  		sio_eof;
>  		sio_setvol;
>  		sio_onvol;
> +		sio_onxrun;
> 
>  		mio_open;
>  		mio_close;
> Index: lib/libsndio/amsg.h
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/amsg.h,v
> diff -u -p -u -p -r1.16 amsg.h
> --- lib/libsndio/amsg.h	24 May 2024 15:16:09 -0000	1.16
> +++ lib/libsndio/amsg.h	30 Dec 2025 09:33:58 -0000
> @@ -80,6 +80,7 @@ struct amsg {
>  #define AMSG_CTLSET	14	/* set control value */
>  #define AMSG_CTLSYNC	15	/* end of controls descriptions */
>  #define AMSG_CTLSUB	16	/* ondesc/onctl subscription */
> +#define AMSG_XRUN	17	/* notification about xruns */
>  	uint32_t cmd;
>  	uint32_t __pad;
>  	union {
> @@ -104,6 +105,9 @@ struct amsg {
>  #define AMSG_DATAMAX	0x1000
>  			uint32_t size;
>  		} data;
> +		struct amsg_start {
> +			uint8_t xrunnotify;
> +		} start;
>  		struct amsg_stop {
>  			uint8_t drain;
>  		} stop;
> @@ -113,6 +117,9 @@ struct amsg {
>  		struct amsg_vol {
>  			uint32_t ctl;
>  		} vol;
> +		struct amsg_xrun {
> +			uint8_t state;
> +		} xrun;

Where / for what is state used ?

>  		struct amsg_hello {
>  			uint16_t mode;		/* bitmap of MODE_XXX */
>  #define AMSG_VERSION	7

Shouldn't you bump the version here with the addition of a new message ?

> Index: lib/libsndio/shlib_version
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/shlib_version,v
> diff -u -p -u -p -r1.15 shlib_version
> --- lib/libsndio/shlib_version	16 Jul 2025 15:33:05 -0000	1.15
> +++ lib/libsndio/shlib_version	30 Dec 2025 09:33:58 -0000
> @@ -1,2 +1,2 @@
>  major=8
> -minor=0
> +minor=1
> Index: lib/libsndio/sio.c
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio.c,v
> diff -u -p -u -p -r1.27 sio.c
> --- lib/libsndio/sio.c	29 Apr 2022 08:30:48 -0000	1.27
> +++ lib/libsndio/sio.c	30 Dec 2025 09:33:58 -0000
> @@ -85,6 +85,7 @@ _sio_create(struct sio_hdl *hdl, struct 
>  	hdl->started = 0;
>  	hdl->eof = 0;
>  	hdl->move_cb = NULL;
> +	hdl->xrun_cb = NULL;
>  	hdl->vol_cb = NULL;
>  }
> 
> @@ -123,6 +124,7 @@ sio_start(struct sio_hdl *hdl)
>  	if (!hdl->ops->start(hdl))
>  		return 0;
>  	hdl->started = 1;
> +	hdl->xrun = 0;
>  	return 1;
>  }
> 
> @@ -517,6 +519,7 @@ _sio_onmove_cb(struct sio_hdl *hdl, int 
>  #endif
>  	if (hdl->move_cb)
>  		hdl->move_cb(hdl->move_addr, delta);
> +	hdl->xrun = 0;
>  }
> 
>  int
> @@ -553,4 +556,27 @@ _sio_onvol_cb(struct sio_hdl *hdl, unsig
>  {
>  	if (hdl->vol_cb)
>  		hdl->vol_cb(hdl->vol_addr, ctl);
> +}
> +
> +void
> +sio_onxrun(struct sio_hdl *hdl, void (*cb)(void *), void *addr)
> +{
> +	if (hdl->started) {
> +		DPRINTF("sio_onxrun: already started\n");
> +		hdl->eof = 1;
> +		return;
> +	}
> +	hdl->xrun_cb = cb;
> +	hdl->xrun_addr = addr;
> +}
> +
> +void
> +_sio_onxrun_cb(struct sio_hdl *hdl)
> +{
> +	if (!hdl->xrun) {
> +		hdl->xrun = 1;
> +		if (hdl->xrun_cb)
> +			hdl->xrun_cb(hdl->xrun_addr);
> +	}
> +	DPRINTFN(1, "sndio: xrun\n");
>  }
> Index: lib/libsndio/sio_aucat.c
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio_aucat.c,v
> diff -u -p -u -p -r1.21 sio_aucat.c
> --- lib/libsndio/sio_aucat.c	29 Apr 2022 08:30:48 -0000	1.21
> +++ lib/libsndio/sio_aucat.c	30 Dec 2025 09:33:58 -0000
> @@ -115,6 +115,10 @@ sio_aucat_runmsg(struct sio_aucat_hdl *h
>  			hdl->delta = 0;
>  		}
>  		break;
> +	case AMSG_XRUN:
> +		DPRINTFN(3, "aucat: xrun\n");
> +		_sio_onxrun_cb(&hdl->sio);
> +		break;
>  	case AMSG_SETVOL:
>  		ctl = ntohl(hdl->aucat.rmsg.u.vol.ctl);
>  		hdl->curvol = hdl->reqvol = ctl;
> @@ -196,6 +200,7 @@ sio_aucat_start(struct sio_hdl *sh)
> 
>  	AMSG_INIT(&hdl->aucat.wmsg);
>  	hdl->aucat.wmsg.cmd = htonl(AMSG_START);
> +	hdl->aucat.wmsg.u.start.xrunnotify = 1;

Don't you want it to 1 only if hdl->xrun_cb is defined ?

>  	hdl->aucat.wtodo = sizeof(struct amsg);
>  	if (!_aucat_wmsg(&hdl->aucat, &hdl->sio.eof))
>  		return 0;
> Index: lib/libsndio/sio_open.3
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio_open.3,v
> diff -u -p -u -p -r1.58 sio_open.3
> --- lib/libsndio/sio_open.3	13 Jun 2025 18:34:00 -0000	1.58
> +++ lib/libsndio/sio_open.3	30 Dec 2025 09:33:58 -0000
> @@ -29,6 +29,7 @@
>  .Nm sio_read ,
>  .Nm sio_write ,
>  .Nm sio_onmove ,
> +.Nm sio_onxrun ,
>  .Nm sio_nfds ,
>  .Nm sio_pollfd ,
>  .Nm sio_revents ,
> @@ -67,6 +68,12 @@
>  .Fa "void (*cb)(void *arg, int delta)"
>  .Fa "void *arg"
>  .Fc
> +.Ft void
> +.Fo sio_onxrun
> +.Fa "struct sio_hdl *hdl"
> +.Fa "void (*cb)(void *arg)"
> +.Fa "void *arg"
> +.Fc
>  .Ft int
>  .Fn sio_nfds "struct sio_hdl *hdl"
>  .Ft int
> @@ -588,6 +595,21 @@ might overrun; in this case recorded dat
>  Similarly if the application cannot provide data to play
>  fast enough, the play buffer underruns and silence is played
>  instead.
> +.Pp
> +The
> +.Fn sio_onxrun
> +function can be used to register the
> +.Fn cb
> +callback function called at buffer underrun or overrun.
> +It is called by
> +.Fn sio_read ,
> +.Fn sio_write ,
> +and
> +.Fn sio_revents .
> +The value of the
> +.Fa arg
> +pointer is passed to the callback and can contain anything.
> +.Pp
>  Depending on the
>  .Fa xrun
>  parameter of the
> Index: lib/libsndio/sio_priv.h
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio_priv.h,v
> diff -u -p -u -p -r1.12 sio_priv.h
> --- lib/libsndio/sio_priv.h	21 May 2024 06:07:06 -0000	1.12
> +++ lib/libsndio/sio_priv.h	30 Dec 2025 09:33:58 -0000
> @@ -30,6 +30,8 @@ struct sio_hdl {
>  	void *move_addr;		/* user priv. data for move_cb */
>  	void (*vol_cb)(void *, unsigned); /* call-back for volume changes */
>  	void *vol_addr;			/* user priv. data for vol_cb */
> +	void (*xrun_cb)(void *);	/* call-back for xruns */
> +	void *xrun_addr;		/* user priv. data for xrun_cb */
>  	unsigned mode;			/* SIO_PLAY | SIO_REC */
>  	int started;			/* true if started */
>  	int nbio;			/* true if non-blocking io */
> @@ -38,6 +40,7 @@ struct sio_hdl {
>  	int wsil;			/* silence to play */
>  	int rused;			/* bytes used in read buffer */
>  	int wused;			/* bytes used in write buffer */
> +	int xrun;			/* xrun reported */
>  	long long cpos;			/* clock since start */
>  	struct sio_par par;
>  #ifdef DEBUG
> @@ -71,6 +74,7 @@ struct sio_hdl *_sio_sun_open(const char
>  void _sio_create(struct sio_hdl *, struct sio_ops *, unsigned, int);
>  void _sio_onmove_cb(struct sio_hdl *, int);
>  void _sio_onvol_cb(struct sio_hdl *, unsigned);
> +void _sio_onxrun_cb(struct sio_hdl *);
>  #ifdef DEBUG
>  void _sio_printpos(struct sio_hdl *);
>  #endif
> Index: lib/libsndio/sio_sun.c
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio_sun.c,v
> diff -u -p -u -p -r1.32 sio_sun.c
> --- lib/libsndio/sio_sun.c	11 Nov 2025 11:08:10 -0000	1.32
> +++ lib/libsndio/sio_sun.c	30 Dec 2025 09:33:58 -0000
> @@ -542,6 +542,9 @@ sio_sun_xrun(struct sio_sun_hdl *hdl)
>  	 */
>  	if (!sio_sun_flush(&hdl->sio))
>  		return 0;
> +
> +	_sio_onxrun_cb(&hdl->sio);
> +
>  	if (!sio_sun_start(&hdl->sio))
>  		return 0;
> 
> Index: usr.bin/sndiod/dev.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
> diff -u -p -u -p -r1.126 dev.c
> --- usr.bin/sndiod/dev.c	26 Nov 2025 08:40:16 -0000	1.126
> +++ usr.bin/sndiod/dev.c	30 Dec 2025 09:33:58 -0000
> @@ -28,6 +28,7 @@
>  #include "utils.h"
> 
>  void zomb_onmove(void *);
> +void zomb_onxrun(void *);
>  void zomb_onvol(void *);
>  void zomb_fill(void *);
>  void zomb_flush(void *);
> @@ -64,6 +65,7 @@ int slot_skip(struct slot *);
> 
>  struct slotops zomb_slotops = {
>  	zomb_onmove,
> +	zomb_onxrun,
>  	zomb_onvol,
>  	zomb_fill,
>  	zomb_flush,
> @@ -91,6 +93,11 @@ zomb_onmove(void *arg)
>  }
> 
>  void
> +zomb_onxrun(void *arg)
> +{
> +}
> +
> +void
>  zomb_onvol(void *arg)
>  {
>  }
> @@ -635,9 +642,13 @@ dev_cycle(struct dev *d)
>  			s->sub.buf.len - s->sub.buf.used <
>  			s->round * s->sub.bpf)) {
> 
> +			if (!s->paused) {
>  #ifdef DEBUG
> -			logx(3, "slot%zu: xrun, pause cycle", s - slot_array);
> +				logx(3, "slot%zu: xrun, paused", s - slot_array);
>  #endif
> +				s->paused = 1;
> +				s->ops->onxrun(s->arg);

The notification is completly independant from the XRUN_IGNORE ?

> +			}
>  			if (s->xrun == XRUN_IGNORE) {
>  				s->delta -= s->round;
>  				ps = &s->next;
> @@ -654,7 +665,15 @@ dev_cycle(struct dev *d)
>  #endif
>  			}
>  			continue;
> +		} else {
> +			if (s->paused) {
> +#ifdef DEBUG
> +				logx(3, "slot%zu: resumed\n", s - slot_array);
> +#endif
> +				s->paused = 0;
> +			}
>  		}
> +
>  		if ((s->mode & MODE_RECMASK) && !(s->pstate == SLOT_STOP)) {
>  			if (s->sub.prime == 0) {
>  				dev_sub_bcopy(d, s);
> @@ -1588,6 +1607,7 @@ slot_start(struct slot *s)
>  		s->sub.prime = d->bufsz / d->round;
>  	}
>  	s->skip = 0;
> +	s->paused = 0;
> 
>  	/*
>  	 * get the current position, the origin is when the first sample
> Index: usr.bin/sndiod/dev.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/dev.h,v
> diff -u -p -u -p -r1.50 dev.h
> --- usr.bin/sndiod/dev.h	26 Nov 2025 08:40:16 -0000	1.50
> +++ usr.bin/sndiod/dev.h	30 Dec 2025 09:33:58 -0000
> @@ -40,6 +40,7 @@
>  struct slotops
>  {
>  	void (*onmove)(void *);			/* clock tick */
> +	void (*onxrun)(void *);			/* xrun */
>  	void (*onvol)(void *);			/* tell client vol changed */
>  	void (*fill)(void *);			/* request to fill a play block */
>  	void (*flush)(void *);			/* request to flush a rec block */
> @@ -100,6 +101,7 @@ struct slot {
>  #define SLOT_RUN	3			/* buffer attached to device */
>  #define SLOT_STOP	4			/* draining */
>  	int pstate;
> +	int paused;				/* paused because of xrun */
> 
>  	struct app *app;
>  };
> Index: usr.bin/sndiod/siofile.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/siofile.c,v
> diff -u -p -u -p -r1.29 siofile.c
> --- usr.bin/sndiod/siofile.c	11 Nov 2025 11:48:03 -0000	1.29
> +++ usr.bin/sndiod/siofile.c	30 Dec 2025 09:33:58 -0000
> @@ -36,6 +36,7 @@
>  #define WATCHDOG_USEC	4000000		/* 4 seconds */
> 
>  void dev_sio_onmove(void *, int);
> +void dev_sio_onxrun(void *);
>  void dev_sio_timeout(void *);
>  int dev_sio_pollfd(void *, struct pollfd *);
>  int dev_sio_revents(void *, struct pollfd *);
> @@ -74,6 +75,19 @@ dev_sio_onmove(void *arg, int delta)
>  }
> 
>  void
> +dev_sio_onxrun(void *arg)
> +{
> +	struct dev *d = arg;
> +	struct slot *s;
> +
> +#ifdef DEBUG
> +	logx(1, "%s: xrun", d->path);
> +#endif
> +	for (s = d->slot_list; s != NULL; s = s->next)
> +		s->ops->onxrun(s->arg);
> +}
> +
> +void
>  dev_sio_timeout(void *arg)
>  {
>  	struct dev *d = arg;
> @@ -213,6 +227,7 @@ dev_sio_open(struct dev *d)
>  	if (d->mode & MODE_PLAY)
>  		d->mode |= MODE_MON;
>  	sio_onmove(d->sio.hdl, dev_sio_onmove, d);
> +	sio_onxrun(d->sio.hdl, dev_sio_onxrun, d);
>  	d->sio.file = file_new(&dev_sio_ops, d, "dev", sio_nfds(d->sio.hdl));
>  	if (d->sioctl.hdl) {
>  		d->sioctl.file = file_new(&dev_sioctl_ops, d, "mix",
> Index: usr.bin/sndiod/sock.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/sock.c,v
> diff -u -p -u -p -r1.55 sock.c
> --- usr.bin/sndiod/sock.c	19 Jun 2025 20:16:34 -0000	1.55
> +++ usr.bin/sndiod/sock.c	30 Dec 2025 09:33:58 -0000
> @@ -40,6 +40,7 @@ void sock_slot_fill(void *);
>  void sock_slot_flush(void *);
>  void sock_slot_eof(void *);
>  void sock_slot_onmove(void *);
> +void sock_slot_onxrun(void *);
>  void sock_slot_onvol(void *);
>  void sock_midi_imsg(void *, unsigned char *, int);
>  void sock_midi_omsg(void *, unsigned char *, int);
> @@ -77,6 +78,7 @@ struct fileops sock_fileops = {
> 
>  struct slotops sock_slotops = {
>  	sock_slot_onmove,
> +	sock_slot_onxrun,
>  	sock_slot_onvol,
>  	sock_slot_fill,
>  	sock_slot_flush,
> @@ -244,6 +246,21 @@ sock_slot_onmove(void *arg)
>  }
> 
>  void
> +sock_slot_onxrun(void *arg)
> +{
> +	struct sock *f = (struct sock *)arg;
> +	struct slot *s = f->slot;
> +
> +#ifdef DEBUG
> +	logx(4, "slot%zu: onxrun: notify = %d", s - slot_array, f->xrunnotify);
> +#endif
> +	if (s->pstate != SOCK_START)
> +		return;
> +	if (f->xrunnotify)
> +		f->xrunpending = 1;
> +}
> +
> +void
>  sock_slot_onvol(void *arg)
>  {
>  	struct sock *f = (struct sock *)arg;
> @@ -301,6 +318,7 @@ sock_new(int fd)
>  	f->midi = NULL;
>  	f->ctlslot = NULL;
>  	f->tickpending = 0;
> +	f->xrunpending = 0;
>  	f->fillpending = 0;
>  	f->stoppending = 0;
>  	f->wstate = SOCK_WIDLE;
> @@ -309,6 +327,7 @@ sock_new(int fd)
>  	f->rtodo = sizeof(struct amsg);
>  	f->wmax = f->rmax = 0;
>  	f->lastvol = -1;
> +	f->xrunnotify = 0;
>  	f->ctlops = 0;
>  	f->ctlsyncpending = 0;
>  	f->file = file_new(&sock_fileops, f, "sock", 1);
> @@ -882,6 +901,9 @@ sock_execmsg(struct sock *f)
>  			return 0;
>  		}
>  		f->tickpending = 0;
> +		f->xrunpending = 0;
> +		if (AMSG_ISSET(m->u.start.xrunnotify))
> +			f->xrunnotify = m->u.start.xrunnotify;

Any reasons others (msb, par) use f->field = m->field ? 0 : 1 but not this one ?

>  		f->stoppending = 0;
>  		slot_start(s);
>  		if (s->mode & MODE_PLAY) {
> @@ -1161,6 +1183,18 @@ sock_buildmsg(struct sock *f)
>  		 * slot->delta
>  		 */
>  		f->slot->delta = 0;
> +		return 1;
> +	}
> +
> +	if (f->xrunpending) {
> +#ifdef DEBUG
> +		logx(4, "sock %d: building XRUN message", f->fd);
> +#endif
> +		AMSG_INIT(&f->wmsg);
> +		f->wmsg.cmd = htonl(AMSG_XRUN);
> +		f->wtodo = sizeof(struct amsg);
> +		f->wstate = SOCK_WMSG;
> +		f->xrunpending = 0;
>  		return 1;
>  	}
> 
> Index: usr.bin/sndiod/sock.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/sock.h,v
> diff -u -p -u -p -r1.9 sock.h
> --- usr.bin/sndiod/sock.h	24 May 2024 15:16:09 -0000	1.9
> +++ usr.bin/sndiod/sock.h	30 Dec 2025 09:33:58 -0000
> @@ -50,6 +50,8 @@ struct sock {
>  #define SOCK_STOP	4		/* draining rec buffers */
>  	unsigned int pstate;		/* one of the above */
>  	int tickpending;		/* tick waiting to be transmitted */
> +	int xrunpending;		/* xrun waiting to be transmitted */
> +	int xrunnotify;			/* client subscribed to xrun messages */
>  	int fillpending;		/* flowctl waiting to be transmitted */
>  	int stoppending;		/* last STOP ack to be sent */
>  	unsigned int walign;		/* align written data to this */