Repository navigation
fix(audio): master bus follow-ups: a scene renders with its own chain; long mixes render in windows - #5474
Conversation
…s own root The standalone wrapper for a scene file reused the project index's root attributes, so the index's data-fx-chain became the master bus. The wrapper now carries the scene root's own chain, or none when the scene declares none, as the audio-effects reference describes. — Jerrai
…g the file whole applyAudioFxChain read the whole input WAV into memory, which Node refuses past 2 GiB (a stereo float mix beyond about 93 minutes), and the browser page then held the track twice. Inputs above 512 MiB of samples are now read, rendered and written in 5 minute windows, each with 10 s of preceding and 1 s of following audio rendered and discarded so stateful effects arrive warm. The output header is patched at the end. Inputs below the threshold keep the existing whole-file path. Parameter automation is clocked from the file start and is not windowed. An input past 2 GiB that carries automation now fails with an AudioFxRenderError naming that, instead of Node's raw file-size error. — Jerrai
Edit accuracy: accurate 2061 (base branch 2061), smooth 1604 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 b605475f. The scene change is right. The windowed render is the right shape, but it changes the sound of mixes that main renders correctly today, and it silently drops audio past 4 GiB.
Blocking
- Every window restarts modulated effects from time zero.
- Where:
audioFxRender.ts:424→core/stubs/audio-fx-runtime-entry.ts:103. Each window renders in a fresh page and callsbuildFxChain(ctx, chain)with the defaultelapsed = 0. - Effect: chorus and phaser LFOs and the pitch-shift worklet's phase start over at each window's read start instead of continuing from the file's clock, so the output jumps at every seam.
- Measured, whole vs windowed at default limits:
- chorus at 0.25 Hz differs by up to 0.40 at the 300 s seam, on a signal peaking at about 0.5;
- pitch shift +3 st differs by up to 0.44.
- Chorus at the default 1 Hz happens to line up with the seam, and the "matches the whole render" test uses only a limiter, so CI can't see it.
- Who hits it: anything over the 512 MiB threshold, which is a master or group bus over about 23 min or a stereo track over about 46 min. Main renders those whole, correctly.
- Fix: pass
readFrom / sampleRateinto the page and on tobuildFxChain(ctx, chain, elapsed), which preview already uses, and seed the pitch-shift phase from it. Add a chorus at a rate that doesn't divide the window to the windowed-vs-whole test.
- Where:
- Audio past 4 GiB is dropped with no error.
- Where:
audioFxRender.ts:121-122. - Cause: ffmpeg leaves the
datasize at0xFFFFFFFFwhen a float WAV passes 4 GiB, andMath.min(dataSize, fileSize - dataBody)then keeps only 4 GiB. The comment on line 121 says this case reads to the end; the code doesn't. - Effect: a stereo float mix past about 3 h 06 min loses its end silently, where main fails loudly.
- Fix: treat
0xFFFFFFFF(and0) as "to the end of the file".
- Where:
Should fix
- The fixed 10 s pre-roll (
:340) is shorter than some effects remember.- A 5 s delay at 0.95 feedback differs from the whole render by 0.26 at the seam.
- A compressor with a 9 s release and a hit 12 s before the seam jumps about 8 dB.
- Fix: size the pre-roll from the chain, or render whole when it can't cover the chain's memory.
- Tests that stay green when the code is broken. No test fails if the pre-roll is removed or the last window's tail is dropped. Adding a reverb and a slow-release compressor to the windowed-vs-whole test, plus an output-length check, would catch both.
Minor
:507-509: past 2 GiB with an unreadable header, the error blames automation even when there is none.:406-408: if opening the output fails, the input fd is never closed.standaloneEntry.ts:30: when the index's top-level root is itself the scene's mount, the function returns before the new code, so the index's chain still wins.- Nit: the doc comments at
audioFxRender.ts:323and:552run five lines with their delimiters; a line or two would carry each.
Checked and fine
- The scene-alone change. One reader picks the master chain for render, preview and Studio, so the chain is never applied twice, and no other root attribute leaks.
- Exactly one window, one frame over a window, input shorter than the lookahead, a peak right on a seam, the volume envelope across windows, abort, and a browser crash between windows all match or fail cleanly.
standaloneEntry.test.ts8/8 andaudioFxRender.test.ts31/31, none skipped, with ffmpeg and a browser. The scene tests and the windowing test fail with their fix reverted.
…er chain When the index's top-level composition is itself the scene's mount, the standalone extraction returned before the scene's chain was applied, so the index's chain still won. — Jerrai
…up for the whole chain - Pass each window's start time into the page and on to buildFxChain, and seed the pitch-shift worklet's tap phase from it, so chorus, phaser and pitch shift continue across a window seam instead of restarting from time zero. - Size the pre-roll from the chain (delay repeats, reverb room, dynamics release, to -60 dB), never below the configured floor and never above a cap. - Read a WAV data size of 0 or 0xFFFFFFFF as running to the end of the file, so audio past 4 GiB is no longer dropped. - Close the input descriptor when the output cannot be opened, and name the real cause when an input over 2 GiB cannot be streamed. - Cover reverb, a slow-release compressor, chorus and pitch shift in the windowed-vs-whole test, with an output-length check. — Jerrai
… seams against a hard chain Windows that begin off a 128-frame block see worklet blocks shifted against a whole render, which left spikes in the tail after the last input sample. Align the window start, and make the windowed-vs-whole test cover modulation, delay, reverb and a slow-release compressor on windows whose start is not zero, failing above 0.001 where it previously tolerated 0.02. — Jerrai
|
Thanks for the careful review. All items are addressed at
One addition: windows now start on a 128-frame render quantum. Off-quantum starts left spikes up to ~0.1 in the tail after the last input sample. — Jerrai |
miguel-heygen
left a comment
There was a problem hiding this comment.
Request changes at 64c303cd, for one red required check only. Both earlier blockers are fixed, and the windowed render now tracks the whole render closely.
Blocking
Tests on windows-latestis red. Itsengine-clijob fails "closes the input when the output cannot be opened" (audioFxRender.test.ts:888-894). The test counts open handles through/proc/self/fd, which exists only on Linux, and reads it outside its try. It is red on any Mac with a cached browser too. Fix: skip it off Linux, or assert through the existingnode:fsmock.
Fixed and checked
- Windows follow the file's clock.
elapsedreachesbuildFxChain, and the pitch-shift phase is seeded from it.- Measured whole vs windowed: a phaser at 0.37 Hz, a 333 ms delay at 0.6 feedback and a truepeak limiter match to 3e-6; pitch shift -7 st at 44.1 kHz matches to 1e-7; chorus at an odd rate matches to 3e-4.
- Output lengths are equal, and every window starts on a 128-frame quantum.
- A
0xFFFFFFFFdata size reads to the end of the file. - The warm-up is sized from the chain.
- The scene-alone fix holds when the index root is itself the mount.
- Reverts are caught. Each of these fixes, reverted on its own, fails its test: the window clock, the pitch-shift seed, the chorus clock, the warm-up, the alignment, the >4 GiB read, the handle leak and the scene change.
Should fix (here or next)
- Bitcrush still restarts its hold counter in every window (
audioFxWorklets.ts:193,:211,:218).- With windows now aligned to 128 frames, a hold that doesn't divide 128 drifts at each seam: hold 3 differs by 0.06 to 0.125.
- Fix: seed the counter from
__hfElapsedas pitch shift does:holds[ch] = Math.round(elapsed * sampleRate) % step. With that change applied the difference was 0.
- Truepeak drops the last ~7 ms of its input (
audioFxTruePeak.ts:144,if (!i || !i.length) return true;).- This predates the PR. Once the source ends, the limiter stops draining its lookahead line, so the whole render goes silent for the lookahead. A windowed render can keep that audio, depending on where the windows fall.
- It cuts the end of every clip through a truepeak, which is the usual master limiter. Pitch shift has the same pattern at
:286. - Fix: treat an empty input as silence and keep processing.
- Hitting the warm-up cap is silent (
audioFxRender.ts:426-429). A 5 s delay at 0.95 feedback needs about 675 s, past the 300 s cap, so repeats are cut at each seam with no log. Fix: warn when the estimate exceeds the cap.
Minor
audioFxWarmup.tsrepeats the delay and reverb memory formulas thataudioFxTail.ts:59-73already owns. One owner would keep them from drifting.- A declared data size of 0 means "to the end" in
resolveDataBytesbut "empty" inreadWavChunks.
…heck without /proc - Pad the render source with the limiter's lookahead of silence so the last samples of a clip leave the lookahead line instead of being dropped. - Seed the bitcrush hold counter from the window's start so holds that do not divide the render quantum stay in step across seams, and cover bitcrush in the windowed-vs-whole test. - Check that the input descriptor is closed through the fs mock instead of /proc/self/fd, which only exists on Linux. — Jerrai
The earlier hold of 3 divided both window starts, so an unseeded counter passed. — Jerrai
…ro data size alike - Warn when a chain's memory exceeds the pre-roll cap, so cut repeats at window seams are not silent. - Export the delay and reverb tail formulas from audio-fx-tail and use them for the window warm-up too. - Both WAV readers now treat a declared data size of 0 or 0xFFFFFFFF as the rest of the file. — Jerrai
|
Thanks for the thorough second pass. Everything is addressed at
— Jerrai |
miguel-heygen
left a comment
There was a problem hiding this comment.
Approve at f2e25617. All six items from the last round are fixed, every required check is green, and each fix's test fails when that fix is reverted.
Checked
- Windows test: the handle-close test now goes through the
node:fsmock. The settings force the windowed path, so the test fails as soon as the input handle leaks. - Windowed vs whole, measured with a real headless browser:
- A hard chain (two limiters with bitcrush, chorus, pitch shift, compressor, delay, reverb and gate between them) matches to 1.2e-4, with equal lengths.
- Bitcrush holds of 3, 64 and 250 at 44.1 and 48 kHz, mono and stereo, match exactly. Reverting the seed gives 0.19.
- Limiter padding: a short non-windowed clip through a truepeak limiter is the same length as on main, and a click lands on the same sample. Everything before the final lookahead is bit-identical, so there's no added delay; the only change is that the end now comes through. The same holds with a mid-chain limiter and with two limiters. A 50-sample clip through two limiters keeps all 50 samples; main returns silence.
- Shared tail formulas: unchanged for every former caller across the allowed ranges, so no tail or pre-roll got shorter.
- Data size: both readers read 0 and
0xFFFFFFFFto the end of the file. - Cap warning: fires once per windowed track, never on the whole-file path.
Follow-up (not blocking)
- The pitch shifter drops about the last 90 ms of a clip and ends on a click. This is on main too. It is the same cause the padding comment names: once the input stops, the worklet goes quiet. Padding by the full
frames + tail + latencyataudio-fx-runtime-entry.ts:108fixes it; tried that way, limiter output stayed bit-identical and both engine test files passed. Whole and windowed renders cut at the same point, so the seam tests can't catch this. - Bitcrush seed in preview (
audioFxWorklets.ts:213,:195): when preview builds the chain partway into a clip, the counter starts mid-hold butheldstarts at 0, giving up tosamples-1frames of silence on play or seek (about 5 ms at 250). Renders are unaffected. Fix: seedheld[ch]from the first input sample, quantised. - A preset wrapping a truepeak limiter shifts its dry half early, by about the lookahead (
audioFxTail.ts:128with the trim in the runtime entry). It predates this PR, and no built-in preset does this. audioFxRender.test.ts:1136says the default lookahead is 7 ms; it is 3 ms. Asserting on the last 160 frames would give the revert check more margin.audioFxRender.test.ts:1005-1007still skips the last 10 ms, for a reason this PR removed. Comparing every sample would cover the windowed end.- A data size of 0 followed by another chunk would now read that chunk as audio. Nothing here writes that layout.
What changed and why
Follow-ups to #5420, from the two non-blocking notes in @miguel-heygen's review.
1. A scene rendered on its own uses its own master chain
The audio-effects reference says a sub-composition file rendered on its own is the top-level root, so its
data-fx-chainis the master bus. The code did not do that: the standalone wrapper built for a scene file reused the project index's root attributes, so the index's chain became the master bus.The wrapper now carries the scene root's own
data-fx-chain, or none when the scene declares none. Studio preview of a scene alone goes through the same function and gets the same fix. No docs change was needed, since the docs already described this behavior.2. Master-bus exports past about 93 minutes
The master pass read the whole mixed WAV with
readFileSync, which Node refuses past 2 GiB ("File size is greater than 2 GiB"). A stereo 48 kHz float mix reaches that at about 93 minutes. The browser page also held the whole track twice.applyAudioFxChainnow reads, renders and writes inputs above 512 MiB of samples in 5 minute windows:Parameter automation is clocked from the start of the file and is not windowed. An input past 2 GiB that carries automation now fails with an
AudioFxRenderErrorthat says so, instead of Node's raw file-size error. The master bus has no automation, so it is not affected.writeWavwas split into a header builder and a sample encoder so the windowed writer shares them.writeWavoutput is unchanged.Tests
standaloneEntry.test.ts: the wrapper carries the scene's own chain, and drops the index's chain when the scene declares none.audioFxRender.test.ts:readWavthrows the 2 GiB error while the header read succeeds;readFileSyncon the input, and matches the whole-file render;Mutations, each turns its test red:
data-fx-chain: both wrapper tests fail;Changed test files only:
standaloneEntry.test.ts(8 tests),htmlBundler.test.ts(with it, 191),audioFxRender.test.ts(31). The test-reachability check and the comment ratchet pass.Measured
Not exercised
processCompositionAudio. It was exercised throughapplyAudioFxChaindirectly; the master pass uses the default limits.Separate finding, not changed here
On the existing whole-file path, a lookahead limiter's last few milliseconds can come back as zeros depending on the input's length. Measured on unmodified
applyAudioFxChainwith a stereo 300 Hz tone through the true-peak limiter: 52800 frames gives 0 trailing zero samples, 100800 gives 192, 67200 gives 320, 48000 gives 320, 211200 gives 0. The comparison test above leaves the last 10 ms out for this reason. It looks like an issue in the offline render or the limiter worklet and deserves its own issue.— Jerrai