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をまとめて読み込むショートハンドを用意する #588

Open
3 tasks done
Tracked by #545
qryxip opened this issue Aug 26, 2023 · 7 comments
Open
3 tasks done
Tracked by #545

VVMをまとめて読み込むショートハンドを用意する #588

qryxip opened this issue Aug 26, 2023 · 7 comments

Comments

@qryxip
Copy link
Member

qryxip commented Aug 26, 2023

内容

廃止(#587)されるload_all_modelsの代用として、以下のようなAPIがあるとよいと思いました。

await synthesizer.load_voice_models("./model/*.vvm")

Pros 良くなる点

  • クイックスタートが簡潔になる

Cons 悪くなる点

  • APIが一つ増える

実現方法

Rust実装としては以下のような感じでよいと思います。

上記のload_voice_modelsのRust実装
    pub async fn load_voice_models(&self, glob: glob::Pattern) -> Result<()> {
        let paths = tokio::task::spawn_blocking(move || {
            glob::glob(glob.as_str())
                .expect("should not fail")
                .collect::<std::result::Result<Vec<_>, _>>()
        })
        .await
        .unwrap_or_else(|e| panic::resume_unwind(e.into_panic()))
        .unwrap_or_else(|_| todo!());

        stream::iter(paths)
            .then(|path| async {
                let model = &VoiceModel::from_path(path).await?;
                self.load_voice_model(model).await
            })
            .try_collect()
            .await
    }

VOICEVOXのバージョン

N/A

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

  • Windows
  • macOS
  • Linux

その他

あまり上手い名前が思い付きませんでした。glob_load_…とか...?

@Hiroshiba
Copy link
Member

load_voice_modelが単体のVoiceModel受付になってるので、パスを受け付ける関数名はload_voice_modelsじゃない方が良さそうかも。
やってることは同じなので、load_voice_models_from_glob・・・?


ちょっと考えまとまってないのですがglobはドキュメントが結構めんどくさいかもと思いました。
再帰的読み込み(**)はするのか、シンボリックリンクディレクトリをどうするか、隠れディレクトリは見るのか、隠しファイルは見えるのか、などなど・・・。

と言っても困るのはドキュメントくらいなので、これらのうちどれかなら良さそう!

  • 「このglob関数を使ってます」でドキュメントを丸投げ
  • 指定したディレクトリにあるファイルを総なめするだけ
  • ファイルリストを関数で与えたら読み込む

@qryxip
Copy link
Member Author

qryxip commented Aug 31, 2023

思い付いたのですがVoiceModelって、「合成」が可能だったりしないでしょうか。

中身のonnxと、 #581 次第ではありますがmanifestを隠蔽すればなんとかなるのではと思っています。そうすればVoiceModel::combine_multiple_files(glob::Pattern) -> Result<Self>みたいなAPIが作れて、一貫性とわかりやすさが出るんじゃないかと。

@Hiroshiba
Copy link
Member

@qryxip 合成を可能にすることはできますが、Synthesizerの主目的が名前の通り合成なので、合成はこっちに任せるのが筋だと思います。
あとVoiceModelが複数のVVMファイルを読み込めるようになってしまうと、複数のVoiceModelを差し込めるSynthesizerとの役割分担が不明瞭になっちゃうと思います。

@qryxip
Copy link
Member Author

qryxip commented Sep 6, 2023

VVMとSynthesizser内の"モデル"の差として、load状態なのかunload状態なのかというのがあるので、"unload状態で合成されたVVM"というのも意義があるのかなと思ってました。まあ役割が不明瞭にはなるとは思います。

まあどうすべきかについては、まず #587 をマージしてから考えるというのでいいのかなと思っています。C++使いの人はexampleを参考に腕力でどうにかして下さい、というのもありなのではと最近思い始めました。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 6, 2023

VVMが合成機能抱える違和感を伝えるのが若干難しいのですが、例えばVVM(VoiceModel)がopenjtalkを持たないといけなくなることになりますが、ちょっと違和感あったりしませんか 👀

C++使いの人はexampleを参考に腕力でどうにかして下さい、というのもありなのではと最近思い始めました。

これってどういうことでしたっけ。。。

@qryxip
Copy link
Member Author

qryxip commented Sep 7, 2023

選択肢としては以下となると思うのですが、どれも一長一短だと思ってます。

  • Synthesizerに複数の.vvmファイルを直接読み込ませるAPI
    • 入力はディレクトリのパス
    • 入力はglob
  • VoiceModel[]を返すAPI (C APIだと表現が難しい)
  • VVMの”合成”

なので何もせずにこのissueをcloseしてしまい、 #587 でのC++ exampleのやり方を紹介する、というのもありなのではと思った次第です。C++の人は頑張ってという感じで。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 7, 2023

なるほどです!
C++ example内でのディレクトリ総なめコードを案内するの良さそうに感じました。

まあそもそも初学者ユーザーの方はディレクトリ以下のファイルを一つずつ指定していくかもです。
チュートリアルがなかったら雑にこんな感じのコード書かれそう。

// `load_model`APIのシグネチャが違うかも
synthesizer.load_model("model/1.vvm");
synthesizer.load_model("model/2.vvm");
synthesizer.load_model("model/3.vvm");

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

2 participants