fix: remove hidden audio gain in renders#362
Conversation
8ea22ae to
f367a36
Compare
f367a36 to
2ba7e4f
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Approving — the root cause analysis and fix are correct. A few follow-ups to address before/after merge, noted below.
Root cause / fix
Both defects match the issue reporter's measurements exactly:
- Hidden +2.6 dB gain.
DEFAULT_CONFIG.audioGain = 1.35was applied as a post-mix FFmpeg filter inaudioMixer.ts([mixed]volume=${masterOutputGain}[out]). The reporter measured a scalar 1.3517 with -47 dB residual — pure multiplication, no filter — which lines up with this code path precisely. Dropping the default to1is the minimal correct fix, andPRODUCER_AUDIO_GAINstays as an env override for anyone who wants the old boost. data-volume="0"not muting.element.volume || 1.0coerces0→1.0. Switching to??preserves explicit zero. Matches the reporter's "data-volume=0does not suppress the gain" observation.
I also checked for other gain paths (loudnorm, dynaudnorm, other volume= filters, audioGain references across the engine) — there are none, so this is the only hidden boost in the pipeline.
The new audioMixer.test.ts covers both regressions (volume=0 preserved in filter graph, [mixed]volume=1[out] post-mix) which is the right shape for this change.
Follow-ups
1. Two regression suites fail on the audio comparison (blocking merge):
font-variant-numeric— correlation-1(threshold 0.9)missing-host-comp-id— correlation0.8776(threshold 0.9)
Both suites' output/output.mp4 reference fixtures were last updated 2026-03-30, i.e. baselined with the old 1.35× gain. Two likely causes:
- If the old reference was hard-clipping at ±1.0 (peaks got pushed past 0 dBFS per the issue repro), envelope correlation against the new unclipped render legitimately drops — Pearson correlation on a clipped reference is not scalar-invariant. This probably explains the
0.8776onmissing-host-comp-id. - The
-1onfont-variant-numericsmells like a harness NaN edge case:computeBestCorrelationinitializesbest = -1and never updates if every lagged correlation produces NaN (e.g., a near-constant/silent envelope). Worth sanity-checking whether this suite is supposed to exercise audio at all, or whether the-1is a pre-existing latent bug inregression-harness.tsthat this PR happens to tickle.
Either way: please regenerate those two reference output.mp4 files on this branch before merging. If font-variant-numeric isn't meant to be an audio suite, consider lowering/removing its minAudioCorrelation in meta.json instead.
2. Unrelated Studio tsconfig change bundled in. The rootDir: ".." / @hyperframes/player path mapping is a workspace typecheck ergonomics fix, not part of the audio bug. The PR description calls this out honestly, but ideally it would be a separate PR so the audio fix stays independently revertible.
3. (Optional, non-blocking) Release note. This is a user-visible level change — anyone who'd been relying on the boosted default will see ~2.6 dB quieter output. Env override preserves the escape hatch, but a line in the release notes would help existing users.
Nice writeup in the PR description — the root-cause trace and the 1.35 ↔ +2.62 dB connection made this very easy to verify.
Merge activity
|
Summary
This fixes a render-time audio correctness bug where Hyperframes applied a hidden post-mix gain to every rendered output, boosting audio by about +2.6 dB and causing clipping on normally leveled sources.
It also fixes a related mute bug where
data-volume="0"was treated as falsy and silently converted back to full volume during audio track preparation.Additionally, this PR fixes the Studio workspace typecheck path for
@hyperframes/player, so local pre-commit/typecheck flows no longer depend on the Player package having been built first.Root Cause
The issue report measured a near-constant gain increase and suspected a hidden normalization step. After tracing the engine audio path, the root cause turned out to be explicit code, not FFmpeg behavior:
packages/engine/src/config.tsdefaultedaudioGainto1.35packages/engine/src/services/audioMixer.tsalways appended a post-mix FFmpeg filter:[mixed]volume=${masterOutputGain}[out]1.35That exactly matches the issue reporter's measured scalar boost.
While investigating the workaround, I also found a second correctness bug:
processCompositionAudio()usedelement.volume || 1.00to1.0data-volume="0"did not actually mute the track in rendered outputSeparately, the repo-level Studio typecheck could fail before any build step because:
packages/studio/src/player/components/Player.tsximports@hyperframes/playerpackages/player/package.jsonpoints TypeScript at builtdist/*outputs@hyperframes/playerduring pre-commit/typecheckWhat Changed
audioGainback to unity (1)element.volume || 1.0toelement.volume ?? 1.0@hyperframes/playerto the local workspace source and widenedrootDirso workspace typecheck succeeds without requiring a prior Player buildWhy These Changes Are Needed
This is not a UX preference issue; it is a correctness and API contract issue.
data-volumeas a direct 0-1 control.data-volume="0"must mean silence, not full-volume playback.Leaving the current behavior in place means:
0.75-ish scaling) to get unity outputTesting
Focused regression tests
Ran:
packages/engine/node_modules/.bin/vitest run packages/engine/src/config.test.ts packages/engine/src/services/audioMixer.test.tsResult:
10 passedThese tests specifically verify:
audioGainis1volume: 0staysvolume=0in the FFmpeg filter graph[mixed]volume=1[out])Broader package verification
Ran:
bun run --filter @hyperframes/engine testbun run --filter @hyperframes/engine buildpackages/engine/node_modules/.bin/vitest run packages/producer/src/services/renderOrchestrator.test.tsbun run --filter @hyperframes/producer typecheckbun run --filter @hyperframes/studio typecheckbunx oxlint packages/engine/src/config.ts packages/engine/src/config.test.ts packages/engine/src/services/audioMixer.ts packages/engine/src/services/audioMixer.test.ts packages/producer/src/services/renderOrchestrator.test.tsbunx oxfmt packages/engine/src/config.ts packages/engine/src/config.test.ts packages/engine/src/services/audioMixer.ts packages/engine/src/services/audioMixer.test.ts packages/producer/src/services/renderOrchestrator.test.ts packages/studio/tsconfig.jsonbunx lefthook run pre-commitResults:
309 passed)7 passed)0 warnings, 0 errors@hyperframes/playermodule-resolution blockerKnown Verification Limitation
There is no meaningful browser UI flow for this bug: the defect is in the engine/CLI audio render pipeline rather than an interactive browser surface. Because of that, verification was done at the renderer and test level rather than through an agent-browser flow.
User Impact
After this change:
data-volume="0"correctly mutes rendered audio@hyperframes/playerartifactsCloses #361.