sndiod: Allow alternative devices to support different modes.

Currently sndiod does not allow you to use alternative devices (-F
devices) which support only a subset of the modes of the main (-f)
device.

For example, if you do `sndiod -f rsnd/0 -F rsnd/1` and:
 - rsnd/0 is full-duplex (rec + play).
 - rsnd/1 is play-only.

Then you will be unable to use rsnd/1 as sndiod deems it incompatible
and refuses to use it (similarly if rsnd/1 is record-only).

This is annoying. It means if you want to use a record-only or play-only
device, you will either have to kill sndiod and restart it specifying
only that device (`sndiod -f rsnd/1` for the above example), or failing
that, downgrade the functionality of the main device (`-m play`).

This diff (a joint effort between ratchov@ and myself) makes mixing
devices with different modes possible. It does this by making both
recording and playing available for all devices, even if the underlying
hardware doesn't support both modes.

For example, if I try to record from a play-only device, then recording
will succeed, but the captured PCM data will be pure silence. Similarly,
if I try to play to a record-only device, then the audio stream will
disappear into the ether.

This is mostly a no-op for sndiod in the default configuration (except
that play-only devices now accept recording clients). If you use
alternative devices (-F), then it's possible for a record-only device to
be found first, which may be confusing if you just want to hear sound.
We can only assume that if you deviate from defaults, then you know what
you are doing.

With guidance from, and OK ratchov@, thanks!

diff --git a/usr.bin/sndiod/dev.c b/usr.bin/sndiod/dev.c
index 6e2e1a6..7dc8bac 100644 (file)
--- a/usr.bin/sndiod/dev.c
+++ b/usr.bin/sndiod/dev.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: dev.c,v 1.90 2021/02/02 11:18:57 ratchov Exp $        */
+/*     $OpenBSD: dev.c,v 1.91 2021/03/02 12:15:46 edd Exp $    */
 /*
  * Copyright (c) 2008-2012 Alexandre Ratchov <alex@caoua.org>
  *
@@ -1142,43 +1142,51 @@ dev_adjpar(struct dev *d, int mode,
 /*
  * Open the device with the dev_reqxxx capabilities. Setup a mixer, demuxer,
  * monitor, midi control, and any necessary conversions.
+ *
+ * Note that record and play buffers are always allocated, even if the
+ * underlying device doesn't support both modes.
  */
 int
 dev_allocbufs(struct dev *d)
 {
-       if (d->mode & MODE_REC) {
-               /*
-                * Create device <-> demuxer buffer
-                */
-               d->rbuf = xmalloc(d->round * d->rchan * sizeof(adata_t));
+       /*
+        * Create record buffer.
+        */
 
-               /*
-                * Insert a converter, if needed.
-                */
-               if (!aparams_native(&d->par)) {
-                       dec_init(&d->dec, &d->par, d->rchan);
-                       d->decbuf = xmalloc(d->round * d->rchan * d->par.bps);
-               } else
-                       d->decbuf = NULL;
-       }
-       if (d->mode & MODE_PLAY) {
-               /*
-                * Create device <-> mixer buffer
-                */
-               d->poffs = 0;
-               d->psize = d->bufsz + d->round;
-               d->pbuf = xmalloc(d->psize * d->pchan * sizeof(adata_t));
-               d->mode |= MODE_MON;
+        /* Create device <-> demuxer buffer */
+       d->rbuf = xmalloc(d->round * d->rchan * sizeof(adata_t));
+
+       /* Insert a converter, if needed. */
+       if (!aparams_native(&d->par)) {
+               dec_init(&d->dec, &d->par, d->rchan);
+               d->decbuf = xmalloc(d->round * d->rchan * d->par.bps);
+       } else
+               d->decbuf = NULL;
+
+       /*
+        * Create play buffer
+        */
+
+       /* Create device <-> mixer buffer */
+       d->poffs = 0;
+       d->psize = d->bufsz + d->round;
+       d->pbuf = xmalloc(d->psize * d->pchan * sizeof(adata_t));
+       d->mode |= MODE_MON;
+
+       /* Append a converter, if needed. */
+       if (!aparams_native(&d->par)) {
+               enc_init(&d->enc, &d->par, d->pchan);
+               d->encbuf = xmalloc(d->round * d->pchan * d->par.bps);
+       } else
+               d->encbuf = NULL;
+
+       /*
+        * Initially fill the record buffer with zeroed samples. This ensures
+        * that when a client records from a play-only device the client just
+        * gets silence.
+        */
+       memset(d->rbuf, 0, d->round * d->rchan * sizeof(adata_t));
 
-               /*
-                * Append a converter, if needed.
-                */
-               if (!aparams_native(&d->par)) {
-                       enc_init(&d->enc, &d->par, d->pchan);
-                       d->encbuf = xmalloc(d->round * d->pchan * d->par.bps);
-               } else
-                       d->encbuf = NULL;
-       }
        if (log_level >= 2) {
                dev_log(d);
                log_puts(": ");
@@ -1935,8 +1943,6 @@ found:
                        slot_log(s);
                        log_puts(": requested mode not supported\n");
                }
-               dev_unref(opt->dev);
-               return NULL;
        }
        s->opt = opt;
        s->ops = ops;
