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

VVMのマニフェストの形式の再考 #581

Closed
3 tasks done
Tracked by #545
qryxip opened this issue Aug 23, 2023 · 17 comments
Closed
3 tasks done
Tracked by #545

VVMのマニフェストの形式の再考 #581

qryxip opened this issue Aug 23, 2023 · 17 comments

Comments

@qryxip
Copy link
Member

qryxip commented Aug 23, 2023

内容

今現在、VVMのmanifestはこのような形になっています。

もしもこの形式を考え直すなら、v0.15のリリースより前に行うのが最善です。

{
  "manifest_version": "0.0.0",
  "metas_filename": "metas.json",
  "decode_filename": "decode.onnx",
  "predict_duration_filename": "predict_duration.onnx",
  "predict_intonation_filename": "predict_intonation.onnx",
  "style_id_to_model_inner_id": {
    "302": 2,
    "303": 3
  }
}

私がとりあえず思ったのは次の2つです。

  1. manifest_versionはSemVerでなくてもよいかもしれません。

    edition的な概念が将来生まれるかもしれませんが、その時は別の概念にすればよいと思います。

    {
      "manifest_version": 2,
      "manifest_edition": "3-model"
    }
  2. style_id_to_model_inner_idについて、"model_inner_id"ではなく"inner_voice_id"とかでもよいかもしれません 。

    styleIdとsession.runに渡す数値が異なっているVVMでも音声合成できるようにする #551 (comment)

Pros 良くなる点

Cons 悪くなる点

実現方法

VOICEVOXのバージョン

N/A

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

その他

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 23, 2023

manifest_versionはSemVerでなくてもよいかもしれません。

これはそうだと思います。
日付などの手もあるかもしれませんが、普通に整数値にしてインクリメントしていく形が良さそうに思いました。

(2023/10/15 1:23追記)
インクリメントするタイミングは、破壊的変更が起こったら(過去のものが新しいものを読めなくなったら)。

style_id_to_model_inner_idについて、"model_inner_id"ではなく"inner_voice_id"とかでもよいかもしれません 。

style_id_to_inner_voice_idで!!

@qryxip
Copy link
Member Author

qryxip commented Aug 26, 2023

追加で:

  1. VVMにもUUIDを振った方がよいのでは?

@qryxip
Copy link
Member Author

qryxip commented Aug 26, 2023

  1. ファイルが生のonnxなのか、復号が必要なのかを示すプロパティもあった方がよいのではないか (filetype = "onnx" | "encrypted"のような形で)

@Hiroshiba
Copy link
Member

VVMにもUUIDを振った方がよいのでは?

これちょっと考えたんですけどどっちに倒そうか難しい感じです 😇
VOICEVOXの都合だけ考えると、必要になるのかどうかがわからず、であれば必要になった時に足す形が一般的には良いのかなと。。

@y-chan さん的にSHAREVOXやユーザーニーズを考えたとき、使うあるいは使いそうだったりであれば是非足した方が良いかなと!!

(ちなみにVOICEVOX的には「使うかも」って感じです。これくらいの淡い温度感で用意していいのかどうかがわからない。。)

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 27, 2023

  1. ファイルが生のonnxなのか、復号が必要なのかを示すプロパティもあった方がよいのではないか (filetype = "onnx" | "encrypted"のような形で)

なるほどです!!
OSS側で対応するのは暗号化有無ぐらいだと思うので、filetypeではなくisEncryptedのが良いかも?
(ブランドエディションや暗号化方法にもし違いがあっても、それはこの引数じゃなく他に引数を足す形を取ると綺麗そうなので)

(2023/10/15 1:25追記)
や、filetypeでも問題なさそう。どっちでも!!

@y-chan
Copy link
Member

y-chan commented Sep 2, 2023

コメント遅くなりました!

VMM Manifestについてですが、SHAREVOXだとUUIDがライブラリを読み込むためのキーとして用いているので、あると嬉しい(VMM形式になるとファイル名を引数にとってモデルを読み込むことになるのでおそらくいらない)かなぁと思いました。
あと欲しいとすれば、VMMの名前でしょうか。
ボイボ寮n期生のような名前("name")があると嬉しいのかなとかはちょっと思いましたが、それくらいな感じですかね...!

@Hiroshiba
Copy link
Member

あ〜 たしかにnameはあると便利かも。賛成寄りです。

@qryxip
Copy link
Member Author

qryxip commented Sep 2, 2023

UUIDについてですが、今はnanoidというライブラリでIDとなる文字列を生み出してそれをVoiceModelIdとしているので、それをUUIDにすればVOICEVOX側としても綺麗になるかなと思います。

@qryxip
Copy link
Member Author

qryxip commented Oct 17, 2023

一つ追加で思い付きました。TFLite使うならこういうのも要るかも?

  1. runtime ("torch" | "onnxruntime" | "tflite")

@Hiroshiba
Copy link
Member

@qryxip おお、なるほどです!
これは必要になってから足せるから、必要になってからでも良いかも?
(何が必要になってくるか本当にわかんないので、今はシンプルにしときたいという気持ちはあります)

@Hiroshiba
Copy link
Member

Hiroshiba commented Oct 17, 2023

ちょっと放送のやり取りで気づいたことをメモです。

manifest_versionが想定した値より大きかったらエラーを出す、という機構は最初から実装していないといけなさそうです。
(あとでバージョンを上げてもエラーを出せないので)

また対象のVVMが読めなくなった時(onnxruntimeのバージョンが上がる・モデルのinput/outputが変わるなどの破壊的変更があった時)に上げるバージョン値を決めておく必要があるかもです。
その機能をmanifest_versionに持たせるのか、はたまた名前を変えるのか、あるいは別のプロパティ値を足すのかは議論の余地があるかも。
プロパティ値を増やしておけば、VVMが未対応フォーマットだったときのエラーメッセージがリッチになると思います。
個人的にはmanifest_versionと呼ぶのをやめてvvm_versionにし、jsonフォーマット含め読めなくなったらこの値を上げる形がいいのかなとか思いました。考えることが一つで良いので・・・。

メモ
  • コアのバージョンはバリデーションに使えない
    • フォーマットが変わらずにコアのバージョンだけ進むため
  • vvmが読めなかった時、「このバージョン以上のコアをインストールしてください」という案内はできない
    • どのコアが対応しているか将来がわからないため
  • vvmが読めなくなったときに必ずインクリメントしていくバージョンが欲しい
    • 例えばjsonフォーマットバージョンとバイナリフォーマットバージョンを分けて管理することもできる
    • でも分けて管理すると、どのバージョン以上のコアをインストールしていいかがわかりにくくなる
    • jsonフォーマットバージョンをインクリメントする時は、vvm全体のバージョンもインクリメントしないといけない
  • vvm_versionがあれば過去のjsonフォーマットをserdeで読む実装は辛くなさそう(thx @qryxip !!)
    • serde::Deserializeのimplを手書きすれば

2023/10/18 1:11追記
@qryxip さんと話した感じ、「vvm_versionがトップなら、それだけでよい気がする」という感じになりました!
一応これで決まったということにしておいて、何か問題に気づいたらまた再考する形で!
(名前はvvm_format_versionが良いかも)

@Hiroshiba
Copy link
Member

こちらのタスクをまとめ直しました。やることは次の3点かなと。

  1. vvm_format_version周り
    • manifest.jsonにvvm_format_versionを足す
    • ↑のバリデーションを書く(ファイルのバージョンが、自分の知っているものと違っていたらエラーという挙動で良いはず)
    • ↑テストを書く
  2. style_id_to_model_inner_idstyle_id_to_inner_voice_id
  3. UUID足す(オプションだけどやっといた方が良さそう)

それ以外はマストじゃないかなという気持ちです(ずれていたら教えてください)。

2は多分僕でも簡単にできて、1も値を足すぐらいだったらできる気がするのですが、バリデーションコードを多言語ラッパーできる形で書いたり、3のUUID足したりするのはだいぶ苦戦する気がします。
とりあえず自分がプルリクエスト作る場合は2と1ちょっとみたいな感じを目指そうと思います 🙇

@qryxip
Copy link
Member Author

qryxip commented Dec 3, 2023

1も値を足すぐらいだったらできる気がするのですが、バリデーションコードを多言語ラッパーできる形で書いたり、

Manifestに触るのは内部だけのはずなので、他言語のは要らない気がします。
serdeレベルのやつなら、テスト含めてこんな感じでしょうか。

一例
use serde::{de, Deserialize, Deserializer};

#[derive(Deserialize)]
struct Manifest {
    vvm_format_version: ManifestVersionV1,
    // …
}

struct ManifestVersionV1;

impl<'de> Deserialize<'de> for ManifestVersionV1 {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        return deserializer.deserialize_str(Visitor);

        struct Visitor;

        impl<'de> de::Visitor<'de> for Visitor {
            type Value = ManifestVersionV1;

            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                formatter.write_str("a string")
            }

            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
            where
                E: de::Error,
            {
                match v {
                    "1" => Ok(ManifestVersionV1),
                    v => Err(E::custom(format!("未知の`vvm_format_version`です: `{v}`"))),
                }
            }
        }
    }
}

#[cfg(test)]
mod tests {
    use rstest::rstest;

    use super::Manifest;

    #[rstest]
    #[case("{\"vvm_format_version\":\"1\"}", Ok(()))]
    #[case(
        "{\"vvm_format_version\":\"2\"}",
        Err("未知の`vvm_format_version`です: `2` at line 1 column 25")
    )]
    fn vvm_format_version(
        #[case] input: &str,
        #[case] expected: Result<(), &str>,
    ) -> anyhow::Result<()> {
        let actual = serde_json::from_str::<Manifest>(input).map_err(|e| e.to_string());
        let actual = actual.as_ref().map(|_| ()).map_err(std::ops::Deref::deref);
        assert_eq!(expected, actual);
        Ok(())
    }
}
Error:
   | {
 2 |   "vvm_format_version": "2"
   |                            ^ 未知の`vvm_format_version`です: `2` at line 2 column 27
   | }

エラー内容が多少雑でいいなら以下のものでもOK。
(確かSHAREVOX COREが今こうなっているはず)

use serde::Deserialize;

#[derive(Deserialize)]
struct Manifest {
    vvm_format_version: ManifestVersion,
    // …
}

#[derive(Deserialize)]
enum ManifestVersion {
    #[serde(rename = "1")]
    V1,
}
Error:
   | {
 2 |   "vvm_format_version": "2"
   |                            ^ unknown variant `2`, expected `1` at line 2 column 27
   | }

(あとこういうのは1ではなく"1"とするのが一般的らしい?のでそうしています)

3のUUID足したりするのはだいぶ苦戦する気がします。

まずRustについてですが、IDを文字列ではなくUuidにし、

pub type RawVoiceModelId = String;

nanoidでIDを発行しているところを削除してManifestで持つようにするだけでいいと思います。

let id = VoiceModelId::new(nanoid!());

C/Python/Javaについては、UserDictの単語IDの取り回しと同様にすればいいと思います。
(別に文字列のままでもいいのですが、speaker_uuidのような事情もないしUserDictとの統一性を出した方がよいと思いました)

@Hiroshiba
Copy link
Member

@qryxip ありがとうございます!!
勉強も兼ねて挑戦してみたいという気持ちがあります!
ただ時間がしばらく取れなさそう(他に優先度の高いボイボのタスクが多い)なのでかなり先になる気がしています。
もしリリースタイミングが決まった場合は、簡単なところだけ実装してしまって難しそうなところは0.16以降に後回しになると思います。
なので、現状でももし他にできそうな方がいたらお任せしたいです 🙏

@qryxip
Copy link
Member Author

qryxip commented Mar 16, 2024

@Hiroshiba さん、 @y-chan さん、 @qryxip の3名で行った2024-03-04のミーティングから:

@qryxip
Copy link
Member Author

qryxip commented May 25, 2024

あ、最後のPRからlinkしてなかった。 #794, #795, #796 により完遂されました。

@qryxip qryxip closed this as completed May 25, 2024
@Hiroshiba
Copy link
Member

ありがとうございます!!🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants