-
Notifications
You must be signed in to change notification settings - Fork 117
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
ストリーミング処理のC API実装 #866
Comments
issue作成ありがとうございます!! 特徴次元数について、かなり特殊になってしまうのですが、ちょっと関数の引数として受け取る形でどうでしょうか・・・? ↓ご説明です 🙇 実はdecodeへ入力するときに特徴量次元が必要になっています。 コア側の
エンジン側からコアを呼ぶ部分の実装 ちなみに固定値80なのでほんとは定数でもよいのですが、APIは一貫性を持たせて以前のに合わせるのが良さそうなので、この形で行ければと思った次第です。。。。。 なんかいびつな実装ですが。。。。。この形でいければ。。。。 🙇 ちょっと3年くらい経ってるので考慮漏れが多くなってしまっていて申し訳ないです 🙇 |
方針3.でよいと思います。 ちなみにもし方針1.にする場合はこうなるかなとuse std::ptr::NonNull;
use ndarray::Array2;
// この`AudioFeatureInternalState`はポインタでしかやりとりしないので、使う側 (i.e. VOICEVOX ENGINE)
// がサイズやアラインメントを知らなくても全く問題無い
type AudioFeatureInternalState = Array2<f32>;
// SAFETY: voicevox_core_c_apiを構成するライブラリの中に、これと同名のシンボルは存在しない
#[unsafe(no_mangle)]
pub unsafe extern "C" fn generate_full_intermediate(
// vvvvvvvvvvvvvvvvvv
length: i64,
phoneme_size: i64,
f0: *mut f32,
phoneme: *mut f32,
speaker_id: *mut i64,
// ^^^^^^^^^^^^^^^^^^ ここまで`decode`と同じ
output: NonNull<Box<AudioFeatureInternalState>>, // ABI的に`*mut *mut AudioFeatureInternalState`として扱ってよく、Cの表現では`**AudioFeatureInternalState`となる
) -> bool {
let internal_state: AudioFeatureInternalState = todo!();
output.write_unaligned(internal_state.into());
true
}
// SAFETY: voicevox_core_c_apiを構成するライブラリの中に、これと同名のシンボルは存在しない
#[unsafe(no_mangle)]
pub unsafe extern "C" fn render_audio_segment(
// vvvvvvvvv
length: i64,
// ^^^^^^^^^ `decode`と同じ
internal_state: &AudioFeatureInternalState, // ABI的には`*const AudioFeatureInternalState`として扱ってよく、Cの表現では`const *AudioFeatureInternalState`となる
// vvvvvvvvvvvvvvvvvv
speaker_id: *mut i64,
output: *mut f32,
// ^^^^^^^^^^^^^^^^^^ `decode`と同じ
) -> bool {
todo!();
}
// SAFETY: voicevox_core_c_apiを構成するライブラリの中に、これと同名のシンボルは存在しない
#[unsafe(no_mangle)]
pub extern "C" fn delete_audio_feature_internal_state(
internal_state: Box<AudioFeatureInternalState>, // ABI的には`*mut AudioFeatureInternalState`として扱ってよく、Cの表現では`*AudioFeatureInternalState`となる
) {
// `Box`として扱った時点で、スコープから外れたときにデストラクトされることが決定されるが、
// 未使用変数警告を黙らせるために明示的に`drop`しておく
drop(internal_state);
} |
@qryxip (たぶんcompatible_engineは以前のAPIと合わせた方が使い勝手が良くて、特にダメな理由がなければVOICEVOX ENGINEから渡してもらう設計にした方が良いかなーーーと思っており。。。) |
あ、方針3.自体を読み違えていました。ヒホさんの案で良いと思います! |
すみませんありがとうございます 🙇 🙇 🙇 |
#866 (comment) もう一つ課題が見つかりまして、
どちらが良いでしょうか。無音パディングはvoicevox engineでやっていたかとおもうのでマージン処理も後者にしてしまったほうが個人的にはいいのかなと思います(rust側のtestでもマージン処理を追加で書くことになりますが..) |
VOICEVOX ENGINEでやり、VOICEVOX ENGINEからは COREとENGINEで同じような実装をすることになると思いますが、Discordのここ(↓)の前後で議論してた「二重実装」はそのことを指していました。今のところ、仮にcompatible_engineをPyO3に置き換えることになったとしても、この二重実装だけは続けることになるかなと思っています。 |
あ、マージン追加というのを誤読していました。 以下のコードは #854 以前ものですが、これがそのまま「ENGINEから見たCOREのAPI = compatible_engine」の実装になっています。これに従えば、マージン処理はcompatible_engine、というより voicevox_core/crates/voicevox_core/src/synthesizer.rs Lines 858 to 1011 in fdae73a
(というか今、compatible_engineから |
たしかにそうですね |
別PRで修正します |
→ #867 |
ちょっとこんがらがってきたんですが、もともと
だったものに対して #854 は
になっていたんですが、これを
とすればいいですかね?無音パディング幅をどこに記録するか問題になりそうですが、面倒なのでハードコーディングしてしまったほうがいいですかね |
分類については私もその認識です。 ただ |
すみません遅くなりました!! @Yosshi999 さんのまとめめちゃくちゃ参考になりました!!!! 僕もお2人と一緒の意見で、
これをどうしようか迷ってます。。。
整理すると、
こうすれば これを @Yosshi999 さんの表に適用してみるとこうなるかな~と思ってます。どっか間違ってるかも。。。
この形だと、 |
マージン幅やパディング幅はvoicevox_coreにマジックナンバーとして埋め込んでしまって、voicevox_engineやAPIユーザーには全く意識させないのがいいのかなと思います。中間生成物をユーザーがいじるのは完全に保証対象外にしてしまい、spec全体, start, endの組でAPIとやり取りするのが一番楽そう 無音パディング幅をどこに記録するかについてもマジックナンバーにしてしまえば問題なくなります |
たしかにと思ったのですが、voicevox engine側はメモリ確保が必要なので、少なくともprecompute用のマジックナンバーは必要になるかもです!! |
一応、方針1. ( |
AudioFeatureを うーんどれが良いかな〜〜〜 ほんとになんとなくなんですが、specを毎回受け渡すのは、実装によってはメモリコピーが結構発生しそうなので避け調子が良い気がしてます。 @qryxip さんのBox使う形式はdelete関数が必要になるのでVOICEVOX ENGINE側の実装が多少ややこしくなるかも、くらい。 メモリコピーが発生しないように意識して実装するか、delete関数を召喚するか、spec範囲の計算をVOICEVOX ENGINE側でもやるか、どれにしますかねって感じかなと!!! 個人的には、これまでもVOICEVOX ENGINE側に前処理書いてる(こういうのとか)のと、あとメモリコピー意識とdelete関数召喚は避けたい気持ちもあり、まあ今回もVOICEVOX ENGINE側でコード書く感じが良いのかな〜とちょっと思ってます! |
メモリについてだいぶ齟齬が発生しかけてる気がするので私の考えを述べておくと、#687 をやることを前提にした場合、どの方法であっても分割部分のコピーは避けられるかなと思っています。何故ならRustの [追記] viewというか、 ENGINE側でNumPyを持つ場合は
|
なるほどです。
これはちょっと読み切れてないんですが、おそらくWEBリクエストのセッションが切れたら破棄が手っ取り早いそうかな~~~~と思ってます。 ただまあもうプルリクエスト作ってくださってるので、あとはcompatible_engine.rsの実装と、PythonのVOICEVOX ENGINEの実装を想像しつつ、どれがいいか選ぶ感じかな~~~と。
個人的には「 |
内容
#853 をC APIのcompatible_engineで対応させる
Pros 良くなる点
ストリーミング処理をvoicevox_engineに実装するための足掛かりになる
Cons 悪くなる点
実現方法
以前の議論:#853 (comment)
現在のdecode_forwardを分割して
generate_full_intermediate
とrender_audio_segment
を実装することになるが、一つ問題がある。中間生成物の音声特徴量(internal_state: ndarray::Array2)が露出することは問題がないが、この配列のwidth (特徴次元数)が未定義であるため、APIを呼び出す側がバッファサイズを設定できない
解決方法:
方針1. spectrogramはrust側に管理してもらう→compatible_engineにステートを持たせてhandleもしくはvoid*でAPIとやりとりする (lockやmemory周りが面倒そう)
方針2. generate_full_intermediateは二回呼んでもらう。一回目はoutput=nullptrでスペクトログラム次元数 or 配列長を返し、二回目でoutputにデータを埋める (二度手間、若干の速度低下)
方針3. スペクトログラム次元数を定数もしくはstyle_idに対する関数のAPIを生やす(モデルに変更が入った時の保守が面倒)
そもそも中間生成物が二次元配列であること自体を隠蔽すべき?そうなると方針1しかなさそう
The text was updated successfully, but these errors were encountered: