Skip to content

Remove legacy ffmpeg frame fallback #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adilei merged 1 commit into from
Aug 24, 2026
Merged

Remove legacy ffmpeg frame fallback #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 3 additions & 13 deletions electron/describer/describer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { existsSync, readFileSync, statSync, writeFileSync } from "node:fs";
import path from "node:path";

import { CAPTURED_FRAME_MANIFEST_VERSION } from "../../common/frames";
import { approveAll, CopilotClient, type CopilotSession } from "@github/copilot-sdk";

import {
Expand Down Expand Up @@ -39,11 +38,9 @@ const NUDGE_PROMPT =
const msg = (err: unknown) => (err instanceof Error ? err.message : String(err));

interface VideoMeta {
file: string;
startEpoch: number;
durationMs: number;
framesFile?: string;
framesVersion?: number;
}

/** A live, resumable analysis session for one recording. */
Expand Down Expand Up @@ -372,17 +369,10 @@ export class Describer {
function buildExtractor(sessionDir: string): FrameExtractor | null {
const video = readJson<VideoMeta>(path.join(sessionDir, "video.json"));
if (!video) return null;
const videoPath = path.join(sessionDir, video.file);
const capturedFramesPath = video.framesFile
? path.join(sessionDir, video.framesFile)
: undefined;
const hasVideo = existsSync(videoPath);
const hasCapturedFrames = Boolean(capturedFramesPath && existsSync(capturedFramesPath));
if (!hasVideo && !hasCapturedFrames) return null;
const capturedFramesPath = video.framesFile && path.join(sessionDir, video.framesFile);
if (!capturedFramesPath || !existsSync(capturedFramesPath)) return null;
return new FrameExtractor({
...(hasVideo ? { videoPath } : {}),
...(hasCapturedFrames ? { capturedFramesPath } : {}),
capturedFramesExpected: video.framesVersion === CAPTURED_FRAME_MANIFEST_VERSION,
capturedFramesPath,
framesDir: path.join(sessionDir, "frames"),
anchorEpochMs: video.startEpoch,
durationSec: video.durationMs > 0 ? video.durationMs / 1000 : undefined,
Expand Down
148 changes: 8 additions & 140 deletions electron/frames/extractor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { execFile, execFileSync } from "node:child_process";
import { randomUUID } from "node:crypto";
import { existsSync, readFileSync, writeFileSync } from "node:fs";
import { copyFile, unlink } from "node:fs/promises";
import { createRequire } from "node:module";
import path from "node:path";
import { promisify } from "node:util";

import {
CAPTURED_FRAME_MANIFEST_VERSION,
Expand All @@ -16,7 +14,6 @@ import { createLogger } from "../logger";

const log = createLogger("Frames");
const require = createRequire(import.meta.url);
const execFileAsync = promisify(execFile);

type Sharp = (typeof import("sharp"))["default"];
let sharpMod: Sharp | null | undefined;
Expand All @@ -37,12 +34,8 @@ interface SourceFrame extends CapturedVideoFrame {
}

export interface ExtractorOptions {
/** Retained for pre-change recordings that need a system-FFmpeg fallback. */
videoPath?: string;
/** Absolute path to the versioned source-frame manifest. */
capturedFramesPath?: string;
/** Suppress the system-FFmpeg legacy path for current recordings with no snapshots. */
capturedFramesExpected?: boolean;
framesDir: string;
/** `video.json` startEpoch — the wall-clock anchor for offset↔epoch mapping. */
anchorEpochMs: number;
Expand Down Expand Up @@ -128,12 +121,10 @@ export function sampleCapturedFrames<T extends CapturedVideoFrame>(

/**
* Extracts sparse, visually distinct JPEGs from the snapshots captured alongside
* the WebM. Pre-change sessions can still use a user-installed FFmpeg, but the
* application no longer downloads or distributes any FFmpeg binary.
* the WebM.
*/
export class FrameExtractor {
private readonly opts: {
videoPath?: string;
capturedFramesPath?: string;
framesDir: string;
anchorEpochMs: number;
Expand All @@ -144,9 +135,7 @@ export class FrameExtractor {
};
private readonly manifestPath: string;
private readonly sourceFrames: SourceFrame[];
private readonly usesCapturedFramePipeline: boolean;
private frames: FrameRecord[] = [];
private warnedLegacy = false;

constructor(opts: ExtractorOptions) {
this.opts = {
Expand All @@ -156,9 +145,6 @@ export class FrameExtractor {
frameGridSec: opts.frameGridSec ?? DEFAULTS.frameGridSec,
};
this.manifestPath = path.join(opts.framesDir, "frames.json");
this.usesCapturedFramePipeline =
opts.capturedFramesExpected === true ||
Boolean(opts.capturedFramesPath && existsSync(opts.capturedFramesPath));
this.sourceFrames = loadSourceFrames(opts.capturedFramesPath);
if (existsSync(this.manifestPath)) {
this.frames = loadRetainedFrames(this.manifestPath);
Expand Down Expand Up @@ -193,17 +179,12 @@ export class FrameExtractor {
if (seen.has(cell)) continue;
seen.add(cell);
try {
const record =
this.sourceFrames.length > 0
? await this.extractCapturedAt(
event.tMs,
source,
event.reason,
seenCapturedFiles,
)
: this.usesCapturedFramePipeline
? null
: await this.extractLegacySingle(offsetSec, source, event.reason);
const record = await this.extractCapturedAt(
event.tMs,
source,
event.reason,
seenCapturedFiles,
);
if (record) added.push(record);
} catch (err) {
log.warn(`frame extraction at ${offsetSec.toFixed(2)}s failed:`, message(err));
Expand All @@ -214,9 +195,7 @@ export class FrameExtractor {
}

async extractWindow(req: WindowRequest): Promise<FrameRecord[]> {
if (this.sourceFrames.length === 0) {
return this.usesCapturedFramePipeline ? [] : this.extractLegacyWindow(req);
}
if (this.sourceFrames.length === 0) return [];

const startMs = Math.max(this.opts.anchorEpochMs, req.startMs);
const endMs = Math.max(startMs, req.endMs);
Expand Down Expand Up @@ -327,102 +306,6 @@ export class FrameExtractor {
return true;
}

private async extractLegacyWindow(req: WindowRequest): Promise<FrameRecord[]> {
const ffmpegPath = this.legacyFfmpegPath();
const videoPath = this.opts.videoPath;
if (!ffmpegPath || !videoPath) return [];

const fps = req.fps ?? DEFAULT_WINDOW_FPS;
const startSec = this.offsetForEpoch(req.startMs);
const endSec = Math.max(startSec, this.offsetForEpoch(req.endMs));
const cap = req.maxFrames ?? DEFAULT_WINDOW_MAX_FRAMES;
const stamp = randomUUID();
const pattern = path.join(this.opts.framesDir, `probe_${stamp}_%04d.jpg`);
const filters = [`fps=${fps}`];
if (req.crop) filters.push(`crop=${req.crop.w}:${req.crop.h}:${req.crop.x}:${req.crop.y}`);

try {
await execFileAsync(
ffmpegPath,
[
"-hide_banner",
"-ss", startSec.toFixed(3),
"-to", endSec.toFixed(3),
"-i", videoPath,
"-vf", filters.join(","),
"-vsync", "vfr",
"-frames:v", String(cap),
"-q:v", "3",
pattern,
],
{ maxBuffer: 32 * 1024 * 1024 },
);
} catch (err) {
log.warn("legacy probe window failed:", message(err));
return [];
}

const added: FrameRecord[] = [];
for (let index = 1; index <= cap; index++) {
const file = path.join(
this.opts.framesDir,
`probe_${stamp}_${String(index).padStart(4, "0")}.jpg`,
);
if (!existsSync(file)) break;
const offsetSec = startSec + (index - 1) / fps;
const record = await this.keepOrDrop(
file,
offsetSec,
"probe",
req.reason ?? "probe:legacy-system-ffmpeg",
);
if (record) added.push(record);
}
this.persist();
return added;
}

private async extractLegacySingle(
offsetSec: number,
source: FrameSource,
reason?: string,
): Promise<FrameRecord | null> {
const ffmpegPath = this.legacyFfmpegPath();
const videoPath = this.opts.videoPath;
if (!ffmpegPath || !videoPath) return null;
const file = path.join(this.opts.framesDir, `${source}_${Math.round(offsetSec * 1000)}.jpg`);
try {
await execFileAsync(
ffmpegPath,
[
"-hide_banner",
"-i", videoPath,
"-ss", offsetSec.toFixed(3),
"-frames:v", "1",
"-q:v", "3",
"-y", file,
],
{ maxBuffer: 16 * 1024 * 1024 },
);
} catch (err) {
log.warn(`legacy frame at ${offsetSec.toFixed(2)}s failed:`, message(err));
return null;
}
if (!existsSync(file)) return null;
return this.keepOrDrop(file, offsetSec, source, reason);
}

private legacyFfmpegPath(): string | null {
const resolved = systemFfmpegPath();
if (!resolved && !this.warnedLegacy) {
this.warnedLegacy = true;
log.warn(
"This recording predates captured source frames. Install FFmpeg to extract legacy video frames.",
);
}
return resolved;
}

private async keepOrDrop(
file: string,
offsetSec: number,
Expand Down Expand Up @@ -566,21 +449,6 @@ function finitePositiveNumber(value: unknown): value is number {
return finiteNumber(value) && value > 0;
}

let ffmpegPath: string | null | undefined;
function systemFfmpegPath(): string | null {
if (ffmpegPath !== undefined) return ffmpegPath;
try {
const finder = process.platform === "win32" ? "where" : "which";
ffmpegPath =
execFileSync(finder, ["ffmpeg"], { encoding: "utf8" })
.trim()
.split(/\r?\n/)[0] || null;
} catch {
ffmpegPath = null;
}
return ffmpegPath;
}

async function dhash(file: string): Promise<string> {
const image = sharp();
if (!image) return "";
Expand Down
113 changes: 113 additions & 0 deletions electron/pipeline.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import assert from "node:assert/strict";
import { existsSync } from "node:fs";
import {
chmod,
copyFile,
link,
mkdir,
mkdtemp,
readFile,
rm,
writeFile,
} from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import test from "node:test";

import type { SessionBundle } from "../common/bundle";
import type { RecEvent, SessionMeta } from "../common/types";
import { processSession } from "./pipeline";

test("processSession skips legacy video-only frame extraction", async (t) => {
const root = await mkdtemp(path.join(tmpdir(), "skill-recorder-pipeline-"));
t.after(() => rm(root, { recursive: true, force: true }));

const startedAt = 1_000;
const meta: SessionMeta = {
id: "legacy-video-only",
startedAt,
stoppedAt: startedAt + 1_000,
platform: process.platform,
appVersion: "0.6.0-test",
};
const event: RecEvent = {
seq: 1,
t: 100,
epoch: startedAt + 100,
type: "app.activate",
source: "test",
payload: { app: "Example", title: "Legacy recording" },
};
await Promise.all([
writeFile(path.join(root, "session.json"), JSON.stringify(meta)),
writeFile(path.join(root, "events.jsonl"), `${JSON.stringify(event)}\n`),
writeFile(
path.join(root, "video.json"),
JSON.stringify({
file: "screen.webm",
startEpoch: startedAt,
durationMs: 1_000,
}),
),
writeFile(path.join(root, "screen.webm"), "legacy video placeholder"),
]);

const lookupMarker = path.join(root, "ffmpeg-lookup-attempted");
const restoreLookup = await installFfmpegLookupTrap(root, lookupMarker);

try {
await processSession(root);
} finally {
restoreLookup();
}

assert.equal(existsSync(lookupMarker), false, "legacy ffmpeg discovery must not run");
const bundle = JSON.parse(
await readFile(path.join(root, "bundle.json"), "utf8"),
) as SessionBundle;
assert.equal(bundle.stats.frameCount, 0);
assert.equal(bundle.stats.stepCount, 1);
assert.equal(existsSync(path.join(root, "description.md")), true);
});

function shellQuote(value: string): string {
return `'${value.replaceAll("'", "'\\''")}'`;
}

async function installFfmpegLookupTrap(root: string, marker: string): Promise<() => void> {
const binDir = path.join(root, "bin");
await mkdir(binDir);
const previousPath = process.env.PATH;
const previousNodeOptions = process.env.NODE_OPTIONS;

if (process.platform === "win32") {
const preload = path.join(root, "mark-ffmpeg-lookup.cjs");
await writeFile(
preload,
`require("node:fs").writeFileSync(${JSON.stringify(marker)}, "called");\n`,
);
const fakeWhere = path.join(binDir, "where.exe");
try {
await link(process.execPath, fakeWhere);
} catch {
await copyFile(process.execPath, fakeWhere);
}
const requirePreload = `--require=${JSON.stringify(preload)}`;
process.env.NODE_OPTIONS = [previousNodeOptions, requirePreload].filter(Boolean).join(" ");
} else {
const fakeWhich = path.join(binDir, "which");
await writeFile(
fakeWhich,
`#!/bin/sh\nprintf called > ${shellQuote(marker)}\nexit 1\n`,
);
await chmod(fakeWhich, 0o755);
}
process.env.PATH = `${binDir}${path.delimiter}${previousPath ?? ""}`;

return () => {
if (previousPath === undefined) delete process.env.PATH;
else process.env.PATH = previousPath;
if (previousNodeOptions === undefined) delete process.env.NODE_OPTIONS;
else process.env.NODE_OPTIONS = previousNodeOptions;
};
}
Loading
Loading