playWaveFile: Repeatedly call stop until we're certain the previous file playback has been stopped.#15763
Merged
Merged
Conversation
…ile playback has been stopped.
seanbudd
approved these changes
Nov 9, 2023
|
Pardon my ignorance, but programs that keep the audio system alive to stop cutting of the start of speech, like silencio, would they muck this up?
Brian
…--
***@***.***
Sent via blueyonder.(Virgin media)
Please address personal E-mail to:-
***@***.***, putting 'Brian Gaff'
in the display name field.
----- Original Message -----
From: James Teh
To: nvaccess/nvda
Cc: Subscribed
Sent: Thursday, November 09, 2023 2:53 AM
Subject: [nvaccess/nvda] playWaveFile: Repeatedly call stop until we're certain the previous file playback has been stopped. (PR #15763)
Link to issue number:
Fixes #15757.
Summary of the issue:
Asynchronously playing multiple files in quick succession can block while one of the files plays. However, async playback should never block.
Description of user facing changes
NVDA no longer sometimes freezes briefly when multiple sounds are played in rapid succession.
Description of development approach
There are several race conditions where the background thread might feed audio after we call stop in the main thread. Some of these are difficult to fix with locks because they involve switches between Python and blocking native code. We now just keep calling stop until we know that the backgroundd thread is done, which means it was successfully stopped. We know when the background thread is done because it sets fileWavePlayer to None.
Testing strategy:
Ran the following Python console snippet:
import nvwave; nvwave.playWaveFile("waves/start.wav"); nvwave.playWaveFile("waves/start.wav")
Before this change, the first call often blocked, resulting in two sounds playing. After the change, it doesn't block and only the final sound plays.
Known issues with pull request:
Strictly speaking, this code "spins" calling stop(). Spinning is generally bad. In practice, this should only call stop() a few times at most, since the background thread should clean up as soon as it is stopped.
Code Review Checklist:
a.. Documentation:
a.. Change log entry
b.. User Documentation
c.. Developer / Technical Documentation
d.. Context sensitive help for GUI changes
b.. Testing:
a.. Unit tests
b.. System (end to end) tests
c.. Manual testing
c.. UX of all users considered:
a.. Speech
b.. Braille
c.. Low Vision
d.. Different web browsers
e.. Localization in other languages / culture than English
d.. API is compatible with existing add-ons.
e.. Security precautions taken.
------------------------------------------------------------------------------
You can view, comment on, or merge this pull request online at:
#15763
Commit Summary
a.. 0b1e4e8 playWaveFile: Repeatedly call stop until we're certain the previous file playback has been stopped.
File Changes (2 files)
a.. M source/nvwave.py (11)
b.. M user_docs/en/changes.t2t (1)
Patch Links:
a.. https://github.com/nvaccess/nvda/pull/15763.patch
b.. https://github.com/nvaccess/nvda/pull/15763.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Contributor
Author
|
No. This is only relevant to this particular stream of audio and is not impacted by any other streams, whether those streams belong to NVDA or other applications. |
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
4 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 #15757.
Summary of the issue:
Asynchronously playing multiple files in quick succession can block while one of the files plays. However, async playback should never block.
Description of user facing changes
NVDA no longer sometimes freezes briefly when multiple sounds are played in rapid succession.
Description of development approach
There are several race conditions where the background thread might feed audio after we call stop in the main thread. Some of these are difficult to fix with locks because they involve switches between Python and blocking native code. We now just keep calling stop until we know that the backgroundd thread is done, which means it was successfully stopped. We know when the background thread is done because it sets fileWavePlayer to None.
Testing strategy:
Ran the following Python console snippet:
import nvwave; nvwave.playWaveFile("waves/start.wav"); nvwave.playWaveFile("waves/start.wav")Before this change, the first call often blocked, resulting in two sounds playing. After the change, it doesn't block and only the final sound plays.
Known issues with pull request:
Strictly speaking, this code "spins" calling stop(). Spinning is generally bad. In practice, this should only call stop() a few times at most, since the background thread should clean up as soon as it is stopped.
Code Review Checklist: