fix(core): thread playback rate into WebAudio audio sources#714
Conversation
WebAudioTransport scheduled AudioBufferSourceNodes with the implicit default playbackRate of 1, so non-1x transport rates desynced visuals from audio: GSAP timelines, the transport clock, and native <video> all sped up while WebAudio-routed <audio> clips kept playing at 1x. - schedulePlayback now accepts a rate, sets sourceNode.playbackRate, and scales the future-clip start delay by the rate (the in-progress buffer offset stays elapsed + mediaStart, which is rate-independent). - New setRate() updates active sources in place and rebases the getTime() reference frame so the audio-master clock stays continuous across mid-playback rate changes. - Runtime onSetPlaybackRate now forwards into webAudio.setRate, and player.play() schedules each clip with state.playbackRate. Fixes #713
- Hoist duplicated test mock helpers (createMockAudioContext / setupTransport / mockBuffer / mockEl) from the two describe blocks to module scope. - Drop redundant math-derivation comments in schedulePlayback; the dedicated rate-aware tests are the canonical proof. - Tighten setRate JSDoc. - Add no-op guard in setRate when the new rate equals the current rate, so a duplicate set-playback-rate postMessage doesn't re-anchor or walk active sources for nothing. - Add a regression test for the no-op guard, and strengthen the clamp test to schedule at rate=2 first so the clamp-to-1 assertion is non-vacuous.
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: Approve. Surgical, well-reasoned fix for #713 with strong TDD coverage. Math checks out, scope is tight, no blockers.
The reference-frame rewrite (_rateAnchorCtx / _rateAnchorComp replacing _scheduleOffset) is the right structural move — a single offset can't represent a variable-slope clock, and rebasing on setRate is exactly how you keep getTime() continuous across a rate change. The math is correct: in-progress buffer offset is rate-invariant (elapsed + mediaStart), future-clip wallclock delay scales as -elapsed / r, and the time-anchor rebase preserves continuity (verified by hand against the new tests).
Blockers
None.
Important
packages/core/src/runtime/webAudioTransport.ts:145-170—setRatedoesn't reschedule future-start clips, so a mid-playback rate change can desync future audio.
The PR body calls this out, and you're right thatplay()/pause()/seek()reschedule. But there's noplay()-equivalent on theonSetPlaybackRatepath —init.ts:1589-1593only callsapplyPlaybackRate+transportClock.setRate+webAudio.setRate. So this scenario is reachable today: start at 1×, audio clip A is mid-playback and clip B hasdata-start=20s(future),set-playback-rate 2×fires at composition t=5s.setRateupdates A'splaybackRate.valueand rebases anchors (good), and bumps B'splaybackRate.valuefrom 1 to 2 (good), but B's wallclockstart(when)was anchored at scheduledAt + 15 (rate-1 delay). Now composition time runs at 2× wallclock, so when B's wallclock fires, composition time is ~35s, not 20s — B is 15s late.
Why it's "important" not "blocker": the original bug (visuals at 2×, audio at 1× for clips already playing) is fully fixed. The reachable residual desync requires (a) the speed selector changing rate mid-playback and (b) a future-start clip in the composition. Common enough that it's worth following up, but it doesn't undo the value of this PR.
Suggested follow-up: haveonSetPlaybackRatecallwebAudio.stopAll()+ reschedule via the same loop asplayer.play(). That makes the JSDoc caveat go away. If you don't want to tackle it here, file a tracking issue and link it.
Nits
-
packages/core/src/runtime/webAudioTransport.ts:165-167—setRatewhile_pausedskips the anchor rebase.
Currently harmless becausegetTime()short-circuits to -1 when paused and the nextschedulePlaybackresets the anchors. A one-line comment explaining that invariant would save the next reader 30 seconds. -
packages/core/src/runtime/webAudioTransport.ts:3-6—normalizeRatehas no upper bound,applyPlaybackRateininit.ts:1475clamps to[0.1, 5].
Not a real divergence sinceonSetPlaybackRaterunsapplyPlaybackRatefirst, then passes the clampedstate.playbackRateintowebAudio.setRate. IfwebAudio.setRatewere ever called directly from a path that didn't clamp, you'd get unbounded rates intoplaybackRate.value. Today the contract is fine; flagging because the two clamp policies diverge. -
packages/core/src/runtime/webAudioTransport.ts:86—schedulePlayback's default rate of 1 silently downshifts.
If a future caller forgets to threadstate.playbackRate, the new source plays at 1× while everything else runs at the master rate — i.e. exactly the bug you just fixed, but caller-induced. Only one in-tree caller exists and it's correct, so this isn't actionable now. IfWebAudioTransportever grows a "current master rate" property, defaulting the param tothis._ratewould close the footgun.
Praise
- TDD discipline is visible in the test set. The "getTime tracks composition time after a mid-playback setRate" test (
webAudioTransport.test.ts:295-313) is exactly the kind of property test that would have caught a sloppy implementation that just updatedplaybackRate.valuewithout rebasing the anchor — nice instinct. - The PR body's "How" section is the clearest "here's the math, here's why each branch is correct" writeup I've seen on a transport fix. The rate-invariance derivation for the in-progress buffer offset is worth keeping as a code comment (you did —
webAudioTransport.ts:111-114). - Issue → fix is minimal and scoped. No drive-by refactors, no premature abstraction.
— Vai
What
Threads the transport playback rate into
WebAudioTransportso audio clips routed through it advance at the same rate as the rest of the player (transport clock, GSAP timelines, native<video>elements).Closes #713.
Why
The player exposes a
playback-rateattribute and a speed selector. At non-1× rates, the transport clock, GSAP timelines, and native<video>elements all advance at the new rate — but<audio>clips routed throughWebAudioTransportkept playing at 1× becauseAudioBufferSourceNode.playbackRatewas never set. Result: visuals run at 2×, audio plays at 1×, hard desync within seconds.How
schedulePlayback(...)now accepts an optionalrate(default 1):sourceNode.playbackRate.value = rateon the buffer source.delayby1 / rateso a future clip still fires at the right composition-time boundary (composition time advancesrate ×wallclock).elapsed + mediaStart— that quantity is rate-independent (wallclock-elapsed × rate = composition-elapsed).setRate(rate):playbackRate.valueon every active source in place._rateAnchorCtx/_rateAnchorComp) sogetTime()stays continuous across the rate change — the audio-master clock keeps reporting the same composition time at the moment of the change, then advances at the new slope.packages/core/src/runtime/init.ts):onSetPlaybackRatenow also callswebAudio.setRate(state.playbackRate)afterapplyPlaybackRateandtransportClock.setRate.player.play()schedules each audio clip withstate.playbackRate, so playback that starts after a non-1× rate has been chosen begins at the right rate.Non-finite / non-positive rates are normalized to 1, matching the existing
applyPlaybackRatepolicy.Known limitation (called out in the JSDoc on
setRate): sources scheduled to start in the future viastart(when, ...)retain their original wallclock start time. Callers that need rate-correct rescheduling for future-start clips shouldstopAll()and reschedule; for the in-tree paths, future audio is rescheduled on everyplay()/pause()/seek()so this is currently a non-issue.Test plan
Used TDD — wrote the failing tests first, then implemented.
webAudioTransport.test.ts:schedulePlaybacksetssourceNode.playbackRate.valuefrom the rate arg1 / ratesetRateupdates active sources in placesetRatebefore any sources is a no-opsetRateclampsNaN/0/ negative to 1getTimeadvances at the configured rate, and stays continuous across a mid-playbacksetRatepackages/corepass.packages/playerpass.bunx oxlintandbunx oxfmt --checkclean on touched files.