Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use endpoint IDs instead of device friendly names to store user's preferred output device #17547

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

SaschaCowley
Copy link
Member

@SaschaCowley SaschaCowley commented Dec 18, 2024

Link to issue number:

Closes #17497

Summary of the issue:

NVDA currently stores the friendly name of the user's preferred audio output device.

Description of user facing changes

None.

Description of development approach

In nvdaHelper/local/wasapi.cpp, rewrote getPreferredDevice to fetch the preferred device directly via MMDeviceEnumerator.GetDevice.
Added manual checks that the fetched device is a render device, and that its status is active, since these conditions were guaranteed to be met since the previous code only iterated over devices which met those prerequisites.
Renamed deviceName to endpointId.

In source/mmwave.py, added a parameter to _getOutputDevices to return a value representing the system default output device.
Also made the type hints more self-documenting by using a NamedTuple.

In source/gui/settingsDialogs.py, used nvwave._getOutputDevices rather than nvwave.getOutputDeviceNames to fetch the available output devices.
When saving, used the selection index of AudioSettingsPanel.deviceList to index into the tuple of IDs to get the value to save to config.

In source/config/configSpec.py, moved the outputDevice key from speech to audio, and incremented the schema version to 14. Added an associated profile upgrade function in profileUpgradeSteps.py, and tests for same in tests/unit/test_config.py. Updated all occurrences of this config key that I am aware of to point to the new location.

In source/synthDrivers/sapi5.py, rewrote the device selection logic again to work with endpoint IDs.

Testing strategy:

Built from source and ensured that changing output devices works as expected.
Ensured that saving the config worked, and that the output device was selected correctly when restarting NVDA.

Tested activating SAPI5 with different output devices selected.

Known issues with pull request:

SAPI4 still doesn't work (#17516 ), this will be fixed in a future PR.

Endpoint ID strings are not human-readable.

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

@AppVeyorBot
Copy link

See test results for failed build of commit 6c9a566a08

source/config/configSpec.py Outdated Show resolved Hide resolved
source/config/profileUpgradeSteps.py Show resolved Hide resolved
source/config/profileUpgradeSteps.py Outdated Show resolved Hide resolved
source/config/profileUpgradeSteps.py Show resolved Hide resolved
source/config/profileUpgradeSteps.py Outdated Show resolved Hide resolved
source/gui/settingsDialogs.py Outdated Show resolved Hide resolved
source/nvwave.py Outdated Show resolved Hide resolved
source/nvwave.py Outdated Show resolved Hide resolved
@SaschaCowley SaschaCowley marked this pull request as ready for review December 19, 2024 06:04
@SaschaCowley SaschaCowley requested a review from a team as a code owner December 19, 2024 06:04
@gexgd0419
Copy link
Contributor

What will be the recommended way to get the WinMM device index of the preferred output device, if a third-party synthesizer plugin cannot be migrated to WASAPI?

@SaschaCowley
Copy link
Member Author

@gexgd0419 said

What will be the recommended way to get the WinMM device index of the preferred output device, if a third-party synthesizer plugin cannot be migrated to WASAPI?

See this Microsoft code sample on how to get the waveform ID from an Endpoint ID string.

@gexgd0419
Copy link
Contributor

There are still some plugins which use nvwave.outputDeviceNameToID to convert the name stored in config to get a WinMM device index.

Will there be a new helper function to convert endpoint IDs to WinMM device indexes? Or will they be encouraged to do conversion themselves instead of relying on helper functions in nvwave?

Seems that in NVDA, the SAPI4-related code is the only place where outputDeviceNameToID is used.

@SaschaCowley
Copy link
Member Author

@gexgd0419 the plan is to remove outputDeviceNameToID and outputDeviceIDToName, and keep this functionality internal only, and only in the SAPI4 driver. Add-on authors are encouraged to migrate to using Windows core audio APIs, and to manage backward compatibility however is appropriate for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to using endpoint IDs for identifying selected output device in user configuration
4 participants