feat(rust): Rust APIのVOICEVOX Song #1073
Conversation
Co-authored-by: Hiroshiba <[email protected]> Co-authored-by: Yuto Ashida <[email protected]>
qryxip
force-pushed
the
pr/feat-voicevox-song
branch
from
86a5f97 to
7c12e27
Compare
Rust APIの`crate::engine`にあるアイテムを、`acoustic_feature_extractor` と`audio_file`を除いて`crate::engine::talk`に移動する。 `acoustic_feature_extractor`や`OjtPhoneme`という名前が微妙になりつつある が、考えるのは今後ということにする。 #1074 (comment) Refs: #1073 See-also: #1065
|
考えていること:
(Discord: https://discord.com/channels/879570910208733277/893889888208977960/1360825986941190357) |
うおーどうでしょう、実際のユースケースを想定して便利に使えそうも考えておいたほうが良いかもです。（ちょっと不安） 例えばUIだとこういう使われ方になります。 hoge.mp4これが無理なく実現できそうなら一旦大丈夫そう！ あとまあ、とりあえずtalk側と歩調合わせる形もありかも･･･？ |
考える必要があるのは「f0だけ再生成」と「音量だけ再生成」ですね。その二つには イメージとしてはこんな感じですね。 query_with_score: FrameAudioQueryWithScore = synth.create_sing_frame_audio_query(...)
_: FrozenScore = query_with_score.score
_: FrozenFrameAudioQuery = query_with_score.query
with query_with_score.modify_query() as query: # Pythonはコールバックに恨みでもあるのか、こういう用途には向かないためコンテキストマネージャ
_: FrameAudioQuery = query
assert type(query) is not type(query_with_score.query)
... # ユーザーが`f0`を編集
volume = synth.generate_sing_frame_volume(query) # https://github.com/VOICEVOX/voicevox/pull/2015
with query_with_score.modify_query() as query:
query.volume = volume
ソングでは元の うーんソングのことがまだよくわからない。例えば VOICEVOX/voicevox#2369 において VOICEVOX/voicevox#2018 みたいな話はどうなるか、みたいなのがよくわからない。エディタを触ってみればもしかしたらイメージが湧くかも。 |
|
使い方ですが、とりあえずこの４通りのデータ編集・生成フローが通れればOKだと思います！
こういう機能をそのままコアでサポートする必要はないと思います。こういうことができればOKかなと。 ちなみにスコアから音声までのデータフローはこんな感じです。 |
今のエディタのトーク機能はそうなっているかもですが、需要としてはソングと同じく元のデータや中間データをもっておきたいこともあると思います！ |
|
もっておきたい、というのはAudioQueryの外に持つ感じでしょうか？確かに うーん |
たぶんそう･･･？
凝りすぎてもという気もしつつ、APIがコロコロ変わるのもアレだし、塩梅が難しいですねぇ。 |
|
ちょっと色々考えましたが、Parse, don’t validateをきっぱり諦めるという手もあるかなと思いました。 というのもRustでも「不正な入力」に対するエラーを処理本体のI/Oエラーに混ぜてしまう例がいくつかあったりするので。
ちょっと個人的には覚悟を決めてパブリックAPIを作るか、そうでなければPRをぶら下げたままの方がよいかなと思っています。APIの決定を長く先延ばしにしたい強い理由があるのならfeature-gateですかね。 |
文脈的に逆かな･･･？
個人的にPRぶら下げたままだとレビューが蓄積していって辛いので、できれば1回マージしたい気持ちはあります！ |
|
"parse with validate"という言葉に聞き馴染みがないので、とりあえず"Parse, don't validate"について補足しておきます。 元記事ではHaskellで書かれていますが、私の理解ではPythonでいうとこれ↓よりは、 def f(xs: list[int]):
if not xs:
raise ValueError("argument `xs` must not be empty")
... # `xs`が空ではないことを期待した処理こうせよということになると思います。チャットAIに記事内容を食わせても上手く要約してくれる…かもしれません。 import dataclasses
from collections.abc import Iterable, Iterator
from typing import Union
@dataclasses.dataclass
class NonEmptyList[T](Iterable[T]):
head: T
tail: list[T]
@staticmethod
def from_list(lst: list[T]) -> Union["NonEmptyList[T]", None]:
match lst:
case [x, *xs]:
return NonEmptyList(x, xs)
case _:
return None
def __repr__(self) -> str:
return repr([self.head, *self.tail])
def __iter__(self) -> Iterator[T]:
return iter([self.head, *self.tail])
def f(xs: NonEmptyList[int]):
... |
|
なるほどです！ validateしながらparseして型付けると良いよ、って感じですかね（from_listがそういう風に見えたので） なんかよくわかってないけど全然良いと思います！！ |
これですがファイルIOとかとは違って、ユーザー視点ではONNXでの推論は基本成功して欲しい感じであるという考えかたもありそうです。 pub struct ContextedFrameAudioQuery {
pub f0: Vec<f32>,
pub volume: Vec<f32>,
pub phonemes: Vec<ContextedFramePhoneme>,
pub volume_scale: f32,
pub output_sample_rate: u32,
pub output_stereo: bool,
}
pub struct ContextedFramePhoneme {
pub phoneme: String,
pub frame_length: U53,
pub note_id: Option<NoteId>,
/// 音階と歌詞。
pub key_and_lyric: KeyAndLyric,
}
impl ContextedFrameAudioQuery {
pub fn new(query: FrameAudioQuery, score: Score) -> Result<Self, _>;
pub fn to_frame_audio_query(&self) -> FrameAudioQuery;
pub fn to_score(&self) -> Score;
pub fn split(self) -> (FrameAudioQuery, Score);
}
// 以下のようなシリアライズ形式でシリアライズ・デシリアライズを行う。
// ```json
// {
// "query": …,
// "score": …
// }
// ```
impl<'de> Deserialize<'de> for ContextedFrameAudioQuery {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let ContextedFrameAudioQuerySerdeRepr { query, score } =
ContextedFrameAudioQuerySerdeRepr::deserialize(deserializer)?;
return Self::new(query, score).map_err(D::Error::custom);
}
}
#[derive(Deserialize, Serialize)]
struct ContextedFrameAudioQuerySerdeRepr {
query: FrameAudioQuery,
score: Score,
} |
Co-authored-by: Yuto Ashida <[email protected]>
|
レビューのしやすさの観点から対象をRust APIに絞り、とりあえずdraftを外しました。 |
|
Claude君にレビューさせたらdiffが大きすぎると言われ、割とどうでもいいことと事実誤認のことを列挙するだけだった…。誤字は一箇所見つけてはくれたけど。 |
Hiroshiba
left a comment
Hiroshiba
left a comment
There was a problem hiding this comment.
AIのレビューが頓珍漢なのは、変更量が大きすぎてAI君のトークンが入らないからかもです。
Claude Code君は[OUTPUT TRUNCATED - exceeded 25000 token limit]となっていました。
とりあえずこっちのAI君が発見した観点をコメントしてみました。
| pub(crate) fn phoneme_lengths( | ||
| consonant_lengths: &NonEmptySlice<i64>, | ||
| note_durations: &NonEmptySlice<U53>, | ||
| ) -> Vec<U53> { |
There was a problem hiding this comment.
これcrate内メソッドだけどpanic条件書いたほうが良いかも（ってAI君が言ってました） 個人的にはどっちでも良さそうで、他の似たようなとこでもpanic条件書いてるなら書いたほうが良さそうかなーくらい
| pub(super) fn hira_to_kana(s: &str) -> SmolStr { | ||
| s.chars() | ||
| .map(|c| match c { | ||
| 'ぁ'..='ゔ' => (u32::from(c) + 96).try_into().expect("should be OK"), | ||
| c => c, | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
１つくらい正常系のテストがあっても良いかも。あ→アになる、みたいな。
| pub fn validate(&self) -> crate::Result<()> { | ||
| self.to_validated().map(|_| ()) | ||
| } | ||
|
|
||
| pub(crate) fn to_validated(&self) -> crate::Result<ValidatedScore> { | ||
| let notes = ValidatedNoteSeq::new(&self.notes)?; | ||
| Ok(ValidatedScore { notes }) | ||
| } | ||
| } | ||
|
|
||
| impl Note { | ||
| pub fn validate(&self) -> crate::Result<()> { | ||
| self.clone().into_validated().map(|_| ()) | ||
| } |
There was a problem hiding this comment.
Score::validateとNote::validateのテストあっても良いかも（ってAI君が）
個人的にはない理由 or なくて良い理由があるならなくても良さそう感。
There was a problem hiding this comment.
この辺docstring的なのあっても良さそう。
どういうものが配置されるのかとか、それぞれ何を示してるのかとか。
このままだとなんでも集まってきそう。
不要ならなくても良いのかも。
There was a problem hiding this comment.
（このファイルと関係ないけど議論を分けたいのでこちらでコメントしています）
レビューの方針を相談したいです！
正直このPRのレビューを正確にするのは難しいと思います。
量が多いのと（これだけなら問題ないけど）、あと実際にAPIに触れないのと（Score型が難しい・Rustに不慣れなため）。
２択ありそうです。
変更行数を100～500行ほどにしたPRに分割していくか、不正確かもだけどえいやでマージするか。
個人的には、設計についてはトークと似てて把握できてるので、後者で良いと思ってます。
例えばこのあと、Python APIの実装とサンプルコードだけ作って、ちゃんと音ができてそうかを見てチェックで良いかな～と。
ちなみにclaude code君に分割案を頼んだら、まあそういう感じだよねって案が来ました。
これを元にcommit履歴を綺麗にしてもらって、コミットごとにAIレビューさせるとかもありかもですね！（面倒だから今回やるかはさておき）
https://gist.github.com/Hiroshiba/82c30500f9b71140edfd1b7ec2524624
There was a problem hiding this comment.
ちょっと今考えているのは逆で、このPRにPython, C, Java APIのも実装したい気持ちがあります。どうせ2000いっちゃってるので、PRとしてアトミックにしたい気持ちの方が強くなってきました。特にPython APIの場合、パブリックAPIの形がpyiの形で出現するのでAPIの方針について考えるならそっちの方がよいかなと。
AIにとっても、「./crates/voicevox_coreだけ見ろ」とか「PRのdiff全体はどうせ見れないだろうからエントリポイントから「ソングAPI」の実装を追え。ENGINEの実装とも比較」とかすればいけそうな気がしてます。
ちなみに出力チェックについては、ONNX推論を無理矢理モックした上でcompatible_engine + ENGINEとCORE APIで比較、というのができるんじゃないかなと思ってます。
There was a problem hiding this comment.
なんだかんだレビューしたので、このPRのコードは問題なさそうだと思っています。
追加して再レビュー大変なので、一旦今のでマージだとありがたいです。
あと2000行の変更程度だからこそ、ぎりぎりAI君が全体を見れたので、詳細なレビューをパスして良いと迅速に判断できた気がします。
これ以上大きかったら、レビューをパスする判断が妥当なのかもわからなかったかもです。
まあ実装した側的にはどうレビューすれば適切なのかわかると思うので、レビュワー向けにひたすら観点を区切ってレビューの段取りを伝えれば、レビュワー側はそれをAI頼りにレビューできるので良いのかも･･･？
でもそのレビュー観点に網羅性があるかの確認ができないので･･･やっぱレビューが難しい気がします。
あとAI君的にはGithubの議論も与えないと精度が出ないことがあるので与えるんですが、それも増えていくのでこれ以上はいろいろ難しくなりそうです。
There was a problem hiding this comment.
あ。
レビューする前提でいましたが、ソングAPIの初期実装はレビューなしでマージしたいという提案なら話は別かもです。
この場合、全言語のAPIも実装するPRであっても、コードは @qryxip さんを信頼して、あとは僕からいくつか設計等について質問しつつ、今後の段取り（APIの動作確認の流れと、リリース前に何するのか）を決めて、マージって流れにできると思います。
There was a problem hiding this comment.
なるほどです。
思い付いたのですが、このPRから部分的に切り出せるものとして、Phoneme型、Sil型、SamplingRate型のパブリック化の部分があるかもしれません。で、パブリック化をPRに切り出すんだったらC, Java, Pythonでどうするべきかも同時に語れるだけの余裕も出てきそうかと。
具体的には↓のような型をPython APIとかJava APIに爆誕させるか、あるいは単にstrとかintにするかですね。なお個人的には後者寄りなかと思ってます。というのもUint53型とかPositiveFiniteFloat型とかも誕生させるべきかという話にもなりかねないし、def validate(self) -> Noneに全てを任せるという形がいいかなと。
# PyO3のオブジェクト
class Sil:
def __new__(cls, s: str = "sil") -> "Sil": ...
def __str__(self) -> str: ...
SIL: Final[Sil] = Sil()
type Phoneme = Sil | Literal["sil", "pau", "A", "E", ...] | _Reserved
# PyO3のオブジェクト
class OptionalLyric:
def __new__(cls, s: str) -> "OptionalLyric": ...
def __str__(self) -> str: ...
# PyO3のオブジェクト
class SamplingRate:
def __new__(cls, value: int = 24000) -> "SamplingRate": ...There was a problem hiding this comment.
Phoneme型やSil型などのパブリック化についての議論を切り出す案、賛成です！
ただどうするかをいろんな話題が入り乱れるこのPRの１コメントの中で議論するのは、後から結論を探しづらくてもったいない気がしています。
issueに切り出すか、あるいは先に提案PR作ってそちらで議論するとかはどうでしょう？
（ソングAPIの仕様の議論、もっとissueに分けていっても良さそうだと今気づきました）
2 participants
内容
以下のPRを参考にする。
frame_query_to_decoder_feature()切り出し voicevox_engine#1041@Hiroshiba, @y-chan と以下の1名の許諾のもと、 #874 にのっとりMITライセンスとしてライセンスする。
関連 Issue
その他