diff --git a/packages/engine/src/utils/ffprobe.test.ts b/packages/engine/src/utils/ffprobe.test.ts index 062662ee4..9c6de657e 100644 --- a/packages/engine/src/utils/ffprobe.test.ts +++ b/packages/engine/src/utils/ffprobe.test.ts @@ -212,40 +212,81 @@ describe("ffprobe missing-binary fallback", () => { expect(calls[0]?.args.slice(0, 2)).toEqual(["-v", "error"]); }); + // `profile` matters now: the packet refinement is an allowlist on AAC-LC, + // because the 1024-sample formula is wrong for LD/ELD/HE and unverified for + // the rest. An unprofiled "aac" stream deliberately keeps its container + // duration rather than being refined on an assumption. it.each([ - { name: "non-AAC metadata", codec: "mp3", packets: undefined, expected: 1.25, calls: 1 }, - { name: "valid AAC packet count", codec: "aac", packets: "783", expected: 16.704, calls: 2 }, + { + name: "non-AAC metadata", + codec: "mp3", + profile: undefined, + packets: undefined, + expected: 1.25, + calls: 1, + }, + { + name: "unprofiled AAC", + codec: "aac", + profile: undefined, + packets: "783", + expected: 1.25, + calls: 1, + }, + { + name: "valid AAC-LC packet count", + codec: "aac", + profile: "LC", + packets: "783", + expected: 16.704, + calls: 2, + }, { name: "missing AAC packet count", codec: "aac", + profile: "LC", packets: undefined, expected: 1.25, calls: 2, }, - { name: "zero AAC packet count", codec: "aac", packets: "0", expected: 1.25, calls: 2 }, + { + name: "zero AAC packet count", + codec: "aac", + profile: "LC", + packets: "0", + expected: 1.25, + calls: 2, + }, { name: "invalid AAC packet count", codec: "aac", + profile: "LC", packets: "invalid", expected: 1.25, calls: 2, }, ])( "derives audio duration for $name", - async ({ codec, packets, expected, calls: expectedCalls }) => { + async ({ codec, profile, packets, expected, calls: expectedCalls }) => { const outcomes: SpawnOutcome[] = [ { kind: "exit", code: 0, stdout: JSON.stringify({ streams: [ - { codec_type: "audio", codec_name: codec, sample_rate: "48000", channels: 2 }, + { + codec_type: "audio", + codec_name: codec, + sample_rate: "48000", + channels: 2, + profile, + }, ], format: { duration: "1.25", bit_rate: "128000" }, }), }, ]; - if (codec === "aac") { + if (codec === "aac" && profile === "LC") { outcomes.push({ kind: "exit", code: 0, @@ -553,7 +594,17 @@ describe("ffprobe option separator", () => { kind: "exit", code: 0, stdout: JSON.stringify({ - streams: [{ codec_type: "audio", codec_name: "aac", sample_rate: "48000", channels: 2 }], + streams: [ + { + codec_type: "audio", + codec_name: "aac", + // LC, so the packet-count refinement actually runs and its + // argv is covered here too. + profile: "LC", + sample_rate: "48000", + channels: 2, + }, + ], format: { duration: "1.25" }, }), }, @@ -574,8 +625,14 @@ describe("ffprobe option separator", () => { await extractAudioMetadata("/tmp/-audio.wav"); await analyzeKeyframeIntervals("/tmp/-video.mp4"); - const args = calls.flatMap((call) => [...(call.args ?? [])]); - expect(args.filter((arg) => arg === "--")).toHaveLength(3); + // Per call, not a flattened count. A total of 3 is satisfied by one call + // emitting three `--` and two emitting none — i.e. it cannot fail for + // misplacement, which is the shape of bug this exists to catch. + expect(calls.map((call) => (call.args ?? []).slice(-2))).toEqual([ + ["--", "/tmp/-audio.wav"], + ["--", "/tmp/-audio.wav"], + ["--", "/tmp/-video.mp4"], + ]); }); }); @@ -847,18 +904,47 @@ describe("AAC duration refinement must never fail or distort the call", () => { // Regression: codec_name is "aac" for HE-AAC too, but its packets carry // 2048 output samples — assuming 1024 halved a 10:00 podcast to 5:00. - it.each(["HE-AAC", "HE-AACv2", "he-aac"])( - "does not apply the LC packet maths to profile %s", - async (profile) => { - const { meta, calls } = await probe( - [{ kind: "exit", code: 0, stdout: aacStream(profile) }], - `/tmp/heaac-${profile}.m4a`, - ); - expect(meta.durationSeconds).toBe(600); - // The second probe is not even attempted. - expect(calls).toHaveLength(1); - }, - ); + it.each([ + "HE-AAC", + "HE-AACv2", + "he-aac", + // 512- and 480-sample framing: the 1024 multiplier overstates these by + // 2x and ~2.13x, overwriting an already-correct container duration. + "LD", + "ELD", + // Not verified for this maths, so not allowlisted. + "Main", + "SSR", + "LTP", + "xHE-AAC", + // Missing or unrecognised profile must NOT fall through to the formula — + // that is how an unknown HE spelling kept the truncation bug. + "", + "SomethingNew", + ])("does not apply the LC packet maths to profile %s", async (profile) => { + const { meta, calls } = await probe( + [{ kind: "exit", code: 0, stdout: aacStream(profile) }], + `/tmp/heaac-${profile}.m4a`, + ); + expect(meta.durationSeconds).toBe(600); + // The second probe is not even attempted. + expect(calls).toHaveLength(1); + }); + + it.each(["LC", " lc "])("still refines AAC profile %s", async (profile) => { + const { meta } = await probe( + [ + { kind: "exit", code: 0, stdout: aacStream(profile) }, + { + kind: "exit", + code: 0, + stdout: JSON.stringify({ streams: [{ nb_read_packets: "861" }], format: {} }), + }, + ], + `/tmp/aac-lc-${profile.trim()}.m4a`, + ); + expect(meta.durationSeconds).toBeCloseTo((861 * 1024) / 44100, 5); + }); it("still refines a plain AAC-LC stream", async () => { const { meta } = await probe( diff --git a/packages/engine/src/utils/ffprobe.ts b/packages/engine/src/utils/ffprobe.ts index ca6893943..acc47bfb9 100644 --- a/packages/engine/src/utils/ffprobe.ts +++ b/packages/engine/src/utils/ffprobe.ts @@ -553,13 +553,24 @@ export async function extractAudioMetadata( // 2. It must honour the caller's AbortSignal. Only the first probe // received it, so aborting during this one was ignored and the call // resolved with full metadata long after cancellation. - // 3. It must not apply to HE-AAC. ffprobe reports codec_name "aac" for - // HE-AAC v1/v2 as well — the marker is in the profile — and with SBR - // each packet carries 2048 output samples against the doubled output - // sample_rate, so assuming 1024 halves the duration. A 10:00 podcast - // became 5:00, truncating the audio at exactly half. - const isHeAac = /he-?aac|aac\s*(?:se?|v[12])\b/i.test(audioStream.profile ?? ""); - if (audioCodec === "aac" && !isHeAac && sampleRate > 0) { + // 3. It must apply ONLY to profiles whose 1024-sample framing is + // established. ffprobe reports codec_name "aac" for every AAC + // variant — the framing lives in the profile: + // + // LC 1024 samples/frame <- the only one this maths fits + // HE-AAC v1/v2 2048 output samples against a doubled sample_rate + // LD 512 + // ELD 480 + // Main/SSR/LTP 1024 nominally, but not verified here + // xHE-AAC (USAC) variable + // + // An ALLOWLIST, not a HE-AAC denylist. The denylist form let LD/ELD + // through (halving to a third of the true duration), and let an + // unknown or missing profile through too — so an unrecognised HE + // spelling preserved the exact truncation this is meant to close. + // A skipped refinement is harmless: format.duration is already correct. + const isAacLc = /^\s*LC\s*$/i.test(audioStream.profile ?? ""); + if (audioCodec === "aac" && isAacLc && sampleRate > 0) { try { const packetStdout = await runFfprobe( filePath,