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

Mute microphone command #16286

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Mute microphone command #16286

wants to merge 11 commits into from

Conversation

mltony
Copy link
Contributor

@mltony mltony commented Mar 9, 2024

Link to issue number:

Closes #16056

Summary of the issue:

A command to mute/unmute microphone.

Description of user facing changes

Added a global command to mute/unmute microphone. No default gesture assigned.
Added wasapi notification listener; when it detects microphone mute/unmute event - it speaks it. This would speak notification even if microphone is muted by other means, e.g. laptop-specific keystroke or windows settings.

Description of development approach

Made use of pycaw for both mute command and notification listener.

Testing strategy:

Tested manually on Windows 11.

Known issues with pull request:

Wasapi notification comes in with a noticeable delay of about 1 second.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@codeofdusk
Copy link
Contributor

Added wasapi notification listener; when it detects microphone mute/unmute event - it speaks it.

Makes sense – I'm guessing this is indicated visually, and a screen reader report here gives us equivalent access.

Added a global command to mute/unmute microphone. No default gesture assigned.

This feels... out of place in a screen reader. Doesn't Windows have a hotkey to do this now?

@mltony
Copy link
Contributor Author

mltony commented Mar 9, 2024

@codeofdusk ,

Doesn't Windows have a hotkey to do this now?

No, see explanation in the original feature request.

@mltony mltony marked this pull request as ready for review March 9, 2024 20:29
@mltony mltony requested review from a team as code owners March 9, 2024 20:29
@XLTechie
Copy link
Collaborator

XLTechie commented Mar 9, 2024 via email

@mltony
Copy link
Contributor Author

mltony commented Mar 9, 2024

@XLTechie, the global command would mute the default microphone. Notification will be spoken if any microphone is muted. If not default microphone is muted, it would also speak its name; while for default one it'll just say "muted/unmuted microphone".

@Brian1Gaff
Copy link

Brian1Gaff commented Mar 10, 2024 via email

@bramd
Copy link
Contributor

bramd commented Mar 11, 2024

The notifications may indeed be useful, but I wonder if we are getting more information than visual users here. As far as I know, Windows shows a notification icon in the notification area/system tray when an app uses the microphone to alert the user to this, however I don't think there is a clear global visual indicator for the mic status. Would it be a better translation of visual information if we give a notification as soon as the microphone notification icon is present?

Some programs, like Zoom or Teams send their own notifications when the microphone is muted/unmuted, so with the global notification on top that might be a bit much? In other programs, an active notification might not be send and there we could enable this feature to have information that is equal to what is visible on the screen. Also, a short sound may be considered instead to not disturb an ongoing voice communication so much.

To recap, I'm not against these notifications and think they might be very useful in some circumstances, but it raises the question if we should provide more information than what is given to visual users.

@beqabeqa473
Copy link
Contributor

beqabeqa473 commented Mar 11, 2024 via email

@XLTechie
Copy link
Collaborator

XLTechie commented Mar 11, 2024 via email

@Adriani90
Copy link
Collaborator

Sighted people are also looking for a solution in this regard, see different discussions on Redit. I found a tool, that solves exactly this use case:
https://github.com/bralexc/mute-unmute-meetings

@mltony given this tool is also something sighted people use, is it something you could do too for solving your use case? Otherwise I don't see a strong argument why this feature should be part of NVDA core if there is already a tool for it. That tool even allows you to define audio indication when the Microfone is muted or unmuted.

@seanbudd
Copy link
Member

seanbudd commented Mar 12, 2024

@bramd - many keyboards (including my laptop) have a microphone mute function key with an associated light to indicate if it is toggled. This issue was initially triaged as there is usually a visual notification on the screen and sometimes keyboard lights to indicate microphone muting. I don't think we necessarily need the keyboard command to perform this, as there are other (e.g. keyboard, app specific) solutions and it is slightly out of scope, but the feature is pretty minor and very handy.

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me, thanks @mltony

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
source/globalCommands.py Outdated Show resolved Hide resolved
source/globalCommands.py Outdated Show resolved Hide resolved
source/audio/notifications.py Outdated Show resolved Hide resolved
source/audio/notifications.py Outdated Show resolved Hide resolved
source/audio/notifications.py Outdated Show resolved Hide resolved
source/audio/notifications.py Outdated Show resolved Hide resolved
@XLTechie
Copy link
Collaborator

XLTechie commented Mar 12, 2024 via email

@Adriani90
Copy link
Collaborator

@mltony can you make sure the original state of the microfone is returned when closing NVDA? Otherwise we run into the same problem as in #16292.

And we don't really want to make sighted users working on the same machine confused.

I still think implementing an announcement of Microfone state when changed globally might be rather something that belongs to NVDA, and not the command itself. However, the native notification of mic state should be preserved when changing with the native scripts of the coresponding app.

