Skip to content

Commit dc43831

Browse files
fix(producer): mix audio into a container that can record encoder delay (#3200)
* fix(producer): mix audio into a container that can record encoder delay Every rendered composition's audio landed 1024 samples (21.33 ms at 48 kHz) after its authored `data-start`, against a frame-accurate video track. The mix is AAC-encoded, and AAC encoders emit ~1024 priming samples. The mix was written to a raw ADTS `.aac` file, which has nowhere to record that delay, so it decoded as real leading silence and every stage downstream preserved it faithfully. Measuring each intermediate localises it precisely: the source WAV is exact, the mixer's own output is already 21.33 ms late, and the pad/trim and mux stages inherit it unchanged. The filter graph itself is correct - run by hand to PCM it lands on the authored start. Switch the artifact to an MP4-family container, which stores the delay as an edit list that decoders strip. Same codec, same bitrate, so no size or quality change. The filename is a contract shared by three consumers - the mux input, the distributed plan artifact, and the PNG-sequence sidecar handed to users for NLE ingest - and its extension is what selects the muxer. Give it one owner in the engine rather than five literals, so those consumers cannot drift onto different containers. Note for reviewers: this renames the distributed plan's audio artifact, which is an on-disk contract between the plan writer and the assembler. Both move together here, but a plan written by an older build would not be found by a newer assembler. Flagging in case that mixed-version window matters for how these are deployed. * fix(cloud): read the plan audio artifact name from the producer contract The aws-lambda and gcp-cloud-run adapters each restated the plan's audio filename in five places, so renaming it in the producer left them looking for a file that is no longer written. CI caught it: the gcp dispatch test asserting a plan has no audio artifact started seeing one. Export the name from `@hyperframes/producer/distributed` and consume it in both adapters. This is the same failure the constant exists to prevent, one package boundary further out: a literal that drifts from the writer's is a silently missing audio track rather than a loud error, because both call sites only ever ask whether the file exists. * fix(cloud): accept a legacy plan's audio artifact name for one release Review raised a rolling-deploy window I had flagged but left undecided: `plan` and `assemble` are separate invocations bridged by object storage, so a pre-rollout planner can be paired with a post-rollout assembler. Both readers locate the artifact by existence alone, which makes that pairing a silently muted video rather than an error. That is reachable enough to be worth two lines, so reads now accept the old name while writes only ever emit the new one. Give the fallback one owner (`resolvePlanAudioPath` / `isPlanAudioArtifactPath`) rather than four call sites, marked for deletion one release out. Also fixes a hole in the first pass of this: the plan-v2 materializer matched either name but then joined the CURRENT one, so a legacy plan resolved to a path that was never written. It now joins the artifact's own name. Review nits in the same pass: correct the pad-branch docstring, which still described a concat-copy shape the pad branch stopped using when it moved to apad + re-encode, and fix the Windows fixture's stale `.aac` output extension so it cannot model a shape that reintroduces the priming delay. * test(producer): rebake the missing-host-comp-id golden without the audio delay The pinned reference was rendered before this branch, so it carries the 1024 sample encoder-priming delay in its audio. With the delay gone the correct audio now sits ahead of the reference and the harness's envelope correlation drops below its floor. Cross-correlating the old and new references at native 48 kHz gives a lag of exactly 1024 samples (21.33 ms) at a correlation of 0.99985: same audio, moved by exactly the amount this branch removes. Regenerated inside the CI container (Dockerfile.test, ffmpeg 5.1.9) rather than natively, so the reference matches the encoder CI will compare against - the container reproduced CI's failure to the digit (correlation 0.3938764027803616, lagWindows -12) before the rebake and passes at correlation 1.0 after it. Note for archaeology: the new reference is also 3 dB louder than the old one. That gap is not from this branch - `main` and this branch render the fixture at the same level - it is pre-existing drift the reference had accumulated, which a scale-invariant correlator could never see. The rebake absorbs it. Only output.mp4 is updated. `--update` also rewrites compiled.html, but that diff is embedded-font churn with no bearing on the comparison, which reports "Failed at compilation: 0" either way. * test(producer): rebake the variables-prod golden without the audio delay Same cause as the missing-host-comp-id rebake, caught by shard-8 once the earlier shard stopped failing and the rest of the matrix could run: this reference also carries the encoder-priming delay this branch removes. Reproduced in the CI container to the digit (correlation 0.42704173048439215, lagWindows -12), rebaked there, and it now passes at correlation 1.0. Worth recording: the shift here is 2048 samples (42.67 ms) at correlation 0.99983, exactly twice the 1024 of the other fixture. The delay compounds once per un-compensated AAC generation, and this fixture's audio needs its duration normalized, so it takes the pad/trim branch's re-encode and picks up a second frame of priming on top of the mixer's. So the pre-fix error was not a fixed 21 ms - it grew with the number of times the audio was re-encoded. All nine shards ran in that CI round with only this one failing, so the matrix has now covered every fixture against this change.
1 parent eee9b26 commit dc43831

32 files changed

Lines changed: 329 additions & 84 deletions

packages/aws-lambda/src/events.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ interface AssembleEventBase {
117117
Action: "assemble";
118118
/** S3 URIs of every chunk, ordered by chunk index. Length must equal `chunkCount`. */
119119
ChunkS3Uris: string[];
120-
/** S3 URI of the planDir's `audio.aac` if the composition has audio; `null` otherwise. */
120+
/** S3 URI of the planDir's audio artifact if the composition has audio; `null` otherwise. */
121121
AudioS3Uri: string | null;
122122
/** Final output S3 URI (`s3://bucket/key.mp4`). */
123123
OutputS3Uri: string;

packages/aws-lambda/src/handler.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ describe("handler dispatch", () => {
563563
);
564564
const renderChunkMock = mock(
565565
async (planDir: string, _chunkIndex: number, outputPath: string): Promise<ChunkResult> => {
566-
expect(existsSync(join(planDir, "audio.aac"))).toBe(false);
566+
expect(existsSync(join(planDir, "audio.m4a"))).toBe(false);
567567
writeFileSync(outputPath, "V2-CHUNK");
568568
return {
569569
outputPath,
@@ -818,7 +818,7 @@ function makeMinimalV1PlanDir(dir: string, withAudio: boolean): void {
818818
JSON.stringify([{ index: 0, startFrame: 0, endFrame: 30 }]),
819819
);
820820
writeFileSync(join(dir, "meta", "encoder.json"), "{}");
821-
if (withAudio) writeFileSync(join(dir, "audio.aac"), "AAC");
821+
if (withAudio) writeFileSync(join(dir, "audio.m4a"), "AAC");
822822
planJson.planHash = recomputePlanHashFromPlanDir(dir);
823823
writeFileSync(join(dir, "plan.json"), JSON.stringify(planJson));
824824
}

packages/aws-lambda/src/handler.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ import {
2424
listPlanV2ArtifactsForTarget,
2525
materializePlanV2Target,
2626
plan,
27+
isPlanAudioArtifactPath,
28+
PLAN_AUDIO_RELATIVE_PATH,
29+
resolvePlanAudioPath,
2730
planV2WithPublisher,
2831
type PlanResult,
2932
type PlanV2Artifact,
@@ -318,9 +321,11 @@ async function handlePlan(event: PlanEvent, deps?: HandlerDeps): Promise<PlanLam
318321
const planTar = join(work, "plan.tar.gz");
319322
await tarDirectory(planDir, planTar);
320323
const planTarUri = `${trimTrailingSlash(event.PlanOutputS3Prefix)}/plan.tar.gz`;
321-
const audioPath = join(planDir, "audio.aac");
324+
const audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
322325
const hasAudio = existsSync(audioPath) && statSync(audioPath).size > 0;
323-
const audioUri = hasAudio ? `${trimTrailingSlash(event.PlanOutputS3Prefix)}/audio.aac` : null;
326+
const audioUri = hasAudio
327+
? `${trimTrailingSlash(event.PlanOutputS3Prefix)}/${PLAN_AUDIO_RELATIVE_PATH}`
328+
: null;
324329
// Plan and audio are independent S3 PUTs; run them in parallel so
325330
// the response returns as soon as the slower of the two completes.
326331
await Promise.all([
@@ -390,7 +395,7 @@ async function handlePlanV2(
390395
Width: manifest.width,
391396
Height: manifest.height,
392397
Format: manifest.format,
393-
HasAudio: manifest.artifacts.some((artifact) => artifact.path === "audio.aac"),
398+
HasAudio: manifest.artifacts.some((artifact) => isPlanAudioArtifactPath(artifact.path)),
394399
AudioS3Uri: null,
395400
FfmpegVersion: manifest.ffmpegVersion,
396401
ProducerVersion: manifest.producerVersion,
@@ -568,7 +573,7 @@ async function handleAssemble(
568573

569574
let audioPath: string | null = null;
570575
if (event.AudioS3Uri) {
571-
audioPath = join(planDir, "audio.aac");
576+
audioPath = resolvePlanAudioPath(planDir) ?? join(planDir, PLAN_AUDIO_RELATIVE_PATH);
572577
await downloadS3ObjectToFile(s3, event.AudioS3Uri, audioPath);
573578
}
574579

@@ -616,7 +621,7 @@ async function handleAssembleV2(
616621
const planDir = await downloadAndMaterializePlanV2(s3, event, { role: "assembler" }, work);
617622
// `downloadAndMaterializePlanV2` materializes atomically. Audio is
618623
// assembler-only and lives at the familiar v1-compatible location.
619-
const audioPath = existsSync(join(planDir, "audio.aac")) ? join(planDir, "audio.aac") : null;
624+
const audioPath = resolvePlanAudioPath(planDir);
620625
const chunkPaths = await downloadChunkObjects(s3, event.ChunkS3Uris, work, event.Format);
621626
const finalOutput =
622627
event.Format === "png-sequence"

packages/engine/src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,11 @@ export {
213213

214214
export { createVideoFrameInjector } from "./services/videoFrameInjector.js";
215215

216-
export { parseAudioElements, processCompositionAudio } from "./services/audioMixer.js";
216+
export {
217+
MIXED_AUDIO_FILENAME,
218+
parseAudioElements,
219+
processCompositionAudio,
220+
} from "./services/audioMixer.js";
217221
export { cloneCaptureWarning, cloneCaptureWarnings } from "./services/captureWarning.js";
218222
export type {
219223
AudioElement,

packages/engine/src/services/audioMixer.level.test.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import { afterEach, describe, expect, it } from "vitest";
66
import { getFfmpegBinary } from "../utils/ffmpegBinaries.js";
7-
import { processCompositionAudio } from "./audioMixer.js";
7+
import { MIXED_AUDIO_FILENAME, processCompositionAudio } from "./audioMixer.js";
88

99
const HAS_FFMPEG = spawnSync(getFfmpegBinary(), ["-version"], { encoding: "utf-8" }).status === 0;
1010
const tempDirs: string[] = [];
@@ -22,6 +22,39 @@ function meanVolumeDb(path: string): number {
2222
return Number(match[1]);
2323
}
2424

25+
/** Seconds until the first sample loud enough to be signal rather than codec noise. */
26+
function firstAudibleSeconds(path: string): number {
27+
const sampleRate = 48_000;
28+
const result = spawnSync(
29+
getFfmpegBinary(),
30+
[
31+
"-nostdin",
32+
"-v",
33+
"error",
34+
"-i",
35+
path,
36+
"-map",
37+
"0:a",
38+
"-ac",
39+
"1",
40+
"-ar",
41+
String(sampleRate),
42+
"-f",
43+
"s16le",
44+
"-",
45+
],
46+
{ maxBuffer: 1 << 28 },
47+
);
48+
if (result.status !== 0) {
49+
throw new Error(`Could not decode ${path}: ${result.stderr?.toString()}`);
50+
}
51+
const pcm = result.stdout;
52+
for (let i = 0; i < pcm.length / 2; i += 1) {
53+
if (Math.abs(pcm.readInt16LE(i * 2)) > 512) return i / sampleRate;
54+
}
55+
throw new Error(`No audible sample found in ${path}`);
56+
}
57+
2558
describe.skipIf(!HAS_FFMPEG)("processCompositionAudio levels", () => {
2659
afterEach(() => {
2760
for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
@@ -75,4 +108,57 @@ describe.skipIf(!HAS_FFMPEG)("processCompositionAudio levels", () => {
75108
expect(result.success).toBe(true);
76109
expect(meanVolumeDb(outputPath) - meanVolumeDb(sourcePath)).toBeGreaterThan(-0.3);
77110
});
111+
112+
it("places a delayed track on its authored start, not one AAC frame later", async () => {
113+
// The mix is AAC-encoded, and AAC encoders emit ~1024 priming samples. A
114+
// raw ADTS container has nowhere to record that delay, so it decodes as
115+
// real leading silence and drags the whole track 21.33 ms late against a
116+
// frame-accurate video. MIXED_AUDIO_FILENAME picks a container that stores
117+
// the delay as an edit list instead; this asserts the artifact we actually
118+
// ship lands on time.
119+
const projectDir = mkdtempSync(join(tmpdir(), "hf-onset-"));
120+
const workDir = mkdtempSync(join(tmpdir(), "hf-onset-work-"));
121+
tempDirs.push(projectDir, workDir);
122+
const sourcePath = join(projectDir, "tone.wav");
123+
const outputPath = join(projectDir, MIXED_AUDIO_FILENAME);
124+
const setup = spawnSync(
125+
getFfmpegBinary(),
126+
[
127+
"-nostdin",
128+
"-v",
129+
"error",
130+
"-f",
131+
"lavfi",
132+
"-i",
133+
"sine=frequency=1000:duration=1:sample_rate=48000",
134+
"-c:a",
135+
"pcm_s16le",
136+
sourcePath,
137+
],
138+
{ encoding: "utf-8" },
139+
);
140+
expect(setup.status, setup.stderr).toBe(0);
141+
142+
const result = await processCompositionAudio(
143+
[
144+
{
145+
id: "tone",
146+
src: "tone.wav",
147+
start: 2,
148+
end: 3,
149+
mediaStart: 0,
150+
layer: 0,
151+
volume: 1,
152+
type: "audio",
153+
},
154+
],
155+
projectDir,
156+
workDir,
157+
outputPath,
158+
4,
159+
);
160+
161+
expect(result.success).toBe(true);
162+
expect(firstAudibleSeconds(outputPath)).toBeCloseTo(2, 2);
163+
});
78164
});

packages/engine/src/services/audioMixer.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,21 @@ import { applyVolumeEnvelopeToWav } from "./audioVolumeEnvelope.js";
3232

3333
export type { AudioElement, MixResult } from "./audioMixer.types.js";
3434

35+
/**
36+
* Filename every caller must use for the mixed-audio artifact.
37+
*
38+
* The extension is load-bearing, not cosmetic: FFmpeg picks the muxer from it,
39+
* and the mix is AAC-encoded. A raw ADTS `.aac` stream has nowhere to record
40+
* the encoder's priming delay, so those leading samples decode as real silence
41+
* and shift the whole track ~1024 samples (21.33 ms at 48 kHz) late against a
42+
* frame-accurate video track. An MP4-family container carries the delay as an
43+
* edit list, which every decoder then strips, so the mix lands on its authored
44+
* start. Keep the choice here rather than at each call site: the same file is
45+
* muxed into the video, shipped in a distributed plan, and handed to users as
46+
* the PNG-sequence sidecar, and all three have to agree.
47+
*/
48+
export const MIXED_AUDIO_FILENAME = "audio.m4a";
49+
3550
function clampVolume(volume: number): number {
3651
if (!Number.isFinite(volume)) return 1;
3752
return Math.max(0, Math.min(1, volume));

packages/gcp-cloud-run/src/server.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ function makeMinimalV1PlanDir(dir: string, withAudio: boolean): void {
8787
JSON.stringify([{ index: 0, startFrame: 0, endFrame: 30 }]),
8888
);
8989
writeFileSync(join(dir, "meta", "encoder.json"), "{}");
90-
if (withAudio) writeFileSync(join(dir, "audio.aac"), "AAC");
90+
if (withAudio) writeFileSync(join(dir, "audio.m4a"), "AAC");
9191
planJson.planHash = recomputePlanHashFromPlanDir(dir);
9292
writeFileSync(join(dir, "plan.json"), JSON.stringify(planJson));
9393
}
@@ -273,7 +273,7 @@ describe("dispatch", () => {
273273
chunkIndex: number,
274274
outputBase: string,
275275
): Promise<ChunkResult> => {
276-
expect(existsSync(join(planDir, "audio.aac"))).toBe(false);
276+
expect(existsSync(join(planDir, "audio.m4a"))).toBe(false);
277277
writeFileSync(outputBase, `chunk-${chunkIndex}`);
278278
return {
279279
outputPath: outputBase,

packages/gcp-cloud-run/src/server.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ import {
3232
listPlanV2ArtifactsForTarget,
3333
materializePlanV2Target,
3434
plan,
35+
isPlanAudioArtifactPath,
36+
PLAN_AUDIO_RELATIVE_PATH,
37+
resolvePlanAudioPath,
3538
planV2WithPublisher,
3639
type PlanResult,
3740
type PlanV2Artifact,
@@ -322,7 +325,7 @@ async function handlePlan(event: PlanEvent, deps?: HandlerDeps): Promise<PlanRes
322325

323326
// Upload the planDir as a single tarball. The workflow cannot pass a
324327
// directory-shaped artifact between steps; we serialize and rely on the
325-
// consumer (renderChunk / assemble) to untar. `audio.aac` lives inside
328+
// consumer (renderChunk / assemble) to untar. The audio artifact lives inside
326329
// planDir, so it already rides along in this tarball — every consumer
327330
// (including assemble) gets it from the untar. We deliberately do NOT
328331
// upload a separate audio object: it would duplicate the bytes on every
@@ -331,7 +334,7 @@ async function handlePlan(event: PlanEvent, deps?: HandlerDeps): Promise<PlanRes
331334
const planTar = join(work, "plan.tar.gz");
332335
await tarDirectory(planDir, planTar);
333336
const planTarUri = `${trimTrailingSlash(event.PlanOutputGcsPrefix)}/plan.tar.gz`;
334-
const audioPath = join(planDir, "audio.aac");
337+
const audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
335338
const hasAudio = existsSync(audioPath) && statSync(audioPath).size > 0;
336339
await uploadFileToGcs(storage, planTar, planTarUri, "application/gzip");
337340

@@ -397,7 +400,7 @@ async function handlePlanV2(
397400
Width: manifest.width,
398401
Height: manifest.height,
399402
Format: manifest.format,
400-
HasAudio: manifest.artifacts.some((artifact) => artifact.path === "audio.aac"),
403+
HasAudio: manifest.artifacts.some((artifact) => isPlanAudioArtifactPath(artifact.path)),
401404
AudioGcsUri: null,
402405
FfmpegVersion: manifest.ffmpegVersion,
403406
ProducerVersion: manifest.producerVersion,
@@ -567,7 +570,7 @@ async function handleAssemble(
567570
// only for backward compatibility with an older Plan that uploaded it
568571
// standalone.
569572
let audioPath: string | null = null;
570-
const planAudio = join(planDir, "audio.aac");
573+
const planAudio = resolvePlanAudioPath(planDir) ?? join(planDir, PLAN_AUDIO_RELATIVE_PATH);
571574
if (existsSync(planAudio) && statSync(planAudio).size > 0) {
572575
audioPath = planAudio;
573576
} else if (event.AudioGcsUri) {
@@ -619,7 +622,7 @@ async function handleAssembleV2(
619622
const work = mkdtempSync(join(deps?.tmpRoot ?? tmpdir(), "hf-cr-assemble-v2-"));
620623
try {
621624
const planDir = await downloadAndMaterializePlanV2(storage, event, { role: "assembler" }, work);
622-
const audioPath = existsSync(join(planDir, "audio.aac")) ? join(planDir, "audio.aac") : null;
625+
const audioPath = resolvePlanAudioPath(planDir);
623626
const chunkPaths = await downloadChunkObjects(storage, event.ChunkGcsUris, work, event.Format);
624627
const finalOutput =
625628
event.Format === "png-sequence"

packages/producer/src/distributed.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,18 @@ export {
149149
// CLI / adopter SDKs can derive runtime allowlists from one source.
150150
export { PlanVideosMetadataError, type DistributedFormat } from "./services/distributed/shared.js";
151151

152+
// ── Plan artifact names ─────────────────────────────────────────────────────
153+
// The cloud adapters locate and publish the plan's audio artifact by name. Its
154+
// extension selects the container, so they must read it from here rather than
155+
// restate it: a literal that drifts from the writer's is a silently missing
156+
// audio track, not a loud failure.
157+
export {
158+
isPlanAudioArtifactPath,
159+
PLAN_AUDIO_LEGACY_RELATIVE_PATH,
160+
PLAN_AUDIO_RELATIVE_PATH,
161+
resolvePlanAudioPath,
162+
} from "./services/distributed/shared.js";
163+
152164
// ── Plan-time shared types from `freezePlan` ───────────────────────────────
153165
// Re-exported so adopters that deserialize a planDir's `meta/encoder.json`
154166
// or `meta/chunks.json` see the same shapes the producer wrote them as.

packages/producer/src/regression-harness-distributed.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { existsSync, mkdirSync } from "node:fs";
3535
import { join } from "node:path";
3636
import type { Fps } from "@hyperframes/core";
3737
import { assemble, plan, renderChunk } from "./distributed.js";
38-
import type { DistributedFormat } from "./services/distributed/shared.js";
38+
import { PLAN_AUDIO_RELATIVE_PATH, type DistributedFormat } from "./services/distributed/shared.js";
3939

4040
/**
4141
* Three-mode contract that backs `--mode=<value>` on the regression
@@ -199,9 +199,9 @@ export async function runDistributedSimulatedRender(
199199
chunkPaths.push(chunkPath);
200200
}
201201

202-
// Step C: assemble. `audio.aac` only exists when the composition has
202+
// Step C: assemble. The audio artifact only exists when the composition has
203203
// audio — pass null otherwise so `assemble()` doesn't try to mux silence.
204-
const audioPath = join(planDir, "audio.aac");
204+
const audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
205205
const audioForAssemble = existsSync(audioPath) ? audioPath : null;
206206
await assemble(planDir, chunkPaths, audioForAssemble, input.renderedOutputPath);
207207
}

0 commit comments

Comments
 (0)