整理: CancellableEngine を明確化
#1448
Open
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
Hiroshiba
reviewed
Jun 29, 2024
Comment on lines
+46
to
+48
| パラメータ use_gpu, voicelib_dirs, voicevox_dir, | ||
| runtime_dirs, cpu_num_threads, enable_mock は、 core_initializer を参照 | ||
| init_processesの数だけプロセスを起動し、procs_and_consに格納する |
There was a problem hiding this comment.
（細かいけど）
「その他の引数はcore_initializerを参照」とかにすると、引数名とドキュメントの密結合を防げるかもです。
Comment on lines
+58
to
+60
| # Requestは接続の監視に使用され、Processは通信切断時のプロセスキルに使用される | ||
| # クライアントから接続があるとlistにtupleが追加される | ||
| # 接続が切断、もしくは音声合成が終了すると削除される |
There was a problem hiding this comment.
ここ、平易な説明でわかりやすいなと感じました！！！
「接続の監視」を「切断の監視」とするとよりわかりやすいかも。
Suggested change
| # Requestは接続の監視に使用され、Processは通信切断時のプロセスキルに使用される | |
| # クライアントから接続があるとlistにtupleが追加される | |
| # 接続が切断、もしくは音声合成が終了すると削除される | |
| # Requestは切断の監視に使用され、Processは切断時のプロセスキルに使用される | |
| # クライアントから接続があるとlistにtupleが追加される | |
| # 切断、もしくは音声合成が終了すると削除される |
| sub_proc_con1, sub_proc_con2 = Pipe(True) | ||
| ret_proc = Process( | ||
| def _start_new_process(self) -> tuple[Process, ConnectionType]: | ||
| """音声合成可能な新しいプロセスを開始し、そのプロセスとそこへのコネクションを返す。""" |
There was a problem hiding this comment.
不明瞭な代名詞は展開してあげるとより確実
Suggested change
| """音声合成可能な新しいプロセスを開始し、そのプロセスとそこへのコネクションを返す。""" | |
| """音声合成可能な新しいプロセスを開始し、そのプロセスと、プロセスへのコネクションを返す。""" |
| watch_con_listからの削除、プロセスの後処理を行う | ||
| プロセスが生きている場合はそのままprocs_and_consに加える | ||
| 死んでいる場合は新しく生成したものをprocs_and_consに加える | ||
| 合成の後処理をおこなう。 |
Comment on lines
+104
to
111
| # 監視対象リストから除外する | ||
| try: | ||
| self.watch_con_list.remove((req, proc)) | ||
| self._watching_reqs_and_procs.remove((req, proc)) | ||
| except ValueError: | ||
| pass | ||
|
|
||
| # 待機中リストへ再登録する | ||
| try: |
There was a problem hiding this comment.
この辺りちょっとややこしいかもですね！
監視も待機の１つなのと、登録ってなんだっけ？となりました。
案ですが、_watching_reqs_and_procsと_waiting_procs_and_consをそれぞれ_request_poolと_process_poolにしちゃうのとかどうでしょう････？
そうするとここのコメントは、１つ目の方はコメントがいらないほど明確で、２つ目は「プロセスを待機中に戻す」とかにできるなーと。
ただそうすると、(Request, Process)のtupleを_request_poolに入れることになってなんか変な感じはあるのですが。。。
そこはこう。。コメントでうまいこと説明する感じ･･･は難しいですかね。。。。。
| 音声合成を行う関数 | ||
| 通常エンジンの引数に比べ、requestが必要になっている | ||
| また、返り値がファイル名になっている | ||
| サブプロセス上において、音声合成用のクエリ・スタイルIDに基づいて音声波形を生成し、音声ファイル名を返す。 |
There was a problem hiding this comment.
平易に。
Suggested change
| サブプロセス上において、音声合成用のクエリ・スタイルIDに基づいて音声波形を生成し、音声ファイル名を返す。 | |
| サブプロセスで音声合成用のクエリ・スタイルIDから音声を生成し、音声ファイル名を返す。 |
| assert len(tts_engines.versions()) != 0, "音声合成エンジンがありません。" | ||
|
|
||
| # 「キュー入力待機 → キュー入力受付 → 音声合成 → ファイル名送信」をループする |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
内容
概要:
CancellableEngineを明確化CancellableEngineはリファクタリングが進んでいない。現時点で「プライベート・パブリック命名規則」「変数名の略称」「docstring とコードコメントの使い分け」などに課題がある。このような背景から、
CancellableEngineを全体的に明確化するリファクタリングを提案します。PR 単位を大きくする方針に従い、本 PR では
CancellableEngine内部に必要なリファクタリングを全て実装しています。関連 Issue
無し