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

feat!: decode.onnxを復活させる #918

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Jan 4, 2025

内容

現在の4モデルのTalkDomain (talk)をExperimentalTalk (experimental_talk)とし、decode.onnxの3モデル版をTalkDomain (talk)とする。

両方のdomainはStyleType::Talkと対応する。ある"type": "talk"のスタイルに対してTalkDomainExperimentalTalkのどちらかが有ればよいことにし、両方有る場合はTalkDomainが優先されるようにする。

関連 Issue

Resolves: #916

その他

#917 が先にマージされることを前提にしたPRです。

Cc: @Yosshi999

@qryxip qryxip requested a review from Hiroshiba January 4, 2025 03:04
@qryxip qryxip marked this pull request as ready for review January 4, 2025 08:30
@qryxip qryxip requested review from Hiroshiba and removed request for Hiroshiba January 4, 2025 14:18
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!

追加に強いアーキテクチャにしておくと、こういうわりとごちゃつきそうな追加も無理なくできて嬉しいんだなぁと感じました。
ありがとうございます!!


あ、1つあまり強くない提案が。

ExperimentalTalkExperimental(実験的)足り得るのは今だけの相対的な存在なので、将来変わるという前提で良いでしょうか? 👀
であれば、どうせ将来変わるなら、将来変わらないかもしれない値でも良いかもと思いました!
(変えなくて良いならVVM作り直しの工数が減ってちょっと楽できる可能性もあるので)

StreamingTalkとかTalkStreamingとかどうでしょう?
あまり強くない意見です!これでも良いならこうしておきたい、くらい。

crates/voicevox_core/src/infer/domains.rs Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

(特に強くない意見です)

streaming用と普通のtalk用とソング用とで、サンプルvvm複数用意してもいいかも。

Copy link
Member Author

Choose a reason for hiding this comment

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

あーなるほど。テストのバリエーションが増えるしよさそう?

ソングも分離となると、別PRに分けた方がよい気もしますが。
(このPRは既に+452-91の変更があるわけですし)

Copy link
Member

Choose a reason for hiding this comment

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

あ、PRは別れてると嬉しいです!!
issue作るほどでもないかなと思っての提案って感じでした 🙏

@qryxip
Copy link
Member Author

qryxip commented Jan 5, 2025

であれば、どうせ将来変わるなら、将来変わらないかもしれない値でも良いかもと思いました!
(変えなくて良いならVVM作り直しの工数が減ってちょっと楽できる可能性もあるので)

StreamingTalkとかTalkStreamingとかどうでしょう?
あまり強くない意見です!これでも良いならこうしておきたい、くらい。

ジャストアイデアなんですが、"vvm_format_version": 1"vvm_format_version": "2-dev"の同時展開というのはどうでしょうか?それなら両方talkのままにできそうかなーと。まあmanifest.jsonの中身はプライベートと考えてよいはずなので、talk_streamingとかでもよさそう。ちょっと悩んでます。

[追記] ↑ いや"vvm_format_version": "2-dev"にしたらVVM作り直しか。無し。

@Hiroshiba
Copy link
Member

Hiroshiba commented Jan 5, 2025

あ~~~そうか、versionを2にしてtalkにするという手もありますね!!
たしかに-dev付けると作り直しになっちゃいそう。

この意図があるならexperimental_talkで良さそうに思いました!!

あと仮にversion 1でtalk_streamingとしてしまい、かつ後々にtalk_streamingのmanifestが変わってしまった場合、talk_streamingという名前を変えるかversionを2にしないといけなそうだなと。
version 2にするのは良いけど、ver 1のままtalk_streamingという名前を使う選択肢がなくなっちゃう可能性あるのはよくなさそう!

VVM作り直すにしてもmanifest.jsonを置き換えたりファイル名変えたりするだけで大変ではないと思うので、必ず作り直す方針で良さそうに思いました!!

@qryxip
Copy link
Member Author

qryxip commented Jan 5, 2025

ですね。experimental_talkがよさそう。

@qryxip qryxip merged commit 87443fd into VOICEVOX:main Jan 5, 2025
29 checks passed
@qryxip qryxip deleted the feat-restore-decode-onnx branch January 5, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

decode.onnxを復活させる
2 participants