From: Stuart Henderson <stu@spacehopper.org>
Subject: Re: sndiod: per-program (instead of per-client) level control
To: Omar Polo <op@omarpolo.com>
Cc: Alexandre Ratchov <alex@caoua.org>, tech@openbsd.org
Date: Tue, 17 Jun 2025 13:40:01 +0100

On 2025/06/17 14:13, Omar Polo wrote:
> Hello,
> 
> Alexandre Ratchov <alex@caoua.org> wrote:
> > This diff is to adjust the volume of all instances of the same program
> > with a single control. Example, instead of:
> > 
> > firefox0.level
> > firefox1.level
> > firefox2.level
> > ...
> > 
> > there will be single "firefox.level" control. There will be no way to
> > control individual tracks anymore, but that will not be a big loss
> > beacause there's currently no way to know in advance which sound
> > corresponds to which firefox instance.
> > 
> > There's a nice side effect:
> > 
> > Currently if a web browser has too many tabs/windows, it may consume
> > all the controls, preventing other programs from using sndiod. Now,
> > the number of connections that a single program can make can be
> > increased (the diff cranks it to 32) which fixes the problem in most
> > cases.
> > 
> > OK?
> 
> can't provide a valuable feedback on the diff, beside that I'm testing
> it and it makes a lot of sense to me, and that it works as advertized.
> 
> allow me tho to point out two nits:
> 
> > Index: dev.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
> > diff -u -p -r1.122 dev.c
> > --- dev.c	16 Jun 2025 06:19:29 -0000	1.122
> > +++ dev.c	16 Jun 2025 18:17:57 -0000
> > [...]
> > @@ -1523,98 +1505,34 @@ struct slot *
> >  slot_new(struct opt *opt, unsigned int id, char *who,
> >      struct slotops *ops, void *arg, int mode)
> >  {
> > [...]
> > +	i = 0;
> > +	s = slot_array;
> > +	while (1) {
> > +		if (i == DEV_NSLOT) {
> > +			logx(1, "%s: too many connections", a->name);
> > +			return NULL;
> >  		}
> > +		if (s->ops == NULL)
> > +			break;
> > +		i++;
> > +		s++;
> >  	}
> 
> I feel like a for loop would be slightly more readable here
> 
> 	s = slot_array;
> 	for (i = 0; i < DEV_NSLOT; i++, s++)
> 		if (s->ops == NULL)
> 			break;
> 	if (i == DEV_NSLOT) {
> 		logx(...)
> 		return NULL;
> 	}
> 
> > [...]
> > Index: sndiod.8
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/sndiod/sndiod.8,v
> > diff -u -p -r1.17 sndiod.8
> > --- sndiod.8	3 May 2024 16:47:15 -0000	1.17
> > +++ sndiod.8	16 Jun 2025 18:17:58 -0000
> > @@ -440,12 +440,13 @@ exposes the audio device clock
> >  and allows audio device properties to be controlled
> >  through MIDI.
> >  .Pp
> > -A MIDI channel is assigned to each stream, and the volume
> > +A MIDI channel is assigned to each program, and the volume
> >  is changed using the standard volume controller (number 7).
> >  Similarly, when the audio client changes its volume,
> >  the same MIDI controller message is sent out; it can be used
> >  for instance for monitoring or as feedback for motorized
> >  faders.
> > +If there are multiple instances of the same program they will share the same setting.
> 
> This line should be folded, (my xterm and) man -Tlint complain it's
> longer than 80 column :p

"Multiple instances of the same program will share the same setting"
is more concise and <80.