Repository navigation
perf(producer): local renders check the audio peak in parallel segments, as cloud plans do - #5942
Conversation
…ts, as cloud plans do measureTruePeakDbfs reads the file's own length and rate, so every caller gets the segmented scan.
Edit accuracy: accurate 271 (base branch 271), smooth 145 of thoseThe gate passes. Quarantined, measured but not gated (0) |
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
Reviewed at 3648018c. Approving.
One owner: measureTruePeakDbfs now probes the file with extractAudioMetadata (audioTruePeak.ts:72) and segments only when the sample rate is 48_000 and durationSeconds is finite (:76-79). Both callers now go through it the same way:
- the local
padOrTrimAudioToVideoFrameCount, ataudioPadTrim.ts:305-310; - the plan's
constrainAacTruePeak, at:452, which no longer runs its own probe.
scanTruePeak is still private, and the segments option is gone, so no caller can skip the decision.
Same digit, same bytes: the segment logic from #5925 is unchanged apart from where durationSeconds comes from:
- The cuts, the one-block lead-in, the
laterBlocksmerge, the whole-file fallback for the last segment and the peer abort are all as they were. - The value it returns feeds only
enforceAacTruePeak's pass decisions. So a digit-identical peak means the same corrections and identical output. - Taking the length from the probe rather than the target can only change the segment count, not the answer, because the last segment runs to the end of the file.
extractAudioMetadatacaches by path plusdev:ino:size:mtimeNs:ctimeNs(ffprobe.ts:344-347), and bypasses the cache when given a signal. So re-measuringcorrectedPathafter a correction pass always sees fresh metadata.
Failure paths:
- Probe failure: if the new probe throws (no audio stream or an abort), it lands in
enforceAacTruePeak'scatch("failed to validate AAC true peak"), the same place a failed scan already went. - Unknown rate or length: a missing rate defaults to 44100 and an unknown length is NaN or 0, and all of these take the single pass. The new test covers NaN.
Tests:
- The new pad/trim test goes in through the local entry point and asserts the three
atrimfilters. The PR shows it failing with the production files reverted. - The real-ffmpeg cases now compare against a direct
ffmpeg … ebur128whole-file run instead of the removedsegmentsoption. They still cover a burst at each cut.
Nit
audioTruePeak.test.tsno longer proves that the real-ffmpeg cases took the segmented path. That now depends on the real probe reading the WAV as 48 kHz and at least 120 s long. If the probe ever misreported either,segmented === wholewould pass on the single pass. A one-line check thatextractAudioMetadatareports{ sampleRate: 48_000 }anddurationSeconds >= 120for a fixture would pin it.
Verdict: APPROVE
Reasoning: The segment decision now lives in one place, and both callers take it. The segmented peak is unchanged from #5925, which keeps the output byte-identical. The new local-path test fails without the change, and required CI is green at this head.
— Via
What
measureTruePeakDbfsnow decides on its own whether to scan in parallel segments: it reads the file's length and sample rate and, for 48 kHz audio long enough to split, runs the segmented scan that distributed plans already use. Every caller gets it, including localhyperframes render, which until now scanned the whole mix in one single-threaded pass on every peak check. The value is the same digit as the single pass, so the rendered file is byte-identical.Why
A local render with audio measures the mix's true peak at least once in the assemble step, and again after each loudness correction. Only the distributed plan passed the segment information, so local renders of long films ran the slow single pass up to four times. Moving the choice into the function means a future caller cannot miss it.
Related work
Refs #5925 (the segmented scan, used only by the distributed plan until now).
How
Callers of
measureTruePeakDbfson main, and what each does now:padOrTrimAudioToVideoFrameCount(local render, fromstages/assembleStage.ts): was one pass; now segmented for 48 kHz audio long enough to split.constrainAacTruePeak(distributed plan, fromdistributed/plan.ts): already segmented. It no longer probes the sample rate itself or passessegments; the function's own probe replaces that probe one for one. Segments are now cut from the probed file length instead of the target length; the count can only differ when the two sit on opposite sides of a segment boundary, and the peak is the same either way.audioTruePeaktest files.scanTruePeakis not exported. Files that are not 48 kHz, or whose length is unknown, keep the single pass. Thesegmentsoption is removed.Cost: one audio probe per measurement, also on short files that stay on the single pass. On a 17 s fixture the assemble step measured 735 ms before and 694 ms after (within noise).
Test plan
Unit tests added/updated: a new fake-ffmpeg test enters through
padOrTrimAudioToVideoFrameCountand fails on the single-pass path (expected [ 'ebur128=peak=true' ] to deeply equal [ …(3) ]with the production files reverted to main), passes with the change. Fake-ffmpeg tests 5/5 and real-ffmpeg tests 4/4, 3 runs each;audioPadTrim.test.ts38 pass; integration 2/2 (Linux).Manual testing performed (Linux, released CLI build vs this branch, one run each):
The four peak measurements on the 142 s film took 2.51 s before and 1.40 s after. Most of that film's assemble time is the loudness-correction re-encodes, which this PR does not touch.
Full
bun run producer:test:unit3 runs in a row, 61/61 files, including the ffprobe argv contract test (Linux).Typecheck, oxlint, oxfmt and fallow clean on the changed files (Linux).
Documentation updated: not applicable.
Comments follow CONTRIBUTING.md "Comments".