Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google_cloud_texttospeech] Add cache option #267

Merged
merged 1 commit into from Sep 8, 2021

Conversation

iory
Copy link
Member

@iory iory commented Jul 12, 2021

What is this

google cloud texttospeech waits for a reply of API communication.
It takes a little time and money for the API.
This PR reducess them by adding cache functionality.

The cache is enabled by default.

[INFO] [1626084805.085691]: sound_play node is ready to play sound
[Text2Wave] Using cached sound file (/home/iory/.ros/google_cloud_texttospeech/cache/3e1189533149d5b129aa1f6211876962--ja-JP--ja-JP-Wavenet-A--1.0.mp3) for これはテストです

Copy link
Member

@k-okada k-okada left a comment

Choose a reason for hiding this comment

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

nice feature.

'--'.join([md5, language_code, name, str(speaking_rate)])
+ '.mp3')
if os.path.exists(cache_filename):
shutil.copy(cache_filename, args.output)
Copy link
Member

Choose a reason for hiding this comment

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

how about rospy.loginfo something like "Using caching sound file ('/home/..../....wav') for 'Hello'"

Copy link
Member Author

Choose a reason for hiding this comment

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

In current implementation, text2wave is not intended to be used in ROS because not called rospy.init_node('nodename') at first.
I agree with your idea of adding logging.
Is it okay to add rospy.init_node at the beginning of text2wave's main function?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, we need to use print here...

Copy link
Member Author

Choose a reason for hiding this comment

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

VoiceSelectionParams = google.cloud.texttospeech.types.VoiceSelectionParams

cache_enabled = os.environ.get(
'GOOGLE_CLOUD_TEXTTOSPEECH_CACHE_ENABLED', False)
Copy link
Member

Choose a reason for hiding this comment

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

This could be param.
PATH, PYTHONENCODING, GOOGLE_APPLICATION_CREDENTIALS must be env, because that is used in google.cloud.texttospeech modules, but GOOGLE_CLOUD_TEXTTOSPEECH_CACHE_ENABLED is only used in this file.

I also feel set cache enable could be True, by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found the solution has a problem.
text2wave will be called at sound_play node using os.system https://github.com/ros-drivers/audio_common/blob/master/sound_play/scripts/soundplay_node.py#L267 .
I want to specify namespace to pass as ROS param, but I can't specify it correctly just by calling it in os.system.

Is there a way to correctly give the ROS namespace to the text2wave called in os.system?

Copy link
Member

Choose a reason for hiding this comment

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

I see, then, using env variable make sense. I think using env is ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK.

Also, I modified the code for caching to default behavior.

@knorth55
Copy link
Member

@iory can you solve the conflict?

@iory
Copy link
Member Author

iory commented Jul 23, 2021

I solved the conflicts. Please check the commit when you have time.

@k-okada k-okada merged commit 539e1c2 into jsk-ros-pkg:master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants