-
Notifications
You must be signed in to change notification settings - Fork 206
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
generate_frame_scale_features
の解体
#790
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PRありがとうございます!
よりテストしやすくなって良い感じになってきたのではないかと思います...!
いくつか、今後のメンテナンスや、他のコントリビュータの事も考えた提案を書いてみました!
@y-chan 全指摘を反映、テストパスを確認しました。re-reviewよろしくお願いします! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!
テストの方、見落としがあったので修正いただけると助かります...!
@y-chan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTMです!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
音声合成処理の根幹に関わる部分なので僕もレビューさせていただきました!
1箇所コメントのsuggestをさせていただきました。多分あってると思うので勝手ながら適用してからマージさせていただこうと思います!
内容
#784 で切り出された第2前処理関数
generate_frame_scale_features
の更なる切り出し。3機能が混在していたため、以下の3つに分解:
calc_frame_per_phoneme
calc_frame_pitch
calc_frame_phoneme
関連 Issue
resolve #783
その他
diff が上手く効かなくてreviewしづらそうです🥺
commit log は段階的になっておりdiffが上手く効いています。
なので最初に commit log を見て変更概要をご理解いただければと思います。