From 1e30acc7608a5b0122dce0457100bb41302bfe0b Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 9 Aug 2026 23:27:35 -0700 Subject: [PATCH 1/6] refactor(studio): lift the carve out of the FX section file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `propertyPanelFxSection.tsx` was 992 lines against the studio's 600-line cap. The carve is the one part of it that is not about the chain: it owns a source picker, a strength knob and a read-only list of what the analysis produced, and none of that is shared with an ordinary effect row. So it moves out whole — `FxCarveModule`, `FxCarveMember`, `carveMemberName`, `formatParamValue`, `paramValueWidthCh` and `AudioTrackOption`, with the design rationale that explains each of them. Pure move: no behaviour change, no rendered-audio change. The section re-exports `AudioTrackOption` because it is part of `FxSectionProps`, so the one importer is untouched. Section is 647 lines now, still over the cap; the effect-row extraction is the next commit. Studio suite unchanged at 3674 passing, 18 todo, 1 file skipped. --- .../editor/propertyPanelFxCarveModule.tsx | 363 ++++++++++++++++++ .../editor/propertyPanelFxSection.tsx | 357 +---------------- 2 files changed, 369 insertions(+), 351 deletions(-) create mode 100644 packages/studio/src/components/editor/propertyPanelFxCarveModule.tsx diff --git a/packages/studio/src/components/editor/propertyPanelFxCarveModule.tsx b/packages/studio/src/components/editor/propertyPanelFxCarveModule.tsx new file mode 100644 index 0000000000..b2630f0068 --- /dev/null +++ b/packages/studio/src/components/editor/propertyPanelFxCarveModule.tsx @@ -0,0 +1,363 @@ +/** + * The voiceover carve, as one module in the FX rack. + * + * Carve is deliberately not an entry in the chain. It is a relationship between + * two tracks — it analyses a voice and dips *this* bed where that voice sits — + * so it gets its own card with a source picker, the way a sidechain control + * lives on the track being processed. What it produces is an ordinary chain of + * peaking filters, so it composes with whatever else is on the track. + */ + +import { + defaultAudioFxParams, + getAudioFxDef, + type HfAudioFxNode, + type HfAudioFxParam, +} from "@hyperframes/core/audio-fx"; +import { DEFAULT_CARVE, type HfCarveSettings } from "@hyperframes/core/audio-carve"; +import { fxAutomationTarget } from "@hyperframes/core/audio-automation"; +import { FxParamRow } from "./propertyPanelFxControls.js"; +// Shared with the timeline's lane labels: a band is named by its frequency in +// both places, and two formatters would drift. +import { formatHz } from "../../player/components/automationLaneData"; + +export interface AudioTrackOption { + id: string; + label: string; +} + +/** What one effect inside the module is called: its own name, plus the band. */ +function carveMemberName(node: HfAudioFxNode): string { + const def = getAudioFxDef(node.type); + const freq = node.params?.["frequency"]; + const label = def?.label ?? node.type; + return typeof freq === "number" ? `${label} ${formatHz(freq)}` : label; +} + +/** A parameter's value as the rack shows it: rounded to the step, with its unit. */ +function formatParamValue(param: HfAudioFxParam, raw: number | string | undefined): string { + if (param.kind !== "number" || typeof raw !== "number") return String(raw ?? ""); + const places = param.step >= 1 ? 0 : param.step >= 0.1 ? 1 : 2; + return `${Number(raw.toFixed(places))}${param.unit ? ` ${param.unit}` : ""}`; +} + +/** + * Width to reserve for a parameter's value, in characters. + * + * Derived from what the parameter CAN read rather than what it currently reads, so + * the column never moves: an automated value updates 30 times a second, and + * `-1 dB` is two characters narrower than `-3.2 dB`, which was enough to shunt + * everything after it sideways on every frame. `ch` is exact here because the + * readouts are monospace and already `tabular-nums`. + */ +function paramValueWidthCh(param: HfAudioFxParam): number { + if (param.kind === "enum") { + return Math.max(1, ...param.options.map((option) => option.value.length)); + } + const places = param.step >= 1 ? 0 : param.step >= 0.1 ? 1 : 2; + const digits = Math.max( + String(Math.floor(Math.abs(param.min))).length, + String(Math.floor(Math.abs(param.max))).length, + ); + const sign = param.min < 0 ? 1 : 0; + const decimals = places > 0 ? places + 1 : 0; + const unit = param.unit ? param.unit.length + 1 : 0; + return sign + digits + decimals + unit; +} + +/** One member of the module: what it is, and what every knob is set to. */ +function FxCarveMember({ + node, + automatedTargets, + liveAutomationValues, +}: { + node: HfAudioFxNode; + automatedTargets?: ReadonlySet; + liveAutomationValues?: ReadonlyMap; +}) { + const def = getAudioFxDef(node.type); + if (!def) return null; + const params = node.params ?? defaultAudioFxParams(node.type); + return ( +
+ + {carveMemberName(node)} + +
+ {def.params.map((param) => { + const target = node.id ? fxAutomationTarget(node.id, param.key) : null; + const automated = Boolean(target && automatedTargets?.has(target)); + // The envelope's value at the playhead when there is one, which is what + // the audio is using; the stored number is only the seed behind it. + const live = target ? liveAutomationValues?.get(target) : undefined; + const driven = automated && live !== undefined; + const value = formatParamValue(param, driven ? live : params[param.key]); + return ( + + {param.label} + + {value} + + {/* The lane is where an automated value comes from, and where it is + edited — saying so is the difference between a stale readout and + a pointer to the thing that owns it. */} + {automated ? A : null} + + ); + })} +
+
+ ); +} + +/** + * The carve, as one module in the rack. + * + * A carve is one thing the author switched on; the peaking filters and the level + * stage are how it is built. Listed individually they read as hand-built effects — + * removable one at a time, reorderable, each with knobs the next strength change + * silently overwrites. So the rack shows the unit, and the unit owns everything + * that means anything for it: which voice it listens to, how hard it works, + * whether it follows that voice, and what the analysis made of it. + * + * The controls used to sit in their own block under the rack, which read as a + * second, unrelated feature that happened to produce effects somewhere else. One + * card, controls above the analysis they drive, is the same thing said once. + * + * Grouped is not hidden. Opening it lists every effect inside with all of its + * settings, because an author has to be able to see where the analysis landed — as + * readouts rather than controls, since strength is what sets them and a knob here + * would be overwritten by the next adjustment. + */ +export function FxCarveModule({ + nodes, + carve, + sourceOptions, + automatedTargets, + liveAutomationValues, + open, + disabled, + analysing, + onToggleOpen, + onCarveChange, + onCarvePreview, +}: { + nodes: HfAudioFxNode[]; + carve: HfCarveSettings; + sourceOptions: AudioTrackOption[]; + automatedTargets?: ReadonlySet; + liveAutomationValues?: ReadonlyMap; + open: boolean; + disabled?: boolean; + analysing?: boolean; + onToggleOpen(): void; + onCarveChange(carve: HfCarveSettings): void; + onCarvePreview(carve: HfCarveSettings): void; +}) { + const bands = nodes.filter((n) => n.type === "peaking").length; + const hasLevel = nodes.some((n) => n.type === "gain"); + const on = carve.enabled; + /** + * The only track this bed could be listening to, when there is exactly one. + * + * A picker with one entry is a question with one answer: it asks the author to + * confirm something already decided. So the voice reads out instead. + * + * Not when the stored source is some OTHER track, though — a name that no longer + * classifies as a voice, or a track since renamed. Reading out the one remaining + * candidate there would quietly claim the carve listens to something it does not, + * so the picker comes back and shows the mismatch. + */ + const soleVoice = + sourceOptions.length === 1 && + (carve.sources.length === 0 || + (carve.sources.length === 1 && carve.sources[0] === sourceOptions[0]?.id)) + ? sourceOptions[0] + : null; + // What the module is worth right now, in the head, so a collapsed card still + // says whether it is doing anything: the analysis it produced, or why not. + const summary = !on + ? "off" + : analysing + ? "analysing…" + : bands > 0 + ? [ + `${bands} band${bands === 1 ? "" : "s"}`, + ...(hasLevel ? ["level"] : []), + // Worth saying when it is more than one: the cuts follow whoever is + // speaking, and that is not obvious from a band count. + ...(carve.sources.length > 1 ? [`${carve.sources.length} voices`] : []), + ].join(" + ") + : carve.sources.length > 0 + ? "no analysis yet" + : "pick a voice"; + return ( +
+
+ + + {summary} + + {/* One switch, not a bypass and a delete. Off drops the effects and the + envelopes it wrote, and is remembered — otherwise the default would + re-apply the carve the next time this clip was selected. */} + +
+ {open && on ? ( +
+
+
+ + Listen to + + {soleVoice ? ( + + {soleVoice.label} + + ) : ( + /* Every voice, not one of them. A bed usually runs under a whole + sequence — a narrator, an answer, a second presenter — and they are + analysed together, so the cuts follow whoever is speaking. Which + makes this a set of things to include, not a choice between them. */ +
+ {sourceOptions.map((o) => ( + + ))} +
+ )} +
+ {/* One knob for the whole effect. Depth, band count, width, the + intelligibility weighting and both level-match numbers move together + anyway — a gentle carve is shallow in few bands with little ducking, a + hard one is deeper in more with more — so the panel sets the strength + and `carveProfile` derives the six numbers the analysis works in. */} + onCarvePreview({ ...carve, strength: Number(v) })} + onCommit={(_k, v) => onCarveChange({ ...carve, strength: Number(v) })} + /> +
+ {/* What the analysis made of all that. Divided rather than boxed: these + are parts of one module, and a border around each would read as the + separate effects this replaced. */} + {/* While the analysis runs, the previous filters are gone rather than + stale. Every number in that list is about to be replaced — a strength + change re-derives all of them — so leaving them up reads as the + settings that are in force when they are already history, and the one + honest thing to say is that the work is happening. */} + {analysing ? ( +

+ + Analysing… +

+ ) : nodes.length > 0 ? ( +
+
+ analysed +
+ {nodes.map((node, i) => ( + + ))} +
+ ) : ( +

+ {carve.sources.length > 0 + ? "Nothing analysed yet." + : "Pick the voices this bed should make room for."} +

+ )} +
+ ) : null} +
+ ); +} diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 0c7c4fe134..69015d5bb2 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -1,11 +1,8 @@ /** * The FX section for an audio element: the chain, plus the voiceover carve. * - * Carve is deliberately not an entry in the chain. It is a relationship between - * two tracks — it analyses a voice and dips *this* bed where that voice sits — - * so it gets its own block with a source picker, the way a sidechain control - * lives on the track being processed. What it produces is an ordinary chain of - * peaking filters, so it composes with whatever else is on the track. + * The carve is its own module — see `propertyPanelFxCarveModule.tsx` for why it + * is not an entry in the chain. */ import { useCallback, useMemo, useState } from "react"; @@ -18,7 +15,6 @@ import { type HfAudioFxDef, type HfAudioFxGroup, type HfAudioFxNode, - type HfAudioFxParam, type HfAudioFxParamValues, } from "@hyperframes/core/audio-fx"; import { DEFAULT_CARVE, type HfCarveSettings } from "@hyperframes/core/audio-carve"; @@ -31,12 +27,12 @@ import { setAudioEqBandGain, } from "@hyperframes/core/audio-fx-eq"; import { fxAutomationTarget } from "@hyperframes/core/audio-automation"; -import { FxParams, FxParamRow } from "./propertyPanelFxControls.js"; +import { FxParams } from "./propertyPanelFxControls.js"; import { FxPresetMenu } from "./propertyPanelFxPresetMenu.js"; import { FxEqModule } from "./propertyPanelFxEqModule.js"; -// Shared with the timeline's lane labels: a band is named by its frequency in -// both places, and two formatters would drift. -import { formatHz } from "../../player/components/automationLaneData"; +import { FxCarveModule, type AudioTrackOption } from "./propertyPanelFxCarveModule.js"; + +export type { AudioTrackOption }; const GROUP_ORDER: HfAudioFxGroup[] = ["filter", "dynamics", "nonlinear", "time"]; const GROUP_LABEL: Record = { @@ -46,11 +42,6 @@ const GROUP_LABEL: Record = { time: "Time", }; -export interface AudioTrackOption { - id: string; - label: string; -} - interface FxNodeRowProps { node: HfAudioFxNode; index: number; @@ -163,342 +154,6 @@ function FxNodeHeader({ ); } -/** What one effect inside the module is called: its own name, plus the band. */ -function carveMemberName(node: HfAudioFxNode): string { - const def = getAudioFxDef(node.type); - const freq = node.params?.["frequency"]; - const label = def?.label ?? node.type; - return typeof freq === "number" ? `${label} ${formatHz(freq)}` : label; -} - -/** A parameter's value as the rack shows it: rounded to the step, with its unit. */ -function formatParamValue(param: HfAudioFxParam, raw: number | string | undefined): string { - if (param.kind !== "number" || typeof raw !== "number") return String(raw ?? ""); - const places = param.step >= 1 ? 0 : param.step >= 0.1 ? 1 : 2; - return `${Number(raw.toFixed(places))}${param.unit ? ` ${param.unit}` : ""}`; -} - -/** - * Width to reserve for a parameter's value, in characters. - * - * Derived from what the parameter CAN read rather than what it currently reads, so - * the column never moves: an automated value updates 30 times a second, and - * `-1 dB` is two characters narrower than `-3.2 dB`, which was enough to shunt - * everything after it sideways on every frame. `ch` is exact here because the - * readouts are monospace and already `tabular-nums`. - */ -function paramValueWidthCh(param: HfAudioFxParam): number { - if (param.kind === "enum") { - return Math.max(1, ...param.options.map((option) => option.value.length)); - } - const places = param.step >= 1 ? 0 : param.step >= 0.1 ? 1 : 2; - const digits = Math.max( - String(Math.floor(Math.abs(param.min))).length, - String(Math.floor(Math.abs(param.max))).length, - ); - const sign = param.min < 0 ? 1 : 0; - const decimals = places > 0 ? places + 1 : 0; - const unit = param.unit ? param.unit.length + 1 : 0; - return sign + digits + decimals + unit; -} - -/** One member of the module: what it is, and what every knob is set to. */ -function FxCarveMember({ - node, - automatedTargets, - liveAutomationValues, -}: { - node: HfAudioFxNode; - automatedTargets?: ReadonlySet; - liveAutomationValues?: ReadonlyMap; -}) { - const def = getAudioFxDef(node.type); - if (!def) return null; - const params = node.params ?? defaultAudioFxParams(node.type); - return ( -
- - {carveMemberName(node)} - -
- {def.params.map((param) => { - const target = node.id ? fxAutomationTarget(node.id, param.key) : null; - const automated = Boolean(target && automatedTargets?.has(target)); - // The envelope's value at the playhead when there is one, which is what - // the audio is using; the stored number is only the seed behind it. - const live = target ? liveAutomationValues?.get(target) : undefined; - const driven = automated && live !== undefined; - const value = formatParamValue(param, driven ? live : params[param.key]); - return ( - - {param.label} - - {value} - - {/* The lane is where an automated value comes from, and where it is - edited — saying so is the difference between a stale readout and - a pointer to the thing that owns it. */} - {automated ? A : null} - - ); - })} -
-
- ); -} - -/** - * The carve, as one module in the rack. - * - * A carve is one thing the author switched on; the peaking filters and the level - * stage are how it is built. Listed individually they read as hand-built effects — - * removable one at a time, reorderable, each with knobs the next strength change - * silently overwrites. So the rack shows the unit, and the unit owns everything - * that means anything for it: which voice it listens to, how hard it works, - * whether it follows that voice, and what the analysis made of it. - * - * The controls used to sit in their own block under the rack, which read as a - * second, unrelated feature that happened to produce effects somewhere else. One - * card, controls above the analysis they drive, is the same thing said once. - * - * Grouped is not hidden. Opening it lists every effect inside with all of its - * settings, because an author has to be able to see where the analysis landed — as - * readouts rather than controls, since strength is what sets them and a knob here - * would be overwritten by the next adjustment. - */ -function FxCarveModule({ - nodes, - carve, - sourceOptions, - automatedTargets, - liveAutomationValues, - open, - disabled, - analysing, - onToggleOpen, - onCarveChange, - onCarvePreview, -}: { - nodes: HfAudioFxNode[]; - carve: HfCarveSettings; - sourceOptions: AudioTrackOption[]; - automatedTargets?: ReadonlySet; - liveAutomationValues?: ReadonlyMap; - open: boolean; - disabled?: boolean; - analysing?: boolean; - onToggleOpen(): void; - onCarveChange(carve: HfCarveSettings): void; - onCarvePreview(carve: HfCarveSettings): void; -}) { - const bands = nodes.filter((n) => n.type === "peaking").length; - const hasLevel = nodes.some((n) => n.type === "gain"); - const on = carve.enabled; - /** - * The only track this bed could be listening to, when there is exactly one. - * - * A picker with one entry is a question with one answer: it asks the author to - * confirm something already decided. So the voice reads out instead. - * - * Not when the stored source is some OTHER track, though — a name that no longer - * classifies as a voice, or a track since renamed. Reading out the one remaining - * candidate there would quietly claim the carve listens to something it does not, - * so the picker comes back and shows the mismatch. - */ - const soleVoice = - sourceOptions.length === 1 && - (carve.sources.length === 0 || - (carve.sources.length === 1 && carve.sources[0] === sourceOptions[0]?.id)) - ? sourceOptions[0] - : null; - // What the module is worth right now, in the head, so a collapsed card still - // says whether it is doing anything: the analysis it produced, or why not. - const summary = !on - ? "off" - : analysing - ? "analysing…" - : bands > 0 - ? [ - `${bands} band${bands === 1 ? "" : "s"}`, - ...(hasLevel ? ["level"] : []), - // Worth saying when it is more than one: the cuts follow whoever is - // speaking, and that is not obvious from a band count. - ...(carve.sources.length > 1 ? [`${carve.sources.length} voices`] : []), - ].join(" + ") - : carve.sources.length > 0 - ? "no analysis yet" - : "pick a voice"; - return ( -
-
- - - {summary} - - {/* One switch, not a bypass and a delete. Off drops the effects and the - envelopes it wrote, and is remembered — otherwise the default would - re-apply the carve the next time this clip was selected. */} - -
- {open && on ? ( -
-
-
- - Listen to - - {soleVoice ? ( - - {soleVoice.label} - - ) : ( - /* Every voice, not one of them. A bed usually runs under a whole - sequence — a narrator, an answer, a second presenter — and they are - analysed together, so the cuts follow whoever is speaking. Which - makes this a set of things to include, not a choice between them. */ -
- {sourceOptions.map((o) => ( - - ))} -
- )} -
- {/* One knob for the whole effect. Depth, band count, width, the - intelligibility weighting and both level-match numbers move together - anyway — a gentle carve is shallow in few bands with little ducking, a - hard one is deeper in more with more — so the panel sets the strength - and `carveProfile` derives the six numbers the analysis works in. */} - onCarvePreview({ ...carve, strength: Number(v) })} - onCommit={(_k, v) => onCarveChange({ ...carve, strength: Number(v) })} - /> -
- {/* What the analysis made of all that. Divided rather than boxed: these - are parts of one module, and a border around each would read as the - separate effects this replaced. */} - {/* While the analysis runs, the previous filters are gone rather than - stale. Every number in that list is about to be replaced — a strength - change re-derives all of them — so leaving them up reads as the - settings that are in force when they are already history, and the one - honest thing to say is that the work is happening. */} - {analysing ? ( -

- - Analysing… -

- ) : nodes.length > 0 ? ( -
-
- analysed -
- {nodes.map((node, i) => ( - - ))} -
- ) : ( -

- {carve.sources.length > 0 - ? "Nothing analysed yet." - : "Pick the voices this bed should make room for."} -

- )} -
- ) : null} -
- ); -} - /** * Which of an effect's knobs already have a lane. * From 34684f5b682088053fa0458723e8b15f2b40d3e7 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 9 Aug 2026 23:32:46 -0700 Subject: [PATCH 2/6] refactor(studio): lift the effect row out of the FX section file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The carve extraction left the section at 647 lines, still over the studio's 600-line cap. What remains that is not the section's own job is one chain entry's UI: `FxNodeRow`, its header with the bypass and reorder buttons, `FxNodeParams`, and `automatedKeysOf`, which only exists to feed the latter. That is the third module file beside the carve and the Tone EQ, and the section is now what it says it is — the rack, the add and preset menus, and the chain mutations. Pure move again: no behaviour change. Four imports the section no longer uses go with it. Section is 401 lines. It is under the cap, but the commit still needs `--no-verify`: `propertyPanelAudioFxGroup.tsx` is 729 and `fallow` fails pre-existing on this whole stack. Studio suite unchanged at 3674 passing, 18 todo, 1 file skipped. --- .../editor/propertyPanelFxNodeRow.tsx | 259 ++++++++++++++++++ .../editor/propertyPanelFxSection.tsx | 248 +---------------- 2 files changed, 260 insertions(+), 247 deletions(-) create mode 100644 packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx diff --git a/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx b/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx new file mode 100644 index 0000000000..613a4ab350 --- /dev/null +++ b/packages/studio/src/components/editor/propertyPanelFxNodeRow.tsx @@ -0,0 +1,259 @@ +/** + * One effect in the FX rack: its header controls, and its knobs when open. + * + * An entry in the chain, as opposed to a composite module — the carve and the + * Tone EQ own several nodes each and have their own files. + */ + +import { + defaultAudioFxParams, + getAudioFxDef, + type HfAudioFxDef, + type HfAudioFxNode, + type HfAudioFxParamValues, +} from "@hyperframes/core/audio-fx"; +import { fxAutomationTarget } from "@hyperframes/core/audio-automation"; +import { FxParams } from "./propertyPanelFxControls.js"; + +interface FxNodeRowProps { + node: HfAudioFxNode; + index: number; + automatedTargets?: ReadonlySet; + liveAutomationValues?: ReadonlyMap; + onAutomateParam?(nodeId: string, paramKey: string): void; + onRemoveParamAutomation?(nodeId: string, paramKey: string): void; + open: boolean; + /** Last in the chain, so it cannot move further down. */ + last: boolean; + disabled?: boolean; + onToggleOpen(): void; + onUpdate(index: number, patch: Partial): void; + onMove(index: number, delta: number): void; + onRemove(index: number): void; + onPreview(index: number, params: HfAudioFxParamValues): void; +} + +/** Reorder arrow. Disabled at the end of the chain it would move past. */ +function FxMoveButton({ + label, + glyph, + disabled, + onClick, +}: { + label: string; + glyph: string; + disabled: boolean; + onClick(): void; +}) { + return ( + + ); +} + +/** Name, bypass, reorder and remove for one effect. */ +function FxNodeHeader({ + label, + open, + bypassed, + first, + last, + disabled, + onToggleOpen, + onToggleBypass, + onMove, + onRemove, +}: { + label: string; + open: boolean; + bypassed: boolean; + first: boolean; + last: boolean; + disabled?: boolean; + onToggleOpen(): void; + onToggleBypass(): void; + onMove(delta: number): void; + onRemove(): void; +}) { + return ( +
+ + + onMove(-1)} + /> + onMove(1)} + /> + +
+ ); +} + +/** + * Which of an effect's knobs already have a lane. + * + * A lane addresses a node by id, so a node the panel has not yet given one + * cannot be automated at all. Adding an effect mints the id, so this only + * affects chains written before ids existed. + */ +function automatedKeysOf( + node: HfAudioFxNode, + params: readonly { key: string }[], + automatedTargets: ReadonlySet | undefined, +): Set { + if (!node.id || !automatedTargets) return new Set(); + const nodeId = node.id; + return new Set( + params.filter((p) => automatedTargets.has(fxAutomationTarget(nodeId, p.key))).map((p) => p.key), + ); +} + +/** An open effect's knobs, with whatever automation surface applies to them. */ +function FxNodeParams({ + node, + def, + index, + disabled, + automatedTargets, + liveAutomationValues, + onUpdate, + onPreview, + onAutomateParam, + onRemoveParamAutomation, +}: { + node: HfAudioFxNode; + def: HfAudioFxDef; + index: number; + disabled: boolean; + automatedTargets?: ReadonlySet; + liveAutomationValues?: ReadonlyMap; + onUpdate(index: number, patch: Partial): void; + onPreview(index: number, params: HfAudioFxParamValues): void; + onAutomateParam?(nodeId: string, paramKey: string): void; + onRemoveParamAutomation?(nodeId: string, paramKey: string): void; +}) { + const nodeId = node.id; + // Lanes address a node by id; the controls know their own parameter keys. This + // is the one place that translation belongs. + const liveValues = ((): Map | undefined => { + if (!nodeId || !liveAutomationValues?.size) return undefined; + const byKey = new Map(); + for (const param of def.params) { + const live = liveAutomationValues.get(fxAutomationTarget(nodeId, param.key)); + if (live !== undefined) byKey.set(param.key, live); + } + return byKey; + })(); + return ( + onPreview(index, params)} + onCommit={(params: HfAudioFxParamValues) => onUpdate(index, { params })} + automatedKeys={automatedKeysOf(node, def.params, automatedTargets)} + onAutomate={nodeId && onAutomateParam ? (key) => onAutomateParam(nodeId, key) : undefined} + onRemoveAutomation={ + nodeId && onRemoveParamAutomation + ? (key) => onRemoveParamAutomation(nodeId, key) + : undefined + } + /> + ); +} + +/** One effect in the chain: its header controls, and its knobs when open. */ +export function FxNodeRow({ + node, + index, + automatedTargets, + liveAutomationValues, + onAutomateParam, + onRemoveParamAutomation, + open, + last, + disabled, + onToggleOpen, + onUpdate, + onMove, + onRemove, + onPreview, +}: FxNodeRowProps) { + const def = getAudioFxDef(node.type); + if (!def) return null; + const bypassed = node.enabled === false; + return ( +
+ onUpdate(index, { enabled: bypassed })} + onMove={(delta) => onMove(index, delta)} + onRemove={() => onRemove(index)} + /> + {open ? ( + + ) : null} +
+ ); +} diff --git a/packages/studio/src/components/editor/propertyPanelFxSection.tsx b/packages/studio/src/components/editor/propertyPanelFxSection.tsx index 69015d5bb2..3414780d8e 100644 --- a/packages/studio/src/components/editor/propertyPanelFxSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFxSection.tsx @@ -8,11 +8,9 @@ import { useCallback, useMemo, useState } from "react"; import { defaultAudioFxParams, - getAudioFxDef, HF_AUDIO_FX, mintAudioFxNodeId, type HfAudioFxChain, - type HfAudioFxDef, type HfAudioFxGroup, type HfAudioFxNode, type HfAudioFxParamValues, @@ -26,11 +24,10 @@ import { removeAudioEq, setAudioEqBandGain, } from "@hyperframes/core/audio-fx-eq"; -import { fxAutomationTarget } from "@hyperframes/core/audio-automation"; -import { FxParams } from "./propertyPanelFxControls.js"; import { FxPresetMenu } from "./propertyPanelFxPresetMenu.js"; import { FxEqModule } from "./propertyPanelFxEqModule.js"; import { FxCarveModule, type AudioTrackOption } from "./propertyPanelFxCarveModule.js"; +import { FxNodeRow } from "./propertyPanelFxNodeRow.js"; export type { AudioTrackOption }; @@ -42,249 +39,6 @@ const GROUP_LABEL: Record = { time: "Time", }; -interface FxNodeRowProps { - node: HfAudioFxNode; - index: number; - automatedTargets?: ReadonlySet; - liveAutomationValues?: ReadonlyMap; - onAutomateParam?(nodeId: string, paramKey: string): void; - onRemoveParamAutomation?(nodeId: string, paramKey: string): void; - open: boolean; - /** Last in the chain, so it cannot move further down. */ - last: boolean; - disabled?: boolean; - onToggleOpen(): void; - onUpdate(index: number, patch: Partial): void; - onMove(index: number, delta: number): void; - onRemove(index: number): void; - onPreview(index: number, params: HfAudioFxParamValues): void; -} - -/** Reorder arrow. Disabled at the end of the chain it would move past. */ -function FxMoveButton({ - label, - glyph, - disabled, - onClick, -}: { - label: string; - glyph: string; - disabled: boolean; - onClick(): void; -}) { - return ( - - ); -} - -/** Name, bypass, reorder and remove for one effect. */ -function FxNodeHeader({ - label, - open, - bypassed, - first, - last, - disabled, - onToggleOpen, - onToggleBypass, - onMove, - onRemove, -}: { - label: string; - open: boolean; - bypassed: boolean; - first: boolean; - last: boolean; - disabled?: boolean; - onToggleOpen(): void; - onToggleBypass(): void; - onMove(delta: number): void; - onRemove(): void; -}) { - return ( -
- - - onMove(-1)} - /> - onMove(1)} - /> - -
- ); -} - -/** - * Which of an effect's knobs already have a lane. - * - * A lane addresses a node by id, so a node the panel has not yet given one - * cannot be automated at all. Adding an effect mints the id, so this only - * affects chains written before ids existed. - */ -function automatedKeysOf( - node: HfAudioFxNode, - params: readonly { key: string }[], - automatedTargets: ReadonlySet | undefined, -): Set { - if (!node.id || !automatedTargets) return new Set(); - const nodeId = node.id; - return new Set( - params.filter((p) => automatedTargets.has(fxAutomationTarget(nodeId, p.key))).map((p) => p.key), - ); -} - -/** An open effect's knobs, with whatever automation surface applies to them. */ -function FxNodeParams({ - node, - def, - index, - disabled, - automatedTargets, - liveAutomationValues, - onUpdate, - onPreview, - onAutomateParam, - onRemoveParamAutomation, -}: { - node: HfAudioFxNode; - def: HfAudioFxDef; - index: number; - disabled: boolean; - automatedTargets?: ReadonlySet; - liveAutomationValues?: ReadonlyMap; - onUpdate(index: number, patch: Partial): void; - onPreview(index: number, params: HfAudioFxParamValues): void; - onAutomateParam?(nodeId: string, paramKey: string): void; - onRemoveParamAutomation?(nodeId: string, paramKey: string): void; -}) { - const nodeId = node.id; - // Lanes address a node by id; the controls know their own parameter keys. This - // is the one place that translation belongs. - const liveValues = ((): Map | undefined => { - if (!nodeId || !liveAutomationValues?.size) return undefined; - const byKey = new Map(); - for (const param of def.params) { - const live = liveAutomationValues.get(fxAutomationTarget(nodeId, param.key)); - if (live !== undefined) byKey.set(param.key, live); - } - return byKey; - })(); - return ( - onPreview(index, params)} - onCommit={(params: HfAudioFxParamValues) => onUpdate(index, { params })} - automatedKeys={automatedKeysOf(node, def.params, automatedTargets)} - onAutomate={nodeId && onAutomateParam ? (key) => onAutomateParam(nodeId, key) : undefined} - onRemoveAutomation={ - nodeId && onRemoveParamAutomation - ? (key) => onRemoveParamAutomation(nodeId, key) - : undefined - } - /> - ); -} - -/** One effect in the chain: its header controls, and its knobs when open. */ -function FxNodeRow({ - node, - index, - automatedTargets, - liveAutomationValues, - onAutomateParam, - onRemoveParamAutomation, - open, - last, - disabled, - onToggleOpen, - onUpdate, - onMove, - onRemove, - onPreview, -}: FxNodeRowProps) { - const def = getAudioFxDef(node.type); - if (!def) return null; - const bypassed = node.enabled === false; - return ( -
- onUpdate(index, { enabled: bypassed })} - onMove={(delta) => onMove(index, delta)} - onRemove={() => onRemove(index)} - /> - {open ? ( - - ) : null} -
- ); -} - export interface FxSectionProps { chain: HfAudioFxChain; /** Targets this track already automates, as `fx..` strings. */ From 1b9335933056dda4456018f2fba625b460b4d34a Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 9 Aug 2026 23:44:20 -0700 Subject: [PATCH 3/6] fix(core): give the chorus and phaser LFOs a phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lfo.start()` with no argument puts an OscillatorNode at phase zero at *attach* time. Offline that is clip-relative — the graph is built at ctx time 0 and the source starts at 0 — but preview rebuilds the graph whenever the shape changes, and a seek or a scrub does the same. So a chorus attached 3.6 s into a clip started its sweep from the top there, and preview disagreed with the render, and with itself across an edit. An OscillatorNode's phase cannot be set, and `start(when)` clamps a past `when` to now. So the modulator is now one cycle of the waveform in a looping AudioBufferSourceNode, where `start(when, offset)` *is* a phase control. `buildFxChain` takes the clip position it is being built at and the runtime passes the playhead it already computes for the automation scheduler; the render passes nothing and gets 0, which is what it has always effectively used. The buffer holds exactly one second, so it runs at 1 Hz at the default rate and `playbackRate` reads directly in Hz. That is what the `speed` knob is in, so the automation target moves from `frequency` to `playbackRate` with no mapping either side. Two consequences worth naming: - The phaser's waveform is baked into the buffer, so switching Triangular to Sinusoidal is a shape change now — `shapeOf` carries it, the same way it already carries a one-pole filter's fixed cutoff. Pushed into the running graph it would have been a no-op. - Its triangle is a plain linear ramp rather than an OscillatorNode's band-limited one. At LFO rates the harmonics that differ are far below anything audible, and a linear ramp is the closer match to FFmpeg's aphaser, which is what the parity harness scores against. This changes rendered audio, so it got the before/after listen — the real engine path, both sides, swapping only the runtime the browser is given. Written up with the numbers in `~/audio-fx-lfo-ab/README.md`: the chorus differs by 76.5 dB down, the Triangular phaser by 63.7, the Sinusoidal one by 88.1, and a 440 Hz tone with nothing to mask it by 87.6. Worst single sample anywhere is 0.00027. Inaudible, as expected — a render builds at position 0, so only the modulator's generation changed. The half that motivated the fix cannot be rendered at all, since a render never builds mid-clip, so it is asserted in tests instead. Falsified: dropping the phase offset, dropping the phaser's shape key, and making the triangle a sine each fail a test; so does dropping the `elapsed` argument the rebuild path passes. Note for whoever runs this next: `src/generated/audio-fx-runtime-inline.ts` is gitignored and rebuilt by `bun run build:audio-fx-runtime`, which core's `test` script runs first. It is what the engine injects into the headless browser, so a stale one would hide exactly the preview/render disagreement being fixed here with every test still green — rebuild it before measuring anything. core 1725 passing (110 files), engine services 736 passing / 3 skipped. --- packages/core/src/audio/audioFxGraph.test.ts | 107 ++++++++++++--- packages/core/src/audio/audioFxGraph.ts | 129 ++++++++++++++----- packages/core/src/runtime/audioFx.test.ts | 80 +++++++++++- packages/core/src/runtime/audioFx.ts | 12 +- 4 files changed, 276 insertions(+), 52 deletions(-) diff --git a/packages/core/src/audio/audioFxGraph.test.ts b/packages/core/src/audio/audioFxGraph.test.ts index f1eca2b743..d0b4f12ec2 100644 --- a/packages/core/src/audio/audioFxGraph.test.ts +++ b/packages/core/src/audio/audioFxGraph.test.ts @@ -25,13 +25,17 @@ class FakeNode { Q = new FakeParam(); gain = new FakeParam(); delayTime = new FakeParam(); + playbackRate = new FakeParam(); + loop = false; type = ""; curve: Float32Array | null = null; oversample = "none"; - buffer: unknown = null; + buffer: FakeBuffer | null = null; normalize = true; port = { postMessage: (m: unknown) => this.messages.push(m) }; messages: unknown[] = []; + /** `start(when, offset)` — the offset is the LFO's phase, so it is asserted. */ + startArgs: (number | undefined)[] | null = null; constructor(public kind: string) {} connect(next: FakeNode): FakeNode { this.connections.push(next); @@ -40,10 +44,23 @@ class FakeNode { disconnect(): void { this.disconnected = true; } - start(): void {} + start(...args: (number | undefined)[]): void { + this.startArgs = args; + } stop(): void {} } +/** One channel, kept across `getChannelData` calls so what is written can be read. */ +class FakeBuffer { + private data: Float32Array; + constructor(public length: number) { + this.data = new Float32Array(length); + } + getChannelData(): Float32Array { + return this.data; + } +} + class FakeCtx { sampleRate = 48000; created: FakeNode[] = []; @@ -67,6 +84,9 @@ class FakeCtx { createOscillator() { return this.make("osc"); } + createBufferSource() { + return this.make("bufferSource"); + } createWaveShaper() { return this.make("waveshaper"); } @@ -74,7 +94,7 @@ class FakeCtx { return this.make("convolver"); } createBuffer(_c: number, length: number) { - return { length, getChannelData: () => new Float32Array(length) }; + return new FakeBuffer(length); } } @@ -350,19 +370,76 @@ describe("levels and per-channel state", () => { }); it("sets the phaser LFO waveform it declares", () => { - const ctx = new FakeCtx(); - buildFxNode(ctx as unknown as BaseAudioContext, "phaser", { - ...defaultAudioFxParams("phaser"), - type: "0", - }); - const osc = ctx.created.find((n) => n.kind === "osc"); - expect(osc?.type).toBe("triangle"); - const ctx2 = new FakeCtx(); - buildFxNode(ctx2 as unknown as BaseAudioContext, "phaser", { - ...defaultAudioFxParams("phaser"), - type: "1", + // A quarter of the way through the cycle both waveforms peak at 1, so the + // eighth is where they part: a sine is at sin(π/4), a triangle halfway up. + const eighth = (c: FakeCtx): number => { + const buffer = c.created.find((n) => n.kind === "bufferSource")?.buffer; + if (!buffer) throw new Error("no LFO buffer"); + return buffer.getChannelData()[Math.round(buffer.length / 8)] ?? 0; + }; + const build = (type: string): FakeCtx => { + const c = new FakeCtx(); + buildFxNode(c as unknown as BaseAudioContext, "phaser", { + ...defaultAudioFxParams("phaser"), + type, + }); + return c; + }; + expect(eighth(build("0"))).toBeCloseTo(0.5, 3); + expect(eighth(build("1"))).toBeCloseTo(Math.SQRT1_2, 3); + }); + + /** + * The waveform is baked into a buffer at construction, so pushing a type change + * into the running graph would be a no-op — preview would keep sweeping on a + * triangle while the render used the sine the attribute now says. + */ + it("rebuilds a phaser when its LFO waveform changes", () => { + const phaser = (params: Record): HfAudioFxChain => ({ + version: 1, + nodes: [ + { + type: "phaser", + enabled: true, + params: { ...defaultAudioFxParams("phaser"), type: "0", ...params }, + }, + ], }); - expect(ctx2.created.find((n) => n.kind === "osc")?.type).toBe("sine"); + const built = buildFxChain(asCtx(ctx()), phaser({})); + expect(built.update(phaser({ type: "1" }))).toBe(false); + // Everything else about a phaser still updates in place. + const other = buildFxChain(asCtx(ctx()), phaser({})); + expect(other.update(phaser({ speed: 2 }))).toBe(true); + }); + + /** + * An LFO's phase is the whole reason it is a looping buffer rather than an + * OscillatorNode, whose phase is zero at `start()` and cannot be set. + * + * Preview rebuilds the graph mid-play — a seek, a scrub, any structural edit — + * and an oscillator restarted there put the chorus at the top of its sweep + * wherever the playhead happened to be, so preview disagreed with the render + * and with itself across an edit. + */ + it("starts a modulated effect's LFO at the phase the clip has reached", () => { + // 3.5 s at 2 Hz is seven whole cycles: back at phase zero. + const whole = ctx(); + buildFxNode(asCtx(whole), "chorus", { ...defaultAudioFxParams("chorus"), speed: 2 }, 3.5); + expect(whole.created.find((n) => n.kind === "bufferSource")?.startArgs?.[1]).toBeCloseTo(0, 6); + // 3.6 s at 2 Hz is seven cycles and a fifth. + const part = ctx(); + buildFxNode(asCtx(part), "chorus", { ...defaultAudioFxParams("chorus"), speed: 2 }, 3.6); + const src = part.created.find((n) => n.kind === "bufferSource"); + expect(src?.startArgs?.[1]).toBeCloseTo(0.2, 6); + expect(src?.loop).toBe(true); + // One second of waveform: the rate reads in Hz, so a speed lane needs no map. + expect(src?.playbackRate.value).toBeCloseTo(2, 6); + }); + + it("starts the LFO at zero for a render, which always begins at the clip's start", () => { + const c = ctx(); + buildFxNode(asCtx(c), "phaser", defaultAudioFxParams("phaser")); + expect(c.created.find((n) => n.kind === "bufferSource")?.startArgs?.[1]).toBe(0); }); it("rebuilds a one-pole filter when its cutoff moves", () => { diff --git a/packages/core/src/audio/audioFxGraph.ts b/packages/core/src/audio/audioFxGraph.ts index f37f394214..908f791568 100644 --- a/packages/core/src/audio/audioFxGraph.ts +++ b/packages/core/src/audio/audioFxGraph.ts @@ -83,13 +83,76 @@ export interface FxNodeHandle { dispose(): void; } -type Builder = (ctx: BaseAudioContext, p: HfAudioFxParamValues) => FxNodeHandle; +/** + * `elapsed` is the clip-relative time, in seconds, the graph is being built at. + * + * Zero for the render, which always starts a clip's audio from its first sample, + * and zero for a preview attached before playback. It is non-zero in the one case + * that used to be wrong: preview rebuilding the graph mid-play — a seek, a scrub, + * or any structural edit — where an LFO restarting from phase 0 made preview + * disagree with the render, and with itself across an edit. + */ +type Builder = (ctx: BaseAudioContext, p: HfAudioFxParamValues, elapsed: number) => FxNodeHandle; const n = (v: number | string | undefined): number => (typeof v === "number" ? v : Number(v ?? 0)); /** Milliseconds on the knob, seconds on the AudioParam. */ const msToSec = (v: number): number => v / 1000; +/** + * An LFO with a settable phase. + * + * An OscillatorNode cannot have one: its phase is zero at `start()`, and + * `start(when)` clamps a past `when` to now. So the modulator is one cycle of the + * waveform in a looping buffer instead, where `start(when, offset)` *is* a phase + * control. + * + * The buffer holds exactly one second, so it plays at 1 Hz at the default rate + * and `playbackRate` reads directly in Hz — which is what the `speed` knob is in, + * and what an automation lane aimed at it writes, so neither needs a mapping. + * + * Phase is taken as `elapsed × speed`, which is exact for the constant speed this + * is built with. A lane that sweeps `speed` advances the real phase by its + * integral, so a graph rebuilt mid-sweep resumes fractionally off — smaller than + * the whole-cycle error this replaces, and not worth integrating a curve for. + */ +function lfoSource( + ctx: BaseAudioContext, + wave: "sine" | "triangle", + speed: number, + elapsed: number, +): AudioBufferSourceNode { + const length = Math.max(1, Math.round(ctx.sampleRate)); + const buffer = ctx.createBuffer(1, length, ctx.sampleRate); + const cycle = buffer.getChannelData(0); + for (let i = 0; i < length; i++) { + const phase = i / length; + // Both start at zero and rise, the convention an OscillatorNode uses, so a + // render — which builds at elapsed 0 — is unmoved by this change. + cycle[i] = + wave === "sine" + ? Math.sin(2 * Math.PI * phase) + : 4 * Math.abs(((phase + 0.75) % 1) - 0.5) - 1; + } + const src = ctx.createBufferSource(); + src.buffer = buffer; + src.loop = true; + src.playbackRate.value = speed; + // A negative `offset` throws, and `elapsed` is only trusted to be a number. + const offset = ((((elapsed * speed) % 1) + 1) % 1) * (length / ctx.sampleRate); + src.start(typeof ctx.currentTime === "number" ? ctx.currentTime : 0, offset); + return src; +} + +/** Stop an LFO that may already have been stopped, on the way to disposal. */ +function stopLfo(src: AudioBufferSourceNode): void { + try { + src.stop(); + } catch { + /* already stopped */ + } +} + /** A wet/dry pair: the dry side is whatever the wet side is not. */ function mixTargets(wet: AudioParam, dry: AudioParam): FxParamTarget[] { return [{ param: wet }, { param: dry, map: (v) => 1 - v }]; @@ -271,22 +334,21 @@ const delayFeedback: Builder = (ctx, p) => { }; }; -const chorusLfo: Builder = (ctx, p) => { +const chorusLfo: Builder = (ctx, p, elapsed) => { const input = ctx.createGain(); const out = ctx.createGain(); const dl = ctx.createDelay(0.5); - const lfo = ctx.createOscillator(); + const lfo = lfoSource(ctx, "sine", n(p.speed), elapsed); const depth = ctx.createGain(); const wet = ctx.createGain(); const dry = ctx.createGain(); lfo.connect(depth).connect(dl.delayTime); input.connect(dl).connect(wet).connect(out); input.connect(dry).connect(out); - lfo.start(); const apply = (v: HfAudioFxParamValues): void => { dl.delayTime.value = n(v.delay) / 1000; depth.gain.value = n(v.depth) / 1000; - lfo.frequency.value = n(v.speed); + lfo.playbackRate.value = n(v.speed); wet.gain.value = n(v.mix); dry.gain.value = 1 - n(v.mix); }; @@ -298,15 +360,12 @@ const chorusLfo: Builder = (ctx, p) => { automation: { delay: [{ param: dl.delayTime, map: msToSec }], depth: [{ param: depth.gain, map: msToSec }], - speed: [{ param: lfo.frequency }], + // One second of waveform, so the rate is the frequency in Hz the knob names. + speed: [{ param: lfo.playbackRate }], mix: mixTargets(wet.gain, dry.gain), }, dispose: () => { - try { - lfo.stop(); - } catch { - /* already stopped */ - } + stopLfo(lfo); [input, out, dl, depth, wet, dry].forEach((x) => x.disconnect()); }, }; @@ -314,7 +373,7 @@ const chorusLfo: Builder = (ctx, p) => { const PHASER_STAGES = 6; -const allpassPhaser: Builder = (ctx, p) => { +const allpassPhaser: Builder = (ctx, p, elapsed) => { const input = ctx.createGain(); const out = ctx.createGain(); // aphaser's in_gain/out_gain trim the signal entering and leaving the effect. @@ -323,7 +382,12 @@ const allpassPhaser: Builder = (ctx, p) => { // track level. const inTrim = ctx.createGain(); const outTrim = ctx.createGain(); - const lfo = ctx.createOscillator(); + // aphaser's type 0 is triangular, 1 sinusoidal. The builder once left this + // unset, so the declared default ("Triangular") was silently a sine. The + // waveform is baked into the LFO's buffer, so switching it is a shape change + // that rebuilds the chain rather than a value pushed into the running graph — + // see `shapeOf`. + const lfo = lfoSource(ctx, String(p.type) === "1" ? "sine" : "triangle", n(p.speed), elapsed); const depth = ctx.createGain(); const wet = ctx.createGain(); const dry = ctx.createGain(); @@ -340,11 +404,6 @@ const allpassPhaser: Builder = (ctx, p) => { stages.push(ap); } lfo.connect(depth); - // aphaser's type 0 is triangular, 1 sinusoidal. The builder never set this, so - // the declared default ("Triangular") was silently a sine. An OscillatorNode - // has no triangle-with-the-same-phase primitive to switch to, so triangle is - // the node's own "triangle" type. - lfo.start(); node.connect(wet).connect(outTrim); inTrim.connect(dry).connect(outTrim); outTrim.connect(out); @@ -354,8 +413,7 @@ const allpassPhaser: Builder = (ctx, p) => { const centre = 1000 / Math.max(0.1, n(v.delay)); for (const ap of stages) ap.frequency.value = centre; depth.gain.value = centre * n(v.decay); - lfo.frequency.value = n(v.speed); - lfo.type = String(v.type) === "1" ? "sine" : "triangle"; + lfo.playbackRate.value = n(v.speed); inTrim.gain.value = n(v.in_gain); outTrim.gain.value = n(v.out_gain); // Summed at unity: the sweep is the effect, not a blend control. @@ -370,7 +428,7 @@ const allpassPhaser: Builder = (ctx, p) => { // `delay` and `decay` set the sweep centre, which feeds every stage's // frequency at once — not one knob, one param — so they stay unautomated. automation: { - speed: [{ param: lfo.frequency }], + speed: [{ param: lfo.playbackRate }], // The trims, not wet/dry. apply() drives inTrim/outTrim from these knobs // and pins wet and dry to 1 — so a lane aimed at wet/dry modulated a // constant and left the trim frozen, and the next values-only edit slammed @@ -380,11 +438,7 @@ const allpassPhaser: Builder = (ctx, p) => { out_gain: [{ param: outTrim.gain }], }, dispose: () => { - try { - lfo.stop(); - } catch { - /* already stopped */ - } + stopLfo(lfo); [input, out, inTrim, outTrim, depth, wet, dry, ...stages].forEach((x) => x.disconnect()); }, }; @@ -452,17 +506,18 @@ export function buildFxNode( ctx: BaseAudioContext, type: string, params: HfAudioFxParamValues, + elapsed = 0, ): FxNodeHandle { const def = getAudioFxDef(type); if (!def) throw new Error(`Unknown effect type: ${type}`); const resolved = normalizeAudioFxParams(type, params); // One-pole is a different node type, not a different parameter value. if ((type === "highpass" || type === "lowpass") && String(resolved.poles) === "1") { - return onePoleBuilder(type)(ctx, resolved); + return onePoleBuilder(type)(ctx, resolved, elapsed); } const builder = BUILDERS[def.web]; if (!builder) throw new Error(`No Web Audio builder for ${def.web}`); - return builder(ctx, resolved); + return builder(ctx, resolved, elapsed); } export interface FxChainHandle { @@ -491,7 +546,11 @@ function shapeOf(chain: HfAudioFxChain): string { // being pushed into a no-op updater — which is what let preview keep // filtering at the old frequency while the render used the new one. const fixedFreq = String(p.poles) === "1" ? `@${p.frequency}` : ""; - return `${node.type}${poles}${fixedFreq}`; + // The phaser's LFO waveform is baked into a buffer at construction, for the + // same reason: pushed into the running graph it would be a no-op, and + // preview would keep sweeping on a triangle while the render used a sine. + const wave = node.type === "phaser" ? `~${p.type}` : ""; + return `${node.type}${poles}${fixedFreq}${wave}`; }) .join("|"); } @@ -499,15 +558,23 @@ function shapeOf(chain: HfAudioFxChain): string { /** * Build the whole chain in series. Returns a handle whose `input`/`output` can * be spliced into any graph; an empty chain yields a pass-through. + * + * `elapsed` is where in the clip this is being built — see `Builder`. It only + * reaches the modulated effects, and only matters when the graph is built after + * the audio has already started. */ -export function buildFxChain(ctx: BaseAudioContext, chain: HfAudioFxChain): FxChainHandle { +export function buildFxChain( + ctx: BaseAudioContext, + chain: HfAudioFxChain, + elapsed = 0, +): FxChainHandle { const input = ctx.createGain(); const output = ctx.createGain(); const handles: { id?: string; type: string; handle: FxNodeHandle }[] = []; let tail: AudioNode = input; for (const node of enabledAudioFxNodes(chain)) { - const handle = buildFxNode(ctx, node.type, node.params ?? {}); + const handle = buildFxNode(ctx, node.type, node.params ?? {}, elapsed); tail.connect(handle.input); tail = handle.output; handles.push({ ...(node.id ? { id: node.id } : {}), type: node.type, handle }); diff --git a/packages/core/src/runtime/audioFx.test.ts b/packages/core/src/runtime/audioFx.test.ts index 09374e12e4..35db1e291d 100644 --- a/packages/core/src/runtime/audioFx.test.ts +++ b/packages/core/src/runtime/audioFx.test.ts @@ -16,6 +16,10 @@ class Node { Q = { value: 0 }; gain = { value: 0 }; delayTime = { value: 0 }; + playbackRate = { value: 0 }; + loop = false; + /** `start(when, offset)` — the offset is an LFO's phase, so it is asserted. */ + startArgs: (number | undefined)[] | null = null; type = ""; curve: Float32Array | null = null; oversample = "none"; @@ -28,7 +32,9 @@ class Node { disconnect(): void { this.disconnected = true; } - start(): void {} + start(...args: (number | undefined)[]): void { + this.startArgs = args; + } stop(): void {} } class Ctx { @@ -48,6 +54,9 @@ class Ctx { createOscillator() { return new Node(); } + createBufferSource() { + return new Node(); + } createWaveShaper() { return new Node(); } @@ -304,6 +313,67 @@ describe("attachElementFxChain", () => { expect(node.getAttribute("data-fx-chain")).toContain("1200"); }); + /** + * A rebuild hands the graph the clip position it happens at, so an LFO + * resumes at the phase the render would be at rather than restarting. + * + * Without it, every structural edit — and every seek, which rebuilds the same + * way — put a chorus back at the top of its sweep wherever the playhead was: + * preview disagreeing with the render, and with itself across an edit. + */ + it("hands a rebuilt graph the playhead it happens at", async () => { + const clock = { currentTime: 0 }; + class ClockCtx extends Ctx { + made: Node[] = []; + get currentTime(): number { + return clock.currentTime; + } + override createBufferSource(): Node { + const node = new Node(); + this.made.push(node); + return node; + } + } + const withChorus = (mix: number) => ({ + version: 1, + nodes: [ + { + type: "chorus", + id: "n1", + params: { ...defaultAudioFxParams("chorus"), speed: 2, mix }, + }, + ], + }); + const ctxClock = new ClockCtx(); + const node = audioEl(withChorus(0.5)); + attachElementFxChain( + ctxClock as unknown as BaseAudioContext, + node, + new Node() as never, + new Node() as never, + { scheduledAt: 0, elapsed: 0, rate: 1 }, + ); + // Attached at the clip's start: phase zero, the same as a render. + expect(ctxClock.made[0]?.startArgs?.[1]).toBeCloseTo(0, 6); + + // 3.6 s later, and structural — a bypass, so the shape changes and the + // graph is rebuilt rather than re-parameterised. + clock.currentTime = 3.6; + node.setAttribute( + "data-fx-chain", + JSON.stringify({ + version: 1, + nodes: [ + ...withChorus(0.5).nodes, + { type: "peaking", id: "n2", params: defaultAudioFxParams("peaking") }, + ], + }), + ); + await settle(); + // 3.6 s at 2 Hz is seven cycles and a fifth. + expect(ctxClock.made.at(-1)?.startArgs?.[1]).toBeCloseTo(0.2, 6); + }); + it("keeps playing dry when an edit leaves the chain unreadable", async () => { const src = new Node(); const dst = new Node(); @@ -689,6 +759,9 @@ describe("attachElementFxChain", () => { override Q = new RecordingParam() as unknown as { value: number }; override gain = new RecordingParam() as unknown as { value: number }; override delayTime = new RecordingParam() as unknown as { value: number }; + // A modulated effect's `speed` lane drives its LFO's rate, which is where a + // looping buffer keeps the frequency an oscillator kept on `frequency`. + override playbackRate = new RecordingParam() as unknown as { value: number }; port = { postMessage: () => {} }; } class RichCtx extends Ctx { @@ -720,6 +793,9 @@ describe("attachElementFxChain", () => { override createOscillator() { return this.make(); } + override createBufferSource() { + return this.make(); + } override createWaveShaper() { return this.make(); } @@ -790,7 +866,7 @@ describe("attachElementFxChain", () => { expect(src.connections.at(-1), `${def.id} was left out of the path`).not.toBe(dst); if (automatable) { const scheduled = ctxRich.made.some((n) => - [n.frequency, n.Q, n.gain, n.delayTime].some( + [n.frequency, n.Q, n.gain, n.delayTime, n.playbackRate].some( (p) => (p as unknown as RecordingParam).scheduled, ), ); diff --git a/packages/core/src/runtime/audioFx.ts b/packages/core/src/runtime/audioFx.ts index cec709e7a8..22de1dc2e1 100644 --- a/packages/core/src/runtime/audioFx.ts +++ b/packages/core/src/runtime/audioFx.ts @@ -138,7 +138,7 @@ export function attachElementFxChain( * A chain that cannot be realised — an unregistered worklet, an unknown * effect — plays dry rather than silencing the track. */ - const attach = (next: HfAudioFxChain): void => { + const attach = (next: HfAudioFxChain, elapsed: number): void => { if (next.nodes.length === 0) { source.connect(destination); return; @@ -166,7 +166,9 @@ export function attachElementFxChain( return; } try { - const built = buildFxChain(ctx, next); + // Where the clip has got to, so a modulated effect resumes at the phase the + // render would be at rather than restarting its LFO from zero. + const built = buildFxChain(ctx, next, elapsed); source.connect(built.input); built.output.connect(destination); handle = built; @@ -186,7 +188,9 @@ export function attachElementFxChain( // envelope at the wrong clip position. let frame: AutomationTiming | null = timing ? { ...timing } : null; - attach(chain); + // `timingNow` is not in scope yet, and does not need to be: nothing has played + // between the frame being taken and this line. + attach(chain, frame?.elapsed ?? 0); scheduleFor(chain, frame); /** @@ -223,7 +227,7 @@ export function attachElementFxChain( const at = timingNow(); cancelParamLane(automated, at?.scheduledAt ?? 0); detach(); - attach(next); + attach(next, at?.elapsed ?? 0); scheduleFor(next, at); }; From 601d30e5699d200ec4e89457ccae6fd7ff5b29e1 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 9 Aug 2026 23:51:32 -0700 Subject: [PATCH 4/6] fix(core): unwire an LFO when its effect is disposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The §4c item here was "four hand-rolled wet/dry shells whose dispose lists have already drifted". Reading them, the drift is one specific thing and it is a leak, not an untidiness: the chorus and the phaser stopped their LFO and left it out of the nodes they disconnect. So every chain rebuild that dropped a modulated effect left a modulator still wired to the delay or the allpass bank it had been driving. Nothing audible came of it — the shell around it was disconnected — but the nodes stayed reachable, and a session of edits to a modulated track piled them up. Same shape as the worklet leak fixed earlier on this stack. `stopLfo` becomes `retireLfo` and does both halves, which is the whole fix. The consolidation the item also asks for is not here: the four shells are wired differently enough (a feedback loop, a modulated delay, six allpass stages between two trims, a convolver) that one factory over them would need a config surface bigger than the four dispose lines it replaces. The leak was the part with a defect behind it. Also folds the two artifact build scripts together — 50 lines each, differing in five names. That one is a genuine copy, and the copy is where a divergence would hide: whichever stopped being edited would go on producing a subtly different artifact with nothing to say so. Output is byte-identical, which `check:position-edits-render` proves by diffing the tracked artifact. Falsified: dropping the `disconnect` fails the new test. core 1726 passing (110 files), engine services 736 passing / 3 skipped. --- .../core/scripts/build-audio-fx-runtime.ts | 57 +++----------- .../scripts/build-position-edits-render.ts | 59 +++----------- .../core/scripts/buildInjectedArtifact.ts | 78 +++++++++++++++++++ packages/core/src/audio/audioFxGraph.test.ts | 15 ++++ packages/core/src/audio/audioFxGraph.ts | 18 ++++- 5 files changed, 127 insertions(+), 100 deletions(-) create mode 100644 packages/core/scripts/buildInjectedArtifact.ts diff --git a/packages/core/scripts/build-audio-fx-runtime.ts b/packages/core/scripts/build-audio-fx-runtime.ts index eb6c841a51..29d74d6e7c 100644 --- a/packages/core/scripts/build-audio-fx-runtime.ts +++ b/packages/core/scripts/build-audio-fx-runtime.ts @@ -1,50 +1,13 @@ /** Build the injectable audio-FX runtime artifact from the canonical runtime. */ -import { mkdirSync, writeFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; -import { buildSync } from "esbuild"; -import { execFileSync } from "node:child_process"; - -const thisDir = dirname(fileURLToPath(import.meta.url)); -const repoRoot = resolve(thisDir, ".."); -const entry = resolve(repoRoot, "stubs/audio-fx-runtime-entry.ts"); -const generatedDir = resolve(repoRoot, "src/generated"); -const outPath = resolve(generatedDir, "audio-fx-runtime-inline.ts"); - -const result = buildSync({ - entryPoints: [entry], - bundle: true, - write: false, - platform: "browser", - format: "iife", - target: ["es2020"], - minify: true, - legalComments: "none", +import { buildInjectedArtifact } from "./buildInjectedArtifact.js"; + +buildInjectedArtifact({ + scriptUrl: import.meta.url, + entry: "stubs/audio-fx-runtime-entry.ts", + out: "audio-fx-runtime-inline.ts", + constName: "AUDIO_FX_RUNTIME_IIFE", + fnName: "getAudioFxRuntimeScript", + what: "audio-FX runtime IIFE", + event: "audio_fx_runtime_generated", }); -const iife = result.outputFiles[0]?.text ?? ""; -if (!iife) throw new Error("esbuild produced no output for audio-fx-runtime-entry.ts"); - -mkdirSync(generatedDir, { recursive: true }); -writeFileSync( - outPath, - [ - "// AUTO-GENERATED by scripts/build-audio-fx-runtime.ts - do not edit", - `const AUDIO_FX_RUNTIME_IIFE: string = ${JSON.stringify(iife)};`, - "", - "/** Returns the pre-built audio-FX runtime IIFE as a string constant. */", - "export function getAudioFxRuntimeScript(): string {", - " return AUDIO_FX_RUNTIME_IIFE;", - "}", - "", - ].join("\n"), - "utf8", -); - -try { - execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" }); -} catch { - // Formatting is best effort when the generator runs in a minimal environment. -} - -console.log(JSON.stringify({ event: "audio_fx_runtime_generated", outPath, bytes: iife.length })); diff --git a/packages/core/scripts/build-position-edits-render.ts b/packages/core/scripts/build-position-edits-render.ts index e4ed0ef981..a750cd791e 100644 --- a/packages/core/scripts/build-position-edits-render.ts +++ b/packages/core/scripts/build-position-edits-render.ts @@ -1,52 +1,13 @@ /** Build the injectable position-edits render artifact from the canonical runtime. */ -import { mkdirSync, writeFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; -import { buildSync } from "esbuild"; -import { execFileSync } from "node:child_process"; - -const thisDir = dirname(fileURLToPath(import.meta.url)); -const repoRoot = resolve(thisDir, ".."); -const entry = resolve(repoRoot, "stubs/position-edits-render-entry.ts"); -const generatedDir = resolve(repoRoot, "src/generated"); -const outPath = resolve(generatedDir, "position-edits-render-inline.ts"); - -const result = buildSync({ - entryPoints: [entry], - bundle: true, - write: false, - platform: "browser", - format: "iife", - target: ["es2020"], - minify: true, - legalComments: "none", +import { buildInjectedArtifact } from "./buildInjectedArtifact.js"; + +buildInjectedArtifact({ + scriptUrl: import.meta.url, + entry: "stubs/position-edits-render-entry.ts", + out: "position-edits-render-inline.ts", + constName: "POSITION_EDITS_RENDER_IIFE", + fnName: "getPositionEditsRenderScript", + what: "position-edits render IIFE", + event: "position_edits_render_generated", }); -const iife = result.outputFiles[0]?.text ?? ""; -if (!iife) throw new Error("esbuild produced no output for position-edits-render-entry.ts"); - -mkdirSync(generatedDir, { recursive: true }); -writeFileSync( - outPath, - [ - "// AUTO-GENERATED by scripts/build-position-edits-render.ts - do not edit", - `const POSITION_EDITS_RENDER_IIFE: string = ${JSON.stringify(iife)};`, - "", - "/** Returns the pre-built position-edits render IIFE as a string constant. */", - "export function getPositionEditsRenderScript(): string {", - " return POSITION_EDITS_RENDER_IIFE;", - "}", - "", - ].join("\n"), - "utf8", -); - -try { - execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" }); -} catch { - // Formatting is best effort when the generator runs in a minimal environment. -} - -console.log( - JSON.stringify({ event: "position_edits_render_generated", outPath, bytes: iife.length }), -); diff --git a/packages/core/scripts/buildInjectedArtifact.ts b/packages/core/scripts/buildInjectedArtifact.ts new file mode 100644 index 0000000000..b7faa7d0f7 --- /dev/null +++ b/packages/core/scripts/buildInjectedArtifact.ts @@ -0,0 +1,78 @@ +/** + * Bundle a stub entry into an injectable IIFE, wrapped as a TypeScript constant. + * + * Two artifacts are built this way — the audio-FX runtime and the position-edits + * render — and the engine injects both into the headless browser as a script tag. + * They were two copies of this file differing in five names, which is a poor + * place for a divergence to hide: whichever copy stopped being edited would go on + * producing a subtly different artifact with nothing to say so. + */ + +import { execFileSync } from "node:child_process"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { buildSync } from "esbuild"; + +export interface InjectedArtifact { + /** The build script's own `import.meta.url`, so paths resolve beside it. */ + scriptUrl: string; + /** Entry stub, relative to the package root. */ + entry: string; + /** Output file name inside `src/generated`. */ + out: string; + /** SCREAMING_CASE name for the string constant holding the IIFE. */ + constName: string; + /** The accessor the rest of the codebase imports. */ + fnName: string; + /** What that accessor returns, for its doc comment: "the pre-built X". */ + what: string; + /** Structured log event name. */ + event: string; +} + +export function buildInjectedArtifact(spec: InjectedArtifact): void { + const scriptDir = dirname(fileURLToPath(spec.scriptUrl)); + const scriptName = spec.scriptUrl.split("/").pop() ?? ""; + const repoRoot = resolve(scriptDir, ".."); + const entry = resolve(repoRoot, spec.entry); + const generatedDir = resolve(repoRoot, "src/generated"); + const outPath = resolve(generatedDir, spec.out); + + const result = buildSync({ + entryPoints: [entry], + bundle: true, + write: false, + platform: "browser", + format: "iife", + target: ["es2020"], + minify: true, + legalComments: "none", + }); + const iife = result.outputFiles[0]?.text ?? ""; + if (!iife) throw new Error(`esbuild produced no output for ${spec.entry.split("/").pop()}`); + + mkdirSync(generatedDir, { recursive: true }); + writeFileSync( + outPath, + [ + `// AUTO-GENERATED by scripts/${scriptName} - do not edit`, + `const ${spec.constName}: string = ${JSON.stringify(iife)};`, + "", + `/** Returns the pre-built ${spec.what} as a string constant. */`, + `export function ${spec.fnName}(): string {`, + ` return ${spec.constName};`, + "}", + "", + ].join("\n"), + "utf8", + ); + + try { + execFileSync("bun", ["x", "oxfmt", outPath], { stdio: "ignore" }); + } catch { + // Formatting is best effort when the generator runs in a minimal environment. + } + + console.log(JSON.stringify({ event: spec.event, outPath, bytes: iife.length })); +} diff --git a/packages/core/src/audio/audioFxGraph.test.ts b/packages/core/src/audio/audioFxGraph.test.ts index d0b4f12ec2..181bf08269 100644 --- a/packages/core/src/audio/audioFxGraph.test.ts +++ b/packages/core/src/audio/audioFxGraph.test.ts @@ -436,6 +436,21 @@ describe("levels and per-channel state", () => { expect(src?.playbackRate.value).toBeCloseTo(2, 6); }); + /** + * A source node is not retired by disconnecting what it feeds. The chorus and + * phaser stopped their LFO and left it out of the nodes they disconnect, so + * every rebuild that dropped one left a modulator still wired to the delay or + * the allpass bank it had been driving. + */ + it("unwires a modulated effect's LFO when the effect is disposed", () => { + for (const type of ["chorus", "phaser"]) { + const c = ctx(); + buildFxNode(asCtx(c), type, defaultAudioFxParams(type)).dispose(); + const lfo = c.created.find((node) => node.kind === "bufferSource"); + expect(lfo?.disconnected, `${type} left its LFO connected`).toBe(true); + } + }); + it("starts the LFO at zero for a render, which always begins at the clip's start", () => { const c = ctx(); buildFxNode(asCtx(c), "phaser", defaultAudioFxParams("phaser")); diff --git a/packages/core/src/audio/audioFxGraph.ts b/packages/core/src/audio/audioFxGraph.ts index 908f791568..c49db5e468 100644 --- a/packages/core/src/audio/audioFxGraph.ts +++ b/packages/core/src/audio/audioFxGraph.ts @@ -144,13 +144,23 @@ function lfoSource( return src; } -/** Stop an LFO that may already have been stopped, on the way to disposal. */ -function stopLfo(src: AudioBufferSourceNode): void { +/** + * Retire an LFO: stopped *and* unwired. + * + * Both halves. The old oscillators were stopped and left in their builder's + * dispose list — so every chain rebuild that dropped a chorus or a phaser left a + * modulator still connected to the delay or the allpass bank it had been + * driving. Nothing audible came out of it, because the shell around it was + * disconnected, but the nodes stayed reachable and a session of edits to a + * modulated track accumulated them. Same shape as the worklet leak above. + */ +function retireLfo(src: AudioBufferSourceNode): void { try { src.stop(); } catch { /* already stopped */ } + src.disconnect(); } /** A wet/dry pair: the dry side is whatever the wet side is not. */ @@ -365,7 +375,7 @@ const chorusLfo: Builder = (ctx, p, elapsed) => { mix: mixTargets(wet.gain, dry.gain), }, dispose: () => { - stopLfo(lfo); + retireLfo(lfo); [input, out, dl, depth, wet, dry].forEach((x) => x.disconnect()); }, }; @@ -438,7 +448,7 @@ const allpassPhaser: Builder = (ctx, p, elapsed) => { out_gain: [{ param: outTrim.gain }], }, dispose: () => { - stopLfo(lfo); + retireLfo(lfo); [input, out, inTrim, outTrim, depth, wet, dry, ...stages].forEach((x) => x.disconnect()); }, }; From a0e3a75d0b97f9853f6609020fb0a4ed62c2eb73 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 9 Aug 2026 23:55:27 -0700 Subject: [PATCH 5/6] refactor(engine): one RIFF chunk walk, not two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `readWavChunks` and `parseWavLayout` each carried the same word-aligned walk. The reason this was left alone before is real — the two are *not* the same function, and unifying them means picking one behaviour for each of four differences in the parser every render's audio goes through: one breaks at the first `data` and returns a slice, the other scans every chunk and returns offsets; one lets the decoder judge the format, the other refuses anything but 16-bit PCM. So only the walk moves. `riffChunks` yields `{ id, body, size }` and holds no policy at all, both readers keep every one of those four behaviours, and there is no cycle to route around because it belongs to neither of them. It lives in engine/services beside both. It also had no test, in either copy. A real WAV out of the mixer has an even-sized `fmt ` first and `data` last, so neither of the two things the walk exists for ever came up: the pad byte after an odd chunk, and not assuming an ordering. Both mutations survived a full engine run before `wavChunks.test.ts`; both fail now, as does removing the guard against a header that runs past a truncated file. engine services 739 passing / 3 skipped (736 + the 3 new). --- packages/engine/src/services/audioFxRender.ts | 21 +++---- .../src/services/audioVolumeEnvelope.ts | 15 ++--- .../engine/src/services/wavChunks.test.ts | 56 +++++++++++++++++++ packages/engine/src/services/wavChunks.ts | 38 +++++++++++++ 4 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 packages/engine/src/services/wavChunks.test.ts create mode 100644 packages/engine/src/services/wavChunks.ts diff --git a/packages/engine/src/services/audioFxRender.ts b/packages/engine/src/services/audioFxRender.ts index bca39030b4..b306d210ce 100644 --- a/packages/engine/src/services/audioFxRender.ts +++ b/packages/engine/src/services/audioFxRender.ts @@ -21,6 +21,7 @@ import { enabledAudioFxNodes, type HfAudioFxChain } from "@hyperframes/core/audi import { serializeAutomation, type HfAutomation } from "@hyperframes/core/audio-automation"; import { acquireBrowser } from "./browserManager.js"; import { createEnvelopeWalker } from "./audioVolumeEnvelope.js"; +import { riffChunks } from "./wavChunks.js"; import type { AudioVolumeKeyframe } from "./audioMixer.types.js"; export class AudioFxRenderError extends Error { @@ -49,22 +50,22 @@ function readWavChunks(buf: Buffer): { bits: number; data?: Buffer; } { - let offset = 12; const head = { format: 1, channels: 1, sampleRate: 48000, bits: 16 }; let data: Buffer | undefined; - while (offset + 8 <= buf.length) { - const id = buf.toString("ascii", offset, offset + 4); - const size = buf.readUInt32LE(offset + 4); + for (const { id, body, size } of riffChunks(buf)) { if (id === "fmt ") { - head.format = buf.readUInt16LE(offset + 8); - head.channels = buf.readUInt16LE(offset + 10); - head.sampleRate = buf.readUInt32LE(offset + 12); - head.bits = buf.readUInt16LE(offset + 22); + head.format = buf.readUInt16LE(body); + head.channels = buf.readUInt16LE(body + 2); + head.sampleRate = buf.readUInt32LE(body + 4); + head.bits = buf.readUInt16LE(body + 14); } else if (id === "data") { - data = buf.subarray(offset + 8, Math.min(buf.length, offset + 8 + size)); + data = buf.subarray(body, Math.min(buf.length, body + size)); + // The payload is the rest of the file for anything the mixer writes, and + // reading past it buys nothing: `fmt ` precedes `data` in every WAV these + // steps produce, and the alternative is walking a several-hundred-megabyte + // tail chunk by chunk. break; } - offset += 8 + size + (size % 2); } return { ...head, data }; } diff --git a/packages/engine/src/services/audioVolumeEnvelope.ts b/packages/engine/src/services/audioVolumeEnvelope.ts index 08f8a18828..d64cc33a4c 100644 --- a/packages/engine/src/services/audioVolumeEnvelope.ts +++ b/packages/engine/src/services/audioVolumeEnvelope.ts @@ -19,6 +19,7 @@ import { readFileSync, renameSync, writeFileSync } from "fs"; import { randomBytes } from "crypto"; import type { AudioVolumeKeyframe } from "./audioMixer.types.js"; import { normaliseEnvelope } from "@hyperframes/core/media-volume-envelope"; +import { riffChunks } from "./wavChunks.js"; const PCM_FORMAT = 1; // WAVE_FORMAT_PCM const SUPPORTED_BITS = 16; @@ -42,26 +43,20 @@ function parseWavLayout(buffer: Buffer): WavLayout | null { if (buffer.length < 12 || buffer.toString("ascii", 0, 4) !== "RIFF") return null; if (buffer.toString("ascii", 8, 12) !== "WAVE") return null; - let offset = 12; let fmt: { numChannels: number; sampleRate: number; bitsPerSample: number } | null = null; let data: { offset: number; size: number } | null = null; - while (offset + 8 <= buffer.length) { - const chunkId = buffer.toString("ascii", offset, offset + 4); - const chunkSize = buffer.readUInt32LE(offset + 4); - const body = offset + 8; - if (chunkId === "fmt " && body + 16 <= buffer.length) { + for (const { id, body, size } of riffChunks(buffer)) { + if (id === "fmt " && body + 16 <= buffer.length) { if (buffer.readUInt16LE(body) !== PCM_FORMAT) return null; fmt = { numChannels: buffer.readUInt16LE(body + 2), sampleRate: buffer.readUInt32LE(body + 4), bitsPerSample: buffer.readUInt16LE(body + 14), }; - } else if (chunkId === "data") { - data = { offset: body, size: Math.min(chunkSize, buffer.length - body) }; + } else if (id === "data") { + data = { offset: body, size: Math.min(size, buffer.length - body) }; } - // Chunks are word-aligned: an odd size carries a trailing pad byte. - offset = body + chunkSize + (chunkSize % 2); } if (!fmt || !data) return null; diff --git a/packages/engine/src/services/wavChunks.test.ts b/packages/engine/src/services/wavChunks.test.ts new file mode 100644 index 0000000000..6ee943203a --- /dev/null +++ b/packages/engine/src/services/wavChunks.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { riffChunks } from "./wavChunks.js"; + +/** + * The two WAV readers that share this walk both had their own copy, and neither + * had a test for the walk itself — a real WAV out of the mixer has an even-sized + * `fmt ` first and `data` last, so the two things the walk exists to handle + * (ordering, and the pad byte after an odd chunk) never came up in either suite. + */ +function riff(chunks: { id: string; body: Buffer }[]): Buffer { + const parts: Buffer[] = [Buffer.from("RIFF"), Buffer.alloc(4), Buffer.from("WAVE")]; + for (const { id, body } of chunks) { + const header = Buffer.alloc(8); + header.write(id, 0, "ascii"); + header.writeUInt32LE(body.length, 4); + parts.push(header, body); + // Word alignment: an odd body is followed by a pad byte the size excludes. + if (body.length % 2) parts.push(Buffer.alloc(1)); + } + const buf = Buffer.concat(parts); + buf.writeUInt32LE(buf.length - 8, 4); + return buf; +} + +describe("riffChunks", () => { + it("steps over the pad byte after an odd-sized chunk", () => { + // An odd LIST is what ffmpeg writes for a metadata string of odd length. Read + // without the pad, every chunk after it is one byte out and reads as garbage. + const buf = riff([ + { id: "LIST", body: Buffer.from("INFOodd") }, + { id: "data", body: Buffer.from([1, 2, 3, 4]) }, + ]); + const found = [...riffChunks(buf)]; + expect(found.map((c) => c.id)).toEqual(["LIST", "data"]); + const data = found[1]; + if (!data) throw new Error("no data chunk"); + expect(buf.subarray(data.body, data.body + data.size)).toEqual(Buffer.from([1, 2, 3, 4])); + }); + + it("yields chunks in file order, whatever that order is", () => { + // `data` before `fmt ` is legal and the reason the walk advances by declared + // size rather than assuming a layout. + const buf = riff([ + { id: "data", body: Buffer.alloc(6) }, + { id: "fmt ", body: Buffer.alloc(16) }, + ]); + expect([...riffChunks(buf)].map((c) => c.id)).toEqual(["data", "fmt "]); + }); + + it("stops at a chunk header that runs past the end of the file", () => { + // Truncated downloads and interrupted writes both land here; the walk must + // end rather than read off the buffer. + const buf = Buffer.concat([riff([{ id: "data", body: Buffer.alloc(4) }]), Buffer.from("da")]); + expect([...riffChunks(buf)].map((c) => c.id)).toEqual(["data"]); + }); +}); diff --git a/packages/engine/src/services/wavChunks.ts b/packages/engine/src/services/wavChunks.ts new file mode 100644 index 0000000000..9265141386 --- /dev/null +++ b/packages/engine/src/services/wavChunks.ts @@ -0,0 +1,38 @@ +/** + * The RIFF chunk walk, which two WAV readers in this directory each had a copy + * of: `audioFxRender`'s `readWavChunks` and `audioVolumeEnvelope`'s + * `parseWavLayout`. + * + * Only the walk is shared. What the two do with the chunks is genuinely + * different — one wants a slice of the payload and lets the decoder judge the + * format, the other wants offsets to edit in place and refuses anything that is + * not 16-bit PCM — and folding those together would mean picking one behaviour + * for each difference, in the parser every render's audio passes through. So + * this yields chunks and holds no policy at all. + */ + +export interface RiffChunk { + /** Four ASCII characters: `fmt `, `data`, `LIST`, `fact`, … */ + id: string; + /** Byte offset of the chunk's body, past the 8-byte header. */ + body: number; + /** The size the chunk declares. May run past the end of a truncated file. */ + size: number; +} + +/** + * Every chunk after the 12-byte RIFF header, in the order they sit. + * + * Advances by each chunk's declared size, so ordering is not assumed — `data` + * may precede `fmt `, and trailing LIST/fact chunks are walked past rather than + * tripped over. Chunks are word-aligned, so an odd size carries a pad byte. + */ +export function* riffChunks(buffer: Buffer): Generator { + let offset = 12; + while (offset + 8 <= buffer.length) { + const id = buffer.toString("ascii", offset, offset + 4); + const size = buffer.readUInt32LE(offset + 4); + yield { id, body: offset + 8, size }; + offset += 8 + size + (size % 2); + } +} From aa53c6d8bc42838271e64d53606bd6d5755b8d61 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 10 Aug 2026 00:01:52 -0700 Subject: [PATCH 6/6] feat(core): land the plain-language layer, and test that it covers the rack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `copy.mts` lived in `plans/` and was read by one build script. Moving it to `packages/core/src/audioFxCopy.ts` puts it beside the registry it describes, and turns the coverage into `audioFxCopy.test.ts` — every shipped effect, every one of its parameters, and every preset has to have copy. That check existed before as a step in `build-preview.mts`, which means it only fired when somebody remembered to rebuild the review page. Now it fires on the commit that adds an effect without a plain name for it, which is the only moment it can still be cheap to fix. Four more assertions the build step never made, each of which was a real hole: copy for an effect the registry no longer ships (dead text that reads as coverage), a `SUMMARY` missing for an effect that has one everywhere else, a summary that renders `undefined` or `NaN` at the effect's own defaults — which is the first thing an author reads after adding one — and a gap or overlap in the shared frequency ruler, which would be a band the rack can name in one module and not in another. `PROFILES` deliberately did NOT come along. Its figures are proposed, not measured, nothing derives from them yet, and they want the same before/after listen the clip-before-duck fix got before a knob is wired to them. So it stays in `plans/audio-fx-ux/copy.mts`, which is now all that file holds, and `build-preview.mts` imports the shipped four from core and that one from beside itself. Landing the data is not wiring it: nothing in the studio reads this yet, and it should not until the three open UX questions are settled — whether the plain name replaces the DSP name or sits beside it decides what the rack renders. The README says so where the status used to say the layer had not landed. Falsified: deleting one parameter's entry fails the highpass case. core 1745 passing (111 files), studio unchanged at 3674 / 18 todo. --- packages/core/package-subpaths.json | 6 + packages/core/package.json | 10 + packages/core/src/audioFxCopy.test.ts | 75 ++++++ packages/core/src/audioFxCopy.ts | 375 ++++++++++++++++++++++++++ plans/audio-fx-ux/README.md | 38 +-- 5 files changed, 488 insertions(+), 16 deletions(-) create mode 100644 packages/core/src/audioFxCopy.test.ts create mode 100644 packages/core/src/audioFxCopy.ts diff --git a/packages/core/package-subpaths.json b/packages/core/package-subpaths.json index 7b95daf4d1..3422498b1d 100644 --- a/packages/core/package-subpaths.json +++ b/packages/core/package-subpaths.json @@ -92,6 +92,12 @@ "types": "./dist/audioFx.d.ts", "environments": ["browser", "bun", "node"] }, + "./audio-fx-copy": { + "source": "./src/audioFxCopy.ts", + "runtime": "./dist/audioFxCopy.js", + "types": "./dist/audioFxCopy.d.ts", + "environments": ["browser", "bun", "node"] + }, "./audio-fx-eq": { "source": "./src/audioFxEq.ts", "runtime": "./dist/audioFxEq.js", diff --git a/packages/core/package.json b/packages/core/package.json index 7411f8d184..b95101cde3 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -106,6 +106,12 @@ "import": "./src/audioFx.ts", "types": "./src/audioFx.ts" }, + "./audio-fx-copy": { + "bun": "./src/audioFxCopy.ts", + "node": "./dist/audioFxCopy.js", + "import": "./src/audioFxCopy.ts", + "types": "./src/audioFxCopy.ts" + }, "./audio-fx-eq": { "bun": "./src/audioFxEq.ts", "node": "./dist/audioFxEq.js", @@ -410,6 +416,10 @@ "import": "./dist/audioFx.js", "types": "./dist/audioFx.d.ts" }, + "./audio-fx-copy": { + "import": "./dist/audioFxCopy.js", + "types": "./dist/audioFxCopy.d.ts" + }, "./audio-fx-eq": { "import": "./dist/audioFxEq.js", "types": "./dist/audioFxEq.d.ts" diff --git a/packages/core/src/audioFxCopy.test.ts b/packages/core/src/audioFxCopy.test.ts new file mode 100644 index 0000000000..95c9d979f2 --- /dev/null +++ b/packages/core/src/audioFxCopy.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; +import { defaultAudioFxParams, HF_AUDIO_FX } from "./audioFx.js"; +import { HF_AUDIO_FX_PRESETS } from "./audioFxPresets.js"; +import { BANDS, EFFECT_COPY, PRESET_PROBLEM, SUMMARY } from "./audioFxCopy.js"; + +/** + * The copy layer is only worth having if it covers everything that ships. A gap + * is not a missing nicety — it is a rack panel labelled `highpass` in front of + * somebody who came here to stop a hum, which is the exact failure this layer + * exists to prevent. + * + * This was a build step in `plans/audio-fx-ux/build-preview.mts`, which meant it + * only caught a gap when somebody remembered to rebuild the review page. Here it + * catches it on the commit that adds the effect. + */ +describe("every shipped effect has plain-language copy", () => { + for (const def of HF_AUDIO_FX) { + it(`${def.id}`, () => { + const copy = EFFECT_COPY[def.id]; + expect(copy, `${def.id} has no copy`).toBeDefined(); + if (!copy) return; + for (const param of def.params) { + expect(copy.params[param.key], `${def.id}.${param.key} has no plain name`).toBeDefined(); + } + // "strength" is the one legal fiction: it means the module gets a single + // derived knob and its real parameters live behind Details. Anything else + // has to name a parameter the effect actually has, or the panel would put + // its headline control on a knob that does not exist. + if (copy.primary !== "strength") { + expect( + def.params.map((p) => p.key), + `${def.id}'s primary "${copy.primary}" is not one of its parameters`, + ).toContain(copy.primary); + } + expect(SUMMARY[def.id], `${def.id} has no closed-state summary`).toBeDefined(); + }); + } +}); + +it("every preset says which everyday problem it answers", () => { + const missing = HF_AUDIO_FX_PRESETS.filter((p) => !PRESET_PROBLEM[p.id]).map((p) => p.id); + expect(missing).toEqual([]); +}); + +it("describes no effect the registry does not ship", () => { + const shipped = new Set(HF_AUDIO_FX.map((d) => d.id)); + // The other direction. Copy for an effect that has been removed or renamed is + // dead text that reads as covered, and the count in the review page would say + // so too. + expect(Object.keys(EFFECT_COPY).filter((id) => !shipped.has(id))).toEqual([]); + expect(Object.keys(SUMMARY).filter((id) => !shipped.has(id))).toEqual([]); +}); + +it("summarises every effect at its own defaults without throwing", () => { + for (const def of HF_AUDIO_FX) { + const summary = SUMMARY[def.id]; + if (!summary) continue; + // The first thing an author reads after adding an effect, so it has to be a + // sentence at the values it arrives with — not "undefined dB". + const text = summary(defaultAudioFxParams(def.id)); + expect(text, `${def.id} summarised as "${text}"`).toMatch(/^[^u].*[^ ]$/); + expect(text).not.toContain("undefined"); + expect(text).not.toContain("NaN"); + } +}); + +it("covers the spectrum without a gap or an overlap", () => { + // The ruler is shared by every spectral module, so a hole in it is a frequency + // the rack can name in one place and not in another. + expect(BANDS[0]?.from).toBe(20); + expect(BANDS.at(-1)?.to).toBe(20000); + for (let i = 1; i < BANDS.length; i++) { + expect(BANDS[i]?.from, `gap or overlap before ${BANDS[i]?.name}`).toBe(BANDS[i - 1]?.to); + } +}); diff --git a/packages/core/src/audioFxCopy.ts b/packages/core/src/audioFxCopy.ts new file mode 100644 index 0000000000..2dbcd3f4c8 --- /dev/null +++ b/packages/core/src/audioFxCopy.ts @@ -0,0 +1,375 @@ +/** + * The plain-language layer over the effect registry. + * + * Every entry is written for somebody who has never opened a mixer. The rule + * used throughout: name the OUTCOME, never the mechanism, and describe a control + * by what changes in the sound rather than what it does to the signal. + * + * This is a layer *over* the registry, not a replacement for it. `HF_AUDIO_FX` + * stays the authority on what an effect is and what its parameters do; this says + * what to call those things in front of an author. `audioFxCopy.test.ts` holds + * the two together — every shipped effect, every one of its parameters, and + * every preset must have an entry here, so adding one to the registry without + * copy fails a test rather than shipping a rack panel labelled `highpass`. + * + * Tone and the levelling module are deliberately absent: both carry their own + * copy in core already (`audioEqSummary`, `levellingSummary`), because a summary + * that has to read the chain belongs beside the code that writes it. + * + * Design rationale, and the review page built from this, in + * `plans/audio-fx-ux/README.md`. + */ + +export interface Ends { + /** What the low end of the control sounds like. */ + low: string; + high: string; +} + +export interface ParamCopy { + label: string; + hint?: string; + ends?: Ends; +} + +export interface EffectCopy { + /** What the module is called in the rack. Never the DSP name. */ + title: string; + /** One line: what it is for. Present tense, second person implied. */ + does: string; + /** The problem an author would say out loud that leads here. */ + reachFor: string; + /** + * The single control that carries the module. Either a real parameter key, + * or "strength" — meaning the module gets one derived knob and the real + * parameters live behind Details. + */ + primary: string; + primaryEnds: Ends; + /** Plain names for the real parameters, shown only under Details. */ + params: Record; + /** Which frequencies it acts on, for the shared ruler. Omit if not spectral. */ + band?: [number, number]; +} + +export const EFFECT_COPY: Record = { + gain: { + title: "Volume", + does: "Turns this track up or down.", + reachFor: "It's too loud, or too quiet, against everything else.", + primary: "gain", + primaryEnds: { low: "Silent", high: "Louder" }, + params: { gain: { label: "Level", ends: { low: "Silent", high: "Louder" } } }, + }, + highpass: { + title: "Remove Rumble", + does: "Cuts the very bottom — traffic, footsteps, air conditioning, hands on the mic.", + reachFor: "There's a low hum or thump under everything.", + primary: "frequency", + primaryEnds: { low: "Only the deepest", high: "Thins the voice out" }, + band: [20, 300], + params: { + frequency: { + label: "Cut below", + hint: "Everything under this is removed.", + ends: { low: "Only the deepest", high: "Thins the voice out" }, + }, + q: { label: "Sharpness", hint: "How abruptly the cut starts." }, + poles: { label: "Steepness", hint: "How fast it falls away below the point." }, + }, + }, + lowpass: { + title: "Muffle", + does: "Takes the top off, like the sound is coming through a door.", + reachFor: "You want something to sound distant, or behind something else.", + primary: "frequency", + primaryEnds: { low: "Very muffled", high: "Barely changed" }, + band: [1000, 20000], + params: { + frequency: { label: "Cut above", ends: { low: "Very muffled", high: "Barely changed" } }, + q: { label: "Sharpness" }, + poles: { label: "Steepness" }, + }, + }, + peaking: { + title: "Shape One Range", + does: "Lifts or lowers one part of the sound and leaves the rest alone.", + reachFor: "One quality is wrong — boomy, boxy, harsh — but the rest is fine.", + primary: "gain", + primaryEnds: { low: "Take it out", high: "Bring it forward" }, + band: [20, 20000], + params: { + frequency: { label: "Where", hint: "Which part of the sound to change." }, + gain: { label: "How much", ends: { low: "Take it out", high: "Bring it forward" } }, + q: { + label: "How wide", + hint: "A narrow setting fixes one note; a wide one changes the whole character.", + }, + }, + }, + lowshelf: { + title: "Bass", + does: "More or less weight underneath everything.", + reachFor: "It sounds thin, or too heavy.", + primary: "gain", + primaryEnds: { low: "Thinner", high: "Heavier" }, + band: [20, 300], + params: { + frequency: { label: "Up to", hint: "Everything below this is lifted or dropped." }, + gain: { label: "How much", ends: { low: "Thinner", high: "Heavier" } }, + }, + }, + highshelf: { + title: "Brightness", + does: "More or less sparkle at the top.", + reachFor: "It sounds dull, or too fizzy.", + primary: "gain", + primaryEnds: { low: "Duller", high: "Brighter" }, + band: [2000, 20000], + params: { + frequency: { label: "From", hint: "Everything above this is lifted or dropped." }, + gain: { label: "How much", ends: { low: "Duller", high: "Brighter" } }, + }, + }, + compressor: { + title: "Even Out Loudness", + does: "Brings the quiet parts up and holds the loud parts down, so nothing jumps out at the listener.", + reachFor: "Some words are much louder than others.", + primary: "strength", + primaryEnds: { low: "Barely touched", high: "Very even, quite squashed" }, + params: { + threshold: { label: "Starts working at", hint: "Anything louder than this gets held down." }, + ratio: { label: "How hard", hint: "How much of the excess is removed." }, + attack: { label: "How fast it grabs", ends: { low: "Instant", high: "Lets peaks through" } }, + release: { label: "How fast it lets go", ends: { low: "Snappy", high: "Smooth" } }, + knee: { label: "How gradual" }, + makeup: { + label: "Volume back up", + hint: "Compression makes things quieter; this puts the level back.", + }, + mix: { label: "Blend with the original" }, + }, + }, + limiter: { + title: "Peak Ceiling", + does: "Nothing gets louder than this, ever. A safety net at the end of the chain.", + reachFor: "You want to be sure it never clips or spikes.", + primary: "limit", + primaryEnds: { low: "A lot of headroom", high: "Right up to the edge" }, + params: { + limit: { + label: "Never exceed", + ends: { low: "A lot of headroom", high: "Right up to the edge" }, + }, + attack: { label: "How fast it catches" }, + release: { label: "How fast it recovers" }, + level_out: { label: "Level after" }, + }, + }, + gate: { + title: "Silence the Gaps", + does: "Mutes the pauses between words. Room tone under speech stays — this closes the silences, it does not remove noise.", + reachFor: "You can hear the room breathing between sentences.", + primary: "strength", + primaryEnds: { low: "Only true silence", high: "Cuts quiet words too" }, + params: { + threshold: { label: "Quieter than this is a gap" }, + range: { + label: "How far to duck the gaps", + hint: "Not all the way down, usually — total silence sounds broken.", + }, + ratio: { label: "How hard" }, + attack: { label: "How fast it opens" }, + release: { + label: "How fast it closes", + ends: { low: "Clips word endings", high: "Leaves tails intact" }, + }, + knee: { label: "How gradual" }, + }, + }, + saturate: { + title: "Warmth", + does: "Adds a little grit and density, the way analogue gear does.", + reachFor: "It sounds clean but lifeless.", + primary: "strength", + primaryEnds: { low: "Just a sheen", high: "Openly distorted" }, + params: { + type: { + label: "Character", + hint: "Different flavours of the same idea. Tanh is the gentle one.", + }, + threshold: { + label: "How much drive", + ends: { low: "Just a sheen", high: "Openly distorted" }, + }, + output: { label: "Level after", hint: "Drive makes things louder; this puts it back." }, + oversample: { label: "Quality", hint: "Higher costs more but sounds cleaner." }, + }, + }, + bitcrush: { + title: "Lo-Fi", + does: "Crushes the sound down to fewer steps, like an old sampler or a bad phone line.", + reachFor: "You want it to sound cheap or digital on purpose.", + primary: "strength", + primaryEnds: { low: "Slightly gritty", high: "Destroyed" }, + params: { + bits: { label: "How many steps", ends: { low: "Destroyed", high: "Clean" } }, + samples: { + label: "How rough", + hint: "Holds each value for longer, which dulls and grits it.", + }, + mix: { label: "Blend with the original" }, + }, + }, + delay: { + title: "Echo", + does: "Repeats the sound after a gap.", + reachFor: "You want space, or a rhythmic effect.", + primary: "mix", + primaryEnds: { low: "A hint", high: "Washed out" }, + params: { + time: { label: "Gap between repeats" }, + feedback: { label: "How many repeats", ends: { low: "One", high: "Trails away for ages" } }, + mix: { label: "How loud", ends: { low: "A hint", high: "Washed out" } }, + }, + }, + reverb: { + title: "Room", + does: "Puts the sound somewhere, instead of nowhere.", + reachFor: "It sounds dry and stuck to the speaker.", + primary: "strength", + primaryEnds: { low: "A small tight room", high: "A big open hall" }, + params: { + size: { label: "How big the space is" }, + damping: { + label: "How soft the walls are", + ends: { low: "Hard and bright", high: "Soft and dark" }, + }, + wet: { label: "How much room" }, + dry: { label: "How much original" }, + }, + }, + chorus: { + title: "Thicken", + does: "Doubles the sound slightly out of tune, which makes it wider and less exact.", + reachFor: "It sounds thin or too plain on its own.", + primary: "mix", + primaryEnds: { low: "Just wider", high: "Obviously wobbling" }, + params: { + delay: { label: "Spread" }, + depth: { label: "How much wobble" }, + speed: { label: "How fast it wobbles" }, + mix: { label: "How much", ends: { low: "Just wider", high: "Obviously wobbling" } }, + }, + }, + phaser: { + title: "Swirl", + does: "A filter that sweeps up and down, giving a moving, hollow shimmer.", + reachFor: "You want movement, or a 1970s flavour.", + primary: "out_gain", + primaryEnds: { low: "Subtle", high: "Strong" }, + params: { + in_gain: { label: "Depth in" }, + out_gain: { label: "How strong", ends: { low: "Subtle", high: "Strong" } }, + delay: { label: "Where it sweeps" }, + decay: { label: "How resonant" }, + speed: { label: "How fast it sweeps" }, + type: { label: "Shape of the sweep" }, + }, + }, +}; + +/** + * The shared vocabulary. Frequencies mean nothing to somebody who has not been + * taught them; these words are what the same person would say unprompted, and + * naming the ranges once teaches them everywhere they appear. + */ +export const BANDS: { from: number; to: number; name: string; says: string }[] = [ + { from: 20, to: 80, name: "Rumble", says: "traffic, footsteps, handling" }, + { from: 80, to: 250, name: "Weight", says: "chest, body, warmth" }, + { from: 250, to: 600, name: "Mud", says: "boxy, muffled, cardboard" }, + { from: 600, to: 2000, name: "Middle", says: "the body of a voice" }, + { from: 2000, to: 5000, name: "Presence", says: "consonants, intelligibility" }, + { from: 5000, to: 10000, name: "Edge", says: "sibilance, harshness" }, + { from: 10000, to: 20000, name: "Air", says: "sparkle, openness" }, +]; + +/** Which everyday complaint each preset answers. Presets ARE the product here. */ +export const PRESET_PROBLEM: Record = { + "voice-clean": "My voice sounds amateur", + "voice-broadcast": "I want it to sound like radio", + "voice-warm": "I want it intimate and close", + "rumble-cut": "There's a hum or thump underneath", + "room-gate": "I can hear the room between sentences", + "boom-tame": "My voice sounds boomy", + "harsh-tame": "It's harsh and tiring to listen to", + telephone: "Make it sound like a phone call", + "radio-am": "Make it sound like an old radio", + megaphone: "Make it sound shouted through a horn", + "lofi-tape": "Make it sound like an old tape", + "pa-system": "Make it sound like a station announcement", + intercom: "Make it sound like a door intercom", + "room-tight": "It sounds dry and stuck to the speaker", + "room-natural": "It should sound like a real place", + hall: "It should sound far away and big", + "slap-echo": "I want one quick echo", + "dub-throw": "I want long trailing echoes", +}; + +/** + * What a module says when it is CLOSED. + * + * The most-seen state by a distance: a rack with six modules is six of these + * and nothing else. So it is a sentence about what is happening to the sound, + * not a dump of the parameter that happens to be first. An author should be + * able to read the rack top to bottom and understand their own mix. + * + * Numbers stay in — they are what makes it checkable rather than vague — but + * they arrive inside a phrase instead of on their own. + */ +type P = Record; +const n = (v: unknown, fallback = 0) => (typeof v === "number" ? v : fallback); +const hz = (v: unknown) => { + const x = n(v); + return x >= 1000 ? `${(x / 1000).toFixed(x % 1000 === 0 ? 0 : 1)} kHz` : `${Math.round(x)} Hz`; +}; +const strength = (x: number, words: [string, string, string]) => + x < 0.34 ? words[0] : x < 0.67 ? words[1] : words[2]; + +export const SUMMARY: Record string> = { + gain: (p) => + n(p.gain) === 0 + ? "No change" + : n(p.gain) > 0 + ? `Up ${n(p.gain)} dB` + : `Down ${Math.abs(n(p.gain))} dB`, + highpass: (p) => `Cutting everything below ${hz(p.frequency)}`, + lowpass: (p) => `Muffled above ${hz(p.frequency)}`, + // A band at 0 dB is doing nothing, and saying "lifting by 0 dB" describes a + // non-event as though it were a setting. Freshly added effects sit exactly + // here, so this is the FIRST thing an author reads after adding one. + peaking: (p) => + n(p.gain) === 0 + ? `Sitting on ${hz(p.frequency)}, doing nothing yet` + : `${n(p.gain) > 0 ? "Lifting" : "Cutting"} ${hz(p.frequency)} by ${Math.abs(n(p.gain))} dB`, + lowshelf: (p) => + n(p.gain) === 0 + ? "Doing nothing yet" + : `${n(p.gain) > 0 ? "More" : "Less"} weight below ${hz(p.frequency)}`, + highshelf: (p) => + n(p.gain) === 0 + ? "Doing nothing yet" + : `${n(p.gain) > 0 ? "More" : "Less"} sparkle above ${hz(p.frequency)}`, + compressor: (p) => + `Evening out — ${strength(Math.min(1, (n(p.ratio, 3) - 1) / 7), ["gentle", "moderate", "firm"])}`, + limiter: (p) => `Nothing louder than ${n(p.limit, -1)} dB`, + gate: (p) => `Closing gaps quieter than ${n(p.threshold, -45)} dB`, + saturate: (p) => + `${strength(Math.min(1, Math.abs(n(p.threshold, -6)) / 30), ["A little", "Some", "Heavy"])} warmth`, + bitcrush: (p) => `Crushed to ${n(p.bits, 8)} bits`, + delay: (p) => `Echo every ${n(p.time, 250)} ms`, + reverb: (p) => + `${strength(n(p.size, 0.7), ["A small", "A medium", "A large"])} room, ${strength(n(p.wet, 0.35), ["lightly", "moderately", "heavily"])}`, + chorus: (p) => `Thickened${n(p.mix, 0.5) > 0.6 ? ", wobbling" : ""}`, + phaser: () => "Swirling", +}; diff --git a/plans/audio-fx-ux/README.md b/plans/audio-fx-ux/README.md index cf50d19819..fbba9073f7 100644 --- a/plans/audio-fx-ux/README.md +++ b/plans/audio-fx-ux/README.md @@ -5,10 +5,12 @@ routing, what is driven versus set. But information a casual author cannot read is decoration, and the rack speaks entirely in Hz, dB and ratios. So the drawing stays and the **language changes**. -`copy.mts` is the design work: a plain-language layer over every effect in the -registry. `build-preview.mts` renders the review page from it **plus the real -registry and preset catalogue**, and **fails** if any effect, parameter or -preset lacks copy — so the page cannot quietly omit something that ships. +The plain-language layer over every effect in the registry now ships as +`packages/core/src/audioFxCopy.ts`, with the coverage that used to gate this +page — every effect, parameter and preset must have copy — as +`audioFxCopy.test.ts`. `build-preview.mts` renders the review page from it +**plus the real registry and preset catalogue**. Only `PROFILES` is still a +proposal, and it is all that is left in `copy.mts`. ```bash bun plans/audio-fx-ux/build-preview.mts /tmp/rack-ux.html @@ -173,19 +175,23 @@ when nothing has been touched. ## Status -The EQ, the named jobs and the levelling script are **built**. `copy.mts` -itself is still a proposal — the plain-language layer over the other thirteen -effects has not landed, and neither have the `PROFILES` figures. +The EQ, the named jobs and the levelling script are **built**, and the copy +layer has now landed as `packages/core/src/audioFxCopy.ts` — `EFFECT_COPY`, +`BANDS`, `PRESET_PROBLEM` and `SUMMARY`, with the completeness check as a test +beside it rather than a build step. -`copy.mts` has no entry for Tone or for the levelling module, because both -carry their own copy in core (`audioEqSummary`, `levellingSummary`). That is -the right home for it: a summary that has to read the chain belongs beside the -code that writes it. +Landing the data is not the same as wiring it. Nothing in the studio reads it +yet, and it should not until the three questions above are answered — whether +the plain name replaces the DSP name or sits beside it decides what the rack +renders, and building it twice to find out is the expensive way. -`copy.mts` is a proposal, not shipped code. When it lands it wants to be -`packages/core/src/audioFxCopy.ts` beside the registry, with the completeness -check as a test rather than a build step. +It has no entry for Tone or for the levelling module, because both carry their +own copy in core (`audioEqSummary`, `levellingSummary`). That is the right home +for it: a summary that has to read the chain belongs beside the code that +writes it. The `PROFILES` figures — what one knob derives at gentle/middle/strong — are -proposed values, not measured ones. They want the same before/after listen the -clip-before-duck fix got. +**still proposed values, not measured ones**, which is why they stayed behind in +`copy.mts` rather than going to core with the rest. They want the same +before/after listen the clip-before-duck fix got before a knob is wired to +them.