Disable application volume and mute features when sound split is off or when wasapi is off #16404
Conversation
See test results for failed build of commit 5f97586b17 |
|
@mltony, this PR totally defeats the use of the mute other apps feature, at least in the majority of its use case. The users will use NVDA with sound split disabled most of the time. But during this time, they may still want to mute other apps quickly with a handy shortcut. I understand that there is a correlation in your implementation between sound split and other apps muting or volume changing. But as already written elsewhere, I really think that you should de-correlate these two features. And if possible link other apps volume setting or muting to Windows Volume Mixer instead, while keeping sound split internal to NVDA. I guess that we should really re-clarify why the mute other apps feature was requested, but probably not in this PR... |
|
Decorellating is hard. Since sound split and apps volume use the same API and if we separate them in NVDA codebase, we'll have to deal with all the pain from their inevitable interference. |
I think a path to investigate is to see if it's possible to modify apps mute/volume so that it is reflected in Windows Volume Mixer.
Why do you think that the same reason apply for other apps muting. As I describe in #16402, the use case between other apps muting and sound split are not the same IMO.
Yes, I know, but NVDA_BOTH_APPS_BOTH is not the default setting; and I do not say that it should be. Another strange thing is that if your other apps are muted and you turn back to sound split off, the other apps are unmuted. That's quite a strange and bad UX. |
|
Shall we drop apps mute then? Looks like you're not happy with this implementation and originally I only intended to add adjusting other apps volume specifically for sound split. I only added apps mute as per your request, but now it seems like it might not be that good of a fit here in the end. Then perhaps you or someone else can implement apps mute later using some other technique. WDYT? |
I would at least take it back to the drawing board. Note that #16273 was merged without some comments in #16042 being addressed. In my opinion, there is also a task for NV Access to manage these types of processes and to be very cautious in merging features about which there is doubt among part of the core community. afterwards. |
|
@mltony asked:
Shall we drop apps mute then?
It is my personal opinion that mute makes more sense to keep than volume. I don't think I was convinced of the value of having these in core, but as I am a power user who has no difficulty using Windows Volume Mixer, I figure I am not the target audience for either feature.
But in thinking on this further, I have come to the conclusion that if I was going to use one of them ever, it would most likely be mute, if I was in a situation where audio ducking didn't work (like a portable copy).
So having a "mute everything but NVDA" toggle, that was active only as long as NVDA was active, makes some sense as a screen reader support feature.
We have gone around and around about why people want the volume feature, and I'm not going to bother arguing against that any more. It's done, it's in core, and that's what users wanted for some reason.
|
|
@mltony Honestly muting other apps when Sound Split is disabled is what, IMO, most users might want to do, so if muting would be disabled when Sound Split is off, it'll lose most of its usefulness and appeal. As such, I think if we are supposed to keep muting other apps, it should work regardless of whether or not Sound Split is enabled. If these can't be separated or if issues like #16402 can't be fixed easily, I suggest removing the mute feature and retaining the volume adjustment. |
|
@XLTechie Oddly enough, I think volume adjustment makes more sense compared with muting, but I understand that this is absolutely personal. Ideally both should be retained if issues can be resolved - I mean stuff like restoring the mute state upon exiting, or restarting, NVDA. But if one is to be retained, I think it should be app volume adjustment. |
|
IMO the discussion here is not should we keep mute or volume? Both feature are quite similar and the issues found with one of them will probably be found with the other one. I guess that most of us also agree or at least accept that these features be integrated in core since they answer to the need of many people. Though, there is an on-going discussion in #16402 about reverting the whole feature. And it should be clarified before going further with this PR. |
|
@mltony for another technique to control per-app volume modification or muting, you can have a look at NVDA Sound Manager add-on from @yplassiard. With this add-on, if you modify the volume of other apps or mute them, it is reflected in Windows Volume Manager. |
|
@CyrilleB79, I checked source code of Sound Manager, they just use simple audio volume. I think you're right in the sense that this might allow us to decouple apps volume adjusting from sound splitting. I would need to make a prototype to confirm that ISimpleAudioVolume and IChannelAudioVolume interfaces don't interfere with each other in some weird unexpected way. |
|
It was clear from the beginning of the feature design that the use case is in situations where sound split is used and still the other apps are too loud. There was never the discussion to control Windows volume mixer via NVDA. So I think there was always actually the understanding that these settings are linked to the sound split which is restored after NVDA exit.
I understand this use case, but this happens with audio ducking set to always as well. The volume is restored. At the other end if you are using a computer together with a person with hearing problems, and this person needs the loudness for its tasks, it is really not a good idea to let any negative traces after using NVDA. |
|
Why is there the desire to control the volume mixer at all via NVDA when the mixer itself is accessible? The use case for these settings in NVDA are for on demand purpose and are only linked to the NVDA user. I don't see any arguments to link the volume mixer here. |
|
@CyrilleB79 wrote:
I don't think this is true. If users will discover the benefits of using sound split, they will rather use that. And I think even having this feature to mute other apps when sound split is on it will make this feature more attractive anyway. But yeah, I am not against having that decoupled from sound split if possible as long as it stays a temporary setting until NVDA exits. |
|
@Adriani90 I have discovered the benefits of the Sound Split, but they really don't fit into my daily use case. I guess there will be many users like me. However, I do want to be able to alter the volume or mute other apps via NVDA's easy-to-access key strokes when the Sound Split is disabled. As @CyrilleB79 mentioned, the feature will be utterly defeated if it fails to work with Sound Split disabled. |
|
@amirsol81 as I said, I am not against decoupling as long as it restores after exiting NVDA. |
|
@Adriani90 asked:
At least for me, the reason is this. Consider the following circumstance:
How does the sighted user get audio back? The same question can be asked for volume changes. Further, if an inexperienced NVDA user mutes audio/changes volume accidentally, how is a sighted user (or Microsoft accessibility support, etc.) likely to help, with or without NVDA? By going to the system volume mixer, I would think. So not having these changes synchronized with the standard way that Windows manipulates audio, seems like a degraded UX all around. As an aside: in my other life, I am an audio technician. When there are problems with audio, you always start solving them by following the signal chain. From input to output. In this case, the input is Firefox playing a video, and the output is the speakers. What's between them, is Windows volume mixer. |
|
Can you please re-target this PR to beta |
|
@seanbudd , |
|
Generally you would have to do In this case I just needed to retarget via the GitHub interface, as master is now identical to beta, so no need to take further action. |
NVDA restores it after exit. @mltony re decoupling in my view it is out of scope for this PR and can be handled in a separate discussion. |
If the feature as is is not acceptable and the current pr needs a product decision (i agree with both), then the preferable solution is reverting the feature and providing a new pr that is in line with the product decision to be made. |
|
This PR makes it acceptable, no product decision needed.Von meinem iPhone gesendetAm 18.04.2024 um 08:13 schrieb Leonard de Ruijter ***@***.***>:
@Adriani90
The feature as is now in alpha is not acceptable. So I would say it is important to remove the blocked label and continue working on this.
If the feature as is is not acceptable and the current pr needs a product decision (i agree with both), then the preferable solution is reverting the feature and providing a new pr that is in line with the product decision to be made.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I also agree that sound split and application volume should be decoupled. |
|
In a separate PRVon meinem iPhone gesendetAm 18.04.2024 um 08:27 schrieb Rowen ***@***.***>:
I also agree that sound split and application volume should be decoupled.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
To my question "How do sighted people get volume back?" @Adriani90 answered:
NVDA restores it after exit.
Not currently, but that's only a small part of the problem. Please read the remainder of what I said in #16404 (comment), and answer what is likely to happen if Microsoft disability or another sighted assistant, is trying to help a user restore sound if that user accidentally messed up other apps sound through NVDA? In that case, NVDA will likely still be running, but the SVM will do nothing.
That could lead to long and unnecessary Windows repair actions, driver reinstallation, etc. All because NVDA is using an unusual mechanism to manipulate app volume.
|
|
@Adriani90 you wrote something puzzling:
All this PR does is make mute/volume features only work with sound split enabled. It does not claim to do anything about restoring states after NVDA closes. Can you explain why you say it fixes that issue? Personally, I agree with those who believe that this PR should be closed--if muting and volume changes exist in NVDA, they should work with or without sound split. |
|
This will be solved by this PR as far as I understand. Audio ducking feature does this already and there is no risk NVDA messes up any audio. That problem won‘t exist after this PR is merged.Von meinem iPhone gesendetAm 18.04.2024 um 09:05 schrieb Luke Davis ***@***.***>:
To my question "How do sighted people get volume back?" @Adriani90 answered:
NVDA restores it after exit.
Not currently, but that's only a small part of the problem. Please read the remainder of my previous comment, and answer what is likely to happen if Microsoft disability or another sighted assistant, is trying to help a user restore sound if that user accidentally messed up other apps sound through NVDA? In that case, NVDA will likely still be running, but the SVM will do nothing.
That could lead to long and unnecessary Windows repair actions, driver reinstallation, etc. All because NVDA is using an unusual mechanism to manipulate app volume.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
The title and description of this PR say otherwise. The code, which I have read, doesn't appear to contain any change that would do what you are saying as far as I can tell. |
|
There are many discussions and various open issues/PRs following the mute and volume control for other apps introduced in #16273. This leads to various impacts of this feature being discussed in various places, which is inefficient. This reveals that this feature is not mature enough or that the design and the use cases have not been correctly explained and listed. I think that @LeonarddeR suggestion to revert the feature until a design with clear use cases is discussed and understood by all is the best course of action. I am quite disappointed to request this revert because @mltony has done a great job to push this in core and because his availability period may reach its end, so there is a risk that this feature may not be reintegrated soon if he is not available to work on it anymore. @seanbudd, @gerald-hartig what do you think? |
See description of user facing changes. @mltony writes:
And sound split restores apps volume after NVDA exits as far as I understand. @mltony can you comment on this?
I disagree. Decoupling from sound split can be handled afterwards at a later stage, we don't have to wait for the perfect world until something is merged in my view at least. If decoupling is the only point we have to solve, this can be discussed separately.
This will be the case for lots of new features which might come into NVDA in the future. Things will be discussed in different places. These discussions have been taken place because of 3 things:
I strongly disagree with reverting this. Further improvements can be discussed separately and there are alot of features which were introduced in NVDA earlier and did not have enough discussions with the community, still they are accepted and improvement requests are in place. For example OCR feature, Add-on store, object navigation etc. |
|
Hi, one thing we need to remind ourselves: master branch code was just merged into beta (for documentation file preparation among other reasons) and 2024.2 release blurb was published. On one hand we could ask Tony to rebase his fix on beta branch in May. On the other hand deferring app volume mute would be better to provide a more stable (and slightly more polished) experience for 2024.2 users. Thanks.
|
|
I just squeezed in a fix for#16409 as well here - hope that's ok. As for decoupling aplication volume from sound split, it seems to me that more people are in favor of decoupling. Personally I am biased towards decoupling too: I especially like the argument that by switching to ISimpleAudioVolume mute/volume changes will be reflected in Windows Volume mixer. Unless there are any concerns, I'll try to prepare anotehr PR for beta branch in the next couple of days. Or please LMK if that shouldn't go into beta.
It has been at the drawing board stage for quite a while - #16052 was opened in January. It would seem counterproductive to me to roll back features because of concerns (unless such concerns are critical). Open source projects typically don't move very fast to begin with, so we don't want to have analysis paralysis. |
|
Tony, as long as the reflected changes in the volume mixer are restored after NVDA exit, I don’t see any issue with that. I think the audio ducking feature reflects also changes there but restores them after NVDA exit.
Von: mltony ***@***.***>
Gesendet: Freitag, 19. April 2024 00:03
An: nvaccess/nvda ***@***.***>
Cc: Adriani90 ***@***.***>; Mention ***@***.***>
Betreff: Re: [nvaccess/nvda] Disable application volume and mute features when sound split is off or when wasapi is off (PR #16404)
I just squeezed in a fix for#16409 as well here - hope that's ok.
As for decoupling aplication volume from sound split, it seems to me that more people are in favor of decoupling. Personally I am biased towards decoupling too: I especially like the argument that by switching to ISimpleAudioVolume mute/volume changes will be reflected in Windows Volume mixer. Unless there are any concerns, I'll try to prepare anotehr PR for beta branch in the next couple of days. Or please LMK if that shouldn't go into beta.
@LeonarddeR <https://github.com/LeonarddeR>
I would at least take it back to the drawing board.
It has been at the drawing board stage for quite a while - #16052 <#16052> was opened in January. It would seem counterproductive to me to roll back features because of concerns (unless such concerns are critical). Open source projects typically don't move very fast to begin with, so we don't want to have analysis paralysis.
—
Reply to this email directly, view it on GitHub <#16404 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGVCP4IXZ52LQ5L6DFMQOOLY6A7IVAVCNFSM6AAAAABGJ4CGVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRVGQYDANZSGQ> .
You are receiving this because you were mentioned. <https://github.com/notifications/beacon/AGVCP4KFV2AEPT3TVOTNUTDY6A7IVA5CNFSM6AAAAABGJ4CGVOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT3DOBZI.gif> Message ID: ***@***.*** ***@***.***> >
|
No, the audio ducking has no impact on Windows Volume Mixer. It's considered a change internal to the screen reader, not from a coding point of view, but in consideration to the fact that the audio ducking volume lowering disappears when the screen reader is exited. There is no automatic process modifying the Volume Mixer today, and that's why I am advocating for the volume mixer not being modified automatically when NVDA exits. Since they will be reflected in the Volume Mixer, the other apps volume or mute commands should just be considered convenience shortcut to modify settings in the Volume Mixer in one keystroke. This makes it the simplest and clearest user experience. @mltony by the way, since the volume / mute other apps become convenience shortcuts to control Windows Volume Mixer, I wonders if it's still useful to keep the associated controls in NVDA's GUI (audio panel). Keeping them leads to a greater risk of synchronization issues with the Volume Mixer. You also do not need to consider profile switch or config reset anymore. |
|
In this case I vote for this feature to be an NVDA internal feature like audio ducking as well, due to my real use case above and the negative impact from not restoring volume and mute state after NVDA exit.If restore volume and mute state after NVDA exit is not possible, only in this case I vote for reverting this feature.Von meinem iPhone gesendetAm 21.04.2024 um 22:30 schrieb Cyrille Bougot ***@***.***>:
Tony, as long as the reflected changes in the volume mixer are restored after NVDA exit, I don’t see any issue with that. I think the audio ducking feature reflects also changes there but restores them after NVDA exit.
No, the audio ducking has no impact on Windows Volume Mixer. It's considered a change internal to the screen reader, not from a coding point of view, but in consideration to the fact that the audio ducking volume lowering disappears when the screen reader is exited.
There is no automatic process modifying the Volume Mixer today, and that's why I am advocating for the volume mixer not being modified automatically when NVDA exits. Since they will be reflected in the Volume Mixer, the other apps volume or mute commands should just be considered convenience shortcut to modify settings in the Volume Mixer in one keystroke. This makes it the simplest and clearest user experience.
@mltony by the way, since the volume / mute other apps become convenience shortcuts to control Windows Volume Mixer, I wonders if it's still useful to keep the associated controls in NVDA's GUI (audio panel). Keeping them leads to a greater risk of synchronization issues with the Volume Mixer. You also do not need to consider profile switch or config reset anymore.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Closing in favour of reverting the mute application feature |
|
@seanbudd, |
|
I also hope to reconsider this. The community has a need for this, and Tony is willing to implement this. If further discussion is needed, please let the community know. |
|
Hello everyone. With the public holiday calendar in Australia and various ongoing discussions this week, we have not been able to respond in detail to this issue or #16440. Rather than rush something out and risk a misunderstanding, I wanted to let you know that it's on our list and we'll respond shortly. |
|
@cary-rowen I've updated the description of #16440 and put a comment in there. |
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."
Link to issue number:
Closes #16402
Closes #16409
Summary of the issue:
Mute applications command works incorrectly when sound split is off. or when wasapi is off.
Description of user facing changes
Now muting other apps or adjusting other apps volume only works when sound split is not set to off.
Also speaking error message when wasapi is off.
Description of development approach
Since #16287 I had to introduce explicit sound split off mode, which is not supposed to touch volumes of other applications through wasapi. Since adjusting and muting volume of other apps done through the same logic, disabling it for sound split off mode.
Testing strategy:
Tested the following conditions:
Known issues with pull request:
N/A
Code Review Checklist: