nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Unmuting other apps does not work as expected

Open CyrilleB79 opened this issue 4 months ago • 23 comments

Steps to reproduce:

  1. Launch NVDA and an audio stream in the background, e.g. youtube playing music in a browser
  2. Press NVDA+alt+delete to mute all apps except NVDA. As expected, Youtube is muted.
  3. Exit NVDA
  4. Start NVDA again
  5. Press NVDA+alt+delete
  6. Press NVDA+alt+delete

Actual behavior:

  1. Youtube stream is muted
  2. Youtube stream remains muted
  3. Youtube stream remains muted
  4. NVDA says "Applications muted" and Youtube stream remains muted
  5. NVDA says "Applications unmuted" and Youtube stream is restored (unmuted)

Expected behavior:

At least, at step 5, NVDA should already know that the other apps are muted and it should unmute them.

I really think that NVDA should not store in its config nor manage an additional muted state. For clarity it should be able to mute and unmute directly the sound as configured in Windows Volume mixer. Else people may have muted through the volume mixer and can wonder why they cannot unmute through NVDA. In other words, putting two switches serially leads to a confusing user experience.

I have not tested but we may encounter the same issues with other apps volume management commands.

In #16273, you indicate that you are using the same framework for mute other apps or other apps volume commands than in sound split. IMO muted state and volume of ther apps should not be stored in NVDA's config, nor should they be restored or re-configured at NVDA exit/startup. They should just be a more easy way to control Windows Mixer's parameters. On the other side, Split mode should be set up according to NVDA's config and disabled when NVDA exits since it's a feature that only has sense when NVDA is active; thus it makes sense to manage it internally to NVDA and Windows Volume Mixer should not be touched for Sound Split feature.

NVDA logs, crash dumps and other attachments:

None

System configuration

NVDA installed/portable/running from source:

From source

NVDA version:

Last alpha, commit 3d4a0a8f7b1d124231e9774d30d07cc549dac3fc.

Windows version:

Windows 10 22H2 (AMD64) build 19045.4170

Name and version of other software in use when reproducing the issue:

N/A

Other information about your system:

N/A

Other questions

Does the issue still occur after restarting your computer?

Not tested.

Have you tried any other versions of NVDA? If so, please report their behaviors.

No; new feature

If NVDA add-ons are disabled, is your problem still occurring?

Yes

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

Not tested

CyrilleB79 avatar Apr 16 '24 12:04 CyrilleB79

Cc @amirsol81, @cary-rowen, @XLTechie, @codeofdusk, @brunoprietog, @Adriani90 who have discussed in Tony's PR.

CyrilleB79 avatar Apr 16 '24 12:04 CyrilleB79

cc: @mltony Cyrille wrote:

IMO muted state and volume of ther apps should not be stored in NVDA's config,

I second this.

nor should they be restored or re-configured at NVDA exit/startup.

Actually in my view volume and mute / unmute should be restored after exiting NVDA, similar to sound split unless the user changed the volume and mute / unmute state manually from the volume manager in Windows. When changing the settings from NVDA, ideally the settings should be temporary, similar to e.g. sleep mode.

Adriani90 avatar Apr 16 '24 14:04 Adriani90

cc: @mltony

Thanks Adriani for this Cc. And Sorry Tony for having forgotten you, the main needed person.

@Adriani90 wrote:

Actually in my view volume and mute / unmute should be restored after exiting NVDA, similar to sound split unless the user changed the volume and mute / unmute state manually from the volume manager in Windows. When changing the settings from NVDA, ideally the settings should be temporary, similar to e.g. sleep mode.

IMO, having a condition such as the following creates a bad UX with corner cases:

unless the user changed the volume and mute / unmute state manually from the volume manager in Windows

If the restoration is done only in some cases, the user may ask why restoration is not always performed. E.g. if the user mute/unmute through Windows Mixer, thus turning back to original state, and then mute again through NVDA, what should happen?

So restoration should happen or always, or never.

I prefer never restoring. The use case is as follow: I use NVDA and have set my other apps volume quite low (e.g. 5%) to continue having music played in the background. During my tasks, I install an add-on and restart NVDA. As soon as NVDA exits, the music begins shouting loud.

More generally, I think that other apps volume and mute should only be modified when the user explicitly modifies them, i.e. through NVDA commands, or Windows Volume Mixer. They should not be modified by surprise on general actions such as NVDA start, NVDA exit, config reset, add-ons reloading, etc.

On the opposite, for the sound split configuration, it makes sense to load and restore its configuration because it is useful only when NVDA is active; moreover, there is no volume change, so there is no risk of unexpected shouting or muting.

CyrilleB79 avatar Apr 16 '24 15:04 CyrilleB79

I also suggest that the volume be restored as NVDA exits.

amirsol81 avatar Apr 16 '24 17:04 amirsol81

Moreover, the documentation should clearly mention the fact that the new key strokes to turn up/down the volume of other apps may very well conflict with the key strokes used by well-established add-ons, name NVDA Remote and Tele NVDA Remote. I myself altered those key strokes for Tele NVDA Remote so as to gain access to the new volume-related ones.

amirsol81 avatar Apr 16 '24 17:04 amirsol81

I disagree to documenting this. Add-ons must react to core changes. Core changes have priority and we will never be able to detect all addons that have conflicting key strokes. One solution might be to let NVDA check if an addon adds  conflicting gestures after installing it, and if there is any conflict found, ask the user via a prompt dialog whether the addon should still be enabled or stay disabled until the user explicitly enables it or solves the conflict manually.But this is another issue. Von meinem iPhone gesendetAm 16.04.2024 um 19:58 schrieb amirsol81 @.***>: Moreover, the documentation should clearly mention the fact that the new key strokes to turn up/down the volume of other apps may very well conflict with the key strokes used by well-established add-ons, name NVDA Remote and Tele NVDA Remote. I myself altered those key strokes for Tele NVDA Remote so as to gain access to the new volume-related ones.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

Adriani90 avatar Apr 16 '24 18:04 Adriani90

@Adriani90 So sorry to say this, but it seems that you are new to NVDA and haven't seen the issues surrounding some add-ons - NVDA Remote in this case. NVDA Core definitely has the priority, but what happens if, for whatever reason, add-on developers don't act as swiftly as required, or don't act at all? Or what happens if, for whatever reason, users don't update the add-ons in a timely manner? I'm afraid simply ignoring that and not mentioning this important issue might affect the users of 2 popular add-ons, and it will result in a bad rap. In an ideal world, all add-ons should have been updated to become compatible with 2024.1, but even some of them haven't been updated despite multiple warnings and user requests.

amirsol81 avatar Apr 16 '24 18:04 amirsol81

To be honest, I was unpleasantly surprised again by the #16273 merge, so I'm not surprised this issue is a result of that. At step 3 in @CyrilleB79's str, the apps should be unmuted IMO.

@Adriani90 wrote:

I disagree to documenting this. Add-ons must react to core changes. Core changes have priority and we will never be able to detect all addons that have conflicting key strokes.

I agree that add-ons must react to core changes at some point, but preferably only with API breaking releases.

LeonarddeR avatar Apr 16 '24 18:04 LeonarddeR

This issue is not intended to list all what you find wrong with #16273.

@Adriani90, @amirsol81 , @LeonarddeR: To discuss shortcut keys added by #16273 and key conflict with add-ons, please open a new issue. This has no link with the initial description of this issue. Thanks.

For clarity, I have changed the title of this issue to make it more precise.

CyrilleB79 avatar Apr 16 '24 20:04 CyrilleB79

@Adriani90 wrote:

Actually in my view volume and mute / unmute should be restored after exiting NVDA, similar to sound split unless the user changed the volume and mute / unmute state manually from the volume manager in Windows. When changing the settings from NVDA, ideally the settings should be temporary, similar to e.g. sleep mode.

@amirsol81 wrote:

I also suggest that the volume be restored as NVDA exits.

@LeonarddeR wrote:

To be honest, I was unpleasantly surprised again by the #16273 merge, so I'm not surprised this issue is a result of that. At step 3 in @CyrilleB79's str, the apps should be unmuted IMO.

Well, you have all given your opinion and you all want the sound to be restored when NVDA exits, which is the opposite of my suggestion to leave other apps sound (volume and mute) unchanged when NVDA exit. I have made the effort to explain why and to provide a use case / user story. Could you please do so, so that we can discuss on a constructive basis? Thanks.

CyrilleB79 avatar Apr 16 '24 20:04 CyrilleB79

I agree with @Adriani90 and @LeonarddeR that apps muted by NVDA should be restored to unmute on NVDA exit; though maybe put it behind a default-to-checked checkbox in audio settings. I would not restore volume though because of what @CyrilleB79 described.

If NVDA mutes apps, it is not reflected in the Windows Volume Mixer. That leaves users no way, outside NVDA, to restore the unmuted state. While that is a really bad UX IMO, it is what NV Access agreed to, so fine. But given that, it is even worse to leave users with an impossibility to restore sound except by running NVDA.

IMO, these are the sort of weird problems we run into when including this kind of global system manipulation in core. There isn't going to be a "best" solution to this, only figuring out which one is "least bad for most situations".

XLTechie avatar Apr 17 '24 03:04 XLTechie

I'm afraid this is all mustard after the meal. We are now actually discussing things that should be on the drawing board before even considering merging the feature. We can continue with this now, but it may be better to follow the steps below.

  1. Revert https://github.com/nvaccess/nvda/pull/16273
  2. Close this issue as well as #16404
  3. Reopen #16052, thereby discussing the following:
    • Should this be in core or in an add-on?
    • Why is this supposed to be an integral part of sound split, as @mltony stated in https://github.com/nvaccess/nvda/issues/16052#issuecomment-1992615194 ?
    • What should be the behavior for the several sound split modes?
    • Should there be default keyboard shortcuts? If so, what would be the impact of someone accidentally touches one of these shortcuts without being able to revert the changes?
    • Just discuss every unexpected outcome you can think of
  4. Wait for explicit approval from NV Access about the proposal, and collect additional feedback about the proposal
  5. Provide a new pr that implements the proposal
  6. Perform very intensive testing before merging it

LeonarddeR avatar Apr 17 '24 05:04 LeonarddeR

I don‘t think this complicated process is needed in such an early alpha stage. Tony seems to work very well and responsive to solve problems discovered after merge and I am sure he has already ideas to provide the desired user experience. If the issues are not solved in the Beta stage at latest, NV Access can revert or postpone the milestone to a later stable version before issuing the next RC. Actually we will never be able to test anything before merging because not all active NVDA people here on Githhub have time to review PRs.

Adriani90 avatar Apr 17 '24 05:04 Adriani90

@LeonarddeR I disagree with reverting the volume/mute change. These are great features for the core, and the issues can be resolved during the alpha/beta stage, and that's also why we have alpha/beta stages anyway.

amirsol81 avatar Apr 17 '24 05:04 amirsol81

I disagree with both @Adriani90 and @amirsol81. Note that my disagreement has nothing to do with any doubt about anyone's competencies. However, by maintaining the current code, we have the risk that various pull requests will now have to be made that will serve as a stopgap. Then this feature becomes a kind of cake where there are things missing from the recipe, and instead of improving the recipe and baking a new, complete cake, we try to improve the cake without looking at the core of the cake again. In other words, it is much better for the quality of the code to reconsider the whole thing than to keep reviewing chunks of new code that are difficult to place in the whole of the feature.

LeonarddeR avatar Apr 17 '24 06:04 LeonarddeR

@LeonarddeR If your argument is to be applied to this, many existing NVDA features should be pulled because of the issues they generated - issues which haven't been fixed so far. Features like #14583 which generated unresolved bugs or issues like #14779 . Now I'm not saying your concern isn't valid at all. Rather, it seems that NVAccess has been following this norm for a long time. Moreover, the features we are discussing, volume/mute, are, IMO, very useful ones for the core.

amirsol81 avatar Apr 17 '24 06:04 amirsol81

@LeonarddeR wrote:

  1. Revert Keystrokes to adjust applications volume and mute #16273

This feature is in an Alpha version, a period of gathering community feedback, and its existing flaws don't appear to be enough to constitute a serious regression. We deserve to fix it rather than revert it and get stuck in endless discussions. btw, @mltony, who introduced the feature, also seems to be actively listening to community feedback and making quick fixes.

  1. Perform very intensive testing before merging it

I think the real time for intensive testing is after the PR has been merged, which is what we do now, rather than before the merge, where only a few power users could test.

Finally, I think the only people participating in GitHub discussions seem to be familiar faces, and to some extent it can also be assumed that it is always some power users discussing.

Should there be default keyboard shortcuts? If so, what would be the impact of someone accidentally touches one of these shortcuts without being able to revert the changes?

I agree, this is worth discussing.

Why is this supposed to be an integral part of sound split, as @mltony stated in Feature request: add command to adjust volume of all applications except for NVDA #16052 (comment) ?

I think this shouldn't be part of the sound split.

cary-rowen avatar Apr 17 '24 06:04 cary-rowen

@amirsol81 wrote:

@LeonarddeR If your argument is to be applied to this, many existing NVDA features should be pulled because of the issues they generated - issues which haven't been fixed so far.

Not necessarily. I think there are several differences with the mute feature and the link reporting feature your mention, like:

  1. I don't think there was as much discussion about link reporting as there was about muting and such topics.
  2. As mentioned, there were several open ends in https://github.com/nvaccess/nvda/issues/16052 . These should at least have been solved before merging the mute feature.

@cary-rowen please consider my rationale in https://github.com/nvaccess/nvda/issues/16402#issuecomment-2060448673 . In short, I'm convinced that reverting the feature, providing a pull request that solves all the concerns is much better than providing fixups, as it forces reviewers to consider the complete code change. There is a pivot point somewhere where we have to say, the current feature introduces unexpected bugs but has a solid ground, or the current feature has elements that introduce bugs inevitably, since it has not been carefully enough considered which edge cases could occur. IMO, The latter is the case here. And again, this is not to blame @mltony for this. NV Access must monitor this equally well.

LeonarddeR avatar Apr 17 '24 07:04 LeonarddeR

@LeonarddeR Well, all I can say is that the issues surrounding NVDA's failure to report the destination for all link types were extensively discussed - both on Mastodon and in the Comments section. However, at the end of the day, issues still exist, but now they seem to have been forgotten with both the generation of new issues and the original developer(s)'s lack of interest in the matter. Strictly speaking, I do agree with your assessment, but given the mostly volunteer-oriented nature of development for NVDA, many new features do suffer from the discussed malady.

amirsol81 avatar Apr 17 '24 07:04 amirsol81

@CyrilleB79 wrote:

If the restoration is done only in some cases, the user may ask why restoration is not always performed. E.g. if the user mute/unmute through Windows Mixer, thus turning back to original state, and then mute again through NVDA, what should happen?

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 use NVDA and have set my other apps volume quite low (e.g. 5%) to continue having music played in the background. During my tasks, I install an add-on and restart NVDA. As soon as NVDA exits, the music begins shouting loud.

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.

Adriani90 avatar Apr 17 '24 20:04 Adriani90

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.

No, it was not clear at all. I agree that the initial use case described when #16052 was opened was the one of sound split. But the discussion in the same issue shows that there were broader expectations of this feature, not only the Sound Split use case. Not mentioning the additional discussions now that it is merged. And the fact that there are many add-ons in the wild where this feature is implemented independently from sound split is an additional demonstration of this expectation.

As written elsewhere, I am a bit disappointed to request this, but I agree with @LeonarddeR: I think that reverting the feature and re-discussing clearly the use cases, the expectations and the design of this feature is the best course now.

CyrilleB79 avatar Apr 18 '24 09:04 CyrilleB79

We plan to revert the mute applications feature for now

seanbudd avatar Apr 23 '24 00:04 seanbudd

So what are NV Access’s recommended next steps?

cary-rowen avatar Apr 23 '24 00:04 cary-rowen