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

Feature/mozilla tts #2713

Merged
merged 17 commits into from
Oct 7, 2020
Merged

Feature/mozilla tts #2713

merged 17 commits into from
Oct 7, 2020

Conversation

domcross
Copy link
Contributor

@domcross domcross commented Oct 1, 2020

Description

Implement TTS module for Mozilla-TTS server

How to test

Fire up Mozilla-TTS server, set mycroft.conf: tts.module "mozilla" and configure tts.mozilla.url, mycroft-speak "My name is Mycroft and I am funky"

Contributor license agreement signed?

CLA [X] (Whether you have signed a CLA - Contributor Licensing Agreement

domcross and others added 11 commits November 22, 2019 20:11
When mycroft_skill.find_resource fails to load a resource for self.lang
fall back to lang 'en-us'
as most skills/resources are available in english by default.

==== Fixed Issues ====
MycroftAI#2120
Implement TTS module that works with Mozilla-TTS server.

==== Environment Notes ====
Requires a Mozilla-TTS server running preferably in your local network.
@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 1, 2020
fix missing import and codestyle
@pep8speaks
Copy link

pep8speaks commented Oct 1, 2020

Hello @domcross! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-10-06 09:45:18 UTC

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@el-tocino
Copy link
Contributor

el-tocino commented Oct 1, 2020

The dev server moz tts includes can be accessed similarly to mimic2/tacotron, but no visemes are returned. A slightly more generic tool that connects to a url and expects a wav file in return might be ideal, and indicate that moz tts works with this option?


def get_tts(self, sentence, wav_file):
wav_name = hashlib.sha1(sentence.encode('utf-8')).hexdigest() + ".wav"
wav_file = "/tmp/" + wav_name
Copy link
Contributor

@el-tocino el-tocino Oct 1, 2020

Choose a reason for hiding this comment

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

there's typically a /tmp/mycroft/ folder that might be better used for this. (in fact there's a /tmp/mycroft/cache/tts/ folder)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should be able to use wav_file without modification that should contain the complete path el-tocino mentions. If this is changed the use of local caching won't work.

fix codestyle
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund
Copy link
Collaborator

forslund commented Oct 1, 2020

Very cool! Are there any good instructions for setting up mozilla TTS server outthere?

@domcross
Copy link
Contributor Author

domcross commented Oct 1, 2020

Very cool! Are there any good instructions for setting up mozilla TTS server outthere?

When you have a pre-trained model you can run server.py from TTS\server which is based on Flask. There are also instructions for Nginx and how to build a server package with the model embedded.

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@JarbasAl
Copy link
Contributor

JarbasAl commented Oct 1, 2020

there is also https://github.com/synesthesiam/docker-mozillatts

using the pip .whl for the pre trained model here did not work out of the box for me, i needed to update numpy and downgrade numba for it to work

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

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

Nice one! You beat me to it :)

We are going to be extracting all the TTS and STT modules out to plugins, but given the work happening on voices right now I think it makes sense to merge this in.

Outside the scope of this PR, but inspired by it - I'm wondering if we should extract out the phrase chunking functions that are currently in the Mimic2 module so that any TTS module can make use of them?

},
"mozilla": {
"lang": "de",
"url": "http://10.0.0.1:5002/api/tts?text="
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if we should default to a 0.0.0.0 IP?

Comment on lines 29 to 35
from urllib import parse
from .mimic_tts import VISIMES
import math
import base64
import os
import re
import json
Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably clean some of these up? Presuming they've just been copied across from the Mimic2 module?

@krisgesling krisgesling added Status: Change requested Type: Enhancement - roadmapped Implementation of a feature that is a priority on the roadmap. labels Oct 2, 2020
@domcross
Copy link
Contributor Author

domcross commented Oct 2, 2020

I'm wondering if we should extract out the phrase chunking functions that are currently in the Mimic2 module so that any TTS module can make use of them?

I have seen these functions and left them out intentionally as Mozilla-TTS server implements phrase chunking functions itself.

fix imports and example config
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Cache handling
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@forslund
Copy link
Collaborator

forslund commented Oct 3, 2020

Finally got the TTS setup, it's working very well!

I could minimize the get_tts method to

   def get_tts(self, sentence, wav_file):                                      
        req_route = self.url + sentence                                         
        response = requests.get(req_route)                                      
        with open(wav_file, 'wb') as f:                                         
            f.write(response.content)                                           
        return (wav_file, None)  # No phonemes

This lets mycroft take care of all caching instead of duplicating the check here.

Another way to make the code cleaner could be to use the params argument in requests.get

setting the url in config to http://0.0.0.0:5002/api/tts

then doing the request call using requests.get(self.url, params={'text': sentence})

This would automatically escape anything in sentence? that could mess up the url.

remove custom cache code
improve url handling
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)


from .tts import TTS, TTSValidator
from mycroft.configuration import Configuration
from mycroft.util.log import LOG
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like now os, hashlib and mycroft.util.log isn't used. Apart from those tiny things it looks great!

Clean up imports
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@domcross
Copy link
Contributor Author

domcross commented Oct 6, 2020

in case this PR is merged adding the "hacktoberfest-accepted" label to it would be a cool thing ;-)

Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

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

Works excellent now! Can without issue say "Sharky & George"

@krisgesling krisgesling merged commit 6e058e4 into MycroftAI:dev Oct 7, 2020
@JarbasAl JarbasAl mentioned this pull request Oct 10, 2020
Copy link
Contributor

@JarbasAl JarbasAl left a comment

Choose a reason for hiding this comment

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

just noticed i made an "invisible" review, @ChanceNCounter pointed out comments dont appear until i submit the review....

doesnt matter now that it is merged, i opened a new PR with the changes i requested. most changes were addressed either way.

I wonder how many times i thought i reviewed something, but not really....

@@ -306,6 +306,10 @@
"espeak": {
"lang": "english-us",
"voice": "m1"
},
"mozilla": {
"lang": "de",
Copy link
Contributor

Choose a reason for hiding this comment

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

lang is not used anywhere

},
"mozilla": {
"lang": "de",
"url": "http://10.0.0.1:5002/api/tts?text="
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be changed to http://10.0.0.1:5002/api/tts see comment bellow

Copy link
Contributor

Choose a reason for hiding this comment

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

the example should probably also use localhost, I'm surprised that it's near real time in my laptop

LOG.info('local response wav found.')
else:
req_route = self.url + sentence
response = requests.get(req_route)
Copy link
Contributor

Choose a reason for hiding this comment

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

text can be passed like this instead`(see comment above about url)

response = requests.get(self.url, params={"text": sentence})

pass

def validate_connection(self):
# TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

in here you could do something like

    def validate_connection(self):
        url = self.tts.config['url']
        response = requests.get(url)
        if not response.status_code == 200:
            raise ConnectionRefusedError

@krisgesling
Copy link
Contributor

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 Type: Enhancement - roadmapped Implementation of a feature that is a priority on the roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants