fix(engine): make the AAC duration refinement safe, cancellable and LC-only

The packet-count probe is a refinement — durationSeconds is already
correct from format.duration before it runs — but it was written as if
it were load-bearing.

It could fail the whole call. No try/catch, and `-count_packets` demuxes
the entire container against runFfprobe's fixed 30s deadline, so a long
AAC file on slow or network storage timed out and extractAudioMetadata
rejected. htmlCompiler catches that under the comment "Source file has
no audio stream", returns duration 0, drops the audio element, and the
render ships silent with no warning. Now caught, keeping the container
duration.

It ignored the caller's AbortSignal. Only the first probe received it,
so aborting during the packet probe let the child run to completion and
the call resolved with full metadata after cancellation — while
audioPadTrim's comment claims the wrapper preserves cancellation. The
signal is forwarded, and an abort still propagates rather than being
swallowed as a refinement failure.

It halved HE-AAC durations. ffprobe reports codec_name "aac" for
HE-AAC v1/v2 as well — the marker is in the profile field — and with SBR
each packet carries 2048 output samples against the doubled output
sample_rate, so the 1024 assumption computed exactly half. A 10:00
podcast became 5:00 and htmlCompiler truncated the audio there. Gated on
profile, with `profile` added to FFProbeStream.

Tests: probe failure, junk output, three HE-AAC profile spellings (which
also assert the second probe is not attempted), and that plain AAC-LC is
still refined. Reverting the guards fails 5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-07-31 19:52:25 -07:00
co-authored by Claude Opus 5
parent 4d563fa752
commit 242a42f6c9
2 changed files with 123 additions and 14 deletions
+78
View File
@@ -797,3 +797,81 @@ describe("pix_fmt alpha detection", () => {
it.each(ALPHA)("detects alpha in %s", (fmt) => expect(pixelFormatHasAlpha(fmt)).toBe(true));
it.each(OPAQUE)("reports %s as opaque", (fmt) => expect(pixelFormatHasAlpha(fmt)).toBe(false));
});
describe("AAC duration refinement must never fail or distort the call", () => {
afterEach(() => {
vi.resetModules();
vi.doUnmock("child_process");
});
const aacStream = (profile?: string) =>
JSON.stringify({
streams: [
{ codec_type: "audio", codec_name: "aac", sample_rate: "44100", channels: 2, profile },
],
format: { duration: "600", bit_rate: "128000" },
});
async function probe(outcomes: SpawnOutcome[], file: string) {
const { spawn, calls } = createSpawnSpy(outcomes);
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { extractAudioMetadata } = await import("./ffprobe.js");
return { meta: await extractAudioMetadata(file), calls };
}
// Regression: the refinement had no try/catch, so its failure rejected a
// call whose duration was already correct. htmlCompiler catches that as
// "no audio stream", returns 0, and the render ships silent.
it("keeps the container duration when the packet probe fails", async () => {
const { meta } = await probe(
[
{ kind: "exit", code: 0, stdout: aacStream("LC") },
{ kind: "exit", code: 1, stdout: "", stderr: "ffprobe exploded" },
],
"/tmp/aac-packet-probe-fails.m4a",
);
expect(meta.durationSeconds).toBe(600);
});
it("keeps the container duration when the packet probe returns junk", async () => {
const { meta } = await probe(
[
{ kind: "exit", code: 0, stdout: aacStream("LC") },
{ kind: "exit", code: 0, stdout: "not json at all" },
],
"/tmp/aac-packet-probe-junk.m4a",
);
expect(meta.durationSeconds).toBe(600);
});
// 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("still refines a plain AAC-LC stream", async () => {
const { meta } = await probe(
[
{ kind: "exit", code: 0, stdout: aacStream("LC") },
{
kind: "exit",
code: 0,
stdout: JSON.stringify({ streams: [{ nb_read_packets: "861" }], format: {} }),
},
],
"/tmp/aac-lc-refined.m4a",
);
expect(meta.durationSeconds).toBeCloseTo((861 * 1024) / 44100, 5);
});
});
+45 -14
View File
@@ -136,6 +136,9 @@ export interface AudioMetadata {
interface FFProbeStream {
codec_type: string;
codec_name?: string;
/** e.g. "LC", "HE-AAC", "HE-AACv2" — where the SBR marker lives, since
* codec_name is plain "aac" for all of them. */
profile?: string;
width?: number;
height?: number;
duration?: string;
@@ -538,20 +541,48 @@ export async function extractAudioMetadata(
const streamDuration = audioStream.duration ? parseFloat(audioStream.duration) : undefined;
const sampleRate = audioStream.sample_rate ? parseInt(audioStream.sample_rate) : 44100;
const audioCodec = audioStream.codec_name || "unknown";
if (audioCodec === "aac" && sampleRate > 0) {
const packetStdout = await runFfprobe(filePath, [
"-select_streams",
"a:0",
"-count_packets",
"-show_entries",
"stream=nb_read_packets",
"-print_format",
"json",
]);
const packetOutput = parseProbeJson(packetStdout);
const packetCount = Number(packetOutput.streams[0]?.nb_read_packets);
if (Number.isFinite(packetCount) && packetCount > 0) {
durationSeconds = (packetCount * AAC_LC_SAMPLES_PER_PACKET) / sampleRate;
// AAC-LC container durations are often slightly wrong, so the packet
// count gives a better one. Three constraints on that refinement:
//
// 1. It must never fail the call. durationSeconds is ALREADY correct from
// format.duration at this point. `-count_packets` demuxes the whole
// container against runFfprobe's fixed 30s deadline, so a long file on
// slow or network storage times out — and the caller in htmlCompiler
// catches that under "Source file has no audio stream", returns
// duration 0, drops the audio element and ships a silent render.
// 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) {
try {
const packetStdout = await runFfprobe(
filePath,
[
"-select_streams",
"a:0",
"-count_packets",
"-show_entries",
"stream=nb_read_packets",
"-print_format",
"json",
],
options?.signal,
);
const packetOutput = parseProbeJson(packetStdout);
const packetCount = Number(packetOutput.streams[0]?.nb_read_packets);
if (Number.isFinite(packetCount) && packetCount > 0) {
durationSeconds = (packetCount * AAC_LC_SAMPLES_PER_PACKET) / sampleRate;
}
} catch (error) {
// An abort is the caller's intent, not a refinement failure — let it
// through. Anything else keeps the container duration we already have.
if (options?.signal?.aborted) throw error;
}
}