Skip to content
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

refactor: 入出力周りのいくつかの処理をsynthesizerから移動 #917

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Jan 4, 2025

内容

#916 をやるためのリファクタ。

関連 Issue

その他

@qryxip
Copy link
Member Author

qryxip commented Jan 4, 2025

このPRで私がした命名:

  • core::adjust: 入出力を"補正"する処理達
    • ensure_minimum_phoneme_length: predict_duration.onnxの後処理
    • pad_decoder_feature: decode.onnx/generate_full_intermediate.onnxの前処理
  • engine::interpret_query: AudioQueryを作った後に、それを"解釈"する処理達 (モーラ達をflattenしたり、(f0, phoneme)の形にしたり)
    • DecoderFeature: (f0, phoneme)
    • initial_process, split_mora: 以前からある関数 (多分qwertyさんかSuitCaseさんが書いたもの。あるいはC++時代?)

@Hiroshiba
Copy link
Member

adjust

データの向きがわからないかもですね!
APIの入力→onnxの入力なのか、onnxの出力→APIの出力なのか。
(最初onnxの出力→APIの出力の方かなと思いました。)

といってもあまり拘らなくても良い気がするので、とりあえず何をどうするものが置かれてるかわかるようにファイルの一番上にこのモジュールの役割を1行メモしておけば良いかなと。
(書かれてないと、コミッターが間違えて違うadjustなものを配置しちゃう)

interpret_query

こちらは特にコメントないです!

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ほぼLGTMです!!

マジックナンバーの扱いだけどうしようかなというところ。
大変そうであればまあこのままでもいいかも、くらいの気持ちです。

crates/voicevox_core/src/core/adjust.rs Outdated Show resolved Hide resolved
@qryxip
Copy link
Member Author

qryxip commented Jan 4, 2025

6bd87ed (#917): adjustadjust::{pre,post}に分割してみました。
384a77e (#917): モジュール単位でdocstingを書いてみました。

@qryxip qryxip requested a review from Hiroshiba January 4, 2025 06:05
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@qryxip qryxip merged commit 6ff13e0 into VOICEVOX:main Jan 4, 2025
29 checks passed
@qryxip qryxip deleted the refactor-move-some-input-output-related-processes-from-synthesizer branch January 4, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants