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

選択した再生デバイスがソングでも適用されるようにする #2375

Merged
merged 21 commits into from
Nov 28, 2024

Conversation

X-20A
Copy link
Contributor

@X-20A X-20A commented Nov 22, 2024

内容

再生機器のソングへの適用自体はAudioContext.setSinkIdでデバイスIDを渡すだけでよかったのですが、デバイスの接続が切れるとかなり厄介ですね。AudioContext(たぶん)が「The AudioContext encountered an error from the audio device or the WebAudio renderer. localhost/:1」を投げて動かなくなります。
理想としては接続が切れたら、
再生中なら停止 → 再生デバイスがデフォルトに戻ったことをユーザーに通知 → 操作を続行できる
としたいかなと思いますが、特定の処理で起こるわけではない(接続が切れた時点でエラー発出)のでcatchが難しいのと、AudioContextを初期化すると関連オブジェクトも再生成しないといけないのとで、どうしようかなあという状況です。

関連 Issue

#2300

@X-20A X-20A requested a review from a team as a code owner November 22, 2024 03:52
@X-20A X-20A requested review from Hiroshiba and removed request for a team November 22, 2024 03:52
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Nov 22, 2024

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

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

@sigprogramming
Copy link
Contributor

特定の処理で起こるわけではない(接続が切れた時点でエラー発出)のでcatchが難しい

たぶんAudioContextの内部でエラーが出ているので、catchは難しそうですね…
AudioContext.statesuspendedになるので、それで一応検知はできると思います)

AudioContextを初期化すると関連オブジェクトも再生成しないといけない

音声グラフを構築する処理がまだ全然整理できていないので、関連オブジェクトの再生成は難しいかもです。
ひとまずこのPRではソングで再生デバイスを変更できるようにするのみ行うのが良いかなと思います。

@X-20A
Copy link
Contributor Author

X-20A commented Nov 22, 2024

そうですね。一旦接続解除のことは考えずに進めようと思います。

AudioContext.stateがsuspendedになるので、それで一応検知はできる

navigator.mediaDevices.addEventListener('devicechange', () => {

audioContext.onstatechange = () => {
でsetSinkIdしてみたのですが、タイミングが競合するのか、エラーの回避は安定しませんでした

@X-20A
Copy link
Contributor Author

X-20A commented Nov 23, 2024

AudioContext.sinkIdについて処理ごとに設定していたのを、読込時と設定変更時に設定するようにしました。こちらのほうが設定漏れが起きづらいかなと。ただし接続解除については考慮していません。

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.

シンプルなコードで良い感じの雰囲気ですね!
エラーメッセージもトーク側と揃えていただいててありがたいです!!

今設定時に変更する形になっていますが、再生時に設定する形にするとよりシンプルになりそうだなと感じました!!
(2箇所にあるapplyDeviceIdが1箇所になるはず)

あとInstrumentにあるaudioContextにsetSinkIdするのではなく、直接AudioContextにsetSinkIdするとシンプルになるかも・・・・・・?
(別のAudioContextなんでしたっけ。。。)

再生関数はSING_PLAY_AUDIOかな。
ここでaudioContext.setSinkIdする感じのイメージです!!

@@ -551,6 +551,7 @@ let clipper: Clipper | undefined;
// NOTE: テスト時はAudioContextが存在しない
if (window.AudioContext) {
audioContext = new AudioContext();
// 読込時にここでAudioContextにsetSinkIdできると簡単かつ確実だけど...
Copy link
Member

Choose a reason for hiding this comment

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

メモ消し忘れかも?

Suggested change
// 読込時にここでAudioContextにsetSinkIdできると簡単かつ確実だけど...

@sigprogramming
Copy link
Contributor

sigprogramming commented Nov 23, 2024

navigator.mediaDevices.addEventListener('devicechange', () => {

audioContext.onstatechange = () => {
でsetSinkIdしてみたのですが、タイミングが競合するのか、エラーの回避は安定しませんでした

音声信号を定期的にバッファに書き込む処理がChromiumにあると思うのですが、たぶんそこでエラーが起こっているので、回避は難しいと思います…!
ユーザーに通知することはできると思うので、ひとまず再起動を促すメッセージを表示するようにするのが良いかもです。
(別プルリクで行っても良いと思います)

@X-20A
Copy link
Contributor Author

X-20A commented Nov 23, 2024

設定をコンポーネントからsinging.tsに移しました。ソング関連の音が出る操作で把握してるのは、
a.再生ボタンクリック
ToolBar.vue > play
singing.ts > SING_PLAY_AUDIO
audioRendering.ts > [Transport.start → Transport.schedule → AudioEventScheduler.schedule → AudioPlayer.play → AudioPlayerVoice.play]

b.再生中に最初に戻る
ToolBar.vue > goToZero
singing.ts > SET_PLAYHEAD_POSITION
audioRendering.ts > Transport.time(setter) → [Transport.start → Transport.schedule → AudioEventScheduler.schedule → AudioPlayer.play → AudioPlayerVoice.play]

c.再生中にクリックで再生位置指定
Container.vue > updatePlayheadTicks
singing.ts > SET_PLAYHEAD_POSITION
audioRendering.ts > Transport.time(setter) → [Transport.start → Transport.schedule → AudioEventScheduler.schedule → AudioPlayer.play → AudioPlayerVoice.play]

d.ノートを打ったときのシンセ音
SequencerKeys.vue > onMouseUp → endPreview
singing.ts > PLAY_PREVIEW_SOUND
audioRendering.ts > PolySynth.noteOn

e.(ノートを打って音声生成前に再生した場合はdから始まって、準備できたら自動でaに切り替わる?)

の5つです。audioRendering.tsでやろうとすると引数で運ぶのがややこしくなりそうなのでsinging.ts内の3箇所で設定しています。

Comment on lines 756 to 761
const applyDeviceId = (device: string) => {
if (transport && previewSynth) {
transport.sinkId = device;
previewSynth.sinkId = device;
}
};
Copy link
Member

Choose a reason for hiding this comment

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

ちょっと頓珍漢なこと言ってるかもなのですが、ここでaudioContext.setSinkId()するのってできたりしますか? 👀
これができればPolySynthTransport側のset sinkIdをなくせるな~~~と思ってます!!

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.

3箇所にコード実装なるほどです!!!

すみません、気づいてなかったのですが、もしかして再生中にsetSinkIdできるのでしょうか 👀
再生中に変更しても問題ないのであれば、設定をwatchする方法で今までの全問題を解決できるかもです!!

たぶんですが、App.vueにこんな感じのコードを書けば良いはず・・・!

useWatchEffect(()=> {
  applyDeviceId(state.savingSetting.audioOutputDevice);
})

(起動直後とstate.savingSetting.audioOutputDeviceが変更されるたびにapplyDeviceIdが呼ばれる・・・・はず!)

@X-20A
Copy link
Contributor Author

X-20A commented Nov 24, 2024

動きますね。盲点でした。
再生中の切り替えも問題ないようです。

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/store/singing.ts Outdated Show resolved Hide resolved
Comment on lines 757 to 758
// AudioContextに再生デバイスのIDを設定
const applyDeviceId = async (device: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

この関数自体をAPPLY_DEVICE_ID内に移してあげるとよりコンパクトかもと思いました!
トーク側はPLAY_AUDIO内に書かれていたので、雰囲気も揃ってメンテナンスしやすいかも。

@@ -1682,6 +1698,12 @@ export const singingStore = createPartialStore<SingingStoreTypes>({
},
},

APPLY_DEVICE_ID: {
Copy link
Member

Choose a reason for hiding this comment

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

ソング側であるということが若干分かりにくいので、後ろにTO_AUDIO_CONTEXTとか付けてあげるのどうでしょう?

Comment on lines 101 to 107
watchEffect(() => {
store.actions
.APPLY_DEVICE_ID({ device: store.state.savingSetting.audioOutputDevice })
.catch((e) => {
console.error(e);
});
});
Copy link
Member

Choose a reason for hiding this comment

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

ここはそのままエラーになって大丈夫です!
(そのエラーが最終的にログに吐き出されます)

なので単純にvoid store.actions.APPLY_DEVICE_ID({ device: store.state.savingSetting.audioOutputDevice })として良さそう?

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/components/App.vue Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
@Hiroshiba Hiroshiba merged commit e0d3869 into VOICEVOX:main Nov 28, 2024
10 checks passed
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