-
Notifications
You must be signed in to change notification settings - Fork 120
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
ストリーミングモードのdecodeを実装(precompute_renderとrender) #854
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.
実装面でのレビューです。
あとヒホさんにも一度目を通して貰った方がよいかなと思いました。
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.
見ました!!!! 全体的に大きな問題はないと思います!!!!
計算も大丈夫そう!!
変数名の名称等に関してコメントしました!
あまり興味なかったら辛いと思うので、結構素直にあまり興味ないことを伝えていただければ・・・!
(ただまあ 綺麗な API を提供したいという気持ちはおそらくみんなあるはずで、議論できるとこちら的にありがたいです!)
あ、推論コードのテストが書かれてないかも。
確かただ通るだけのテストが書かれてるので、もしよければ!
(これは生放送の時に @qryxip さんと話したのですが)
音声の結果が変わってないことをスナップショットで確認したいかも・・・・・・!
ただ OS によって結果が変わるらしいので、やるならWindowsのみでテストするとかになりそう。
さすがにOSが一緒なら計算結果は一緒だろう、くらいの気持ちで。
まあ仮にPC によって計算結果が違うなら、Github Actions上でだけテストする感じにするとか。
ただこれを @Yosshi999 さんにお願いするのは、ちょっとテストのスナップショット周りが少し入り組んでいるので難しそうなのかなと感じています。
難しそうであれば @qryxip さんにお願いしたい気持ちがあります・・・・・!
ちなみにe2eのスナップショットテストはpredict durationとintonationについては行われていそうでした。この辺りです。
voicevox_core/crates/voicevox_core_c_api/tests/e2e/testcases/compatible_engine.rs
Lines 88 to 91 in 683da55
float_assert::close_l1(&phoneme_length, &EXAMPLE_DATA.duration.result, 0.01); | |
float_assert::close_l1(&intonation_list, &EXAMPLE_DATA.intonation.result, 0.01); | |
assert!(wave.iter().copied().all(f32::is_normal)); |
テストはどういったものを書きましょうか。デグレ防止という意味だと現状の |
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.
今気付いた点として:
旧 いくつかのWAVの完全一致を手元で手動で…とか?(テストはテストで将来のために作るとして) |
あっ @qryxip さんの @Yosshi999 さんの作ってくださったpython版サンプルが2つとも実装されているので、これでチェックするのが簡単&安心できそうと思いました!
みたいな・・・!WAVフォーマットがちょっと変わるとかは問題なさそう。 予感として、DirectMLだと何かしらの計算誤差が出るかも。 |
@Yosshi999 比較ありがとうございます!! 別リポジトリに切り出すかどうか置いといて、「これが作られるべき」なものをファイル保存しといてそれと比較するテストのことをスナップショットテストと呼んだりします! 比較結果なんですが、完全一致しない感じなんですね!!!!!! |
誤差については完全に謎です。トリミングしたことで(このPRの実装ではsynthesisで全体を生成したときもトリミングが入ります)計算順序がずれたのかなと思いますが、確実なことは言えないです |
なるほどです!! 僕もありえるとしたら計算順序の違いだろうなという気はしています! とりあえずこちらの数値誤差であれば、まあおそらく問題ないだろうとは感じています・・・! |
現在のPRコミットabdc696でPADDING_SIZEを0にしてデフォルトのテキストを生成したり「あ」を生成したりしてみましたがエラーにはなりませんでした(workaroundのときに言われていたように音声冒頭が切れてはいました) |
何度考えてもこの機能楽しみですねぇ!!! ・・・!!!! あれ、なるほどです! これって一応確認すると、以前はdecode.onnxだけでdecode推論してたのを、sosoa.onnxとvocoder.onnxの部分に分けてdecode相当を推論しても、結果が完全一致したってことですよね。 もう1つ確認すると、このコメントの↓の結果って、どちらも「sosoaとvocoderでdecode相当を推論してる」という認識であってそうでしょうか? 👀
もしあってたら、原因箇所はonnxruntimeではないはずなので、原因はかなり絞れそうだなと!!! パッと思いつくのはwave化するときにfloat32にしてるかfloat64にしてるかとか、flameの丸め込み方向が変わったとかがありえそう! |
991fbc8 -> 7dd6738 (つまり#851 による変更)ではおっしゃる通りdecode.onnxをふたつに分割し、中間物(sosoaのoutput)はそのままvocoderに入力しています。この変更で誤差が生じなかったのはこちら の
での言及を再現したものになってます。 mainとこのPR(streaming無し)で初めて誤差が発生していますが、おそらくこれはsynthesize時に中間物をそのまま入れるのではなく、「再生領域全体」を指定してrender()に投げているために、始端終端のパディング量がMARGINに切り揃えられたことが原因と考えられます。もちろんvocoderのreceptive fieldの幅から考えれば変化しないはずですが、、、計算順じゃないかな〜とは思います。 main->PR(streaming無し)での変化がどうしても気になるなら、実装を修正してsynthesize関数の挙動をmain branchのものと完全に一致させることはできます。ただrender_all関数を生やすみたいな感じで実行パスが一個増えるのを保守の観点でどう捉えるかになります |
あーーーーーーーーーーーーーーーーーなるほどです!!!!! 確かにパディングの長さが違うというか、入力が異なるので何らかの何らかが発生してずれうる気がします!!! おっしゃる通り実装を以前のに合わせれば、つまり余分に計算して切り揃えれば完全一致する予感がありますが、少なくとも実装を戻す必要はないと思います! いやーーすみません、認識ずれていてお手数おかけしてしまいました、ありがとうございました!! |
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!
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.
大丈夫だと思います。
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!!
ちょっとドキュメントの部分部分だけ揃えさせていただきます!!
あとちょっとタイトルだけわかりやすく変えさせていただきます!
Co-authored-by: Nanashi. <[email protected]>
@Yosshi999 このPRのdescriptionの"Solve"ですが、おそらく正しいのは"Resolve"かと思います。"Solve"だとissueはlinkされず、PRをマージしても自動で閉じられません。 なので今私が手動で #853 をlinkしてからマージしようと思います。 もし意図と外れているのであれば #853 をre-openして頂ければ! |
内容
ストリーミング処理をするための、中間表現の出力
seekable_synthesis
と 時間区間を指定して音声(16bit PCMバッファ)を出力するrender
の実装元の
synthesis
はWAVバイナリを出力していたが、ストリーミング処理ではWAVの結合が面倒なのでPCMを出す形にした。関連 Issue
Solve #853
その他