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

[project-s] ハミング機能・歌機能向けのモデル・API(compatible_engineのみ)を追加 #724

Merged
merged 20 commits into from
Jan 8, 2024

Conversation

y-chan
Copy link
Member

@y-chan y-chan commented Jan 6, 2024

内容

題の通りです。
テストは一旦無視しています。
また、ダミーモデルも既存のものを適当に刺しているため、動きません。

関連 Issue

その他

各ネーミングは適当なので、後で書き換えたほうがいいかもしれない...?

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!!!

結構ごちゃつきますが、もうほぼこのままリリースになるのではと思ってます。
VVMに合わせてリファクタリングなりなんなりかなと。

ガッツリ1行1行見てないので、もしかしたらどこかミスしてるとかあり得るかもですが、まあ流石に超ミスってたら音声にならないでしょうということで!

名前に関してのコメントです!
(まあmodel系はAPIとして露出しないので後から変えられるのですが)

  • source filter decode
    • 今思うとsource filterだけ手法の名前なので不揃いかも
      • 将来入出力が変わらないのにsource filterベースの手法じゃなくなったときにややこしくなる
    • decodeが本質で、prefixとして何付けるかかなと
      • あとあとtalkにも使うかもだけど、記念にhummingでも良さそう
      • phoneme・f0・volumeのdecodeだからpfv_decodeとかでも
      • sf_decodeでもy_decodeでも。
  • talk model
    • 良さそう
  • sing style moel
    • styleがスタイルと被るので変えた方が良いかも
    • 「音響特徴量」からfeatureを借りてsing feature model
    • ちょっとエモくしてsing teacher modelもありかも
      • InstructorとかTrainerとかでもGuideとかでも
  • source filter model
    • これだけ手法の名前なのはやっぱりちょっと気になるかも

とりあえずマージしてOKかなと!
名前変更するのも後のPR(metas.json周り変更とか)にくっつけちゃってもらってもOKです。


@qryxip さんにちょっと共有まで。

現状増えるcompatible engineのAPIは以上の予定です。
あとはたしかmetas.jsonのstyleの中にモデルのtypeが増える予定です。

model・・・というか、VVM版におけるInferenceDomainが3種類に増える見込みです。
1つが今までのtalk、1つが歌い方を生成するもの、1つがハミングです。

どれがどの機能を持ってるのかはmetas.jsonのstyleのtypeで判断する形になるかなと・・・!
設計などで疑問点などあればご指摘いただければ!!!

crates/voicevox_core/src/publish.rs Outdated Show resolved Hide resolved
crates/voicevox_core/src/status.rs Show resolved Hide resolved
crates/voicevox_core/src/publish.rs Show resolved Hide resolved
@Hiroshiba
Copy link
Member

Hiroshiba commented Jan 6, 2024

あ、ちなみにgenerate APIのテストが落ちてるのはcargo xtask generate-c-headerでヘッダー更新すれば解決すると思います!

@y-chan
Copy link
Member Author

y-chan commented Jan 8, 2024

source filter decode -> sf_decode
sing style model -> sing teacher model
source filter model -> sf_decode_model

上記のように変更しました!
また、レビューいただいた点を反映しました!
generate APIのテストが落ちているのはよくわかりませんでした...
手元では動いたけど、GitHub Actions上だとダメそうな感じで...

shpinx-autoapiパッケージのバージョンを上げればいい...?
https://stackoverflow.com/questions/77257145/sphinx-autoapi-error-module-object-has-no-attribute-doc-with-various-sphi

@qryxip
Copy link
Member

qryxip commented Jan 8, 2024

generate API

v0.15ではSphinxはv6に上げることで解決してました。(#626)
0.14だともうgenerate API documentsだけ ❌ のまま通してしまうか、起動しないようにするというのもありかと思います。

@qryxip
Copy link
Member

qryxip commented Jan 8, 2024

0.15の方で質問なのですが、"sing teacher"と"sf decode"が別VVMに入ることってありそうですか? もしそうであるなら、パブリックAPIの形をちょっと考えなおす必要がありそうです(歌声を触りたい人がどれだけいるかはわかりませんが)。

@Hiroshiba
Copy link
Member

たしかに、0.15(ハミング)で更新されるのはcompatible engineの部分だけで、ドキュメントに現れるAPIは1個も変わらないですね!
なのでgenerate API documentは切ってしまっても確かに問題なさそう。
けどまあ後で元に戻さないといけないですし、サクッとできるなら #626 をcherry-pickするのもありかも。
どっちでも良さそう!

@Hiroshiba
Copy link
Member

Hiroshiba commented Jan 8, 2024

0.15の方で質問なのですが、"sing teacher"と"sf decode"が別VVMに入ることってありそうですか? もしそうであるなら、パブリックAPIの形をちょっと考えなおす必要がありそうです

ある想定です!
歌い方生成対応キャラ(sing teacher)はなかなか増えないけど、ハミング対応キャラ(sf decode)は増えていくので。

あ、あとモデルの種類が変わるとStyleIdも必ず変えるようにする予定・・・・・だったのですが、今思うとsing teacher modelsf_decode_modelは同じStyleIdにしたくなりそうですね・・・・・・・。
(トークとハミングはStyleIdを変える、というところまでは考えてました。)

VVMでの制約について考えていなかったのですが、1つのVVM内では1つのInferenceDomainしか持てない、みたいな制約は設けられる・・・かも・・・?

歌声を触りたい人がどれだけいるかはわかりませんが

僕も需要は分かりませんが、自分が知る範囲では歌が生成できる動的ライブラリを見たことがないです。
それが無料で、有名なキャラクターもいて、マルチOS対応なものがリリースできれば、まあ結構楽しいことになるんじゃないかな~~~~と期待してます。

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!!

1箇所ミスありそうでしたが、後続のPRでついでに直しちゃう感じでいいかなと思ったのと、差分がわからなくなってしまいそうなので、一旦マージさせていただきます!!

あと自分で提案しといてなんですが、sing teacherは意味が変なので、singing teacherが良いかもとか思いました 😇
(sing volumeとかは別に良さそう感)

singとかsingingとかsongのなんとなくのルールの所感はこうかなと!

  • 生成する系のAPIは動詞(sing・talk)
    • sing_volume、sing_audio_query
  • 物を指すときは名詞(song・talk)
    • song_model、song_library、UI上の「ソング」
  • 英語圏で一般的におかしいときはその限りではない
    • singing_teacher、singing_synthesize

_f0_vector: &[f32],
phoneme: &[i64],
note: &[i64],
_f0: &[f32],
Copy link
Member

Choose a reason for hiding this comment

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

_ミスかも?

Copy link
Member Author

Choose a reason for hiding this comment

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

あ、これはlinterに使ってない引数だと怒られたので、_を入れて回避しています
あとで使うことになるかと思うので、一旦引数として入れていますが、後で変更することになるかと...!

@Hiroshiba Hiroshiba merged commit 1412ec8 into VOICEVOX:project-s Jan 8, 2024
43 of 44 checks passed
Copy link
Member

@qryxip qryxip left a comment

Choose a reason for hiding this comment

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

後追いですがLGTM

Hiroshiba added a commit that referenced this pull request Jan 27, 2024
* remove contour and rename to talk xxx

* fix speaker id map

* rename functions and variables

* add models to model file

* add sing style and source filter models to model file set

* add new models to status

* rename get model index and speaker id

* add new models session

* change i32 to i64

* add new predictor to inference core

* add new predictor to core

* add new predictor to compatible engine

* rename source filter to sf decode

* fix rename miss

* rename sing style to sing teacher

* fix rename miss

* remove vector

* add TODO comment (add sing tests)

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

* fix comment out

* lint

---------

Co-authored-by: Hiroshiba <[email protected]>
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.

3 participants