Stop WASAPI streams if they've been inactive for more than a minute.#15097
Merged
Conversation
Contributor
Author
|
Actually, i wonder whether pausing the stream (instead of closing it) might be enough. Marking as draft until we get that tested. |
See test results for failed build of commit 77cb061a32 |
|
What about letting them configure the interval? In advanced settings perhaps? With one minute as the default, unless you can somehow get the system sleep time value and other power settings. I personally have that disabled when plugged in so wouldn't need it to do so, but unsure how viable that would be really. I'd settle for configuring it myself. |
seanbudd
approved these changes
Jul 14, 2023
seanbudd
added a commit
that referenced
this pull request
Jul 25, 2023
Changes to handle #15150 Follow up to #14697, #14896, #15038, #15097, #15145 Summary of the issue: WASAPI usage is known to cause intermittent crashing in #15150. Generally, WASAPI code has not been proven to be stable. Due to this, it should not be enabled by default in 2023.2. WASAPI can be re-enabled by default once it is proven to be stable. Description of user facing changes Disable WASAPI by default, preventing intermittent crashing #15150 Description of development approach Turn the WASAPI checkbox into a feature flag, so that it can easily be re-enabled in future. Testing strategy: Manual testing Upgrading the profile: Test starting NVDA with the WASAPI config value set to "True/False" instead of a "enabled/disabled/default". Test the various controls related to WASAPI - ensure they are saved, applied and respected correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #14913.
Summary of the issue:
On some systems, when WASAPI is enabled, the system won't automatically go to sleep, even though it goes to sleep fine when WASAPI is disabled.
Description of user facing changes
When WASAPI audio is enabled, the computer now automatically goes to sleep if appropriate.
Description of development approach
When a WASAPI stream isn't stopped, this unfortunately prevents some systems from going to sleep automatically. I think this is on Windows 10 and earlier, but the precise details aren't clear. According to a Microsoft engineer, this is by design.
I originally thought this occurred when a WASAPI stream remained open. However, some testing revealed that stopping the stream seems to be enough to fix this. Previously, we didn't explicitly stop idle streams. Stopping is better than closing because opening a stream after it is closed can introduce delays or truncated audio.
Specific implementation details:
We use a class-wide check to avoid continually resetting timers on every audio chunk. See the docstring for
WasapiWavePlayer._idleCheckfor details.Testing strategy:
Used beeps in the code to verify that streams get closed when they should.
I can't reproduce this sleep issue myself, so I asked users experiencing it to test with a try build. They reported that it fixed the issue for them.
Known issues with pull request:
None.
Change log entries:
No change log entry required because WASAPI hasn't been in a release yet.
Code Review Checklist: