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

CharacterInfoをエンジンから取って来るようにする #450

Merged
merged 12 commits into from
Dec 14, 2021

Conversation

shirowanisan
Copy link
Contributor

@shirowanisan shirowanisan commented Nov 7, 2021

内容

題の通り

動作確認には VOICEVOX ENGINE 0.10.preview.2 以降のpreview版エンジンが必要
https://github.com/VOICEVOX/voicevox_engine/releases/tag/0.10.preview.2

関連 Issue

ref #429
close #433

query: queryParameters,
});

return new runtime.JSONApiResponse(response, (jsonValue) => jsonValue.map(SpeakerInfoFromJSON));
Copy link
Contributor Author

@shirowanisan shirowanisan Nov 7, 2021

Choose a reason for hiding this comment

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

JSONのparseが上手くいかない。

単純に自分がバクに気付いていないだけか、内容が断定できない以下のjsonのparseができないのか検討中。

mainにSpeakerInfoを取得する部分が作られていたので、そこからマージする。

@shirowanisan
Copy link
Contributor Author

shirowanisan commented Nov 7, 2021

エンジンから取れるスピーカー情報の不一致が起こっている。

http://localhost:50021/speakers

[
  {
    "name": "dummy1",
    "styles": [
      {
        "name": "dummy1",
        "id": 0
      }
    ],
    "speaker_uuid": "7ffcb7ce-00ec-4bdc-82cd-45a8889e43ff",
    "version": "0.8.0"
  },
  {
    "name": "dummy2",
    "styles": [
      {
        "name": "dummy2",
        "id": 1
      }
    ],
    "speaker_uuid": "388f246b-8c41-4ac1-8e2d-5d79f3ff56d9",
    "version": "0.8.0"
  }
]

http://localhost:50021/speaker_info?speaker_uuid=7ffcb7ce-00ec-4bdc-82cd-45a8889e43ff

{
  "policy": "string",
  "portrait": "string",
  "icons": {
    "dummy1_0": "string",
    "dummy1_2": "string",
    "dummy1_4": "string",
    "dummy1_6": "string",
  },
  "voice_samples": {
    "dummy1_0_001": "string",
    "dummy1_0_002": "string",
    "dummy1_0_003": "string",
    "dummy1_2_001": "string",
    "dummy1_2_002": "string",
    "dummy1_2_003": "string",
    "dummy1_3_001": "string",
    "dummy1_3_002": "string",
    "dummy1_3_003": "string",
    "dummy1_4_001": "string",
    "dummy1_4_002": "string",
    "dummy1_4_003": "string",
  }
}

不一致部分

speakersでは、styleが1つだが、speaker_infoのiconsを見る限りstyleが4つ

解消案

UI側でのスピーカー情報の構造はこうなっている。

export type CharacterInfo = {
  portraitPath: string;
  metas: {
    speakerUuid: string;
    speakerName: string;
    styles: StyleInfo[];
    policy: string;
  };
};

export type StyleInfo = {
  styleName?: string;
  styleId: number;
  iconPath: string;
  voiceSamplePaths: string[];
};

エンジンでspeakersにstyleを持たせているので、その構造をUIのStyleInfoと合わせる方が良いように感じる。
そこで、speakersエンドポイントでspeaker_info情報を含んだspeaker情報を含めて返す。
(その場合、speaker_infoエンドポイントは使わなくなる可能性が高い。)

メリット

  • エンジン側でのstyle矛盾実装をしてしまうことを防げる。
  • UI側でstyleとvoice_sampleと文字列一致でマッピングしないといけなくなることを防ぐ

懸念点

今のUIのStyleInfoがPathになっているので、内部の扱い方をbase64と変える判断をどうするか。

以下のように拡張して、iconPathvoiceSamplePathsがnullならiconBase64voiceSampleBase64sを使う?
もしくはiconPathvoiceSamplePaths自体を廃止する(パスではアクセスしないことにする。)

