Repository navigation
feat(audio): composition master bus (data-fx-chain on the root) - #5420
Conversation
miguel-heygen
left a comment
There was a problem hiding this comment.
Request changes at b2f8e12b on one decision. The code is otherwise sound.
Blocking: a new authored attribute
data-master-fx-chain (packages/core/src/audioGroups.ts:135, documented at docs/reference/audio-effects.mdx:183) is a new authoring attribute, plus a lint rule for it. The project's rule is no new authoring format unless it rides an existing pattern, and there is one here.
data-fx-chainalready carries a chain on<audio>,<video>and<hf-audio-group>.- The same attribute on the top-level composition root, read by the same
resolveMasterBusElement, gives a master bus without a new name. - The only reason visible in the code for a separate name is keeping the root's
data-automationfrom being read as the master's. The reader can ignore that attribute on the root.
Please reuse data-fx-chain on the root, or hold this until the new name is agreed.
Minor
packages/engine/src/services/audioMixer.ts:1136: an error in the master pass skips the temp-dir cleanup at:1674. The full float sum (about 1.4 GB per hour of audio) stays on disk; groups have the same gap at:1643.- A bad master chain makes a video-only export. Under the default warning policy, render reports
audio_processing_failedand ships video only, while preview plays dry with no message (runtime/audioFx.ts:66-69). One typo on the root silences every export of that composition, and no lint rule checks chain contents. The docs' "fails the whole mix" should say so. packages/lint/src/rules/media.ts:1157skips every element withdata-composition-id. A nested sub-composition root carrying the attribute, which is ignored at runtime, gets no warning, though the docs say it does.- Test gap in
audioFxRender.test.ts("limits the sum"): nothing proves the sum stays float before the limiter. A clipped 16-bit sum would still pass. - Comment blocks over four lines:
audioGroups.ts:137(5),runtime/audioFx.ts:123(5),runtime/webAudioTransport.ts:628(7),audioMixer.ts:1116(7).
Checked
- Render without a master chain is byte-identical to before: same ffmpeg call, same AAC args.
- 48 kHz stereo float throughout, with no clipping between the sum, the chain and the encode.
- Preview and render agree on order (after each group's chain and fader, once over the full mix) and on gain staging.
- Volume, mute and the meter come after the chain.
- Sub-composition roots are ignored in both preview and render.
- Live attribute edits are heard, and teardown disposes the chain.
b2f8e12 to
6b7b93e
Compare
Edit accuracy: accurate 2061 (base branch 2061), smooth 1623 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
miguel-heygen
left a comment
There was a problem hiding this comment.
Request changes at 6b7b93e2 on one parity bug. The authoring question is resolved.
Resolved
- The master bus is the existing
data-fx-chainon the top-level composition root.data-master-fx-chainand its lint rule are gone, with no leftovers. - No tracked composition in the repo carries
data-fx-chainon its root, so nothing changes its audio on upgrade.
Blocking: preview skips the master chain for a plainly audible <video>
- The master chain is attached only through the WebAudio graph (
packages/core/src/runtime/webAudioTransport.ts:634,:669). - A
<video>joins that graph only for volume over 1, its own chain, automation or a group (packages/core/src/runtime/init.ts:384-393,packages/core/src/audibleVideo.ts:16-25). A root master chain isn't a reason. - Render does sum that video into the master (
packages/engine/src/services/audioMixer.ts:587-622). - Scenario: a talking-head video plus music under a root
truepeak. Preview limits only the music, and the meter never sees the video; the export limits both. A composition whose only audio is video gets no master chain in preview at all. - Fix: a root master chain makes every audible video join.
Minor
- The "-6 dB over two clips gives -12 dB" test can't tell a per-clip chain from one over the sum, because gain is linear. Its comment's "-9" is wrong, though the
truepeaktest beside it does catch the per-clip case. - The fader-order test uses
groupVolume: 1, so it doesn't test the fader. docs/reference/audio-effects.mdx:510says "Four rules exist" but lists three.- A bad root chain ships a video-only export with a warning, and the docs don't say so.
- A sub-composition file previewed or rendered on its own applies its root chain. That's undocumented; the docs say it is "not applied to anything".
audioMixer.ts:1136: the master pass has no catch, so it skips the temp cleanup at:1674.
6b7b93e to
6dfbb76
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Request changes at 6dfbb76a on two blockers. Preview routing of plain videos is fixed.
Blocking
- The required check
Studio and player capturesis red. The PR now touchespackages/studio(the meter), and the body has no## Before/## Aftercaptures. - The export silently drops the master chain when every audio track is auto-detected.
- The parser emits video audio only for
video[id][data-has-audio="true"](packages/engine/src/services/audioMixer.ts:651), and attachesmasterFxChainonly to the elements it emits (:622). - A plain
<video>without that attribute gets its audio track from the producer (packages/producer/src/services/render/stages/extractVideosStage.ts:571), with no chain. - The mixer takes the chain from the first element that has one (
audioMixer.ts:1520). - So a composition whose only audio is
<video src="talk.mp4">under a roottruepeakexports with no ceiling and no warning, while preview applies it (packages/core/src/runtime/init.ts:395). - Passing the master chain once per composition, not per element, fixes it.
- The parser emits video audio only for
Fixed
- A plain audible video now joins WebAudio only when the top-level root has a chain. Muted videos and
data-has-audio="false"stay out, and sub-composition roots don't trigger it. - The temp dir is removed on every exit, with tests that fail without it.
- No
data-master-fx-chainis left anywhere.
Follow-up
- A root chain added or removed mid-play isn't picked up by videos that are already playing until the next play.
- A cross-origin video without
crossoriginplays dry in preview but is limited in render. Worth a docs line. - The fader-order test uses linear gains, so it can't tell the two orders apart.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
782f6bf to
a504464
Compare
A clip can belong to only one audio group, so there was no way to put one effect, such as a limiter, across the whole mix. The data-fx-chain on the top-level composition root now runs once over the sum of every clip and every group, after each group's own chain and fader. No new attribute: a sub-composition root's data-fx-chain was never applied to anything and still is not. Render: when a master chain exists, mixAudioTracks writes a float WAV instead of AAC, applyAudioFxChain runs the chain over it, and ffmpeg encodes AAC last, so the chain reads unclipped samples. An unreadable chain is a mix failure, as for a group. Preview: the transport splices the chain between the clips' sum and the monitor, re-anchored per play generation and watched for edits. Only the top-level root is read. No master automation: the root's data-automation is not read as the bus's. No lint rule: data-fx-chain is valid on clips, groups and roots, and the one place it does nothing (a sub-composition root) has always been silent. — Jerrai
A root data-fx-chain is attached only inside the Web Audio graph, but a plainly audible <video> joined that graph only for its own chain, group, automation or a volume over 1. So preview limited the music and left the talking head dry, and a composition whose only audio is video got no master chain at all, while export sums every audible element into the master. A master chain on the top-level composition root is now a reason to join, the same as a video's own chain. Elements under a root with no chain keep native output. Sub-composition roots are still ignored. — Jerrai
…meter) - audioMixer: a fatal FX error in the master pass or a group bus left the work directory (the full float sum, about 1.4 GB an hour) on disk. The final mix now runs in try/finally and the group rethrow removes it first. - Tests: the "once over the sum" case now uses a limiter then a gain, so a per-clip chain (-10.4) and a doubled one (-18) both fail it; a new case proves the sum stays float until the chain reads it; the fader case uses a group volume of 0.5 so the fader's position is exercised; the work-dir removal is covered for both paths. - Docs: "Three rules exist" (the master-bus lint rule is gone), a bad master chain stops the render with audio_processing_failed, a sub-composition file opened on its own has its root chain as the master bus, and a seek clears a group or master limiter. - Studio: TimelineElement.masterChain carries the root's data-fx-chain, so the program meter shows for a video-only composition whose only audio routing is the master chain, as preview now plays it. - Comment blocks over four lines cut; webAudioTransport.setRate split so its CRAP score is under the Fallow threshold. — Jerrai
…-limiter copy - Master chain: it was carried on each parsed AudioElement, so a track the producer adds itself (appendAutoDetectedVideoAudio, the browser-discovered audio in probeStage) never had it, and a composition whose only audio was such a track exported with no master chain, no ceiling and no warning. It is now a composition property: parseMasterFxChain(html) at compile, carried on CompiledComposition and CompositionMetadata, passed to processCompositionAudio by runAudioStage (in-process and distributed plan). AudioElement.masterFxChain is removed so there is one source. Each forward (compile stage, local orchestrator, distributed plan) has a test that fails when it is dropped. - Plain limiter: it follows the level with no lookahead and Level after is applied past the ceiling, so it cannot promise "nothing gets past" or "never clips". Studio copy (description, panel text, knob label, summary) and the hyperframes-audio skill now say a fast or inter-sample peak can pass it and point to the true-peak limiter for a delivery limit. A copy test guards it. — Jerrai
…lan test with the ffmpeg tests Drop three one-line field comments that raised their files' comment share, and list plan.masterBus.test.ts with the integration tests because plan() reads the ffmpeg version. — Jerrai
a504464 to
9e6781f
Compare
…ability map The reachability map pins the hash of test-classification.mjs, which now lists one more integration test. The map's producer lane command already covers every test under src, in both the unit and integration matrix lanes. — Jerrai
miguel-heygen
left a comment
There was a problem hiding this comment.
Approve at 435694da. Both blockers from 6dfbb76a are resolved, every required check is green, and behaviour is unchanged since 782f6bfb apart from the rebase and two CI-only commits.
What I checked
- Range-diff
782f6bfb..435694da: the five #5419 commits drop out (merged). Three commits are identical. The fourth differs only in surrounding lines that came from main (dash cleanups inaudioFx.tsandaudioFxCopy.ts). The two new commits only drop three field comments and listplan.masterBus.test.tswith the integration tests (and repin its hash). - Blocker 1 (captures):
Studio and player capturespasses and the body has Before and After. - Blocker 2 (master chain dropped for producer-added tracks): the chain is now read once from the top-level root (
parseMasterFxChain) and handed toprocessCompositionAudiobeside the tracks. Both callers pass it: the local render (renderOrchestrator.ts:3359) and the distributed plan (plan.ts:1133), which mixes once before chunking. So a composition whose only audio is a plain<video>gets its root chain in the export, once, over the sum. - Tests: core 438/438, engine 94/94 (none skipped, with ffmpeg and a headless browser), producer 25/25 plus
htmlCompiler.test.ts142/142, studio 61/61. Each of the eight key fixes was reverted in turn and its test failed.
Follow-up (not blocking)
- A scene rendered on its own inside a project uses the index's master chain, not its own.
standaloneEntry.tswraps the scene in a shallow clone of the index root, which keeps the index'sdata-fx-chain, so the scene root is nested and its chain is never applied.docs/reference/audio-effects.mdx:201-203says the opposite. Either fix the docs (and the audio skill) or carry the scene's chain onto the wrapper, with a test that puts chains on both roots. - Long exports with a master chain fail. The master pass writes the whole mix as one float WAV and reads it back in one call (
audioMixer.ts:1054,audioFxRender.ts:286), so past about 93 minutes the read exceeds Node's 2 GiB file limit. Process it in blocks, or fail early with a clear message. audioMixer.ts:1445says a bad master chain fails fast, but it is parsed after every clip has been processed. Parsing it first gives the same error with no wasted work.- The tests that prove the chain runs once over the sum skip in CI because that job has no browser.
resolveMasterBusElement(audioGroups.ts:135) is a second rule for "the root" that differs fromfindRootCompositionElementondata-root="true".- The docs line that preview routes "every audible
<video>" through the master bus doesn't hold for a cross-origin video withoutcrossorigin, which plays dry in preview but gets the chain in the render. - Nit: the
parseMasterFxChaindoc comment (audioMixer.ts:525) runs five lines with its delimiters; two lines would say it.
Before
The composition below is one
<video>with audio and atruepeaklimiter in the root'sdata-fx-chain. Studio read nothing from the root chain, so it did not count the video as audio: no meter toggle in the timeline toolbar, no program meter.After
Same project. The timeline toolbar has the audio meter toggle and the Monitor meter strip shows (toggled on here), because the root's master chain is read onto the clip and an audible video under a master chain counts as audio.
Captured in a real Studio (Vite dev server) on a 6 s test video with a 330 Hz tone and a root
truepeakat -1 dBTP. "Before" is this branch with the twomasterChainreads inpackages/studio/src/player/lib/timelineDOM.tsdisabled, which is exactly the Studio diff against thetruepeakPR. Not exercised: playing audio through the meter, dark theme, a muted video, a sub-composition root chain.What
This adds a composition master bus. A clip can belong to only one audio group, so there was no way to put one effect, such as a true-peak limiter, across the whole mix.
data-fx-chainon the top-level composition root. No new attribute: a root'sdata-fx-chainwas previously applied to nothing.data-fx-chainstill does nothing (it is neither the master nor its clips' chain), and on clips and groupsdata-fx-chainmeans what it always did.truepeakPR.Preview and render
mixAudioTrackswrites a float WAV,applyAudioFxChainruns the chain, and ffmpeg encodes AAC lastdata-fx-chainleft on a top-level root, inert before, now runs as the master bus. Nothing in this repo writes one there.elementId: "master". In preview it plays dry, the same as a broken group chain.data-automationon the root is not read as the master's.Preview joins every audible video (round 2)
The master chain exists only inside the Web Audio graph, and a
<video>used to join that graph only for its own chain, group, automation or a volume over 1. So a talking head under a roottruepeakplayed dry in preview while the music was limited, and a composition whose only audio is video got no master chain at all. Render already summed every audible element into the master.A master chain on the top-level root is now a reason to join, like a video's own
data-fx-chain:audibleVideoNeedsWebAudiotakes amasterChainfield (packages/core/src/audibleVideo.ts);joinsWebAudioinpackages/core/src/runtime/init.tspassesreadMasterFxChain(el.ownerDocument);Unchanged: a root with no chain keeps a unity video on native output; muted and
data-has-audio="false"videos stay out; a chain on a sub-composition root does not count. The root chain is read when media is scheduled (play, seek, rate change), the same moment a video's owndata-fx-chainis read.Tests: a plain audible video at unity under a root chain is scheduled and acquires a media element source; the same video with no root chain stays native (existing test); a video-only composition plus an audio clip both join; a muted video stays out; a sub-composition root chain does not count; at transport level a plain video played from its element goes through the master bus (
masterGainis spliced out of the monitor). Reverting thejoinsWebAudioline fails 3 of the new init tests, and reverting theaudibleVideo.tschange fails those 3 plus the predicate test.Tests
audioMixer.test.ts).Docs
docs/reference/audio-effects.mdxgets a master-bus section. Thehyperframes-audioskill is updated in SKILL.md,attributes.mdandfx-registry.md.Round 2 minors
audioMixer.ts): the final mix now runs intry/finallyand the groupAudioFxRenderErrorrethrow removes the work directory first, so neither path leaves the float sum on disk. Both are covered by tests that fail without the change.groupVolumeis 0.5 now, so the fader sits between the group limiter and the master and the test fails if either order changes.data-master-fx-chain).audio_processing_failedblocks the render (RenderQualityError) under every strictness setting, so nothing is exported; preview plays dry with no message. They also say that no lint rule checks a chain's contents.data-composition-idelements went withdata-master-fx-chain; no lint code is in this PR, and the docs no longer promise a warning.resetas the group bus, in thetruepeakPR).hasProjectAudiohid the meter for a video-only composition whose only routing is the root chain.TimelineElement.masterChainnow carries the root'sdata-fx-chain(set intimelineDOM.ts, both the manifest and DOM paths), and the existingaudibleVideoNeedsWebAudiocall sees it. Tests inaudioMeterMath.test.tsandtimelineDOM.test.ts; not run locally (no Studio dependencies installed), left to CI.webAudioTransport.setRatewas over the CRAP threshold; its group and master loops are onesetBusRate. An introduced 8-line clone in the engine test is a sharedmixedPeakDbhelper. Exit 0 locally.audibleVideo.tsis gone because the comment-share ratchet (run by CI, first time on that commit) refuses a raised share.Round 4
AudioElement, so a track the producer adds itself never had it. That isappendAutoDetectedVideoAudio(extractVideosStage.ts) and the browser-discovered audio inprobeStage.ts: a<video>whosesrcis set at runtime has no parsed audio entry, so the producer adds one after probing. When that was the only audio, nothing carried the chain and the export had no ceiling and no warning.parseMasterFxChain(html)runs at compile,CompiledCompositionandCompositionMetadatacarry it, andrunAudioStagepasses it toprocessCompositionAudio(in-process and distributed plan).AudioElement.masterFxChainis removed so there is one source.<video>with nodata-has-audiowas not affected: the compiler stampsdata-has-audio="true"and the parsed track carried the chain.<video>whosesrcis set by a script, and the root has atruepeakat -6 dBTP. The source clip peaks at -0.2 dBFS. Before this change the output peaks at -1.4 dBFS (sample and 4x true peak; the pipeline's own -1 dBTP safety lowered it 1.2 dB), 4.6 dB over the authored ceiling. With it the output peaks at -5.7 dBFS, 0.3 dB over the ceiling (the limiter holds a 4x estimate on the float mix, then AAC encoding adds a fraction of a dB). One fixture, one run each.audioStage.test.ts(a track added byappendAutoDetectedVideoAudioover an emptyaudios, chain reaches the mixer);htmlCompiler.test.ts(the compiled composition exposes the root chain); one test per forward (compileStage.test.ts,renderOrchestrator.masterBus.test.ts,distributed/plan.masterBus.test.ts), each red when its forward is removed; the engine master-bus tests now pass the chain beside the tracks. Dropping the forwarding inaudioStage.tsfails the stage test; dropping it from the compiled composition fails the compiler test; ignoring it in the mixer fails four engine tests.truepeakPR): the plainlimiteris level-following with no lookahead, and Level after is applied past its ceiling. Studio's description, panel text, knob label and summary, and thehyperframes-audioskill, no longer say nothing gets past it. They say a fast or inter-sample peak can pass and point to the true-peak limiter for a delivery limit. The copy lives in files from thetruepeakPR (audioFx.ts,audioFxCopy.ts), changed here. A copy test guards the wording; restoring the old text fails it.Follow-ups
chainLatencySamplesis where that fix would go.— Jerrai