From bd94a039ec3b27b9a9eea5cc11621af303bb0350 Mon Sep 17 00:00:00 2001 From: edd Date: Tue, 2 Mar 2021 12:15:46 +0000 Subject: [PATCH] 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! --- usr.bin/sndiod/dev.c | 82 +++++++++++------------ usr.bin/sndiod/siofile.c | 137 +++++++++++++++++++++------------------ 2 files changed, 116 insertions(+), 103 deletions(-) diff --git a/usr.bin/sndiod/dev.c b/usr.bin/sndiod/dev.c index 6e2e1a6fa6e..7dc8bac2a0b 100644 --- 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 * @@ -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 ddc6a5973d1..d620b1f36c1 100644 --- 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 * @@ -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; -- 2.20.1