diff --git a/packages/core/src/audioCarve.test.ts b/packages/core/src/audioCarve.test.ts index be4dcfb14..60fdbec2b 100644 --- a/packages/core/src/audioCarve.test.ts +++ b/packages/core/src/audioCarve.test.ts @@ -10,6 +10,8 @@ import { clipsOverlap, mixCarveSources, couldBeCarveSource, + couldBeCarveBed, + isNamedCarveBed, DEFAULT_CARVE, normalizeCarveSettings, } from "./audioCarve.js"; @@ -538,6 +540,30 @@ describe("classifyAudioName", () => { expect(couldBeCarveSource("sfx-explosion")).toBe(false); }); + // The near-end rule, which nothing used to ask. `couldBeCarveSource` shipped + // with its own doc comment ("music and sfx are out") and no caller; the bed + // side had no predicate at all, so a narration clip was offered the carve and + // — finding one candidate — had one applied for it, against the group it was + // a member of. + it("never offers a voice track as the bed, but keeps an unnamed one eligible", () => { + expect(couldBeCarveBed("music-bed")).toBe(true); + expect(couldBeCarveBed("sfx-riser")).toBe(true); + expect(couldBeCarveBed("a1")).toBe(true); + expect(couldBeCarveBed("vo-2")).toBe(false); + expect(couldBeCarveBed("voiceover")).toBe(false); + expect(couldBeCarveBed("narration-3")).toBe(false); + }); + + // Showing the control is a suggestion; writing the attribute is a decision. + // A decision taken off a name that said nothing is how a carve appears that + // nobody remembers configuring — so `a1` may be offered but never chosen. + it("only self-applies to a name that positively reads as a bed", () => { + expect(isNamedCarveBed("music-bed")).toBe(true); + expect(isNamedCarveBed("sfx-riser")).toBe(true); + expect(isNamedCarveBed("a1")).toBe(false); + expect(isNamedCarveBed("vo-2")).toBe(false); + }); + it("treats underscores as separators, not word characters, for short hints", () => { // `\b` treats `_` as a word character, so `\bbed\b` used to miss `bed_01` — // an underscore-separated bed classified as "unknown" and could end up diff --git a/packages/core/src/audioCarve.ts b/packages/core/src/audioCarve.ts index 265b48352..c43500c5d 100644 --- a/packages/core/src/audioCarve.ts +++ b/packages/core/src/audioCarve.ts @@ -188,6 +188,40 @@ export function couldBeCarveSource(...parts: readonly (string | null | undefined return kind === "voice" || kind === "unknown"; } +/** + * Could this track be the BED a carve is written onto? + * + * The other half of `couldBeCarveSource`, and the half nothing used to ask. A + * carve makes room in a bed for a voice; a voice track has no room to make for + * itself, and offering it the control is offering a track to duck against its + * own kind. Observed: a narration clip in a Voiceover group carved against that + * group — a member ducking the bus it feeds. + * + * Loose in the same direction as its sibling: a name that says nothing stays + * eligible, because a name is a hint and an author may know better. Only a name + * that positively reads as speech is refused. + */ +export function couldBeCarveBed(...parts: readonly (string | null | undefined)[]): boolean { + return classifyAudioName(...parts) !== "voice"; +} + +/** + * Does this track's name positively say "bed"? + * + * Stricter than `couldBeCarveBed`, for the one act the author did not ask for: + * applying a carve on their behalf. Offering the control on a track named `a1` + * is a suggestion they can ignore; writing `data-fx-carve` onto it is a decision, + * and a decision taken off a name that said nothing is how a carve appears that + * nobody remembers configuring. + * + * The same split the source side already makes between what the picker may show + * and what `autoSourceIds` may choose unprompted. + */ +export function isNamedCarveBed(...parts: readonly (string | null | undefined)[]): boolean { + const kind = classifyAudioName(...parts); + return kind === "music" || kind === "sfx"; +} + export const DEFAULT_CARVE: HfCarveSettings = { enabled: true, sources: [], diff --git a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx index ea368a90b..d6e6d2cbf 100644 --- a/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx @@ -1341,6 +1341,62 @@ describe("AudioFxGroup carve by default", () => { expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-carve")).toBe(false); expect(host.querySelector(".hf-fx-carve-module")).toBeNull(); }); + + /** + * Mount a track of the test's own naming as the selection, with siblings. + * + * `mount` always calls the selected track `bed`, which is what let the near-end + * hole go unnoticed: nothing ever selected a track whose NAME said voice. + */ + const mountNamed = (id: string, siblings: string[]) => { + const onSetAttributeQuiet = vi.fn(); + const selected = document.createElement("audio"); + selected.id = id; + document.body.append(selected); + for (const other of siblings) { + const el = document.createElement("audio"); + el.id = other; + document.body.append(el); + } + const host = document.createElement("div"); + document.body.append(host); + act(() => { + createRoot(host).render( + , + ); + }); + return { host, onSetAttributeQuiet }; + }; + + // The reported bug. A carve makes room in a bed for a voice, so a voice track + // is the one thing that can never be the bed — `couldBeCarveSource` has said + // as much since it was written, and nothing called it. Selecting a narration + // clip offered it the module, found one candidate, and applied a carve nobody + // asked for. + it("never offers the carve on a track whose name says voice", () => { + const { host, onSetAttributeQuiet } = mountNamed("vo-2", ["music-bed"]); + expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-carve")).toBe(false); + expect(host.querySelector(".hf-fx-carve-module")).toBeNull(); + }); + + // Offering is a suggestion, applying is a decision. An unnamed track keeps the + // module — the author may know better than the name does — but nothing is + // written until they say so. + it("offers but does not apply on a track whose name says nothing", () => { + const { host, onSetAttributeQuiet } = mountNamed("a1", ["vo-1"]); + expect(host.querySelector(".hf-fx-carve-module")).not.toBeNull(); + expect(onSetAttributeQuiet.mock.calls.some((c) => c[0] === "data-fx-carve")).toBe(false); + }); }); describe("AudioFxGroup carve source list", () => { diff --git a/packages/studio/src/components/editor/useFxCarve.ts b/packages/studio/src/components/editor/useFxCarve.ts index 1cb6f531c..ea2f440e7 100644 --- a/packages/studio/src/components/editor/useFxCarve.ts +++ b/packages/studio/src/components/editor/useFxCarve.ts @@ -26,11 +26,12 @@ import { DEFAULT_CARVE, mixCarveSources, HF_AUDIO_CARVE_ATTR, - normalizeCarveSettings, type HfCarveSettings, } from "@hyperframes/core/audio-carve"; import { resolveAudioGroups, resolveCarveSourceIds } from "@hyperframes/core/audio-groups"; import { + carveBedRoles, + carverAgainst, collectCarveCandidates, CARVE_ABORTED, isPromiseLike, @@ -54,17 +55,6 @@ import type { AudioTrackOption } from "./propertyPanelFxCarveModule.js"; */ const DECODE_SAMPLE_RATE = 48000; -/** Whether some element's own carve attribute names `targetId` as a source. */ -function carvesAgainst(other: HTMLElement, targetId: string): boolean { - try { - const raw = other.getAttribute(HF_AUDIO_CARVE_ATTR); - return Boolean(raw && normalizeCarveSettings(JSON.parse(raw)).sources.includes(targetId)); - } catch { - // An unreadable carve on some other element says nothing about this one. - return false; - } -} - /** * Which carve setting actually moved, by comparing the two snapshots. * @@ -304,24 +294,10 @@ export function useFxCarve( */ onAutoGroupCarveSources?: (clipIds: readonly string[], groupId: string) => Promise, ) { - /** - * Is some other track carving against this one? - * - * A carve is a relationship — a bed is carved against a voice — and the voice is - * the far end of it. Offering the same control there offers to carve a track - * against itself by proxy, and switching it on left a setting with no source it - * could legally name. Read off the other elements' own carve attributes, because - * that is where the relationship is recorded. - */ - const carvedAgainstBy = ((): string | null => { - const doc = element.element?.ownerDocument; - if (!doc || !element.id) return null; - const others = Array.from(doc.querySelectorAll(`[${HF_AUDIO_CARVE_ATTR}]`)); - const carver = others.find( - (other) => other.id !== element.id && carvesAgainst(other, element.id ?? ""), - ); - return carver ? carver.id || "another track" : null; - })(); + const carvedAgainstBy = carverAgainst(element.element?.ownerDocument, element.id); + + // Can this be a bed at all, and may one be applied unasked. See carveBedRoles. + const { couldBeBed, autoBed } = carveBedRoles(element.id, element.element); /** * The tracks worth offering as the voice. @@ -336,13 +312,17 @@ export function useFxCarve( * safe direction: a name that says nothing stays in, voice-shaped names sort * first, and if filtering would leave nothing at all every track comes back. A * picker that hides the track somebody needs is worse than a long one. + * + * Empty on a track that cannot be a bed at all, which is what withholds the + * whole module: `showCarve` already asks "is there anything to carve against", + * so the near-end rule rides that rather than a second flag to thread. */ const { sourceOptions, autoSourceIds } = ((): { sourceOptions: AudioTrackOption[]; autoSourceIds: string[]; } => { const doc = element.element?.ownerDocument; - if (!doc) return { sourceOptions: [], autoSourceIds: [] }; + if (!doc || !couldBeBed) return { sourceOptions: [], autoSourceIds: [] }; const others = Array.from(doc.querySelectorAll("audio[id]")).filter( (a) => a.id !== element.id, ); @@ -353,7 +333,7 @@ export function useFxCarve( const overlapsBed = (a: Element): boolean => clipsOverlap(bedSpan, spanOf(a.getAttribute("data-start"), a.getAttribute("data-duration"))); - const described = collectCarveCandidates(doc, others, overlapsBed); + const described = collectCarveCandidates(doc, others, overlapsBed, element.id ?? undefined); const plausible = described.filter((t) => t.kind === "voice" || t.kind === "unknown"); const offered = plausible.length > 0 ? plausible : described; const byVoiceFirst = (list: typeof described) => @@ -543,7 +523,7 @@ export function useFxCarve( // both guards passing for a single candidate fired two setCarve calls with // the same result — two decodes, two FFT runs, two concurrent attribute // writes. - if (carvedAgainstBy || autoSourceIds.length <= 1) return; + if (carvedAgainstBy || !autoBed || autoSourceIds.length <= 1) return; const all = autoSourceIds; // Nothing configured: the default carve, pointed at everything it could hear. if (carve === null) { @@ -557,7 +537,7 @@ export function useFxCarve( // Keyed on the identity of the decision, not on setCarve — which is rebuilt // every render and would re-fire this. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [carve, carvedAgainstBy, candidateIds]); + }, [carve, carvedAgainstBy, autoBed, candidateIds]); /** * A bed with one obvious voice above it carves itself. @@ -578,7 +558,7 @@ export function useFxCarve( * reason the flag exists rather than "off" being an absent attribute. */ useEffect(() => { - if (carvedAgainstBy || autoSourceIds.length !== 1) return; + if (carvedAgainstBy || !autoBed || autoSourceIds.length !== 1) return; const only = autoSourceIds[0]; if (!only) return; // Nothing configured: the default carve, pointed at the one candidate. @@ -594,7 +574,7 @@ export function useFxCarve( // Deliberately keyed on the identity of the decision, not on setCarve — which // is rebuilt every render and would re-fire this. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [carve, carvedAgainstBy, autoSourceIds.length, autoSourceIds[0]]); + }, [carve, carvedAgainstBy, autoBed, autoSourceIds.length, autoSourceIds[0]]); return { carvedAgainstBy, sourceOptions, setCarve }; } diff --git a/packages/studio/src/components/editor/useFxCarveGrouping.test.ts b/packages/studio/src/components/editor/useFxCarveGrouping.test.ts new file mode 100644 index 000000000..e7a56e6b0 --- /dev/null +++ b/packages/studio/src/components/editor/useFxCarveGrouping.test.ts @@ -0,0 +1,55 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from "vitest"; +import { collectCarveCandidates } from "./useFxCarveGrouping"; + +function previewDoc(html: string): Document { + const doc = document.implementation.createHTMLDocument("preview"); + doc.body.innerHTML = html; + return doc; +} + +/** What the panel would offer as sources for `bedId`, the way useFxCarve asks. */ +function candidatesFor(doc: Document, bedId: string) { + const others = Array.from(doc.querySelectorAll("audio[id]")).filter( + (a) => a.id !== bedId, + ); + return collectCarveCandidates(doc, others, () => true, bedId).map((c) => c.id); +} + +const GROUPED_VOICES = ` + + + + +`; + +describe("collectCarveCandidates", () => { + // The observed bug: selecting vo-2 offered "Voiceover (2)" — the group vo-2 is + // itself a member of. The caller filters out the bed element, but vo-1 survives + // that filter and rolls up into exactly that group. Being the only candidate, it + // was then applied without the author asking: a member ducking the bus it feeds. + it("never offers a member the group it belongs to", () => { + expect(candidatesFor(previewDoc(GROUPED_VOICES), "vo-2")).toEqual(["music-bed"]); + }); + + // And the mirror: a group bed's own id matches no