Support for audio output using WASAPI #14697
Conversation
source/config/configSpec.py
Outdated
| @@ -56,6 +56,7 @@ | |||
| # Audio settings | |||
| [audio] | |||
| audioDuckingMode = integer(default=0) | |||
| wasapi = boolean(default=false) | |||
There was a problem hiding this comment.
The "audio" section is saved in profiles. However it seems to me that this options is only relevant for the base profile since it takes effect after restart.
Would it be worth to put this option in another or a new section (e.g. global settings / startup settings)?
Note that there is probably already some confusion on this point regarding some advanced settings; I am specifically thinking to "Registration for UI Automation events and property changes" option; but there may be others in adv settings.
There was a problem hiding this comment.
I agree this could be a point of confusion. That said, it seems overkill to create an entirely new config section for a setting which will eventually be removed. (I don't think it makes sense to keep both audio backends long-term.) It also doesn't fit so well in any of the existing global sections. Nevertheless, I'm of course happy to change this if the reviewer has a specific recommendation.
source/config/configSpec.py
Outdated
| @@ -56,6 +56,7 @@ | |||
| # Audio settings | |||
| [audio] | |||
| audioDuckingMode = integer(default=0) | |||
| wasapi = boolean(default=false) | |||
There was a problem hiding this comment.
NV Access will probably request a ternary setting in the config (auto/wasapi/wave out), even if it's only exposed as a checkbox in the UI during development. The "auto" setting will be treated like "wave out" until the default is changed, at which point "auto" becomes "wasapi".
There was a problem hiding this comment.
I'm not really clear on when we should be using a feature flag versus a boolean. Some advanced settings (even some added recently) are simple booleans, while others are feature flags. Again, I'm happy to implement whatever is recommended during review.
There was a problem hiding this comment.
I've opened jcsteh#1 to resolve this item.
jcsteh
force-pushed
the
wasapi
branch
3 times, most recently
from
18cd716
to
8b9513b
Compare
See test results for failed build of commit d30c626079 |
See test results for failed build of commit bc69b7422a |
|
I actually have no idea what the policy or procedure is for generating signed try builds these days. |
|
@LeonarddeR a try build is building here: https://ci.appveyor.com/project/NVAccess/nvda/builds/46590616/artifacts, by pushing a branch named "try-*" to this repo |
|
Here are my first findings. Note that I have WASAPI enabled on secure screens as well.
|
Oof. That's going to be hellish to debug. I guess the Windows audio service or the audio device isn't initialised at that point? I don't really know what we're supposed to do in that case. I would've thought the Windows Accessibility manager thing wouldn't try to start a screen reader until audio was up, but I guess there's no such restriction. |
|
I did just discover and fix a bug where if you have a specific output device configured (not default, AKA Microsoft Sound Mapper) and you start NVDA without that device connected, nvwave would barf and end up setting the synth to no speech. I don't think that's the bug you're seeing, since I'm guessing you have the device set to default on secure screens, but still, it's an important fix. |
|
Hi,
Is there a try build for this?
From: James Teh ***@***.***>
Sent: Friday, March 24, 2023 12:27 PM
To: nvaccess/nvda ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [nvaccess/nvda] Support for audio output using WASAPI (PR #14697)
I did just discover and fix a bug where if you have a specific output device configured (not default, AKA Microsoft Sound Mapper) and you start NVDA without that device connected, nvwave would barf and end up setting the synth to no speech. I don't think that's the bug you're seeing, since I'm guessing you have the device set to default on secure screens, but still, it's an important fix.
—
Reply to this email directly, view it on GitHub <#14697 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVCDE2KZHPJDUCTBXBGPQLW5WAGPANCNFSM6AAAAAAVRCRW5Q> .
You are receiving this because you are subscribed to this thread. <https://github.com/notifications/beacon/ACVCDE36JT4D3VPTTOWUVNDW5WAGPA5CNFSM6AAAAAAVRCRW5SWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTSYL55X2.gif> Message ID: ***@***.*** ***@***.***> >
|
|
I've been running this PR for several weeks. So far so good! |
|
@zstanecic |
|
A try build with the latest commit has been built here: https://ci.appveyor.com/project/NVAccess/nvda/builds/46648246/artifacts |
|
I tried this build and three out of three times, NVDA started talking on the logon screen. Furthermore, I actually don't see why WASAPI would be unable to find devices while WinM would be able to, since WinM is just a WASAPI client as you said. I'd say, lets bring this to 2023.2 as is and then try to make it default in 2023.3! |
|
Is it possible you accidentally configured a specific (non-default) device on the logon screen? As per #14697 (comment), I did have to fix a bug with that if said device was not connected. |
|
I have |
|
That sounds pretty defaulty to me. No idea what went wrong then. :( |
|
Late to this party and not able to easily review NVDA code anyway, but with respect to logon screens I'm pretty sure that WaveOut is implemented on top of Wasapi these days and would expect that Wasapi would be working if WaveOut is. years and years ago when I maintained Unspoken it was using Wasapi on logon screens without issue for quite a while, until people got annoyed enough that it ended up going through NVWave. But that was only because people wanted it to follow audio devices, which almost no one implements right on Wasapi (Windows 10 and later can do it automatically, but it was much harder in 2015 or so). That said, if you guys find out you want to use a third-party implementation given all the Wasapi edge cases, miniaudio is public domain and I can attest to it's stability, as it has been used by System Fault (an audiogame) for a while. Someone is going to need to test on weird channel configurations. In particular, one place Wasapi likes to be fragile is around surround sound setups, requesting/using exclusive devices, etc etc. Time will tell on that, I imagine. Figured dropping some context might be useful given my background. |
|
@ahicks92, you noted that WASAPI in Windows 10 can auto follow audio devices. Do you have any references or pointers for that? I ended up having to implement that manually and it was annoying. It'd be nice if it could be done more cleanly, at least at some point in the future when we no longer support anything less than Windows 10. |
|
It's available in Windows 10 version 1607 and later. See: https://learn.microsoft.com/en-us/windows/win32/coreaudio/automatic-stream-routing Miniaudio does it via listening to events instead, which works on older versions of Windows, and I'd not be surprised if they know about some bug or something I don't. Honestly Wasapi implementations are stupidly tricky and I regret having ever tried to write one myself (it didn't hurt that I was a worse programmer then, but even so...). I don't know that Miniaudio's API is compatible with NVDA and obviously this work is already done, but reading their code mightn't be the worst idea: https://github.com/mackron/miniaudio Admittedly I haven't heavily tested Miniaudio's device switching because it's something I haven't cared a lot about. I've occasionally gotten reports of it not working, so idk if it's actually perfect. One problem with Wasapi that comes up is that devices sometimes flat out lie. Steel Series headsets for example say that their minimum period is 1 sample. So in the general it's kind of hard to tell sometimes if a bug is because (1) your implementation is broken, (2) the device is buggy and WaveOut had some sort of patch over that, or (3) WaveOut is so conservative about what it does that it just papers over things. Event-based switching being buggy vs. miniaudio being buggy vs. drivers that are doing bad things? You decide... And if you don't know about the device events, I'm referring to these: https://learn.microsoft.com/en-us/windows/win32/coreaudio/device-events |
|
Will this PR break API compatibility? |
|
Hi,
Tony should make adjustment to his code. He prefers win mm, instead of trying to check interfaces.
From: Rowen ***@***.***>
Sent: Monday, May 1, 2023 11:03 AM
To: nvaccess/nvda ***@***.***>
Cc: Zvonimir Stanečić ***@***.***>; Mention ***@***.***>
Subject: Re: [nvaccess/nvda] Support for audio output using WASAPI (PR #14697)
Will this PR break API compatibility?
I have a problem with "tonysEnhancements" add-on, here is the log snippet:
File "nvwave.pyc", line 718, in playWaveFile
File "nvwave.pyc", line 823, in __init__
File "C:\Users\cary\AppData\Roaming\nvda\addons\tonysEnhancements\globalPlugins\tonysEnhancements\__init__.py", line 618, in preWaveOpen
winmm.waveOutSetVolume(selfself._waveout, volume2)
AttributeError: 'WasapiWavePlayer' object has no attribute '_waveout'
—
Reply to this email directly, view it on GitHub <#14697 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVCDEY3EXLNQCUX62JB4YLXD535LANCNFSM6AAAAAAVRCRW5Q> .
You are receiving this because you were mentioned. <https://github.com/notifications/beacon/ACVCDE5YJGWVJYCBLU52NGTXD535LA5CNFSM6AAAAAAVRCRW5SWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTS3FJKQ2.gif> Message ID: ***@***.*** ***@***.***> >
|
|
Thanks I also created an Issue for Tony. |
|
But, when it comes to wasapi, it is a good move.
I confirm that there are more good sides, than bad ones of this move.
I am rugging now the latest alpha with the code, and RHVoice synthesizer.
From: Rowen ***@***.***>
Sent: Monday, May 1, 2023 11:12 AM
To: nvaccess/nvda ***@***.***>
Cc: Zvonimir Stanečić ***@***.***>; Mention ***@***.***>
Subject: Re: [nvaccess/nvda] Support for audio output using WASAPI (PR #14697)
Thanks I also created an Issue for Tony.
—
Reply to this email directly, view it on GitHub <#14697 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVCDE67A5U4AX24BXU274TXD545VANCNFSM6AAAAAAVRCRW5Q> .
You are receiving this because you were mentioned. <https://github.com/notifications/beacon/ACVCDEYWAB4WQZ3ERVF3ZWTXD545VA5CNFSM6AAAAAAVRCRW5SWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTS3FJZTU.gif> Message ID: ***@***.*** ***@***.***> >
|
|
This change isn't API breaking in the sense that it doesn't change anything in the public API. However, it will certainly break anything that monkey patched or relied upon private implementation details. It looks like this add-on does that, so I'm not surprised it broke. It will need to be updated. |
|
Hello, Running NVDA Alpha 28166, and using WASAPI, my system does not go to sleep automatically even though this is set in Power Options. When using NVDA without WASAPI this works correctly. For me, I rely on my system being able to automatically go into sleep regularly and other users might as well, so I wanted to bring this issue up early, as I could see there being some concern about this and it is a concern I have as well. Other than this issue, however, WASAPI support is working well for me. Thank you very much for your time and I look forward to any input you may have. Sincerely, Brandon Tyson |
|
Hi James, |
|
@josephsl it seems using WASAPI in combination with the sound splitter addon the sounds for switching to focus or browse mode are not working anymore. I guess this needs an update on the add-on's side. |
|
Hi, any add-on that patches nvwave module contents must be edited to detect this change, and in case of Sound Splitter, try asking Luke Davis as I’m no longer maintaining Sound Splitter. Thanks.
|
|
NVDA WASAPI is already using shared mode. |
|
Shouldn't a change like this only occur in version year.1?
It will impact a lot of add-ons...
Best regards,
Rui Fontes
NVDA portuguese team
Às 21:53 de 03/05/2023, Adriani90 escreveu:
…
@josephsl <https://github.com/josephsl> it seems using WASAPI in
combination with the sound splitter addon the sounds for switching to
focus or browse mode are not working anymore. I guess this needs an
update on the add-on's side.
—
Reply to this email directly, view it on GitHub
<#14697 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADZAPRXAZWSOZVSZJGMN6Y3XELAV3ANCNFSM6AAAAAAVRCRW5Q>.
You are receiving this because you are subscribed to this
thread.Message ID: ***@***.***>
|
|
As per the Developer Guide:
By that definition, this change is not API breaking. It does not make changes to public (non-underscore prefixed) symbols. The fact that add-ons have chosen to touch private symbols (prefixed with an underscore) is their choice and they are thus subject to breakage outside of .1 releases. That said, pragmatically speaking, I acknowledge this has an impact on users, not just add-on authors. So, I guess we need to decide whether the benefits of landing this sooner (responsiveness, volume adjustment, etc.) are outweighed by the risk of add-on breakage. |
|
If it is decided that add-on breakage is too much of a problem, an alternative to reverting this could be to disable it by default. That way, users who don't depend on these add-ons can still benefit if they choose to. |
I very much agree with what @jcsteh said, I think addon authors need to update these addons. It is more important that the option for this PR should remain checked by default, so that more users can test it and find potential problems. Thanks |
|
Yes, I'm hoping to have time next week to spend figuring out how to fix that,
and any other side effects of WASAPI..
@Adriani90
… it seems using WASAPI in combination with the sound splitter addon the sounds for switching to focus or browse mode are not working anymore. I
guess this needs an update on the add-on's side.
|
|
Hello,
what about start sound of NVDA? when I tested it (yes, without any
addon), the start piano sound has not been played. Stop sound is
played when exiting NVDA. When I start NVDA, I hear only a woarning
sound.
however, thanks for your good work.
Jožef
2023-05-04 4:03 GMT+02.00, Luke Davis ***@***.***>:
… Yes, I'm hoping to have time next week to spend figuring out how to fix
that,
and any other side effects of WASAPI..
@Adriani90
> it seems using WASAPI in combination with the sound splitter addon the
> sounds for switching to focus or browse mode are not working anymore. I
> guess this needs an update on the add-on's side.
--
Reply to this email directly or view it on GitHub:
#14697 (comment)
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
|
|
Must be a particular add-on that must be causing problems. I can definitely hear the NVDA starting sound when NVDA is starting.
… Message ID: ***@***.***>
|
|
Hi, On Windows 10, I'm encountering an issue when using WASAPI where my Otherwise I think this being on by default is good, but this is an This issue does not occur on Windows 11, however. Thanks, Brandon |
|
@btman16 @gregjozk - At this stage, issues with NVDA are alpha issues, not issues with this PR. |
Link to issue number:
Fixes #11615. Fixes #5096. Fixes #10185. Fixes #11061. Partially addresses #10413 and #1409. May address #11169.
Summary of the issue:
NVDA's existing audio output code (nvwave) is largely very old and uses WinMM, a very old legacy Windows audio API. It is also written in pure Python, contains quite a few threading locks necessitated by WinMM, and parts of it have become rather difficult to reason about. There are several known stability and audio glitching issues that are difficult to solve with the existing code.
Description of user facing changes
At the very least, this fixes audio glitches at the end of some utterances as described in #10185 and #11061.
I haven't noticed a significant improvement in responsiveness on my system, but my system is also very powerful. It's hard to know whether the stability issues (e.g. #11169) are fixed or not. Time will tell as I run with this more.
Description of development approach
Testing strategy:
Known issues with pull request:
Change log entries:
New features
NVDA now outputs audio via the Windows Audio Session API (WASAPI), which may improve the responsiveness, performance and stability of NVDA speech and sounds. This can be disabled in Advanced settings if audio problems are encountered.
Code Review Checklist: