Page MenuHomeFreeBSD

Implement sndstat nvlist-based enumeration ioctls.
ClosedPublic

Authored by khng on Oct 21 2020, 10:07 AM.
Tags
None
Referenced Files
F99457107: D26884.diff
Wed, Oct 9, 7:46 PM
Unknown Object (File)
Mon, Oct 7, 11:59 PM
Unknown Object (File)
Sat, Oct 5, 4:05 AM
Unknown Object (File)
Fri, Oct 4, 9:06 AM
Unknown Object (File)
Wed, Oct 2, 5:22 AM
Unknown Object (File)
Wed, Oct 2, 5:22 AM
Unknown Object (File)
Wed, Oct 2, 5:22 AM
Unknown Object (File)
Wed, Oct 2, 5:22 AM

Details

Summary

This is an API draft of the sndstat IOCTLs addition. These IOCTL commands are
drafted to provide an easier way for user space applications to enumerate
existing audio devices and the node they can potentially use.

The exchange of arguments between user space and kernel is based on nvlist.
A few IOCTLs are added to dev sndstat node:

  • SNDSTAT_REFRESH_DEVS
  • SNDSTAT_GET_DEVS
  • SNDSTAT_ADD_USER_DEVS
  • SNDSTAT_FLUSH_USER_DEVS

SNDSTAT_REFRESH_DEVS

This IOCTL command takes NO arguments. If there exists a snapshot of device
list, the snapshot will be freed.

SNDSTAT_GET_DEVS

This IOCTL command takes struct sndstat_nvlbuf_arg. Callers need to
provide a sufficiently large buffer to hold a packed nvlist stream. If there is no
existing device list snapshot available in the corresponding sndstat_file of the
opened sndstat FD, a new snapshot will be automatically fetched. Callers could
call this IOCTL command with arg.nbytes = 0. If it is the case, the buffer size
required to hold a packed nvlist stream of current snapshot will be returned.
Otherwise, callers are required to point arg.nbytes to the size of buffer
provided. If the buffer is not sufficiently large, the command returns with
arg.nbytes set to 0. If the buffer provided is sufficiently large, arg.nbytes will be
the size of packed nvlist stream written to the provided buffer.
Once a snapshot is returned to user-space successfully, the snapshot will be freed.
The schema of the packed nvlist stream is as follows:

dsps (NVLIST ARRAY): 3
    from_user (BOOL): FALSE
    nameunit (STRING): [pcm0]
    devnode (STRING): [dsp0]
    desc (STRING): [Generic (0x8086) (Analog Line-out)]
    pchan (NUMBER): 1 (1) (0x1)
    rchan (NUMBER): 0 (0) (0x0)
    pminrate (NUMBER): 48000 (48000) (0xbb80)
    pmaxrate (NUMBER): 48000 (48000) (0xbb80)
    pfmts (NUMBER): 2097168 (2097168) (0x200010)
    provider_info (NVLIST):
        unit (NUMBER): 0 (0) (0x0)
        bitperfect (BOOL): FALSE
        pvchan (NUMBER): 1 (1) (0x1)
        rvchan (NUMBER): 0 (0) (0x0)
    provider (STRING): [sound(4)]
    ,
    from_user (BOOL): TRUE
    pchan (NUMBER): 8 (8) (0x8)
    rchan (NUMBER): 8 (8) (0x8)
    nameunit (STRING): [vdsp0]
    devnode (STRING): [vdsp0]
    desc (STRING): [Virtual OSS]
    pminrate (NUMBER): 48000 (48000) (0xbb80)
    pmaxrate (NUMBER): 48000 (48000) (0xbb80)
    pfmts (NUMBER): 16 (16) (0x10)
    rminrate (NUMBER): 48000 (48000) (0xbb80)
    rmaxrate (NUMBER): 48000 (48000) (0xbb80)
    rfmts (NUMBER): 16 (16) (0x10)
    provider (STRING): []
    ,
    from_user (BOOL): TRUE
    pchan (NUMBER): 8 (8) (0x8)
    rchan (NUMBER): 8 (8) (0x8)
    nameunit (STRING): [lvdsp0]
    devnode (STRING): [lvdsp0]
    desc (STRING): [Virtual OSS]
    pminrate (NUMBER): 48000 (48000) (0xbb80)
    pmaxrate (NUMBER): 48000 (48000) (0xbb80)
    pfmts (NUMBER): 16 (16) (0x10)
    rminrate (NUMBER): 48000 (48000) (0xbb80)
    rmaxrate (NUMBER): 48000 (48000) (0xbb80)
    rfmts (NUMBER): 16 (16) (0x10)
    provider (STRING): []
    ,
SNDSTAT_ADD_USER_DEVS

This IOCTL command takes struct sndstat_nvlbuf_arg. Callers need to provide a
buffer holding a packed nvlist stream. Userspace-provided DSP nodes should be
listed inside the packed nvlist stream. An example of minimally-defined packed
nvlist stream is as follows:

dsps (NVLIST ARRAY): 2
    devnode (STRING): [vdsp0]
    desc (STRING): [Virtual device from userspace]
    pchan (NUMBER): 1 (1) (0x1)
    rchan (NUMBER): 0 (0) (0x0)
    ,
    devnode (STRING): [vdsp1]
    desc (STRING): [Virtual device from userspace 2]
    pchan (NUMBER): 1 (1) (0x1)
    rchan (NUMBER): 1 (1) (0x1)
    ,
SNDSTAT_FLUSH_USER_DEVS

This IOCTL command takes NO arguments. It flushes all the user space devices
added to the sndstat_file of the FD so far.


sound(4)-devices-specific labels in dsps:

  • unit
  • pvchan
  • rvchan
  • bitperfect

These labels will not be available if pchan is 0:

  • pminrate
  • pmaxrate
  • pfmts

These labels will not be available if rchan is 0:

  • rminrate
  • rmaxrate
  • rfmts

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 34433
Build 31537: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
  • Add nameunit label to device items

What about providing the default sample bit depth used?

What about providing the default sample bit depth used?

Seems okay. A question pops up in my head is how could we possibly deal with sound(4) devices with vchan disabled?

I think the bit-perfect parameters should be used, if possible.

  • Remove spurious pf->lock unlock and replace with SNDSTAT_UNLOCK()
  • Remove spurious pf->lock unlock and replace with SNDSTAT_UNLOCK() 2
  • Remove spurious SNDSTAT_UNLOCK() in sndstat_create_devs_nvlist()
  • Add pminrate/pmaxrate and rminrate/rmaxrate labels
  • Add pfmts and rfmts labels
  • Add bitperfect label for sound(4) dsps
  • Fix nvlist_add_boolean() typo.
  • Remove bitperfect field from ud::userdev
  • Use CHN_F_VIRTUAL to get rid of virtual channels
  • Supported format fixes for user devices

Do you think it makes the API easier to use to allow 0 for fields
which aren't useful in certain combinations? Such as exposing
rmaxrate=0 if rchan=0 (no recording direction available)?

EDIT: Will not consider this currently.

  • Invalid minrate is now initialized to be 0
  • Minor code cleanups
  • Remove /dev/ parsing from user-created CUSE node
  • SNDSTAT_REFRESH_DEVS will not fetch a new snapshot of available devices from now on

Rebased to the most recent -CURRENT

  • Add sndstat ioctls draft
  • Remove a spurious line from sys/sndstat.h
  • Remove ver_major and ver_minor from sndstat nvlist
  • Add license preamble for sys/sys/sndstat.h
  • Minor fix for the license preamble for sys/sys/sndstat.h
  • Add SNDSTAT_FLUSH_USER_DEVS IOCTL command to sndstat
  • Add FWRITE checking for SNDSTAT_FLUSH_USER_DEVS IOCTL
  • Address delphij's comments
  • Add nameunit label to device items
  • Remove spurious pf->lock unlock and replace with SNDSTAT_UNLOCK()
  • Remove spurious pf->lock unlock and replace with SNDSTAT_UNLOCK() 2
  • Remove spurious SNDSTAT_UNLOCK() in sndstat_create_devs_nvlist()
  • Cosmetic fixes
  • Add pminrate/pmaxrate and rminrate/rmaxrate labels
  • Add pfmts and rfmts labels
  • Add bitperfect label for sound(4) dsps
  • Fix nvlist_add_boolean() typo.
  • Remove bitperfect field from ud::userdev
  • Use CHN_F_VIRTUAL to get rid of virtual channels
  • Supported format fixes for user devices
  • - Invalid minrate is now initialized to be 0
  • Remove /dev/ parsing from user-created CUSE node
  • SNDSTAT_REFRESH_DEVS will not fetch a new snapshot of available devices from now on
  • Add "provider" member to indicate the implementations
  • Move non-common members into "provider_info" member
  • Numerous fixes on userdevs handling

I'll have a closer look tomorrow. Thank you for working on this!

  • Remove unused version preprocessor macros
khng edited the test plan for this revision. (Show Details)
  • Add an example to sndstat.4
  • Some changes to the authorship of sndstat.4
  • Add copyright line to sys/dev/sound/pcm/sndstat.c
  • Fix mandoc -T lint warnings
  • Add nv(9) to SEE ALSO section of sndstat.4
  • Make SNDSTAT_ADD_USER_DEVS's schema requirement stricter
khng retitled this revision from Add sndstat IOCTLs draft to Add sndstat nvlist-based enumeration IOCTLs.Mar 1 2021, 5:13 PM

Rebased to match git-arc workflow

sys/dev/sound/pcm/sndstat.c
176

Because you remove all elements here you can simply do:

while ((ud = TAILQ_FIRST(&pf->userdev_list)) != NULL) {

}

489

space after TAILQ_FOREACH

khng marked 2 inline comments as done.

Style and code cleanups suggested by hps@

This revision is now accepted and ready to land.Mar 2 2021, 9:32 AM

Looks good to me.

Do you think adding a device id field that is generated from name:pnpinfo:location along the device path is a good idea to get some stable usable identifiers?

In D26884#649492, @khng300_gmail.com wrote:

Looks good to me.

Do you think adding a device id field that is generated from name:pnpinfo:location along the device path is a good idea to get some stable usable identifiers?

EDIT: I would consider this as next part of work, since the addition of an device identifier field wouldn't break the ABI in userspace.

Looks good to me.

Approved by: philip (mentor)

khng retitled this revision from Add sndstat nvlist-based enumeration IOCTLs to Implement sndstat nvlist-based enumeration ioctls..Mar 17 2021, 11:01 AM