Support Pitch, Rate and Volume commands properly in SAPI4 #15271
Conversation
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
See test results for failed build of commit 54960dbeb9 |
See test results for failed build of commit 94c4bb6d1b |
seanbudd
added
the
conceptApproved
See test results for failed build of commit 32028255ca |
6 tasks
seanbudd
reviewed
Sep 14, 2023
seanbudd
reviewed
Sep 20, 2023
seanbudd
approved these changes
Sep 22, 2023
seanbudd
pushed a commit
that referenced
this pull request
Sep 22, 2023
Closes #15262 Blocked by #15271 Summary of the issue: When implementing a synthesizer driver, there are two ways to change pitch: Change the pitch setting Send a pitch command that changes the pitch for the duration of the sequence or until there is a new pitch command. Capital pitch change uses pitch commands to change pitch for capital letters. When doing this, it calls isSupported("pitch") on the driver, which returns True when the pitch setting is supported. However in order to support capital pitch changes, the driver should implement support for the PitchCommand instead. Description of user facing changes None Description of development approach Changed all situations where isSupported("pitch") is called to generate a pitch command to check whether the PitchCommand is in supportedCommands.
|
The changelog entry for this is under 2023.3, not 2024.1. Is this intentional? |
5 tasks
seanbudd
added a commit
that referenced
this pull request
Sep 25, 2023
seanbudd
pushed a commit
that referenced
this pull request
Sep 25, 2023
Fixes #15500 Follow up of #15271 Summary of the issue: Some SAPI4 synthesizers reset all their prosody values to their defaults when they come across prosody in a sequence. Description of user facing changes Capital pitch changes no longer cause some SAPI4 voices to reset their rate and volume. Description of development approach When a sequence contains any prosody command, for the other prosody commands supported by the synth, we add commands to set them to the currently configured value.
5 tasks
seanbudd
pushed a commit
that referenced
this pull request
Oct 5, 2023
…ce (#15583) Fixes #15580 Follow up of #15502 Summary of the issue: In #15502, I fixed some regressions regarding capital pitch changes introduced in #15271. This meant that we'd add prosody commands to every speech sequence to support a specific version of IBM TTS that seemed to need that. However, it introduced a regression in that it was no longer possible to change speech parameters using the gui. Description of user facing changes It is again possible to change SAPI4 parameters using the GUI. Description of development approach Only add blank prosody commands when there is any prosody in the sequence. IF not, don't do anything.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
None
Summary of the issue:
Pitch, volume and rate changes can be embedded in speech sequences. However, the latter two were never implemented for SAPI4. Pitch support was implemented last year, but it was pretty strictly bound to the capital pitch change setting (i.e. it didn't respect the offset on the PitchCommand.
Description of user facing changes
Capital pitch change intervals might be bigger with some engines.
Description of development approach
Pitch, Volume and Rate commands are now implemented according to the respective control tags
Testing strategy:
Tested that capital pitch change still works, though the difference in pitch might be bigger now. However, I compared the capital pitch change with a real pitch change and the intervals are equal.
Known issues with pull request:
I'm not sure whether all engines support the control tags out there, but since SAPI4 is pretty legacy, we should stop bothering at some point and we should consider implementing exceptions in add-ons, especially now there is an add-on store.
Change log entries:
Bug fixes
Code Review Checklist: