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

マルチトラック:再生・書き出しに対応 #2163

Merged
merged 21 commits into from
Jul 22, 2024

Conversation

sevenc-nanashi
Copy link
Member

内容

再生と書き出しに対応します。

関連 Issue

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

(なし)

その他

@sevenc-nanashi sevenc-nanashi requested a review from a team as a code owner July 7, 2024 11:15
@sevenc-nanashi sevenc-nanashi requested review from Hiroshiba and removed request for a team July 7, 2024 11:15
Comment on lines 239 to 277
export const singingStorePlugin: StorePlugin = (store) => {
watch(
() => store.state.tracks,
async (tracks) => {
if (!audioContext || !globalChannelStrip) {
return;
}
const shouldPlays = shouldPlayTracks(tracks);
for (const [trackId, track] of tracks) {
if (!trackChannelStrips.has(trackId)) {
const channelStrip = new ChannelStrip(audioContext);
trackChannelStrips.set(trackId, channelStrip);
channelStrip.output.connect(globalChannelStrip.input);

trackChannelStrips.set(trackId, channelStrip);
}

const channelStrip = getOrThrow(trackChannelStrips, trackId);
channelStrip.volume = track.gain;
channelStrip.pan = track.pan;
channelStrip.mute = !getOrThrow(shouldPlays, trackId);
}
},
{ deep: true, immediate: true },
);

watch(
() => store.state.selectedTrackId,
async (selectedTrackId) => {
if (!audioContext || !globalChannelStrip || !previewSynth) {
return;
}
const channelStrip = getOrThrow(trackChannelStrips, selectedTrackId);
previewSynth.output.disconnect();
previewSynth.output.connect(channelStrip.input);
},
{ immediate: true },
);
};
Copy link
Member Author

Choose a reason for hiding this comment

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

singing.tsでプラグインを作るようにしました(良い方法が思い浮かばなかった)。
うーん...

src/store/singing.ts Outdated Show resolved Hide resolved
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.

プラグインにする実装良いと思いました!!
singingだけならsingEditorとかでも良いのですが、他のStoreのstateもということを考えるとこの実装が良さそう。

ドキュメントの付け足しとかのコメントが主です!

src/helpers/generateWriteErrorMessage.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/type.ts Outdated Show resolved Hide resolved
src/store/index.ts Outdated Show resolved Hide resolved
Hiroshiba added a commit that referenced this pull request Jul 10, 2024
## 内容

いつものアレです。

## 関連 Issue

- ref: #2163 

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

(なし)

## その他

(なし)
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!!!

僕的には OK です!!
あとは @sigprogramming さんのapproveがあれば良さそう!!

どちらかにしか存在しないtrackIdがある場合はエラーを投げる

この方針良いですね!!!

src/store/singing.ts Show resolved Hide resolved
Comment on lines +185 to +190
const shouldPlays = shouldPlayTracks(tracks);
for (const [trackId, track] of tracks) {
const channelStrip = new ChannelStrip(offlineAudioContext);
channelStrip.volume = track.gain;
channelStrip.pan = track.pan;
channelStrip.mute = !getOrThrow(shouldPlays, trackId);
Copy link
Member

Choose a reason for hiding this comment

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

(このプルリクエストには関係ないけど)

shouldPlayTracks、MapじゃなくてshouldPlayなトラックIDリストを返すようにすればこの辺のgetOrThrowなくせるかも

src/store/singing.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@sigprogramming sigprogramming 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
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
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.

マージします!!


trackChannelStrips、offlineの方はレンダーのたびに作られていて、RENDER関数側ではキャッシュされる形になってますね。
なんか統一できそう感ありますね!


関数切り出しに関しては僕もコメントしようかちょっと迷ってました。
でもどれを関数切り出しするべきで、どれは内部関数で良くて、ってのを考え始めるとよくわかんなくなってきたのでコメントできてませんでした。
なので @sigprogramming さんのコメント助かりました 🙇

たぶんですが、RENDERとofflineRender周りだけ別に切り出しちゃうのが分け方として綺麗かもです。
Vuex Storeが巨大すぎて、関数切り出しをしたらメインのコードと関数のコードが離れすぎてしまうのが厄介な考慮点に感じたので。。
何かリファクタリングの参考になれば!

@Hiroshiba Hiroshiba merged commit f383998 into VOICEVOX:project-multitrack Jul 22, 2024
9 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