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

Log never handled speech indexes, and don't make the speech queue lag behind #9946

Merged
merged 3 commits into from
Jul 19, 2019
Merged

Log never handled speech indexes, and don't make the speech queue lag behind #9946

merged 3 commits into from
Jul 19, 2019

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Jul 17, 2019

Link to issue number:

Related to #9937

Summary of the issue:

From the speech manager docs:

For every index reached, L{_handleIndex} is called.
The completed sequence is removed from L{_pendingSequences}.
If there is an associated callback, it is run.
If the index marks the end of an utterance, L{_pushNextSpeech} is called to push more speech.

_handleIndex calls _removeCompletedFromQueue for the specified index. However, that code did check whether currentIndex >= lastCommand.index. In the case where the speech synthesizer sometimes doesn't send a notification for an index, this means the following:

  1. Speech synth misses an index, calls the extension point for the index after the missing one
  2. _removeCompletedFromQueue removes the sequence from the queue that is associated with the missed index, thereby ignoring the current index.
  3. More importantly, _removeCompletedFromQueue checks the missed index to see whether it is the index for the end of an utterance.
  4. If it is not, it expects the sequence to continue. However, if the current index was actually the end of the current utterance, the current index would be handled as soon as the next utterance starts as it is still in the queue, but as the end of the utterance is never handled, speech simply ceases.

Description of how this pull request fixes the issue:

Log cases of indexes we missed, and clean up the sequences for the missed indexes.

Testing performed:

Tested with #9937 that it did no longer stop speaking with espeak, which tends to miss indexes.

Known issues with pull request:

It really looks like espeak's index handling is slightly buggy.

Change log entry:

None

@LeonarddeR LeonarddeR requested review from feerrenrut and jcsteh July 17, 2019 09:33
@LeonarddeR
Copy link
Collaborator Author

@jcsteh: I also asked you for a quick review as you are the initial author of speech refactor. May be I'm missing something essential here.

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these indexes that are never handled continue to accumulate? With this change are they ever removed?

@LeonarddeR
Copy link
Collaborator Author

Will these indexes that are never handled continue to accumulate? With this change are they ever removed?

Yes, they are removed in the line: del self._curPriQueue.pendingSequences[:seqIndex + 1]

@feerrenrut feerrenrut merged commit 6ec2eaa into nvaccess:threshold Jul 19, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Jul 19, 2019
@LeonarddeR LeonarddeR added the BabbageWork Pull requests filed on behalf of Babbage B.V. label Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BabbageWork Pull requests filed on behalf of Babbage B.V.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants