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

Fix SAPI4 failing to load some voices #17726

Merged
merged 7 commits into from
Feb 25, 2025
Merged

Conversation

gexgd0419
Copy link
Contributor

@gexgd0419 gexgd0419 commented Feb 22, 2025

Link to issue number:

None

Summary of the issue:

The SAPI4 synth driver fails to load some voices, because the voices do not support certain parameters, or because the voices do not expect the client to create multiple instances of ITTSCentral objects (with feature flag TTSFEATURE_SINGLEINSTANCE).

The synth driver tries to detect whether a parameter is supported or not when loading the voice. However, removeSetting is checking against the name attribute of each setting, which should now be called id. This can cause errors when loading a voice that does not support all the parameters.

Description of user facing changes

Some of the SAPI4 voices will no longer fail to load.

Description of development approach

In removeSetting, if s.name == name is changed to if s.id == name.

Before creating a new ITTSCentral object, and when terminate is called, set both _ttsCentral and _ttsAttrs to None to release the previous ITTSCentral object.

Ignore the exception thrown from _ttsCentral.UnRegister. Some voices do not handle this well, and the whole ITTSCentral will be released anyway.

Some voices keep the pausing state even after resetting, meaning that they will still pause the audio after _ttsCentral.AudioReset and silence the output. A variable _paused is used to track the pausing state, and if it's paused, unpause it before resetting.

Testing strategy:

Manually tested with some voices.

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

@gexgd0419 gexgd0419 marked this pull request as ready for review February 22, 2025 13:26
@gexgd0419 gexgd0419 requested a review from a team as a code owner February 22, 2025 13:26
@gexgd0419 gexgd0419 requested a review from seanbudd February 22, 2025 13:26
@AppVeyorBot
Copy link

See test results for failed build of commit fe03cedc25

@seanbudd
Copy link
Member

Given there's no related issue filed, can you please provide more information?
What voices fail to load - can you provide examples?

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Code changes look good

@cary-rowen
Copy link
Contributor

Given there's no related issue filed, can you please provide more information? What voices fail to load - can you provide examples?

I'd love to file an Issue later today to provide more background about this fix.
Many thanks, @gexgd0419

@wmhn1872265132
Copy link
Contributor

wmhn1872265132 commented Feb 25, 2025

There is a very old Chinese speech engine called IBM Chinese TTS Runtime that had been unusable for many years, even causing the Microsoft Speech API version 4 driver to fail to load for users who installed it. This pull request fixes the issue and makes the speech engine work properly again.

@seanbudd seanbudd merged commit ba0a057 into nvaccess:master Feb 25, 2025
3 of 4 checks passed
@github-actions github-actions bot added this to the 2025.1 milestone Feb 25, 2025
@cary-rowen
Copy link
Contributor

Ahh, sorry I didn't say it earlier, PitchCommand doesn't work in the current implementation.

@gexgd0419 gexgd0419 deleted the sapi4-fix branch February 25, 2025 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants