mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-11 06:30:03 +00:00
fix(studio,core): unify the audio id space, emit group elements, gate both canaries
Six of the fifteen findings from the max-effort review of the audio
groups / mute-solo / pitch-shift stack. Nothing in the stack is merged;
this sits on top of wa-24-timeline-fx.
The id space (findings 1-3). Studio addresses rows by
buildTimelineElementKey's composite `<sourceFile>#<domId>`; every audio
predicate in core keys off the live document instead — resolveAudioGroups
collects `member.id`, isAudibleUnderSolo compares `el.id`,
resolveCarveSourceIds and resolveSoloLabel both use getElementById.
Nobody checked the boundary, so:
* solo put a composite key in the set the runtime matches against
`el.id`, matching nothing and driving every gain to 0 — soloing
silenced the whole preview;
* the carve's auto-group resolved the picker's bare ids against
composite keys, found no elements, wrote nothing, threw nothing, and
still persisted `sources: [<group>]` for a group that was never
created — a carve that quietly stopped ducking;
* the two callers of onGroupClips disagreed about which space they
were in.
Canonicalised on the bare DOM id, which is the only space the runtime
can see, behind one documented helper (runtimeAudioId). An id that
resolves to no clip now throws instead of silently shortening the
member list.
The group element (finding 4). Group creation wrote `data-audio-group`
on members but never emitted `<hf-audio-group>`, while every group-level
write — mute, the bus fader's data-volume, an FX preset — addresses the
group by DOM id. Groups the product created were exactly the groups
nothing could edit. Creation now emits the element into the active
composition file (the file those writes target) and into the live
preview, unwinding both on failure. Group ids are validated before being
interpolated into markup.
The canary leaks (findings 5-6). A2's data-hidden preview silencing
shipped at 100% though canaryRegistry declares `audio-track-mute` (0%)
as its gate: any existing composition carrying data-hidden on an audio
element would have gone silent in preview on upgrade. Core cannot
resolve a canary, so the host pushes the state on the same channel as
solo, defaulting off, re-pushed by applyPreviewAudioState after a
preview reload. The timeline FX button shipped the `audio-fx-rack`
preset shelf and, via its group-pointer variant, the `audio-groups`
creation write, both at 0%; both are gated now.
Tests. Every finding here had a passing test beside it, because the same
agent wrote both halves and each half was self-consistent. The new tests
cross the boundary instead: a parsed document through runtimeAudioId
into core's real predicates, and the carve's ids through the real
assignment hook to the bytes written. Each was mutation-checked against
the pre-fix code.
Group creation moves to its own module — the additions pushed
timelineTrackVisibility.ts past the 600-line ceiling. Also swaps two raw
NUL bytes in useFxCarve.ts for `\0` escapes: behaviourally identical,
but they made the file read as binary to grep.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1d5405e3e0
commit
5850a0c978
@@ -0,0 +1,107 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
/**
|
||||
* The studio → runtime boundary, which nothing else crosses.
|
||||
*
|
||||
* Studio addresses rows by `buildTimelineElementKey`'s composite
|
||||
* `<sourceFile>#<domId>`; every audio predicate in `@hyperframes/core` keys off
|
||||
* the live document instead. Both halves have their own passing tests — one
|
||||
* with composite keys, one with bare ids — and the mismatch between them lived
|
||||
* in the gap. These parse a real document, take the ids the way the UI does,
|
||||
* and hand them to the real core predicates.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
audioGroupOf,
|
||||
isAudibleUnderSolo,
|
||||
resolveAudioGroups,
|
||||
} from "@hyperframes/core/audio-groups";
|
||||
import { parseTimelineFromDOM } from "./timelineDOM";
|
||||
import { runtimeAudioId } from "./timelineElementHelpers";
|
||||
|
||||
function docWith(body: string): Document {
|
||||
const doc = document.implementation.createHTMLDocument("comp");
|
||||
doc.body.innerHTML = body;
|
||||
return doc;
|
||||
}
|
||||
|
||||
const COMPOSITION = `
|
||||
<div data-composition-id="root" data-duration="30"></div>
|
||||
<audio id="voice-1" data-start="0" data-duration="10" data-audio-group="voiceover"></audio>
|
||||
<audio id="voice-2" data-start="10" data-duration="10" data-audio-group="voiceover"></audio>
|
||||
<audio id="music-bed" data-start="0" data-duration="30"></audio>
|
||||
<hf-audio-group id="voiceover"></hf-audio-group>
|
||||
`;
|
||||
|
||||
describe("solo ids cross into the runtime", () => {
|
||||
it("keeps the soloed clip audible and silences the rest", () => {
|
||||
const doc = docWith(COMPOSITION);
|
||||
const elements = parseTimelineFromDOM(doc, 30);
|
||||
const voice1 = elements.find((el) => el.domId === "voice-1");
|
||||
expect(voice1).toBeDefined();
|
||||
// The store key is NOT the runtime's id space — that is the whole point.
|
||||
expect(voice1?.key).not.toBe("voice-1");
|
||||
|
||||
const soloTargetId = runtimeAudioId(voice1 ?? {});
|
||||
expect(soloTargetId).toBe("voice-1");
|
||||
const soloed = new Set([soloTargetId as string]);
|
||||
|
||||
const audible = (id: string) => {
|
||||
const el = doc.getElementById(id);
|
||||
expect(el).not.toBeNull();
|
||||
return isAudibleUnderSolo(soloed, (el as Element).id, audioGroupOf(el as Element));
|
||||
};
|
||||
expect(audible("voice-1")).toBe(true);
|
||||
expect(audible("music-bed")).toBe(false);
|
||||
// Soloing a member does not open its sibling — group solo is the other button.
|
||||
expect(audible("voice-2")).toBe(false);
|
||||
});
|
||||
|
||||
it("soloing the group opens every member", () => {
|
||||
const doc = docWith(COMPOSITION);
|
||||
const group = resolveAudioGroups(doc)[0];
|
||||
const soloed = new Set([group.id]);
|
||||
for (const id of group.memberIds) {
|
||||
const el = doc.getElementById(id) as Element;
|
||||
expect(isAudibleUnderSolo(soloed, el.id, audioGroupOf(el))).toBe(true);
|
||||
}
|
||||
const bed = doc.getElementById("music-bed") as Element;
|
||||
expect(isAudibleUnderSolo(soloed, bed.id, audioGroupOf(bed))).toBe(false);
|
||||
});
|
||||
|
||||
it("a composite key matches nothing — the regression this file exists for", () => {
|
||||
const doc = docWith(COMPOSITION);
|
||||
const voice1 = parseTimelineFromDOM(doc, 30).find((el) => el.domId === "voice-1");
|
||||
const soloed = new Set([voice1?.key ?? ""]);
|
||||
const el = doc.getElementById("voice-1") as Element;
|
||||
expect(isAudibleUnderSolo(soloed, el.id, audioGroupOf(el))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("group membership ids cross into the runtime", () => {
|
||||
it("the ids the timeline hands to onGroupClips are the ids resolveAudioGroups reads back", () => {
|
||||
const doc = docWith(COMPOSITION);
|
||||
const trackElements = parseTimelineFromDOM(doc, 30).filter(
|
||||
(el) => el.tag.toLowerCase() === "audio",
|
||||
);
|
||||
const clipIds = trackElements.map(runtimeAudioId).filter((id): id is string => id !== null);
|
||||
expect(clipIds).toEqual(["voice-1", "voice-2", "music-bed"]);
|
||||
|
||||
// Same space membership is read back in — a composite key here produces a
|
||||
// group whose members nothing can find.
|
||||
const memberIds = resolveAudioGroups(doc).flatMap((g) => g.memberIds);
|
||||
expect(memberIds.every((id) => doc.getElementById(id) !== null)).toBe(true);
|
||||
for (const id of memberIds) expect(clipIds).toContain(id);
|
||||
});
|
||||
|
||||
it("an element with no DOM id is not groupable or soloable", () => {
|
||||
const doc = docWith(`
|
||||
<div data-composition-id="root" data-duration="10"></div>
|
||||
<audio data-start="0" data-duration="5"></audio>
|
||||
`);
|
||||
const [clip] = parseTimelineFromDOM(doc, 10);
|
||||
expect(clip).toBeDefined();
|
||||
expect(runtimeAudioId(clip)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -346,6 +346,25 @@ export function getTimelineElementIdentity(element: { key?: string | null; id: s
|
||||
return element.key ?? element.id;
|
||||
}
|
||||
|
||||
/**
|
||||
* The id space the RUNTIME matches on — a bare DOM id, never a store key.
|
||||
*
|
||||
* Studio addresses rows by `buildTimelineElementKey`'s composite
|
||||
* `<sourceFile>#<domId>`, but everything audio in `@hyperframes/core` keys off
|
||||
* the live document: `resolveAudioGroups` collects `member.id`,
|
||||
* `isAudibleUnderSolo` compares `el.id`, `resolveCarveSourceIds` and
|
||||
* `resolveSoloLabel` both go through `getElementById`. Anything crossing into
|
||||
* that space — a solo id, a group membership list, a carve source — has to be
|
||||
* converted here first; a composite key silently matches nothing.
|
||||
*
|
||||
* `null` for a row with no DOM id at all (selector-addressed elements): such an
|
||||
* element cannot be soloed or grouped, because `resolveAudioGroups` skips
|
||||
* members without an `id` and would build a group that is half there.
|
||||
*/
|
||||
export function runtimeAudioId(element: { domId?: string | null }): string | null {
|
||||
return element.domId || null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Timeline store key for a z-reorder entry built OUTSIDE the timeline
|
||||
* expansion (canvas context menu / LayersPanel), so the reorder commit can
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
|
||||
import type { TimelineElement } from "../store/playerStore";
|
||||
import type { IframeWindow } from "./playbackTypes";
|
||||
import { isCanaryEnabled } from "../../telemetry/canary";
|
||||
import { readClipTiming } from "@hyperframes/core/composition-contract";
|
||||
import {
|
||||
getTimelineElementSelector,
|
||||
@@ -142,6 +143,39 @@ export function setPreviewMediaVolume(iframe: HTMLIFrameElement | null, volume:
|
||||
} catch {}
|
||||
}
|
||||
|
||||
/** Push the `audio-track-mute` canary state into the preview runtime, which
|
||||
* defaults it off (see `window.__hf.setAudioMuteHidden`). Direct call, not a
|
||||
* control message: it is a runtime flag, not a transport command, and the
|
||||
* player host has no equivalent property to set. */
|
||||
function setPreviewMuteHidden(iframe: HTMLIFrameElement | null, enabled: boolean): void {
|
||||
if (!iframe) return;
|
||||
try {
|
||||
const win = iframe.contentWindow as
|
||||
| (Window & { __hf?: { setAudioMuteHidden?: (enabled: boolean) => void } })
|
||||
| null;
|
||||
win?.__hf?.setAudioMuteHidden?.(enabled);
|
||||
} catch {}
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything the preview runtime has to be told about audio after it loads:
|
||||
* the transport's mute, and the canary flags core cannot resolve for itself.
|
||||
* Called from `applyPreviewAudioState`, which is the path that re-runs after a
|
||||
* preview reload — the runtime comes back with every flag at its default and
|
||||
* nothing else pushes them again.
|
||||
*/
|
||||
export function applyPreviewAudioFlags(
|
||||
iframe: HTMLIFrameElement | null,
|
||||
muted: boolean,
|
||||
volume: number,
|
||||
): void {
|
||||
setPreviewMediaMuted(iframe, muted);
|
||||
// Volume too: the transport comes back at unity after a reload, so a preview
|
||||
// the author had turned down came back loud.
|
||||
setPreviewMediaVolume(iframe, volume);
|
||||
setPreviewMuteHidden(iframe, isCanaryEnabled("audio-track-mute"));
|
||||
}
|
||||
|
||||
export function setPreviewPlaybackRate(
|
||||
iframe: HTMLIFrameElement | null,
|
||||
playbackRate: number,
|
||||
|
||||
Reference in New Issue
Block a user