Skip to content

fix(core): a clip's volume lane stays smooth in preview while the page is busy - #6057

Merged
miguel-heygen merged 6 commits into
mainfrom
fix/preview-volume-lane-one-writer
Oct 22, 2026
Merged

miguel-heygen merged 6 commits into
mainfrom
fix/preview-volume-lane-one-writer

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

What

A clip with a volume automation lane now plays its lane on a gain node of its own in preview, so the per-tick volume write can no longer step it. Preview of such a clip stays smooth while the page is busy.

Why

In preview, a clip with a data-automation volume lane had two writers on one GainNode:

  1. WebAudioTransport scheduled the lane as ramps on the clip's gainNode when the clip started.
  2. syncRuntimeMedia sampled the same lane every tick and sent it through setElementVolume, which wrote gainNode.gain.value, about 60 times a second.

Setting .value on a param with scheduled ramps inserts a step at the context's current time. Each step carries the lane value from whenever the main thread ran the tick, so on a busy page (videos decoding, heavy UI) the gain jumps and the clip sounds choppy. The more the main thread lags, the larger the steps.

Repro, headless Chrome on Linux, synthetic 1 kHz tone through highpass, gain, compressor and true-peak limiter with a 0 → 1 → 0 volume lane over 6 s, 48 kHz context, main thread blocked 30-60 ms every 100 ms:

sample discontinuities per 6 s, not counting the clip's own end ramp ripple (deviation from a straight ramp)
before 2 to 14 (6 runs) up to 2.3%
after 0 in every run 0.021% to 0.023%

The same clip without a lane measures the same before and after. On an idle page, before and after are both smooth (the steps are small when ticks are on time), but before still writes ~360 times per 6 s onto the scheduled param; after, none.

Related work

Refs the earlier change that fed the lane through the per-tick path so a scheduled envelope was not erased by the per-tick data-volume write (comment in media.test.ts, "volume automation lane"). That fix stays for clips that sound through el.volume; this PR gives routed clips the scheduled lane without a second writer.

How

Invariant: one writer per gain param. A clip's volume lane owns a gain node of its own; the per-tick volume of a routed clip never folds the lane in. This mirrors the group bus in the same file, where the lane rides fader and per-tick and hidden state ride muteGain.

  • webAudioTransport.ts: every routed clip, decoded or media-element, gets a lane gain of its own between its FX and its gainNode (attachClipStages), disconnected at every teardown. It is unity while the clip has no lane.

  • audioFx.ts: the clip's FX handle books that lane gain alongside the clip's other envelopes. The lane is re-aimed on setRate and reanchor, re-booked when data-automation changes, cleared when the lane is removed, and booked when a lane is added mid-play. The group fader rides the same handle, so a group's lane follows rate changes and edits too.

  • media.ts: while the transport routes the element, the per-tick volume no longer folds the lane in (it carries only what multiplies the lane, the clip fades). A routed element always has a lane gain the handle books, so routing and lane ownership are the same condition. Unrouted clips keep driving el.volume from the lane as before.

  • audioAutomationVolume.ts: doc comment updated; it said a volume lane is never scheduled on a gain.

  • scripts/package-size-budgets.json: core's unpacked budget goes from 3935667 to 3936240 bytes (+573). The budget sits at main's exact size; the lane gain, its teardown and the handle booking it are what this fix adds to the runtime bundles.

Render is unchanged: it already bakes lane × fades into the samples.

Test plan

  • Lane follows the transport mid-play (webAudioTransport.test.ts, "volume lanes follow the transport mid-play"): rate change, data-automation edit, lane removal, lane added mid-play, and the group fader's rate change and edit. All 5 fail on the previous head and pass now; transport, media, FX and idle suites 300/300, 3 runs in a row; 24 neighbouring suites 467/467.
  • Unit tests added: webAudioTransport.test.ts (decoded and media-element routes keep the lane's gain untouched by setElementVolume) and media.test.ts (a routed clip's per-tick volume excludes the lane). All 3 fail on main (expected 0.5 to be 1; lane value re-sent) and pass with the fix; both files 242/242, 3 runs in a row.
  • Neighbouring audio suites: 26 files, 520 tests pass; core tsc --noEmit clean; oxfmt --check and oxlint clean.
  • Manual: real-browser A/B of the built runtime with a synthetic tone, recorded on the audio thread (numbers above).
  • Documentation updated (if applicable): n/a
  • Comments follow CONTRIBUTING.md "Comments"

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 22, 2026 10:11

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at c9254348 (two commits on f5873b80). The diagnosis is right, and moving the lane onto a gain of its own fixes the choppiness. One writer per param is the correct invariant. But the new lane gain is only ever scheduled once, when the clip starts. Until this PR, a routed clip's lane was also applied per tick, which kept it in line with edits and speed changes during playback. Without that per tick path, those two cases now regress in preview.

What holds

  • Both routes build the lane stage ahead of gainNode, and FX feed into it (webAudioTransport.ts:436-437, :765-766). All three teardowns disconnect it (:711, :789, :890).
  • routesElement is true only while an active source exists for the element, so transportOwnsLane and the lane node are tied to the same source. Hidden and mute state, and fades, still go through gainNode per tick.
  • The new tests do what they say: setElementVolume leaves the lane gain alone, and a routed clip's per tick volume excludes the lane.

Blocker: the clip's lane gain doesn't follow a rate change or an edit during playback
clipVolumeLaneStage (:119-130) books the envelope against the start-time frame and never touches it again. The FX lanes on the same clip are re-aimed in both cases, and setRate's own doc comment (:828-839) describes this exact bug for them.

  • Preview speed change mid-play. setRate (:841-858) re-aims source.fx, not laneNode. The runtime's stopAll() and reschedule (init.ts, applyWebAudioRate) runs only when a bounded source is active.
    • So an unbounded clip keeps its 1× plan at 2× for the rest of the pass, for example a music bed with no data-duration carrying a volume lane: a fade-out lands at twice its clip time.
    • Before this PR, the per tick write resampled the lane at composition time on every frame, so preview stayed close to the render.
  • A lane edit during playback. The FX handle's observer (audioFx.ts:280-297) reschedules FX lanes on a data-automation edit. Nothing reschedules laneNode, so a moved point isn't heard until the next seek or play.
    • Deleting the lane mid-play is worse. The old envelope keeps running on laneNode, and the per tick path now sends data-volume as well. A deleted fade-out still fades to silence.
    • Adding a lane mid-play gives laneGain !== null with no laneNode, and transportOwnsLane is true. So the clip plays at unity and ignores both the new lane and data-volume.
  • Repro at c9254348: a throwaway test in webAudioTransport.test.ts (not committed) scheduled an unbounded media-element clip with a 0 → 1 → 0 lane, then called setRate(2), then edited the lane. The lane gain's linearRamp/setValue/cancel call counts stayed 2/1/1 after both, and hasBoundedActiveSources() was false.

Suggested fix

  • Keep the lane's frame on the scheduled source. In setRate, re-book laneNode the way fx.setRate does: clear the lane, then scheduleVolumeLane at the live elapsed time and the new rate.
  • On a data-automation edit, re-book the lane as well. The simplest route may be to let the FX handle's existing observer and setRate carry the volume lane as one more automated target.
  • In media.ts:460, gate on whether the active source actually has a lane node, not on routesElement. A lane added mid-play would then fall back to the per tick path instead of unity.
  • Tests: setRate(2) on an unbounded source re-books the lane, and a data-automation edit re-books it, or clears it when the lane is removed.

The group fader that this PR mirrors has the same rate and edit gap. That predates this PR, but the same fix would cover it.

CI: still running at c9254348. Test (support) failed in verify-packed-manifests.test.mjs "installs a same-day dependency bump whose only matching version is an hour old" (bun minimum-release-age). It passes on the base, and this diff doesn't touch it, so it looks time-dependent and is worth a re-run.

Verdict: REQUEST CHANGES
Reasoning: Moving the lane to a gain that is only scheduled at clip start drops the per tick path that kept routed volume lanes correct across speed changes and live edits. The FX lanes in this file already guard against that bug class. A small re-booking on setRate and on edits keeps the smoothness fix without the regression.

— Via

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 153 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@miguel-heygen

miguel-heygen commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks, the blocker was real. Fixed at e94771f5 (plus 5ca2f88c, which shortens the attachClipStages comment).

Clip lane gain now follows the transport. The clip's FX handle books the lane gain as one more target (attachClipStages in webAudioTransport.ts, scheduleVolume in audioFx.ts), so it rides the paths that already re-aim the FX lanes:

  • setRate re-aims it at the live elapsed time and the new rate. An unbounded bed at 2× now fades on clip time.
  • A data-automation edit re-books it through the handle's observer. Removing the lane clears it (cancelScheduledValues(0), back to unity), and a lane added mid-play gets booked.

Group fader. It rides the same handle (fader.gain passed as the volume target), so a group lane also follows rate changes and edits. reanchor clears the fader lane before re-applying data-volume.

media.ts gate. I kept routesElement and did not add a lane-node check. Every routed clip now gets a lane gain, unity until it has a lane, and the handle books any lane the element carries, including one added mid-play. So "routed" and "the transport owns the lane" are now the same condition. A per-tick fallback for a lane added mid-play would bring the second writer back.

