On Fri, Jan 03, 2020 at 11:59:06PM -0500, Peter Piwowarski wrote:
> >Synopsis: sndiod no longer does channel duplication as of
> >src/usr.bin/sndiod/dev.c:1.61
> >Category: user
> >Environment:
> System : OpenBSD 6.6
> Details : OpenBSD 6.6-current (GENERIC.MP) #0: Thu Jan 2
> 22:06:44 EST 2020
> [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> sndiod(8) describes the -j option, which when set causes
> streams to have their channels duplicated or joined as needed
> to match the (sub)device's channel count; for example, a mono
> stream has the single channel duplicated in each channel of a
> stereo device. As of revision 1.61 of src/usr.bin/sndiod/dev.c
> this no longer appears to be working; no duplication seems to
> be done. This revision replaces the following logic
> (paraphrased, showing the expansion/duplication case only):
>
> dev_nch = s->opt->pmax - s->opt->pmin + 1;
> /* ~snip~ */
> if (dev_nch > s->mix.nch)
> s->mix.expand = dev_nch / s->mix.nch;
>
> with this:
>
> if (s->mix.cmap.nch > s->mix.nch)
> s->mix.expand = s->mix.cmap.nch / s->mix.nch;
>
> I believe the problem occurs because both s->mix.cmap.nch and
> s->mix.nch vary with the input stream's channel count.
>
Thank you for looking at this.
Indeed both of above formulas are wrong.
For the "expand count", the correct formula is the number of channels
on the device (what you call target_nch) divided by the number of
channels of the client (aka s->mix.nch), as in your diff.
Similarly the "join count" is the number of channels of the client
divided by the number of channels of the device.
The number of channels on the device is restricted by the
s->opt->{pmin,pmax} variables. It is the number of channels common to
the [0 : dev->pchan - 1] and [s->opt->pmin : s->opt->pmax] ranges.
Note that there may be no overlap if s->opt->pmin >= dev->pchan, there
are no audible channels in this case. For instance this may happen for
a program playing on channels 2:3 of a 4-channel device which is
dynamically replaced by a 2-channel device.
In this case s->mix.cmap.nch == 0.
> Index: dev.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 dev.c
> --- dev.c 21 Sep 2019 04:42:46 -0000 1.62
> +++ dev.c 4 Jan 2020 01:01:20 -0000
> @@ -1501,9 +1501,14 @@ dev_mmcloc(struct dev *d, unsigned int o
> void
> slot_initconv(struct slot *s)
> {
> + unsigned int target_nch;
> struct dev *d = s->dev;
>
> if (s->mode & MODE_PLAY) {
> + target_nch = d->pchan < (s->opt->pmax - s->opt->pmin + 1)
> + ? d->pchan
> + : (s->opt->pmax - s->opt->pmin + 1);
> +
AFAIU this works only if s->opt->pmin == 0. As s->opt->pmin >= 0, the
number of device channels is:
target_nch = (d->pchan < s->opt->pmax + 1) ?
d->pchan - s->opt->pmin : s->opt->pmax - s->opt->pmin + 1;
which is valid only if there are audible channels, i.e. if
s->mix.cmap.nch > 0, so this could be handled by changing below
condition to:
if (s->opt->dup && s->mix.cmap.nch > 0)
and moving the target_nch calculation inside the "true" conditional
code block.
Last point, do you mind renaming target_nch to dev_nch ?
> cmap_init(&s->mix.cmap,
> s->opt->pmin, s->opt->pmin + s->mix.nch - 1,
> s->opt->pmin, s->opt->pmin + s->mix.nch - 1,
> @@ -1519,14 +1524,18 @@ slot_initconv(struct slot *s)
> s->mix.join = 1;
> s->mix.expand = 1;
> if (s->opt->dup) {
> - if (s->mix.cmap.nch > s->mix.nch)
> - s->mix.expand = s->mix.cmap.nch / s->mix.nch;
> - else if (s->mix.cmap.nch > 0)
> - s->mix.join = s->mix.nch / s->mix.cmap.nch;
> + if (target_nch > s->mix.nch)
> + s->mix.expand = target_nch / s->mix.nch;
> + else if (s->mix.cmap.nch > target_nch)
> + s->mix.join = s->mix.nch / target_nch;
> }
> }
>
> if (s->mode & MODE_RECMASK) {
> + target_nch = d->rchan < (s->opt->rmax - s->opt->rmin + 1)
^^^^^^^^
d->pchan in the (s->mode & MODE_MON) case.
> + ? d->rchan
> + : (s->opt->pmax - s->opt->pmin + 1);
^^^^^^^^^^^^^^^^^^^^^^^^^^^
rmin and rmax
> +
> cmap_init(&s->sub.cmap,
> 0, ((s->mode & MODE_MON) ? d->pchan : d->rchan) - 1,
> s->opt->rmin, s->opt->rmax,
> @@ -1542,10 +1551,10 @@ slot_initconv(struct slot *s)
> s->sub.join = 1;
> s->sub.expand = 1;
> if (s->opt->dup) {
> - if (s->sub.cmap.nch > s->sub.nch)
> - s->sub.join = s->sub.cmap.nch / s->sub.nch;
> - else if (s->sub.cmap.nch > 0)
> - s->sub.expand = s->sub.nch / s->sub.cmap.nch;
> + if (target_nch > s->sub.nch)
> + s->sub.join = target_nch / s->sub.nch;
> + else if (s->sub.nch > target_nch)
> + s->sub.expand = s->sub.nch / target_nch;
> }
