Skip to content

Commit 9c6f0a3

Browse files
vanceingallsclaude
andauthored
fix(studio-server): give every freeze still its own immutable name
Two clip ids that sanitise alike (a.b / a_b), ids sharing an 80-character prefix, or a second freeze of one clip at the same playhead all produced the same still path, and ffmpeg's -y replaced the first still's pixels. The name now carries a hash of the raw id and a per-extraction token, the route refuses a name that already exists, and ffmpeg runs with -n so it can never overwrite. The freeze media time reads data-playback-start before data-media-start, as playback does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 71c6eee commit 9c6f0a3

4 files changed

Lines changed: 163 additions & 27 deletions

File tree

‎packages/studio-server/src/helpers/freezeFrame.test.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ describe("freezeFrameMediaTime", () => {
6161
});
6262

6363
describe("freezeExtractArgs", () => {
64-
it("seeks before the input and writes one frame", () => {
64+
it("seeks before the input and writes one frame, never over an existing file", () => {
6565
expect(freezeExtractArgs("/p/a.mp4", 7.2004, "/p/assets/freeze/a-3200.png")).toEqual([
66-
"-y",
66+
"-n",
6767
"-ss",
6868
"7.2",
6969
"-i",
@@ -98,6 +98,14 @@ describe("readFreezeSource", () => {
9898
});
9999
});
100100

101+
it("reads the in-point as playback does: data-playback-start before data-media-start", () => {
102+
const both = project.replace(
103+
'data-media-start="2"',
104+
'data-media-start="2" data-playback-start="5"',
105+
);
106+
expect(readFreezeSource(both, { id: "talk" }, 3.2)?.mediaTime).toBe(7.2);
107+
});
108+
101109
it("refuses a playhead outside the clip or a non-video", () => {
102110
expect(readFreezeSource(project, { id: "talk" }, 0.5)).toBeNull();
103111
expect(readFreezeSource(project, { id: "later" }, 8.5)).toBeNull();
@@ -156,9 +164,23 @@ describe("applyFreezeFrameToHtml", () => {
156164

157165
describe("freezeStillFileName", () => {
158166
it("reduces a clip id to one safe filename component", () => {
159-
expect(freezeStillFileName("talk", 2.5)).toBe("talk-2500.png");
160-
expect(freezeStillFileName("../../etc/x", 1)).toBe("______etc_x-1000.png");
161-
expect(freezeStillFileName("a\\b:c", 1)).toBe("a_b_c-1000.png");
162-
expect(freezeStillFileName("", 1)).toBe("clip-1000.png");
167+
expect(freezeStillFileName("talk", 2.5, "t0")).toMatch(/^talk-[0-9a-f]{10}-2500-t0\.png$/);
168+
expect(freezeStillFileName("../../etc/x", 1, "t0")).toMatch(
169+
/^______etc_x-[0-9a-f]{10}-1000-t0\.png$/,
170+
);
171+
expect(freezeStillFileName("a\\b:c", 1, "t0")).toMatch(/^a_b_c-[0-9a-f]{10}-1000-t0\.png$/);
172+
expect(freezeStillFileName("", 1, "t0")).toMatch(/^clip-[0-9a-f]{10}-1000-t0\.png$/);
173+
});
174+
175+
it("keeps ids that sanitise or truncate alike apart", () => {
176+
expect(freezeStillFileName("a.b", 2.5, "t0")).not.toBe(freezeStillFileName("a_b", 2.5, "t0"));
177+
const prefix = "v".repeat(80);
178+
expect(freezeStillFileName(`${prefix}1`, 2.5, "t0")).not.toBe(
179+
freezeStillFileName(`${prefix}2`, 2.5, "t0"),
180+
);
181+
});
182+
183+
it("names every extraction of one clip at one time differently", () => {
184+
expect(freezeStillFileName("talk", 2.5)).not.toBe(freezeStillFileName("talk", 2.5));
163185
});
164186
});

‎packages/studio-server/src/helpers/freezeFrame.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { createHash, randomBytes } from "node:crypto";
12
import { ensureHfIds } from "@hyperframes/parsers/hf-ids";
3+
import { readMediaOffsetSeconds } from "@hyperframes/parsers/media-duration";
24
import { resolveRateSpec, sourceTimeAt } from "@hyperframes/core/speed-ramp";
35
import {
46
findTargetElement,
@@ -38,12 +40,19 @@ export function freezeFrameMediaTime(input: {
3840
}
3941

4042
export function freezeExtractArgs(src: string, mediaTime: number, output: string): string[] {
41-
return ["-y", "-ss", String(round3(mediaTime)), "-i", src, "-frames:v", "1", output];
43+
return ["-n", "-ss", String(round3(mediaTime)), "-i", src, "-frames:v", "1", output];
4244
}
4345

44-
export function freezeStillFileName(clipId: string, playhead: number): string {
45-
const stem = clipId.replace(/[^A-Za-z0-9_-]/g, "_").slice(0, 80) || "clip";
46-
return `${stem}-${Math.round(playhead * 1000)}.png`;
46+
export const randomStillToken = (): string => randomBytes(4).toString("hex");
47+
48+
export function freezeStillFileName(
49+
clipId: string,
50+
playhead: number,
51+
token: string = randomStillToken(),
52+
): string {
53+
const stem = clipId.replace(/[^A-Za-z0-9_-]/g, "_").slice(0, 48) || "clip";
54+
const idHash = createHash("sha256").update(clipId).digest("hex").slice(0, 10);
55+
return `${stem}-${idHash}-${Math.round(playhead * 1000)}-${token}.png`;
4756
}
4857

4958
export interface FreezeSource {
@@ -72,7 +81,7 @@ export function readFreezeSource(
7281
mediaTime: freezeFrameMediaTime({
7382
clipStart: start,
7483
playhead,
75-
mediaStart: numberAttr(el, "data-media-start") ?? numberAttr(el, "data-playback-start") ?? 0,
84+
mediaStart: readMediaOffsetSeconds((name) => el.getAttribute(name)),
7685
playbackRate: numberAttr(el, "data-playback-rate") ?? 1,
7786
automation: el.getAttribute("data-automation"),
7887
}),

‎packages/studio-server/src/routes/freezeFrame.test.ts‎

Lines changed: 109 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "vitest";
22
import { Hono } from "hono";
33
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
44
import { tmpdir } from "node:os";
5-
import { dirname, join } from "node:path";
5+
import { basename, dirname, join } from "node:path";
66
import { registerFreezeFrameRoutes, type FrameExtractor } from "./freezeFrame";
77
import { fileContentVersion } from "../helpers/fileVersion";
88
import type { StudioApiAdapter } from "../types";
@@ -16,7 +16,11 @@ const html = `<div data-composition-id="main" data-start="0" data-duration="6">
1616
<video id="talk" class="clip" src="media/talk.mp4" data-start="0" data-duration="6" data-track-index="0"></video>
1717
</div>`;
1818

19-
function setup(extract: FrameExtractor, file = { path: "index.html", html }) {
19+
function setup(
20+
extract: FrameExtractor,
21+
file = { path: "index.html", html },
22+
stillToken?: () => string,
23+
) {
2024
const dir = mkdtempSync(join(tmpdir(), "hf-freeze-"));
2125
tempDirs.push(dir);
2226
mkdirSync(dirname(join(dir, file.path)), { recursive: true });
@@ -31,7 +35,7 @@ function setup(extract: FrameExtractor, file = { path: "index.html", html }) {
3135
startRender: () => ({ id: "j", status: "rendering", progress: 0, outputPath: "/tmp/o.mp4" }),
3236
};
3337
const app = new Hono();
34-
registerFreezeFrameRoutes(app, adapter, extract);
38+
registerFreezeFrameRoutes(app, adapter, extract, stillToken);
3539
const post = (body: unknown) =>
3640
app.request("http://localhost/projects/demo/file-mutations/freeze-frame", {
3741
method: "POST",
@@ -56,17 +60,20 @@ describe("freeze-frame route", () => {
5660
});
5761
const body: { before?: string; after?: string; imageSrc?: string } = await res.json();
5862
expect(res.status).toBe(200);
63+
const output = calls[0]?.at(-1) ?? "";
5964
expect(calls[0]).toEqual([
60-
"-y",
65+
"-n",
6166
"-ss",
6267
"2.5",
6368
"-i",
6469
join(dir, "media/talk.mp4"),
6570
"-frames:v",
6671
"1",
67-
join(dir, "assets/freeze/talk-2500.png"),
72+
output,
6873
]);
69-
expect(body.imageSrc).toBe("assets/freeze/talk-2500.png");
74+
expect(dirname(output)).toBe(join(dir, "assets/freeze"));
75+
expect(basename(output)).toMatch(/^talk-[0-9a-f]{10}-2500-[0-9a-f]{8}\.png$/);
76+
expect(body.imageSrc).toBe(`assets/freeze/${basename(output)}`);
7077
expect(body.before).toBe(html);
7178
expect(readFileSync(join(dir, "index.html"), "utf-8")).toBe(body.after);
7279
expect(body.after).toContain('id="talk-freeze"');
@@ -118,11 +125,102 @@ describe("freeze-frame route", () => {
118125
});
119126
const body: { imageSrc?: string; after?: string } = await res.json();
120127
expect(res.status).toBe(200);
121-
const output = calls[0]?.at(-1);
122-
expect(output).toBe(join(dir, "assets/freeze/____________outside_frame-2500.png"));
123-
expect(dirname(output ?? "")).toBe(join(dir, "assets/freeze"));
124-
expect(body.imageSrc).toBe("../assets/freeze/____________outside_frame-2500.png");
125-
expect(body.after).toContain('src="../assets/freeze/____________outside_frame-2500.png"');
128+
const output = calls[0]?.at(-1) ?? "";
129+
expect(basename(output)).toMatch(/^____________outside_frame-[0-9a-f]{10}-2500-/);
130+
expect(dirname(output)).toBe(join(dir, "assets/freeze"));
131+
expect(body.imageSrc).toBe(`../assets/freeze/${basename(output)}`);
132+
expect(body.after).toContain(`src="../assets/freeze/${basename(output)}"`);
126133
expect(existsSync(join(dir, "..", "outside"))).toBe(false);
127134
});
135+
136+
describe("still identity", () => {
137+
const twoVideos = (
138+
a: string,
139+
b: string,
140+
) => `<div data-composition-id="main" data-start="0" data-duration="6">
141+
<video id="${a}" class="clip" src="media/a.mp4" data-start="0" data-duration="6" data-track-index="0"></video>
142+
<video id="${b}" class="clip" src="media/b.mp4" data-start="0" data-duration="6" data-track-index="1"></video>
143+
</div>`;
144+
145+
function writingExtractor(outputs: string[]): FrameExtractor {
146+
return async (args) => {
147+
const output = args.at(-1) ?? "";
148+
outputs.push(output);
149+
await new Promise((resolve) => setTimeout(resolve, 5));
150+
try {
151+
writeFileSync(output, `frame of ${args[4]} #${outputs.length}`, {
152+
flag: args.includes("-y") ? "w" : "wx",
153+
});
154+
return { ok: true };
155+
} catch (error) {
156+
return { ok: false, error: String(error) };
157+
}
158+
};
159+
}
160+
161+
function freeze(post: (body: unknown) => Promise<Response>, dir: string, id: string) {
162+
return post({
163+
path: "index.html",
164+
expectedVersion: fileContentVersion(readFileSync(join(dir, "index.html"), "utf-8")),
165+
target: { id },
166+
playhead: 2.5,
167+
});
168+
}
169+
170+
it("gives two ids that sanitise alike distinct stills and keeps the first one's pixels", async () => {
171+
const outputs: string[] = [];
172+
const source = twoVideos("a.b", "a_b");
173+
const { dir, post } = setup(writingExtractor(outputs), { path: "index.html", html: source });
174+
expect((await freeze(post, dir, "a.b")).status).toBe(200);
175+
const first = readFileSync(outputs[0] ?? "", "utf-8");
176+
expect((await freeze(post, dir, "a_b")).status).toBe(200);
177+
expect(outputs[1]).not.toBe(outputs[0]);
178+
expect(readFileSync(outputs[0] ?? "", "utf-8")).toBe(first);
179+
});
180+
181+
it("keeps ids sharing an 80-character prefix apart", async () => {
182+
const outputs: string[] = [];
183+
const prefix = "v".repeat(80);
184+
const source = twoVideos(`${prefix}1`, `${prefix}2`);
185+
const { dir, post } = setup(writingExtractor(outputs), { path: "index.html", html: source });
186+
expect((await freeze(post, dir, `${prefix}1`)).status).toBe(200);
187+
expect((await freeze(post, dir, `${prefix}2`)).status).toBe(200);
188+
expect(new Set(outputs).size).toBe(2);
189+
});
190+
191+
it("writes a new still when the same clip is frozen again at the same time", async () => {
192+
const outputs: string[] = [];
193+
const { dir, post } = setup(writingExtractor(outputs));
194+
expect((await freeze(post, dir, "talk")).status).toBe(200);
195+
const first = readFileSync(outputs[0] ?? "", "utf-8");
196+
writeFileSync(join(dir, "index.html"), html);
197+
expect((await freeze(post, dir, "talk")).status).toBe(200);
198+
expect(outputs[1]).not.toBe(outputs[0]);
199+
expect(readFileSync(outputs[0] ?? "", "utf-8")).toBe(first);
200+
});
201+
202+
it("gives concurrent requests distinct stills", async () => {
203+
const outputs: string[] = [];
204+
const { dir, post } = setup(writingExtractor(outputs));
205+
const results = await Promise.all([freeze(post, dir, "talk"), freeze(post, dir, "talk")]);
206+
expect(results.map((res) => res.status).sort()).toEqual([200, 409]);
207+
expect(new Set(outputs).size).toBe(2);
208+
expect(outputs.map((output) => readFileSync(output, "utf-8"))).toEqual([
209+
expect.stringContaining("#"),
210+
expect.stringContaining("#"),
211+
]);
212+
});
213+
214+
it("refuses, without extracting, when the still's name is already taken", async () => {
215+
const outputs: string[] = [];
216+
const { dir, post } = setup(writingExtractor(outputs), undefined, () => "fixed");
217+
expect((await freeze(post, dir, "talk")).status).toBe(200);
218+
const first = readFileSync(outputs[0] ?? "", "utf-8");
219+
writeFileSync(join(dir, "index.html"), html);
220+
expect((await freeze(post, dir, "talk")).status).toBe(409);
221+
expect(outputs).toHaveLength(1);
222+
expect(readFileSync(outputs[0] ?? "", "utf-8")).toBe(first);
223+
expect(readFileSync(join(dir, "index.html"), "utf-8")).toBe(html);
224+
});
225+
});
128226
});

‎packages/studio-server/src/routes/freezeFrame.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { execFile } from "node:child_process";
2-
import { readFileSync, statSync } from "node:fs";
2+
import { existsSync, readFileSync, statSync } from "node:fs";
33
import { dirname, join, relative, sep } from "node:path";
44
import type { Hono } from "hono";
55
import { findFfBinary } from "@hyperframes/parsers/ff-binaries";
@@ -16,6 +16,7 @@ import {
1616
applyFreezeFrameToHtml,
1717
freezeExtractArgs,
1818
freezeStillFileName,
19+
randomStillToken,
1920
readFreezeSource,
2021
type FreezeSource,
2122
} from "../helpers/freezeFrame.js";
@@ -84,19 +85,21 @@ async function extractStill(
8485
absPath: string,
8586
source: FreezeSource,
8687
playhead: number,
87-
extract: FrameExtractor,
88+
tools: { extract: FrameExtractor; stillToken: () => string },
8889
): Promise<{ imageSrc: string } | Failure> {
8990
const fileDir = dirname(absPath);
9091
const mediaPath = pinWithinProject(projectDir, relative(projectDir, join(fileDir, source.src)));
9192
if (!mediaPath) return { error: `forbidden media path: ${source.src}`, status: 403 };
9293
const freezeDir = join(projectDir, ...FREEZE_DIR);
93-
const fileName = freezeStillFileName(source.id, playhead);
94+
const fileName = freezeStillFileName(source.id, playhead, tools.stillToken());
9495
mkdirWithinProject(projectDir, freezeDir);
9596
const imagePath = pinWithinProject(projectDir, join(...FREEZE_DIR, fileName));
9697
if (!imagePath || dirname(imagePath) !== freezeDir) {
9798
return { error: `forbidden freeze path: ${fileName}`, status: 403 };
9899
}
99-
const extracted = await extract(freezeExtractArgs(mediaPath, source.mediaTime, imagePath));
100+
if (existsSync(imagePath))
101+
return { error: `freeze still already exists: ${fileName}`, status: 409 };
102+
const extracted = await tools.extract(freezeExtractArgs(mediaPath, source.mediaTime, imagePath));
100103
if (!extracted.ok) {
101104
return {
102105
error: `Could not extract the frame: ${extracted.error ?? "ffmpeg failed"}`,
@@ -132,6 +135,7 @@ export function registerFreezeFrameRoutes(
132135
api: Hono,
133136
adapter: StudioApiAdapter,
134137
extract: FrameExtractor = ffmpegExtractor,
138+
stillToken: () => string = randomStillToken,
135139
): void {
136140
// A straight line of request guards, each its own early return.
137141
// fallow-ignore-next-line complexity
@@ -151,7 +155,10 @@ export function registerFreezeFrameRoutes(
151155

152156
const source = readFreezeSource(before, body.target, body.playhead);
153157
if (!source) return c.json({ error: "Move the playhead inside a video clip to freeze" }, 400);
154-
const still = await extractStill(project.dir, absPath, source, body.playhead, extract);
158+
const still = await extractStill(project.dir, absPath, source, body.playhead, {
159+
extract,
160+
stillToken,
161+
});
155162
if ("error" in still) return c.json({ error: still.error }, still.status);
156163
const { imageSrc } = still;
157164
const folded = applyFreezeFrameToHtml(before, {

0 commit comments

Comments
 (0)