Re: human-readable audio device descriptions

2021-07-07 Thread Alexandre Ratchov
On Wed, Jul 07, 2021 at 03:13:37PM +0100, Edd Barrett wrote:
> Hi,
> 
> This diff is an attempt to make identifying audio devices easier by
> giving each device a human-readable description that can be read from
> userspace (and without scraping dmesg).
> 
> This is implemented on a per-audio-device basis using a new `descr`
> interface to `audio_hw_if`. For now I've only implemented this on:
> 
>  - azalia(4), which uses the codec info, e.g. `Realtek ALC1150`
> 
>  - uaudio(4), which uses the USB vendor and model info, e.g.
>`Logitech HD Pro Webcam C920`.
> 
> For drivers, which currently don't implement this interface, the
> description string defaults to the audio device node name, e.g.
> `uaudio2`.
> 
> The string is exposed to userspace via the AUDIO_GETDEV ioctl(2). I've
> reclaimed the space currently occupied by the (unused) `version` and
> `config` fields in `audio_device_t` and used it for a new `descr` field.
> Thus the size of `audio_device_t` remains the same for now.
> 
> I've also made audioctl(8) display the description string, e.g.:
> ```
> # audioctl   
> name=azalia1
> descr=Realtek ALC1150
> ...
> # audioctl -f /dev/audioctl1 
> name=uaudio0
> descr=SteelSeries SteelSeries Arctis 
> ...
> ```
> 
> I'm hoping that later we can have sndiod read these strings and expose
> them to its clients. For example, I'd like for sndioctl(1) to allow
> choosing a device based on the device description string (instead of the
> sndio device index), and for xfce4-mixer to allow choosing a device via
> its description in the GUI using a drop-down box.
> 
> Diff follows.
> 

[...]

> @@ -4019,6 +4021,37 @@ azalia_set_nblks(void *v, int mode,
>   nblks = HDA_BDL_MAX;
>  
>   return nblks;
> +}
> +
> +void
> +azalia_descr(void *arg, char *descr, size_t len)
> +{
> + azalia_t *az = arg;
> + codec_t *codec;
> + const char *vendor;
> + char buf[MAX_AUDIO_DESCR_LEN];
> + int i;
> +
> + *descr = '\0';
> + for (i = 0; i < az->ncodecs; i++) {

azalia(4) fetches all codecs, but uses (and supports) only one of
them. You should copy the name of the one in use, ie
az->codecs[az->codecno].

> + codec = >codecs[i];
> + /* adapted from azalia_print_codec() */
> + if (codec->name == NULL) {
> + vendor = pci_findvendor(codec->vid >> 16);
> + if (vendor == NULL)
> + snprintf(buf, MAX_AUDIO_DESCR_LEN,
> + "0x%04x/0x%04x", codec->vid >> 16,
> + codec->vid & 0x);
> + else
> + snprintf(buf, MAX_AUDIO_DESCR_LEN, "%s/0x%04x",
> + vendor, codec->vid & 0x);
> + } else
> + snprintf(buf, MAX_AUDIO_DESCR_LEN, "%s", codec->name);
> +
> + if (i > 0)
> + strlcat(descr, ", ", len);
> + strlcat(descr, buf, len);
> + }
>  }
>  
>  int
> Index: sys/sys/audioio.h
> ===
> RCS file: /cvs/src/sys/sys/audioio.h,v
> retrieving revision 1.27
> diff -u -p -r1.27 audioio.h
> --- sys/sys/audioio.h 14 Sep 2016 06:12:20 -  1.27
> +++ sys/sys/audioio.h 7 Jul 2021 10:23:09 -
> @@ -76,10 +76,10 @@ struct audio_status {
>   * audio devices.
>   */
>  #define MAX_AUDIO_DEV_LEN16
> +#define MAX_AUDIO_DESCR_LEN  32
>  typedef struct audio_device {
>   char name[MAX_AUDIO_DEV_LEN];
> - char version[MAX_AUDIO_DEV_LEN];
> - char config[MAX_AUDIO_DEV_LEN];
> + char descr[MAX_AUDIO_DESCR_LEN]; /* device description string */
>  } audio_device_t;
>

The intent of the "name" field was to be human-readable as well. You
could just replace it. There's no benefit in having two fields for the
same thing.



Re: human-readable audio device descriptions

2021-07-07 Thread Alexandre Ratchov
On Wed, Jul 07, 2021 at 08:25:16AM -0600, Theo de Raadt wrote:
> > This diff is an attempt to make identifying audio devices easier by
> > giving each device a human-readable description that can be read from
> > userspace (and without scraping dmesg).
> 
> But why does anyone want that?
> 
> Isn't everyone served best when there is a public portable interface,
> and all the back-ends must try to work the same?

Exactly. The long-term aim is to expose this information to programs
through sndiod with a stable libsndio interface, which in turn would
ease switching between audio devices.

This diff is the first step; IMO, it's not necessary until we get the
whole chain, but it doesn't hurt either; it improves audioctl(8)
output, which is a dignostics-only tool.



Re: human-readable audio device descriptions

2021-07-07 Thread Theo de Raadt
> This diff is an attempt to make identifying audio devices easier by
> giving each device a human-readable description that can be read from
> userspace (and without scraping dmesg).

But why does anyone want that?

Isn't everyone served best when there is a public portable interface,
and all the back-ends must try to work the same?



human-readable audio device descriptions

2021-07-07 Thread Edd Barrett
Hi,

This diff is an attempt to make identifying audio devices easier by
giving each device a human-readable description that can be read from
userspace (and without scraping dmesg).

This is implemented on a per-audio-device basis using a new `descr`
interface to `audio_hw_if`. For now I've only implemented this on:

 - azalia(4), which uses the codec info, e.g. `Realtek ALC1150`

 - uaudio(4), which uses the USB vendor and model info, e.g.
   `Logitech HD Pro Webcam C920`.

For drivers, which currently don't implement this interface, the
description string defaults to the audio device node name, e.g.
`uaudio2`.

The string is exposed to userspace via the AUDIO_GETDEV ioctl(2). I've
reclaimed the space currently occupied by the (unused) `version` and
`config` fields in `audio_device_t` and used it for a new `descr` field.
Thus the size of `audio_device_t` remains the same for now.

I've also made audioctl(8) display the description string, e.g.:
```
# audioctl   
name=azalia1
descr=Realtek ALC1150
...
# audioctl -f /dev/audioctl1 
name=uaudio0
descr=SteelSeries SteelSeries Arctis 
...
```

I'm hoping that later we can have sndiod read these strings and expose
them to its clients. For example, I'd like for sndioctl(1) to allow
choosing a device based on the device description string (instead of the
sndio device index), and for xfce4-mixer to allow choosing a device via
its description in the GUI using a drop-down box.

Diff follows.



Index: sys/dev/audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.193
diff -u -p -r1.193 audio.c
--- sys/dev/audio.c 16 May 2021 15:12:37 -  1.193
+++ sys/dev/audio.c 7 Jul 2021 10:21:25 -
@@ -1791,6 +1791,11 @@ audio_getdev(struct audio_softc *sc, str
if (sc->dev.dv_parent == NULL)
return EIO;
strlcpy(adev->name, sc->dev.dv_parent->dv_xname, MAX_AUDIO_DEV_LEN);
+   if (sc->ops->descr != NULL)
+   sc->ops->descr(sc->arg, adev->descr, MAX_AUDIO_DESCR_LEN);
+   else
+   strlcpy(adev->descr, sc->dev.dv_parent->dv_xname,
+   MAX_AUDIO_DESCR_LEN);
return 0;
 }
 
Index: sys/dev/audio_if.h
===
RCS file: /cvs/src/sys/dev/audio_if.h,v
retrieving revision 1.36
diff -u -p -r1.36 audio_if.h
--- sys/dev/audio_if.h  5 Sep 2019 05:33:57 -   1.36
+++ sys/dev/audio_if.h  2 Jul 2021 19:15:53 -
@@ -137,6 +137,8 @@ struct audio_hw_if {
struct audio_params *, struct audio_params *, unsigned int);
unsigned int (*set_nblks)(void *, int,
struct audio_params *, unsigned int, unsigned int);
+
+   void(*descr)(void*, char *, size_t); /* the device's description */
 };
 
 struct audio_attach_args {
Index: sys/dev/pci/azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.263
diff -u -p -r1.263 azalia.c
--- sys/dev/pci/azalia.c11 Jun 2021 15:46:09 -  1.263
+++ sys/dev/pci/azalia.c2 Jul 2021 19:15:53 -
@@ -256,6 +256,7 @@ unsigned int azalia_set_blksz(void *, in
struct audio_params *, struct audio_params *, unsigned int);
 unsigned int azalia_set_nblks(void *, int,
struct audio_params *, unsigned int, unsigned int);
+void   azalia_descr(void *, char *, size_t);
 intazalia_halt_output(void *);
 intazalia_halt_input(void *);
 intazalia_set_port(void *, mixer_ctrl_t *);
@@ -315,7 +316,8 @@ struct audio_hw_if azalia_hw_if = {
NULL,   /* copy_output */
NULL,   /* underrun */
azalia_set_blksz,
-   azalia_set_nblks
+   azalia_set_nblks,
+   azalia_descr/* descr */
 };
 
 static const char *pin_devices[16] = {
@@ -4019,6 +4021,37 @@ azalia_set_nblks(void *v, int mode,
nblks = HDA_BDL_MAX;
 
return nblks;
+}
+
+void
+azalia_descr(void *arg, char *descr, size_t len)
+{
+   azalia_t *az = arg;
+   codec_t *codec;
+   const char *vendor;
+   char buf[MAX_AUDIO_DESCR_LEN];
+   int i;
+
+   *descr = '\0';
+   for (i = 0; i < az->ncodecs; i++) {
+   codec = >codecs[i];
+   /* adapted from azalia_print_codec() */
+   if (codec->name == NULL) {
+   vendor = pci_findvendor(codec->vid >> 16);
+   if (vendor == NULL)
+   snprintf(buf, MAX_AUDIO_DESCR_LEN,
+   "0x%04x/0x%04x", codec->vid >> 16,
+   codec->vid & 0x);
+   else
+   snprintf(buf, MAX_AUDIO_DESCR_LEN, "%s/0x%04x",
+   vendor, codec->vid & 0x);
+   } else
+