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

ソング:音素タイミングの編集をクエリに適用する関数とそのテストを追加 #2356

Merged

Conversation

sigprogramming
Copy link
Contributor

@sigprogramming sigprogramming commented Nov 16, 2024

内容

音素タイミングの編集をクエリに適用する関数と、その関数のテストを追加します。

音素タイミング編集の適用と調整の流れ

  1. フレーズごとの音素列から全体の音素タイミング列に変換する
    • FramePhoneme[][]PhonemeTiming[]
  2. 音素タイミング編集を適用する
    • フレーズ末尾のpauseはフレーズ最後のノートに含まれるものとして扱う
      • ex. [r a] [pau][r a pau]
  3. 音素タイミングとフレーズの終了フレームを調整する
    1. 各音素のフレーム長が1以上になるように後方から調整する(音素タイミングを変更)
      • 最後の音素は開始フレームではなく終了フレームの方を変更する
      • フレーズの最初の(pauseではない)音素の開始フレームがフレーズの開始フレーム+1以上になるようにする
        • フレーム長が1以上ではなくなるので、次の ii. の調整で1以上にする
    2. 各音素のフレーム長が1以上になるように前方から調整する(音素タイミングを変更)
    3. フレーズ末尾のpauseのフレーム長が1以上になるように調整する(フレーズの終了フレームを変更)
  4. 全体の音素タイミング列からフレーズごとの音素列に変換する
    • PhonemeTiming[]FramePhoneme[][]

関連 Issue

その他

@sigprogramming sigprogramming requested a review from a team as a code owner November 16, 2024 03:46
@sigprogramming sigprogramming requested review from Hiroshiba and removed request for a team November 16, 2024 03:46
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Nov 16, 2024

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ:d1ac958

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.

一旦まだテストまでしか見れていないのですが、ひとまずコメントまで!

設計良さそうに感じました!!
(テストから気持ちが読み取れるのですごくわかりやすかったです!!)

tests/unit/domain/sing/applyPhonemeTimingEdit.spec.ts Outdated Show resolved Hide resolved

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.

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

コメントドキュメントが多くてすごい読みやすかったです!!

ちょくちょくコメント書いていますが、まあぶっちゃけ全部そのままでも問題はなさそう!
共感できるのあったらくらいの気持ちです 🙏

準備できたらマージの合図いただければ!

src/sing/utility.ts Outdated Show resolved Hide resolved
src/sing/domain.ts Outdated Show resolved Hide resolved
let cumulativeFrame = 0;
for (const phoneme of phonemes) {
phonemeTimings.push({
noteId: phoneme.noteId != undefined ? NoteId(phoneme.noteId) : undefined,
Copy link
Member

Choose a reason for hiding this comment

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

(ただのコメントです)

FramePhoneme型の.noteIdがNoteId型になってるEditorFramePhonemeみたいなの用意しても便利かもですね!

src/sing/domain.ts Outdated Show resolved Hide resolved
@sigprogramming
Copy link
Contributor Author

npm run fmtが動かなくなってるかも…?

@Hiroshiba
Copy link
Member

npm run fmtが動かなくなってるかも…?

手元の環境では動きました!!
npm ci(クリーンインストール)で環境戻るかもです。
あるいはファイルによって対象になってなくて動かないとかあるかもです、このあたりは相談いただければ!

@sigprogramming
Copy link
Contributor Author

sigprogramming commented Nov 23, 2024

npm ciしましたが動かないです…
lintのエラーはsrc/sing/domain.tsで出てます。

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.

フォーマッターが動いてなさそうなエラーが確かにいっぱい出てますね・・・!
ちょっとこちらで試してみます!

@Hiroshiba
Copy link
Member

Hiroshiba commented Nov 25, 2024

こちらでも同じように実行できないことが分かりました。
理由はよく分かりませんが、日本語ファイル名だとeslintがsegmentation faultになるようでした!

ちなみにこんな感じのエラーでした。
npm: line 65: 880 Segmentation fault "$NODE_EXE" "$NPM_CLI_JS" "$@"

とりあえず応急処置としては、package.jsonfmtのコマンドの中にあるtestsを省けば通りそうです。
あるいは vscode のフォーマットフォーマットしてくれるプラグイン(pritterとか)を使えば迂回できるかもです。
一旦こちらでコミットさせていただきます!

@Hiroshiba Hiroshiba merged commit ec8ca40 into VOICEVOX:main Nov 25, 2024
10 checks passed
@sigprogramming sigprogramming deleted the add_phoneme_timing_edit_feature branch November 28, 2024 14:22
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.

2 participants