[Rust] voicevox_tts_from_kana の実装 #193
Conversation
crates/voicevox_core/src/c_export.rs
Outdated
| unsafe { | ||
| output_binary_size.write(output.len() as c_int); | ||
| let wav_heap = libc::malloc(output.len()); | ||
| libc::memcpy(wav_heap, output.as_ptr() as *const c_void, output.len()); | ||
| output_wav.write(wav_heap as *mut u8); | ||
| } |
There was a problem hiding this comment.
[NITS] ここ voicevox_tts と処理が全く同じだと思うので共通化したほうがいいかもです
crates/voicevox_core/src/error.rs
Outdated
| @@ -54,6 +52,9 @@ pub enum Error { | |||
| base_error_message(VOICEVOX_RESULT_FAILED_EXTRACT_FULL_CONTEXT_LABEL) | |||
| )] | |||
| FailedExtractFullContextLabel(#[source] FullContextLabelError), | |||
|
|
|||
| #[error("{},{0}", base_error_message(VOICEVOX_RESULT_FAILED_PARSE_KANA))] | |||
| FailedParseKana(#[source] KanaParseError), | |||
There was a problem hiding this comment.
ここの #[source] ですが、 #[from] にしておくとKanaParseErrorに対して?したときにFailedParseKanaに変換できるようになります
ここのmap_err とかは不要になります。
#[from] については Errorのenumのパターンと元になるErrorが1:1で対応してる時のみにしたほうがよいですが、これは1:1で対応できそうなのでやったほうがいいかもです(FailedExtractFullContextLabelも同じことが言えます。反対にGetSupportedDevicesが持っているSourceErrorについては他のパターンでも返ってくる可能性があるのでやめたほうが良いです。)
There was a problem hiding this comment.
#[from] について失念していました（#[source] とごっちゃになってました……）。FailedExtractFullContextLabel と FailedParseKana の両方について #[from] に変えました！ 実際にエラーを変換している箇所についても、map_err せず単に ? 演算子だけで変換するように変更しました。
| VOICEVOX_RESULT_FAILED_PARSE_KANA => { | ||
| "入力テキストをAquesTalkライクな読み仮名としてパースすることに失敗しました\0" | ||
| } |
There was a problem hiding this comment.
ちょっと思ったんですが、エラーメッセージとエラーコードの名前対応してないのでは？
同様に voicevox_tts_from_kana という関数名では AquesTalkライクなテキストを期待することが使う側に伝えられてないように感じます。
今更という気もしますが
There was a problem hiding this comment.
あ、 voicevox_tts_from_kana についてはもう元から定義されてるから少なくとも今は変えないほうが良さそうですね
There was a problem hiding this comment.
ドメイン用語としては「AquesTalkライクな記法／読み仮名」で、コード上ではkanaという感じです。
AquesTalkという商品名を関数につけるのはうーんと思って、良い名前が思いつかず「kana」となった経緯があります。
個人的にはまあ良い名前思いつかないしkanaで良さそう、という気持ちです。
There was a problem hiding this comment.
なるほどそうなるとKanaでいいのかもしれませんがちょっともやもやしますね。 🤔
しかしそういう理由だとメッセージにAquesTalkと入っているのは大丈夫でしょうか？
There was a problem hiding this comment.
なんとかしたいと思いつつも、いったん大丈夫という判断で･･･！
* implements voicevox_tts_from_kana * allow dead_code をもう少し整理 * wavの書き込み処理を共通化 * FullContextLabelErrorとKanaParseErrorについて、fromでErrorに変換できるようにする
内容
voicevox_tts_from_kana関数を実装しました。また、現段階の C++ 実装にある機能の再実装がこれでほぼ終わるので、clippy のエラーを抑制する目的で付けられていた
#[allow(dead_code)]の対象を整理しました。関連 Issue
ref #128