Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion packages/aws-lambda/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ interface AssembleEventBase {
Action: "assemble";
/** S3 URIs of every chunk, ordered by chunk index. Length must equal `chunkCount`. */
ChunkS3Uris: string[];
/** S3 URI of the planDir's `audio.aac` if the composition has audio; `null` otherwise. */
/** S3 URI of the planDir's audio artifact if the composition has audio; `null` otherwise. */
AudioS3Uri: string | null;
/** Final output S3 URI (`s3://bucket/key.mp4`). */
OutputS3Uri: string;
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-lambda/src/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ describe("handler dispatch", () => {
);
const renderChunkMock = mock(
async (planDir: string, _chunkIndex: number, outputPath: string): Promise<ChunkResult> => {
expect(existsSync(join(planDir, "audio.aac"))).toBe(false);
expect(existsSync(join(planDir, "audio.m4a"))).toBe(false);
writeFileSync(outputPath, "V2-CHUNK");
return {
outputPath,
Expand Down Expand Up @@ -818,7 +818,7 @@ function makeMinimalV1PlanDir(dir: string, withAudio: boolean): void {
JSON.stringify([{ index: 0, startFrame: 0, endFrame: 30 }]),
);
writeFileSync(join(dir, "meta", "encoder.json"), "{}");
if (withAudio) writeFileSync(join(dir, "audio.aac"), "AAC");
if (withAudio) writeFileSync(join(dir, "audio.m4a"), "AAC");
planJson.planHash = recomputePlanHashFromPlanDir(dir);
writeFileSync(join(dir, "plan.json"), JSON.stringify(planJson));
}
15 changes: 10 additions & 5 deletions packages/aws-lambda/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
listPlanV2ArtifactsForTarget,
materializePlanV2Target,
plan,
PLAN_AUDIO_RELATIVE_PATH,
planV2WithPublisher,
type PlanResult,
type PlanV2Artifact,
Expand Down Expand Up @@ -318,9 +319,11 @@ async function handlePlan(event: PlanEvent, deps?: HandlerDeps): Promise<PlanLam
const planTar = join(work, "plan.tar.gz");
await tarDirectory(planDir, planTar);
const planTarUri = `${trimTrailingSlash(event.PlanOutputS3Prefix)}/plan.tar.gz`;
const audioPath = join(planDir, "audio.aac");
const audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
const hasAudio = existsSync(audioPath) && statSync(audioPath).size > 0;
const audioUri = hasAudio ? `${trimTrailingSlash(event.PlanOutputS3Prefix)}/audio.aac` : null;
const audioUri = hasAudio
? `${trimTrailingSlash(event.PlanOutputS3Prefix)}/${PLAN_AUDIO_RELATIVE_PATH}`
: null;
// Plan and audio are independent S3 PUTs; run them in parallel so
// the response returns as soon as the slower of the two completes.
await Promise.all([
Expand Down Expand Up @@ -390,7 +393,7 @@ async function handlePlanV2(
Width: manifest.width,
Height: manifest.height,
Format: manifest.format,
HasAudio: manifest.artifacts.some((artifact) => artifact.path === "audio.aac"),
HasAudio: manifest.artifacts.some((artifact) => artifact.path === PLAN_AUDIO_RELATIVE_PATH),
AudioS3Uri: null,
FfmpegVersion: manifest.ffmpegVersion,
ProducerVersion: manifest.producerVersion,
Expand Down Expand Up @@ -568,7 +571,7 @@ async function handleAssemble(

let audioPath: string | null = null;
if (event.AudioS3Uri) {
audioPath = join(planDir, "audio.aac");
audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Backward-compat concern — plan-writer/assembler mixed-version window

Miguel raised this himself in the PR body ("a plan written by an older build would not be found by a newer assembler"). This is the primary read site for the S3 v1 assemble path, and I think the window IS reachable given the plan-store-and-forward shape here — the plan is uploaded to S3 as a tarball, and there's no coordination that forbids a NEWER handleAssemble from picking up a plan tarball whose audio.aac was written by the pre-fix build (in-flight batches during a deploy, replay-on-failure of an older event, cross-region propagation lag, whatever). When that happens, existsSync(join(planDir, PLAN_AUDIO_RELATIVE_PATH)) at the sibling v2 site (:622-624) is false, handleAssemble here reads a non-existent path and the assembler proceeds without audio — silent no-audio render, not a loud failure.

A one-release compat window closes it cheaply. Something like:

const audioCandidates = [PLAN_AUDIO_RELATIVE_PATH, "audio.aac"]; // legacy fallback — remove after one release cycle
const audioPath = audioCandidates
  .map((name) => join(planDir, name))
  .find((p) => existsSync(p)) ?? null;

Or, for the S3 v1 path here, just OR the legacy name into the existing branch. Same treatment needed at:

  • handler.ts:622-624 (S3 v2 assemble)
  • packages/gcp-cloud-run/src/server.ts:571-577 (GCS v1)
  • packages/gcp-cloud-run/src/server.ts:622-624 (GCS v2)

Delete the legacy name in the next release. If the plan-store-and-forward mixed-version window ISN'T reachable in practice (all planners + assemblers deploy atomically together, no in-flight replay of older plans), then this reduces to a no-op comment saying so. Either resolution is fine — the current state (no fallback, no note documenting the reasoning) is what leaves someone else in the dark on the deploy day.

— Rames D Jusso

await downloadS3ObjectToFile(s3, event.AudioS3Uri, audioPath);
}

Expand Down Expand Up @@ -616,7 +619,9 @@ async function handleAssembleV2(
const planDir = await downloadAndMaterializePlanV2(s3, event, { role: "assembler" }, work);
// `downloadAndMaterializePlanV2` materializes atomically. Audio is
// assembler-only and lives at the familiar v1-compatible location.
const audioPath = existsSync(join(planDir, "audio.aac")) ? join(planDir, "audio.aac") : null;
const audioPath = existsSync(join(planDir, PLAN_AUDIO_RELATIVE_PATH))
? join(planDir, PLAN_AUDIO_RELATIVE_PATH)
: null;
const chunkPaths = await downloadChunkObjects(s3, event.ChunkS3Uris, work, event.Format);
const finalOutput =
event.Format === "png-sequence"
Expand Down
6 changes: 5 additions & 1 deletion packages/engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ export {

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

export { parseAudioElements, processCompositionAudio } from "./services/audioMixer.js";
export {
MIXED_AUDIO_FILENAME,
parseAudioElements,
processCompositionAudio,
} from "./services/audioMixer.js";
export { cloneCaptureWarning, cloneCaptureWarnings } from "./services/captureWarning.js";
export type {
AudioElement,
Expand Down
88 changes: 87 additions & 1 deletion packages/engine/src/services/audioMixer.level.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { getFfmpegBinary } from "../utils/ffmpegBinaries.js";
import { processCompositionAudio } from "./audioMixer.js";
import { MIXED_AUDIO_FILENAME, processCompositionAudio } from "./audioMixer.js";

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

/** Seconds until the first sample loud enough to be signal rather than codec noise. */
function firstAudibleSeconds(path: string): number {
const sampleRate = 48_000;
const result = spawnSync(
getFfmpegBinary(),
[
"-nostdin",
"-v",
"error",
"-i",
path,
"-map",
"0:a",
"-ac",
"1",
"-ar",
String(sampleRate),
"-f",
"s16le",
"-",
],
{ maxBuffer: 1 << 28 },
);
if (result.status !== 0) {
throw new Error(`Could not decode ${path}: ${result.stderr?.toString()}`);
}
const pcm = result.stdout;
for (let i = 0; i < pcm.length / 2; i += 1) {
if (Math.abs(pcm.readInt16LE(i * 2)) > 512) return i / sampleRate;
}
throw new Error(`No audible sample found in ${path}`);
}

describe.skipIf(!HAS_FFMPEG)("processCompositionAudio levels", () => {
afterEach(() => {
for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
Expand Down Expand Up @@ -75,4 +108,57 @@ describe.skipIf(!HAS_FFMPEG)("processCompositionAudio levels", () => {
expect(result.success).toBe(true);
expect(meanVolumeDb(outputPath) - meanVolumeDb(sourcePath)).toBeGreaterThan(-0.3);
});

it("places a delayed track on its authored start, not one AAC frame later", async () => {
// The mix is AAC-encoded, and AAC encoders emit ~1024 priming samples. A
// raw ADTS container has nowhere to record that delay, so it decodes as
// real leading silence and drags the whole track 21.33 ms late against a
// frame-accurate video. MIXED_AUDIO_FILENAME picks a container that stores
// the delay as an edit list instead; this asserts the artifact we actually
// ship lands on time.
const projectDir = mkdtempSync(join(tmpdir(), "hf-onset-"));
const workDir = mkdtempSync(join(tmpdir(), "hf-onset-work-"));
tempDirs.push(projectDir, workDir);
const sourcePath = join(projectDir, "tone.wav");
const outputPath = join(projectDir, MIXED_AUDIO_FILENAME);
const setup = spawnSync(
getFfmpegBinary(),
[
"-nostdin",
"-v",
"error",
"-f",
"lavfi",
"-i",
"sine=frequency=1000:duration=1:sample_rate=48000",
"-c:a",
"pcm_s16le",
sourcePath,
],
{ encoding: "utf-8" },
);
expect(setup.status, setup.stderr).toBe(0);

const result = await processCompositionAudio(
[
{
id: "tone",
src: "tone.wav",
start: 2,
end: 3,
mediaStart: 0,
layer: 0,
volume: 1,
type: "audio",
},
],
projectDir,
workDir,
outputPath,
4,
);

expect(result.success).toBe(true);
expect(firstAudibleSeconds(outputPath)).toBeCloseTo(2, 2);
});
});
15 changes: 15 additions & 0 deletions packages/engine/src/services/audioMixer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@ import { applyVolumeEnvelopeToWav } from "./audioVolumeEnvelope.js";

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

/**
* Filename every caller must use for the mixed-audio artifact.
*
* The extension is load-bearing, not cosmetic: FFmpeg picks the muxer from it,
* and the mix is AAC-encoded. A raw ADTS `.aac` stream has nowhere to record
* the encoder's priming delay, so those leading samples decode as real silence
* and shift the whole track ~1024 samples (21.33 ms at 48 kHz) late against a
* frame-accurate video track. An MP4-family container carries the delay as an
* edit list, which every decoder then strips, so the mix lands on its authored
* start. Keep the choice here rather than at each call site: the same file is
* muxed into the video, shipped in a distributed plan, and handed to users as
* the PNG-sequence sidecar, and all three have to agree.
*/
export const MIXED_AUDIO_FILENAME = "audio.m4a";

function clampVolume(volume: number): number {
if (!Number.isFinite(volume)) return 1;
return Math.max(0, Math.min(1, volume));
Expand Down
4 changes: 2 additions & 2 deletions packages/gcp-cloud-run/src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function makeMinimalV1PlanDir(dir: string, withAudio: boolean): void {
JSON.stringify([{ index: 0, startFrame: 0, endFrame: 30 }]),
);
writeFileSync(join(dir, "meta", "encoder.json"), "{}");
if (withAudio) writeFileSync(join(dir, "audio.aac"), "AAC");
if (withAudio) writeFileSync(join(dir, "audio.m4a"), "AAC");
planJson.planHash = recomputePlanHashFromPlanDir(dir);
writeFileSync(join(dir, "plan.json"), JSON.stringify(planJson));
}
Expand Down Expand Up @@ -273,7 +273,7 @@ describe("dispatch", () => {
chunkIndex: number,
outputBase: string,
): Promise<ChunkResult> => {
expect(existsSync(join(planDir, "audio.aac"))).toBe(false);
expect(existsSync(join(planDir, "audio.m4a"))).toBe(false);
writeFileSync(outputBase, `chunk-${chunkIndex}`);
return {
outputPath: outputBase,
Expand Down
13 changes: 8 additions & 5 deletions packages/gcp-cloud-run/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
listPlanV2ArtifactsForTarget,
materializePlanV2Target,
plan,
PLAN_AUDIO_RELATIVE_PATH,
planV2WithPublisher,
type PlanResult,
type PlanV2Artifact,
Expand Down Expand Up @@ -322,7 +323,7 @@ async function handlePlan(event: PlanEvent, deps?: HandlerDeps): Promise<PlanRes

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

Expand Down Expand Up @@ -397,7 +398,7 @@ async function handlePlanV2(
Width: manifest.width,
Height: manifest.height,
Format: manifest.format,
HasAudio: manifest.artifacts.some((artifact) => artifact.path === "audio.aac"),
HasAudio: manifest.artifacts.some((artifact) => artifact.path === PLAN_AUDIO_RELATIVE_PATH),
AudioGcsUri: null,
FfmpegVersion: manifest.ffmpegVersion,
ProducerVersion: manifest.producerVersion,
Expand Down Expand Up @@ -567,7 +568,7 @@ async function handleAssemble(
// only for backward compatibility with an older Plan that uploaded it
// standalone.
let audioPath: string | null = null;
const planAudio = join(planDir, "audio.aac");
const planAudio = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
if (existsSync(planAudio) && statSync(planAudio).size > 0) {
audioPath = planAudio;
} else if (event.AudioGcsUri) {
Expand Down Expand Up @@ -619,7 +620,9 @@ async function handleAssembleV2(
const work = mkdtempSync(join(deps?.tmpRoot ?? tmpdir(), "hf-cr-assemble-v2-"));
try {
const planDir = await downloadAndMaterializePlanV2(storage, event, { role: "assembler" }, work);
const audioPath = existsSync(join(planDir, "audio.aac")) ? join(planDir, "audio.aac") : null;
const audioPath = existsSync(join(planDir, PLAN_AUDIO_RELATIVE_PATH))
? join(planDir, PLAN_AUDIO_RELATIVE_PATH)
: null;
const chunkPaths = await downloadChunkObjects(storage, event.ChunkGcsUris, work, event.Format);
const finalOutput =
event.Format === "png-sequence"
Expand Down
7 changes: 7 additions & 0 deletions packages/producer/src/distributed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ export {
// CLI / adopter SDKs can derive runtime allowlists from one source.
export { PlanVideosMetadataError, type DistributedFormat } from "./services/distributed/shared.js";

// ── Plan artifact names ─────────────────────────────────────────────────────
// The cloud adapters locate and publish the plan's audio artifact by name. Its
// extension selects the container, so they must read it from here rather than
// restate it: a literal that drifts from the writer's is a silently missing
// audio track, not a loud failure.
export { PLAN_AUDIO_RELATIVE_PATH } from "./services/distributed/shared.js";

// ── Plan-time shared types from `freezePlan` ───────────────────────────────
// Re-exported so adopters that deserialize a planDir's `meta/encoder.json`
// or `meta/chunks.json` see the same shapes the producer wrote them as.
Expand Down
6 changes: 3 additions & 3 deletions packages/producer/src/regression-harness-distributed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { existsSync, mkdirSync } from "node:fs";
import { join } from "node:path";
import type { Fps } from "@hyperframes/core";
import { assemble, plan, renderChunk } from "./distributed.js";
import type { DistributedFormat } from "./services/distributed/shared.js";
import { PLAN_AUDIO_RELATIVE_PATH, type DistributedFormat } from "./services/distributed/shared.js";

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

// Step C: assemble. `audio.aac` only exists when the composition has
// Step C: assemble. The audio artifact only exists when the composition has
// audio — pass null otherwise so `assemble()` doesn't try to mux silence.
const audioPath = join(planDir, "audio.aac");
const audioPath = join(planDir, PLAN_AUDIO_RELATIVE_PATH);
const audioForAssemble = existsSync(audioPath) ? audioPath : null;
await assemble(planDir, chunkPaths, audioForAssemble, input.renderedOutputPath);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/producer/src/services/distributed/assemble.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ afterAll(() => {
/**
* Build a synthetic planDir whose `meta/chunks.json` declares N chunks of
* `framesPerChunk` frames each. Does NOT materialize compiled/, video-frames/,
* audio.aac — assemble only reads `plan.json` + `meta/chunks.json`, and we
* audio.m4a — assemble only reads `plan.json` + `meta/chunks.json`, and we
* pass chunk paths explicitly. Keeping the dir lean speeds up the test
* loop.
*/
Expand Down Expand Up @@ -287,7 +287,7 @@ describe("assemble()", () => {
);

it(
"muxes audio with frame-count-derived duration when audio.aac is present",
"muxes audio with frame-count-derived duration when audio.m4a is present",
async () => {
if (!hasFfmpeg) return;

Expand All @@ -301,7 +301,7 @@ describe("assemble()", () => {

const chunkAPath = join(planDir, "chunk-0.mp4");
const chunkBPath = join(planDir, "chunk-1.mp4");
const audioPath = join(planDir, "audio.aac");
const audioPath = join(planDir, "audio.m4a");
makeMp4Chunk(chunkAPath, 6);
makeMp4Chunk(chunkBPath, 6);
// Audio is half a second longer than the video — `padOrTrimAudioToVideoFrameCount`
Expand Down Expand Up @@ -346,7 +346,7 @@ describe("assemble()", () => {

const chunkAPath = join(planDir, "chunk-0.mp4");
const chunkBPath = join(planDir, "chunk-1.mp4");
const audioPath = join(planDir, "audio.aac");
const audioPath = join(planDir, "audio.m4a");
makeMp4Chunk(chunkAPath, 6);
makeMp4Chunk(chunkBPath, 6);
// Audio is shorter than the video, forcing the distributed pad branch.
Expand Down
Loading
Loading