Skip to content

fix(producer): an export over the peak ceiling loses only what it must - #7246

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/delivery-limit-adaptive
Nov 5, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/delivery-limit-adaptive

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

What

An export whose audio peaks pass the -1 dBTP delivery ceiling now loses only what it takes to get under it. Before, every peak over the ceiling was pulled down to -3 dBTP, 2 dB more than the rule asks, so a loud mix came out quieter on every transient.

Why

A report: a -13.3 LUFS source came out quieter, about -1.9 dB on transients and recovering in 100-150 ms. That is the delivery limiter (audioDeliveryLimit.ts, #6940): when the encoded mix's true peak passes -1 dBTP, it re-encodes the source through a true-peak limiter at -3 dBTP, a fixed 2 dB under the target because the limiter's 4x detector and the AAC encode can land over its ceiling. Its 80 ms release is the 100-150 ms recovery.

Measured with the CLI render of the audio-mux-parity fixture, its track replaced by a drum hit every 0.5 s at -13.4 LUFS with a true peak of +0.1 dBTP. A source like this must lose about 1.1 dB at its peaks to meet -1 dBTP; anything past that is the margin.

limiter export loudness export true peak at each hit
source -13.4 LUFS +0.1 dBTP
shipped: one pass at -3 dBTP -15.9 LUFS -2.9 dBTP -2.74 dB
one pass at -2 dBTP -15.1 LUFS -1.9 dBTP -1.86 dB
one pass at -1 dBTP, then the whole-mix cut -14.9 LUFS -1.5 dBTP -1.56 dB
this PR -14.6 LUFS -1.0 dBTP -1.30 dB

"At each hit" is the gain over the source in the first 30 ms of each hit; plain AAC of the same source, with no limiting, reads -0.4 to -0.5 dB there. The first three rows were measured on the devbox with the CLI 0.9.13 bundle and its ceiling edited; this PR's row on a second Linux box with this branch's CLI built from source.

How

  • The limiter runs at -1 dBTP first, the encode is measured, and only if it still lands over does the next pass lower the ceiling by what it measured over (plus 0.05 dB). At most three passes, each from the original mix, never below -3 dBTP; the whole-mix cut in constrainAacTruePeak stays behind it for what still gets over.
  • One pass (limit, encode, measure) is its own function, limitPass; the loop only picks the ceiling.
  • Passes write to two scratch files in turn, so a pass whose limit fails leaves the last good one, and the source is replaced with the file the delivered AAC was encoded from.
  • Same input, same output bytes: the limiter and the encoder are deterministic and the passes depend only on the measured peaks.

Time it adds

On a 10-minute mix over the ceiling: main's one pass at -3 dBTP took 116.9 s; this branch took 122.0 s, one pass that landed at -1.00 dBTP plus the extra peak read. A mix whose first encode still lands over takes one or two more passes of about the same length each.

Test plan

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

  • fails without the fix: limitDeliveryPeaks on a real mix > brings the encode under -1 dBTP losing no more than 0.2 dB past it, the same bytes every time (real limiter in the headless browser, real AAC encode, a mix at +0.1 dBTP) fails with main's one pass at -3 dBTP restored (expected -3 to be greater than or equal to -1.2) and passes here; two runs give the same bytes.

  • Unit: a mix whose encode lands under after the first pass is limited once at -1; each later pass lowers the ceiling by the measured excess, from the original mix, never past -3; a later pass that cannot run keeps the last good one.

  • 3 runs in a row at the earlier head: audioDeliveryLimit.test.ts, audioDeliveryLimit.integration.test.ts, audioTruePeak.test.ts (14 tests); audioPadTrim.test.ts under bun (45). The real-file test is listed in scripts/integration-tests.json, so CI's integration job runs it; test-classification.test.mjs passes. tsc, oxfmt --check and fallow audit clean at the pushed head.

  • plan.test.ts "keeps assembled loudness within 0.1 LUFS of the frame-fitted float delivery": its reference now goes through the same peak limit and whole-mix cut as delivery, from the track read as 16-bit, as the mixer reads it. Built from the float file instead, the first AAC encode of this white-noise fixture read +2.7 dBTP against the delivered mix's +3.8 (a 0.4 dB swing on the same audio rounded to 16 bits), so the limiter took different steps and the two landed 0.6 LU apart. The 16-bit reference is bit-identical to the mixer's PCM (md5 of the float samples).

  • At this head, on the devbox (the shared test box was down): plan.test.ts 3 runs (74 pass each); audioDeliveryLimit.test.ts, audioDeliveryLimit.integration.test.ts, audioTruePeak.test.ts, audioTruePeak.fakeFfmpeg.test.ts, audioStage.test.ts 3 runs (39 each); audioPadTrim.test.ts under bun 3 runs (45 each); the producer's unit lane (1196 pass, 1 skip) and integration lane (278 + 18 under vitest, the bun files all pass); oxfmt --check and the comment ratchet clean.

  • Not covered: a mix needing three passes on a real file (the unit tests drive that path), and the real-file test skips where no headless browser is installed, as the engine's browser tests do.

The delivery limiter cut every peak over -1 dBTP down to -3, a fixed 2 dB margin for its detector. It now limits at -1, measures the encode, and lowers the ceiling only by what that encode measured over.
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 94 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES · 3 blocker · 0 important · 0 nits · head e937e600

The pass logic is sound. Ceilings step down from -1 and stop at -3, and switching between the a/b scratch files keeps the last good limited source if a later pass fails. Renaming encodedFrom also keeps constrainAacTruePeak's whole-mix cut reading the source the AAC was actually encoded from. CI is red on three counts, all caused by this diff:

  • blocker: audioDeliveryLimit.integration.test.ts is not listed in packages/producer/scripts/integration-tests.json. Producer: unit tests and Producer: integration tests both fail on test-classification.test.mjs:75 ("add each to scripts/integration-tests.json"). Until it is listed, the test named on the fails without the fix: line never runs in CI, so nothing in CI protects against the regression.
  • blocker: Format and every Preflight (lint + format) fail on oxfmt --check, which flags audioDeliveryLimit.ts (the body says oxfmt is clean). The Windows render/tests and regression failures are downstream of that preflight. ↓ inline
  • blocker: Fallow audit fails: limitDeliveryPeaks has a cognitive complexity of 17, over the 15 threshold. ↓ inline
Scope

Read audioDeliveryLimit.ts end to end, its caller guardDeliveryPeaks in audioStage.ts, and both test files. No earlier reviews. CI run 42039904327: Format, Producer unit/integration and Fallow audit failed; CodeQL was still running.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES · 3 resolved, 0 open, 1 new · e937e60 → c1fbba8

  • blocker (new): registering the integration lane makes Producer: integration tests run, and two existing tests in src/services/distributed/plan.test.ts ("stitched mp4 plans") now fail. They fail the same way in both CI runs on this head (42042315743, 42042346040):
    • fits the audio length and peak… (plan.test.ts:1075): the stitched audio.m4a max_volume is now -1 and the test expects < -1. This assertion counted on the old 2 dB margin.
    • keeps assembled loudness within 0.1 LUFS of the frame-fitted float delivery (plan.test.ts:1287): the assembled file is 0.5 LUFS from the reference (limit is 0.1). The reference goes through constrainAacTruePeak only, while delivery now goes through the -1 dBTP limiter first. Build the reference through limitDeliveryPeaks and then constrainAacTruePeak, as guardDeliveryPeaks does, or explain why distributed assembly should differ from the frame-fitted delivery. Loosen the peak assertion to <= -1 only if -1.0 is the intended sample peak.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 1 resolved, 0 open, 0 new · c1fbba8 → 66654cc

CI note

The loudness reference now goes through the same steps as delivery: 16-bit read, float fit, limitDeliveryPeaks, then constrainAacTruePeak. The 16-bit step matches the mixer, whose prepared tracks are pcm_s16le (audioVolumeEnvelope.ts:13, audioMixer.ts:813). Producer: unit tests and Producer: integration tests pass, including both stitched mp4 plans tests. The remaining red has nothing to do with the code: Catalog payloads (ffmpeg-static download failed 3 times) and Test (runtime) (BtbN ffmpeg download returned 504), which also turns the Test aggregate red.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit d40d0b2 Nov 5, 2026
149 of 153 checks passed
@miguel-heygen
miguel-heygen deleted the fix/delivery-limit-adaptive branch November 5, 2026 16:37
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 6, 2026
…view

Adopts heygen-com#7250 (SVG stroke drawing is not a dead stretch), heygen-com#7248
(contract check reads a timeline registered in a local script), heygen-com#7252
(comma-list selectors hidden by authored CSS), heygen-com#7238 (snapshot and
preview show a clip on the render frame), heygen-com#7237 (stalled cloud upload
retries) and heygen-com#7251 (per-scene SVG id namespacing), with prerequisite
heygen-com#6829. Defers heygen-com#7246 (needs the delivery limiter from heygen-com#6940) and heygen-com#7232
(needs the WebKit decode-first pipeline), with conditions in the
ledger. Watermark advances to b108f85.

Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant