search

VOICEVOX / voicevox_core

無料で使える中品質なテキスト読み上げソフトウェア、VOICEVOXのコア
https://voicevox.hiroshiba.jp/
MIT License
835 stars 115 forks source link

change: `Onnxruntime`型を追加し、そこから`dlopen`/`LoadLibrary*`を行う #802

Closed qryxip closed 1 week ago

qryxip commented 2 weeks ago

内容

パブリックAPIにOnnxruntime型を追加し、そこからlibonnxruntimeのdlopen/LoadLibrary*をするようにします。

Onnxruntime型

Synthesizerのコンストラクトはこのようになります。

ort = Onnxruntime.load_once()
ojt = OpenJtalk("./open_jtalk_dic_utf_8-1.11")
synth = Synthesizer(ort, ojt)

オプションとしてDLLの"filename"を指定することもできます。

ort = Onnxruntime.load_once(filename="./path/to/onnxruntime.dll")

あと、supported_deviceはOnnxruntimeのメソッドになりました。

supported_devices = ort.supported_devices()

iOSのXCFramework

Rust APIとC APIだけ、dlopen/LoadLibrary*ではなくロード時動的リンクで動く形でビルドできるようにしています。

C APIだと、GHAでのパッケージング時に次のどちらかのコメントアウトをsedで外すようにします。

//#define VOICEVOX_ONNXRUNTIME_LIBLOADING
//#define VOICEVOX_ONNXRUNTIME_LINK_DYLIB

#if !(defined(VOICEVOX_ONNXRUNTIME_LIBLOADING) || defined(VOICEVOX_ONNXRUNTIME_LINK_DYLIB))
#error "either `VOICEVOX_ONNXRUNTIME_LIBLOADING` or `VOICEVOX_ONNXRUNTIME_LINK_DYLIB` must be enabled"
#endif

#if defined(VOICEVOX_ONNXRUNTIME_LIBLOADING) && defined(VOICEVOX_ONNXRUNTIME_LINK_DYLIB)
#error "`VOICEVOX_ONNXRUNTIME_LIBLOADING` or `VOICEVOX_ONNXRUNTIME_LINK_DYLIB` cannot be enabled at the same time"
#endif

iOSのXCFrameworkのみVOICEVOX_ONNXRUNTIME_LINK_DYLIBで、他はVOICEVOX_ONNXRUNTIME_LIBLOADINGです。

libonnxruntimeとlibvoicevox_onnxruntime

Onnxruntime.LIB_NAME = "onnxruntime"としていますが、https://github.com/VOICEVOX/voicevox_project/issues/24以降はこれを"voicevox_onnxruntime"にする方向で考えています。考えているのは大体次の通りです。

  1. libonnxruntimeも普通に読めるようにした上で、リポジトリ上のテストでは引き続きlibonnxruntimeを使う
  2. VOICEVOX_ONNXRUNTIME_LINK_DYLIBの場合リンク対象をlibonnxruntimeのままにする。変えたければpatchelfかinstall_name_toolでバイナリを変更するという形 (実際、今のCOREのXCFrameworkをビルドするにあたってはinstall_name_toolが使われています)
  3. voicevox-ortはlibvoicevox_onnxruntimeのダウンロードを行わない。ユーザーはダウンローダーでlibvoicevox_onnxruntimeをダウンロードする

関連 Issue

Resolves #721. Fixes #537. Closes #445.

その他

qryxip commented 2 weeks ago

まだドキュメントやテストが詰めきれていませんが、とりあえずこのパブリックAPIがUX的にどうなのかご意見を頂けたらなと思っています。

qryxip commented 2 weeks ago

onnxruntime-libloadingとonnxruntime-link-dylibですが、実行時挙動はどうにもならないとして、APIの形だけでも一つにまとまらないかと考えてみました。macOSのXCFrameworkや、Swift API作成のことを考えるとAPIの形が分かれているのが気になった次第です。

以下のようにすれば、onnxruntime-link-dylibはonnxruntime-libloadingから単にできることを減らしたものになるはずなので、ドキュメントをちゃんと書けばユーザーを驚かせずにすむのではないかと思いました。 (着想を得たのはPythonのctypesです。どういう理由で用意されているかはわかりませんが、Unix限定でctypes.CDLL(None)というのができます)

class Onnxruntime:
    LIB_VERSIONED_FILENAME: str

    # Python APIやJava APIにおいては`filename=None`は実際には許可しない
    def init_once(filename: str | None = LIB_VERSIONED_FILENAME) -> Self:
        ...
Hiroshiba commented 2 weeks ago

@qryxip とりあえず最後のコメントへのコメントです！

ctypes.CDLLのような形、面白いですね。UXとしてありな気がしました！

Windows, link-dylib, filename=None: 「Windowsでは無理」というエラー

このときって「ロード時動的リンクされている方のもの」は使えない感じですかね？ たぶん今のコアがこの形（ビルド時リンク）だと思ってて、だとしたらできるんじゃないかなと思った次第です。 というのも、たぶんエンジンもこの形になるので、雑にNULLでいけるなら楽そうだな〜と。

他は僕の知識量からコメントできることはなさそうでした！！ UX的には結構便利そうに感じます。

追記：あ、この辺りはそもそもWindowsで２種類作るのかによりそう。 １種のがよければエンジン側が頑張って合わせるのが良さそう。

あ。仮称かもなのですが、libloadingやlink-dylibの名称がややこしめなのがちょっと気になりました！ たぶんdylibは避けて、役目で分けちゃうのがわかりやすいかなと。 で、「iOSのdylibはこれ使うと良いよ」みたいな案内がある感じとか。

名称･･･この辺りのリンク方法？、なんて呼ぶんですかね。。。 libloadingは実行時の読み込みだから、dynamic-loadとか･･･？ link-dylibはビルド時のリンクだから、buildtime-linkとか･･･？ うーん。。。

２値だから１つにまとめてtrue/falseで制御するとかもありかも。 だとしたらlinkする（link-dylib）かしない（libloading）かだから、onnxruntime-linkでtrueとfalseとか･････？

qryxip commented 2 weeks ago

Windows, link-dylib, filename=None: 「Windowsでは無理」というエラー

このときって「ロード時動的リンクされている方のもの」は使えない感じですかね？ たぶん今のコアがこの形（ビルド時リンク）だと思ってて、だとしたらできるんじゃないかなと思った次第です。

SymInitialize(W)を使ってよいのなら「externなONNX RuntimeのAPIに対する実体を持っているonnxruntime.dll」を見つけだすことができるのですが、ちょっとこれを使うのには抵抗を覚えました。

というのも、たぶんエンジンもこの形になるので、雑にNULLでいけるなら楽そうだな〜と。

追記：あ、この辺りはそもそもWindowsで２種類作るのかによりそう。 １種のがよければエンジン側が頑張って合わせるのが良さそう。

単に"onnxruntime.dll"とだけ指定すれば前もってctypes.CDLLで開いた方のonnxruntime.dllを開いてくれるので、compatible_engineだとそれでいいのではと思います。voicevox_onnxruntime.dllでも同様 (確かめました)。

あ。仮称かもなのですが、libloadingやlink-dylibの名称がややこしめなのがちょっと気になりました！ たぶんdylibは避けて、役目で分けちゃうのがわかりやすいかなと。 で、「iOSのdylibはこれ使うと良いよ」みたいな案内がある感じとか。

２値だから１つにまとめてtrue/falseで制御するとかもありかも。 だとしたらlinkする（link-dylib）かしない（libloading）かだから、onnxruntime-linkでtrueとfalseとか･････？

RustのfeatureはAPIの拡張を行うもので「デフォルトの挙動」を変えるべきではないとされているので、それでやるとしたらonnxruntime-link | onnxruntime-no-linkの二択で両方選択したらコンパイルエラーという形ですね。 (といってもfeatureでデフォルトの挙動を変えるクレートは世に溢れていて、ortもその一つだったりするのですが…)

名称･･･この辺りのリンク方法？、なんて呼ぶんですかね。。。 libloadingは実行時の読み込みだから、dynamic-loadとか･･･？ link-dylibはビルド時のリンクだから、buildtime-linkとか･･･？ うーん。。。

「静的リンク」, 「ロード時動的リンク」, 「実行時動的リンク」の三つのどれかなのか混乱しないような名前ですかね… (pykeio/ortはONNX Runtimeの静的リンクをやっているライブラリだったりするし)

Hiroshiba commented 2 weeks ago

SymInitialize(W)を使ってよいのなら「externなONNX RuntimeのAPIに対する実体を持っているonnxruntime.dll」を見つけだすことができるのですが、ちょっとこれを使うのには抵抗を覚えました。 単に"onnxruntime.dll"とだけ指定すれば前もってctypes.CDLLで開いた方のonnxruntime.dllを開いてくれるので、compatible_engineだとそれでいいのではと思います。voicevox_onnxruntime.dllでも同様 (確かめました)。

よくわかってないのですが、エンジン側はたしかに名前指定で良さそうに思いました！ よくよく考えれば今そんな感じになってました。ファイル名で検索して見つかったらそれ指定、みたいな感じだった記憶。

となると、掲げられた2^3パターンの挙動で良さそう感！！

RustのfeatureはAPIの拡張を行うもので「デフォルトの挙動」を変えるべきではないとされているので

おおーなるほどです。それはそうかも。 となると、片方をデフォルトにし、もう片方はプラスにするみたいなのもありかも？ 今のmainブランチの実装（ビルド時にリンクする）をデフォルトとするならonnxruntime-no-linkだけ、ビルド時リンクしないのをデフォルトにするならonnxruntime-linkで良さそう。link-on-buildとかでも良いかもしれない。

「静的リンク」, 「ロード時動的リンク」, 「実行時動的リンク」の三つのどれかなのか混乱しないような名前ですかね…

link-on-buildとlink-on-runtimeとかで良い気もしてきました。 まあ最悪注釈コメントかドキュメントがあれば別に良いかも。

qryxip commented 2 weeks ago

onnxruntime-loadとonnxruntime-link-cdylibでどうでしょうか? 「ロード時動的リンク」(run-time dynamic linking)ってあまり言わない気がしますし、"link"で通じそうな気がします。 (といっても正確に区別するときにロード時動的リンクと言ったりはするのですが)

追記: 静的リンクをやる予定が一切無いならonnxruntime-linkでもいいかも?

Hiroshiba commented 2 weeks ago

onnxruntime-loadとonnxruntime-link-cdylibでどうでしょうか? 「ロード時動的リンク」(run-time dynamic linking)ってあまり言わない気がしますし、"link"で通じそうな気がします。

良さそう！！ onnxruntime-linkも良さそう。他にはonnxruntime-load-cdylibも良さそう！ どれもややこしさは解消されてると感じます！！

qryxip commented 2 weeks ago

b1dca9c (#802) f0c720c (#802) 4af36e0 (#802) 249f8f3 (#802) 7b57268 (#802) e8dc442 (#802) フィーチャ名はload-onnxruntimeとlink-onnxruntimeにしてみました。

…ちょっと"load"と"link"で目が滑りそう。load-ortとlink-ortにしたらましになるかも? (実際今CIが落ちまくったのは、私が"load"と"link"を二箇所で間違えたためです)

qryxip commented 2 weeks ago

https://github.com/VOICEVOX/voicevox_core/pull/802#issuecomment-2195444056 放送でも話したのですが、やはりどうあがいても二つは別々の挙動ですし、Swift APIも多分XCFramework経由でONNX Runtimeを読むことになりそう(i.e. "load"ではなく"link"で固定)なのでこのアイデアは取り止める方向にしようと思います。

qryxip commented 2 weeks ago
qryxip commented 2 weeks ago

ae1c71a (#802) ドキュメントで"ONNX Runtimeのfilename"としていたところを、"ONNX Runtimeのファイル名（モジュール名）もしくはファイルパス"にしてみました。

Hiroshiba commented 2 weeks ago

やはりどうあがいても二つは別々の挙動ですし、Swift APIも多分XCFramework経由でONNX Runtimeを読むことになりそう(i.e. "load"ではなく"link"で固定)なのでこのアイデアは取り止める方向にしようと思います。

どういうときにどっちを使うのか、がわかりやすい形で案内できるならそれでも良いと思います！

ちなみに２つの経路で別の関数を使うのであれば、featureで切り替えることなくどちらも実装できたりしますか･･･？

qryxip commented 2 weeks ago

ちなみに２つの経路で別の関数を使うのであれば、featureで切り替えることなくどちらも実装できたりしますか･･･？

APIの表面は別にこうしなきゃいけないという制約は無いと思うんですが、2つの経路はほぼ直交しているので片方は"unsupported"と返すくらいしかできないんじゃないかと思ってます。ならconditional compilationが楽にできるC, Rust, (あとSwift?)は分岐したままでいいんじゃないかなーと。ロード時動的リンクの用途も「dlopen入りのバイナリはストアにリジェクトされるので抜きにしたい」くらいしか思い浮かばないですし…

Hiroshiba commented 2 weeks ago

あ、dlopenがバイナリに含まれてるとリジェクトされるんでしたっけ。だとしたら無くさないといけない気がします。

分ける場合（片方の関数が見えない場合）、ドキュメントの案内が大変になるだろうなーと想像しています。 C APIを使う場合、

みたいなのがいろんなとこに必要になりそうです。

書いてて実行時にdefineしなきゃいけないことに気づきました。 この辺りのややこしさがあるので、どっちでも良いならまとめた方が良いのでは派です。

qryxip commented 2 weeks ago

バイナリのリリースではiOSだけlinkで、他は全部loadでいいと思います (今のPRの状態がそうなっています)。iOSのリリース形態はXCFrameworkなのでちょうどよい区分になるかと。あとバイナリリリースではヘッダをsedして片方を#defineしているので、ユーザーが選ぶという感じではないような感じがします。ただ「iOSだけ違うので注意」ということにはなりそうてすが。

qryxip commented 2 weeks ago

c79c3f2 (#802) init_onceとload_onceの片方のみが利用であることがC APIのドキュメントからわからないので、"Availability"というセクションを追加してみました。

image image

Hiroshiba commented 2 weeks ago

バイナリのリリースではiOSだけlinkで、他は全部loadでいいと思います (今のPRの状態がそうなっています)。

こちらは賛成です！

あと気づいたのですが、C 言語・Rust以外もlink/load片方だけ提供で良さそうなことに気づきました！ なのでCとRustのドキュメントがOK なら 良さそう！

Availability

良さそう！　だけど結構見ない人多そう･･･！　それでもここに案内があればそれで良さそう！

UX に関して色々考えてみました！ 結論から言うと、関数は分けていいと思います！！

先ほどの通り、CとRust以外のドキュメントはそもそもlink/load片方しか出ないので無視できるはず。 あとはCとRustでどう調理するかかなと！

RustとC言語APIは、結構プログラミングを知ってる人を想定して良いと思います。 なので、結構プログラミングが分かってる人に伝わるドキュメントが書けるならOK。

Rustユーザーは相当分かっていると思うので、link/loadをそのまま 説明してもOK。 link用とload用の関数が分かれているのもOK。多分むしろ別れていた方が良い。

Cユーザーはわりとライト層だと思うので、link/loadをそのまま説明するのは避けた方が良さそう。 代わりに「iOSかどうか」で説明を分けるのなら伝わりそう。この説明だったら別れてても良いはず。

Rust向けドキュメントは置いといて、C向けドキュメントは各関数ごとに「iOSでしか使えないか、iOS以外でしか使えないか」が案内されてれば良さそう！ 一応ちゃんとわかってる人向けに、リンクなのかロードなのかみたいな説明もあっても良さそう。

Cユーザー向けのVOICEVOXコアの使い方がいろんな記事で紹介され始めたらもっと簡略化しても良さそう。 それまではちょっとメンテナンスめんどくさいけど丁寧かつ初学者にもちょっと分かりやすい説明の方が良さそう

Rust向けドキュメントは･･･お任せします！！ 多分 @qryxip さんが分かりやすい形が一番わかりやすい！

qryxip commented 2 weeks ago
qryxip commented 1 week ago

c545f89 (#802) f60cd0e (#802) このPRの主旨から若干外れますが、cargo test -p voicevox_core --features {load,link}-onnxruntime (非--workspace)がdoctestを含めて通るようにしました。 (このPRで織り込んだdoctest用の工夫が今後壊れるかもしれないため) 追記: CIはそのままにしました。非--workspaceビルドでのテストはそんなに重要じゃないので。

qryxip commented 1 week ago
Hiroshiba commented 1 week ago

ちょんと把握できてないのですが、とりあえず問題なければ（mainにマージすればテストは通るとか、リリースを一度すれば通るとか）マージしても大丈夫かなと！

qryxip commented 1 week ago

問題はある（ダウンローダーで--device directmlを指定するとonnxruntime_providers_cuda.dllまで付いてくるし、リリースはできてもdownload_testは落ちる）のですが、onnxruntime-rsからortに切り替えた時点でこの問題は発生しており、このPRに起因はしていないと思います。

qryxip commented 1 week ago

804

Hiroshiba commented 1 week ago

すいませんちょっとわかっておらず。。。 🙇 あ！　ortに切り替える時点でdownload_testは落ちるはずだったんだけど、download_testの稼働条件にcargo.tomlの更新が含まれいなくてテストが回ってなく、このプルリクエストで初めてテストが回った、みたいな感じでしょうか？

ま～だとしたらこのプルリクエストはマージしちゃってもいいのかなと思いました！！ ただmainブランチのテストが落ちちゃうことにはなる？ので、気になるようでしたら先にそっちを修正でもいいかなと。

どちらでも！！別にいいならマージしていただければ！ （個人的にはどっちにしろ一長一短ですが、このプルリクエストは大きいのでとりあえずマージが進めやすいのかなと思いました。）

qryxip commented 1 week ago

mainブランチのテストが落ちちゃうことにはなる？ので

このリポジトリでdownload_testが最後に動いたのは5ヶ月前ですね。 https://github.com/VOICEVOX/voicevox_core/actions/workflows/download_test.yml

download_testの稼働条件にcargo.tomlの更新が含まれいなくてテストが回ってなく、このプルリクエストで初めてテストが回った、みたいな感じでしょうか？

こんな感じです…

https://github.com/VOICEVOX/voicevox_core/blob/d3b559cc0dc56b298c4a4e67453c8af9449e935c/.github/workflows/download_test.yml#L10-L19

私が貼ったGHAのログは、qryxip/voicevox_coreで「バージョン999.999.999のリリース」を行うことによってdownload_testを作動させたものです。

Hiroshiba commented 1 week ago

あーーーーーーなるほどです！！！ このPRのテストが落ちていたのが、download_test由来だと勘違いしてました！！！ そうか、テスト落ちるよねってことでそういえば停止してましたね。。。

追加コミットも見ました、コード的にはOKなのでマージしてOKならしていただければ！！ （完了のタイミングがわからない）

qryxip commented 1 week ago

@takejohn 共有です!