Bug 218605

Summary:	Commit 1601cd53c7e3 causes userspace regression in probably alsa-ucm-conf or maybe PipeWire
Product:	Drivers	Reporter:	Niklāvs Koļesņikovs (pinkflames.linux)
Component:	Sound(ALSA)	Assignee:	Jaroslav Kysela (perex)
Status:	NEW ---
Severity:	normal	CC:	alexander, kl, tiwai
Priority:	P3
Hardware:	All
OS:	Linux
Kernel Version:		Subsystem:
Regression:	Yes	Bisected commit-id:	1601cd53c7e3197181277326dbfc131d20a74e46

Description Niklāvs Koļesņikovs 2024-03-15 19:45:56 UTC

While testing Torvalds' master branch of Linux which will eventually become 6.9-rc1, I have stumbled upon PipeWire/WirePlumber apparently no longer applying the UCM profile for my USB connected on-board audio controller. Git bisection leads to commit 1601cd53c7e3197181277326dbfc131d20a74e46 .

It's possible that I'm wrong in concluding that the issue is UCM no longer applying/matching but that's my best guess looking at seemingly ACP named devices and the lack of any ucm strings in the output of the pw-dump tool, when the regression is present.

It's worth noting that the only known [to me] users of alsa-ucm-conf are PipeWire/WirePlumber and the legacy PulseAudio daemons, which to some extent share their ALSA backend implementation, so while this is probably a userspace regression, the breakage might be limited enough for everyone to agree on pressing forward with the change anyway.

I have opened an alsa-ucm-conf issue, too: https://github.com/alsa-project/alsa-ucm-conf/issues/404 .

The result of git bisection:

```
1601cd53c7e3197181277326dbfc131d20a74e46 is the first bad commit
commit 1601cd53c7e3197181277326dbfc131d20a74e46
Author: Kenny Levinsen <kl@kl.wtf>
Date:   Sat Mar 2 00:11:07 2024 +0100

    ALSA: usb-audio: Name feature ctl using output if input is PCM
    
    When building feature controls from a unit without a name, we try to
    derive a name first from the feature unit's input, then fall back to the
    output terminal.
    
    If a feature unit connects directly to a "USB Streaming" input terminal
    rather than a mixer or other virtual type, the control receives the
    somewhat meaningless name "PCM", even if the output had a descriptive
    type such as "Headset" or "Speaker".
    
    Here is an example of such AudioControl descriptor from a USB headset
    which ends up named "PCM Playback" and is therefore not recognized as
    headphones by userspace:
    
          AudioControl Interface Descriptor:
            bLength                12
            bDescriptorType        36
            bDescriptorSubtype      2 (INPUT_TERMINAL)
            bTerminalID             4
            wTerminalType      0x0101 USB Streaming
            bAssocTerminal          5
            bNrChannels             2
            wChannelConfig     0x0003
              Left Front (L)
              Right Front (R)
            iChannelNames           0
            iTerminal               0
          AudioControl Interface Descriptor:
            bLength                 9
            bDescriptorType        36
            bDescriptorSubtype      3 (OUTPUT_TERMINAL)
            bTerminalID             5
            wTerminalType      0x0402 Headset
            bAssocTerminal          4
            bSourceID               6
            iTerminal               0
          AudioControl Interface Descriptor:
            bLength                13
            bDescriptorType        36
            bDescriptorSubtype      6 (FEATURE_UNIT)
            bUnitID                 6
            bSourceID               4
            bControlSize            2
            bmaControls(0)     0x0002
              Volume Control
            bmaControls(1)     0x0000
            bmaControls(2)     0x0000
            iFeature                0
    
    Other headsets and DACs I tried that used their output terminal for
    naming only did so due to their input being an unnamed sidetone mixer.
    
    Instead of always starting with the input terminal, check the type of it
    first. If it seems uninteresting, invert the order and use the output
    terminal first for naming.
    
    This makes userspace recognize headsets with simple controls as
    headphones, and leads to more consistent naming of playback devices
    based on their outputs irrespective of sidetone mixers.
    
    Signed-off-by: Kenny Levinsen <kl@kl.wtf>
    Link: https://lore.kernel.org/r/20240301231107.42679-1-kl@kl.wtf
    Signed-off-by: Takashi Iwai <tiwai@suse.de>

 sound/usb/mixer.c | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)
```

Comment 1 Takashi Iwai 2024-03-16 08:25:23 UTC

Thanks for the report.  Indeed the change of the existing control name elements could lead to a regression.  I applied the patch with a hope that there would be only few such breakage, but it seems that it's more visible than my wish, unfortunately.

I guess the issue is only for the devices with the fixed configuration like UCM, and those could be worked around with the additional fixed mapping or making the change conditional with a blacklist / whitelist.  But it needs a bit more intensive tests.

I'm going to revert the change for 6.9-rc1.  We may take this again in a modified form later for 6.10.

