-
Notifications
You must be signed in to change notification settings - Fork 120
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
change: Onnxruntime
型を追加し、そこからdlopen
/LoadLibrary*
を行う
#802
Conversation
まだドキュメントやテストが詰めきれていませんが、とりあえずこのパブリックAPIがUX的にどうなのかご意見を頂けたらなと思っています。 |
以下のようにすれば、 class Onnxruntime:
LIB_VERSIONED_FILENAME: str
# Python APIやJava APIにおいては`filename=None`は実際には許可しない
def init_once(filename: str | None = LIB_VERSIONED_FILENAME) -> Self:
...
|
@qryxip とりあえず最後のコメントへのコメントです! ctypes.CDLLのような形、面白いですね。UXとしてありな気がしました!
このときって「ロード時動的リンクされている方のもの」は使えない感じですかね? 他は僕の知識量からコメントできることはなさそうでした!! 追記:あ、この辺りはそもそもWindowsで2種類作るのかによりそう。 あ。仮称かもなのですが、 名称・・・この辺りのリンク方法?、なんて呼ぶんですかね。。。 2値だから1つにまとめてtrue/falseで制御するとかもありかも。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一通り全部眺めました!!良さそう!!!
サンプル見た感じとてもわかりやすいですね!!
特にPythonとかの。
C APIのサンプルは、たぶん一方のしかないからわかりやすいんだろなーと思いました。
ビルド時リンクか動的読み込みかにかかわらず、同じ関数を使えるようになれば、実際に使うときも混乱を減らせる気がします・・・!!
いや〜〜〜楽しみですね!!!!!
単に
RustのfeatureはAPIの拡張を行うもので「デフォルトの挙動」を変えるべきではないとされているので、それでやるとしたら
「静的リンク」, 「ロード時動的リンク」, 「実行時動的リンク」の三つのどれかなのか混乱しないような名前ですかね… |
よくわかってないのですが、エンジン側はたしかに名前指定で良さそうに思いました! となると、掲げられた2^3パターンの挙動で良さそう感!!
おおーなるほどです。それはそうかも。
|
追記: 静的リンクをやる予定が一切無いなら |
良さそう!! |
…ちょっと"load"と"link"で目が滑りそう。 |
#802 (comment) |
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
ドキュメントはもしかしたらリリースまでに変更させていただくかもですが、とりあえずマージして進めるのは問題なさそう!!
楽しみですね!!
もしかしたらまだ何か残ってるかもなのでマージせず置いています。
問題なければマージしていただければ!!
|
ちょんと把握できてないのですが、とりあえず問題なければ(mainにマージすればテストは通るとか、リリースを一度すれば通るとか)マージしても大丈夫かなと! |
問題はある(ダウンローダーで |
すいませんちょっとわかっておらず。。。 🙇 ま~だとしたらこのプルリクエストはマージしちゃってもいいのかなと思いました!! どちらでも!!別にいいならマージしていただければ! |
このリポジトリで
こんな感じです… voicevox_core/.github/workflows/download_test.yml Lines 10 to 19 in d3b559c
私が貼ったGHAのログは、qryxip/voicevox_coreで「バージョン |
あーーーーーーなるほどです!!! 追加コミットも見ました、コード的にはOKなのでマージしてOKならしていただければ!! |
@takejohn 共有です!
|
内容
パブリックAPIに
Onnxruntime
型を追加し、そこからlibonnxruntimeのdlopen
/LoadLibrary*
をするようにします。Onnxruntime
型Synthesizer
のコンストラクトはこのようになります。オプションとしてDLLの"filename"を指定することもできます。
あと、
supported_device
はOnnxruntime
のメソッドになりました。iOSのXCFramework
Rust APIとC APIだけ、
dlopen
/LoadLibrary*
ではなくロード時動的リンクで動く形でビルドできるようにしています。C APIだと、GHAでのパッケージング時に次のどちらかのコメントアウトを
sed
で外すようにします。VOICEVOX_ONNXRUNTIME_LIBLOADING
の場合、dlopen
/LoadLibrary*
でlibonnxruntimeをロードします。VOICEVOX_ONNXRUNTIME_LINK_DYLIB
の場合、libonnxruntimeをロード時動的リンクします。load_once
はinit_once
となり、filename
のオプションが消失します。iOSのXCFrameworkのみ
VOICEVOX_ONNXRUNTIME_LINK_DYLIB
で、他はVOICEVOX_ONNXRUNTIME_LIBLOADING
です。libonnxruntimeとlibvoicevox_onnxruntime
Onnxruntime.LIB_NAME = "onnxruntime"
としていますが、VOICEVOX/voicevox_project#24以降はこれを"voicevox_onnxruntime"
にする方向で考えています。考えているのは大体次の通りです。VOICEVOX_ONNXRUNTIME_LINK_DYLIB
の場合リンク対象をlibonnxruntimeのままにする。変えたければpatchelf
かinstall_name_tool
でバイナリを変更するという形 (実際、今のCOREのXCFrameworkをビルドするにあたってはinstall_name_tool
が使われています)関連 Issue
Resolves #721.
Fixes #537.
Closes #445.
その他