Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions packages/engine/src/services/chunkEncoder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,6 @@ describe("muxVideoWithAudio audio codec handling", () => {
"copy",
"-movflags",
"+faststart",
"-avoid_negative_ts",
"make_zero",
...renderProvenanceArgs("/tmp/output.mp4"),
"-r",
"30",
Expand All @@ -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 }));
Expand All @@ -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 }));
Expand All @@ -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 },
);

Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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;
}
Expand Down
28 changes: 19 additions & 9 deletions packages/engine/src/services/chunkEncoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
}
Expand Down Expand Up @@ -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.
Expand Down
19 changes: 7 additions & 12 deletions packages/producer/src/services/distributed/assemble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
Expand All @@ -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 },
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export async function runAssembleStage(input: AssembleStageInput): Promise<Assem
abortSignal,
{
audioCodec: "aac",
preserveAudioPrimingEditList: normalizeResult.operation !== "copy",
},
job.config.fps,
);
Expand Down
Loading