Comment 2 Kenny Levinsen 2024-03-16 11:28:27 UTC

Hmm, the regression is unfortunate.

If my grep-fu serves me right, only 5 alsa-ucm configurations appear to rely on PCM naming. I wonder if it would acceptable to handle the new names on the alsa-ucm side by adding conditions in those few places for the new names.

Alternatively, none of these rules appear to apply to devices with Headset or Headphone outputs, so restricting the patch to headsets would avoid breakage while still benefiting from userspace recognizing headphones and headsets correctly.

Comment 3 Niklāvs Koļesņikovs 2024-03-16 13:27:39 UTC

I'm not sure if that counts but I do have the HDA spec PC front audio headphone jack wired up and in use. The chip itself can be configured by integrator to support, I believe, two such headphone jacks. But maybe we're talking about different things?

Comment 4 Kenny Levinsen 2024-03-16 14:19:13 UTC

> I'm not sure if that counts but I do have the HDA spec PC front audio
> headphone jack wired up and in use. The chip itself can be configured by
> integrator to support, I believe, two such headphone jacks. But maybe we're
> talking about different things?

HDA is unaffected, and the number of connectors shouldn't matter (apart from your UCM configuration needing to support it).

The discussed change only affected naming of controls for USB audio devices, based on the exposed audio control descriptors that define what controls are available (e.g., volume and mute, selectors, effects) and what the controls connect to (e.g., "sound comes from USB, goes into a mixer, then into volume control, then to speakers").

The old logic tried to name the control using its input first:

- USB -> Volume/Mute -> Headset: "PCM Playback Volume"
- USB -> Volume/Mute -> Speaker: "PCM Playback Volume"
- USB -> Sidetone Mixer -> Volume/Mute -> Headset: "Headset Playback Volume"
- USB -> Sidetone Mixer -> Volume/Mute -> Speaker: "Speaker Playback Volume"

For the last two, the presence of a sidetone mixer makes naming based on input fail, falling back to using the output - thus giving a good name almost in error.

The new logic prioritized the output if the input had no useful information: 

- USB -> Volume/Mute -> Headphones: "Headset Playback Volume"
- USB -> Volume/Mute -> Speaker: "Speaker Playback Volume"
- USB -> Sidetone Mixer -> Volume/Mute -> Headset: "Headset Playback Volume"
- USB -> Sidetone Mixer -> Volume/Mute -> Speaker: "Speaker Playback Volume"

More consistent, and ensured that headphones and headsets without sidetone mixers also showed up correctly instead of being branded "PCM".

(Sidetone mixers inject microphone input into headsets or speakers to make you not feel like you have earplugs in your ears when talking - look for a "Mic Playback" control - and is often present on headsets and headphone/mic jacks)

Comment 5 Niklāvs Koļesņikovs 2024-03-16 14:59:13 UTC

Sorry, I meant the HDA specified front panel audio header, which replaced the older AC97(?) motherboard header i.e. it's a regular tower PC with the usual front audio headphone and mic ports. So I do have a headphones jack but probably no microphone input injection functionality.

It is now increasingly common to deploy USB audio as motherboard on-board audio, which at least for my audio codec means that instead of directly attaching via I2S, there's now a USB device controller packaged along with the same old HDA audio codec IC, making it appear as a hotpluggable UAC device. From about 2022/2023 onwards this is increasingly common in higher end motherboards while other devices, I guess, are supposed to use the SoundWire approach with its lower BOM and smaller footprint.

Comment 6 Kenny Levinsen 2024-03-16 16:26:54 UTC

That's also fine. It should reasonably only become a regression in the cases where:

1. A USB audio device exposes simple controls (e.g. no side-tone mixer)
2. No dedicated name is provided (explicit names appear somewhat rare)
3. A UCM configuration exists for this setup directly referencing a control by name (e.g. PCM Playback Volume"), this failing to take effect

While USB audio is very common, on my inspection it seems that very few UCM configurations are affected. Ideally we want both meaningful names so that things work well out of the box *and* working UCM configurations for things that don't work well out of the box.

Comment 7 Niklāvs Koļesņikovs 2024-03-16 16:34:52 UTC

Agreed. As I said, I'm not really against fixing alsa-ucm-conf instead. All I wanted to highlight, is that while the number of devices might not be large, there's reasons to think it will affect thousands upon thousands of PC motherboards and will likely pop up as a regression if some distro ships a new enough kernel with old alsa-ucm-conf, which while hopefully unlikely is not entirely impossible e.g. Ubuntu LTS with a considerably newer HWE kernel.

Comment 8 Niklāvs Koļesņikovs 2024-03-16 17:12:19 UTC

I'll just use this place to apologize for accidentally sending a top replied e-mail to the mailing list. I do not use e-mail, so I did not realize that the three dots at the bottom of the panel were the collapsed original e-mail about the revocation commit.