-
Notifications
You must be signed in to change notification settings - Fork 206
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
追加: TTSEngine.create_accent_phrases_from_kana()
#983
追加: TTSEngine.create_accent_phrases_from_kana()
#983
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この変更は一長一短かもしれません。
今まではkana_converterの依存はrun.pyからしかなかったのですが、これでTTSEngineからの依存が生まれそうです。
AquesTalk風記法はtts_pipelineと関係ないとする方針と、関係あるようにする方針がありそうです。
分けられるなら分けたほうが良い気がしますが、AquesTalk風記法もTTSだというのもそうな気がしないでもないなぁ、という気持ちです。
(AudioQueryも同じようにTTSなのかそうじゃないのか決めることになりそう?)
まさに「AquesTalk風記法は
👍 アーキテクチャ論であるため、上記議論をベースにメンテナ判断で方向性を決めて頂くのがベターだと感じます。 |
なるほどです! ただまあ依存が二箇所から発生している状態に一時的になってしまうので、残ってる依存の方にメモ書きするか、方針のissueがあるか、2つPRが出ているほうが良いんだろうな〜と感じました。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
変更がきれいなので、この感じのPRはtest追加も一緒のPRで大丈夫だったりします!
(やりやすい方で!)
内容
TTSEngine.create_accent_phrases_from_kana()
の追加VOICEVOX ENGINE では TTS 関連機能を
TTSEngine
クラスへ集約し、run.py
でそのメソッドをコールしている。しかし
run.py
のaccent_phrases(is_kana=True)
では個別関数を複数コールしている。これらは
TTSEngine
へ集約できる。このような背景から、
is_kana=True
フローを集約したTTSEngine.create_accent_phrases_from_kana()
の追加を提案します。関連 Issue
無し