fix(core,player,studio): bound trimmed audio playback to the clip window#1430
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
When iframe autoplay is blocked, audible playback is promoted to a parent-frame audio proxy. The proxy read the clip's data-start/data-duration once at adopt time and mirrorTime() only skipped (never paused) the element outside that window — so a trimmed/moved music clip kept playing the full source past its on-timeline end, even though the iframe element was correctly paused. Fix: the proxy keeps a reference to its source iframe element and re-reads data-start/data-duration each mirror tick (live trims/moves apply), pauses the proxy when the playhead leaves [start, start+duration), and resumes it when the playhead re-enters during parent-owned playback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
Trimmed audio played to the source file's natural end instead of stopping at the clip edge, on every audio path: - WebAudio (the audible path in Studio): schedulePlayback now passes the clip's data-duration as the third start() arg, so the decoded buffer stops at the trimmed edge instead of running to the file end. - Runtime element gating: the duration resolver caps each clip by its own data-duration (min of source length, host window, authored duration), so a trimmed <audio>/<video> element pauses at its edge. Studio trim UX: - Resize live-patches the media-start/playback-start offset, so a start-edge drag trims into the source instead of only repositioning the clip. - AudioWaveform windows the rendered peaks to the trimmed slice so the waveform tracks the clip edges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
Review follow-ups on the parent-audio-proxy / WebAudio bound: - seekAll now re-reads live source bounds (_refreshEntryBounds) before gating, so a paused scrub right after a trim/move uses the current clip window instead of the adopt-time one. - playAll and clip adoption only start a proxy when the playhead is inside the clip's window (_playEntryIfActive), so bulk starts / promotion no longer blip audio for clips outside their window until the next tick. - The WebAudio buffer is now bounded by the host-composition window too (matching resolveDurationSeconds), so a sub-composition-nested clip stops at the same edge on the WebAudio and HTMLMedia paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
e1b76df to
3530fa3
Compare
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Verdict: approve with one concern + one nit. The fix targets a real, well-characterized bug across three independent audio paths, and the body's "confirmed live: audioOwner=runtime, iframe <audio> paused at edge but WebAudio kept buffer running" is exactly the kind of evidence that makes this PR easy to trust. Tests are tight on the bound math. One nit on mid-playback rate-change interaction with the bounded start() arg.
Verified at HEAD 3530fa3
What ships
webAudioTransport.ts: new optionalclipDurationarg onschedulePlayback; extractedstartBoundedSourcehelper that passes the thirddurationarg toAudioBufferSourceNode.start(when, offset, duration)so the decoded buffer stops at the clip edge.init.ts(runtime element gating):resolveDurationSecondsnow mixesdata-durationinto themin(sourceDuration, hostRemaining, ownDuration)floor; the WebAudio path ondata-composition-idancestors caps the clip span byinheritedStart + inheritedDuration - compStartso sub-composition-nested clips stop at the same edge on both HTMLMedia and WebAudio.parent-media.ts(audio proxy): every entry now retains asourceHTMLMediaElement ref;_refreshEntryBoundsre-readsdata-start/data-durationeachmirrorTime/seekAll/ play-start;_gateEntryPlaybackpauses outside the window and resumes on re-entry;playAlland adopt-time start go through_playEntryIfActiveso bulk starts don't blip out-of-window clips.useTimelineEditing.ts: start-edge resize live-patchesdata-playback-start/data-media-startto the iframe (was: onlydata-start+data-duration) — the missing piece that made the left-edge drag trim into the source instead of just repositioning.useRenderClipContent.ts+AudioWaveform.tsx: peaks are windowed to[mediaStart / sourceDur, (mediaStart + duration*rate) / sourceDur]so the waveform tracks the trimmed slice rather than squeezing the whole source into the clip width.
Bug + fix shape
PR body's "three independent paths" framing is accurate and proven by the diff:
- WebAudio buffer never bounded —
start(when, offset)(no third arg) was the actual silent-fail; the iframe<audio>was correctly paused but inaudible-anyway because it was muted. This is the load-bearing fix. - Runtime element gating missed
data-durationin themin()— fixed by mixing it into a[sourceDuration, hostRemaining, explicitDuration]candidates list. - Parent proxy read
data-start/data-durationonce at adopt time and mirror-skipped (didn't pause) outside the window — fixed by re-reading live + pausing.
Fix shapes are consistent across the three paths (everyone now reads data-duration live; gating is symmetric). The "review follow-ups" commit (3530fa3) adds the sub-composition WebAudio bound + seekAll live-refresh + _playEntryIfActive for adoption — these close the same gap on tangent paths.
Edge cases verified
- Past-end schedule:
schedulePlaybackreturns null + disconnects nodes whenremaining <= 0. ✅ tested. - Future-start schedule with bound:
start(scheduledAt + delay, mediaStart, clipSourceLen)— bound is in buffer seconds, so real-time stop isclipSourceLen / safeRate = clipDurationwall-clock seconds. Math checks out. ✅ tested with rate=2. - Unbounded (legacy):
clipDuration = +∞→hasBound = false→ 2-argstart(). ✅ tested. - Seek to past-end (proxy):
seekAllskips set whenrelTime >= duration, andmirrorTimepauses on the next tick via_gateEntryPlayback. ✅. - Live trim during playback (proxy):
_refreshEntryBoundsre-read eachmirrorTime. ✅ tested. - Stack coherence with HF#1424 (beat detection) / HF#1439 (keyframes dragged onto beats): a keyframe dragged to a beat past the clip's
data-durationwould land outside the audio window — runtime element gating + proxy gate both pause cleanly, no leaked audio. WebAudio buffer just doesn't get scheduled past-end. Clean.
Determinism contract
No new Date.now() / performance.now() calls. All time math goes through AudioContext.currentTime (deterministic per-context) or the existing _getCurrentTime injection. ✅.
:x: emoji significance
Most likely interpretation: regression-shards CI was still in-progress when Vance posted (the only non-success checks are 8 regression-shards (shard-N) jobs still running at the time of review). Every other required check (CI: Test/Typecheck/Build/Lint/CLI smoke/SDK contract, Windows render, Player perf, preview parity, CodeQL) is green. No known-broken state in the PR body or comments. I read the :x: as "regression not done yet, hold on stamp" rather than a defect callout.
Concerns
• webAudioTransport.ts:131,144-146 — Mid-playback rate change after a bounded source is scheduled. The third arg to start(when, offset, duration) is in buffer-sample seconds, fixed at schedule time. setRate() (line 211) mutates sourceNode.playbackRate.value on already-started sources without rescheduling, so a rate bump while a bounded source is playing shortens the audible window in wall time (the buffer is consumed faster than the originally-budgeted duration / oldRate seconds → stops before the trim edge). The doc comment on setRate already acknowledges "Sources scheduled to start in the future keep their original wallclock start time — callers that need rate-correct future starts should stopAll() and reschedule." Worth extending that contract to active sources too when a clip is bounded, or have setRate call stopAll() + reschedule for any clip with a finite bound. Not a launch blocker — the symptom is "audio cuts off slightly early when user changes playback speed mid-clip in Studio," which is way better than the current "audio runs past trim edge." But it's a determinism-adjacent footgun that's worth a follow-up.
Nits
• parent-media.ts:147 — parseFloat(m.source.getAttribute("data-duration") || "Infinity") falls back to "Infinity" only when the attribute is missing; an empty-string attribute (data-duration="") yields NaN, and relTime >= NaN is false → the gate degrades silently to "always inside window." Unlikely in practice (the Studio writes formatted numbers), but Number.isFinite(parsed) ? parsed : Infinity is the robust shape and matches how the runtime resolver guards elsewhere. (nit)
• init.ts:1469-1486 — the candidates.filter(...).Math.min(...) shape is clean but allocates per resolve. If this is hot (per-frame for every <audio>/<video> in the composition), a 3-branch min would be cheaper — though probably below noise. (nit)
Stack notes
Sibling subagents are reviewing HF#1424 (beat detection foundation) and HF#1439 (drag keyframes with live beat snapping) in parallel. Cross-PR check: a beat-snapped keyframe past the audio clip's trimmed end will land outside the WebAudio bound and outside the proxy gate — both paths pause cleanly. No cross-coupling issue between this PR and #1439's drag behavior.
CI
38/45 checks green at HEAD 3530fa3 (CI required-suite all green: Test, Typecheck, Build, Lint, CLI smoke, SDK contract+smoke, Studio load smoke; Player perf load/fps/scrub/drift/parity all green; Windows render tests green; preview-regression green; CodeQL JS+Python green). 8 regression-shards (shard-1..8) still in-progress. This is the likely source of the :x: reaction — recommend holding stamp until those land green.
Review by Rames D Jusso
miguel-heygen
left a comment
There was a problem hiding this comment.
Strengths
-
webAudioTransport.ts:startBoundedSource— clean extraction of the start-with-bound logic into a named helper with a clear boolean contract (returnsfalsewhen the window is already elapsed). The explicitif (hasBound)branches keep the legacy unbounded path orthogonal to the new bounded path, preventing regression. -
init.ts:1456–1489(resolveDurationSeconds) — thecandidates = [sourceDuration, hostRemaining, explicitDuration].filter(...)→Math.min(...candidates)pattern is notably better than the previous two-branchif/returnchain; it makes the three-way contract explicit and handles any subset of finite constraints without extra cases. -
parent-media.ts:_refreshEntryBounds— re-readingdata-start/data-durationfrom the live DOM element each tick is the right architectural choice here. Using asource?reference (withisConnectedguard) avoids any risk of stale closure data while keeping the GC story clean.
Blockers
None found.
Important
-
trimFractionsandrenderAudioClipare untested.packages/studio/src/hooks/useRenderClipContent.test.tshas zero coverage for the newtrimFractions()helper (useRenderClipContent.test.tsonly testsnormalizeCompositionSrc). The waveform window is the most user-visible part of the fix — a regression there (e.g. whenrate == 0, whensourceDuration == el.duration, or whenmediaStart > sourceDuration) would be silent. A few unit tests fortrimFractionswith edge inputs (untrimmed, start-only trim, full trim, rate > 1,sourceDurationunknown) would close this gap. TherenderAudioCliprefactor similarly has no test that passestrimStartFraction/trimEndFractionintoAudioWaveform. -
useTimelineEditing.tslive-patch ofmedia-start/playback-starthas no dedicated test. The new branch atuseTimelineEditing.ts:152–160(patchingdata-playback-startvsdata-media-startdepending onplaybackStartAttr) is the other UI-visible piece of the fix but has no coverage intimelineEditing.test.ts. A test that exercises a start-edge drag on an element whoseplaybackStartAttris"playback-start"vs"media-start"and asserts the correct attribute name is patched would be valuable. -
Sub-composition host-window calc in the WebAudio path (init.ts:2191–2199) uses a different start resolution than the HTMLMedia path. The HTMLMedia resolver calls
resolveMediaStartSeconds(element, inheritedStart)for the audio clip's start (which respectsdata-hf-auto-start). The WebAudio path usesrawEl.dataset.startdirectly (line 2178). Both feed the sameclipDurationcap formula, but if an auto-start audio element inside a sub-composition hasdata-hf-auto-start(no explicitdata-start),rawEl.dataset.startwould beundefined/NaN, causing the whole WebAudio schedule to be skipped (Number.isFinitecheck at line 2180). This is a pre-existing behavior, but the new host-window cap at 2191–2199 makes it newly observable: for those elements theclipDurationdefaults toInfinity, defeating the trim. Worth a follow-up issue if auto-start audio clips with explicitdata-durationtrims are a real scenario.
Nits
useRenderClipContent.ts:82— the// fallow-ignore-next-line complexitysuppression is appropriate (pre-existing handler), but the same technique applied three times acrossuseTimelineEditing.ts:184, 260, 344adds noise; consider a single file-level suppression or a refactor of the outer function if that's in-scope.parent-media.ts:148–158—_playEntryIfActivesilently skips the_audioOwner === "parent"check that_gateEntryPlaybackrelies on (line 158). Both methods call_playEntryunconditionally when inside the window. This is fine because_playEntryIfActiveis only invoked fromplayAllandadopt(both of which check owner/paused before calling it), but the asymmetry is a future footgun — a brief comment at the call sites would preempt confusion.
Verdict: APPROVE
Reasoning: All three independent audio trim paths are now correctly bounded; the fix is well-reasoned, the math is sound, and the new tests cover the critical WebAudio and parent-proxy contracts. The missing unit tests for trimFractions and the useTimelineEditing live-patch are notable but not merge-blocking given the manual verification described in the PR and the breadth of existing CI coverage passing.
— Magi
…aN bounds A bounded WebAudio source's wall-clock length is baked into start()'s duration arg (in buffer-sample seconds) at its scheduling rate. Mutating playbackRate in place on a later rate change does not rescale that bound, so a trimmed clip ends early (fast) or late (slow). setRate now reports whether the rate changed and exposes hasBoundedActiveSources(); the runtime stopAll()+reschedules active clips at the new rate when any bounded source is live. The per-clip schedule loop is extracted to a shared closure so play() and the rate path agree. Also guard _refreshEntryBounds against a non-numeric duration attribute parsing to NaN, which would make every window check false and let the proxy play past its clip end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>

Problem
Trimmed audio clips kept playing past the clip edge — the audio ran on to the source file's natural end even though the timeline clip ended earlier. This happened across every audio path, so fixing one didn't fix the symptom in the Studio.
Root cause
Three independent paths each ignored the clip's authored window (
data-duration/media-start):<hyperframes-player>when iframe autoplay is blocked) — the proxy played to the source end and never re-read live trims.core/runtime) — the duration resolver computed clip length only from source-file length capped by host-composition duration, never consulting the element's owndata-duration.<audio>is muted while WebAudio plays the decoded buffer).schedulePlaybackcalledAudioBufferSourceNode.start(when, offset)with no duration, so the buffer played from the trim offset to the file's natural end regardless of the clip length.Confirmed live:
audioOwner: "runtime", the iframe<audio>wasmuted: true+paused: trueat its trimmed edge (element gating worked), but WebAudio kept the buffer running — so the trim never bound the audible output.Fix
webAudioTransport.ts+init.ts):schedulePlaybacknow takes the clip'sdata-durationand passes a boundeddurationtostart(when, offset, duration), so the buffer stops at the trimmed edge. Past-end schedules are skipped.init.ts): the duration resolver caps each clip bymin(sourceLength, hostWindow, ownDataDuration), so a trimmed<audio>/<video>element pauses at its edge.parent-media.ts): the proxy re-reads its source clip's livedata-start/data-durationeach tick and pauses outside the clip window, resuming on re-entry.Studio trim UX (same root area)
useTimelineEditing.ts): a start-edge drag now live-patches themedia-start/playback-startoffset to the iframe, so dragging the left edge trims into the source instead of only repositioning the clip.AudioWaveform.tsx+useRenderClipContent.ts): the rendered peaks are windowed to the trimmed slice[mediaStart, mediaStart + duration·rate], so the waveform tracks the clip edges instead of squeezing the whole file into the clip width.Tests
webAudioTransport.test.ts: 5 new cases for the clip-duration bound (in-progress, future, past-end skip, rate scaling, unbounded-when-omitted).parent-media.test.ts: pause past trimmed end; re-read livedata-duration.Verification
In Studio: trim the music clip, play past the trimmed edge → audio now goes silent at the edge (previously ran to the source end). Untrimmed playback unchanged.
🤖 Generated with Claude Code