Make continuous reading work when using SAPI5 voices without bookmark support

[gexgd0419]

Link to issue number:

Fixes #16691

Summary of the issue:

Some SAPI5 synthesizers did not work correctly with continuous reading, stopping speech at the end of a sentence, because they don't properly support bookmark events.

Description of user facing changes

Some of the problematic SAPI5 synthesizers will be able to work correctly with continuous reading.

Description of development approach

NVDA relies on bookmark events, which have to be implemented by the TTS engine itself. But no matter what events the engine supports, StartStream and EndStream events are always generated by the SAPI5 framework.

This fix does the following.

SAPI5 fix

When requested to speak a new sequence, store the bookmarks in a list (deque), and put the bookmark list into a dict _streamBookmarksNew with the stream number as its key. This dict stores bookmark lists before the stream actually starts, because the previous stream (which may use the same stream number) may not have been ended when self.tts.Speak returns.

When StartStream event is raised, move the bookmark list of this stream from _streamBookmarksNew to _streamBookmarks, which stores bookmark lists of currently speaking streams.

When EndStream event is raised, bookmarks stored for the current stream will be triggered (through synthIndexReached) before synthDoneSpeaking. This way NVDA will continue to the next line instead of waiting indefinitely.

As most synthesizers do support bookmarks, to prevent triggering the same bookmark twice, the bookmark stored in the list will be removed when the Bookmark event for it is raised.

Thread-safety: self.tts.Speak and TTS events are handled in the same thread, so there should be no multithread-related problems.

SAPI4 fix

Similar to the SAPI5 fix, which stores bookmarks and replay them when AudioStop is raised.

However, SAPI4 does not provide "stream numbers" to distinguish each request, yet SAPI4 supports queueing speech requests. The fix assumes that speech requests are always processed in a first-in, first-out manner, and uses a deque _bookmarkLists to store bookmark lists from different requests.

When cancel is called, the fix assumes that all queued speech requests are cancelled immediately, and clears the bookmark lists.

Testing strategy:

Tested with the v0.1 version of NaturalVoiceSAPIAdapter. I don't have access to those proprietary voices, so further testing is needed.

Known issues with pull request:

None yet

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

[zstanecic]

@gexgd0419,
This will probably also fix legacy Ivona tts voices, sold by Harpo Sp.Zo.o or with all other old synthesizers.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: License check.
• PASS: Unit tests.
• PASS: Lint check.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/dofc7by5cwbm0wo4/artifacts/output/nvda_snapshot_pr17523-34798,75c580bc.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 1.2,
  INSTALL_END 1.0,
  BUILD_START 0.0,
  BUILD_END 25.0,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.3,
  TEST_START 0.0,
  TEST_END 18.3,
  FINISH_END 0.1

See test results for failed build of commit 75c580bcf6

[seanbudd]

Nice fix @gexgd0419 ! Can you please add a changelog entry

[gexgd0419]

Just found that even when SAPI5 framework reuses the previous stream number, self.tts.Speak can still return before the previous stream ended. For example:

• Speak returned stream number 1
• StartStream event of stream number 1
• Another Speak returned stream number 1
• EndStream event of stream number 1 (old stream)
• StartStream event of stream number 1 (new stream)
• EndStream event of stream number 1

My code assumes that when Speak returned, the previous stream has ended, so the bookmark list is assigned to the stream number. But if the above happened, bookmarks in the new stream will be triggered when the old stream ends. So this needs to be fixed.

change of approach required

[SaschaCowley]

We would prefer this be handled for all synthesisers in the speech framework.

[gexgd0419]

We would prefer this be handled for all synthesisers in the speech framework.

What does this mean?

This fix affects all SAPI5 and MSSP synthesizers, even those that support bookmark events. But synthesizers that support bookmarks will have "consumed" all stored bookmarks before the end of stream is reached.

If you mean SAPI4 synthesizers, I don't know much about that framework. If SAPI4 has some way to tell the client that it has done speaking, then similar approach may be possible. But in the current SAPI4 implementation, synthDoneSpeaking actually relies on bookmarks.

[gexgd0419]

After some studying, I think that applying similar fix to SAPI4 synthesizers is possible. We will need to introduce another sink class which implements ITTSNotifySinkW to get its AudioStart and AudioStop notifications, so that we can replay the missing bookmarks when AudioStop is received.

As for the other two speech frameworks that NVDA supports: eSpeak and OneCore, I don't think that they need to be fixed, as currently there's no third-party OneCore voice that I know of.

Without a unified audio processing system, this fix can only be applied per framework, and only if the framework supports end of audio notifications.