Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

mozilla TTS validation #2719

Closed
wants to merge 2 commits into from
Closed

mozilla TTS validation #2719

wants to merge 2 commits into from

Conversation

JarbasAl
Copy link
Contributor

@JarbasAl JarbasAl commented Oct 10, 2020

adds validation to mozilla TTS, if a wrong url is entered this will now be handled and fallback to default TTS engine, instead of the user simply having no TTS output

removed unused lang param set to "de"

i messed up in posting my review of #2713 , changes have been tested at HelloChatterbox/text2speech

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 10, 2020
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling
Copy link
Contributor

Hey I just tried this against a local Mozilla TTS server and it seems that a non-empty text parameter is required.

Passing no params resulted in:

File "/home/user/TTS/.venv/lib/python3.6/site-packages/TTS/server/synthesizer.py", line 143, in split_into_sentences
    text = " " + text + "  <stop>"
TypeError: must be str, not NoneType
[INFO] 127.0.0.1 - - [12/Oct/2020 15:56:41] "GET /api/tts HTTP/1.1" 500 -

and passing an empty string:

ValueError: zero-size array to reduction operation maximum which has no identity
[INFO] 127.0.0.1 - - [12/Oct/2020 16:00:11] "GET /api/tts?text= HTTP/1.1" 500 -

@krisgesling
Copy link
Contributor

Just had a look at their server app routes and there's only two by default:

  • /
  • /api/tts

So we could send a non-empty text param to the synthesizer or ping the root instead?

@JarbasAl
Copy link
Contributor Author

ups, i intended to ping the root, not the tts endpoint.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund
Copy link
Collaborator

forslund commented Nov 7, 2020

Works exactly as advertised 👍

Since the mozilla tts hasn't been merged to master yet, do you think the config should be changed to just the root? So in the config it would be

"url": "http://0.0.0.0:5002"

and the execute call would join the /api/tts to the url from the config instead of the validation removing it?

@JarbasAl
Copy link
Contributor Author

JarbasAl commented Nov 7, 2020

totally agree, i thought this was on master already, but if its only on dev i will make that change!

@JarbasAl
Copy link
Contributor Author

JarbasAl commented Dec 2, 2020

will re-open upon clarification of privacy ramifications of the latest mycroft partnership

code is free, you can copy and open a new PR if you do not want to wait for me re-opening it

@JarbasAl JarbasAl closed this Dec 2, 2020
krisgesling added a commit that referenced this pull request Feb 8, 2021
Replication of PR #2719
Original code from JarbasAl
krisgesling added a commit that referenced this pull request Feb 8, 2021
Replication of PR #2719
Original code from JarbasAl
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants