diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22ebe40685..4a7e2343cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -271,10 +271,10 @@ jobs: test: name: Test - needs: [changes, producer-source-tests] - # Keep the existing required `Test` context authoritative for producer - # failures too. The dedicated producer matrix remains parallel and legible, - # while this job fails closed if either lane fails or is cancelled. + needs: [changes, producer-source-tests, audio-pad-ffmpeg7] + # Keep the existing required `Test` context authoritative for the producer + # and pinned-FFmpeg lanes too. Those stay parallel and legible on their + # own, while this job fails closed if any of them fails or is cancelled. if: always() && needs.changes.outputs.code == 'true' runs-on: ubuntu-latest timeout-minutes: 10 @@ -284,6 +284,11 @@ jobs: run: | echo "::error::Producer unit/integration tests did not succeed." exit 1 + - name: Require the pinned-FFmpeg audio lane + if: needs.audio-pad-ffmpeg7.result != 'success' + run: | + echo "::error::Audio pad chain tests on pinned FFmpeg 7.x did not succeed." + exit 1 - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: lfs: true @@ -332,6 +337,75 @@ jobs: - run: bun run --filter @hyperframes/engine build - run: bun run producer:test:${{ matrix.lane }} + # The audio pad/trim chain regressed on the FFmpeg 7.x line only: `atrim` + # reading an indefinite `apad`'s timestamps drops the last-delayed branch + # from a mix of four or more. No other lane can catch it. `Test` has no + # FFmpeg at all (its gated audio tests report as skipped), and every FFmpeg + # the other lanes do get is unaffected: `apt-get install ffmpeg` on the + # runner image is the 6.x line, and the Windows lane pins an 8.x master + # build. Both produce correct output from the broken chain, so the + # regression test passes on them whether or not the fix is present. + # + # So this lane exists to pin one build that actually fails without the fix. + # Verified on linux/amd64 against the pinned 7.1.3 below: with the fix the + # test passes, and with `buildPadToDurationFilter` reverted to the bare + # `apad,atrim=0:` it fails on the `t=0` silence assertion. + # + # Same pinning rule as .github/actions/install-ffmpeg-windows: the asset + # filename embeds the git hash, so the tag and the filename must be bumped + # together. Do not float this to `latest` — `latest` carries only 8.x and + # 9.x, neither of which reproduces the bug. + audio-pad-ffmpeg7: + name: "Audio: pad chain on FFmpeg 7.x" + needs: changes + if: needs.changes.outputs.code == 'true' + runs-on: ubuntu-latest + timeout-minutes: 15 + env: + FFMPEG_7_URL: https://github.com/BtbN/FFmpeg-Builds/releases/download/autobuild-2026-03-31-13-11/ffmpeg-n7.1.3-43-g5a1f107b4c-linux64-gpl-7.1.tar.xz + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + lfs: true + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 22 + - name: Install pinned FFmpeg 7.x + run: | + set -euo pipefail + dir="$RUNNER_TEMP/ffmpeg7" + mkdir -p "$dir" + for attempt in 1 2 3 4 5; do + if curl -fsSL --retry 3 -o "$RUNNER_TEMP/ffmpeg7.tar.xz" "$FFMPEG_7_URL"; then + break + fi + echo "download attempt $attempt failed" + if [ "$attempt" = 5 ]; then exit 1; fi + sleep $((15 * attempt)) + done + tar -xf "$RUNNER_TEMP/ffmpeg7.tar.xz" -C "$dir" --strip-components=2 --wildcards '*/bin/ffmpeg' '*/bin/ffprobe' + chmod +x "$dir/ffmpeg" "$dir/ffprobe" + "$dir/ffmpeg" -version | head -n 1 + # The engine resolves both binaries through these two variables, so + # the skip gate and the code under test agree on which build ran. + echo "HYPERFRAMES_FFMPEG_PATH=$dir/ffmpeg" >> "$GITHUB_ENV" + echo "HYPERFRAMES_FFPROBE_PATH=$dir/ffprobe" >> "$GITHUB_ENV" + - uses: ./.github/actions/prepare-ffmpeg-bin + - run: bash scripts/ci/install-workspace-dependencies.sh + - run: bun run --filter '@hyperframes/{parsers,lint,studio-server}' build + - run: bun run --cwd packages/core build + # Fail loudly rather than skipping: a silent skip here is the exact + # failure this lane was added to remove. + - name: Assert the pinned FFmpeg is the one under test + run: | + set -euo pipefail + test -x "$HYPERFRAMES_FFMPEG_PATH" + "$HYPERFRAMES_FFMPEG_PATH" -version | head -n 1 | grep -q 'version n7\.' + - run: >- + npx vitest run --root packages/engine + src/services/audioMixer.padTimestamps.integration.test.ts + # Tests under skills/**/*.test.mjs are bare `node --test` files with only # `node:` built-in imports. They aren't part of any workspace package, and # the main `Test` job's `code` path filter excludes `skills/**`, so without diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index b8f8177f3b..f08fcfa70c 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -218,6 +218,7 @@ export { parseAudioElements, processCompositionAudio, } from "./services/audioMixer.js"; +export { buildPadToDurationFilter } from "./services/audioPadFilter.js"; export { cloneCaptureWarning, cloneCaptureWarnings } from "./services/captureWarning.js"; export type { AudioElement, diff --git a/packages/engine/src/services/audioMixer.padTimestamps.integration.test.ts b/packages/engine/src/services/audioMixer.padTimestamps.integration.test.ts new file mode 100644 index 0000000000..15defa424f --- /dev/null +++ b/packages/engine/src/services/audioMixer.padTimestamps.integration.test.ts @@ -0,0 +1,149 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { getFfmpegBinary, getFfprobeBinary } from "../utils/ffmpegBinaries.js"; +import { processCompositionAudio } from "./audioMixer.js"; +import type { AudioElement } from "./audioMixer.types.js"; + +/** + * Real-FFmpeg regression test for the pad/trim branch template. + * + * `apad,atrim=0:` — an indefinite pad bounded by a downstream trim — + * behaves on FFmpeg 4.2.7 and misbehaves on 7.0.2 and newer: audio leaks to + * `t=0` from three mixed branches onward, and from four branches onward the + * branch with the largest `adelay` disappears from the mix. Nothing errors; + * the render succeeds with wrong audio, which is why only an output + * measurement catches it. + * + * Three or fewer clips is not enough — the dropped-branch half of the bug does + * not appear until four. This mixes five. + */ + +const dirs: string[] = []; +afterEach(() => dirs.splice(0).forEach((dir) => rmSync(dir, { recursive: true, force: true }))); + +// Resolve the same binary `processCompositionAudio` will, so the gate and the +// code under test cannot disagree under `HYPERFRAMES_FFMPEG_PATH`, which is +// exactly how you point this test at a specific FFmpeg to reproduce the bug. +const FFMPEG = getFfmpegBinary(); + +const hasFfmpeg = (() => { + try { + execFileSync(FFMPEG, ["-version"], { stdio: "ignore" }); + execFileSync(getFfprobeBinary(), ["-version"], { stdio: "ignore" }); + return true; + } catch { + return false; + } +})(); + +const TOTAL_SECONDS = 43.3; + +/** start (s), source duration (s), tone frequency (Hz) */ +const CLIPS = [ + { start: 0.3, duration: 7.01, frequency: 400 }, + { start: 19.6, duration: 0.4, frequency: 1000 }, + { start: 20.6, duration: 0.4, frequency: 1200 }, + { start: 30.0, duration: 0.4, frequency: 1500 }, + { start: 40.0, duration: 0.4, frequency: 1800 }, +] as const; + +/** + * Mean RMS in dB over a 250ms window starting at `atSeconds`. Digital silence + * reports `-inf`, which is the signal for "this branch is not in the mix". + */ +function rmsDb(path: string, atSeconds: number): number { + const probe = spawnSync( + FFMPEG, + [ + "-hide_banner", + "-v", + "info", + "-ss", + atSeconds.toFixed(3), + "-t", + "0.25", + "-i", + path, + "-af", + "astats=metadata=1", + "-f", + "null", + "-", + ], + { encoding: "utf8" }, + ); + const stderr = probe.stderr ?? ""; + const overall = stderr.slice(stderr.lastIndexOf("Overall")); + const match = /RMS level dB:\s*(\S+)/.exec(overall); + if (!match) throw new Error(`astats reported no RMS level for ${path} at ${atSeconds}s`); + const raw = match[1]!; + return raw.endsWith("inf") + ? raw.startsWith("-") + ? -Infinity + : Infinity + : Number.parseFloat(raw); +} + +describe.skipIf(!hasFfmpeg)("mixed audio branch padding", () => { + it("keeps t=0 silent and lands all five staggered clips at their offsets", async () => { + const baseDir = mkdtempSync(join(tmpdir(), "hf-padts-base-")); + const workDir = mkdtempSync(join(tmpdir(), "hf-padts-work-")); + dirs.push(baseDir, workDir); + + const elements: AudioElement[] = CLIPS.map((clip, i) => { + const src = `clip-${i}.wav`; + execFileSync(FFMPEG, [ + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + `sine=frequency=${clip.frequency}:duration=${clip.duration}`, + "-ar", + "44100", + "-ac", + "2", + "-y", + join(baseDir, src), + ]); + return { + id: `clip-${i}`, + src, + start: clip.start, + end: clip.start + clip.duration, + mediaStart: 0, + layer: 0, + volume: 1, + type: "audio", + }; + }); + + const outputPath = join(baseDir, "out.m4a"); + const result = await processCompositionAudio( + elements, + baseDir, + workDir, + outputPath, + TOTAL_SECONDS, + ); + + expect(result.error).toBeUndefined(); + expect(result.success).toBe(true); + expect(result.tracksProcessed).toBe(CLIPS.length); + + // The earliest clip starts at 0.3s, so the head of the mix is silence. + // The bug puts every clip's audio here at once, ~-29 dB. + expect(rmsDb(outputPath, 0)).toBeLessThan(-60); + + // Every clip is audible at its own offset. `-Infinity` here means the + // branch is missing from the mix entirely, which is the second half of + // the bug and only shows from four branches onward. + for (const clip of CLIPS) { + expect(rmsDb(outputPath, clip.start + 0.02)).toBeGreaterThan(-45); + } + }, 120_000); +}); diff --git a/packages/engine/src/services/audioMixer.test.ts b/packages/engine/src/services/audioMixer.test.ts index 341c9130da..b41d37d471 100644 --- a/packages/engine/src/services/audioMixer.test.ts +++ b/packages/engine/src/services/audioMixer.test.ts @@ -296,7 +296,7 @@ describe("processCompositionAudio", () => { expect(filter).toContain("volume=0"); expect(filter).toContain("[mixed]volume=1[out]"); - expect(filter).toContain("apad,atrim=0:2"); + expect(filter).toContain("apad,asetpts=N/SR/TB,atrim=0:2"); expect(filter).not.toContain("whole_dur"); expect(filter).not.toContain("normalize="); expect(filter).not.toContain("weights="); @@ -464,7 +464,7 @@ describe("processCompositionAudio", () => { // 2 s clip + the 1.9 s tail 0.6 + size * 2.6 generates. expect(filter).toContain("atrim=0:3.9,"); // And still cut at the composition's end, so a tail cannot extend the video. - expect(filter).toContain("apad,atrim=0:8"); + expect(filter).toContain("apad,asetpts=N/SR/TB,atrim=0:8"); }); it("hands the volume envelope to the FX pass instead of ducking the file after it", async () => { @@ -1076,6 +1076,9 @@ describe("processCompositionAudio", () => { // indefinite `apad` to cap the padded stream at composition duration. expect((filter?.match(/atrim=/g) ?? []).length).toBe(trackCount * 2); expect((filter?.match(/apad,/g) ?? []).length).toBe(trackCount); + // The timestamp rebuild between the two is what keeps that cap correct on + // FFmpeg 7+; without it the last-delayed track is dropped from the mix. + expect((filter?.match(/apad,asetpts=N\/SR\/TB,atrim=/g) ?? []).length).toBe(trackCount); }); it("retries with the current file-valued filter option when a nightly removes the legacy alias", async () => { diff --git a/packages/engine/src/services/audioMixer.ts b/packages/engine/src/services/audioMixer.ts index 2d856d1d7e..2153c02da2 100644 --- a/packages/engine/src/services/audioMixer.ts +++ b/packages/engine/src/services/audioMixer.ts @@ -31,6 +31,7 @@ import type { MixResult, } from "./audioMixer.types.js"; import { applyVolumeEnvelopeToWav } from "./audioVolumeEnvelope.js"; +import { buildPadToDurationFilter } from "./audioPadFilter.js"; import { HF_AUDIO_FX_ATTR, parseAudioFxChain } from "@hyperframes/core/audio-fx"; import { HF_AUDIO_AUTOMATION_ATTR, @@ -710,7 +711,7 @@ async function mixAudioTracks( const trimDuration = track.end - track.start + (track.tailSeconds ?? 0); const volumeFilter = buildVolumeExpression(track, ignoreAutomation); filterParts.push( - `[${i}:a]atrim=0:${formatFilterNumber(trimDuration)},${volumeFilter},adelay=${delayMs}|${delayMs},apad,atrim=0:${formatFilterNumber(totalDuration)}[a${i}]`, + `[${i}:a]atrim=0:${formatFilterNumber(trimDuration)},${volumeFilter},adelay=${delayMs}|${delayMs},${buildPadToDurationFilter(formatFilterNumber(totalDuration))}[a${i}]`, ); }); diff --git a/packages/engine/src/services/audioPadFilter.test.ts b/packages/engine/src/services/audioPadFilter.test.ts new file mode 100644 index 0000000000..9251e77b22 --- /dev/null +++ b/packages/engine/src/services/audioPadFilter.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { buildPadToDurationFilter } from "./audioPadFilter.js"; + +describe("buildPadToDurationFilter", () => { + it("pads indefinitely, rebuilds timestamps, then bounds at the target", () => { + expect(buildPadToDurationFilter("43.3")).toBe("apad,asetpts=N/SR/TB,atrim=0:43.3"); + }); + + it("keeps off `whole_dur`, which the bundled Windows FFmpeg builds reject", () => { + expect(buildPadToDurationFilter("5.000000")).not.toContain("whole_dur"); + }); + + it("uses the caller's number formatting verbatim", () => { + // Each call site formats seconds differently (fixed-6 in the producer, + // trimmed in the mixer). The helper must not normalize either away. + expect(buildPadToDurationFilter("5.000000")).toContain("atrim=0:5.000000"); + expect(buildPadToDurationFilter("5")).toContain("atrim=0:5"); + }); + + it("places asetpts between apad and atrim, which is the whole fix", () => { + // `atrim` reading an indefinite `apad`'s timestamps directly is what drops + // and misplaces mixed clips on FFmpeg 7+. Order is the behaviour here, so + // pin it rather than only pinning membership. + const chain = buildPadToDurationFilter("8").split(","); + expect(chain).toEqual(["apad", "asetpts=N/SR/TB", "atrim=0:8"]); + }); +}); diff --git a/packages/engine/src/services/audioPadFilter.ts b/packages/engine/src/services/audioPadFilter.ts new file mode 100644 index 0000000000..57617505f6 --- /dev/null +++ b/packages/engine/src/services/audioPadFilter.ts @@ -0,0 +1,44 @@ +/** + * Pad-to-duration filter chain, shared by every audio path that has to hold a + * stream to the composition's length. + * + * Two live call sites build this chain (the engine mixer and the producer's + * pad/trim step), plus the producer's audio extractor, which is currently + * unreferenced. Before this module they each rebuilt the string by hand, so a + * fix in one did not reach the others. + * + * ## Why the chain is three filters and not one + * + * `apad` with no duration pads indefinitely; the trailing `atrim` is what + * bounds it. That pairing is deliberate: `apad=whole_dur=` says the + * same thing in one filter, but the bundled Windows FFmpeg builds reject the + * option outright (`Error applying option 'whole_dur': Option not found`), and + * it does not bound a branch that is already *longer* than the target — an FX + * tail that overruns the composition survives `whole_dur` and is cut by + * `atrim`. + * + * On the FFmpeg 7.x line, however, `atrim` reading `apad`'s output timestamps + * directly is what broke the mix: audio leaked to `t=0` from three mixed + * branches onward, and from four branches onward the branch with the largest + * `adelay` vanished from the output entirely. Nothing errored. + * + * Measured on linux/amd64, the affected window is 7.x only: 4.2.7, 6.0.1, git + * master from 2026-05 and 8.1.1 all produce correct output from the bare + * `apad,atrim` pairing, and produce byte-identical output with the `asetpts` + * in place. So this chain is a no-op everywhere except the versions it fixes, + * and it stays because the next release to regress here cannot be predicted. + * + * `asetpts=N/SR/TB` between them rebuilds each frame's timestamp from the + * running sample count, so `atrim` sees a monotonic sample-accurate timeline + * instead of whatever the release propagates through an indefinite `apad`. + * Measured against FFmpeg 7.0.2, the output is then sample-for-sample what + * `apad=whole_dur` produces — without giving up the `atrim` bound or the + * Windows builds. + * + * @param seconds Target duration, already formatted for a filter string by the + * caller. Each call site has its own number formatting and this helper must + * not silently change it. + */ +export function buildPadToDurationFilter(seconds: string): string { + return `apad,asetpts=N/SR/TB,atrim=0:${seconds}`; +} diff --git a/packages/producer/src/services/audioExtractor.ts b/packages/producer/src/services/audioExtractor.ts index ccddf32055..f4d3d390e3 100644 --- a/packages/producer/src/services/audioExtractor.ts +++ b/packages/producer/src/services/audioExtractor.ts @@ -9,7 +9,7 @@ import { spawn } from "node:child_process"; import { existsSync, mkdirSync, rmSync, readFileSync } from "node:fs"; import { join, dirname } from "node:path"; -import { getFfmpegBinary, trackChildProcess } from "@hyperframes/engine"; +import { buildPadToDurationFilter, getFfmpegBinary, trackChildProcess } from "@hyperframes/engine"; export interface AudioElement { id: string; @@ -212,7 +212,7 @@ async function mixTracks( const trimDuration = track.duration > 0 ? track.duration : totalDuration; filterParts.push( - `[${i}:a]atrim=0:${trimDuration},volume=${track.volume},adelay=${delayMs}|${delayMs},apad,atrim=0:${totalDuration}[a${i}]`, + `[${i}:a]atrim=0:${trimDuration},volume=${track.volume},adelay=${delayMs}|${delayMs},${buildPadToDurationFilter(String(totalDuration))}[a${i}]`, ); }); diff --git a/packages/producer/src/services/render/audioPadTrim.test.ts b/packages/producer/src/services/render/audioPadTrim.test.ts index 6ea808c94b..a567d930ea 100644 --- a/packages/producer/src/services/render/audioPadTrim.test.ts +++ b/packages/producer/src/services/render/audioPadTrim.test.ts @@ -30,7 +30,7 @@ describe("buildPadTrimAudioArgs", () => { expect(plan.steps).toHaveLength(1); const args = plan.steps[0]!.args; expect(args[args.indexOf("-i") + 1]).toBe("/tmp/in.aac"); - expect(args[args.indexOf("-af") + 1]).toBe("apad,atrim=0:5.000000"); + expect(args[args.indexOf("-af") + 1]).toBe("apad,asetpts=N/SR/TB,atrim=0:5.000000"); expect(args.join(" ")).not.toContain("whole_dur"); expect(args[args.indexOf("-t") + 1]).toBe("5.000000"); expect(args[args.indexOf("-c:a") + 1]).toBe("aac"); @@ -105,7 +105,7 @@ describe("buildPadTrimAudioArgs", () => { expect(winPlan.operation).toBe("pad"); const args = winPlan.steps[0]!.args; expect(args).toContain("-af"); - expect(args[args.indexOf("-af") + 1]).toBe("apad,atrim=0:5.000000"); + expect(args[args.indexOf("-af") + 1]).toBe("apad,asetpts=N/SR/TB,atrim=0:5.000000"); expect(args.join(" ")).not.toContain("whole_dur"); }); }); diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index ae98e985f9..d7b7d7278f 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -22,6 +22,7 @@ import { spawn } from "node:child_process"; import { rmSync } from "node:fs"; import { + buildPadToDurationFilter, extractAudioMetadata, formatFfmpegError, getFfprobeBinary, @@ -109,10 +110,11 @@ export interface PadTrimAudioPlan { * sequence that materializes it. Exported separately so unit tests can pin * every branch without spawning ffmpeg. * - * - `sourceDuration < targetDuration` → pad with `apad` to the exact target - * and re-encode AAC. This decodes and re-encodes the already mixed audio; - * an earlier concat-copy shape avoided that but could not produce a - * portable result on the bundled Windows FFmpeg builds. + * - `sourceDuration < targetDuration` → pad to the exact target with the + * shared `buildPadToDurationFilter` chain and re-encode AAC. This decodes + * and re-encodes the already mixed audio; an earlier concat-copy shape + * avoided that but could not produce a portable result on the bundled + * Windows FFmpeg builds. * - `sourceDuration > targetDuration` → filter to the exact target and * re-encode AAC so packet padding cannot outlast the video. * - `|Δ| < AUDIO_DURATION_TOLERANCE_SECONDS` → no-op `copy`, but we still @@ -143,7 +145,7 @@ export function buildPadTrimAudioPlan( "-i", audioPath, "-af", - `apad,atrim=0:${targetSec}`, + buildPadToDurationFilter(targetSec), "-t", targetSec, "-c:a", @@ -170,6 +172,10 @@ export function buildPadTrimAudioPlan( "-i", audioPath, "-af", + // `PTS-STARTPTS` here, not the pad branch's `N/SR/TB`: this branch + // only has to rebase an already-bounded stream to zero, while the + // pad branch has to rebuild a timeline an indefinite `apad` made + // untrustworthy for the `atrim` that follows it. `atrim=duration=${targetSec},asetpts=PTS-STARTPTS`, // `atrim` limits decoded samples but does not cap muxer timestamps // introduced by encoder delay/flush. The output duration contract