Bug 246206 - dev/sound: SNDCTL_AUDIOINFO does not report PCM_CAP_VIRTUAL
Summary: dev/sound: SNDCTL_AUDIOINFO does not report PCM_CAP_VIRTUAL
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.1-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Hans Petter Selasky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-05 01:42 UTC by Kevin Zheng
Modified: 2020-05-18 09:08 UTC (History)
2 users (show)

See Also:


Attachments
Demonstration of issue (786 bytes, text/plain)
2020-05-05 01:42 UTC, Kevin Zheng
no flags Details
Proposed patch (1.05 KB, patch)
2020-05-05 04:02 UTC, Kevin Zheng
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Zheng 2020-05-05 01:42:48 UTC
Created attachment 214136 [details]
Demonstration of issue

In sys/soundcard.h, PCM_CAP_VIRTUAL is documented as "Virtual device":

#	define PCM_CAP_VIRTUAL		0x00040000	/* Virtual device */

When SNDCTL_AUDIOINFO is called on /dev/mixer, no virtual devices have have PCM_CAP_VIRTUAL set. This means, apart from being smart, consumers of the OSS4 API can't tell virtual devices from real devices. This leads to a confusing choice in, say, audio programs that allow selection of vchans.

A short example program that demonstrates the issue is attached. Example output on my computer is:

$ clang vpctest.c -o vpctest
$ ./vpctest
dev /dev/dsp0.p0        virtual 0
dev /dev/dsp0.vp0       virtual 0
dev /dev/dsp0.vp1       virtual 0
dev /dev/dsp0.r0        virtual 0
dev /dev/dsp0.vr0       virtual 0
dev /dev/dsp0.vr1       virtual 0

The vp* and vr* devices are virtual, but PCM_CAP_VIRTUAL is not set.

The root cause of this issue appears to be in dev/sound/pcm/dsp.c:

	ai->caps = PCM_CAP_REALTIME | PCM_CAP_MMAP | PCM_CAP_TRIGGER | CHANNEL_GETCAPS(ch)
		((ch->direction == PCMDIR_PLAY) ? PCM_CAP_OUTPUT : PCM_CAP_INPUT);

Most notably, PCM_CAP_VIRTUAL is never set. Further, the channel capabilities queried earlier:

caps = chn_getcaps(ch);

Do not appear to be used, either.

I suggest that virtual channels return PCM_CAP_VIRTUAL in vchan_getcaps() (dev/sound/pcm/vchan.c), and then channel capabilities get OR'ed into ai->caps.
Comment 1 Kevin Zheng 2020-05-05 04:02:28 UTC
Created attachment 214140 [details]
Proposed patch

Attached proposed patch. Proposed patch works on my machine.
Comment 2 Hans Petter Selasky freebsd_committer 2020-05-05 07:26:47 UTC
This probably also affects virtual_oss in ports. Can you make a patch for that application too?

--HPS
Comment 3 Ed Maste freebsd_committer 2020-05-07 17:47:36 UTC
Kevin, is it OK to add your sample code to a test application under tools/ with a standard 2-clause FreeBSD license?

HPS will you take care of shepherding the kernel patch in?
Comment 4 Hans Petter Selasky freebsd_committer 2020-05-07 17:52:23 UTC
I'll take care of the kernel patch.
Comment 5 commit-hook freebsd_committer 2020-05-07 18:15:44 UTC
A commit references this bug:

Author: hselasky
Date: Thu May  7 18:15:36 UTC 2020
New revision: 360790
URL: https://svnweb.freebsd.org/changeset/base/360790

Log:
  Set PCM_CAP_VIRTUAL for virtual DSP devices.

  Submitted by:	Kevin Zheng <kevinz5000@gmail.com>
  PR:		246206
  MFC after:	1 week
  Sponsored by:	Mellanox Technologies

Changes:
  head/sys/dev/sound/pcm/dsp.c
Comment 6 Hans Petter Selasky freebsd_committer 2020-05-07 18:16:09 UTC
Submitted with some modifications.
Comment 7 Hans Petter Selasky freebsd_committer 2020-05-07 18:17:32 UTC
Virtual OSS already sets: PCM_CAP_VIRTUAL

--HPS
Comment 8 Kevin Zheng 2020-05-07 19:53:54 UTC
Thanks all for the review and commit!

I, the author of this test code, hereby make it available under a standard 2-clause FreeBSD license.

Which test program is this? I'm okay with it as-is, but if it's going to land in base there are a couple more things that we could make do printing out :)
Comment 9 Ed Maste freebsd_committer 2020-05-07 21:08:25 UTC
(In reply to Kevin Zheng from comment #8)
> Which test program is this?

It doesn't yet exist, but I found myself writing a bunch of little things like this to explore both /dev/dsp* and /dev/video* and adding them to the tree under tools/ seems sensible.
Comment 10 commit-hook freebsd_committer 2020-05-18 09:07:47 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 09:07:15 UTC 2020
New revision: 361174
URL: https://svnweb.freebsd.org/changeset/base/361174

Log:
  MFC r360790:
  Set PCM_CAP_VIRTUAL for virtual DSP devices.

  Submitted by:	Kevin Zheng <kevinz5000@gmail.com>
  PR:		246206
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/12/
  stable/12/sys/dev/sound/pcm/dsp.c
Comment 11 commit-hook freebsd_committer 2020-05-18 09:08:49 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 09:07:51 UTC 2020
New revision: 361175
URL: https://svnweb.freebsd.org/changeset/base/361175

Log:
  MFC r360790:
  Set PCM_CAP_VIRTUAL for virtual DSP devices.

  Submitted by:	Kevin Zheng <kevinz5000@gmail.com>
  PR:		246206
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/11/
  stable/11/sys/dev/sound/pcm/dsp.c