From: "Jonathan Armani" <jonathan@armani.tech>
Subject: Re: Expose better display names for audio(4) devices
To: tech@openbsd.org
Date: Sun, 28 Sep 2025 10:37:47 +0200

Hello Alex,

I still agree this is a really usefull feature :)

For PCI devices, maybe we could rely on a more generic approch with pcidevs instead of manually reverse with a switch ? 
The only downside I see is that name will typically include "Audio" (or similar).

The USB with filtering and the audio part looks good and ok for me. 

While reviewing your diff I saw that some functions documented in audio.9 as declared as returning an int, but are actually unsigned int in the headers, is that expected ?

On Sat, Sep 27, 2025, at 20:00, Alexandre Ratchov wrote:
> On Tue, Jul 15, 2025 at 03:36:01PM +0100, Stuart Henderson wrote:
>> On 2025/07/13 18:37, Alexandre Ratchov wrote:
>> > If there are multiple uaudio(4) devices, it is difficult to know which
>> > sndioctl(1) selector corresponds to which hardware interface without
>> > searching in the dmesg output. Example:
>> > 
>> > $ sndioctl -i server.device
>> > server.device=0(azalia0),1(envy0),2(cmpci0),3(uaudio0),4(uaudio1),5(uaudio2),6,7
>> > 
>> > The diff below makes the kernel expose a slightly more useful string
>> > instead of the device->dv_xname as the device display name. Ex., the
>> > product name for uaudio(4), the codec type for azalia(4), the card
>> > name for envy(4) and cmpci(4). The result:
>> > 
>> > $ sndioctl -i server.device
>> > server.device=0(HDA: Analog),1(ESI Julia),2(CMI8738),3(U-24),4(Webcam C310),5(AudioBox Go),6,7
>> > 
>> > These are short strings mainly intended for GUIs or as simple hints.
>> > 
>> > OK?
>> 
>> Be prepared for poor quality product strings coming from some USB
>> devices - might not be a bad idea if sndioctl sanitized them, e.g.
>> stravis(...VIS_SAFE)/similar, though "," might also be unwelcome
>> if someone actually parsed sndioctl output in a shell script etc...
>
> Here's a new diff: the uaudio(4) part skips non-ascii and control
> characters. This way audioctl(1), sndioctl(1) or anything else using
> the ioctl, get the same string with no control chars.
>
> Index: share/man/man9/audio.9
> ===================================================================
> RCS file: /cvs/src/share/man/man9/audio.9,v
> diff -u -p -u -p -r1.35 audio.9
> --- share/man/man9/audio.9	15 Oct 2023 15:49:47 -0000	1.35
> +++ share/man/man9/audio.9	27 Sep 2025 17:49:20 -0000
> @@ -79,6 +79,7 @@ struct audio_hw_if {
>  		    struct audio_params *, struct audio_params *, int);
>  	int	(*set_nblks)(void *, int, int,
>  		    struct audio_params *, int);
> +	size_t	(*display_name)(void *, char *, size_t);
>  };
> 
>  struct audio_params {
> @@ -453,6 +454,22 @@ function.
>  is the desired number of blocks in the ring buffer.
>  It may be lowered to at least two, to match hardware constraints.
>  This function returns the adjusted number of blocks.
> +.It Ft size_t Fn (*display_name) "void *hdl" "char *buf" "size_t size"
> +This function produces a string suitable for user interfaces and
> +intended to help the user to identify the hardware.
> +Like
> +.Xr snprintf 3 ,
> +this function returns the total length of the string it tried to create
> +(excluding the final
> +.Ql \e0 ) .
> +Unless
> +.Fa size
> +is 0, it writes at most
> +.Fa size Ns \-1
> +characters followed by a terminating
> +.Ql \e0
> +at the location pointed by
> +.Fa buf .
>  .El
>  .Pp
>  If the audio hardware is capable of input from more
> Index: sys/dev/audio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/audio.c,v
> diff -u -p -u -p -r1.211 audio.c
> --- sys/dev/audio.c	14 Feb 2025 13:29:00 -0000	1.211
> +++ sys/dev/audio.c	27 Sep 2025 17:49:22 -0000
> @@ -1735,12 +1735,22 @@ audio_write(struct audio_softc *sc, stru
>  }
> 
>  int
> -audio_getdev(struct audio_softc *sc, struct audio_device *adev)
> +audio_getdev(struct audio_softc *sc, struct audio_device *p)
>  {
> -	memset(adev, 0, sizeof(struct audio_device));
> -	if (sc->dev.dv_parent == NULL)
> -		return EIO;
> -	strlcpy(adev->name, sc->dev.dv_parent->dv_xname, MAX_AUDIO_DEV_LEN);
> +	size_t sz;
> +
> +	memset(p, 0, sizeof(struct audio_device));
> +	sz = 0;
> +
> +	if (sc->ops->display_name)
> +		sz = sc->ops->display_name(sc->arg, p->name, sizeof(p->name));
> +
> +	if (sz == 0) {
> +		if (sc->dev.dv_parent == NULL)
> +			return EIO;
> +		strlcpy(p->name, sc->dev.dv_parent->dv_xname, sizeof(p->name));
> +	}
> +
>  	return 0;
>  }
> 
> Index: sys/dev/audio_if.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/audio_if.h,v
> diff -u -p -u -p -r1.42 audio_if.h
> --- sys/dev/audio_if.h	2 Nov 2022 10:41:34 -0000	1.42
> +++ sys/dev/audio_if.h	27 Sep 2025 17:49:22 -0000
> @@ -125,6 +125,7 @@ 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);
> +	size_t	(*display_name)(void *, char *, size_t);
>  };
> 
>  struct audio_attach_args {
> Index: sys/dev/pci/azalia.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> diff -u -p -u -p -r1.290 azalia.c
> --- sys/dev/pci/azalia.c	18 Aug 2024 14:42:56 -0000	1.290
> +++ sys/dev/pci/azalia.c	27 Sep 2025 17:49:25 -0000
> @@ -269,6 +269,7 @@ int	azalia_trigger_output(void *, void *
>  	void (*)(void *), void *, audio_params_t *);
>  int	azalia_trigger_input(void *, void *, void *, int,
>  	void (*)(void *), void *, audio_params_t *);
> +size_t  azalia_display_name(void *, char *, size_t);
> 
>  int	azalia_params2fmt(const audio_params_t *, uint16_t *);
> 
> @@ -305,6 +306,7 @@ const struct audio_hw_if azalia_hw_if = 
>  	.trigger_input = azalia_trigger_input,
>  	.set_blksz = azalia_set_blksz,
>  	.set_nblks = azalia_set_nblks,
> +	.display_name = azalia_display_name,
>  };
> 
>  static const char *pin_devices[16] = {
> @@ -4208,6 +4210,30 @@ azalia_trigger_input(void *v, void *star
>  	az->rstream.swpos = 0;
> 
>  	return azalia_stream_start(&az->rstream);
> +}
> +
> +size_t
> +azalia_display_name(void *self, char *buf, size_t size)
> +{
> +	azalia_t *az = (azalia_t *)self;
> +	codec_t *codec = &az->codecs[az->codecno];
> +	const char *name;
> +
> +	switch (codec->codec_type) {
> +	case AZ_CODEC_TYPE_ANALOG:
> +		name = "HDA: Analog";
> +		break;
> +	case AZ_CODEC_TYPE_DIGITAL:
> +		name = "HDA: Digital";
> +		break;
> +	case AZ_CODEC_TYPE_HDMI:
> +		name = "HDA: HDMI";
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return strlcpy(buf, name, size);
>  }
> 
>  /* --------------------------------
> Index: sys/dev/pci/cmpci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/cmpci.c,v
> diff -u -p -u -p -r1.54 cmpci.c
> --- sys/dev/pci/cmpci.c	24 May 2024 06:02:53 -0000	1.54
> +++ sys/dev/pci/cmpci.c	27 Sep 2025 17:49:26 -0000
> @@ -145,6 +145,7 @@ int cmpci_trigger_output(void *, void *,
>  int cmpci_trigger_input(void *, void *, void *, int,
>  				    void (*)(void *), void *,
>  				    struct audio_params *);
> +size_t cmpci_display_name(void *, char *, size_t);
> 
>  const struct audio_hw_if cmpci_hw_if = {
>  	.open = cmpci_open,
> @@ -161,6 +162,7 @@ const struct audio_hw_if cmpci_hw_if = {
>  	.round_buffersize = cmpci_round_buffersize,
>  	.trigger_output = cmpci_trigger_output,
>  	.trigger_input = cmpci_trigger_input,
> +	.display_name = cmpci_display_name,
>  };
> 
>  /*
> @@ -1860,6 +1862,31 @@ cmpci_trigger_input(void *handle, void *
>  	cmpci_reg_set_4(sc, CMPCI_REG_FUNC_0, CMPCI_REG_CH1_ENABLE);
>  	mtx_leave(&audio_lock);
>  	return 0;
> +}
> +
> +size_t
> +cmpci_display_name(void *self, char *buf, size_t size)
> +{
> +	struct cmpci_softc *sc = (struct cmpci_softc *)self;
> +	const char *name;
> +
> +	switch (PCI_PRODUCT(sc->sc_id)) {
> +	case PCI_PRODUCT_CMI_CMI8338A:
> +		name = "CMI8338A";
> +		break;
> +	case PCI_PRODUCT_CMI_CMI8338B:
> +		name = "CMI8338B";
> +		break;
> +	case PCI_PRODUCT_CMI_CMI8738:
> +		name = "CMI8738";
> +		break;
> +	case PCI_PRODUCT_CMI_CMI8738B:
> +		name = "CMI8738B";
> +		break;
> +	default:
> +		name = sc->sc_dev.dv_xname;
> +	}
> +	return strlcpy(buf, name, size);
>  }
> 
>  /* end of file */
> Index: sys/dev/pci/envy.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/envy.c,v
> diff -u -p -u -p -r1.89 envy.c
> --- sys/dev/pci/envy.c	20 Sep 2025 13:50:33 -0000	1.89
> +++ sys/dev/pci/envy.c	27 Sep 2025 17:49:26 -0000
> @@ -110,6 +110,7 @@ int envy_halt_input(void *);
>  int envy_query_devinfo(void *, struct mixer_devinfo *);
>  int envy_get_port(void *, struct mixer_ctrl *);
>  int envy_set_port(void *, struct mixer_ctrl *);
> +size_t envy_display_name(void *, char *, size_t);
>  #if NMIDI > 0
>  int envy_midi_open(void *, int, void (*)(void *, int),
>      void (*)(void *), void *);
> @@ -191,6 +192,7 @@ const struct audio_hw_if envy_hw_if = {
>  	.freem = envy_freem,
>  	.trigger_output = envy_trigger_output,
>  	.trigger_input = envy_trigger_input,
> +	.display_name = envy_display_name,
>  };
> 
>  #if NMIDI > 0
> @@ -2433,6 +2435,14 @@ envy_set_port(void *self, struct mixer_c
>  	if (idx < ndev)
>  		return sc->card->dac->set(sc, ctl, idx);
>  	return ENXIO;
> +}
> +
> +size_t
> +envy_display_name(void *self, char *buf, size_t size)
> +{
> +	struct envy_softc *sc = (struct envy_softc *)self;
> +
> +	return strlcpy(buf, sc->card->name, size);
>  }
> 
>  #if NMIDI > 0
> Index: sys/dev/usb/uaudio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> diff -u -p -u -p -r1.179 uaudio.c
> --- sys/dev/usb/uaudio.c	14 Jul 2025 23:49:08 -0000	1.179
> +++ sys/dev/usb/uaudio.c	27 Sep 2025 17:49:31 -0000
> @@ -212,6 +212,7 @@ struct uaudio_softc {
>  	struct device dev;
>  	struct usbd_device *udev;
>  	int version;
> +	int instnum;
> 
>  	/*
>  	 * UAC exposes the device as a circuit of units. Input and
> @@ -442,6 +443,7 @@ int uaudio_halt_input(void *);
>  int uaudio_query_devinfo(void *, struct mixer_devinfo *);
>  int uaudio_get_port(void *, struct mixer_ctrl *);
>  int uaudio_set_port(void *, struct mixer_ctrl *);
> +size_t uaudio_display_name(void *, char *, size_t);
> 
>  int uaudio_process_unit(struct uaudio_softc *,
>      struct uaudio_unit *, int,
> @@ -493,6 +495,7 @@ const struct audio_hw_if uaudio_hw_if = 
>  	.copy_output = uaudio_copy_output,
>  	.underrun = uaudio_underrun,
>  	.set_blksz = uaudio_set_blksz,
> +	.display_name = uaudio_display_name,
>  };
> 
>  /*
> @@ -2800,6 +2803,7 @@ uaudio_process_conf(struct uaudio_softc 
>  			i = uaudio_iface_index(sc, ifnum);
>  			if (i != -1 && usbd_iface_claimed(sc->udev, i)) {
>  				DPRINTF("%s: %d: AC already claimed\n", __func__, ifnum);
> +				sc->instnum++;
>  				break;
>  			}
>  			if (sc->unit_list != NULL) {
> @@ -4480,6 +4484,62 @@ uaudio_set_port(void *arg, struct mixer_
>  	rc = uaudio_set_port_do(sc, ctl);
>  	usbd_ref_decr(sc->udev);
>  	return rc;
> +}
> +
> +size_t
> +uaudio_display_name(void *arg, char *buf, size_t size)
> +{
> +	struct uaudio_softc *sc = arg;
> +	char *vendor = sc->udev->vendor;
> +	char *product = sc->udev->product;
> +	char *p = buf, *end = buf + size;
> +	size_t i;
> +	int c;
> +
> +	if (product == NULL || vendor == NULL)
> +		return strlcpy(buf, DEVNAME(sc), size);
> +
> +	/* 
> +	 * If the product name is prefixed with the vendor, drop the prefix
> +	 */
> +	for (i = 0; product[i] != 0; i++) {
> +		if (vendor[i] == 0) {
> +			while (product[i] == ' ')
> +				i++;
> +			product += i;
> +			break;
> +		}
> +		if (vendor[i] != product[i])
> +			break;
> +	}
> +
> +	/*
> +	 * Copy the product name removing control and non-ascii chars. The
> +	 * destination pointer 'p' is advanced, but chars are not written
> +	 * past the 'end' pointer (end of the buffer).
> +	 */
> +	while ((c = *product++) != 0) {
> +		if (c < ' ' || c > '~')
> +			continue;
> +		if (p < end)
> +			*p = c;
> +		p++;
> +	}
> +
> +	/*
> +	 * Terminating '\0'
> +	 */
> +	if (size > 0)
> +		*(p < end ? p : end - 1) = 0;
> +
> +	/*
> +	 * Append the instance number (if any), similarly advance 'p' but
> +	 * don't write past 'end'
> +	 */
> +	if (sc->instnum > 0)
> +		p += snprintf(p, p < end ? end - p : 0, "#%u", sc->instnum + 1);
> +
> +	return p - buf;
>  }
> 
>  int