-
Notifications
You must be signed in to change notification settings - Fork 309
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
Add: ソングの書き出しダイアログを追加 #2287
Add: ソングの書き出しダイアログを追加 #2287
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.
一旦大まかな部分だけ見させていただきました!良さそう!!
のちほど細かい部分のコードも見ます!
Co-authored-by: Hiroshiba <[email protected]>
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.
あの後mp3とoggエクスポートを作りました。
src/components/Sing/SingEditor.vue
Outdated
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.
マルチエンジンオフのボタンと合わせました。
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.
ファイルフォーマットの追加で機能変更がすごい増えた感じありますね!!!
ちょっと手動で確かめて置いた方がいいリストを作ってみたので、試したものからチェックしていきましょう・・・!!!!
- トークで、書き出し先を固定せず1つだけ.wav書き出し
- トークで、書き出し先を固定せず連結して.wav書き出し(これはオプションでも)
- トークで、書き出し先を固定せず全部をそれぞれ.wav書き出し(これはオプションでも)
- トークで、書き出し先を固定して1つだけ.wav書き出し
- トークで、書き出し先を固定して連結して.wav書き出し
- トークで、書き出し先を固定して全部をそれぞれ.wav書き出し
- トークで、config.jsonのバージョンを以前のものにしてファイルフォーマットを指定し、このプルリクエストで起動してちゃんと
.wav
が正しく消えてるか確認 - トークで、テキストを保存してみる
- ソングで、ミックスして.wav書き出し
- ソングで、ミックスして.ogg書き出し(これはオプションでも)
- ソングで、ミックスして.mp3書き出し(これはオプションでも)
- 書き出せるけど開始タイミングがずれる(こちらのコメント)
- ソングで、トラックごとに.wav書き出し
- ソングで、トラックごとに.ogg書き出し(これはオプションでも)
- ソングで、トラックごとに.mp3書き出し(これはオプションでも)
- ソングのoggの品質チェック
- ソングのmp3の品質チェック
- 波形を上下反転させてミックスして聞けば差がわかるぐらいの軽微な差!
- ビルド後の容量チェック
- 実際にビルドして測定してみました、310KBくらいだったので問題なし!
動作チェックじゃなくてテストを書く でももちろん OK だと思います。
特にソングで音声書き出しの動作チェックがしたいのは、エクスポート関数がちゃんと動くかどうかのチェックをしたいだけなので、単体テストの方がむしろ良さそう。
ただファイル保存まで含めたe2eテストはfailしたかチェックできない(してないだけかも)ので難しいかも。
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.
mp3のタイミングのずれが発覚したので一旦レビューを止めました 🙇
まだコードの細かいところは見れてないです。
mp3の出力のタイミングがずれてしまうのが致命的かどうかわからないですね・・・!!
不具合があるままリリースしていいのかダメなのか・・・。
原因が特定できそうだったらそれが一番いいのですが、できなさそうだったら・・・ちょっとDTMやってる方の判断も聞いてみますか。
バグっててもいいからリリースした方がいいのかもしれないし、タイミングがずれていることに気づかずにVOICEVOXソングの評価をされかねないから延期した方がいいという説もあるかもしれない。
0.012秒がどれくらい致命的なのかわからない・・・。
・・・・・もしmp3リリースは延期した方がいいとなると、書き出しダイアログを作るところだけにプルリクエストを切り分けていただくことになるかもです。。。 🙇 🙇 🙇
(こういうことが起こることも考慮して、プルリクエスト1つで1機能が良いとされてるんだな~~~と感じました。ファイルフォーマットも追加するつもりだとお聞きしたタイミングで引き返すことを提案すべきだったかもです、申し訳ない。。)
遅延はLAMEの仕様感があります:https://lame.sourceforge.io/tech-FAQ.txt 音ズレが嫌ならmp3使うな、をFAQで案内するのが楽かつ現実的そう? |
なーーーーーーるほどです!!! たしかにmp3はタイミングがずれるのが仕様そうな雰囲気。 これ歌においてはかなり致命的になるので他のソフトどうしてるか見てみたんですが、VoiSonaもSynthVもたぶん せっかくだし関数自体は実装しても良いかも。トーク側で活用の場がありそうな予感があるので・・・! |
</template> | ||
|
||
<script setup lang="ts"> | ||
// NOTE: 前回の設定を引き継ぐため、他のダイアログでやっているようなinitializeValuesはやらない |
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.
(ただのコメントです)
ここの設定って保存できるようにしてあげるとより使いやすいかもですね!
変更量多いからこのプルリクエストではちょっとやらない方が良さそうだけど。
気が向いたらissue作っても良いかもしれない。
if (result.result !== "SUCCESS") { | ||
return result; | ||
} |
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.
あ~ トラック書き出しは途中で書き出しに失敗したファイルがあったら止まっちゃうんですね。
多分エラーのあるトラックがあっても他のトラックは書き出したい、みたいなことある気がするので、全部書き出してあげると良いかも。
issue作っておくのどうでしょう。できれば解決策まで案内できていると手がつけやすそう。
まあもし仕様としてイマイチだと感じれば・・・!
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.
多分エラーの要因はファイルシステム周りだと思うので、となると継続しても特に何も無い気がします。
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.
確かにファイルシステム周りのエラーが多そうですが、トラックごとにエラーで落ちることは全然あると思います!
例えばあるトラックだけノートのレンダリングに失敗する、とかがありえると思います。
ありそうなユーザーの操作の流れとしては、一旦あるトラックでエラーが発生している状態だけど、他のトラックの変更してそこだけ書き出したいみたいな。
例えば一時的にトラック A でノートの配置がバグっているけれども(コーラス用の2つのノートが重なって置かれてるとか)、トラック A 以外の音声を書き出したいみたいな。
今現状だとエラーが出ないかもですが、将来的にそういうこともありえるだろうなーと。
再掲
issue作っておくのどうでしょう。できれば解決策まで案内できていると手がつけやすそう。
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です!!!!!!!
素敵な機能をありがとうございます!!!!!!
ちょくちょくコードをまとめられてますが、結構いい感じにまとめられてるなと感じました!成長!!
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Sig <[email protected]>
title="音声のサンプリングレート" | ||
description="音声のサンプリングレートを変更できます。" |
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.
量子化ビット数(ビット解像度)も設定できると良いかも。まあ必須ではないと思うので後で実装でも良さそう。
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でやります。
src/type/preload.ts
Outdated
// NOTE: ファイル名パターンは拡張子を含まない | ||
fileNamePattern: z.string().default(""), |
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.
ここちょっと他のに合わせて右側にコメントつけるようにさせていただきます!
// NOTE: ファイル名パターンは拡張子を含まない | |
fileNamePattern: z.string().default(""), | |
fileNamePattern: z.string().default(""), // NOTE: ファイル名パターンは拡張子を含まない |
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!!!
ちょっとこちら側の都合で調整だけさせていただきます!
見た目もかっこいいし実装もいい感じだし、良い機能になったと思います!
変更いただきありがとうございました!!!
テストが通ったのでマージさせていただきます! こんな感じでツイートさせていただく予定です!
|
chromatic見て、Story側でエラーが出てそうなことに気づきました 🙇 🙇 🙇 .wavのとこが |
内容
書き出しダイアログを追加し、色々設定出来るようにします。
関連 Issue
(なし)
スクリーンショット・動画など
その他
(なし)