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

同バージョンの別エディションを利用可能にする #303

Closed
Hiroshiba opened this issue Jan 26, 2022 · 9 comments
Closed
Assignees

Comments

@Hiroshiba
Copy link
Member

内容

近い将来やりたいなと思っていることの1つに、高速版と高品質版のコアを作りたいと思っています。
現状、同じバージョンの複数のコアを利用する手段が(たぶん)想定されていないのでissue化してみました。

Pros 良くなる点

VOICEVOXの戦略を広げつつ、ユーザー体験が増えて色々楽しくなる見込み

Cons 悪くなる点

APIの引数がちょっとややこしくなる

実現方法

コア側にコア名とコアUUIDを持たせ、エンジン側のコア管理をコアバージョン+コアUUIDで行う感じを想像しました(もっと良い方法あるかも)

その他

@Hiroshiba
Copy link
Member Author

@PickledChair さんをアサインさせて頂こうと思います!
ref VOICEVOX/voicevox_project#3 (comment)

@tarepan
Copy link
Contributor

tarepan commented Feb 23, 2024

実装目処

リファクタリングの結果、こちらの機能は容易に実装できそうだと判明しました。
現在の Engine は特定ディレクトリ傘下のコアDLLを無差別ロードし、.metas() でコア側から申告されたバージョンを見てリスト登録しています。

metas = json.loads(core.metas())
core_version = metas[0]["version"]
print(f"Info: Loading core {core_version}.")

なので .edition() をコア側に実装し、同じように .edition() でコア側から申告されたエディションを見てリスト登録するだけで基本的には実装可能です(あとはAPI側にエディション引数を新設するだけ)。

本 issue を実際に実装する場合、コア関数新設および VV API 更新を含むものになります。

必要性

@Hiroshiba
2024年2月の時点で、本実装の必要性はどの程度でしょうか?
現状、実装済みのマルチバージョン機能に関しても editor 側では利用されていないと認識しています。

もし現時点で必要性が無くなっているのであれば、実装目処検討に成功したということで、本 issue は close 可能かと思います。

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Feb 23, 2024

2024年2月の時点で、本実装の必要性はどの程度でしょうか?

こちらを聞いてくれるのとても助かります!!
一言で言うと、直ちに必要になることはなさそうです。
でも今後VOICEVOXが発展していくと、結構な確率で必要になるような気がしています。

プロジェクト側にタスクリストを移せば、少なくともこのissueに関しては一旦closeすることは可能・・・かも・・・?

@Hiroshiba
Copy link
Member Author

ものすごい昔の話なので記憶がかなりあやふやなのですが、 @PickledChair さんにもちょっとどうしたらいいか伺えると助かるかもです 🙇
個人的にはまあ何がどうあれ先にコアなので、エンジン側のissueはcloseで良いのかなと思ってます!

@tarepan
Copy link
Contributor

tarepan commented Feb 24, 2024

直ちに必要になることはなさそう

👍
YAGNI の精神に則り、今すぐの実装は早計、ということですね。

一旦closeすることは可能・・・かも

「検討が完了したので来るべき re-open 前提で close する」という建付けはどうでしょうか?
project側に「エンジン側実装の検討完了済み。実装時に当該 issue を reopen して議論再開」と案内すればこれが達成できそうです。

@PickledChair
Copy link
Member

PickledChair commented Feb 24, 2024

@Hiroshiba

ものすごい昔の話なので記憶がかなりあやふやなのですが

私もです 😅

Issue の最初で説明されているように「高速版と高品質版のコア」の構想が(具体的ではないが)あって、通常版のコアと同時に、同じバージョンを付与してリリースするイメージだったのだと思います。具体的な構想が始まるまでは close で良いと思いました!

@tarepan

Issue 整理をありがとうございます!

.edition() をコア側に実装

過去のコアに対しては .edition() を呼ぶことができないため、実装には過去のコアに対して何かしらの考慮が必要であるように思いました……と書いたところで現在のコアのロードの実装を見てみたのですが、 #1008 で追加された以下の行により、過去のコアはきっと読み込めなくなっていますね……(存在しない API を取り扱おうとするので AttributeError になると思います)。

_type_predict_sing_consonant_length_forward(self.core)
_type_predict_sing_f0_forward(self.core)
_type_predict_sing_volume_forward(self.core)
_type_sf_decode_forward(self.core)

以前このあたりの実装をした時の方針では、絶対にエラーを出さないように、この API の型付け前にコアのバージョン判定をなるべく厳格に行なっていましたが、存在しない API にも普通にアクセスしに行って、エラーになったらその API には非対応であるという情報を CoreWrapper が保持するのでも良い気がしました(以前実装した時はそこまで頭が回りませんでした)。その方針に変更すれば、.edition() の API をコアに足してエンジンから呼ぶのも気軽にできそうですね。

@tarepan
Copy link
Contributor

tarepan commented Feb 27, 2024

エラーになったらその API には非対応であるという情報を CoreWrapper が保持するのでも良い

👍
DLLがクラッシュしないかなど考慮すべき事項はありますが、ご指摘の通り、error を catch して非対応フラグ持つ形もありうると思います。

=============
VOICEVOX/voicevox_project#3 (comment) にて本 issue の状況を報告しました。
3人とも close 可能との判断であるため、本 issue は一旦 close とします。

@Hiroshiba
(本 issue の議題と外れる内容ですが)上記で @PickledChair さんが指摘下さったコア後方互換性破壊について、project-s 由来だと思うのですがどこかで議論されているでしょうか?
バグのような気がします。

@Hiroshiba
Copy link
Member Author

コア後方互換性破壊について、project-s 由来だと思うのですがどこかで議論されているでしょうか?
バグのような気がします。

おっしゃる通りバグですね・・・。
とりあえず把握している状況についてissueを作ってみます 🙇

@Hiroshiba
Copy link
Member Author

issueを切り出したのでこのissueは一旦closeが良いのかなと思いました!

VOICEVOXが成長していくと実装の機運が高まると思います。
その際はまた一緒に考えられると嬉しいです!

@Hiroshiba Hiroshiba closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants