From a4bea527709a44809ae2140d87a41327cb1051b3 Mon Sep 17 00:00:00 2001 From: Alexandru Mincu Date: Tue, 25 Aug 2026 11:20:00 +0300 Subject: [PATCH] fix(engine): stop destroying the AAC priming edit list when muxing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 #3487 Co-Authored-By: Claude Fable 5 --- .../engine/src/services/chunkEncoder.test.ts | 17 +++++++---- packages/engine/src/services/chunkEncoder.ts | 28 +++++++++++++------ .../src/services/distributed/assemble.ts | 19 +++++-------- .../render/stages/assembleStage.test.ts | 2 +- .../services/render/stages/assembleStage.ts | 1 - 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/packages/engine/src/services/chunkEncoder.test.ts b/packages/engine/src/services/chunkEncoder.test.ts index b9471d2746..d8429474aa 100644 --- a/packages/engine/src/services/chunkEncoder.test.ts +++ b/packages/engine/src/services/chunkEncoder.test.ts @@ -402,8 +402,6 @@ describe("muxVideoWithAudio audio codec handling", () => { "copy", "-movflags", "+faststart", - "-avoid_negative_ts", - "make_zero", ...renderProvenanceArgs("/tmp/output.mp4"), "-r", "30", @@ -424,7 +422,7 @@ describe("muxVideoWithAudio audio codec handling", () => { }); }); - it("keeps negative-timestamp repair for an M4A without a known priming edit list", async () => { + it("never repairs negative timestamps for an M4A sidecar (regression #3487)", async () => { const { spawn, calls } = createSpawnSpy(); vi.resetModules(); vi.doMock("child_process", () => ({ spawn })); @@ -442,13 +440,15 @@ describe("muxVideoWithAudio audio codec handling", () => { await flushMuxCodecResolution(); expect(calls).toHaveLength(1); expect(calls[0]!.args).toContain("copy"); - expect(calls[0]!.args).toContain("-avoid_negative_ts"); + // `make_zero` would discard the sidecar's AAC priming edit list, shift + // the copied video forward ~21ms and leave an empty video edit at t=0. + expect(calls[0]!.args).not.toContain("-avoid_negative_ts"); emitClose(calls[0]!.proc, 0); await expect(muxPromise).resolves.toMatchObject({ success: true }); }); - it("preserves a known M4A priming edit list instead of shifting copied video", async () => { + it("ignores the deprecated preserveAudioPrimingEditList option", async () => { const { spawn, calls } = createSpawnSpy(); vi.resetModules(); vi.doMock("child_process", () => ({ spawn })); @@ -459,7 +459,7 @@ describe("muxVideoWithAudio audio codec handling", () => { "/tmp/audio.duration-normalized.m4a", "/tmp/output.mp4", undefined, - { audioCodec: "aac", preserveAudioPrimingEditList: true }, + { audioCodec: "aac", preserveAudioPrimingEditList: false }, { num: 30, den: 1 }, ); @@ -582,6 +582,7 @@ describe("muxVideoWithAudio audio codec handling", () => { expect(calls[0]!.args[calls[0]!.args.indexOf("-c:a") + 1]).toBe("aac"); expect(calls[0]!.args).toContain("-b:a"); expect(calls[0]!.args).toContain("+faststart"); + expect(calls[0]!.args).not.toContain("-avoid_negative_ts"); emitClose(calls[0]!.proc, 0); await expect(muxPromise).resolves.toMatchObject({ success: true }); @@ -629,6 +630,10 @@ describe("muxVideoWithAudio audio codec handling", () => { if (ext !== ".webm") await flushMuxCodecResolution(); const call = calls[calls.length - 1]!; expect(call.args).not.toContain("-shortest"); + // Same for every container we mux into: ffmpeg's `auto` default is + // already `disabled` for mp4/mov, and forcing `make_zero` breaks the + // AAC priming edit list (#3487). + expect(call.args).not.toContain("-avoid_negative_ts"); emitClose(call.proc, 0); await muxPromise; } diff --git a/packages/engine/src/services/chunkEncoder.ts b/packages/engine/src/services/chunkEncoder.ts index dbad9fc716..6128644d17 100644 --- a/packages/engine/src/services/chunkEncoder.ts +++ b/packages/engine/src/services/chunkEncoder.ts @@ -78,7 +78,13 @@ export interface MuxVideoWithAudioOptions extends Partial< * depend on the file extension alone. */ audioCodec?: "aac"; - /** Preserve a priming edit list known to have been created by AAC re-encoding. */ + /** + * @deprecated No longer used. `-avoid_negative_ts` is never passed for + * mp4/mov muxing (ffmpeg's `auto` default already resolves to `disabled` + * for those containers), so the AAC priming edit list is preserved + * unconditionally and this flag has no effect. See issue #3487. Kept for + * source compatibility; it will be removed in a future major. + */ preserveAudioPrimingEditList?: boolean; /** Hard cap copied audio to the already-encoded video's exact duration. */ } @@ -693,14 +699,18 @@ export async function muxVideoWithAudio( args.push("-c:a", "aac", "-b:a", "192k", "-movflags", "+faststart"); } } - const copiesContainerizedAac = - !isWebm && shouldCopyAudio && config?.preserveAudioPrimingEditList === true; - // PTS bases can diverge during mux and reintroduce negative DTS. See - // buildEncoderArgs for the full reasoning on why that breaks playback. - // A freshly encoded M4A is the exception: its edit list already hides the - // AAC priming packet. `make_zero` discards that edit and shifts copied video - // forward by one AAC frame (~21ms), creating a visible first-frame offset. - if (!copiesContainerizedAac) args.push("-avoid_negative_ts", "make_zero"); + // No `-avoid_negative_ts` here, in any mode. ffmpeg's default is `auto`, + // which the mp4/mov muxers (AVFMT_TS_NEGATIVE) already resolve to + // `disabled` — the correct behavior for the containers this function + // writes. Passing `make_zero` explicitly overrides that default and, on the + // dominant audio-copy path, discards the AAC priming edit list the sidecar + // encode created: the video start_time shifts forward one AAC frame + // (~21ms) and the muxer writes an empty video edit at t=0, which + // edit-list-honoring players (QuickTime/Safari) show as a black first + // frame. See issue #3487. The video-only encoder args (buildEncoderArgs) + // still pass the flag deliberately — those chunks are consumed as raw + // elementary output, not as a delivered mp4/mov. + // // Re-assert provenance here: this stage re-muxes into the delivered // container, and the mp4 muxer drops the encode stage's tags without the // use_metadata_tags flag that appendRenderProvenanceArgs adds. diff --git a/packages/producer/src/services/distributed/assemble.ts b/packages/producer/src/services/distributed/assemble.ts index 309c26db88..9ae023676f 100644 --- a/packages/producer/src/services/distributed/assemble.ts +++ b/packages/producer/src/services/distributed/assemble.ts @@ -302,10 +302,7 @@ export async function assemble( } // ── 3. Audio: pad-or-trim then mux ──────────────────────────────────── - let normalizedAudio: { - path: string; - preserveAudioPrimingEditList: boolean; - } | null = null; + let normalizedAudioPath: string | null = null; if (audioPath !== null && existsSync(audioPath)) { const paddedAudioPath = join(workDir, "audio-padded.m4a"); const padTrimResult = await padOrTrimAudioToVideoFrameCount({ @@ -317,10 +314,7 @@ export async function assemble( if (!padTrimResult.success) { throw new Error(`[assemble] audio pad/trim failed: ${padTrimResult.error}`); } - normalizedAudio = { - path: paddedAudioPath, - preserveAudioPrimingEditList: padTrimResult.operation !== "copy", - }; + normalizedAudioPath = paddedAudioPath; log.info("[assemble] audio normalized for mux", { operation: padTrimResult.operation, targetDurationSeconds: padTrimResult.targetDurationSeconds, @@ -333,16 +327,17 @@ export async function assemble( // because it operates on a `RenderJob` and emits `updateJobStatus` // payloads — the distributed activity has no job to thread through. const muxOutputPath = - normalizedAudio !== null ? join(workDir, `mux.${plan.dimensions.format}`) : postConcatPath; - if (normalizedAudio !== null) { + normalizedAudioPath !== null + ? join(workDir, `mux.${plan.dimensions.format}`) + : postConcatPath; + if (normalizedAudioPath !== null) { const muxResult = await muxVideoWithAudio( postConcatPath, - normalizedAudio.path, + normalizedAudioPath, muxOutputPath, abortSignal, { audioCodec: "aac", - preserveAudioPrimingEditList: normalizedAudio.preserveAudioPrimingEditList, }, { num: plan.dimensions.fpsNum, den: plan.dimensions.fpsDen }, ); diff --git a/packages/producer/src/services/render/stages/assembleStage.test.ts b/packages/producer/src/services/render/stages/assembleStage.test.ts index ae782ac75a..3d3f8e65b9 100644 --- a/packages/producer/src/services/render/stages/assembleStage.test.ts +++ b/packages/producer/src/services/render/stages/assembleStage.test.ts @@ -69,7 +69,7 @@ describe("runAssembleStage audio duration parity", () => { "/tmp/audio.duration-normalized.m4a", "/tmp/output.mp4", undefined, - { audioCodec: "aac", preserveAudioPrimingEditList: true }, + { audioCodec: "aac" }, { num: 30, den: 1 }, ); }); diff --git a/packages/producer/src/services/render/stages/assembleStage.ts b/packages/producer/src/services/render/stages/assembleStage.ts index 39dc431ca6..b69fd86d67 100644 --- a/packages/producer/src/services/render/stages/assembleStage.ts +++ b/packages/producer/src/services/render/stages/assembleStage.ts @@ -78,7 +78,6 @@ export async function runAssembleStage(input: AssembleStageInput): Promise