Tests. A new describe block, "volume lanes follow the transport mid-play", covers rate change, edit, removal, a lane added mid-play, and the group fader's rate change and edit. All 5 fail at 85a0c5f9 and pass now. Transport, media, FX and idle suites pass 300/300, three runs in a row; the 24 neighbouring suites pass 467/467; tsc, oxfmt and oxlint are clean. A busy-page recording on the audio thread with the rebuilt runtime shows 0 discontinuities and no .value writes onto scheduled params in 3 runs.

Budget. Core unpacked goes to 3936240 (+290 over the previous head), reason in the body.

CI. Test (support) is the Bun 1.4.4 minimum-release-age message-order break on main. #6059 fixes it; I'll rebase once it lands.

…e is busy

A clip with a volume automation lane had two writers on one gain in
preview: the Web Audio transport scheduled the lane as ramps on the
clip's gain, and the per-tick media sync wrote the same lane's value onto
that gain about 60 times a second. Each write landed whenever the main
thread got to it, so a busy page stepped the gain and the clip sounded
choppy. The lane now has its own gain node, like a group's fader beside
its mute gain, and the per-tick volume of a routed clip no longer folds
the lane in.
… gain

The preview fix gives a clip's volume lane its own gain node and tears it
down with the clip, which takes core to 3935950 bytes unpacked, 283 over
the budget main set at its exact size.
@miguel-heygen
miguel-heygen force-pushed the fix/preview-volume-lane-one-writer branch from 37b16ca to bdcbdc4 Compare October 22, 2026 14:22

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed at bdcbdc48. That is 37b16cac (Miguel's pin) rebased onto 2c1d9d50, with every changed file byte-identical to 37b16cac, and main touched none of them since f5873b80. This closes my change request at c9254348 (5612701093).

Since c9254348

  • The lane follows the transport. attachClipStages builds laneNode for every routed clip and hands laneNode.gain to attachElementFxChain as the volume target, so scheduleFor books the lane alongside the FX lanes. That puts it on every path that re-aims them:
    • setRate rebases the frame at the old rate and replays at the new one;
    • reanchor covers seek and play;
    • the data-automation observer reschedules on an edit, or clears with cancelScheduledValues(0) when the lane is removed, so laneNode returns to unity.
  • The mid-play gate is right again. laneNode now always exists for a routed clip, so the media.ts:460 isWebAudioRouted gate is correct. A lane added mid-play is booked by the observer instead of leaving the clip at unity.
  • The group fader uses the same path. It is booked through its own handle (groupEl ? fader.gain : undefined). reanchor re-books from the live element only when it differs from the one the bus was built with.
  • The tests cover the regressions. The five new "volume lanes follow the transport mid-play" tests cover rate, edit, removal, mid-play add, and the group fader at a new rate and on an edit, all on an unbounded media-element clip. Dropping the scheduleVolume line in scheduleFor turns those five and the two existing lane tests red.
  • Locally at 5ca2f88c: the webAudioTransport, audioFx and media tests pass 284/284, and tsc is clean.
  • 5ca2f88c..37b16cac (= this head's content) is comments only. audioAutomationVolume.ts:6 now cites attachClipStages, and the startBounded doc names remaining instead of the nonexistent clipSourceLen. That clears the Comments failure.

Nit (non-blocking)

  • A group bus built before its <hf-audio-group> exists gets the stub element and no fader target, so its lane follows only reanchor, not a rate change or an edit until the next seek or play. That predates this PR (it was reanchor-only before) and is rare.

CI: green at bdcbdc48 (and before that at 37b16cac). All 8 required checks pass (Build, Typecheck, Test, Test: runtime contract, Tests on windows-latest, Render on windows-latest, Semantic PR title, regression), and so does Comments. No check is red. GitHub still lists Test (runtime) as in progress, but its job (119880579937) finished every step with success at 14:28Z, including bun run --filter '@hyperframes/{cli,core,engine}' test. Its CI run 39795109640 concluded success, and the Test aggregate that waits on it went green at 14:31Z. The check-run status is stale.

Verdict: APPROVE
Reasoning: The clip's lane stays on its own gain, so the choppiness fix holds. Booking it through the FX handle puts it on the same rate, edit and reanchor paths as the FX lanes, which closes the regression I flagged. Tests pin each case, and a mutation shows they bite.

— Via

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit cfff449 Oct 22, 2026
79 checks passed
@miguel-heygen
miguel-heygen deleted the fix/preview-volume-lane-one-writer branch October 22, 2026 15:33
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