Keystrokes to adjust applications volume and mute #16273

mltony · 2024-03-07T01:08:54Z

Link to issue number:

Summary of the issue:

Needd commands to adjust volume of all applications other than NVDA.

Description of user facing changes

Adding commands to adjust volume of all applications other than NVDA bound to nvda+alt+pageUp/pageDown.
Also adding a command to mute all applications other than NVDA bound to nvda+alt+delete.
Also adding corresponding settings to audio panel.

Description of development approach

Making use of sound split code - we already have code to set volume of all other apps, but it's only used to mute left or right channel. Reusing same code to adjust volume as well.

Testing strategy:

Tested manually.

Known issues with pull request:

N/A

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.

AppVeyorBot · 2024-03-07T03:39:54Z

FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
PASS: Unit tests.
PASS: Lint check.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/pabqh08b19rq8sn4/artifacts/output/nvda_snapshot_pr16273-31341,dd9a25b6.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.1,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 27.5,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 2.4,
FINISH_END 0.2

See test results for failed build of commit dd9a25b6f1

amirsol81 · 2024-03-07T05:51:57Z

@mltony Thanks for implementing these highly requested items. One note, though:
NVDA+ALT+PageUp and PageDown are already used by NVDA Remote and Tele NVDA. I suggest SHIFT+NVDA+PageUp and PageDown.

CyrilleB79 · 2024-03-07T07:56:43Z

@mltony have you tested this feature in combination with Sound split feature?
E.g. Split sound, change volume, then unsplit sound, etc.

@amirsol81 we may consider key conflicts with very popular add-ons, but remember that at the end NVDA may be favoured and the add-ons need to adapt if there is no other acceptable gesture found for NVDA.
The gestures you suggest are already used in NVDA laptop layout.

cary-rowen · 2024-03-07T08:29:50Z

Hi @mltony

Demands related to volume adjustment and audio device control are very popular.

So @huaiyinfeilong from the Chinese community developed the Audio Manager add-on.

There are several features that we consider indispensable.

Set an audio output/input device as the system default playback/recording device. This allows users with multiple sound cards to freely manage their devices in different situations.
Set up output/input devices for different applications, for example I can set the browser's output device to a connected bluetooth speaker and keep the screen reader on the laptop speaker since bluetooth devices have higher latency.

Maybe you or others can get some inspiration from it.

Thanks
Cary

CyrilleB79 · 2024-03-07T08:39:04Z

@cary-rowen, the volume adjustment and muting of other apps as presented and implemented by @mltony has already been triaged (validated) by NV Access and already fulfills the need of many users.

I understand that some users may need more specific features as described by you. I'd recommend to discuss this in a specific issue rather than in this PR. I think that there is already one opened, but if not, you can open a new one.

If your issue is accepted (triaged), it may be implemented in a subsequent PR and maybe Tony is willing to do this. If it is rejected as a core feature because considered to target too few users, you still can ask the imploementation of a clean interface (specific functions, extension points, etc.) to ease the interfacing of the add-on (e.g. no patch).

cary-rowen · 2024-03-07T08:42:57Z

@CyrilleB79
I'll try creating a Feature Request to see if anyone else is interested.

mltony · 2024-03-07T19:16:00Z

@amirsol81,
Happy to change to whatever keystrokes people agree upon. But I think Sean is likely to ask me to remove default keystrokes all together.

@CyrilleB79

have you tested this feature in combination with Sound split feature?

Yes, both features use the same codepath, there are no issues as far as I can see.

@cary-rowen
I personally don't see Audio Manager being useful for myself - I don't switch audio devices often. But I agree if there is enough demand - the question of including this into core can be raised. Also I looked at its github repo - it seems to be using a lot of native code for which I couldn't easily find source code - so you might have to contact the author if he is willing to do the job himself.

CyrilleB79

Thanks for this very useful PR.

Here is my feedback. Except for English grammar, let me know if you find some remark not relevant and why.

Also I have two open questions that I'll ask separately so that it can be discussed out of this review.

user_docs/en/changes.t2t

user_docs/en/userGuide.t2t

CyrilleB79 · 2024-03-08T08:04:55Z

user_docs/en/userGuide.t2t

+==== Mute other applications ====[MuteApplications]
+
+This checkbox allows to mute all applications other than NVDA.
+If any new applications start to output sound, their sound output will also be muted.


Same as previous paragraph; feel free to accept or decline.

Suggested change

If any new applications start to output sound, their sound output will also be muted.

If any new applications start to output sound, its sound output will also be muted.

source/gui/settingsDialogs.py

user_docs/en/userGuide.t2t

CyrilleB79 · 2024-03-08T09:09:55Z

Three more general questions regarding the user experience.

I understand that Other apps volume is stored in the config, so that the preferred volume can be memorized for subsequent runs of NVDA.
On the contrary, memorizing the mute other apps in the config seems useless to me and even risky:

if a user mutes other apps without noticing, e.g. when no other sound is played, and then the config is saved, we may end up with bug reports such as "NVDA breaks the audio of other applications"
Is there a use case to memorize the mute state in the config? I imagine mute as a temporary feature, but no one with the other apps sound muted permanently.

The original sound level is not restored when NVDA exits. Is it on purpose?
Note that I have no strong opinion on this topic, but it seems quite strange to have on one hand this volume set when NVDA start but not restored when it exits.
IMO:

either the volume should not be stored at all in the config and the shortcuts would only allow to permanently modify the volume which remains as is when NVDA exits, or when NVDA's config is reset (NVDA+control+R)
or the volume is stored in the config and it is set when NVDA starts, exits or its config is restored (NVDA+control+R)

What is the link with the volumes in the Windows volume mixer?

cary-rowen · 2024-03-08T09:27:10Z

Sorry, I haven't tested this yet, but I saw @CyrilleB79's comment and I felt the need to give my opinion:

Imo, adjusting the volume of other applications should be in sync with the system's volume synthesizer, whether it is mute state or volume, and conversely, when the user adjusts the volume or mute state of an app using the system volume synthesizer, NVDA should also Reflect this.

Regarding what I call "system volume synthesizer", you can find it by pressing Win+B to find the speaker icon, pressing the Application key to pop up the menu, or open a similar application volume management by executing the settings URI in the run dialog(ms-settings:apps-volume).

The AudioManager add-on I mentioned in my previous comment does this. Doing this ensures a consistent user experience.

I'll test this PR later and if there's anything I've misunderstood in the above comment I'll collapse it.

Thanks
Cary

XLTechie

Some responses to @CyrilleB79's suggestions, and a few of my own.

user_docs/en/changes.t2t

user_docs/en/userGuide.t2t

XLTechie · 2024-03-08T09:51:30Z

A general comment about the documentation (apart from me still not being sure why sndvol.exe or other Windows methods are insufficient for this purpose): Regardless of how it is phrased, the control descriptions say something like: " This slider allows to set volume of all currently running applications except for NVDA. " Note the words "currently running". But then, Cyrille and I are debating in the line comments, about how to indicate that this also effects "all new applications" that are started. Why does it bother to say "currently running", if it also includes "running in the future"? Why isn't it just "all applications other than NVDA"? The only reason you have to make clear "new applications" later, is because you started from the prospective of only "currently running applications".

CyrilleB79 · 2024-03-08T11:47:49Z

(apart from me still not being sure why sndvol.exe or other Windows methods are insufficient for this purpose):

Because when you open a video (or an add) on internet, if the volume is too loud with respect to NVDA, it will be difficult to go to sndvol.exe and find the slider or the checkbox of your browser to lower or mute the volume of your browser.
Could you explain how you manage this situation?

I agree that if possible, interacting with Windows mixer's volumes would be better.

codeofdusk · 2024-03-08T18:34:32Z

Because when you open a video (or an add) on internet, if the volume is too loud with respect to NVDA, it will be difficult to go to sndvol.exe and find the slider or the checkbox of your browser

In this situation, I use NVDA+Shift+d to duck down that audio, then adjust if needed.

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

mltony · 2024-03-16T20:23:47Z

@CyrilleB79,

memorizing the mute other apps in the config seems useless to me

I changed this; now I restore muted to False at startup.

The original sound level is not restored when NVDA exits.

Fixed

What is the link with the volumes in the Windows volume mixer?

In my understanding there is no link. My guess is that the volume in volume mixer can be adjusted using ISimpleAudioVolume interface and we're using IChannelAudioVolume. But I haven't tested this hypothesis.

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

XLTechie · 2024-03-18T00:54:18Z

In my understanding there is no link. My guess is that the volume in volume mixer can be adjusted using ISimpleAudioVolume interface and we're using IChannelAudioVolume. But I haven't tested this hypothesis.

Doesn't this just confuse the situation for users even more? CC @jcsteh

seanbudd · 2024-04-15T06:29:02Z

Thanks @mltony - can you reserve merge conflicts?

Qchristensen

Looks good.

This reverts commit 275ab4d.

Reverts PR Reverts #16273 Issues fixed Fixes #16409 Fixes #16402 Issues reopened #16052 Brief reason for revert We started with mltony creating: #16051 Feature request: Sound split Which was a duplicate of: #12985 Audio settings with stereo headset (or speakers) - Send NVDA sounds on one side and the rest of Windows sounds on the other side And then implemented by: #16071 Sound split mltony also created: #16052 Feature request: add command to adjust volume of all applications except for NVDA Which was implemented in: #16273 Keystrokes to adjust applications volume and mute This PR was approved and merged and was then found to cause issues: #16402 Unmuting other apps does not work as expected #16409 Apps mute and volume features work very unexpectedly with WASAPI disabled Due to these issues and the considerable debate on the approach, the above PR #16273 was reverted by: #16440 As an alternative to the revert #16440 to resolve the 2 issues (#16402, #16409) and keep #16273, mltony created: #16404 The question now becomes, how do we proceed from here? NV Access's position is that the sound split functionality (#16051) is a useful feature to add into core. However, due to the following reasons, we believe that further work on the volume adjustment features (#16273, #16404) to improve the UX is required on a branch (off master/alpha) before it can be added back in: Windows sound mixer has reasonable accessibility. Sound split on its own provides value to users. The UX of swapping between NVDA volume control and windows volume control needs to be resolved. The UX of resolving volume issues due to NVDA crashes needs to be resolved. As one of the contributors on the threads said, "So now there are two mixers in the chain, one of which can be invisible, and overrides the other, or makes its settings relative instead of absolute."

Closes #16052. Please note that this is my second attempt to implement this. The first attempt was #16273 which was reverted in #16440# by commit f63841f. Summary of the issue: Provide a way to adjust volume of other applications, so that when using sound split the volume of aplications channel can be adjusted separately from NVDA volume. Description of user facing changes New keystrokes: NVDA+alt+pageUp/pageDown adjusts volume of all other applications. NVDA+alt+delete Cycles between three states Disabled (default) Enabled Muted - all applications except NVDA will be muted Description of development approach I refactored sound split code. Sound split and applications volume adjustment need to use same callbacks in a similar fashion. In order to reduce code reduplication, I extracted common logic and put it in audio/utils.py. Now it provides an abstract class AudioSessionCallback and customers (sound split and app volume adjuster) are implementing this class. Two methods that need to be implemented are onSessionUpdate and onSessionTerminated, which operate on a single audio session. All the plumbing is hidden in utils.py so that clients don't need to worry about setting up and dealing with multiple lower-level callbacks. Applications' voluem adjusting logic is implemented in audio/appsVolume.py and is quite straightforward now. Volume and mute status are adjusted via ISimpleAudioVolume interface, which means that both are reflected in Windows Volume Mixer. 4.Initial volume and mute status of applications is storedand upon exit will be restored. Applciation volume adjustment is now completely independent of sound split and both features can operate independently of each other.

Keystrokes to adjust applications volume and mute

6afcc0c

Translation string

47fbe7b

mltony marked this pull request as ready for review March 7, 2024 19:16

mltony requested review from a team as code owners March 7, 2024 19:16

mltony requested review from Qchristensen and SaschaCowley March 7, 2024 19:16

CyrilleB79 reviewed Mar 8, 2024

View reviewed changes

XLTechie suggested changes Mar 8, 2024

View reviewed changes

mltony and others added 9 commits March 8, 2024 11:10

Update user_docs/en/changes.t2t

13b4b37

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update user_docs/en/userGuide.t2t

13221d8

Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>

Update user_docs/en/userGuide.t2t

b0e7e1e

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update user_docs/en/userGuide.t2t

7570604

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/gui/settingsDialogs.py

1cf708c

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/gui/settingsDialogs.py

6eb08ea

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update user_docs/en/userGuide.t2t

35c7d5f

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update user_docs/en/userGuide.t2t

e55cf52

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update user_docs/en/userGuide.t2t

92c05a6

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

seanbudd marked this pull request as draft March 14, 2024 04:43

mltony and others added 5 commits March 16, 2024 11:05

Update user_docs/en/userGuide.t2t

7063e81

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/globalCommands.py

ec8f50d

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/globalCommands.py

7ffb178

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/globalCommands.py

48ed11d

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

Update source/gui/settingsDialogs.py

193faaf

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

mltony and others added 5 commits March 16, 2024 13:30

Update user_docs/en/changes.t2t

9314e87

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update user_docs/en/userGuide.t2t

a2402ad

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update user_docs/en/userGuide.t2t

7f91b72

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update user_docs/en/userGuide.t2t

a193e64

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Update user_docs/en/userGuide.t2t

fc1f399

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

mltony marked this pull request as ready for review March 16, 2024 20:32

seanbudd approved these changes Apr 15, 2024

View reviewed changes

Qchristensen approved these changes Apr 15, 2024

View reviewed changes

Merge branch 'master' into appsVolume

9fc24de

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Apr 16, 2024

seanbudd merged commit 275ab4d into nvaccess:master Apr 16, 2024
1 check passed

nvaccessAuto added this to the 2024.2 milestone Apr 16, 2024

CyrilleB79 mentioned this pull request Apr 16, 2024

Unmuting other apps does not work as expected #16402

Closed

LeonarddeR mentioned this pull request Apr 17, 2024

Disable application volume and mute features when sound split is off or when wasapi is off #16404

Closed

5 tasks

seanbudd added a commit that referenced this pull request Apr 23, 2024

Revert "Keystrokes to adjust applications volume and mute (#16273)"

f63841f

This reverts commit 275ab4d.

seanbudd mentioned this pull request Apr 23, 2024

Revert "Keystrokes to adjust applications volume and mute" #16440

Merged

mltony mentioned this pull request Apr 24, 2024

Sound split: add an ability to adjust applications volume separately from NVDA volume #16444

Closed

This was referenced May 23, 2024

Feature request: add command to adjust volume of all applications except for NVDA #16052

Closed

Keystrokes to adjust applications volume and mute [take 2] #16591

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keystrokes to adjust applications volume and mute #16273

Keystrokes to adjust applications volume and mute #16273

mltony commented Mar 7, 2024

AppVeyorBot commented Mar 7, 2024

amirsol81 commented Mar 7, 2024

CyrilleB79 commented Mar 7, 2024

cary-rowen commented Mar 7, 2024

CyrilleB79 commented Mar 7, 2024

cary-rowen commented Mar 7, 2024

mltony commented Mar 7, 2024

CyrilleB79 left a comment

CyrilleB79 Mar 8, 2024

CyrilleB79 commented Mar 8, 2024 •

edited

Loading

cary-rowen commented Mar 8, 2024

XLTechie left a comment

XLTechie commented Mar 8, 2024 via email

CyrilleB79 commented Mar 8, 2024

codeofdusk commented Mar 8, 2024

mltony commented Mar 16, 2024

XLTechie commented Mar 18, 2024 via email

seanbudd commented Apr 15, 2024

Qchristensen left a comment

	If any new applications start to output sound, their sound output will also be muted.
	If any new applications start to output sound, its sound output will also be muted.

Keystrokes to adjust applications volume and mute #16273

Keystrokes to adjust applications volume and mute #16273

Conversation

mltony commented Mar 7, 2024

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

AppVeyorBot commented Mar 7, 2024

amirsol81 commented Mar 7, 2024

CyrilleB79 commented Mar 7, 2024

cary-rowen commented Mar 7, 2024

CyrilleB79 commented Mar 7, 2024

cary-rowen commented Mar 7, 2024

mltony commented Mar 7, 2024

CyrilleB79 left a comment

Choose a reason for hiding this comment

CyrilleB79 Mar 8, 2024

Choose a reason for hiding this comment

CyrilleB79 commented Mar 8, 2024 • edited Loading

cary-rowen commented Mar 8, 2024

XLTechie left a comment

Choose a reason for hiding this comment

XLTechie commented Mar 8, 2024 via email

CyrilleB79 commented Mar 8, 2024

codeofdusk commented Mar 8, 2024

mltony commented Mar 16, 2024

XLTechie commented Mar 18, 2024 via email

seanbudd commented Apr 15, 2024

Qchristensen left a comment

Choose a reason for hiding this comment

CyrilleB79 commented Mar 8, 2024 •

edited

Loading