Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

thinkpad t14s amd: unable to control built-in mic and headphones mic independently #64

Closed
tanuk opened this issue Dec 5, 2020 · 15 comments

Comments

@tanuk
Copy link

tanuk commented Dec 5, 2020

I did some investigation on a pulseaudio bug report, and found that the UCM configuration for the internal mic and headset mic both use "Capture" as the CaptureMixerElem. This means that muting one device mutes also the other (and I guess volume control is shared too). In amixer output I don't see a separate control for the headset mic, but for the internal mic volume I guess "Digital" can be used (there's no switch, though).

So it seems that these changes are needed:

  • don't set CaptureMixerElem, CaptureSwitch or CaptureVolume for the headset mic
  • set the internal mic CaptureMixerElem and CaptureVolume to Digital
  • you may want to also set CaptureMasterElem for both both mics to Capture
@tanuk
Copy link
Author

tanuk commented Dec 5, 2020

Here's the pa-info output (containing alsa-info):
pa-info-4.txt

@perexg
Copy link
Member

perexg commented Dec 5, 2020

I believe that the Digital control belongs the HDA audio codec and it is not related to mic.

And yes, the HDA capture control is used also for AMD ACP mic to "emulate" the HDA behaviour and to handle the mic mute LED. The AMD ACP mic does not have any volume or mute control in hardware. If you remove "Capture" CaptureMixerElem, the LED won't work. The coupling is a side effect, but I don't think that it's something fatal. We can also describe that Mic1 and Mic2 devices are conflicting.

@ville-h
Copy link

ville-h commented Dec 5, 2020

Having the built-in mic and the headphones mic coupled whilst not fatal is annoying at the least. In a setup where one doesn't use push-to-talk the built-in mic is able to pick up unwanted noises, such as typing on a separate keyboard that is on the same desk the laptop is on. Personally I would be willing to sacrifice the led functionality to be able to control to mics separately if that's the trade off.

@tanuk
Copy link
Author

tanuk commented Dec 5, 2020

Can you explain the "picking up unwanted noises" thing, I don't quite understand what you mean. Is the conference application recording from the headset mic, but the internal mic somehow gets mixed in too?

@ville-h
Copy link

ville-h commented Dec 5, 2020

In case of discord going to "User settings > Audio & Video" if I leave the input device as "Default" the application picks up both mics and the aforementioned unwanted noises. If I switch to "Family 17h (Models 10h-1fh) HD Audio Controller Headphones Stereo Microphone" then it picks up only the headphones mic as expected and greatly reduced typing noises.

For other applications there may not be quite so obvious mechanism to select the audio input device. Probably possible through manually editing pulseaudio configuration files? Simplest from my point of view, as a non-technical user, would be to be able to mute one of the mics from pavucontrol-qt or similar.

@perexg
Copy link
Member

perexg commented Dec 5, 2020

I don't see the useability to have both mics working at the same time. Users expect to use the build-in mic or the external mic in the normal operation and the mic LED should cover both inputs.

You may try this change in the UCM configuration (/usr/share/alsa/ucm2/ tree):

diff --git a/ucm2/HDA-Intel/HiFi.conf b/ucm2/HDA-Intel/HiFi.conf
index f09ce1c..a0ad7e0 100644
--- a/ucm2/HDA-Intel/HiFi.conf
+++ b/ucm2/HDA-Intel/HiFi.conf
@@ -19,9 +19,12 @@ If.analog {
                        True {
                                RenameDevice."Mic1" "Mic"
                        }
-                       False.Include.acp {
-                               Before.SectionDevice "Mic1"
-                               File "/HDA-Intel/HiFi-acp.conf"
+                       False {
+                               Include.acp {
+                                       Before.SectionDevice "Mic1"
+                                       File "/HDA-Intel/HiFi-acp.conf"
+                               }
+                               SectionDevice."Mic1".ConflictingDevice.0 "Mic2"
                        }
                }

But I think that it will require my unmerged patch for the conflicting devices in PA. It's bad that we don't have this merged yet.

https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/290

@tanuk
Copy link
Author

tanuk commented Dec 6, 2020

In case of discord going to "User settings > Audio & Video" if I leave the input device as "Default" the application picks up both mics and the aforementioned unwanted noises. If I switch to "Family 17h (Models 10h-1fh) HD Audio Controller Headphones Stereo Microphone" then it picks up only the headphones mic as expected and greatly reduced typing noises.

I assume the "Default" device is set to the internal mic (if a headset is connected, PulseAudio should automatically switch the default to that, but that's a separate issue). Your desktop environment should have a way to select the default device, and you can do this with pavucontrol too (Input Devices tab) so if you set the default source to headset, this issue should be solved.

It's bad if recording from the internal mic gets the headset mic mixed in (are you sure the headset mic is picked up when recording from the internal mic?), but I guess that's rarely a problem, because if you have a headset connected, you probably don't want to use the internal mic.

We can also describe that Mic1 and Mic2 devices are conflicting.

I'm not sure it makes sense to make the two mics conflicting. Sure, the mute and volume are coupled, but that's not necessary a showstopper if there is a reason for using both mics at the same time. That said, I don't think marking them conflicting does much harm either.

But I think that it will require my unmerged patch for the conflicting devices in PA. It's bad that we don't have this merged yet.

Yes, it's bad, I wish I could do a better job... I'll try to get to it soon.

@ville-h
Copy link

ville-h commented Dec 6, 2020

I assume the "Default" device is set to the internal mic (if a headset is connected, PulseAudio should automatically switch the default to that, but that's a separate issue). Your desktop environment should have a way to select the default device, and you can do this with pavucontrol too (Input Devices tab) so if you set the default source to headset, this issue should be solved

It seems I've been incredibly dumb and possibly bamboozled by the pavucontrol/pavucontrol-qt interfaces somewhat. In the "Input Devices" tab there are three buttons: Mute, Lock, Fallback going by the tooltips shown in both applications. The "Default" device corresponds to the mic that has the Fallback button pressed down? To me the use of the term "fallback" in the tooltip made it sound like that would make the mic have lower priority - opposite of default in some sense. So turns out I've had the wrong mic set as "fallback"/"default".

Does the coupling of the mute state still mean that my laptop will act as a spying device when I physically unplug the headphones and forget to use pavucontrol to mute the mics through software as the built-in mic now remains active? This is something I would like to be able to avoid as well.

@perexg
Copy link
Member

perexg commented Dec 7, 2020

Does the coupling of the mute state still mean that my laptop will act as a spying device when I physically unplug the headphones and forget to use pavucontrol to mute the mics through software as the built-in mic now remains active? This is something I would like to be able to avoid as well.

This is a good point and I see that using CaptureMixerElem with the unrelated HDA control 'Capture' is not good just to turn on/off LED. @tanuk, correct me, if I'm wrong but it detaches the software volume control, right? Does PA do the software mute (silence the input PCM stream) when the mute is active? The AMD mic driver does not have any hardware volume/mute control.

The scenario is: plug headphones with mic -> enable headphones mic (which will enable the internal mic, too) -> unplug headphones -> the internal mic is sharing the mute (and volume) state for headphones.

@tiwai - it was your idea. We probably need another way to resolve this. The LED status should be merged from multiple drivers in the ALSA core in my opinion. We cannot even set the Capture control from Enable/DisableSequence, because it will break the HDA state. My original proposal was to add "Mic LED Switch" to the AMD driver and set it from Enable/DisableSequence.

#30 @VijendarReddy CC

I reverted 3320b1a for now. We need a better way to handle the mic LED state for this hardware.

perexg added a commit that referenced this issue Dec 7, 2020
…ic device"

This reverts commit 3320b1a.

This solution does not work. The mute state is shared between
the HDA and AMD ACP in PA, so it may cause the security issue, if
the users do not set the mute manually.

BugLink: #64
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
@tiwai
Copy link
Contributor

tiwai commented Dec 7, 2020

If the usage of "Capture Switch" does matter, alternatively, we may change "Mic Mute-LED mode" to either "On" or "Off" in Enable/DisableSequence explicitly, instead. This will detach the LED control from the capture state, so this has to be adjusted properly in both built-in and external mic sections.

But I still don't know whether the sequence will be reflected when you apply the software mute. Would the software mute invoke the Enable/DisableSequence always accordingly?

perexg added a commit that referenced this issue Dec 7, 2020
…ic device"

This reverts commit 3320b1a.

This solution does not work. The mute state is shared between
the HDA and AMD ACP in PA, so it may cause the security issue, if
the users do not set the mute manually.

BugLink: #64
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
@tanuk
Copy link
Author

tanuk commented Dec 7, 2020

This is a good point and I see that using CaptureMixerElem with the unrelated HDA control 'Capture' is not good just to turn on/off LED. @tanuk, correct me, if I'm wrong but it detaches the software volume control, right? Does PA do the software mute (silence the input PCM stream) when the mute is active?

Even if there's a hardware mute, PulseAudio implements software mute as well (IIRC, that's an optimization to avoid unnecessary mixing when the result is all zeroes anyway).

But I still don't know whether the sequence will be reflected when you apply the software mute. Would the software mute invoke the Enable/DisableSequence always accordingly?

No, changing the mute state in PulseAudio doesn't run Enable/DisableSequence.

Is it an option to provide switches in the kernel driver that don't control any hardware, but implement muting in software? PulseAudio could then use the "virtual" mute controls to implement independent mute control and the kernel knows when to enable the led.

Simply marking the two devices as conflicting already mitigates the problem somewhat (once PulseAudio is fixed to handle conflicting devices properly). There will be only one source at a time, and the mute state is stored independently for the sources, so this sequence works quite sensibly:

  1. No headset plugged in, the user mutes the internal mic (and expects it to stay muted).
  2. Headset is plugged in. PulseAudio will remove the internal mic source and create the headset mic source. The headset mic mute state is restored (defaulting to unmuted).
  3. Headset is unplugged. PulseAudio will remove the headset mic source and create the internal mic source. The internal mic muet state is restored to muted.

@perexg
Copy link
Member

perexg commented Dec 7, 2020

If the usage of "Capture Switch" does matter, alternatively, we may change "Mic Mute-LED mode" to either "On" or "Off" in Enable/DisableSequence explicitly, instead. This will detach the LED control from the capture state, so this has to be adjusted properly in both built-in and external mic sections.

It does not resolve the state merge issue.

But I still don't know whether the sequence will be reflected when you apply the software mute. Would the software mute invoke the Enable/DisableSequence always accordingly?

The sequences are executed when the device is active (regarless of the mute state). So yes, this solution is not perfect, too, but better than share the one shared switch for multiple devices.

PA (or the UCM application) should be probably aware, that the switch purpose is only for the streaming state reflection. We can add a new value to UCM to describe this like 'CaptureMixerStateElem'. If we set the same value for all related UCM devices, the PA can probably merge the state and set this control based on the internal mute state. @tanuk ?

@tanuk
Copy link
Author

tanuk commented Dec 7, 2020

Even if there's a hardware mute, PulseAudio implements software mute as well (IIRC, that's an optimization to avoid unnecessary mixing when the result is all zeroes anyway).

When I wrote that, I was thinking of sinks, but it seems that "double mute" is done with sources as well.

@tanuk
Copy link
Author

tanuk commented Dec 7, 2020

PA (or the UCM application) should be probably aware, that the switch purpose is only for the streaming state reflection. We can add a new value to UCM to describe this like 'CaptureMixerStateElem'. If we set the same value for all related UCM devices, the PA can probably merge the state and set this control based on the internal mute state. @tanuk ?

Setting an extra mixer value when muting should be very doable.

@perexg
Copy link
Member

perexg commented May 27, 2022

This issue was fixed at the kernel level (separated software LED switch + LED on/off logic in the kernel merging all states). No UCM and application updates are required now.

@perexg perexg closed this as completed May 27, 2022
ninelore pushed a commit to ninelore/alsa-ucm-conf-cros that referenced this issue Jun 10, 2024
…ic device"

This reverts commit 3320b1a.

This solution does not work. The mute state is shared between
the HDA and AMD ACP in PA, so it may cause the security issue, if
the users do not set the mute manually.

BugLink: alsa-project#64
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants