Node.js向けのFFIを実装 #758
Node.js向けのFFIを実装 #758
Conversation
This reverts commit ea809fb.
|
Ready for reviewとしてマークしました。 |
There was a problem hiding this comment.
Pythonと違ってJavaScriptは非同期がメイン（主観）なので、blocking APIは提供しなくてもいいと思います（個人の思想ですが）
There was a problem hiding this comment.
NodeではないですがDenoだと例えば標準ライブラリの各機能に対して同期版があったりします (e.g. Deno.readFileに対してDeno.readFileSync)。ただNodeでもそれをやるべきかというと微妙かもしれませんが。
@Hiroshiba #662 の観点から一言お願いします。
There was a problem hiding this comment.
絶対ないといけないとは思わない、とはいえ実装を拒む理由も特にない、って感じですかね！
webのjsならともかく、nodeを使う人のレベル感は割と高そうなので、非同期版のみでも大丈夫だと思います。
とはいえ別にあったらダメというわけでもない気がしました。トップレベルasyncが書けない場合があったりとか、何かと便利ではあると思うので。
There was a problem hiding this comment.
-muslまでの差分は提供しなくてもいいと思います。
|
ありがとうございます。自分で対応してみます。 |
| __dirname, | ||
| "..", | ||
| "..", | ||
| "..", |
There was a problem hiding this comment.
| "..", | |
| "test_util", | |
| "data", |
There was a problem hiding this comment.
.gitattributesでlinguist-vendored linguist-generatedを設定した方がよいかも。
| `npm test` でテストを行うことができます。 | ||
|
|
||
| ```console | ||
| ❯ npm test |
There was a problem hiding this comment.
テスト前にcargo build -p test_utilが要るんじゃないかと思います。
(クリーンなリポジトリからvoicevox_core_node_apiだけビルドした場合、crates/test_util/dataは生成されない)
参考:
voicevox_core/.github/workflows/test.yml
Line 279 in 18aec9f
voicevox_core/.github/workflows/test.yml
Line 315 in 18aec9f
npm test内に仕込むのがいいかもしれませんが、どうやるかは私にはよくわからないです。なんかnpm-run-all2っていう、かつてのnpm-run-allのフォークがあるっぽい…?
There was a problem hiding this comment.
"test": "run-s test:build-util test:ava",
"test:build-util": "cargo build -p test_util",
"test:ava": "ava"npm-run-all2使うとこんな感じだと思います。
まぁ直列ならOS差は困るほどはないので、
"test": "cargo build -p test_util && ava"でいいかも。avaにセットアップのHookがあるならそれでビルドしてもいいかも？
| #![deny(clippy::all)] | ||
|
|
There was a problem hiding this comment.
NAPI-RSの慣習だと思いますが、このリポジトリの場合はCIでやってるので不要かと思います。
voicevox_core/.github/workflows/test.yml
Lines 75 to 76 in 18aec9f
ClippyというのはVSCodeとかのエディタ上で動くので(もし設定されていなかったのであれば設定することをお勧めします)、少しうっとおしくなってしまうかと思います。
| #![deny(clippy::all)] |
|
https://github.com/VOICEVOX/voicevox_core/actions/runs/9202752835/job/25313764046?pr=758 あ、なんかの依存の更新でCargo.lockのcbindgenのバージョンが上がったっぽいですね。このCIの直しかたは ❯ cargo xtask update-c-headerです。 |
| pub fn unload_voice_model(&self, voice_model_id: String) -> Result<()> { | ||
| convert_result( | ||
| self.synthesizer | ||
| .unload_voice_model(&VoiceModelId::new(voice_model_id)), |
| impl JsSynthesizer { | ||
| /// `Synthesizer`をコンストラクトする。 | ||
| #[napi(constructor)] | ||
| pub fn new(open_jtalk: &JsOpenJtalk, options: Option<InitializeOptions>) -> Result<Self> { |
There was a problem hiding this comment.
Onnxruntimeが追加され、Synthesizerのコンストラクタに要求されるようになりました。
| [dependencies] | ||
| napi = { version = "2.16.0", features = ["napi4", "serde-json", "tokio_rt"] } | ||
| napi-derive = "2.16.0" | ||
| voicevox_core.workspace = true |
There was a problem hiding this comment.
- featureとして
load-onnxruntimeとlink-onnxruntimeが追加されました。ただしPythonとJavaではload-onnxruntimeで固定です。Node.jsもそうすることになると思います。
| voicevox_core.workspace = true | |
| voicevox_core = { workspace = true, features = ["load-onnxruntime"] } |
| /// `SupportedDevices`をコンストラクトする。 | ||
| #[napi(factory)] | ||
| pub fn create() -> Result<Self> { | ||
| match SupportedDevices::create() { | ||
| Ok(val) => Ok(JsSupportedDevices { | ||
| supported_devices: val, | ||
| }), | ||
| Err(err) => Err(Error::from_reason(err.to_string())), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
supported_devicesはOnnxruntimeのメソッドになりました。
| } | ||
|
|
||
| #[napi] | ||
| impl JsSynthesizer { |
There was a problem hiding this comment.
SynthesizerはOnnxruntimeのgetterを持つようになります。
|
@takejohn お久しぶりです。 |
お願いします！ |
|
@takejohn わかりました！ではこちらで進めさせていただきます。 @Hiroshiba これからの段取りについて、次の形を考えています。どうでしょうか？
あと正式リリースできる日が来たらNode.js APIを作った人として @takejohn さんのお名前も… あれ？どういう風な紹介になりますかね？ |
|
方針良さそうに感じました！！ お名前紹介の際は２人の名前を紹介する感じで良いかなと！ |
内容
NAPI-RS を用いて Node.js 向けの FFI を実装します。
関連 Issue
ref #550
その他
#757 で Neon を使おうと思っていましたが、より開発が活発で情報が見つかる(と思われる) NAPI-RS を使うことにしました。