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

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

Closed
mltony opened this issue Jan 16, 2024 · 12 comments · Fixed by #16273 or #16591
Closed

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

mltony opened this issue Jan 16, 2024 · 12 comments · Fixed by #16273 or #16591
Labels
blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. component/audio-ux p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Milestone

Comments

@mltony
Copy link
Contributor

mltony commented Jan 16, 2024

I propose to add command that would adjust volume of all running applications except for NVDA. As usual, I will implement this myself if approved.

Description

This feature is really an integral part of #16051. The rationale is when we are splitting all sounds into NVDA channel and all other apps channel, there must be a way to control the volume of both channels separately.
As for UI, I see there are two options:

  1. To be consistent with NVDA volume, we can add a new setting in synth settings ring. The drawback of this is that it's not logical since this setting has nothing to do with synth settings.
  2. Alternatively, we can create a separate command for that, say NVDA+Alt+PageUp/PageDown.

Prior discussion

#16037

Technical details

Technical detail 1 from #16051 applies here as well as we need to call the same wasapi COM interface.

@mltony mltony mentioned this issue Jan 20, 2024
5 tasks
@seanbudd seanbudd added the triaged Has been triaged, issue is waiting for implementation. label Jan 22, 2024
@seanbudd
Copy link
Member

This setting should go in audio settings and have a separate command

@seanbudd seanbudd added component/audio-ux p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority labels Jan 22, 2024
@CyrilleB79
Copy link
Collaborator

Please also add a "mute all other apps" command. It would be very useful in the following use case: I opening various webpages or browser instances and suddenly in one of them, an ad or another video starts shouting out. It's very difficult with NVDA to find in which webpage the ad is shouting and to close it.

@Adriani90
Copy link
Collaborator

I am in favor of renaming the synth settings ring to "settings ring" and start improving it by adding such settings like this to it.
There could be two volume settings in the ring:

  • Synth volume and
  • Apps volume

@Adriani90
Copy link
Collaborator

The synth volume is already part of the ring.

@CyrilleB79
Copy link
Collaborator

I am in favor of renaming the synth settings ring to "settings ring" and start improving it by adding such settings like this to it. There could be two volume settings in the ring:

  • Synth volume and
  • Apps volume

The settings ring may be a good idea and is actually already tracked in #7747. But it has not even been triaged (you may ask NV Access for this); and it seems that neither NV Access nor anybody else is likely to implement it in the near future.

However, #7747 should not block the current issue at all. Tony is available these days and offers to implement the current issue; we should not miss this opportunity because of an endless discussion on the ring.
When the current issue is implemented and merged, it is still possible to add it to a settings ring if and when it see the light of the day.

@LeonarddeR
Copy link
Collaborator

Similar to #16056, I'd personally vote for this to be part of an add-on rather than part of core.

@Adriani90
Copy link
Collaborator

After giving it further thoughts in this case I think I agree with @LeonarddeR. We have already the audio ducking feature in NVDA which kind off serves for purposes of this use case. If there is something we can look into, then maybe to add a setting to configure by which percentage the volume of other audio output should be reduced when audio ducking is on.

@LeonarddeR
Copy link
Collaborator

See also #16292 . I'm very concerned that a feature like changing application volume will cause unwanted side effects.

@LeonarddeR LeonarddeR added the blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. label Mar 12, 2024
@mltony
Copy link
Contributor Author

mltony commented Mar 12, 2024

@Adriani90, @LeonarddeR,
This feature is an integral part of sound split. If we do sound split, there must be a way to adjust volume of left and right channels separately. Without this feature sound split might not be usable. So as long as sound split is part of core, this one should also be part of core.

@Adriani90
Copy link
Collaborator

@mltony when audio ducking is on while sound splitting NVDA from other applications, the volume decreases on the one side automatically when NVDA is speaking. Or am I wrong? Why should we adjust the volume of other applications separately?
If a configurable volume for audio ducking is implemented, this should solve your use case and avoid having the same problem as #16292. Please correct me if I misunderstood something.

@mltony
Copy link
Contributor Author

mltony commented Mar 13, 2024

The whole point of sound splitting is to be able to consume two streams of audio at the same time. My use case is attending boring online meetings, where you kind of have to listen to what's going on, but at the same time it's not interesting or not relevant enough to occupy your whole attention, so I am trying to work at the same time - like writing code. But when someone mentions your name you still need to catch that. That's why I need to balance the volume on the left and on the right. Ducking doesn't help me at all in this case, because I don't want to reduce volume of other apps. Ducking essentially implies that you are consuming only one stream of audio at a time, whereas sound split is for people who need to consume both streams at at the same time.

seanbudd pushed a commit that referenced this issue Apr 16, 2024
Closes #16052.

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.
@nvaccessAuto nvaccessAuto modified the milestone: 2024.2 Apr 16, 2024
seanbudd added a commit that referenced this issue May 10, 2024
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."
@mltony
Copy link
Contributor Author

mltony commented May 23, 2024

This should be reopened as #16273 was reverted in commit f63841f.

@github-actions github-actions bot added this to the 2025.1 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. component/audio-ux p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
6 participants