mltony and others added 8 commits March 12, 2024 14:29
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@AppVeyorBot
Copy link

See test results for failed build of commit 908cb9f8e8

Copy link
Collaborator

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mltony, I have tried to test this build, fixing the two syntax errors below.

Just wanted to report the following issue:
I have assigned a key (NVDA+shift+delete) to the mute / unmute command. Trying to use the command, it seems to change the mute state of the mic.
However, I cannot hear any mute / unmute notification. Is this expected?

I am running from source, under Windows 10 22H2 (AMD64) build 19045.4046.

source/audio/notifications.py Outdated Show resolved Hide resolved
source/audio/notifications.py Outdated Show resolved Hide resolved
mltony and others added 2 commits March 14, 2024 11:47
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
@mltony
Copy link
Contributor Author

mltony commented Mar 14, 2024

@CyrilleB79 , I can reproduce this on Windows 10. Will try to investigate.

@mltony
Copy link
Contributor Author

mltony commented Mar 14, 2024

Ok, here are more results of my testing:

  1. I have a Windows 11 laptop. Mute notifications work as expected.
  2. I have a Windows 10 laptop. Mute notifications work as expected.
  3. I have a virtual machine with Windows 10. It's got a microphone device. Mute notifications are not coming at all.

@CyrilleB79 , do you experience your issue on a real installation or VM?
I couldn't figure out the difference between two windows 10 installations. I verified that notification are not coming even when I use pycaw example, so the issue is not with my code, but most likely with wasapi itself.
Googling doesn't return any relevant results at all - I guess not many people use this type of audio notifications.
The conclusion I would like to make is this: wasapi notifications appear to be unreliable. Given that, do we want to still include them in the coreand risk having bug reports from people for whom they don't work? Or better not include them?
I am thinking that the best course would be to make mute mic command to speak its own update, but keep audio notifications, but make a note in user_guide saying that these notifications are not reliable.

@CyrilleB79
Copy link
Collaborator

My test was done on a real installation, but with no recording or application using the mic.

Same result with a second Windows 10 machine running Windows 10 2004 (AMD64) build 19041.388.
On this second machine, I have tested the mute/unmute command while audacity is recording and the notification is not heard; the mic is correctly muted/unmuted though.

In addition, I have 3 or 4 notifications heard when I open audacity, before I record anything. May it be a matter of sound card disabling the mic when not needed.

@mltony
Copy link
Contributor Author

mltony commented Mar 15, 2024

At this point I'm biased to drop mute notifications all together. While it works perfectly on my Windows 11 laptop, it has problems on both my Windows 10 installations and also Cyrille reported similar problems on Windows 10 as well.
So the symptoms I'm observing are:

  1. Sometimes mute notification doesn't fire at all.
  2. Sometimes we receive multiple mute notification. On my Windows 10 laptop manually unmuting it in settings (well unchecking "disable microphone" checkbox) triggers 37 notifications in a row.
  3. Sometimes we receive mute notification on Windows login - e.g. without even explicitly muting/unmuting microphone.
  4. (reported by Cyrille) Mute notifications are fired when opening Audacity.

I did some debugging, and checked notifications with pycaw example - I see the same symptoms, so likely this is not caused by a bug in my code. I don't think pycaw is at fault either - it's just a thin wrapper over com interfaces. Most likely something is wrong with Windows 10. It's possible that Microsoft fixed that only in Windows 11. Would be great if anyone else could test if this works on Windows 11. But other than that - if people don't have any better ideas - I might get rid of notifications and have mute microphone command speak microphone status.
Another potential idea is to have a thread check whetehr microphone is muted say every 1 second and announce updates there. But I'm not excited about creating a new thread.

@Adriani90
Copy link
Collaborator

Adriani90 commented Mar 15, 2024 via email

@mltony
Copy link
Contributor Author

mltony commented Mar 15, 2024

Makes sense; then it would be good if people can test if this works correctly on Win11.

@mltony
Copy link
Contributor Author

mltony commented Mar 16, 2024

More testing shows notifications are not reliable on my Windows 11 as well. My configuration is laptop with windows 11; it has two microphones: builtin microphone array, and microphone in my bluetooth headphones.

  1. When Bluetooth microphone is set as default, I get all notifications correctly. From both microphones. I get no repeating notifications.
  2. When builtin microphone array is set as default, then I get no notifications at all. Neither from builtin nor from bluetooth. Pycaw example listener also doesn't get any notifications in this case.

I am biased to conclude that notification API (specifically IMMNotificationClient::OnPropertyValueChanged) is broken on Windows side (also seems like in Windows 11 Microsoft tried to fix it and mostly fixed it, but still not all the way) and we shouldn't rely on it.

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: mute microphone command
10 participants