feat(core): the audio FX preset catalogue, and applying one from the rack - #3177
Conversation
574d198 to
540056a
Compare
68263fa to
5fb2c91
Compare
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review: feat(core): the audio FX preset catalogue — #3177
Verdict: LGTM (as load-bearing pair with #3178)
Catalog integrity is solid: 18 presets across 4 families, all with unique ids, valid effect types, and param values checked against declared min/max ranges. The test suite is unusually thorough for preset data — checking domain invariants (limiters always last, boost-bounded presets, shelves stocked), not just structure.
Apply/re-apply mechanics are correct. Append-by-default is the right call for the primary workflow (stacking character on voice). Re-apply uses findIndex to locate the first fromPreset match, then reconstructs with kept.slice(0, existing) + new nodes + kept.slice(existing). The index arithmetic is subtle but sound — everything before the first match has fromPreset !== preset.id, so the kept filter doesn't shift indices below existing. The ordering test (telephone → hall → re-apply telephone) proves the position-preservation.
The steep() helper (two cascaded 2-pole filters for 24 dB/oct) is well-documented — worth encoding once here rather than having every author rediscover the registry's 12 dB/oct-per-node limit.
Concur with tai's load-bearing pair finding. At #3177's isolated head, fromPreset is on the HfAudioFxNode interface but NOT in parseAudioFxChain or serializeAudioFxChain — those additions are in #3178's diff. So at this PR's head, fromPreset drops on save/reload, and the next re-apply silently doubles the preset. The round-trip test at line 171-176 only checks n.type, so it wouldn't catch this. #3178 is precisely the fix (with an upgraded round-trip test that checks fromPreset and label). These two must merge together.
Nit: activeAudioFxPresetIds uses Array.includes for a linear scan — O(N*M) for N nodes × M distinct presets. Harmless at current scale but a Set would be trivially better.
No blocking issues.
Review by Miga
🤖 Generated with Claude Code
5fb2c91 to
def70d3
Compare
540056a to
382f657
Compare
def70d3 to
df2cc3e
Compare
382f657 to
bd69e6f
Compare
df2cc3e to
6a84ad0
Compare
bd69e6f to
d0770bf
Compare
6a84ad0 to
804bc1e
Compare
d0770bf to
6d8c95f
Compare
# Conflicts: # packages/core/src/runtime/webAudioTransport.ts
…a main merge An earlier merge with main brought this deleted file back (git's merge/delete handling on an unchanged-on-one-side file); package.json already points at build-inline-artifact.ts, so it sat unreachable and duplicating that file's config, both of which fallow flagged.
* feat(core): carve against every voice over a bed, always dynamically A bed usually runs under a whole sequence — a narrator, an interview answer, a second presenter — and carving against one of them left the others fighting it. `source` becomes `sources`, and `mixCarveSources` sums every voice onto the BED's clock before anything is measured. That is what keeps one analysis sufficient: the chain is fixed, so there is no per-voice filter to switch between, and bands drawn from all the speech there is with envelopes that rise wherever any of it happens answer the actual question — where and when is speech masking this bed. Summed rather than averaged: two people talking at once mask more than either alone. Audio before the bed starts is dropped rather than folded in at zero, since it plays over nothing and shifting it would put a cut where there is no voice. `dynamic` is gone. A fixed depth thins the bed through every pause, and once both have been heard there is no reason to want it, so every carve follows the speech. Two helpers the panel and the headless script now share instead of each carrying a copy — two definitions of "what does this name suggest" drift, and then the two disagree about which track is the voice: - `classifyAudioName` reads a track's kind from its id and filename together. `unknown` is deliberately common: treating an unrecognised name as "not a voice" would hide the one track somebody needs to pick. - `clipsOverlap` keeps out a voice that never plays while the bed does. An unwritten duration counts as unbounded, not zero — refusing a clip whose length the composition leaves to the media would drop the commonest case there is. Files written before this still load: a single `source` reads as a one-voice list, a stored `dynamic` is ignored, and an absent attribute means the defaults whole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(core): stop \b from missing underscore-separated names, guard clipsOverlap's negative duration \b treats `_` as a word character, so \bbed\b never matched bed_01, music_bed_loop, or theme_song, and \bvo\b/\bvox\b/\btts\b had the same gap — an underscore-separated bed classified as "unknown" and could end up offered as its own carve source. Replaced the short hints with a boundary that actually excludes letters and digits on both sides. clipsOverlap computed end = start + duration without guarding sign, so a negative duration put end before start — an interval that does not describe anything, and one specific case showed it silently dropping a real overlap (a shorter, earlier broken end rejected a clip that genuinely contained the point). Duration clamps to zero instead: a clip cannot un-play time, and a zero-length clip at its start is the sane reading of "duration nobody wrote down as positive." Review by Miga (PR #3212). --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3212 (accidentally squash-merged into this branch instead of main) changed HfCarveSettings from a single `source` + `dynamic` toggle to a `sources` list with dynamic mode removed outright — the multi-voice UI consumer that goes with that shape lands in the very next PR, so this branch was left with a type that no longer matched its own code. Minimal port, not the multi-voice redesign that PR does properly: the "Listen to" picker and analyse() treat sources[0] as the one voice this UI still understands, and every dynamic-mode branch (the automated envelope lanes, the toggle, the checkbox) is gone along with the field — a carve is now always the static value the analysis computes, matching what the type change made permanent. Test suite trimmed the same way: the automation-lane and toggle tests covered behavior that no longer exists.
# Conflicts: # packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx # packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx # packages/studio/src/components/editor/propertyPanelFxSection.tsx
# Conflicts: # packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx # packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx # packages/studio/src/components/editor/propertyPanelFxSection.tsx # skills-manifest.json # skills/hyperframes-audio/SKILL.md # skills/hyperframes-audio/references/attributes.md # skills/hyperframes-audio/scripts/carve.mjs
…EADME Blocks the regression workflow's required preflight gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
18 presets in 4 families — voice 3, repair 4, character 6, space 5.
Applying one appends by default: stacking a character preset onto an already-cleaned voice is a real thing to want, and replacing silently throws work away. Re-applying a preset already present replaces its own nodes in place, because position in the chain is signal order and signal order is audible.
Carries the design docs for the rack UX that follows.
🤖 Generated with Claude Code