> Date: Sat, 5 Feb 2022 09:29:42 +0100
> From: Anton Lindqvist <[email protected]>
>
> Hi,
> I recently got a USB headset with physical volume buttons, handled by
> ucc(4). However, after enabling the device in sndiod the volume buttons
> does not cause the volume to change. Turns out wskbd_set_mixervolume()
> is only propagating volume changes to first attached audio device. Is
> their any good not to consider all attached audio devices?
I think this is tricky. The mixer values of different audio devices
may start out differently and may have different granularity and
probably operate on a different scale. This may lead to situations
where as you turn the volume up and down, the relative output volume
between devices changes considerably. I also think that your
implementation will unmute all audio devices as soon as you touch the
volume control buttons, which is probably not desirable.
Thinking about other ways to do this, we could:
- Add a knob that allows the user to control which audio device is
controlled by the volume control buttons. The choice could include
"none" and "all" as well as the individual devices.
- Add infrastructure to bind specific keyboards to specific audio
devices, a bit like how we support binding specific wskbd devices to
specific wsdisplay devices.
The first suggestion is probably relatively easy to achieve. The
implementation of the latter would defenitely need more thought and
discussion.
The "none" choice above would (partly) solve another issue where
userland applications see the key presses and act upon them even
though the kernel already did the volume adjustment.
Cheers,
Mark
> The diff below gives me the desired behavior by propagating volume
> changes to all attached audio devices.
>
> diff --git sys/dev/audio.c sys/dev/audio.c
> index ec52ee6ef01..ca19557d39e 100644
> --- sys/dev/audio.c
> +++ sys/dev/audio.c
> @@ -2452,10 +2452,6 @@ wskbd_mixer_init(struct audio_softc *sc)
> };
> int i;
>
> - if (sc->dev.dv_unit != 0) {
> - DPRINTF("%s: not configuring wskbd keys\n", DEVNAME(sc));
> - return;
> - }
> for (i = 0; i < sizeof(spkr_names) / sizeof(spkr_names[0]); i++) {
> if (wskbd_initvol(sc, &sc->spkr,
> spkr_names[i].cn, spkr_names[i].dn))
> @@ -2569,19 +2565,26 @@ wskbd_set_mixermute(long mute, long out)
> int
> wskbd_set_mixervolume(long dir, long out)
> {
> - struct audio_softc *sc;
> - struct wskbd_vol *vol;
> + int error = ENODEV;
> + int minor;
>
> - sc = (struct audio_softc *)device_lookup(&audio_cd, 0);
> - if (sc == NULL)
> - return ENODEV;
> - vol = out ? &sc->spkr : &sc->mic;
> - if (dir == 0)
> - vol->mute_pending ^= WSKBD_MUTE_TOGGLE;
> - else
> - vol->val_pending += dir;
> - if (!task_add(systq, &sc->wskbd_task))
> - device_unref(&sc->dev);
> - return 0;
> + for (minor = 0; minor < audio_cd.cd_ndevs; minor++) {
> + struct audio_softc *sc;
> + struct wskbd_vol *vol;
> +
> + sc = (struct audio_softc *)device_lookup(&audio_cd, minor);
> + if (sc == NULL)
> + continue;
> + vol = out ? &sc->spkr : &sc->mic;
> + if (dir == 0)
> + vol->mute_pending ^= WSKBD_MUTE_TOGGLE;
> + else
> + vol->val_pending += dir;
> + if (!task_add(systq, &sc->wskbd_task))
> + device_unref(&sc->dev);
> + error = 0;
> + }
> +
> + return error;
> }
> #endif /* NWSKBD > 0 */
>
>
