Add WASAPI toggle to SAPI5 by gexgd0419 · Pull Request #18352 · nvaccess/nvda

gexgd0419 · 2025-06-27T04:18:13Z

Link to issue number:

Summary of the issue:

Users cannot turn off WASAPI for SAPI5 yet.

Description of user facing changes:

Add a synthesizer setting called useWasapi, which is displayed as a checkbox labeled Use modern audio output system (WASAPI) in the voice settings when the built-in SAPI5 synthesizer is selected.

Description of developer facing changes:

None

Description of development approach:

A new SynthSetting based on BooleanDriverSetting called useWasapi is added and defaults to True. It is not in the settings ring.

Legacy SAPI5 code are re-introduced to be used when WASAPI for SAPI5 is disabled.

Testing strategy:

Tested manually.

Known issues with pull request:

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.

@coderabbitai summary

gexgd0419 · 2025-06-27T04:23:21Z

@seanbudd @SaschaCowley What's your thought on the name, location, and display text of this new synth setting? (It's added at the end of the synth settings list for now)

Should the existing config config.conf["speech"]["useWASAPIForSAPI4"] be migrated? How should I do this?

SaschaCowley · 2025-06-27T05:31:16Z

@gexgd0419 note I haven't reviewed any code:

I'd rather the toggle be a FeatureFlag so we can change the default behaviour in future
I think you can probably change the display name of the setting to "Use modern audio output", and note in the user guide that this pertains to WASAPI.

Ideally, we should have config.conf["speech"]["sapi4"]["useWASAPI"] and config.conf["speech"]["sapi5"]["useWASAPI"]

To ensure backwards compatibility, ["speech"]["useWASAPIForSAPI4"] and ["speech"]["sapi4"]["useWASAPI"]` should be linked. See for example

nvda/source/config/__init__.py

Lines 1308 to 1356 in e1d0d9d

    
           		# Alias [documentFormatting][reportFontAttributes] for backwards compatibility. 
        
           		# TODO: Comment out in 2025.1. 
        
           		if version_year < 2025 and NVDAState._allowDeprecatedAPI(): 
        
           			self._linkDeprecatedValues(key, val) 
        
           	def _linkDeprecatedValues(self, key: aggregatedSection._cacheKeyT, val: aggregatedSection._cacheValueT): 
        
           		"""Link deprecated config keys and values to their replacements. 
        
           		Args: 
        
           			key: The configuration key to link to its new or old counterpart. 
        
           			val: The value associated with the configuration key. 
        
           		postconditions: 
        
           			- If self.path is "documentFormatting": 
        
           				- If key is "reportFontAttributes": 
        
           					- If val is True, "documentFormatting.fontAttributeReporting" is set to OutputMode.SPEECH_AND_BRAILLE, otherwise, it is set to OutputMode.OFF. 
        
           				- If key is "fontAttributeReporting": 
        
           					- if val is OutputMode.OFF, "documentFormatting.reportFontAttributes" is set to False, otherwise, it is set to True. 
        
           		""" 
        
           		match self.path: 
        
           			case ("documentFormatting",): 
        
           				match key: 
        
           					case "fontAttributeReporting": 
        
           						# Alias documentFormatting.fontAttributeReporting to documentFormatting.reportFontAttributes for backwards compatibility. 
        
           						key = "reportFontAttributes" 
        
           						val = bool(val) 
        
           					case "reportFontAttributes": 
        
           						# Alias documentFormatting.reportFontAttributes to documentFormatting.fontAttributeReporting for forwards compatibility. 
        
           						log.warning( 
        
           							"documentFormatting.reportFontAttributes is deprecated. Use documentFormatting.fontAttributeReporting instead.", 
        
           							# Include stack info so testers can report warning to add-on author. 
        
           							stack_info=True, 
        
           						) 
        
           						key = "fontAttributeReporting" 
        
           						val = OutputMode.SPEECH_AND_BRAILLE if val else OutputMode.OFF 
        
           					case _: 
        
           						# We don't care about other keys in this section. 
        
           						return 
        
           			case _: 
        
           				# We don't care about other sections. 
        
           				return 
        
           		# Update the value in the most recently activated profile. 
        
           		# If we have reached this point, we must have a new key and value to set. 
        
           		self._getUpdateSection()[key] = val 
        
           		self._cache[key] = val

gexgd0419 · 2025-06-27T05:40:33Z

BooleanDriverSetting is represented as a checkbox. If we want to use a FeatureFlag instead, should a new kind of DriverSetting be added? Or maybe this shouldn't be put inside "driver settings"?

SaschaCowley · 2025-06-27T07:01:30Z

Hmm. I guess we have 3 options:

Keep this as a boolean driver setting.
- Removes the advantages of feature flags
- It is tried and tested, and keeps the code clean
Add a new type of driver setting that supports feature flags (or similar)
- More effort
- More testing required
- Are there any other instances where this would be useful?
Move this out of the synth driver itself.
- For instance, ["speech"]["useWASAPIForSAPI4"] and ["speech"]["useWASAPIForSAPI5"].
- We could then only show them when the relevant engine is selected.
- Slightly "dirty" approach

@seanbudd, @michaelDCurran do you have any thoughts? I am leaning towards just keeping this as a checkbox (1) or not having it as part of the driver settings (3). I don't think (2) is worth the effort at this point. Given the time constraints and the tidiness of the code, I think I would probably go with option 1, but I'd like a second opinion.

CyrilleB79 · 2025-06-27T07:18:39Z

I have not reviewed this PR. I wonder:
Wasn't there a difficultiy integrating feature flags in drivers? I guess someone had tried this recently, maybe @LeonarddeR

LeonarddeR · 2025-06-27T07:23:46Z

I think that the magic code that creates gui items based on config spec doesn't yet support feature flags.

gexgd0419 · 2025-06-27T09:49:44Z

I think that the magic code that creates gui items based on config spec doesn't yet support feature flags.

If it is useful enough, this can be implemented, I think.

If there are going to be "per-voice driver settings", having this toggle as a driver setting may make it easier to fit in.

By the way, when WASAPI for SAPI5 is disabled, "rate boost" will also be disabled. Should anything be done to disable or hide the "rate boost" checkbox in that case, or just leave it there?

CyrilleB79 · 2025-06-27T11:04:42Z

By the way, when WASAPI for SAPI5 is disabled, "rate boost" will also be disabled. Should anything be done to disable or hide the "rate boost" checkbox in that case, or just leave it there?

If possible, it would be better to disable (gray out) the rate boot checkbox in this case to denote that it does not apply in this situation.

gexgd0419 · 2025-06-27T11:28:49Z

If possible, it would be better to disable (gray out) the rate boot checkbox in this case to denote that it does not apply in this situation.

"Rate boost" is a driver setting, so the checkbox control is generated by the "magic code" in AutoSettingsMixin. Not sure if it is a good idea to disable such a control, or how I can fetch such a control after creation.

gexgd0419 · 2025-07-04T09:19:28Z

Hmm. I guess we have 3 options:

Keep this as a boolean driver setting.

Removes the advantages of feature flags

It is tried and tested, and keeps the code clean

Add a new type of driver setting that supports feature flags (or similar)

More effort

More testing required

Are there any other instances where this would be useful?

Move this out of the synth driver itself.

For instance, ["speech"]["useWASAPIForSAPI4"] and ["speech"]["useWASAPIForSAPI5"].

We could then only show them when the relevant engine is selected.

Slightly "dirty" approach

Any guidance on what I should do? @SaschaCowley @seanbudd

zstanecic · 2025-07-04T09:39:50Z

We need to keep compatibility with older NVDA.

seanbudd · 2025-07-07T03:37:28Z

@gexgd0419 - can you mark PRs ready for review when they are awaiting feedback from us

Copilot

Pull Request Overview

This PR introduces a new Use modern audio output system (WASAPI) toggle for both SAPI4 and SAPI5 synthesizers, replacing the old advanced-only SAPI4 setting and re-enabling legacy paths when WASAPI is turned off.

Add new useWasapi setting via BooleanDriverSetting, exposed in voice settings for SAPI4/SAPI5
Implement WASAPI vs. legacy audio initialization and audio ducking logic in both drivers
Update user guide, GUI dialogs, and remove deprecated useWASAPIForSAPI4 config spec

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
user_docs/en/userGuide.md	Document new WASAPI setting under voice settings and remove old SAPI4 advanced UI
source/synthDrivers/sapi5.py	Add SPAudioState enum, audio ducking integration, WASAPI/legacy init and handling
source/synthDrivers/sapi4.py	Add `UseWasapiSetting`, `useWasapi` property, toggle between WASAPI and MMAudio
source/synthDriverHandler.py	Introduce `UseWasapiSetting` factory for driver settings
source/gui/settingsDialogs.py	Include `useWasapi` in standardSettings and remove old `useWASAPIForSAPI4` UI
source/config/configSpec.py	Remove deprecated `useWASAPIForSAPI4` configuration entry

Comments suppressed due to low confidence (4)

source/config/configSpec.py:49

The removal of the legacy 'useWASAPIForSAPI4' config spec will reset existing user preferences. Consider adding migration logic to preserve users’ existing settings.

	trimLeadingSilence = boolean(default=true)

source/synthDrivers/sapi4.py:844

[nitpick] New WASAPI toggle has been introduced; consider adding unit and integration tests to cover enabling/disabling audio paths for the SAPI4 synthesizer.

	supportedSettings = [SynthDriver.VoiceSetting(), SynthDriver.UseWasapiSetting()]

source/synthDrivers/sapi5.py:252

[nitpick] A new setting for WASAPI support has been added; please add tests verifying both paths (WASAPI enabled and disabled) for SAPI5 audio initialization and streaming behavior.

		SynthDriver.UseWasapiSetting(),

source/synthDrivers/sapi5.py:39

IntEnum is not imported in this file, so defining SPAudioState(IntEnum) will raise a NameError. Please add 'from enum import IntEnum' to the imports.

class SPAudioState(IntEnum):

seanbudd · 2025-07-10T05:24:40Z

@SaschaCowley

I think we should go with (1) on your comment #18352 (comment)

In regards to (3) I think we need to keep the old SAPI4 config option and sync with it, or not add this to SAPI4 until 2026.1

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

gexgd0419 · 2025-07-10T13:39:50Z

So should I keep both toggles for SAPI4, or keep only the old one?

SaschaCowley · 2025-07-11T01:10:59Z

@gexgd0419 please keep only the old toggle for SAPI4 for now

gexgd0419 · 2025-07-14T06:46:30Z

This pull request is updated to contain only changes to SAPI5. Not sure why #18310 isn't unlinked though.

Should the "(WASAPI)" part in the description Use modern audio output system (WASAPI) be taken out? Should the word "WASAPI" in useWasapi be written in all-caps or not?

seanbudd · 2025-07-14T07:07:36Z

I can't seem to manually unlink it. Maybe we don't worry about that.

I think keeping WASAPI there as is, is fine

Qchristensen

Reads well, good work.

Fixup of #18352. Summary of the issue: When WASAPI is turned off, bookmark events are not handled properly. Description of user facing changes: Users might not notice that, because although the bookmarks are not handled in time, they are still processed at the end of the utterance. Description of developer facing changes: None Description of development approach: Code that handles the bookmark events when WASAPI is off is added.

gexgd0419 changed the base branch from master to beta June 27, 2025 05:24

gexgd0419 added 2 commits June 27, 2025 13:30

Add useWasapi setting to SAPI4 and SAPI5

3d4c8a5

Remove old useWASAPIForSAPI4 config and update user guide help message

4c8e6a3

gexgd0419 force-pushed the add-wasapi-switch branch from 45b91c3 to 4c8e6a3 Compare June 27, 2025 05:33

Pre-commit auto-fix

27b43db

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 1, 2025

seanbudd marked this pull request as ready for review July 7, 2025 03:36

Copilot AI review requested due to automatic review settings July 7, 2025 03:36

seanbudd requested review from a team as code owners July 7, 2025 03:36

seanbudd requested review from Qchristensen and seanbudd July 7, 2025 03:36

seanbudd added the blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. label Jul 7, 2025

Copilot AI reviewed Jul 7, 2025

View reviewed changes

seanbudd removed the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 7, 2025

gexgd0419 mentioned this pull request Jul 10, 2025

nvda switches to windows one core voices after restart of program and or reboot. #18301

Closed

seanbudd added this to the 2025.2 milestone Jul 10, 2025

seanbudd requested a review from SaschaCowley July 10, 2025 02:03

seanbudd reviewed Jul 10, 2025

View reviewed changes

Apply suggestions from code review

ad45dbe

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

gexgd0419 added 2 commits July 11, 2025 12:33

Rename SPAudioState to _SPAudioState

21983ed

Restore old SAPI4 config in advanced settings

cf90df6

gexgd0419 marked this pull request as draft July 11, 2025 04:39

Revert new WASAPI toggle for SAPI4

e69601b

seanbudd mentioned this pull request Jul 14, 2025

remove SAPI4 from NVDA #18310

Closed

seanbudd removed the blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. label Jul 14, 2025

gexgd0419 changed the title ~~Add WASAPI toggle to SAPI4 and SAPI5~~ Add WASAPI toggle to SAPI5 Jul 14, 2025

gexgd0419 added 2 commits July 14, 2025 12:31

Set the audio device whether WASAPI is enabled or not

9b1236a

Split legacy Speak into another method

1452e69

gexgd0419 marked this pull request as ready for review July 14, 2025 06:47

seanbudd approved these changes Jul 14, 2025

View reviewed changes

seanbudd reviewed Jul 14, 2025

View reviewed changes

Comment thread source/synthDriverHandler.py

Add note for translators

71ce069

Qchristensen approved these changes Jul 15, 2025

View reviewed changes

SaschaCowley merged commit aebd5b8 into nvaccess:beta Jul 15, 2025
15 of 16 checks passed

wmhn1872265132 mentioned this pull request Jul 15, 2025

Fix changes.md for 2025.2 #18470

Merged

5 tasks

gexgd0419 deleted the add-wasapi-switch branch July 15, 2025 07:01

gexgd0419 mentioned this pull request Jul 15, 2025

Fix SAPI5 #18473

Merged

5 tasks

CyrilleB79 mentioned this pull request Jul 21, 2025

User Guide for SAPI5 WASAPI option is confusing and mixes SAPI4 information #18522

Closed

Uh oh!

Conversation

gexgd0419 commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

gexgd0419 commented Jun 27, 2025

Uh oh!

SaschaCowley commented Jun 27, 2025

Uh oh!

gexgd0419 commented Jun 27, 2025

Uh oh!

SaschaCowley commented Jun 27, 2025

Uh oh!

CyrilleB79 commented Jun 27, 2025

Uh oh!

LeonarddeR commented Jun 27, 2025

Uh oh!

gexgd0419 commented Jun 27, 2025

Uh oh!

CyrilleB79 commented Jun 27, 2025

Uh oh!

gexgd0419 commented Jun 27, 2025

Uh oh!

gexgd0419 commented Jul 4, 2025

Uh oh!

zstanecic commented Jul 4, 2025

Uh oh!

seanbudd commented Jul 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanbudd commented Jul 10, 2025

Uh oh!

gexgd0419 commented Jul 10, 2025

Uh oh!

SaschaCowley commented Jul 11, 2025

Uh oh!

gexgd0419 commented Jul 14, 2025

Uh oh!

seanbudd commented Jul 14, 2025

Uh oh!

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gexgd0419 commented Jun 27, 2025 •

edited

Loading