ADD: AudioItemにSpeakerIdを追加 #1092

sabonerune · 2023-01-05T09:18:52Z

内容

AudioItemにSpeakerID(SpeakerUUID)を追加します。

その他

複数エンジンが実装されるとマイグレーションが複雑になる可能性があるためとりあえず追加だけしました。

src/components/DictionaryManageDialog.vue

src/store/project.ts

Hiroshiba

PRありがとうございます！！
とりあえず実装しておくの、良いのかなと思いました！

レビュー前に、ちょっとモチベと相談なのですが、いくつか気になったことが･･･！

プロジェクトファイルのSpeakerID追加マイグレーションに関して、MYCOEIROINKが大変そう
- エンジンから変換リストを作ろうとし、無理だったらフォールバックでビルトイン変換リストを使う、とか･･･？
COMMAND_CHANGE_STYLE_IDなど、Voiceを渡してるけど名前がSTYLE_IDになってる
- 別にそのままでもまあ問題ないと言えば問題ない
- もしよければ、くらいです
COMMAND_PUT_TEXTSなど、engine/speaker/style IDをそれぞれ渡してるのもありそう
- これも全く問題ないのですが、もしよければ統一しとくと便利かも、くらいです
AudioItemにVoiceを持たせる･･･？
- 将来仮にVoiceVersionをもたせるときに楽そう
- なんか変なとこに影響範囲ありそうなので、まあこれももしよければくらい

Hiroshiba · 2023-01-14T12:55:50Z

あと設計レベルで @aoirint さんや @Segu-g さんにお伺いしたいことが！
engine/speaker/style IDをVoice構造体とすることになにか問題とか思いついたりとかありますか･････？

sabonerune · 2023-01-14T14:02:43Z

プロジェクトファイルのSpeakerID追加マイグレーションに関して、MYCOEIROINKが大変そう

大変そうなので複数エンジン実装までに入れられないかなと急いでPR作った感じですね。
間に合いそうでなければ一旦PR取り下げた方がいいかもしれません。
エンジンからSpeakerIDを取れるように待機してからマイグレーションするようにするとか？

Hiroshiba · 2023-01-15T06:26:20Z

複数エンジンリリースは月末で、その１週間くらい前まではバグフィックスとか含めると思います。そこまでにできると嬉しいなって感じですね…！

待機、良さそうに感じました。
実際のコード追っかけてみたのですが、たぶんbackground.tsにあるdid-finish-load内でエンジン準備完了前にプロジェクトファイルが読まれてるんだと思います。
ここの処理を全部エンジン起動後のとこ、例えばVIEX_INIT（VUEX_READYだったかも）に移せば、待機する感じの処理になりそうな予感がします。

スマホからなので色々情報不足ですが、参考になれば！！

Segu-g · 2023-01-15T14:48:49Z

engine/speaker/style IDをVoice構造体とすることになにか問題とか思いついたりとかありますか･････？

基本的に情報を足りなくて困る事はあっても、多くて困ることは可読性が悪くなるくらいだと思うので大丈夫でしょう。

ただ、ロジックの変更まで含めると時間も掛かりそうなので取り敢えずロジックはそのままに追加でもいいと思います。

Hiroshiba · 2023-01-15T22:04:08Z

@Segu-g さんコメントありがとうございます！！

ロジックはそのままに追加

こちらのコメントで課題点がはっきりしました！

このままマージするとMYCOEIROINKで過去のプロジェクトファイルが読めなくなる
- ビルトイン以外のキャラがいる場合にマイグレーション処理が書けないため
解決手段は２つ
- １つは諦める
  - 開発者に連絡すればOK
- もう１つは追加実装する
  - まずvvprojを読むタイミングをVUEX_INIT内にする
  - あとはエンジンに話者情報を問い合わせて突合する
  - ロジックもきれいになるので嬉しい。

まあ、個人的には一旦諦めてとりあえず実装でも良いかなと思います！

その場合でも、Voiceを引数にしたり、engine/speaker/styleのペアだったり、関数名がSTYLE_IDだけど渡すのは前者どちらかだったりと、結構複雑度が増してしまっている印象があります。
たぶん、プロジェクトファイル部分の変更以外を全部戻してからAudioItemにspeaker_idを足し、エラーが起きる２～３箇所だけ修正するのが最小PRになって一番早いかなと思いました！

正直どうすべきかとても迷っています 😇
想定より面倒だとわかって辛かったり、限られた時間だとプレッシャーだったりしたら仰って頂ければ 🙇‍♂️

sabonerune · 2023-01-15T23:04:32Z

COEIROINKのエディタのマイグレーションのことを完全に失念していました。
(そういった変換リストで済まないものがVOICEVOXに導入される前にやってしまおうという発想で急いで作りました)

そうなると急いで複数エンジンに間に合わせる必要性は薄くなるのでエンジンに話者情報を問い合わせてマイグレーションするようにした方がいいかもしれません。

あとプロジェクトファイル以外の変更点ですがStyleIDを操作するときに同時にSpeakerIdも操作するように処理を付け加えただけですね。
これをしないとSpeakerIDとStyleIdの組み合わせが乖離してしまうので必要なものだと思います。

sabonerune · 2023-01-16T09:31:05Z

src/store/audio.ts

@@ -417,6 +418,7 @@ export const audioStore = createPartialStore<AudioStoreTypes>({
      payload: {
        text?: string;
        engineId?: string;
+        speakerId?: string;


GENERATE_AUDIO_ITEMは引数がOptionalなのでそのままにしている。

これご興味あればという感じなのですが、audio_itemもengine/speaker/styleではなくvoiceにするのどうでしょう…？

こうするとたぶんAudioCell辺りも↑の三変数管理する必要なくなったりと、結構コードがすらっとしそうだな〜と…！
マイグレーション周りもちょいいじらないとダメそうですが。。

でももう十分良い感じなので、もしよければ…！！

Hiroshiba · 2023-01-16T09:49:46Z

そうなると急いで複数エンジンに間に合わせる必要性は薄くなるので

あ、確かに！！
のんびりやっちゃって良いかもですね。

StyleIDを操作するときに同時にSpeakerIdも操作するように処理を付け加えた

あーーーなるほどです！
普通にstyle指定するとこほぼ全部が影響範囲ですね、盲点でした！！

sabonerune · 2023-01-16T12:32:53Z

* まずvvprojを読むタイミングをVUEX_INIT内にする

VUEX_INIT完了後に発火されるON_VUEX_READY時点ではUSER_ORDERED_CHARACTER_INFOSは取れませんでした。VUEX_INITは設定ファイルの読み込みまででON_VUEX_READYがウインドウを表示してエンジンの起動が始まるという感じかもしれません。

voicevox/src/views/EditorHome.vue

Lines 519 to 523 in 7292b6b

    
                 await store.dispatch("LOAD_USER_CHARACTER_ORDER"); 
        
                 await store.dispatch("LOAD_DEFAULT_STYLE_IDS"); 
        
                 // 辞書を同期 
        
                 await store.dispatch("SYNC_ALL_USER_DICT");

この辺りでbackground.tsにエンジンの起動が完了したことを通知する必要があるかもしれません。

Hiroshiba · 2023-01-16T13:10:12Z

あーーーーーーー　そのとおりでした、VUEX作りが完了したタイミングだからダメですね。。 🙇‍♂️

この辺りでbackground.tsにエンジンの起動が完了したことを通知する必要

仰るとおりだと思います。VUEX_INITなどと同じく、ON_ENGINE_READYとかENGINE_INITとかですよね。

あるいはデータの流れをよりわかりやすくするなら、セーフモード引数と同様にvvprojのパスをbackground.tsからvuex側渡し、エンジン起動後にEditorHomeでそのパスがあればloadする、というのもありだと思います。
（個人的にはこっちのが近代的かもと思いました！）

Hiroshiba · 2023-01-16T13:14:49Z

どちらにせよちょっとややこしく、このPR内でやるのは範囲外かもですね。。。
（あとそもそもプロジェクトファイルはエンジン起動後に読み込まれることを想定している箇所があるのを思い出しました。）

プロジェクトファイルのマイグレーションの流れを正確にする意図でまず必ず起動後にvvprojが読まれるようにしたあと、今のPRを進めるのがまあきれいかもです。
最初に思ってたよりまた範囲が広がってしまったので、もしモチベ的にイマイチでしたら言っていただければ･･･ 🙇

（修正範囲がそこまで大きくなさそうであれば、このPRで一緒に修正でもまあ大丈夫です！）

sabonerune · 2023-01-16T13:25:06Z

もう少し進めてみようとおもいます。

StyleIdの取得をUSER_ORDERED_CHARACTER_INFOS経由に変更

sabonerune · 2023-01-24T15:22:00Z

試しにエンジン起動後にプロジェクトを読み込むように変更してみました。

Hiroshiba

おー！！面白いですね！！！
SET_PROJECT_FILEPATHがきれいに機能している印象です。

プロジェクトファイルを読み込む部分だけプルリクにする、というのはどうでしょう。
このプルリクのタイトルと内容が若干乖離していることと、なによりこれだけで有用なので提案してみた次第です。

sabonerune · 2023-01-25T10:22:57Z

プロジェクトファイルの読み込み部分のみのプルリクエストを作成します。

sabonerune

一通り完了。
#1147 がマージされることが前提。

src/store/project.ts

Hiroshiba · 2023-02-03T19:23:56Z

src/components/DictionaryManageDialog.vue

+const speakerIdComputed = computed(() => {
+  if (!store.getters.USER_ORDERED_CHARACTER_INFOS)
+    throw new Error("assert USER_ORDERED_CHARACTER_INFOS");
+  return store.getters.USER_ORDERED_CHARACTER_INFOS[0].metas.speakerUuid;
+});


ここvoiceComputedにできそうだなと思いました！
まあ修正範囲広くなっちゃうのでこのままでも良さそう。FIXME書いても良いかも？

voiceComputedにしました。

src/store/audio.ts

Hiroshiba · 2023-02-03T19:46:30Z

src/store/audio.ts

-      const engineId = payload.engineId ?? state.engineIds[0];
-
-      // FIXME: engineIdも含めて探査する
-      const styleId =
-        payload.styleId ??
-        state.defaultStyleIds[
-          state.defaultStyleIds.findIndex(
-            (x) =>
-              x.speakerUuid === userOrderedCharacterInfos[0].metas.speakerUuid // FIXME: defaultStyleIds内にspeakerUuidがない場合がある
-          )
-        ].defaultStyleId;
+      const defaultStyleId = state.defaultStyleIds[0];
+      const voice = payload.voice ?? {
+        engineId: defaultStyleId.engineId,
+        speakerId: defaultStyleId.speakerUuid,
+        styleId: defaultStyleId.defaultStyleId,
+      };


レビューメモです。
ここ問題ないか処理を追いかけてみてたのですが、
・voiceがない場合の処理以外は大丈夫
・voiceがない場合は起動後のaudioCell作成だけ
・デフォルトスタイルIDの0番はエンジン起動後に必ず作られる
・最初のaudioCell作成はエンジン起動・デフォルトスタイル作成後
なので大丈夫そうでした！

src/store/audio.ts

src/components/AudioCell.vue

src/store/project.ts

Hiroshiba

LGTM！！！！

差分を極力少なくしてくださっているのを感じました、とても見やすかったです！！！

audioItemの～～id系はundefinableじゃなくせたんですね！！気づきませんでした。
そこプラスVoice統合でかなりコードが扱いやすくなったように感じました、改良ありがとうございます！！

Co-authored-by: Hiroshiba <[email protected]>

Hiroshiba

@Segu-g さんすみません！
もしお時間あったら「AudioItemにSpeakerIdを追加する」点に関して問題点がありそうかご意見だけもらっても良いですか･･･？ 👀
（コードレビューは無しで大丈夫です！忙しかったら難しいとおっしゃって頂ければ！！）

sabonerune · 2023-02-06T08:32:17Z

指摘点を直しました

Segu-g · 2023-02-06T12:20:54Z

@Segu-g さんすみません！もしお時間あったら「AudioItemにSpeakerIdを追加する」点に関して問題点がありそうかご意見だけもらっても良いですか･･･？ 👀 （コードレビューは無しで大丈夫です！忙しかったら難しいとおっしゃって頂ければ！！）

卒論書いてる最中なので流し読みした程度ですが、話者に関する情報が纏められてかなり読みやすくなったと思います！取りあえずSpeakerIdを追加しておくのはCharactersから情報を取ってき易くなるのでやり得だと思いました！

ただ自分は詳しくないので何とも言えないのですが、キャラを追加するタイプのエンジン（マイコエとか？）やバージョン違いが実装された場合、(engineId, styleId) ではなく (engineId, version, speakerId, styleUUID) のペアで比較する必要など出てくると思うのでその辺りの仕様定義や変更もしていく必要があるのかなと思います。

Hiroshiba · 2023-02-06T12:57:29Z

たしかにバージョン情報を足したくなる気がします。
そこはキーがないVoice objectに足す形になるので、マイグレーション面はかなり簡単になりそうです。

あ、engineは既に足されてる感じです！
ざっと考えた感じ将来的な変更にも強そうに感じました。お忙しいところレビューありがとうございます！！！

Hiroshiba · 2023-02-06T12:58:34Z

問題ないと思うのでマージしたいと思います！！

Segu-g · 2023-02-06T13:10:33Z

あ、engineは既に足されてる感じです！

変更しなければいけない可能性があるのは styleId を speakerId + styleUUID にする変更なのかなと思っています。
styleId が number なのでキャラのインストールがライブラリ形式になると衝突しないように静的な値に固定されず動的に割り振られることもあるのかなと（この辺りのドキュメント読み切れてないので間違ってたらすみません）
その場合今の仕様がどうだったか覚えていませんが、styleId は衝突しないことしか保証されないのでvoiceの比較に speakerId (UUID) と styleUUID? (UUID) を使う必要が出ます

ADD: AudioItemにspeakerIdを追加

981d07d

sabonerune commented Jan 5, 2023

View reviewed changes

src/components/DictionaryManageDialog.vue Outdated Show resolved Hide resolved

src/store/project.ts Outdated Show resolved Hide resolved

sabonerune marked this pull request as ready for review January 5, 2023 09:30

sabonerune requested a review from a team as a code owner January 5, 2023 09:30

sabonerune requested review from Hiroshiba and removed request for a team January 5, 2023 09:30

Hiroshiba reviewed Jan 14, 2023

View reviewed changes

FIX: Voiceを渡す関数名を変更、COMMAND_PUT_TEXTSにVoiceを渡すように

7292b6b

sabonerune commented Jan 16, 2023

View reviewed changes

sabonerune added 2 commits January 24, 2023 19:10

FIX: プロジェクトファイルの読み込みタイミングを変更

a1dfffb

StyleIdの取得をUSER_ORDERED_CHARACTER_INFOS経由に変更

Merge branch 'main' into working/add-speaker-id

422cd8d

sabonerune marked this pull request as draft January 24, 2023 15:23

Hiroshiba reviewed Jan 25, 2023

View reviewed changes

ENH: プロジェクトファイルをエンジンが起動した後に読み込まれるようにする

a9a86cd

sabonerune mentioned this pull request Jan 25, 2023

ENH: プロジェクトファイルをエンジンが起動した後に読み込まれるようにする #1147

Merged

sabonerune added 2 commits January 26, 2023 18:26

FIX: パラメーターの複製を除去

7b8e4c6

FIX: ファイルパスをpropで渡すようにした

122ba97

FIX: GENERATE_AUDIO_ITEMの引数をVoiceへ

2d64a9d

sabonerune commented Jan 30, 2023

View reviewed changes

src/store/project.ts Outdated Show resolved Hide resolved

src/store/project.ts Show resolved Hide resolved

sabonerune added 3 commits February 1, 2023 17:46

FIX: バージョンを変更

7989f06

Merge branch 'main' into add-speaker-id

b11fa26

FIX: マージのミス修正

d59cf5c

sabonerune marked this pull request as ready for review February 3, 2023 10:46