mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 23:29:50 +00:00
fix(engine): stop destroying the AAC priming edit list when muxing (#3505)
`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: Alexandru Mincu <alex@mountsoftware.ro>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Alexandru Mincu
Claude Fable 5
parent
18409c9f27
commit
ee64c3b116
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 },
|
||||
);
|
||||
|
||||
@@ -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 },
|
||||
);
|
||||
});
|
||||
|
||||
@@ -78,7 +78,6 @@ export async function runAssembleStage(input: AssembleStageInput): Promise<Assem
|
||||
abortSignal,
|
||||
{
|
||||
audioCodec: "aac",
|
||||
preserveAudioPrimingEditList: normalizeResult.operation !== "copy",
|
||||
},
|
||||
job.config.fps,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user