Repository navigation
fix(core): preview audio fades play as ramps on the audio clock, so cuts no longer click - #6397
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blocker · 1 important · 0 nits · head 1070a48
The ramp approach is right. But fadeNode now suppresses the stepped fade even where the node can't carry the whole fade, so some fades are lost entirely. Each blocker below was reproduced with a scratch test against this head. ↓ inline
Scope
Read all 5 files end-to-end, plus audioFade.ts, setRate/applyWebAudioRate, scheduleWebAudioFor and the media.ts volume/early-start path. PR tests pass locally (267). CI on this head was still running at review time; the prior head's Comments failure is what 1070a48 fixes. No prior reviews.
— Claude (Genesis)
Edit accuracy: accurate 271 (base branch 271), smooth 153 of thoseThe gate passes. Quarantined, measured but not gated (0) |
…uts no longer click data-fade-in / data-fade-out were folded into one volume per 50 ms sync tick and applied as a step (el.volume, or gain.value in the Web Audio transport), so a 20 ms fade never played and each clip started and stopped at near full level: a click at every cut. The export applies the fades (ffmpeg afade) and does not click. The transport now books each clip's fades on a gain of its own when it schedules the clip, for both decoded buffers and captured media elements: a linear ramp from where the clip starts sounding up to its fade-in, and down to silence at its end. The sync tick hands the transport the level without the fade (onElementVolume's new fifth argument, sent only where a fade applies), so a booked fade is not applied twice. The plain el.volume fallback keeps its stepped fade. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…kip unbounded ends Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
0b83a0c to
518066b
Compare
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 0 nits · 3 resolved, 0 open, 0 new · 1070a48 → 518066b
Scope
Re-read the full webAudioTransport.ts fade path at the new head. The rebase onto main leaves the PR's runtime patch unchanged apart from hunk offsets. Runtime tests pass locally (270). CI is green on this head. 518066be only raises the producer unpacked budget, and CI's Build passes with it.
— Claude (Genesis)
What
Preview audio fades (
data-fade-in/data-fade-out) now play as gain ramps on the audio clock. Before, they were applied as a step once per 50 ms sync tick, so cuts clicked in the preview.Why
A Desktop podcast episode is many short audio clips, each with a 20 ms fade at its cut. The export applies the fades (ffmpeg afade) and is clean; the preview clicked at every cut. In
runtime/media.ts,fadeGain(...)was multiplied into the volume once per 50 ms tick and written as a step:el.volumeon the HTMLMedia fallback, orgainNode.gain.valueinwebAudioTransport.tssetElementVolume. A 20 ms fade never played, so each clip started and stopped at near full level.Related work
Refs #6198 (the same preview, video catch-up at cuts).
How
webAudioTransport.ts:attachClipStagesnow books the clip's fades on a gain of its own (bookedFades), between the source and its clip gain, when the clip is scheduled. That covers both routes: decoded buffers (schedulePlayback) and captured media elements (scheduleMediaElementPlayback, which is the primary route ininit.tsand now receivesclipDuration). The fade is linear, asaudioFade.tsdefines it, clamped to the clip. It starts from where the clip actually starts sounding: pastwindowStartand the playhead, at the partial level if the playhead is mid-fade. It ramps up to the end of the fade-in, holds, and ramps to 0 at the clip's end. The node is torn down with the clip's other nodes.syncRuntimeMedianow also handsonElementVolumethe level without the fade, as a new optional fifth argument sent only where a fade changes the level.setElementVolumeuses that unfaded level for a source whose fades are booked. A source without booked fades, and the plainel.volumefallback (which cannot be scheduled), keep the stepped fade as before.setElementVolume(el, volume, clipGain)behave as before.Test plan
webAudioTransport.test.ts"clip fades on the audio clock". A decoded clip ramps 0 to 1 over its fade-in from its start and 1 to 0 at its end. A clip started halfway into its fade-in starts at 0.5. A captured media element books the same ramps. A clip without fades books nothing. The per-tick volume leaves out a booked fade and keeps an unbooked one.media.test.ts: the transport also gets the level without the fade.media.ts, the media test fails.vitest run src/runtime: 2485 tests pass (after rebuildingpackages/parsers, whose stale dist failed 5 unrelatedvalidateVariablestests onmaintoo). Coretypecheck, the studiotsc --noEmit, oxlint, oxfmt and the fallow audit all exit 0, through the pre-commit hook.bookedFadessays what a stepped fade did.Package size budget.
@hyperframes/producercarries the core runtime in several bundles, so the fade booking pushed its unpacked size to 2,868,742 bytes, 1% over the 2,840,135 budget.scripts/package-size-budgets.jsonraises it to the measured size plus 1% (2,897,429). The packed budget is unchanged.🤖 Generated with Claude Code