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.

Sign up for GitHub

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

Jump to bottom

feat: アクセント句単位でまとめてモーラ設定値を変更できるようにした #623

Merged
merged 9 commits into from
Jan 16, 2022
Merged

feat: アクセント句単位でまとめてモーラ設定値を変更できるようにした #623

merged 9 commits into from
Jan 16, 2022

Conversation

qwerty2501
Copy link
Contributor

内容

Altを押してる間は一緒に上がるように修正した

関連 Issue

refs #551

スクリーンショット・動画など

accent_phrase-evidence

その他

@qwerty2501 qwerty2501 force-pushed the feature/accent_phrases_adjust_togetter branch 4 times, most recently from 462aff0 to 266bd53 Compare December 29, 2021 13:17
@qwerty2501 qwerty2501 force-pushed the feature/accent_phrases_adjust_togetter branch from 266bd53 to d975a8c Compare December 29, 2021 14:14
@Hiroshiba
Copy link
Member

良いですね！！！

プレビュー中の値も一緒に変動するとより最高かもと思いました。
ただpreview valueの実装周りはデータ構造の都合上かなり大変だと思うので、やるにしても別PRが良いのかなと感じました。

１点だけ、無声化（pitchの値が0）部分だけ変動しないように修正をお願いしたいです･･･！
（pitch==0は特別な値になっているので変動させたくない感じです）

@qwerty2501
pitchが0の場合は変更しないようにした
e3793b7 
pitch0は固定にするという仕様のため
@qwerty2501
Copy link
Contributor Author

pitch0の場合は動かさないように変更しました

Hiroshiba
Hiroshiba requested changes Jan 11, 2022
View reviewed changes
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.

良いですね！！！！！！！
実際使ってみたらなかなか便利に感じました。ぜひ実装したいです！！

public/howtouse.md Outdated Show resolved Hide resolved
src/components/AudioDetail.vue Outdated Show resolved Hide resolved
src/components/AudioDetail.vue Outdated Show resolved Hide resolved
src/components/AudioDetail.vue Outdated
Comment on lines 377 to 386
store.dispatch("COMMAND_SET_AUDIO_MORA_DATA", {
audioKey: props.activeAudioKey,
accentPhraseIndex,
moraIndex,
data: Math.max(
minMoraLength,
Math.min(maxMoraLength, mora.consonantLength + diffData)
),
type: "consonant",
});
Copy link
Member

Choose a reason for hiding this comment

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

なるほど！！

この実装だと、コマンドが複数登録されてしまうので、ctrl+zで一気にやり直すことができなそうです。
一括変更する新しいCOMMAND関数を作ってそちらで修正するのが、一番良い解決だと思います。

が、それはCOMMANDの実装などを把握する必要があって結構大変かもしれません。
実装が難しそうであれば、一旦experimental機能にする手もありそうです。

お手数おかけしちゃいますが、どちらか実装をお願いしたいです･･･！
（個人的には、まあ適当なCOMMANDをコピペして、この処理を移動すれば実装完了な気がしているので、実装しちゃっても良いのかなとちょっと思っています。）

Copy link
Contributor Author

Choose a reason for hiding this comment

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

COMMAND内部でaccentPhraseを取得してるところが見当たらなかったのですが、COMMANDの引数としてaccentPhraseを渡しても問題ないでしょうか？
ここでの作法がよくわからずどう実装してよいか迷っています

Copy link
Member

Choose a reason for hiding this comment

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

accentPhraseは渡しても問題ないはずです！
COMMANDに渡して良い値は、いまのところ制約がないと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

とりあえずコピペしてCOMMAND化しました

src/components/AudioDetail.vue Show resolved Hide resolved
@qwerty2501
Copy link
Contributor Author

shiftで思い出したんですが、スクロール止めることができて何がうれしいんですかね。
shiftでスクロール止めれることはhelpにも書いてないですし、修飾キーが足りないので今の仕様を変更してAlt+Shiftでその文章のすべてのアクセントフレーズの値調整をできるようにしても良いかもと思いました。

@qwerty2501 @Hiroshiba
指摘による説明文の修正反映
958a25b 
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
@Hiroshiba
Copy link
Member

実はElectronはデフォルトで、shiftキーを押しながらスクロールすると横方向にスクロールするという操作があります。
shift＋スクロールで横方向にスクロールしているときにスライダーとマウスが重なると、スライダー操作に変わっちゃうんですよね。
なのでshift押してるときはスライダー操作を無視するようになっています。

これたしかに知らないと絶対わからないのでコメント追加しときます。

@Hiroshiba Hiroshiba mentioned this pull request Jan 12, 2022
Merged
Hiroshiba
Hiroshiba requested changes Jan 13, 2022
View reviewed changes
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.

良いですね！！！

１つコードの修正のお願いと、１つバグっぽい挙動を見つけたので報告です。

長さ調整する時、母音を操作すると母音だけが調整され、子音を操作すると全部の音素が調整されそうです。
https://user-images.githubusercontent.com/4987327/149400122-5ae8fa96-3ad6-422d-9772-04f589a61a28.mp4
挙動としては、母音でも子音でも全体が調整されるのが良いのかなと感じました。

src/store/audio.ts Outdated Show resolved Hide resolved
@qwerty2501
Copy link
Contributor Author

一緒に上がるようにしてたはずですが、移動させる際におかしくなったかもしれません。修正します

@qwerty2501
Copy link
Contributor Author

子音が増減しないバグを修正したので今度こそ大丈夫なはずです

Hiroshiba
Hiroshiba approved these changes Jan 14, 2022
View reviewed changes
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！！

@Hiroshiba Hiroshiba mentioned this pull request Jan 16, 2022
Open
@Hiroshiba
Copy link
Member

マーシしちゃいます！
プレビュー表示する件に関してissueを作成してみました。もしよければぜひ･･･！

@Hiroshiba Hiroshiba merged commit cbf2f37 into VOICEVOX:main Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hiroshiba Hiroshiba Hiroshiba approved these changes

@MT224244 MT224244 Awaiting requested review from MT224244

@Segu-g Segu-g Awaiting requested review from Segu-g

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@qwerty2501 @Hiroshiba