Skip to content

fix(engine): hundreds of effects in one audio group mix in time, and a mix timeout is named - #6101

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/group-submix-no-pad
Oct 23, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/group-submix-no-pad

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

A group (data-audio-group) of hundreds of short effects now mixes well inside the ffmpeg deadline, and when that deadline does kill a mix the error says it timed out.

Why

An export of a 10-minute film with ~680 effects in one audio group failed with audio_processing_failed ... FFmpeg exited with code 255 after about 5.5 minutes. Two defects:

  • The group sub-mix padded every member to the full film length before summing, so its cost grew with members x film length. The ungrouped mix already sums first and pads once.
  • The kill at ffmpegProcessTimeout was reported as a bare exit code with the stream-mapping lines of stderr. The failure reason was already ffmpeg_timeout, but the detail text never said so, because the audio mixer formatted the error itself instead of using describeFfmpegFailure as the other ffmpeg callers do.

Related work

Follows the group sub-mix gain work that introduced normalize=0 there. No overlapping PR.

How

  • mixGroupMembers: when amix supports normalize=0, members are no longer padded one by one; the sum is padded and trimmed once to the film length, as the main mix does. Members that end early add silence either way, so the output is the same. Old ffmpeg builds without normalize keep the per-member padding, because their input-count compensation needs it.
  • ffmpegFailure now builds its detail with describeFfmpegFailure, so a deadline kill reads "FFmpeg timed out after N ms (ffmpegProcessTimeout; ... FFMPEG_PROCESS_TIMEOUT_MS)" ahead of the stderr tail. Its callers pass the timeout they already hold.

Measured end to end through processCompositionAudio on the same machine, 680 members of 0.4 s spread over a 600 s film, one group: 482 s before, 107 s after (the deadline is 300 s by default). That time still includes preparing each member file; the group graph itself no longer scales with film length per member.

Test plan

  • audioMixer.test.ts: the group graph has one apad, after the sum (fails on main: adelay=...,apad on every member); a group sub-mix killed at the deadline reports "FFmpeg timed out after 300000 ms" with the env var named (fails on main: "FFmpeg exited with code 255").

  • src/services/audioMixer* 6 files, 109 tests pass, including the real-ffmpeg grouped-versus-flat level checks that guard the unity sum. Typecheck, lint and format hooks pass.

  • 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

@somanshreddy somanshreddy 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.

Reviewed at 9f17a3b7. No blockers.

What I checked:

  • The sum is unchanged. With normalize=0, amix is a plain sum, and dropout_transition=0 means a member that ends early adds nothing either way. So padding each [aN] to the film length and trimming it there was pure cost. The new graph pads and trims the sum once, which is the same apad,asetpts=N/SR/TB,atrim=0:T tail the main mix already uses (audioMixer.ts:834).
    • A member that runs past totalDuration (late start, or an FX tail) used to be cut per member; it is now cut by the final atrim. The output is the same.
    • The old-ffmpeg branch without normalize still pads per member (padMembers = !useNormalize). That is correct, because its divide-by-N compensation needs all N inputs active for the whole span.
  • The default is safe for other callers. buildTrackInputFilter's new padToDuration = true default leaves every other caller unchanged.
  • The timeout message reaches every mixer call site. ffmpegFailure now uses describeFfmpegFailure(result, timeoutMs), and each call site passes the timeout it actually ran with: extract, silence, the main mix, the group sub-mix and mixThroughFloatSum. The reason classification still reads stderr as before.
  • The tests pin both fixes. One asserts exactly one apad, placed after the sum. The other asserts that a deadline kill on the group sub-mix reports ffmpeg_timeout with "timed out after 300000 ms" and names the env var.

Non-blocking:

  1. The Comments ratchet is red. audioMixer.ts went from 235/1703 to 237/1712 comment lines because the two new lines on the normalize=0 branch push its comment share up. It's not a ruleset-required check, but cutting one line (or folding it into the existing comment on the else branch) would turn it green.
  2. Per the body, the 107 s that remain are mostly per-member preparation, which is one ffmpeg spawn per effect. That's fine for now. If groups of 1k+ effects become normal, batching member preparation is the next lever.

— Somu

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 190 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 (0)

@somanshreddy somanshreddy 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.

Re-reviewed the delta 9f17a3b7..2cc247a2: one commit, audioMixer.ts +2/-3. No blockers.

The two-line comment became the named value padUnitySumOnceToFilm. The resulting amix ... normalize=0[mixed];[mixed]apad,asetpts=N/SR/TB,atrim=0:T[out] string is byte-identical to the approved head, and the per-member branch is untouched. This also addresses my earlier Comments-ratchet nit. Everything in my review at 9f17a3b7 still holds.

— Somu

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 23, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 1accb70 Oct 23, 2026
75 checks passed
@miguel-heygen
miguel-heygen deleted the fix/group-submix-no-pad branch October 23, 2026 02:15
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.

2 participants