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

generate_frame_scale_features の解体 #790

Merged
merged 18 commits into from
Dec 4, 2023

Conversation

tarepan
Copy link
Contributor

@tarepan tarepan commented Nov 28, 2023

内容

#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 を見て変更概要をご理解いただければと思います。

@tarepan tarepan requested a review from a team as a code owner November 28, 2023 05:45
@tarepan tarepan requested review from y-chan and removed request for a team November 28, 2023 05:45
Copy link

github-actions bot commented Nov 28, 2023

Coverage Result

Resultを開く
Name Stmts Miss Cover
run.py 480 326 coverage-32%
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 23 0 coverage-100%
voicevox_engine/cancellable_engine.py 91 71 coverage-22%
voicevox_engine/dev/core/init.py 2 0 coverage-100%
voicevox_engine/dev/core/mock.py 27 12 coverage-56%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 36 2 coverage-94%
voicevox_engine/engine_manifest/EngineManifest.py 34 0 coverage-100%
voicevox_engine/engine_manifest/EngineManifestLoader.py 12 0 coverage-100%
voicevox_engine/engine_manifest/init.py 3 0 coverage-100%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/library_manager.py 93 5 coverage-95%
voicevox_engine/metas/Metas.py 33 0 coverage-100%
voicevox_engine/metas/MetasStore.py 29 12 coverage-59%
voicevox_engine/metas/init.py 2 0 coverage-100%
voicevox_engine/model.py 162 9 coverage-94%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/morphing.py 70 46 coverage-34%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetError.py 2 0 coverage-100%
voicevox_engine/preset/PresetManager.py 81 2 coverage-98%
voicevox_engine/preset/init.py 4 0 coverage-100%
voicevox_engine/setting/Setting.py 11 0 coverage-100%
voicevox_engine/setting/SettingLoader.py 19 0 coverage-100%
voicevox_engine/setting/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/core_wrapper.py 201 146 coverage-27%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 59 30 coverage-49%
voicevox_engine/synthesis_engine/synthesis_engine.py 133 11 coverage-92%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 67 9 coverage-87%
voicevox_engine/user_dict.py 144 12 coverage-92%
voicevox_engine/utility/init.py 5 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 37 0 coverage-100%
voicevox_engine/utility/core_version_utility.py 8 1 coverage-88%
voicevox_engine/utility/mutex_utility.py 10 0 coverage-100%
voicevox_engine/utility/path_utility.py 26 8 coverage-69%
TOTAL 2184 706 coverage-68%

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

PRありがとうございます!
よりテストしやすくなって良い感じになってきたのではないかと思います...!

いくつか、今後のメンテナンスや、他のコントリビュータの事も考えた提案を書いてみました!

voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
@tarepan tarepan mentioned this pull request Nov 29, 2023
3 tasks
@tarepan tarepan requested a review from y-chan November 29, 2023 18:26
@tarepan tarepan mentioned this pull request Dec 1, 2023
3 tasks
@tarepan
Copy link
Contributor Author

tarepan commented Dec 3, 2023

@y-chan 全指摘を反映、テストパスを確認しました。re-reviewよろしくお願いします!

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

ほぼLGTMです!
テストの方、見落としがあったので修正いただけると助かります...!

voicevox_engine/synthesis_engine/synthesis_engine.py Outdated Show resolved Hide resolved
test/test_synthesis_engine.py Outdated Show resolved Hide resolved
test/test_synthesis_engine.py Outdated Show resolved Hide resolved
@tarepan
Copy link
Contributor Author

tarepan commented Dec 3, 2023

@y-chan
全指摘箇所反映・テストパスを確認しました。Re-Re-reviewよろしくお願いします!

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

LGTMです!

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

音声合成処理の根幹に関わる部分なので僕もレビューさせていただきました!
1箇所コメントのsuggestをさせていただきました。多分あってると思うので勝手ながら適用してからマージさせていただこうと思います!

@Hiroshiba Hiroshiba merged commit 2160538 into VOICEVOX:master Dec 4, 2023
3 checks passed
@tarepan tarepan deleted the refactor/preprocess3 branch December 5, 2023 02:44
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.

Refactor: _synthesis_impl 内前処理の整理
3 participants