fix(engine): stop passing -avoid_negative_ts make_zero in muxVideoWithAudio - #3488
Open
mincua wants to merge 1 commit into
Open
fix(engine): stop passing -avoid_negative_ts make_zero in muxVideoWithAudio#3488mincua wants to merge 1 commit into
mincua wants to merge 1 commit into
Conversation
`muxVideoWithAudio` passed `-avoid_negative_ts make_zero` unless the caller
set `preserveAudioPrimingEditList`. In practice the dominant path is an AAC
sidecar copied into mp4, where that flag is actively harmful: ffmpeg's
default is `auto`, which the mp4/mov muxers (AVFMT_TS_NEGATIVE) already
resolve to `disabled`. Forcing `make_zero` overrides the correct default,
discards the priming edit list the sidecar encode created, shifts the video
start_time forward by one AAC frame and writes an empty video edit at t=0 —
which edit-list-honoring players (QuickTime/Safari) render as a black first
frame.
Verified with ffprobe on a copy mux of a 30fps h264 mp4 and an AAC sidecar:
with `make_zero` video start_time 0.066000, elst: [media time -1,
dur 5940] + [media time 6000, dur 180000]
audio start_time 0.042993, elst: [media time -1, ...]
without (this fix) video start_time 0.000000, elst: [media time 6000,
dur 180000]
audio start_time 0.000000, elst: [media time 1024, ...]
The empty leading edit and the offset both disappear, and the audio keeps
its 1024-sample priming edit.
The flag is now never passed for a mux, in any mode. `preserveAudioPrimingEditList`
is part of the exported engine API, so it stays on `MuxVideoWithAudioOptions`
as `@deprecated` and no-op rather than being removed; the two internal callers
that set it (`assembleStage`, distributed `assemble`) drop it.
`buildEncoderArgs` and `streamingEncoder` still pass the flag for video-only
output and are deliberately left alone — those chunks are consumed as
intermediates, not as a delivered mp4/mov.
Fixes heygen-com#3487
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3487
What
muxVideoWithAudiono longer passes-avoid_negative_ts make_zero— in any mode, for any container. One flag removed from the mux path; thepreserveAudioPrimingEditListoption that used to gate it is deprecated and ignored rather than deleted.Why
The flag is actively harmful on the dominant mux path (an AAC sidecar copied into mp4), and it is not needed on any of the others.
The defect. ffmpeg's default is
-avoid_negative_ts auto, i.e. "enabled when required by the target format". The mp4/mov muxers setAVFMT_TS_NEGATIVE(libavformat/movenc.c), soautoalready resolves to disabled for the containers this function writes — mp4 expresses an offset start natively through an edit list. Passingmake_zeroexplicitly overrides that deliberate per-format opt-out. On the copy path it discards the AAC priming edit list the sidecar encode created, shifts the copied video forward by one AAC frame (~21 ms), and makes the muxer write an empty video edit at t=0 (media time: -1). Players and thumbnailers that honor edit lists — QuickTime, Safari, WhatsApp previews,requestVideoFrameCallback— find no video sample at t=0 and render a black first frame.The regression chain. The flag was added in v0.4.45 as belt-and-suspenders against negative DTS, a problem the same commit had already fixed at the source with
-bf 0on the encoder. #1615 (v0.6.116) diagnosed the black-first-frame symptom and fixed it by dropping a second AAC encode. Then:config?.preserveAudioPrimingEditList === true, so the copy path — dominant, because the mixer already pads every input to the exact composition duration — ranmake_zeroagain. Harmless at the time: the mixer wrote raw ADTSaudio.aac, which has no edit list to destroy.audio.aactoaudio.m4a, precisely so the container would carry the priming edit list — but the v0.7.72 gate was not revisited. The callers still passedpreserveAudioPrimingEditList: operation !== "copy", somake_zeroran on virtually every render and destroyed the very edit list fix(producer): mix audio into a container that can record encoder delay #3200 had introduced.Why omitting the flag is correct rather than merely convenient. For mp4/mov, omitting it is the flag's own documented behavior —
auto→ disabled — so this restores ffmpeg's per-format default instead of imposing a new policy. Genuine leading negative DTS is still handled: the priming interval is carried by the audio track's edit list, and the encoder keeps-bf 0, which removes negative DTS at the source.make_zeroalso isn't idempotent — every re-encode pass through it stacks another priming interval (our delivery upscale turned 21 ms into 42 ms, a full frame at 24 fps). And the scenario it was guarding against does not materialize: muxing B-frame video with real leading negative DTS into mp4 comes out correct with the default and broken withmake_zero.How
packages/engine/src/services/chunkEncoder.ts— drop thecopiesContainerizedAaccondition and theargs.push("-avoid_negative_ts", "make_zero")it guarded. Replaced by a comment recording why the flag must not be reintroduced here, so the next person reading the file doesn't re-derive it as a missing safety net.MuxVideoWithAudioOptions.preserveAudioPrimingEditListis part of the exported engine API, so it stays, marked@deprecatedand documented as a no-op, rather than being removed. No breaking change for external callers; removal can happen in a future major.packages/producer/.../render/stages/assembleStage.tsandpackages/producer/.../distributed/assemble.ts— the two internal callers stop passing the option. Indistributed/assemble.tsthenormalizedAudioobject existed only to carry that boolean alongside the path, so it collapses back to a plainnormalizedAudioPath: string | null."keeps negative-timestamp repair for an M4A without a known priming edit list", asserting the flag is passed) is inverted into"never repairs negative timestamps for an M4A sidecar (regression #3487)". Anot.toContain("-avoid_negative_ts")assertion is added to the re-encode case and to the per-container loop, so the flag can't come back for any container without a test failing.Deliberately left alone:
buildEncoderArgsandstreamingEncoderstill pass-avoid_negative_ts make_zerofor video-only output. Those chunks are intermediates consumed by the concat/mux stages, not delivered mp4/mov, they carry no audio priming edit list to destroy, and the flag's original negative-DTS rationale still applies there. Changing them is out of scope for this fix and would need its own evidence.Test plan
1. Caller-shaped ffmpeg repro — a 30 fps h264 mp4 plus a 48 kHz AAC
.m4asidecar, muxed with exactly the argvmuxVideoWithAudiobuilds, with and without the flag.ffprobe:make_zero(before)start_time0.0210290.000000elstmedia time: -1, dur 323+media time: 0, dur 30720media time: 0, dur 30720elstmedia time: 0(priming trim destroyed)media time: 1024(priming preserved)duration2.0213332.000000The empty leading video edit and the offset both disappear, and the audio keeps its 1024-sample priming edit.
2. Minimal repro from the issue (24 fps, both 44.1 kHz and 48 kHz) —
start_time0.023031/0.020996with the flag,0.000000without; themedia time: -1empty edit is present in each broken case and absent in each fixed one.3. Full render through the fixed pipeline — a real 24 fps mp4 with audio out of the producer: video
start_time=0.000000, audiostart_time=0.000000, one edit per track (media time: 0), no empty edit.4. Unit + static checks (ffmpeg 8.1.2, bun 1.3.14, macOS):
MuxVideoWithAudioOptions.Investigated, written, and tested with Claude Code (Claude Fable 5).