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

追加:queryに無音時間調整関連のパラメータを作成 #1308

Merged
merged 35 commits into from
Jun 7, 2024
Merged

追加:queryに無音時間調整関連のパラメータを作成 #1308

merged 35 commits into from
Jun 7, 2024

Conversation

X-20A
Copy link
Contributor

@X-20A X-20A commented May 23, 2024

内容

良く分からないことになってので整理しました
isPauseLengthUseScale・・・倍率で指定するか, デフォルト値:true
pauseLength・・・絶対値で指定する場合の秒数, デフォルト値:0.3
isPauseLengthFixed・・・絶対値で指定する場合に話速を無視するか, デフォルト値:false
pauseLengthScale・・・倍率で指定する場合の乗数, デフォルト値:1

(queryにテキスト内の無音時間を調整するための新しいパラメータ)

関連 Issue

ref #1289

@X-20A X-20A requested a review from a team as a code owner May 23, 2024 07:01
@X-20A X-20A requested review from Hiroshiba and removed request for a team May 23, 2024 07:01
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.

@X-20A プルリクエストありがとうございます!

仕様に関してより良い形がありそうなので相談させてください!

isPauseLengthUseScale・・・倍率で指定するか, デフォルト値:true
pauseLength・・・絶対値で指定する場合の秒数, デフォルト値:0.3
isPauseLengthFixed・・・絶対値で指定する場合に話速を無視するか, デフォルト値:false
pauseLengthScale・・・倍率で指定する場合の乗数, デフォルト値:1

とされていますが、isPauseLengthUseScaleisPauseLengthFixedどちらも指定されていた場合どうなるのかなどが曖昧だと感じました!
あとこれを実現するためのパラメータはもっと減らせるように思いました。

このような形はどうでしょうか?

pauseLength・・・絶対値で指定する場合の秒数, デフォルト値:null(置き換えない)
pauseLengthScale・・・倍率で指定する場合の乗数, デフォルト値:1

以下、なくしたパラメーターの解説です!

isPauseLengthUseScale

これはpauseLengthScale=1かそうじゃないかでfalse/trueを完全に表現できる。
同じことを表現する方法が2パターンあってしまうので、無い方がいい。(ややこしくなってしまう)

isPauseLengthFixed

迷いどころだがpauseLength=nullの時に「fixしない」とすればこのフラグが不要になる。
またisPauseLengthFixed=falsepauseLengthに値が指定されてる場合の挙動がややこしいので、無い方がいい。
pauseLengthを与えたのにポーズの長さが変わらない、となってしまうため)

@X-20A
Copy link
Contributor Author

X-20A commented May 24, 2024

実装イメージに食い違いがあるように感じるので確認したいのですが
私は
・倍率で指定する
・絶対値で指定する(話速の影響あり)
・絶対値で指定する(話速の影響なし)
の三種類の実装を想定していました

・倍率で指定する
・絶対値で指定する(話速の影響なし)

この二種類に絞るということですか?

@Hiroshiba
Copy link
Member

Hiroshiba commented May 24, 2024

@X-20A 指摘感謝です!!

おっしゃる通りのことを考えていました!
絶対値で指定する(話速の影響あり)はいらないと考えてました。 でもどっちかというと要らないのは絶対値で指定する(話速の影響なし)`な気がしました!!

ということで実装したいのはこの2つかなと・・・!
・倍率で指定する
・絶対値で指定する(話速の影響あり)

(ちなみにAPI の種類は、あらゆる可能性を想定するよりも、需要を想定してシンプルに実装する方が良いと思ってます!)

@X-20A
Copy link
Contributor Author

X-20A commented May 25, 2024

パラメータをpauseLengthとpauseLengthScaleに絞りました
float型にNoneを代入しようとするとエラーになるのでpauseLengthのデフォルト値は-1としています

@Hiroshiba
Copy link
Member

Hiroshiba commented May 25, 2024

@X-20A ありがとうございます!!

ちなみにこんな感じにすればNoneを代入可能になると思います!

consonant: str | None = Field(title="子音の音素")
consonant_length: float | None = Field(title="子音の音長")

ちなみにおすすめはNoneです!
特定の値に特別な意味をもたせるのはバグが起きやすいので、値ではなく型を意味と関連付けてあげる感じです。

@Hiroshiba
Copy link
Member

@X-20A あ、すみません!!
もしかして実装完了のステータスでしょうか 🙇  もしそうであれば気づくのが遅れてすみません。。

@X-20A
Copy link
Contributor Author

X-20A commented Jun 4, 2024

そうですね
不具合や要望があったらまたお願いします

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です!!!

ちょっと細かい点でコメントをいくつかさせていただきました!

build_util/check_release_build.py Outdated Show resolved Hide resolved
test/e2e/single_api/preset/test_presets.py Outdated Show resolved Hide resolved
test/tts_pipeline/test_tts_engine.py Outdated Show resolved Hide resolved
voicevox_engine/app/routers/tts_pipeline.py Outdated Show resolved Hide resolved
voicevox_engine/model.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
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!!!

いろいろ調整していただきありがとうございます!!!
ちょっとこちらで最終調整させていただこうと思います!!

test/tts_pipeline/test_tts_engine.py Outdated Show resolved Hide resolved
test/tts_pipeline/test_wave_synthesizer.py Outdated Show resolved Hide resolved
test/tts_pipeline/test_wave_synthesizer.py Outdated Show resolved Hide resolved
voicevox_engine/tts_pipeline/tts_engine.py Outdated Show resolved Hide resolved
Comment on lines 27 to 28
pauseLength: float | None = Field(title="テキスト内の無音時間(絶対値)")
pauseLengthScale: float = Field(title="テキスト内の無音時間(倍率)")
Copy link
Member

Choose a reason for hiding this comment

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

ちょっとこのあたりの案内の変更も今のうちにさせていただこうと思います!!
(また変えさせていただくかもですが)

@Hiroshiba
Copy link
Member

Hiroshiba commented Jun 7, 2024

マージします!! エディタの方もぜひ完成できると嬉しいです!!


@tarepan 音声合成クエリのパラメータが増えたので共有です 🙏

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