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

サンプリングレートに「エンジンのデフォルト」を追加 #973

Conversation

Hiroshiba
Copy link
Member

@Hiroshiba Hiroshiba commented Oct 6, 2022

内容

の実装です。

エンジンのデフォルトサンプリングレートを指定可能にする方法は3つほど思いつきました。

  1. エンジン側のAPIを変更して「デフォルトサンプリングレートで合成」を指定可能にする
  2. エディタ側でoutputSamplingRate=="default"だったとき、エンジンのデフォルトサンプリングレートに置き換える
  3. エディタ側でoutputSamplingRate==0だったとき、(同上)

エンジン側のAPIを変えにエディタ側のコード変更だけで対応したかったので、2番の方法で実装しました。
ただそうしたことで、保存時とエンジンAPI実行時で2回型変換を行わないといけなくなり、実装が煩雑になりました・・・。

2022/11/26 22:47
↑の方法だとややこしかったので保存時は変換しない実装にしました。

関連 Issue

close #925

スクリーンショット・動画など

image

その他

src/electron/preload.ts Outdated Show resolved Hide resolved
Copy link
Member Author

@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.

型周りのあれこれが含まれてるので、詳しそうな @Segu-g さんか @MT224244 さんにレビュー頂けると嬉しいです 🙇
設計周りで @yamachu さんも意見頂けると心強いかもと思っています。

あまり良い実装だと思えてないので、素敵な提案があれば抜本的にプルリクを作り直すのもありだと思っています。

src/type/ipc.ts Outdated Show resolved Hide resolved
@Segu-g
Copy link
Member

Segu-g commented Oct 9, 2022

LGTM!

@Hiroshiba
Copy link
Member Author

メインプロセス←→レンダラープロセスで型の変換が不要なようにしてみました。変更行数がガクッと減りました。
union型が難しそうだったのでoneOfを使ってみました。
https://ajv.js.org/json-schema.html#oneof

@Hiroshiba
Copy link
Member Author

大丈夫だと思うのでマージします!
レビューありがとうございました!!

@Hiroshiba
Copy link
Member Author

あ、いくつか変更したんでした・・・。
@Segu-g さんすみません、さらっとで大丈夫なので変更箇所に目を通していただけると・・・ 🙇

@Hiroshiba
Copy link
Member Author

@MT224244 さん・ @y-chan さん、もしよかったらお時間あるときにPR見るのお願いしても良いでしょうか 🙇
24000を暗黙的にdefaultサンプリングレートとしていたのですが、そうではなく「エンジンのデフォルトサンプリングレートを使う」という設定を設けた形です。

Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

まだ動かせてませんが、自分は大丈夫だと思います。

src/type/ipc.ts Outdated Show resolved Hide resolved
@Hiroshiba Hiroshiba merged commit 65ad413 into VOICEVOX:main Nov 29, 2022
@Hiroshiba Hiroshiba deleted the サンプリングレートに「エンジンのデフォルト」を追加 branch November 29, 2022 15:46
@Hiroshiba
Copy link
Member Author

マージしました!レビューありがとうございます!!

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.

サンプリングレートに「エンジンのデフォルト」を追加
6 participants