@@ -2055,13 +2061,7 @@ slot_attach(struct slot *s)
         * because dev_xxx() functions are supposed to
         * work (i.e., not to crash)
         */
-#ifdef DEBUG
-       if ((s->mode & d->mode) != s->mode) {
-               slot_log(s);
-               log_puts(": mode beyond device mode, not attaching\n");
-               panic();
-       }
-#endif
+
        s->next = d->slot_list;
        d->slot_list = s;
        if (s->mode & MODE_PLAY) {

diff --git a/usr.bin/sndiod/siofile.c b/usr.bin/sndiod/siofile.c
index ddc6a59..d620b1f 100644 (file)
--- a/usr.bin/sndiod/siofile.c
+++ b/usr.bin/sndiod/siofile.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: siofile.c,v 1.22 2020/06/28 05:21:39 ratchov Exp $    */
+/*     $OpenBSD: siofile.c,v 1.23 2021/03/02 12:15:46 edd Exp $        */
 /*
  * Copyright (c) 2008-2012 Alexandre Ratchov <alex@caoua.org>
  *
@@ -87,37 +87,76 @@ dev_sio_timeout(void *arg)
        dev_abort(d);
 }
 
+static int
+dev_sio_openalt(struct dev *d, struct dev_alt *n,
+    struct sio_hdl **rhdl, struct sioctl_hdl **rctlhdl, unsigned int *rmode)
+{
+       struct sio_hdl *hdl;
+       struct sioctl_hdl *ctlhdl;
+       unsigned int mode = d->reqmode & (MODE_PLAY | MODE_REC);
+
+       hdl = fdpass_sio_open(d->num, n->idx, mode);
+       if (hdl == NULL) {
+               if (mode != (SIO_PLAY | SIO_REC))
+                       return 0;
+               hdl = fdpass_sio_open(d->num, n->idx, SIO_PLAY);
+               if (hdl != NULL)
+                       mode = SIO_PLAY;
+               else {
+                       hdl = fdpass_sio_open(d->num, n->idx, SIO_REC);
+                       if (hdl != NULL)
+                               mode = SIO_REC;
+                       else
+                               return 0;
+               }
+               if (log_level >= 1) {
+                       log_puts("warning, device opened in ");
+                       log_puts(mode == SIO_PLAY ? "play-only" : "rec-only");
+                       log_puts(" mode\n");
+               }
+       }
+
+       ctlhdl = fdpass_sioctl_open(d->num, n->idx, SIOCTL_READ | SIOCTL_WRITE);
+       if (ctlhdl == NULL) {
+               if (log_level >= 1) {
+                       dev_log(d);
+                       log_puts(": no control device\n");
+               }
+       }
+
+       *rhdl = hdl;
+       *rctlhdl = ctlhdl;
+       *rmode = mode;
+       return 1;
+}
+
 /*
  * open the device using one of the provided paths
  */
-static struct sio_hdl *
-dev_sio_openlist(struct dev *d, unsigned int mode, struct sioctl_hdl **rctlhdl)
+static int
+dev_sio_openlist(struct dev *d,
+    struct sio_hdl **rhdl, struct sioctl_hdl **rctlhdl, unsigned int *rmode)
 {
        struct dev_alt *n;
-       struct sio_hdl *hdl;
-       struct sioctl_hdl *ctlhdl;
        struct ctl *c;
        int val;
 
        for (n = d->alt_list; n != NULL; n = n->next) {
                if (d->alt_num == n->idx)
                        continue;
-               hdl = fdpass_sio_open(d->num, n->idx, mode);
-               if (hdl != NULL) {
+               if (log_level >= 2) {
+                       dev_log(d);
+                       log_puts(": trying ");
+                       log_puts(n->name);
+                       log_puts("\n");
+               }
+               if (dev_sio_openalt(d, n, rhdl, rctlhdl, rmode)) {
                        if (log_level >= 2) {
                                dev_log(d);
                                log_puts(": using ");
                                log_puts(n->name);
                                log_puts("\n");
                        }
-                       ctlhdl = fdpass_sioctl_open(d->num, n->idx,
-                           SIOCTL_READ | SIOCTL_WRITE);
-                       if (ctlhdl == NULL) {
-                               if (log_level >= 1) {
-                                       dev_log(d);
-                                       log_puts(": no control device\n");
-                               }
-                       }
                        d->alt_num = n->idx;
                        for (c = d->ctl_list; c != NULL; c = c->next) {
                                if (c->addr < CTLADDR_ALT_SEL ||
@@ -130,11 +169,10 @@ dev_sio_openlist(struct dev *d, unsigned int mode, struct sioctl_hdl **rctlhdl)
                                if (val)
                                        c->val_mask = ~0U;
                        }
-                       *rctlhdl = ctlhdl;
-                       return hdl;
+                       return 1;
                }
        }
-       return NULL;
+       return 0;
 }
 
 /*
@@ -144,38 +182,19 @@ int
 dev_sio_open(struct dev *d)
 {
        struct sio_par par;
-       unsigned int mode = d->mode & (MODE_PLAY | MODE_REC);
 
-       d->sio.hdl = dev_sio_openlist(d, mode, &d->sioctl.hdl);
-       if (d->sio.hdl == NULL) {
-               if (mode != (SIO_PLAY | SIO_REC))
-                       return 0;
-               d->sio.hdl = dev_sio_openlist(d, SIO_PLAY, &d->sioctl.hdl);
-               if (d->sio.hdl != NULL)
-                       mode = SIO_PLAY;
-               else {
-                       d->sio.hdl = dev_sio_openlist(d,
-                           SIO_REC, &d->sioctl.hdl);
-                       if (d->sio.hdl != NULL)
-                               mode = SIO_REC;
-                       else
-                               return 0;
-               }
-               if (log_level >= 1) {
-                       log_puts("warning, device opened in ");
-                       log_puts(mode == SIO_PLAY ? "play-only" : "rec-only");
-                       log_puts(" mode\n");
-               }
-       }
+       if (!dev_sio_openlist(d, &d->sio.hdl, &d->sioctl.hdl, &d->mode))
+               return 0;
+
        sio_initpar(&par);
        par.bits = d->par.bits;
        par.bps = d->par.bps;
        par.sig = d->par.sig;
        par.le = d->par.le;
        par.msb = d->par.msb;
-       if (mode & SIO_PLAY)
+       if (d->mode & SIO_PLAY)
                par.pchan = d->pchan;
-       if (mode & SIO_REC)
+       if (d->mode & SIO_REC)
                par.rchan = d->rchan;
        if (d->bufsz)
                par.appbufsz = d->bufsz;
@@ -211,14 +230,14 @@ dev_sio_open(struct dev *d)
                log_puts(": unsupported sample size\n");
                goto bad_close;
        }
-       if ((mode & SIO_PLAY) && par.pchan > NCHAN_MAX) {
+       if ((d->mode & SIO_PLAY) && par.pchan > NCHAN_MAX) {
                dev_log(d);
                log_puts(": ");
                log_putu(par.pchan);
                log_puts(": unsupported number of play channels\n");
                goto bad_close;
        }
-       if ((mode & SIO_REC) && par.rchan > NCHAN_MAX) {
+       if ((d->mode & SIO_REC) && par.rchan > NCHAN_MAX) {
                dev_log(d);
                log_puts(": ");
                log_putu(par.rchan);
@@ -254,17 +273,15 @@ dev_sio_open(struct dev *d)
        d->par.sig = par.sig;
        d->par.le = par.le;
        d->par.msb = par.msb;
-       if (mode & SIO_PLAY)
+       if (d->mode & SIO_PLAY)
                d->pchan = par.pchan;
-       if (mode & SIO_REC)
+       if (d->mode & SIO_REC)
                d->rchan = par.rchan;
        d->bufsz = par.bufsz;
        d->round = par.round;
        d->rate = par.rate;
-       if (!(mode & MODE_PLAY))
-               d->mode &= ~(MODE_PLAY | MODE_MON);
-       if (!(mode & MODE_REC))
-               d->mode &= ~MODE_REC;
+       if (d->mode & MODE_PLAY)
+               d->mode |= MODE_MON;
        sio_onmove(d->sio.hdl, dev_sio_onmove, d);
        d->sio.file = file_new(&dev_sio_ops, d, "dev", sio_nfds(d->sio.hdl));
        if (d->sioctl.hdl) {
@@ -291,18 +308,13 @@ dev_sio_open(struct dev *d)
 int
 dev_sio_reopen(struct dev *d)
 {
-       struct sioctl_hdl *ctlhdl;
        struct sio_par par;
        struct sio_hdl *hdl;
+       struct sioctl_hdl *ctlhdl;
+       unsigned int mode;
 
-       hdl = dev_sio_openlist(d, d->mode & (MODE_PLAY | MODE_REC), &ctlhdl);
-       if (hdl == NULL) {
-               if (log_level >= 1) {
-                       dev_log(d);
-                       log_puts(": couldn't open an alternate device\n");
-               }
+       if (!dev_sio_openlist(d, &hdl, &ctlhdl, &mode))
                return 0;
-       }
 
        sio_initpar(&par);
        par.bits = d->par.bits;
@@ -310,10 +322,10 @@ dev_sio_reopen(struct dev *d)
        par.sig = d->par.sig;
        par.le = d->par.le;
        par.msb = d->par.msb;
-       if (d->mode & SIO_PLAY)
-               par.pchan = d->pchan;
-       if (d->mode & SIO_REC)
-               par.rchan = d->rchan;
+       if (mode & SIO_PLAY)
+               par.pchan = d->reqpchan;
+       if (mode & SIO_REC)
+               par.rchan = d->reqrchan;
        par.appbufsz = d->bufsz;
        par.round = d->round;
        par.rate = d->rate;
@@ -343,6 +355,7 @@ dev_sio_reopen(struct dev *d)
        }
 
        /* update parameters */
+       d->mode = mode;
        d->par.bits = par.bits;
        d->par.bps = par.bps;
        d->par.sig = par.sig;