Skip to content

Fix audio output device selection#17532

Merged
SaschaCowley merged 7 commits into
masterfrom
fixAudioOutputSel
Dec 18, 2024
Merged

Fix audio output device selection#17532
SaschaCowley merged 7 commits into
masterfrom
fixAudioOutputSel

Conversation

@SaschaCowley

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #17530

Summary of the issue:

With the switch to exclusively WASAPI, selecting among more than 2 output devices did not work properly.

Description of user facing changes

Selection of output devices should now be more reliable.

Description of development approach

Fixed up the check for whether the selected device is the default output device. Also removed some unneeded initialisation parameters and constants.

Testing strategy:

Manually tested switching between 3 output devices (plus default), with various devices set as the Windows and NVDA default.

Known issues with pull request:

Some legacy winmm code remains, but this will be removed once SAPI4 support has been fixed.

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 SaschaCowley requested a review from a team as a code owner December 16, 2024 03:59
Comment thread source/nvwave.py
Comment thread source/nvwave.py
@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Dec 16, 2024
Comment thread user_docs/en/changes.md Outdated
Comment thread user_docs/en/changes.md Outdated
@SaschaCowley SaschaCowley merged commit bcccf14 into master Dec 18, 2024
@SaschaCowley SaschaCowley deleted the fixAudioOutputSel branch December 18, 2024 06:29
@github-actions github-actions Bot added this to the 2025.1 milestone Dec 18, 2024
@jcsteh

jcsteh commented Dec 18, 2024

Copy link
Copy Markdown
Contributor

While we're removing things, is it worth renaming WasapiWavePlayer to just WavePlayer as well, thus removing the redundant alias? The fact that it's called WasapiWavePlayer was an internal implementation detail necessitated by the coexistence of WinMM and WASAPI. In hindsight, I probably should have underscore prefixed it in the first place.

@SaschaCowley

Copy link
Copy Markdown
Member Author

@jcsteh yes, I think we should. I'm planning to do this in the new year.

@seanbudd

seanbudd commented Jan 6, 2025

Copy link
Copy Markdown
Member

@SaschaCowley - I think this also missed being announced on the API mailing list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default output device detection does not work correctly

3 participants