Skip to content

WASAPI: Switch back to the preferred device if it becomes available.#15781

Merged
michaelDCurran merged 2 commits into
nvaccess:masterfrom
jcsteh:wasapiPrefer
Nov 19, 2023
Merged

WASAPI: Switch back to the preferred device if it becomes available.#15781
michaelDCurran merged 2 commits into
nvaccess:masterfrom
jcsteh:wasapiPrefer

Conversation

@jcsteh

@jcsteh jcsteh commented Nov 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #15759.

Summary of the issue:

If the audio output device is set to something other than the default and that device is (or becomes) unavailable (e.g. it is disconnected), NVDA will switch to the default device. However, when that device becomes available again (e.g. it is reconnected), NVDA continues to use the default device instead of switching back to the configured device. Worse, this can't be fixed by opening the audio preferences and pressing OK, since the Settings dialog only looks at the configuration and doesn't realise that the WASAPI code has fallen back to the default device.

Description of user facing changes

If the audio output device is set to something other than the default and that device becomes available again after being unavailable, NVDA will now switch back to the configured device instead of continuing to use the default device. (#15759)

Description of development approach

  1. Pass the device name to the WasapiPlayer C++ code instead of the device id. This means that WasapiPlayer is created with the device name of the preferred device, even if that device is unavailable, allowing us to manage that in the C++ code.
  2. Removed wasPlay_getDevices and its Python callers, since they are no longer needed.
  3. Changed WasapiPlayer::open to detect whether the specified device is unavailable using an error code. If so, it falls back to the default device. We leave the preferred device name alone so we can use it later.
  4. If the device is unavailable, WasapiPlayer::feed still falls back to the default device, but again does not clear the device name.
  5. In NotificationClient, we now track device state change notifications in the same way we track default device changes.
  6. In WasapiPlayer::feed, we now handle device state changes similar to how we handle default device changes.
  7. If a device state change occurs, we scan for the preferred device by name. If it is available, we switch back to that.

Testing strategy:

  1. Configured NVDA to use a USB headset.
  2. Set the Windows default device to laptop speakers.
  3. Unplugged the headset.
  4. Observed that NVDA switches to speakers.
  5. Plugged the headset back in.
  6. Observed that NVDA switches back to the headset.
  7. Restarted NVDA.
  8. Observed that NVDA uses the headset.
  9. Exited NVDA.
  10. Unplugged the headset.
  11. Started NVDA.
  12. Observed that NVDA uses the speakers.
  13. Plugged the headset back in.
  14. Observed that NVDA uses the headset.
  15. Configured NVDA to use Microsoft Sound Mapper.
  16. Observed that NVDA switches to the speakers.
  17. Switched the Windows default device to the headset.
  18. Observed that NVDA switches to the headset.

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.

@jcsteh jcsteh requested a review from a team as a code owner November 13, 2023 13:22
@jcsteh jcsteh requested a review from seanbudd November 13, 2023 13:22
@kirill-jjj

kirill-jjj commented Nov 13, 2023

Copy link
Copy Markdown

@jcsteh Very strange bug, when changing audio devices, some of them do not change. That is, despite the selected device, nvda speaks to the default device. When I rename the playback devices through properties, it starts working as expected. But when the devices have there original names, i.e. Virtual Out 1/2; NEOM USB, Virtual Out 3/4; NEOM USB, Virtual Out 5/6; NEOM USB, they don’t work, but if I, for example, replace the capital o in the word Out with a lowercase one, then the device for which I changed the letter starts working. Attaching a log file, here I'm switching between audio devices. The only one nvda Actually switched to is Virtual Out 5/6.
nvda devices.log

@jcsteh

jcsteh commented Nov 13, 2023

Copy link
Copy Markdown
Contributor Author

Urgh. My guess is that you have multiple devices with the same names, but some of them are unavailable, possibly from previous installations of the driver or something like that. In order to support switching back to preferred devices, NVDA has to look at unavailable devices as well as available ones because the preferred device might be unavailable when NVDA starts.

I'm not really sure how to fix this cleanly. I don't see how we can tell which is the actual preferred device, since we only store the name in the config, but both of them have the same name. Ideally, we'd store the id in the config instead of the name, but that would break existing configurations.

I guess we could make the C++ code take names instead of ids and then scan for the name instead of the id. That's rather ugly, but we might have to live with it.

@jcsteh jcsteh marked this pull request as draft November 13, 2023 23:39
@OzancanKaratas

OzancanKaratas commented Nov 14, 2023

Copy link
Copy Markdown
Collaborator

I'm not convinced that the problem mentioned above needs to be solved. It is the user's responsibility to remove old devices from the system. Complicating NVDA code may create problems that will be harder to deal with in the future. I think @seanbudd should also join the discussion.

@seanbudd

seanbudd commented Nov 14, 2023

Copy link
Copy Markdown
Member

Wouldn't there be a way to safely upgrade the config? I think we should probably display and store name/id pairs, so we can show all available devices. This makes it clear duplicates need to be cleaned up.

@jcsteh

jcsteh commented Nov 14, 2023

Copy link
Copy Markdown
Contributor Author
  1. Upgrading the config isn't a great idea while we still have WinMM support. Maybe we can do this after WinMM is removed, but for now, we would break users that switch between WASAPI and WinMM.
  2. Windows does have a tendency to leave old device crap lying around. Sometimes, this is beyond the control of the user; e.g. it could be due to an updated driver or something like that. Most of the time, users can't even see this unless they do some obscure stuff in Device Manager. There's not even a way to show these in the Windows Sound properties. Thus, I don't think cleaning up duplicates is a reasonable expectation for most users.
  3. I think showing all devices, even unavailable ones, would be really confusing for the user. No other application does this.

@jcsteh jcsteh marked this pull request as ready for review November 14, 2023 11:39
@michaelDCurran michaelDCurran merged commit 2b2f347 into nvaccess:master Nov 19, 2023
@nvaccessAuto nvaccessAuto added this to the 2024.1 milestone Nov 19, 2023
@OzancanKaratas OzancanKaratas mentioned this pull request Feb 11, 2024
Closed
@OzancanKaratas OzancanKaratas mentioned this pull request Mar 12, 2024
Closed
@jcsteh jcsteh deleted the wasapiPrefer branch June 26, 2024 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelDCurran michaelDCurran michaelDCurran approved these changes

@seanbudd seanbudd Awaiting requested review from seanbudd seanbudd is a code owner automatically assigned from nvaccess/developers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2024.1

Development

Successfully merging this pull request may close these issues.

NVDA with wasapi does not reload current audio device on enter in audio settings panel

6 participants

@jcsteh @kirill-jjj @OzancanKaratas @seanbudd @michaelDCurran @nvaccessAuto