Page MenuHomeFreeBSD

sound: Handle unavailable devices in various OSS IOCTLs
ClosedPublic

Authored by christos on May 19 2024, 9:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 2, 8:59 AM
Unknown Object (File)
Thu, May 30, 11:32 PM
Unknown Object (File)
Thu, May 30, 11:18 PM
Unknown Object (File)
Fri, May 24, 12:31 AM
Unknown Object (File)
Thu, May 23, 11:05 PM
Unknown Object (File)
Thu, May 23, 1:56 AM
Unknown Object (File)
Wed, May 22, 7:47 PM
Unknown Object (File)
Wed, May 22, 7:32 PM
Subscribers

Details

Summary

mixer(8)'s -a option is used to print information about all mixer
devices in the system. To do this, it loops from 0 to
mixer_get_nmixers(), and tries to open "/dev/mixer%d". However, this
approach doesn't work when there are disabled/unregistered mixers in the
system, or when an audio device simply doesn't have a mixer.

mixer_get_nmixers() calls SNDCTL_SYSINFO and returns
oss_sysinfo->nummixers, whose value is the number of currently _enabled_
mixers only. Taking the bug report mentioned below (277615) as an
example, suppose a system with 8 mixer devices total, but 3 of them are
either disabled or non-existent, which means they will not show up under
/dev, meaning we have 5 enabled mixer devices, which is also what the
value of oss_sysinfo->nummixers will be. What mixer(8) will do is loop
from 0 to 5 (instead of 8), and start calling mixer_open() on
/dev/mixer0, up to /dev/mixer4, and as is expected, the first call will
fail right away, hence the error shown in the bug report.

To fix this, modify oss_sysinfo->nummixers to hold the value of the
maximum unit in the system, which, although not necessarily "correct",
is more intuitive for applications that will want to use this value to
loop through all mixer devices.

Additionally, notify applications that a device is
unavailable/unregistered instead of skipping it. The current
implementations of SNDCTL_AUDIOINFO, SNDCTL_MIXERINFO and
SNDCTL_CARDINFO break applications that expect to get information about
a device that is skipped. Related discussion can be found here:
https://reviews.freebsd.org/D45135#1029526

It has to be noted, that other applications, apart from mixer(8), suffer
from this.

PR: 277615
Sponsored by: The FreeBSD Foundation
MFC after: 3 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

@dev_submerge.ch I am not really sure if we can implement this in SNDCTL_ENGINEINFO as well. In SNDCTL_ENGINEINFO we choose an "engine"/channel on a device that is already opened, so in the case of an unavailable device, we won't be able to open it in the first place.

sys/dev/sound/pcm/dsp.c
2046
2066

All of these assignments of 0 are unneeded.

sys/dev/sound/pcm/mixer.c
1399
1407

Same comment about zero fields.

christos marked 4 inline comments as done.

Address Mark's comments.

@dev_submerge.ch I am not really sure if we can implement this in SNDCTL_ENGINEINFO as well. In SNDCTL_ENGINEINFO we choose an "engine"/channel on a device that is already opened, so in the case of an unavailable device, we won't be able to open it in the first place.

There's no 1:1 mapping from engines (virtual channels) to pcm devices anyway. I think the only thing we care about with SNDCTL_ENGINEINFO is that applications can iterate them using numaudioengines as expected.

@dev_submerge.ch I am not really sure if we can implement this in SNDCTL_ENGINEINFO as well. In SNDCTL_ENGINEINFO we choose an "engine"/channel on a device that is already opened, so in the case of an unavailable device, we won't be able to open it in the first place.

There's no 1:1 mapping from engines (virtual channels) to pcm devices anyway. I think the only thing we care about with SNDCTL_ENGINEINFO is that applications can iterate them using numaudioengines as expected.

D45164 fixes the numaudioengines and numaudios values so this patch should be ready.

sys/dev/sound/pcm/sound.c
774

This change implicitly affects mixer_get_nmixers() in the mixer(3) library. Don't forget to adjust the documentation there. The described return value in the mixer(3) man page is wrong, BTW.

christos marked an inline comment as done.
christos edited the summary of this revision. (Show Details)

Update mixer_get_nmixers() entry in mixer.3.

In my tests this worked as expected with unavailable devices, both in mixer(8) and audio/oss ossinfo.

This revision is now accepted and ready to land.Wed, May 22, 12:53 PM