fix(engine): stop passing -avoid_negative_ts make_zero in muxVideoWithAudio

[miga-heygen]

Summary

Drops the unconditional -avoid_negative_ts make_zero flag from muxVideoWithAudio. ffmpeg's default (auto) already resolves to disabled for mp4/mov muxers (AVFMT_TS_NEGATIVE), so the explicit override was unnecessary — and on the dominant audio-copy path it actively broke things: it discarded the AAC priming edit list the sidecar encode created, shifting the video start_time forward by one AAC frame (~21ms) and inserting an empty video edit at t=0 that edit-list-honoring players (QuickTime/Safari) showed as a black first frame.

The preserveAudioPrimingEditList option that partially guarded against this is deprecated (the flag is never needed now) and the distributed assembler no longer threads it.

Takeover of #3488 by @mincua — original work preserved, re-committed with signed commits per org policy.

Closes #3487
Supersedes #3488

— Miga

[james-russo-rames-d-jusso]

🟢 Clean, well-scoped fix — merge is Miguel's call. Drops the unconditional -avoid_negative_ts make_zero from muxVideoWithAudio on the audio-copy path, which was destroying the AAC priming edit list and producing the 21ms A/V shift + black first frame on QuickTime/Safari (#3487). Removal is correctly bounded to the delivered-mp4/mov mux; unrelated encoder / stream / -ss uses of the flag are left alone. Deprecation of preserveAudioPrimingEditList is done cleanly (typed as @deprecated no-op, callers stripped, ignored-option test added).

Scope of removal — correctly bounded
• packages/engine/src/services/chunkEncoder.ts:702-712 — the flag is gone from muxVideoWithAudio, and the new comment explicitly calls out that buildEncoderArgs (chunkEncoder.ts:454) still emits it deliberately because those chunks are consumed as raw elementary output, not delivered mp4/mov. That distinction is the right one to preserve.
• Verified no other mux/delivery path drops through here: streamingEncoder.ts:431 still keeps the flag as "belt-and-suspenders against negative DTS at stream start", but that's the fresh video-only image2pipe encode — no audio track, no priming edit list to destroy — so the specific #3487 symptom doesn't apply. Consistent-across-mp4 argument would suggest a follow-up ticket, but explicitly out-of-scope for this PR.
• skills/media-use/scripts/transcript-cut.mjs keeps it for -ss -c copy transcript cuts, which is precisely where the issue body says make_zero legitimately belongs. Correct.

ffmpeg-internals claim — corroborated
• The auto → disabled for mp4/mov via AVFMT_TS_NEGATIVE claim rests on libavformat/mux.c + libavformat/movenc.c, and #3487's body includes a reproducible ffprobe diff (start_time=0.020996 with the flag vs 0.000000 without) plus a linked upstream trac ticket (10380). Miga's own production evidence — 121/121 finals probing at video start_time=0.021 — is the strongest single data point. I did not independently open the ffmpeg source; trust-verified via issue evidence.

Deprecation hygiene
• chunkEncoder.ts:81-88 — field kept on the interface, @deprecated docstring names the removal timeline ("removed in a future major"). Good middle path: no breaking source-compat change today, no half-shim logic path.
• assemble.ts:305-341 — normalizedAudio: { path, preserveAudioPrimingEditList } correctly collapsed to normalizedAudioPath: string | null. The whole reason for the wrapper object was that dropped field; collapse is clean.
• assembleStage.ts — same, one-line drop from the option bag.
• Two remaining references at HEAD are the interface field itself + the deprecation test asserting it's ignored. No live callers thread it.

Test coverage — strong
• chunkEncoder.test.ts:394-411 — happy-path exact-args assertion now positively pins the absence of -avoid_negative_ts (whole args array literal, not just a .not.toContain).
• chunkEncoder.test.ts:425-448 — new #3487 regression test on the M4A-sidecar copy path, negative-asserts the flag.
• chunkEncoder.test.ts:451-471 — deprecated-option test passes preserveAudioPrimingEditList: false (which, under the old code, would have pushed the flag) and asserts it's absent — this is a stronger deprecation test than passing true would have been.
• chunkEncoder.test.ts:582-585 — AAC re-encode path also negative-asserts.
• chunkEncoder.test.ts:625-639 — container-matrix loop asserts across every container the muxer supports. Nice.

What I didn't verify
• Did not open the ffmpeg source (libavformat/movenc.c, libavformat/mux.c) or run the ffprobe reproduction locally — trusting issue #3487's evidence.
• Did not run the tests locally.
• Did not deep-dive streamingEncoder.ts — treated it as out-of-scope (video-only fresh encode, no priming edit list at risk).

CI note (non-blocker)
The Test job under CI shows FAILURE, but its only step is echo "::error::Producer unit/integration tests did not succeed." && exit 1 — the aggregator collapsed because Detect changes for that workflow was CANCELLED, causing all producer test shards to show SKIPPED. This looks like spurious infra fallout, not a real regression. The meaningful signal — 9 regression-shards, Tests on windows-latest, Render on windows-latest — was still in-flight at review time. Worth a re-run to see them green.

Merge stance
Merge is Miguel's call — HF PR authored by @miga-heygen, no OG-route from my side. From where I sit: fix is correct, well-scoped, well-explained, and well-tested. Ready to ship once CI settles.

— Review by Rames D Jusso

[jrusso1020]

APPROVED. Stamp basis: Miguel's explicit request in the HyperFrames channel (trusted stamper), which is also what clears the standing "don't stamp a bot-authored PR on its own initiative" rule for a miga-heygen PR. Rames D Jusso's pass is on record; the verification below is my own and independent of it.

I reproduced #3487 rather than reading the comment

The PR's central claim is that -avoid_negative_ts make_zero shifts copied video forward by one AAC frame. I ran it on ffmpeg n8.0.1-52, muxing a +0.5s-offset h264 stream against an AAC sidecar with -c:v copy -c:a copy -movflags +faststart:

output	video start_time
no flag (ffmpeg auto)	0.500000
-avoid_negative_ts make_zero	0.522982

+22.982ms, which is 1024 samples at 44.1kHz (23.22ms) — my sidecar's rate. At the 48kHz this pipeline actually encodes, the same frame is 21.3ms, matching the "~21ms" in the issue. So the mechanism is confirmed at source, and make_zero is demonstrably the thing introducing the offset, not merely correlated with it.

The webm path is a verified no-op, not an assumed one

The removed line was if (!copiesContainerizedAac) …, and copiesContainerizedAac requires !isWebm — so webm previously got make_zero unconditionally, and the new comment justifies removal only via mp4/mov's AVFMT_TS_NEGATIVE. That justification does not extend to matroska, so I checked instead of assuming. Muxing a VP8 stream at the same +0.5s offset:

webm_auto.webm   video 0.500000  audio -0.007000  first video pkt pts 0.500000
webm_zero.webm   video 0.500000  audio -0.007000  first video pkt pts 0.500000

Byte-for-byte equivalent timestamps. Removing the flag changes nothing on the webm path, so the widened scope is safe. Worth noting in case a future reader tries to extend the comment's mp4/mov reasoning to matroska — it happens to land in the same place, but not for the stated reason.

Scope and blast radius

buildEncoderArgs correctly keeps the flag (chunkEncoder.ts:454) — those chunks are consumed as raw elementary output. I checked the one case where a chunk-encoded file reaches delivery without a mux: in assemble.ts, muxOutputPath falls back to postConcatPath when there is no audio. That path keeps make_zero, but with no audio there is no priming edit list, so there is nothing to shift against. No exposure.

The preserveAudioPrimingEditList deprecation is done properly: @deprecated no-op with the reason and a removal note, both call sites stripped (assembleStage.ts, assemble.ts), and assemble.ts collapses the now-pointless {path, preserveAudioPrimingEditList} object back to a bare path rather than leaving a dead field. Tests invert cleanly — the old "keeps negative-timestamp repair" case becomes an explicit not.toContain regression test for #3487, plus a container-matrix assertion.

Non-blocking

• chunkEncoder.ts:224 still reads "The unconditional -avoid_negative_ts make_zero near the bottom of this function covers the mux level". The referent (line 454) still exists so it is not stale, but "covers the mux level" was already loose and is now actively misleading, since the mux level is precisely where the flag no longer is. One-line touch-up whenever this file is next open.

CI

Test is red at ce875f79, and I checked whether that is real: Detect changes was cancelled, which skipped all 22 shards, and the aggregator collapses on skipped dependencies. No individual shard carries a failure conclusion — 27 success, 22 skipped, 2 cancelled. A re-run should surface it green. Not gating on it either way.

— Rames