change: liberate VOICEVOX CORE #825

qryxip · 2024-08-30T10:26:32Z

内容

manifest.jsonの"…_filename"部分を変更し、.binを認識できるようにします。.binの場合、change: liberate VOICEVOX CORE ort#8で追加されるSessionBuilder::commit_from_vv_binを用います。

    "predict_duration": {
      "type": "onnx",
      "filename": "predict_duration.onnx"
    },
    "predict_intonation": {
      "type": "onnx",
      "filename": "predict_intonation.onnx"
    },
    "decode": {
      "type": "onnx",
      "filename": "decode.onnx"
    },

    "predict_duration": {
      "type": "vv-bin",
      "filename": "pd.bin"
    },
    "predict_intonation": {
      "type": "vv-bin",
      "filename": "pi.bin"
    },
    "decode": {
      "type": "vv-bin",
      "filename": "d.bin"
    },

Onnxruntime::LIB_NAMEを"onnxruntime"から"voicevox_onnxruntime"にします。

compatible_engineの場合だけ、"voicevox_onnxruntime"で失敗すると"onnxruntime"にフォールバックするようにしています。
（モック目的で使えるように）
change: liberate VOICEVOX CORE ort#8でログのフィルタリングをやめる代わりに、C APIのログフィルタのort=infoをort=warnにします。

（現行のONNX Runtime v1.17.3とsample.vvmだとログが大量に出てしまいました）
build_and_deployのis_production周りを吹き飛ばします。

その他

qryxip · 2024-08-31T19:57:16Z

a5f009c (#825)
.binの呼び方をbinからvv-binとしました。

      "type": "vv-bin",

Hiroshiba

draft状態ですけどちょっと見させていただきました！
コードは良さそう！！

製品版周りの扱いに関して相談があります！

VOICEVOXエンジンやエディタは、OSSリポジトリのreleasesで製品版をリリースしています。
コアも同じ感じにしていたのですが、このプルリクエストでis_productionフラグが消えて、一部製品版ではないものをビルドする方に倒されてるかもと感じました！

そのため製品版のonnxruntimeであるvoicevox_onnxruntimeを使っているけど、READMEは製品版のものに変えず、製品版のVVMのサポートが外れ、代わりにOSSサンプルのVVMがビルドされる形になっていそうです。
is_producrionフラグを消すのは賛成で、更に全部製品版に寄せるのが手っ取り早い気がしました。

この辺りは考え方いろいろあると思います！
とりあえずエンジンとエディターに合わせる感じとかどうかなと思い、その前提でちょっと色々コメントしてみました。

.github/workflows/build_and_deploy.yml

Hiroshiba · 2024-09-01T14:18:09Z

.github/workflows/build_and_deploy.yml

-      ASSET_NAME: model-${{ needs.config.outputs.version }}
+      ASSET_NAME: sample-model-${{ needs.config.outputs.version }}
    steps:
      - uses: actions/checkout@v4
-      - name: Checkout VOICEVOX FAT RESOURCE
-        if: inputs.is_production
-        uses: actions/checkout@v4
-        with:
-          repository: VOICEVOX/voicevox_fat_resource
-          ref: ${{ env.VOICEVOX_FAT_RESOURCE_VERSION }}
-          path: download/fat_resource
-      - name: Raplace resource
-        if: inputs.is_production
-        shell: bash
-        run:
-          rm -r ./model; mv download/fat_resource/core/model ./model


この変更によってダウンローダーが製品版のVVMをダウンロードできなくなるので、最新版のmainブランチを見て製品版のVVMを使う方法がわからなくなったかもです。
ちょっとどういう運用にするか迷いますが、とりあえずダウンローダーがダウンロードできるものは製品版のままにしておくのはどうでしょう。

（将来的にはVVM置き場を別で作ったりするのもアリ。全部ダウンロードすると重いし。）

#825 (comment)と重複しますが、このPRでダウンローダから直接voicevox_fat_resouceのファイルをダウンロードするようにすることを考えています。どうでしょうか？

今の範囲でレビューしちゃったので、可能なら分けていただけると助かる思いはあります！
他にも理由は２つほどありそうでした。

このPRは製品版コアのパッチ取得を辞めることが主目的だと思うので、VVMダウンロード方法とは独立してる

fat_resourceの運用が定まっておらず、あとからどうせコア側の変更が必要になるかもしれない（fat_resourceのバージョン管理とか）

まあでもどうしてもそうしてほしいというレベルじゃないです。must/should/want/canのshouldから少しwant寄りくらい！

わかりました。このPRでダウンローダは手を付けないことにします。

Cargo.toml

Hiroshiba · 2024-09-01T14:22:26Z

crates/voicevox_core/src/voice_model.rs

+pub(crate) enum ModelBytes {
+    Onnx(Vec<u8>),
+    VvBin(Vec<u8>),
+}


（判断メモ）

enumで2つの種類のバイナリを想定するのではなく、別途typeを持っておく手もあると思うのですが、今のこの方法で問題ないと思いました！
こっちの方が管理しやすそうですし、いろんなところで型を判定しまくることもなさそうですし、フォークする場合はこのVvBin側を変えれば良いし！

こうしている理由としてはどちらかというと、「OnnxのVec<u8>」と「VvBinのVec<u8>」が別個の役割を持っているからですね。TSだとしても{ type: "Onnx"; onnx: Uint8Array } | { type: "VvBin"; vvBin: Uint8Array; }にする…のはちょっと冗長かもしれませんが、Rustの直和型ならこれが綺麗かなと思った次第です。

ただmanifest.jsonの方のこっちはどうしましょうかね… こっちはJSONの{ "type": "onnx", "filename": "decode.onnx" }を表しますし、VVMのZIPからの読み込み時のこともあってtypeを分離させるメリットがあるんですよね…
(追記) もし分離させるんだったらRust的な観点からはtypeじゃなくてkindとでもしたいんですが、manifest.jsonに書く内容としては、うーん

#[derive(Deserialize, Clone)] #[serde(tag = "type", rename_all = "kebab-case")] pub(crate) enum ModelFile { Onnx { filename: String }, VvBin { filename: String }, }

あ、manifest.jsonは現状PRのModelFileのままtypeとfilenameのキーをもたせるのが良さそうだと感じました！
（しっかりわかってないのでなにか変なこと言ってるかも）

#830 をマージしたときに若干つらみが出てきてしまったので、こうしてしまいました。

#[derive(Deserialize, Clone)] pub(crate) struct ModelFile { pub(crate) r#type: ModelFileType, pub(crate) filename: Arc<str>, } #[derive(Deserialize, Clone, Copy)] #[serde(rename_all = "snake_case")] pub(crate) enum ModelFileType { Onnx, VvBin, }

accb03f (#825)

Hiroshiba · 2024-09-01T14:25:27Z

crates/voicevox_core_c_api/src/compatible_engine.rs

+        .or_else(|err| {
+            warn!("{err}");
+            warn!("falling back to `{alt_onnxruntime_filename}`");
+            voicevox_core::blocking::Onnxruntime::load_once()
+                .filename(alt_onnxruntime_filename)
+                .exec()
+        })


（ただのコメントです）
ここのフォールバックは将来必要なくなるかもですね！
まあまだ色々わからないけど。

crates/voicevox_core/src/manifest.rs

VOICEVOX#825 (comment)

qryxip · 2024-09-04T09:15:46Z

80345e5 (#825): 嘘コメントのリバート
148fa5d (#825): ↑で修正したところのコメントに従い、install_name_toolsでのrpath変更をvoicevox_onnxruntime.framework宛てに

qryxip · 2024-09-05T14:15:59Z

voicevox_onnxruntime自体の用意ができていませんが、多分時間がかかりそうであることを考えるとこのPRはさっさとマージした方がよいですかね？
（その場合 VOICEVOX/voicevox_project#24 と #388 のlinkを外した上でdraftを外します）

Hiroshiba · 2024-09-05T16:30:23Z

@qryxip ちょっと迷いどころですが、 @qryxip さんの都合に合わせてマージしてもいいと思います！

というのも、多分このプルリクエストをマージすると、製品版をビルドしてもVVMがロードできなくなるんですよね～～
別に必ずビルドする必要はないと思うので問題はないのですが、ちょっと後ろ髪を引かれる部分があるなと。

ただこのプルリクエストがマージされていないと進められないタスクもあると思うので、マージするのに反対ではないです。
本当に都合次第だと思います。

まあもしmainブランチで製品版ビルドしたものが動かない状態が微妙というところでしたら、一旦projectブランチを作ってそちらにマージするとかはありかもです。
コンフリクトが発生していくので早くなんとかしたい気持ちもありますが。

ということで、ちょっと問題は生じそうです＋とはいえ進めやすい方法でいいと思います、って感じです！

qryxip · 2024-09-06T10:44:21Z

そうですね… このPRによってブロックされるというタスクは多分そんなに無い気がするので、draftのままでいいのかなと思ってます。ダウンローダーもVVMについては別で進められるかと。

(projectブランチも作らなくていいのではと思ってます。結局コンフリクト解消の手間をいつ取るのかという話になりそう。)

change: liberate VOICEVOX CORE

c6e0935

qryxip mentioned this pull request Aug 30, 2024

VOICEVOXコアのプロプライエタリ部分をonnxruntimeに逃がして別々にビルド可能にする VOICEVOX/voicevox_project#24

Open

qryxip added 2 commits September 1, 2024 04:51

ortをアップデート

6e1cafb

bin → vv-bin

a5f009c

ortをアップデート

92fb4c9

Hiroshiba reviewed Sep 1, 2024

View reviewed changes

crates/voicevox_core/src/manifest.rs Outdated Show resolved Hide resolved

qryxip added 5 commits September 2, 2024 19:08

ortをアップデート

38687d5

"type": "vv-bin" → "type": "vv_bin"

ff70980

VOICEVOX#825 (comment)

inputs.is_productionを残す

b87afd8

VOICEVOX#825 (comment)

嘘コメントをリバート

80345e5

install_name_toolでのrpath変更をvoicevox_onnxruntime.framework宛てに

148fa5d

qryxip added 2 commits September 6, 2024 02:15

Merge branch 'main' into change-liberate-voicevox-core

688c03f

Merge branch 'main' into change-liberate-voicevox-core

89ea1f9

qryxip added 4 commits September 13, 2024 01:50

Merge branch 'main' into change-liberate-voicevox-core

accb03f

Merge branch 'main' into change-liberate-voicevox-core

026ff6f

Merge branch 'main' into change-liberate-voicevox-core

c5db7b8

Merge branch 'main' into change-liberate-voicevox-core

d5bce52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change: liberate VOICEVOX CORE #825

change: liberate VOICEVOX CORE #825

qryxip commented Aug 30, 2024 •

edited

Loading

qryxip commented Aug 31, 2024

Hiroshiba left a comment

Hiroshiba Sep 1, 2024

qryxip Sep 2, 2024

Hiroshiba Sep 2, 2024 •

edited

Loading

qryxip Sep 2, 2024

Hiroshiba Sep 1, 2024

qryxip Sep 2, 2024 •

edited

Loading

Hiroshiba Sep 2, 2024 •

edited

Loading

qryxip Sep 12, 2024

Hiroshiba Sep 1, 2024

qryxip commented Sep 4, 2024

qryxip commented Sep 5, 2024 •

edited

Loading

Hiroshiba commented Sep 5, 2024

qryxip commented Sep 6, 2024 •

edited

Loading

change: liberate VOICEVOX CORE #825

Are you sure you want to change the base?

change: liberate VOICEVOX CORE #825

Conversation

qryxip commented Aug 30, 2024 • edited Loading

内容

関連 Issue

その他

qryxip commented Aug 31, 2024

Hiroshiba left a comment

Choose a reason for hiding this comment

Hiroshiba Sep 1, 2024

Choose a reason for hiding this comment

qryxip Sep 2, 2024

Choose a reason for hiding this comment

Hiroshiba Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

qryxip Sep 2, 2024

Choose a reason for hiding this comment

Hiroshiba Sep 1, 2024

Choose a reason for hiding this comment

qryxip Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

Hiroshiba Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

qryxip Sep 12, 2024

Choose a reason for hiding this comment

Hiroshiba Sep 1, 2024

Choose a reason for hiding this comment

qryxip commented Sep 4, 2024

qryxip commented Sep 5, 2024 • edited Loading

Hiroshiba commented Sep 5, 2024

qryxip commented Sep 6, 2024 • edited Loading

qryxip commented Aug 30, 2024 •

edited

Loading

Hiroshiba Sep 2, 2024 •

edited

Loading

qryxip Sep 2, 2024 •

edited

Loading

Hiroshiba Sep 2, 2024 •

edited

Loading

qryxip commented Sep 5, 2024 •

edited

Loading

qryxip commented Sep 6, 2024 •

edited

Loading