export type CharacterInfo = {
  portraitPath: string;
  metas: {
    speakerUuid: string;
    speakerName: string;
    styles: StyleInfo[];
    policy: string;
  };
};

export type StyleInfo = {
  styleName?: string;
  styleId: number;
  iconPath?: string;
  iconBase64?: string;
  voiceSamplePaths?: string[];
  voiceSampleBase64s?: string[];
};

申し訳ありませんが、エンジン側の修正をご検討お願いしますmm

@Hiroshiba
Copy link
Member

文字列でデータを引っ張ってくる必要があるのは確かに手間がかかってしまうと思います!ぜひエンジン側のデータ構造を変更したいです。
speakersでspeaker_infoの内容を全部返すのはちょっと避けたいです。高解像度の立ち絵なども含まれており、キャラクターが増えるとレスポンスが悪くなためです。

ということで、speaker_infoとspeakersを混ぜるとエディタ側のデータ構造に近くなるように、speaker_infoの構造を変えるのはどうでしょう。
具体的には、stylesキーを増やし、その下にiconキーとvoice_samplesキーがある感じです。

@Hiroshiba
Copy link
Member

あ、懸念点の方のpathとbase64で型が違う点は、2つ持ってるとロジックがややこしくなりそうなので、とりあえず片方に寄せるのが良いかなと思います!
個人的にはpathからデータを読んでbase64として持っておく方に寄せると良いのかなと思いました。(パスとして持っても良いけど、tmpファイルの扱いが大変そうなので。)

@takana-v
Copy link
Member

takana-v commented Nov 7, 2021

エンジン側の変更が必要な場合は対応したいと思います。
0.8.0リリース前に、エンジンの/speaker_infoのdocstring辺りにデータ構造が変わる可能性が高い旨を書いておいた方が良さそうです。

@Hiroshiba
Copy link
Member

@takana-v たしかに、記載した方が良さそうですね!リリースノートに書き加えようと思います。

@shirowanisan
Copy link
Contributor Author

shirowanisan commented Nov 7, 2021

ご検討ありがとうございます。上記をふまえて要件を考えました。

要件定義

  • http://localhost:50021/speaker_infoのレスポンス形式の修正
{
  "policy": "string",
  "portrait": "string",
  "style_infos": [{
    "name": "steing"
    "id": "int"
    "icon": "string"
    "voice_samples": "List[string]"
  },
  {
    "name": "string"
    "id": "int"
    "icon": "string"
    "voice_samples": "List[string]"
  }],
}
  • http://localhost:50021/speakersで得られるstylesstyle_infos不一致の可能性があるが、UIではhttp://localhost:50021/speaker_infoの情報を優先する。
  • UI側のCharacterInfoでの画像・音声のデータの持ち方はbase64を主とし、pathから取得するときはbase64に変えて持たせる
  • style_infosnamehttp://localhost:50021/speakers情報からやって欲しい場合は連絡ください。
    • その場合の懸念点
      • style_idの突合が少し面倒
      • http://localhost:50021/speakersとstyles矛盾がある場合の対応案の変更

再度ご検討いただき、問題なければ、エンジン側の変更をお願いしたいと思いますmm

@Hiroshiba
Copy link
Member

speaker情報とstyle情報で仕様が異なる(nameがあったりなかったり)のは混乱を生みそうとちょっと思いました。
/speakerエンドポイント側にない情報を返すか、/speakerと同じ情報を完全に含んだものを返すと統一感があるかもです。
(正直提案の形でもあまり違いは無さそうなので、まあなんでも良いかなとも思います!)

@shirowanisan
Copy link
Contributor Author

shirowanisan commented Nov 7, 2021

/speakerエンドポイント側にない情報を返す

では、/speaker_infoは/speakerエンドポイント側にない情報を返すようにしましょうか。
idは突合のため必要ですが、nameはなくして以下はどうですか。

  • http://localhost:50021/speaker_infoのレスポンス形式の修正
{
  "policy": "string",
  "portrait": "string",
  "style_infos": [{
    "id": "int"
    "icon": "string"
    "voice_samples": "List[string]"
  },
  {
    "id": "int"
    "icon": "string"
    "voice_samples": "List[string]"
  }],
}
  • UI側ではhttp://localhost:50021/speakersで得られるstylesを主軸とし、追加情報(画像・音声)をhttp://localhost:50021/speaker_infoから得られたstyle_infosのidと突合して追加する。
  • エンジン側のモックのhttp://localhost:50021/speakersで返される。stylesのidの数をhttp://localhost:50021/speaker_infoと矛盾をなくす

※ name突合のため計算量は size(style_infos) * size(speakersでのstyles)になります。

@Hiroshiba
Copy link
Member

はい!そちらの感じで完璧だと思います!!

@shirowanisan
Copy link
Contributor Author

ご検討ありがとうございます!

@takana-v
申し訳ありませんが、こちらの仕様でエンジンの修正が可能ならば対応お願いしますmm

https://github.com/Hiroshiba/voicevox/pull/450#issuecomment-962551574

@shirowanisan shirowanisan force-pushed the feature/character-info branch from c9b3565 to 8899936 Compare November 23, 2021 09:43
@Hiroshiba
Copy link
Member

すみません、リリースを優先して作業していたらまたコンフリクトが発生してしまいました・・・
解決の程よろしくおねがいします。

@shirowanisan shirowanisan force-pushed the feature/character-info branch from bdcd8f9 to 638e36d Compare November 28, 2021 03:09
@shirowanisan
Copy link
Contributor Author

CharacterInfoをエンジンから取って来るようにする

こちら実装しました。お手数をおかけしますが、ご確認お願いします。

@shirowanisan
Copy link
Contributor Author

shirowanisan commented Nov 28, 2021

現状エンジン側に2キャラのデータしかないため、これをマージすると2キャラしか表示されなくなります。
また、現状のengineのmasterでは

  • style_id:0の画像が①_2、(サンプル音声もノーマルではない可能性があります)
  • style_id:2の画像が①_1

といったことが起こっています。

仕様ならば問題なし
#533 (comment)

@Hiroshiba
Copy link
Member

@shirowanisan おまたせしてしまって大変申し訳無いです。。
こちらをマージすると、製品版のエンジンを使ってのエディタの開発ができなくなることに気づきました。
(こちらにまとめています)

こちらが解決されるまでお待ち頂ければと思います。申し訳ないです。。

@Hiroshiba Hiroshiba self-requested a review December 1, 2021 18:58
src/background.ts Outdated Show resolved Hide resolved
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.

長らくおまたせしてしまいました。良さそうです!!
修正ありがとうございます!

4096pxのbase64のdecodeが毎回実行されるためか、テキスト欄左のキャラクター選択UIの表示が毎回とても時間かかるのが気になりました。
簡単に直せそうならそちらも変更して頂けると嬉しいです。
難しそうならissue化したいと思います。

src/store/index.ts Show resolved Hide resolved
src/views/Home.vue Outdated Show resolved Hide resolved
@shirowanisan shirowanisan force-pushed the feature/character-info branch from ac8e189 to 3166476 Compare December 5, 2021 05:20
@shirowanisan
Copy link
Contributor Author

4096pxのbase64のdecodeが毎回実行されるためか、テキスト欄左のキャラクター選択UIの表示が毎回とても時間かかるのが気になりました。

こちら、今回の修正でエンジンが起動するまでキャラクター画像が表示されなくなったので、それで遅くなったと感じている可能性はありませんか?
そうではなくbase64のdecodeによる遅延であるならば、htmlでbase64を表示する上での遅延なので修正は難しいと思います。

@shirowanisan
Copy link
Contributor Author

@Hiroshiba
ご指摘点、修正いたしました。base64のdecodeは必要であるならばissue化します。

申し訳ありませんが、再度ご確認お願いしますmm

@Segu-g
Copy link
Member

Segu-g commented Dec 5, 2021

4096pxのbase64のdecodeが毎回実行されるためか、テキスト欄左のキャラクター選択UIの表示が毎回とても時間かかるのが気になりました。

こちら、今回の修正でエンジンが起動するまでキャラクター画像が表示されなくなったので、それで遅くなったと感じている可能性はありませんか? そうではなくbase64のdecodeによる遅延であるならば、htmlでbase64を表示する上での遅延なので修正は難しいと思います。

恐らくbase64のdecodeよりも画像サイズが大きいファイルを何度もhtmlにinlineで埋め込むことで,DOMのサイズが肥大化しvueのv-nodeの生成,及びhtmlのパース辺りに時間が掛かっているのだと思います.(素人なので間違っていたらすみません)

StyleInfoで渡ってきたbase64文字列をデータ構造に持つと描画時はそのまま埋め込むことになるので,fetch時にdecodeしてBlobに変換したものからurlを取得し,データ構造にはurlのみを持つ形にすれば描画が早くなりました.
この結果だけではデコードに時間が掛かっているのか描画に時間が掛かっているのかは分かりませんが,同じ処理を何度もするのは無駄なのでBlobに変換して持つのは自然だと思います.

const buffer = Buffer.from(styleInfo.icon, "base64");
const iconBlob = new Blob([buffer.buffer], { type: "image/png" });
const iconBlobUrl = URL.createObjectURL(iconBlob);

@shirowanisan
Copy link
Contributor Author

@Segu-g
おおー、そうだったんですね。知識不足ですみません。
ありがとうございます!試してみます。

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.

getDefaultStyleIdsに関するコメントを追記しました!

@Hiroshiba
Copy link
Member

起動してみたのですが、もしかしたら設定ファイルを一度消してから実行するとUIが何も表示されないかもしれません。
%appdata%/voicevox/config.jsonが設定ファイルなので、こちらを一度消して実行してみて頂けると!

@shirowanisan
Copy link
Contributor Author

@Hiroshiba

起動してみたのですが、もしかしたら設定ファイルを一度消してから実行するとUIが何も表示されないかもしれません。

すみません。こちら私の所持する「Mac」「Windows10」で共に試してみたのですが、自分の場合は問題なく表示されました。

今までの挙動の違いとして、config.jsonがないときに
今まではエンジンが起動していなくてもスタイルの選択画面に行きましたが、
今回の修正では、エンジンが起動している状態でないとスタイルの選択画面に遷移しない
という違いがあります。

この違いではなく「設定ファイルを一度消してから実行するとUIが何も表示されない」が起こっているのであれば、とても申し訳ないのですが、自分の環境では今のところ再現できず、誰かのお力添えを得られればと思っています...

@Hiroshiba
Copy link
Member

Hiroshiba commented Dec 13, 2021

あ!! なるほどです、すみません、preview版のエンジンを使えていませんでした!
レビュワーの方のためにプルリク概要に追記させていただこうと思います。
お時間取らせてしまって申し訳ありません・・・ 

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!!
読みやすい良いコードだと感じました!

デフォルトスタイル周りもちょっと変わってくるので、 @MT224244 さん辺りのレビューも頂けると心強いです!

src/store/audio.ts Outdated Show resolved Hide resolved
@Hiroshiba Hiroshiba requested a review from MT224244 December 13, 2021 23:33
@shirowanisan
Copy link
Contributor Author

@Hiroshiba
レビューありがとうございます。LGTMもらえてよかったです。
マージは自分でやる形なのでしょうか?
他の方のレビューもありそうなので、特に希望がなければ、マージはレビュワーの方にお願いしたいと思います。

@Hiroshiba
Copy link
Member

マージはレビュワーにしか権限がないので、お任せください!

Copy link
Contributor

@MT224244 MT224244 left a comment

Choose a reason for hiding this comment

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

LGTMです!

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.

CharacterInfoをエンジンから取って来るようにする
5 participants