From 87e22d52414415e25a346653634ca3c6a79d4233 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:03:18 +0000 Subject: [PATCH 1/2] fix(media-server): accept frames withheld behind a lost reference frame --- ...recording-verification.integration.test.ts | 142 ++++++++++++++++++ .../src/lib/recording-verification.ts | 117 ++++++++++++++- 2 files changed, 255 insertions(+), 4 deletions(-) diff --git a/apps/media-server/src/__tests__/lib/recording-verification.integration.test.ts b/apps/media-server/src/__tests__/lib/recording-verification.integration.test.ts index 3da0952f539..9327f9cc590 100644 --- a/apps/media-server/src/__tests__/lib/recording-verification.integration.test.ts +++ b/apps/media-server/src/__tests__/lib/recording-verification.integration.test.ts @@ -29,6 +29,7 @@ import { inspectRecordingSources, isRetryableRecordingVerificationError, type PreverifiedVideoRun, + referenceGapsExplaining, verifyRecording, verifyRemoteRecording, verifyRemoteRecordingBytes, @@ -3907,3 +3908,144 @@ describe("remote recording object identity", () => { }, ); }); + +describe("sources missing a reference frame", () => { + async function recordingMissingPacket(name: string, dropped?: number) { + const fragments = await mkdtemp(join(directory, `${name}-`)); + await run([ + "ffmpeg", + "-v", + "error", + "-f", + "lavfi", + "-i", + "testsrc2=size=320x180:rate=30", + "-frames:v", + "120", + "-c:v", + "libx264", + "-profile:v", + "baseline", + "-preset", + "ultrafast", + "-g", + "30", + "-keyint_min", + "30", + "-sc_threshold", + "0", + "-pix_fmt", + "yuv420p", + ...(dropped === undefined + ? [] + : ["-bsf:v", `noise=drop=eq(n\\,${dropped})`]), + "-f", + "dash", + "-seg_duration", + "1", + "-init_seg_name", + "init.mp4", + "-media_seg_name", + "fragment-$Number%05d$.m4s", + join(fragments, "manifest.mpd"), + ]); + const init = join(fragments, "init.mp4"); + const segments = (await readdir(fragments)) + .filter((file) => /^fragment-\d+\.m4s$/.test(file)) + .sort() + .map((file) => join(fragments, file)); + const combined = join(directory, `${name}-combined.mp4`); + await writeFile( + combined, + Buffer.concat( + await Promise.all([init, ...segments].map((path) => readFile(path))), + ), + ); + const output = join(directory, `${name}-output.mp4`); + await muxMediaTracksToMp4(combined, null, output); + return { init, segments, combined, output }; + } + + const previous = process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS; + afterAll(() => { + if (previous === undefined) + delete process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS; + else process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS = previous; + }); + + // Packet 16 carries frame_num 0 after the counter wraps, the loss that makes + // the decoder withhold the frames behind it instead of concealing them. + test("accepts frames withheld behind a lost reference frame, in one pass or in chunks", async () => { + const recording = await recordingMissingPacket("reference-gap", 16); + expect(recording.segments.length).toBe(4); + const warn = spyOn(console, "warn"); + try { + delete process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS; + const single = await verifyRemuxedRecording( + recording.combined, + null, + recording.output, + { requireAudio: false }, + ); + process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS = "2"; + const chunked = await verifyRemuxedRecording( + recording.combined, + null, + recording.output, + { requireAudio: false }, + { init: recording.init, segments: recording.segments }, + ); + expect(single.sourcePreserved).toBe(true); + expect(single.video.frameCount).toBeLessThan(119); + expect(single.video.frameCount).toBeGreaterThan(119 - 16); + expect(chunked.video.frameCount).toBe(single.video.frameCount); + const accepted = warn.mock.calls + .map((call) => String(call[0])) + .filter((line) => line.includes("recording_reference_gaps_accepted")) + .map((line) => JSON.parse(line)); + expect(accepted).toHaveLength(2); + expect(accepted[0]).toMatchObject({ + packets: 119, + missingFrames: 119 - single.video.frameCount, + gaps: 1, + frameNumCycle: 16, + }); + } finally { + warn.mockRestore(); + delete process.env.MEDIA_SERVER_VERIFY_DECODE_CHUNKS; + } + }); + + test("explains a shortfall only with gaps that can account for it", async () => { + const clean = await recordingMissingPacket("reference-clean"); + const gap = await recordingMissingPacket("reference-gap-bound", 16); + const signal = new AbortController().signal; + expect(await referenceGapsExplaining(clean.combined, 1, signal)).toBeNull(); + expect(await referenceGapsExplaining(gap.combined, 16, signal)).toEqual({ + gaps: 1, + frameNumCycle: 16, + }); + expect(await referenceGapsExplaining(gap.combined, 17, signal)).toBeNull(); + }); + + test("needs no gap check when the decoder conceals the lost frame", async () => { + const recording = await recordingMissingPacket("reference-concealed", 20); + const warn = spyOn(console, "warn"); + try { + const verified = await verifyRemuxedRecording( + recording.combined, + null, + recording.output, + { requireAudio: false }, + ); + expect(verified.video.frameCount).toBe(119); + expect( + warn.mock.calls.some((call) => + String(call[0]).includes("recording_reference_gaps_accepted"), + ), + ).toBe(false); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/apps/media-server/src/lib/recording-verification.ts b/apps/media-server/src/lib/recording-verification.ts index 540d5a08b71..537c9ad73ea 100644 --- a/apps/media-server/src/lib/recording-verification.ts +++ b/apps/media-server/src/lib/recording-verification.ts @@ -1272,6 +1272,96 @@ async function decodePacketProvenChunks( } } +async function scanStderr( + args: string[], + signal: AbortSignal, + onLine: (line: string) => void, +): Promise { + const child = spawn("ffmpeg", args, { + stdio: ["ignore", "ignore", "pipe"], + signal, + }); + let pending = ""; + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (text: string) => { + pending += text; + const lines = pending.split("\n"); + pending = lines.pop() ?? ""; + for (const line of lines) onLine(line); + }); + const code = await new Promise((resolve) => { + child.once("error", () => resolve(null)); + child.once("close", resolve); + }); + if (pending) onLine(pending); + return code === 0; +} + +// A source missing a reference frame (a frame_num gap) decodes without any +// error, but the decoder withholds the frames after the gap until its frame +// numbering comes back around. Every packet is still proven byte for byte, so +// a shortfall the gaps fully account for is the recording as it was captured. +export async function referenceGapsExplaining( + videoPath: string, + shortfall: number, + signal: AbortSignal, +): Promise<{ gaps: number; frameNumCycle: number } | null> { + let gaps = 0; + let log2MaxFrameNum: number | undefined; + const input = ["-protocol_whitelist", "file", "-i", videoPath]; + const [decoded, parsed] = await Promise.all([ + scanStderr( + [ + "-hide_banner", + "-nostdin", + "-v", + "debug", + ...input, + "-map", + "0:v:0", + "-f", + "null", + "-", + ], + signal, + (line) => { + if (/\bFrame num gap -?\d+ -?\d+/.test(line)) gaps++; + }, + ), + scanStderr( + [ + "-hide_banner", + "-nostdin", + ...input, + "-map", + "0:v:0", + "-c", + "copy", + "-bsf:v", + "trace_headers", + "-frames:v", + "1", + "-f", + "null", + "-", + ], + signal, + (line) => { + const value = /\blog2_max_frame_num_minus4\s+\S+\s+=\s+(\d+)$/.exec( + line.trim(), + )?.[1]; + if (value !== undefined) + log2MaxFrameNum = Math.max(log2MaxFrameNum ?? 0, Number(value) + 4); + }, + ), + ]); + signal.throwIfAborted(); + if (!decoded || !parsed || gaps === 0 || log2MaxFrameNum === undefined) + return null; + const frameNumCycle = 2 ** log2MaxFrameNum; + return shortfall <= gaps * frameNumCycle ? { gaps, frameNumCycle } : null; +} + export async function verifyRemuxedRecording( videoPath: string, audioPath: string | null, @@ -1359,12 +1449,31 @@ export async function verifyRemuxedRecording( proof, ); await proof.assertUnchanged(); - if ( - result.video.frameCount !== proof.videoPackets || - Boolean(result.audio) !== proof.hasAudio - ) { + if (Boolean(result.audio) !== proof.hasAudio) { throw new Error("Recording decoded tracks do not match the encoded source"); } + if (result.video.frameCount !== proof.videoPackets) { + const shortfall = proof.videoPackets - result.video.frameCount; + const gaps = + shortfall > 0 + ? await referenceGapsExplaining(videoPath, shortfall, signal) + : null; + await proof.assertUnchanged(); + if (!gaps) { + throw new Error( + "Recording decoded tracks do not match the encoded source", + ); + } + console.warn( + JSON.stringify({ + event: "recording_reference_gaps_accepted", + packets: proof.videoPackets, + missingFrames: shortfall, + gaps: gaps.gaps, + frameNumCycle: gaps.frameNumCycle, + }), + ); + } return { ...result, video: { From deb450a6f28930fd5a44dea8350eff8db37ba975 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:25:36 +0000 Subject: [PATCH 2/2] fix(media-server): honour the verify decode thread limit in the gap check --- apps/media-server/src/lib/recording-verification.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/media-server/src/lib/recording-verification.ts b/apps/media-server/src/lib/recording-verification.ts index 537c9ad73ea..b1b9f0acd90 100644 --- a/apps/media-server/src/lib/recording-verification.ts +++ b/apps/media-server/src/lib/recording-verification.ts @@ -1316,6 +1316,12 @@ export async function referenceGapsExplaining( "-nostdin", "-v", "debug", + "-filter_threads", + "1", + "-filter_complex_threads", + "1", + "-threads", + decodeThreadsFromEnv("MEDIA_SERVER_VERIFY_DECODE_THREADS", 1), ...input, "-map", "0:v:0",