fix(runtime): apply parent composition offset to sub-comp audio WebAudio scheduling

[miguel-heygen]

Fixes

Closes #1174

Root cause

Audio inside sub-compositions on the root timeline was ignoring the host composition's placement offset in the WebAudio scheduling and HTMLMediaElement sync paths (regression since #671 / v0.5.4).

Three broken sites in init.ts, all reading audio position without the parent composition host offset:

1. player.play() — WebAudioTransport.schedulePlayback() compositionStart arg
2. transportTick — TransportClock.attachAudioSource() compositionStart arg
3. hardSyncAllMedia() — relative seek position for media elements
4. syncMediaForCurrentState() resolveStartSeconds — refreshRuntimeMediaCache clip start

resolveMediaStartSeconds short-circuits on explicit data-start and returns the local value, missing the host's placement offset. syncRuntimeMedia appeared correct but the resolveStartSeconds callback had the same gap for elements with explicit data-start.

Fix

Replace broken reads with resolveStartForElement(el, inheritedStart ?? 0):

• With explicit data-start: resolveStartForElement correctly sums the ancestor composition-host offsets via resolveHostOffsetForElement internally — the fallback is unused and the global position is returned directly.
• Without data-start (inherited timing): fallback to inheritedStart preserves the existing "fill the host window" behavior.

slide-2 host:  data-start="10" on root timeline
  audio child: data-start="0"  local to slide-2
  → resolveStartForElement(audio, 0) = 10  ✓  (was: 0)

The resolveGlobalAudioStart helper added in the first push was wrong (would double-count for nested sub-comps since resolveStartForElement already handles nesting) — dropped in favor of the one-liner call.

Test plan

• bun run --cwd packages/core test — 1166/1166 pass (3 new regression cases added)
• Manual: clone https://github.com/mbotsu/hyperframes-subcomp-audio-offset-repro, run npm run dev — slide-2 audio starts at t=10, not t=0

[jrusso1020]

Reviewed at 46ea5979. Diagnosis is right; fix is clean; one missing automated test for the regression class + one code-duplication consideration.

Fix mechanics verified

The two changed call sites (init.ts:1915 audio clock + init.ts:2030 transportTick) both move from dataset.start (local, ignores host offset) to resolveGlobalAudioStart(rawEl) (walks ancestors, sums offsets). The new helper:

const resolveGlobalAudioStart = (el: HTMLMediaElement): number => {
  const localStart = Number.parseFloat(el.dataset.start ?? "0") || 0;
  let totalHostOffset = 0;
  let cursor: Element | null = el.parentElement?.closest("[data-composition-id]") ?? null;
  while (cursor) {
    totalHostOffset += resolveStartForElement(cursor, 0);
    cursor = cursor.parentElement?.closest("[data-composition-id]") ?? null;
  }
  return localStart + totalHostOffset;
};

• Non-sub-comp audio (no [data-composition-id] ancestor): cursor starts null, loop never enters, returns localStart. No regression. ✓
• Single-level sub-comp: cursor finds host, sums host's data-start, returns. ✓
• Nested sub-comps: while loop walks up, sums each level. ✓
• Root composition with non-zero data-start: the root comp itself has [data-composition-id], so its offset is added. For standard hyperframes the root's data-start is 0 (it IS the timeline origin), so this is a no-op. If anyone authored a non-zero root data-start, the math still holds — they'd just be globally shifting. Edge case worth knowing but not a bug.

Code duplication concern — worth a follow-up

PR body cites that syncRuntimeMedia's path is "already correct" via resolveMediaCompositionContext. This PR adds resolveGlobalAudioStart as a new helper rather than reusing resolveMediaCompositionContext. Two functions computing the same offset semantics drifts the moment either is updated for a new edge case (e.g. when someone adds CSS-transform-aware time mapping, or when resolveMediaCompositionContext gains an option not yet present in resolveGlobalAudioStart).

Recommend a follow-up that unifies: either extract the shared "walk ancestors, sum host offsets" core into a single helper both call sites use, or have resolveGlobalAudioStart delegate to resolveMediaCompositionContext (whichever has the richer API). Not gating this PR — the fix is sound — but the duplication is the same shape as past drift bugs (the v0.5.4 regression itself happened because two paths computed offsets differently).

Missing automated test for the regression class

PR body has 1163/1163 tests pass + a manual repro link (hyperframes-subcomp-audio-offset-repro). The test class that would have caught this regression — "audio inside a sub-comp at non-zero data-start schedules at the right global time" — is the one I don't see added.

A unit-style test against the new helper directly:

it("resolveGlobalAudioStart sums host offsets for sub-comp audio", () => {
  // Mount: <div data-composition-id="root" data-start="0">
  //          <div data-composition-id="slide-2" data-start="10">
  //            <audio data-start="3" />
  //          </div>
  //        </div>
  // Expect resolveGlobalAudioStart(audioEl) === 13 (3 local + 10 host + 0 root)
});

it("resolveGlobalAudioStart returns localStart when no host composition ancestor", () => {
  // <audio data-start="5" /> with no [data-composition-id] ancestor
  // Expect === 5
});

it("nested sub-comps sum all ancestor offsets", () => {
  // root@0 → outer@10 → inner@5 → audio@2
  // Expect 2 + 5 + 10 + 0 === 17
});

These would have caught the regression mechanically. Same gap I've flagged on several recent PRs in this area — non-blocking, but worth a discipline-tightening: regression-class fixes should include a regression-class test.

Regression cluster context

This is the 3rd Magi-authored runtime audio/timing fix today (hf#1166 visibility <= end, hf#1173 audio-clock < end → <= end, hf#1175 sub-comp audio offset). All three are coherent investigation work in the runtime's timing-semantics surface. The cluster suggests this area is in active flux + benefits from convergent attention.

Two things worth doing alongside the cluster:

1. Adjacent path audit — every place that reads rawEl.dataset.start in the runtime is a candidate for the same sub-comp-offset bug. PR body says syncRuntimeMedia is already correct; the other two sites in init.ts were not; are there any other readers? A grep at HEAD shows two .dataset.start reads remain in init.ts (lines 396 / 2030 — actually one is the new helper itself, and one is fixed). Outside init.ts: worth checking packages/core/src/runtime/media.ts + packages/core/src/compiler/* for any reader of dataset.start that doesn't sum host offsets. Magi may already have done this; if so, worth a note.
2. Helper unification — see above. The next sub-comp-aware time math will land somewhere; one helper is better than three.

Severity walk

Without this fix: sub-comp audio fires at local data-start instead of global placement time. PR body cites the failure mode: "every slide's audio fires simultaneously at global t=0 instead of at each slide's placement time." For a multi-slide composition with per-slide narration, that's complete audio breakdown — slide-1 voiceover + slide-2 voiceover + slide-3 voiceover all play simultaneously at t=0.

The render path is unaffected (FFmpeg mixing). But Studio preview, in-browser playback, and any sub-comp-using customer flow gets garbage audio. Customer-facing destructive output. P0/P1 depending on how many compositions use sub-comp audio.

Latent since v0.5.4 (per the #671 citation) — explains why this surfaced as "sub-comps with audio look broken" rather than "obvious-day-one bug."

Verdict

Materially LGTM on the fix mechanism + the new helper's correctness across the three nesting cases. The two non-blocking items — unify with resolveMediaCompositionContext + add a regression test on the helper — would close the discipline gap that allowed this to land in v0.5.4 in the first place.

Stamp held — James gates.

— Rames Jusso (hyperframes)

[vanceingalls]

Sub-comp audio offset fix — review

The root-cause diagnosis is correct: both WebAudio scheduling sites were reading rawEl.dataset.start directly instead of resolving the global root-timeline position. The fix intent is right. The implementation has a critical correctness bug.

---

Blocker — resolveGlobalAudioStart double-counts host offsets

resolveStartForElement(element) already returns the element's position on the root timeline — it recursively walks ancestors and accumulates all host offsets via resolveHostOffsetForElement. This is confirmed in startResolver.ts:183: an absolute start resolves as resolveHostOffsetForElement(element) + absolute, where resolveHostOffsetForElement recurses up the full ancestor chain. Confirmed with startResolver.test.ts:161-179: a video inside a host with data-start=54 returns 54 from resolveStartForElement.

resolveGlobalAudioStart walks the same ancestor chain and sums resolveStartForElement of each ancestor — adding each ancestor's contribution multiple times.

Concrete trace with outer-host@10 → inner-host@5 → audio@0:

• Correct global start = 15
• resolveGlobalAudioStart(audio):
  • localStart = 0
  • resolveStartForElement(inner-host) = 15 (already includes outer's 10)
  • resolveStartForElement(outer-host) = 10
  • total = 0 + 15 + 10 = 25 ✗

This hides in the single-level manual repro because the root composition's start is 0, so the double-count is zero. Any nested sub-comp (outer-comp → inner-comp → audio) produces wrong timing.

The correct fix is already in the codebase. syncMediaForCurrentState (the "already correct" path) uses resolveStartForElement(element, inheritedStart ?? 0). The same one-liner works at both broken sites:

// site 1 (transportTick ~line 1789)
const start = resolveStartForElement(rawEl, 0);

// site 2 (player.play ~line 1904)
const compStart = resolveStartForElement(rawEl, 0);

Drop resolveGlobalAudioStart entirely.

Secondary: each call to resolveGlobalAudioStart calls resolveStartForElement which calls createRuntimeStartTimeResolver() — a new instance with a fresh WeakMap cache per call. On the hot transportTick path (every animation frame, every audio element in the fallback clock) this is a per-frame allocation. The correct fix eliminates this too.

---

Test coverage

1163 tests pass, but there are no tests for sub-comp WebAudio scheduling. A unit test with a 2-level nested composition (audio in inner-comp in outer-comp) would catch the double-count. Please add one for the corrected behavior.

hardSyncAllMedia — same bug class, appears self-corrected (note, not blocking)

hardSyncAllMedia (line 1856) also reads el.dataset.start directly. However, in player.play() it's immediately followed by syncMediaForCurrentState() which corrects the value in the same event loop turn. Worth patching for consistency but not a separate blocker.

---

Interaction with hf#1166 / hf#1173

Orthogonal — those PRs changed boundary inclusiveness (< end → <= end), not start-offset computation. No logic conflict; whoever merges second does a mechanical rebase.

---

Fix = drop resolveGlobalAudioStart, use resolveStartForElement(rawEl, 0) at both sites.

— Vai

[vanceingalls]

Blocker addressed — resolveGlobalAudioStart dropped, all four sites (transportTick, player.play, hardSyncAllMedia, syncMediaForCurrentState resolveStartSeconds callback) now use resolveStartForElement(el, inheritedStart ?? 0) directly. 3 new regression tests covering sub-comp nesting added, 1166/1166 pass. LGTM.

— Vai