This repository has been archived by the owner on Sep 8, 2024. It is now read-only.
Fix #2929: use returned TTS audio file path #2934
Merged
Conversation
This file contains 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
|
Hello @krisgesling! 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 2021-06-29 12:24:05 UTC |
devops-mycroft
added
the
CLA: Yes
|
Voight Kampff Integration Test Succeeded (Results) |
krisgesling
force-pushed
the
bugfix/tts-audio-file-path
branch
from
4a46881
to
168cb05
Compare
This reverts an unintentional breaking change. A TTS engine may return a different file path than was requested. This again uses the returned path but adds a deprecation warning that this behaviour will no longer be supported in an upcoming release. Fixes #2929
krisgesling
force-pushed
the
bugfix/tts-audio-file-path
branch
from
168cb05
to
0394568
Compare
|
Voight Kampff Integration Test Succeeded (Results) |
1 similar comment
|
Voight Kampff Integration Test Succeeded (Results) |
chrisveilleux
approved these changes
Jun 29, 2021
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 1, 2021
cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object
JarbasAl
reviewed
Jul 1, 2021
| # TODO 21.08: remove mutation of audio_file.path. | ||
| returned_file, phonemes = self.get_tts( | ||
| sentence, str(audio_file.path)) | ||
| if returned_file.path != audio_file.path: |
There was a problem hiding this comment.
returned file is a string, not a cached object, this check will throw an exception
should be if returned_file != audio_file.path
JarbasAl
reviewed
Jul 1, 2021
| "the maintainer of this plugin, please adhere to " | ||
| "the file path argument provided. Modified paths " | ||
| "will be ignored in a future release.")) | ||
| audio_file = returned_file |
There was a problem hiding this comment.
returned file is a string, not a cached object, should be audio_file.path = returned_file
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 1, 2021
cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 5, 2021
cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object self.audio_ext code has been cleaned up, it is now detected instead of hard coded
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 5, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 5, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 6, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 6, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 6, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 7, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 7, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 7, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 7, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 7, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
JarbasAl
added a commit
to HelloChatterbox/HolmesIV
that referenced
this pull request
Jul 9, 2021
hotfix/dynamic TTS audio extension engines (#60) self.audio_ext code has been cleaned up, it is now detected instead of hard coded tts engine has a default audio extension, but we can not be sure that the file actually uses the same extension, for instance in chatterbox a mp3 engine might include some cache in wav format, cached utterances might also be saved by the user in the resources directory hotfix/tts_cache (#61) fiX - MycroftAI#2934 was not a functional fix, get_tts returns a string not a cache object companion PR - MycroftAI#2938 cache was deleted on boot, the min_percent param is set to 100%, "Remove cache data if disk space is running low." actually means "always remove cache data". Value was made configurable (per tts engine) and defaults to 75% now
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
This reverts an unintentional breaking change.
A TTS engine may return a different file path than was requested. This again uses the returned path but adds a deprecation warning that this behaviour will no longer be supported in an upcoming release.
Unless there are reasons why a TTS engine may need to save the files in an alternative location?
Fixes #2929
How to test
Updated unit test.
Test also throws the DeprecationWarning as the mocked value returned differs from the path requested.
Contributor license agreement signed?