Remove winmm support from NVDA

[SaschaCowley]

Link to issue number:

Closes #16080
Closes #2067

Summary of the issue:

The support for WASAPI (and Windows' Core Audio APIs more broadly) in NVDA is now quite mature, and maintenance of our winmm support is becoming untenable.

Description of user facing changes:

The option to use WASAPI for audio output has been removed from NVDA's advanced settings. This has been on by default for some time, so it is unlikely to affect most users.

As the Mmdevice API does not have the concept of an ID to refer to the default output device, "Microsoft Sound Mapper" is no longer an option in the output device selection in Audio Settings. Users can instead choose "Default output device".

Description of development approach

Removed all references to winmm in nvwave.py. Rewrote the device enumeration to use the Mmdevice API, via pycaw. Slightly modified the device selection logic in the settings GUI in light of Mmdevice not having an equivalent to Microsoft Sound Mapper.

Testing strategy:

System and unit tests, as well as running NVDA, and changing output devices, including plugging and unplugging headphones while NVDA was running.

Known issues with pull request:

None

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

[SaschaCowley]

@coderabbitai please open an issue related to this one, for switching to using endpoint IDs rather than friendly names for identifying the selected output device in user configuration.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@SaschaCowley: I've opened a new GitHub issue as requested: #17497

🔗 Analysis chain

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Open a new GitHub issue with the requested details
gh issue create \
  --title "Switch to using endpoint IDs for identifying selected output device in user configuration" \
  --body "As per [PR #17496](https://github.com/nvaccess/nvda/pull/17496) and [this comment](https://github.com/nvaccess/nvda/pull/17496#issuecomment), we need to switch to using endpoint IDs rather than friendly names for identifying the selected output device in user configuration. Requested by @SaschaCowley." \
  --assignee "SaschaCowley"

Length of output: 499

---

✏️ Learnings added

Learnt from: SaschaCowley
URL: https://github.com/nvaccess/nvda/pull/17496

Timestamp: 2024-12-10T06:33:03.209Z
Learning: When identifying selected output devices in NVDA's user configuration, use endpoint IDs rather than friendly names.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[CyrilleB79]

This PR seems incomplete:

• Various audio features (e.g. sound split, mute other apps, etc.) only work if WASAPI is enabled:
  • in settings dialog, the check of WASAPI to enable or disable controls for such settings should be removed (see (nvwave.usingWasapiWavePlayer calls)
  • in the User Guide, the documentation of these settings mention WASAPI requirement; this should be removed
• In config spec, WASAPI should be removed from audio section, and this removal should be documented in API-breaking changes. Or in case you want to keep it as a no-effect parameter, you should at least document it in the change log and remove all of its usage in NVDA's code.
• I also wonder if the removal of AdvancedPanelControls.wasapiComboBox should be documented in the API-breaking changes.

[SaschaCowley]

@CyrilleB79 thanks for the thorough notes. I believe I've addressed all of your points now.

[seanbudd]

Thanks @SaschaCowley - mostly looks good to me!

[seanbudd]

Thanks @SaschaCowley

[Qchristensen]

Reads well, good work.

[Adriani90]

Thanks Sascha this is very appreciated cleanup.

[CyrilleB79]

@SaschaCowley, after this PR, if I plug an USB headset, but select my PC speaker for NVDA audio output, the sound still comes in the headset rather than from PC sdpeakers.

[Adriani90]

@SaschaCowley it seems the NVWave advanced logging channel is still there, I mean the one introduced in #11582 and #11614. Does it have to be removed as well? Or is it not related?