add polly TTS #13
add polly TTS #13
Conversation
|
do not merge, will need to rebase this PR once #15 has been merged |
NeonJarbas
force-pushed
the
feat/polly_TTS
branch
from
caf87ac to
78db338
Compare
rebased and ready to go |
| "pulse_duck": false, | ||
| "module": "mimic", | ||
| "module": "polly", | ||
| "polly": { |
There was a problem hiding this comment.
Note that we also track language and gender in tts (as well as secondary language, gender, and voice). Language and Gender are redundant for tts since it only cares about voice, but are necessary for translating tts
There was a problem hiding this comment.
i saw that, i will tackle in the multiple user support PR
see https://github.com/NeonJarbas/NeonCore/blob/dev/mycroft/tts/__init__.py#L338
translation is handled in the base TTS class, once i have the multiple user support PR the data will available in that line, i will need to make TTS change params according to the user data
There was a problem hiding this comment.
also note that gender is not needed for translation in current implementation, only language is needed for translation.
see comment bellow in regards to handling gender
There was a problem hiding this comment.
I think we will want to keep gender available for translation, since some languages will have different translations for each gender. My thought with the tts engines is that it would be easiest to determine everything in the base tts class and then pass that data along with the utterance to the module (you can see this in https://github.com/NeonGeckoCom/neon-core/blob/dev/mycroft/tts/amazon_tts.py as the "speaker" param). Keeping this in the message is generally best for server use; the config values are only necessary for local users.
There was a problem hiding this comment.
note that the message is available here. So in this section whatever tts engine is used will be configured according to user preferences
it is irrelevant for this integration, gender, speaker and so on will be handled in the base TTS class, my reasoining was to make it TTS engine independent
There was a problem hiding this comment.
Okay, I think we will end up wanting to pass more data to get_tts and adapt all of the modules to accept the extra parameter, but that is outside the scope of this PR and we'll figure that out when we get there.
There was a problem hiding this comment.
see #24 regarding user preferences / passing extra data around, i think we should pass the User object around as a general good practice whenever the message object is not available (user can be retrieved from message.context in that case)
There was a problem hiding this comment.
I agree, all server execution for Neon now uses profile info from the message which is always made available (read from SQL upon Message creation). I think it would be good practice to do this for all execution (not just server) and include user profile data in each message rather than importing and calling config in each skill intent, but that's again another project in itself.
adds amazon polly TTS