sound: Make verbose sndstat output more readable
AbandonedPublic
Actions

Authored by christos on Mar 28 2024, 11:06 AM.

Details

Reviewers

markj
emaste
dev_submerge.ch

Summary

The current verbose output of /dev/sndstat is rather cryptic and
convoluted. Make it more human-readable and easy-to-parse.

Print each property on each own line.
List each property in "name=value" format.
Print channel format as a string instead of a hex number.
Do not print channel flags twice.
Introduce FEEDER_F_BITS (similar to CHN_F_BITS and SD_F_BITS) and use it to print the feeder flags as strings as well as hex values.
Slightly modify the feeder chain line.
Make buffer layout printing more readable.
Rename "spd" to "rate".
Rename "fmt" to "format".
For VCHANs, print parent channel on a separate line.
Make "(<n>p:<n>v/<n>r:<n>v)" more verbose and move it out of the header line.
Print "default" in header as "(default)", similar to mixer(8).
Always print "(play/rec)" string.
Remove redundant "simplex" string from header, as it gets printed in "snddev flags" as well.

Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks

Test Plan

Before:

FreeBSD Audio Driver (64bit 2009061500/amd64)
Installed devices:
pcm0: <Focusrite Scarlett Solo USB> on uaudio0 (1p:1v/1r:1v) default
        snddev flags=0x2e6<AUTOVCHAN,SOFTPCMVOL,BUSY,MPSAFE,REGISTERED,VPC>
        [pcm0:play:dsp0.p0]: spd 48000, fmt 0x00200010/0x00201000, flags 0x00002100, 0x00000006
        interrupts 1781, underruns 0, feed 1780, ready 0 [b:3072/1536/2|bs:2048/1024/2]
        channel flags=0x2100<BUSY,HAS_VCHAN>
        {userland} -> feeder_mixer(0x00200010) -> feeder_format(0x00200010 -> 0x00201000) -> {hardware}
        pcm0:play:dsp0.p0[pcm0:virtual:dsp0.vp0]: spd 48000, fmt 0x00201000/0x00200010, flags 0x10000000, 0x00000023
        interrupts 0, underruns 0, feed 0, ready 0 [b:0/0/0|bs:16384/2048/8]
        channel flags=0x10000000<VIRTUAL>
        {userland} -> feeder_root(0x00201000) -> feeder_format(0x00201000 -> 0x00200010) -> feeder_volume(0x00200010) -> {hardware}
        [pcm0:record:dsp0.r0]: spd 48000, fmt 0x00200010/0x00201000, flags 0x00002100, 0x00000007
        interrupts 0, overruns 0, feed 0, hfree 3072, sfree 2048 [b:3072/1536/2|bs:2048/1024/2]
        channel flags=0x2100<BUSY,HAS_VCHAN>
        {hardware} -> feeder_root(0x00201000) -> feeder_format(0x00201000 -> 0x00200010) -> feeder_mixer(0x00200010) -> {userland}
        pcm0:record:dsp0.r0[pcm0:virtual:dsp0.vr0]: spd 8000, fmt 0x00100008, flags 0x10000000, 0x00000000
        interrupts 0, overruns 0, feed 0, hfree 0, sfree 0 [b:0/0/0|bs:0/0/0]
        channel flags=0x10000000<VIRTUAL>
        {hardware} -> feeder_root(0x00000000) -> {userland}
No devices installed from userspace.

After:

FreeBSD Audio Driver (64bit 2009061500/amd64)
Installed devices:
pcm0: <Focusrite Scarlett Solo USB> on uaudio0 (play/rec) (default)
        snddev flags=0x2e6<AUTOVCHAN,SOFTPCMVOL,BUSY,MPSAFE,REGISTERED,VPC>
        playback channels=[hardware=1, virtual=1]
        recording channels=[hardware=1, virtual=1]
        pcm0:play:dsp0.p0
                channel flags=0x2108<TRIGGERED,BUSY,HAS_VCHAN>
                rate=48000
                format=s16le:2.0/s32le:2.0
                interrupts=167,
                underruns=0
                feed=166
                ready=0
                hardware buffer=[size=3072, blocksize=1536, blockcount=2]
                software buffer=[size=2048, blocksize=1024, blockcount=2]
                feeder flags=0x6<FORMAT,MIXER>
                feeder chain=[userland -> feeder_mixer(0x00200010) -> feeder_format(s16le:2.0 -> s32le:2.0) -> hardware]
        pcm0:virtual:dsp0.vp0
                parent channel=pcm0:play:dsp0.p0
                channel flags=0x1000010c<RUNNING,TRIGGERED,BUSY,VIRTUAL>
                rate=48000
                format=s32le:2.0/s16le:2.0
                pid=818 (mpv)
                interrupts=0,
                underruns=0
                feed=1494
                ready=14848
                hardware buffer=[size=0, blocksize=0, blockcount=0]
                software buffer=[size=16384, blocksize=2048, blockcount=8]
                feeder flags=0x23<ROOT,FORMAT,VOLUME>
                feeder chain=[userland -> feeder_root(0x00201000) -> feeder_format(s32le:2.0 -> s16le:2.0) -> feeder_volume(0x00200010) -> hardware]
        pcm0:record:dsp0.r0
                channel flags=0x2100<BUSY,HAS_VCHAN>
                rate=48000
                format=s16le:2.0/s32le:2.0
                interrupts=0,
                overruns=0
                feed=0
                hfree=3072
                sfree=2048
                hardware buffer=[size=3072, blocksize=1536, blockcount=2]
                software buffer=[size=2048, blocksize=1024, blockcount=2]
                feeder flags=0x7<ROOT,FORMAT,MIXER>
                feeder chain=[hardware -> feeder_root(0x00201000) -> feeder_format(s32le:2.0 -> s16le:2.0) -> feeder_mixer(0x00200010) -> userland]
        pcm0:virtual:dsp0.vr0
                parent channel=pcm0:record:dsp0.r0
                channel flags=0x10000000<VIRTUAL>
                rate=8000
                format=u8:1.0
                interrupts=0,
                overruns=0
                feed=0
                hfree=0
                sfree=0
                hardware buffer=[size=0, blocksize=0, blockcount=0]
                software buffer=[size=0, blocksize=0, blockcount=0]
                feeder flags=0x0
                feeder chain=[hardware -> feeder_root(0x00000000) -> userland]
No devices installed from userspace.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 56821
Build 53709: arc lint + arc unit

Event Timeline

christos created this revision.Mar 28 2024, 11:06 AM

Herald added a subscriber: imp. · View Herald TranscriptMar 28 2024, 11:06 AM

christos requested review of this revision.Mar 28 2024, 11:06 AM

Harbormaster completed remote builds in B56821: Diff 136302.Mar 28 2024, 11:06 AM

christos edited the test plan for this revision. (Show Details)Mar 28 2024, 11:07 AM

christos added a parent revision: D44545: sound: Get rid of pcm/sndstat.h and turn macros into regular code.

This is definitely more readable, but my main concern would be that this form of output is quite lengthy and not very grep-friendly. Imagine 8 pcm devices with 4 channels each (when idle). Ideally, we could easily grep for the whole output of a single device, or check which output is used by an application (cat /dev/sndstat | grep mpv). Would be handy for both personal use and bug reports from users. Or maybe we can integrate these selective queries into a user space tool?

Another useful addition would be buffer sizes in samples or milliseconds, makes it easier to grasp the latency induced.

Note that there is software that parses /dev/sndstat, e.g.
https://github.com/obsproject/obs-studio/blob/6f3f7b4e5a561a02bc0c7cd37260cd7814fba1a9/plugins/oss-audio/oss-input.c#L463

In D44546#1015789, @dev_submerge.ch wrote:

This is definitely more readable, but my main concern would be that this form of output is quite lengthy and not very grep-friendly. Imagine 8 pcm devices with 4 channels each (when idle). Ideally, we could easily grep for the whole output of a single device, or check which output is used by an application (cat /dev/sndstat | grep mpv). Would be handy for both personal use and bug reports from users. Or maybe we can integrate these selective queries into a user space tool?

I am planning to write a userland tool similar to mixer(8) and OpenBSD's audioctl(8) (https://man.openbsd.org/audioctl.8), which will have some overlapping functionality (in terms of output) with /dev/sndstat, but I think it's good to keep /dev/sndstat self-contained and be able to work with it using just cat(1). The format in this patch is grep-friendly in the sense that each property is a separate line and also the format is standardized, but it'd probably be tedious to do it for multiple devices, as you mentioned. Do you have any suggestion for a more concise design?

Another useful addition would be buffer sizes in samples or milliseconds, makes it easier to grasp the latency induced.

You mean something similar to hw.usb.uaudio.buffer_ms?

In D44546#1015790, @emaste wrote:

Note that there is software that parses /dev/sndstat, e.g.
https://github.com/obsproject/obs-studio/blob/6f3f7b4e5a561a02bc0c7cd37260cd7814fba1a9/plugins/oss-audio/oss-input.c#L463

So, the patch will break this, but I think OBS has to be patched anyway since /dev/sndstat info can now be fetched from the SNDSTIOC_GET_DEVS ioctl as an nvlist.
https://github.com/freebsd/freebsd-src/commit/c96151d33509655efb7fb26768cb56a041c176f1

So, the patch will break this, but I think OBS has to be patched anyway...

It's not just OBS, it was just the first match that I recognized. GitHub turns up 1.8k instances of "/dev/sndstat": https://github.com/search?q=%2Fdev%2Fsndstat&type=code
Of course some (lots?) of these won't apply (e.g. old unused Linux OSS functionality) but this will definitely take some work and coordination.

In D44546#1015790, @emaste wrote:

Note that there is software that parses /dev/sndstat, e.g.
https://github.com/obsproject/obs-studio/blob/6f3f7b4e5a561a02bc0c7cd37260cd7814fba1a9/plugins/oss-audio/oss-input.c#L463

Note that the patch modifies "verbose" output

In D44546#1015938, @emaste wrote:

So, the patch will break this, but I think OBS has to be patched anyway...

It's not just OBS, it was just the first match that I recognized. GitHub turns up 1.8k instances of "/dev/sndstat": https://github.com/search?q=%2Fdev%2Fsndstat&type=code
Of course some (lots?) of these won't apply (e.g. old unused Linux OSS functionality) but this will definitely take some work and coordination.

Indeed, we can't just declare that applications have to be patched. That's a lot of work and will cause frustration among users, and the benefits of the patch don't seem commensurate.

If your plan is to add a new userland utility which makes it easier to inspect and modify sound(4) configuration, then I'd suggest focusing on that rather than changing the legacy /dev/sndstat.

(OTOH, does setting hw.snd.verbose=1 break OBS today?)

In D44546#1015938, @emaste wrote:

So, the patch will break this, but I think OBS has to be patched anyway...

It's not just OBS, it was just the first match that I recognized. GitHub turns up 1.8k instances of "/dev/sndstat": https://github.com/search?q=%2Fdev%2Fsndstat&type=code
Of course some (lots?) of these won't apply (e.g. old unused Linux OSS functionality) but this will definitely take some work and coordination.

Good point, although these applications would only want to parse verbose=0. A search on github brings up some projects that explicitly check for hw.snd.verbose == 0, but none that rely on a higher verbosity level. I suppose if we keep the output at verbose=0 intact (first line of each pcm device), we should be fine.

In D44546#1015877, @christos wrote:

In D44546#1015789, @dev_submerge.ch wrote:

This is definitely more readable, but my main concern would be that this form of output is quite lengthy and not very grep-friendly. Imagine 8 pcm devices with 4 channels each (when idle). Ideally, we could easily grep for the whole output of a single device, or check which output is used by an application (cat /dev/sndstat | grep mpv). Would be handy for both personal use and bug reports from users. Or maybe we can integrate these selective queries into a user space tool?

I am planning to write a userland tool similar to mixer(8) and OpenBSD's audioctl(8) (https://man.openbsd.org/audioctl.8), which will have some overlapping functionality (in terms of output) with /dev/sndstat, but I think it's good to keep /dev/sndstat self-contained and be able to work with it using just cat(1). The format in this patch is grep-friendly in the sense that each property is a separate line and also the format is standardized, but it'd probably be tedious to do it for multiple devices, as you mentioned. Do you have any suggestion for a more concise design?

Ok, the general direction I would go for:

Make the (virtual) channel line show up in e.g. grep mpv
Group related properties into one line, like
- User side format and rate
- User side buffer size values
- Feeder related values (interrupts, X-runs...)
- Feeder chain
- Backend side format and rate
- Backend buffer size values

But this gets into bikeshed territory quickly. I'd rather just fix the pain points (format output) and establish the userland tool as an alternative ASAP.

Since we're at it, another pain point is the feeder chain of virtual channels ending in "hardware". That is misleading, it ends in the parent channel.

Another useful addition would be buffer sizes in samples or milliseconds, makes it easier to grasp the latency induced.

You mean something similar to hw.usb.uaudio.buffer_ms?

Yes, but as an approximate info to the user, instead of a setting. Interpretation is a bit difficult, as it depends on the read / write strategy. I think just the buffer sizes in samples (frames!) would be nice.

After thinking a bit more about it, and taking the comments into consideration, I think it's better to abandon this to avoid any unwanted side-effects and instead just focus on the userland tool.