Skip to content

Commit 2f96d5c

Browse files
jrusso1020claude
andcommitted
fix(engine,producer): URL-clamp sub-comp src paths and warn on silent extraction misses
A <video src='../assets/foo.mp4'> inside a sub-composition silently dropped from extraction; the rendered output froze on the first decoded frame for the entire clip, with no error in stdout. Root cause: browser URL resolver clamps '..' at origin root (studio preview loads fine), but path.join(projectDir, '../assets/foo.mp4') normalizes to parent-of-project/assets/foo.mp4, which usually doesn't exist. existsSync returns false, extraction is skipped, no frame lookup is built, the per-frame injector has nothing to swap, and the <video> element's first decoded frame paints every screenshot. - Adds resolveProjectRelativeSrc in videoFrameExtractor that mirrors browser clamping (literal join first, then leading '..' stripped). - Surfaces a loud stderr warning when the resolver misses. - Mirrors fix in audioMixer.ts (same bug for <audio src='../'>) and renderOrchestrator HDR probe loop. - +6 regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0e54167 commit 2f96d5c

5 files changed

Lines changed: 118 additions & 16 deletions

File tree

packages/engine/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ export {
115115
parseImageElements,
116116
extractVideoFramesRange,
117117
extractAllVideoFrames,
118+
resolveProjectRelativeSrc,
118119
getFrameAtTime,
119120
createFrameLookupTable,
120121
FrameLookupTable,

packages/engine/src/services/audioMixer.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { downloadToTemp, isHttpUrl } from "../utils/urlDownloader.js";
1212
import { DEFAULT_CONFIG, type EngineConfig } from "../config.js";
1313
import { runFfmpeg } from "../utils/runFfmpeg.js";
1414
import { unwrapTemplate } from "../utils/htmlTemplate.js";
15+
import { resolveProjectRelativeSrc } from "./videoFrameExtractor.js";
1516
import type { AudioElement, AudioTrack, MixResult } from "./audioMixer.types.js";
1617

1718
export type { AudioElement, AudioTrack, MixResult } from "./audioMixer.types.js";
@@ -325,13 +326,10 @@ export async function processCompositionAudio(
325326
}
326327
try {
327328
let srcPath = element.src;
328-
// Use isAbsolute() rather than startsWith("/"). On Windows, absolute paths
329-
// like "C:\…" are not detected by the latter, so we'd re-join them under
330-
// baseDir and produce duplicated, nonexistent paths.
331329
if (!isAbsolute(srcPath) && !isHttpUrl(srcPath)) {
332-
const fromCompiled = compiledDir ? join(compiledDir, srcPath) : null;
333-
srcPath =
334-
fromCompiled && existsSync(fromCompiled) ? fromCompiled : join(baseDir, srcPath);
330+
// Same browser-vs-filesystem path semantics as videos — see
331+
// resolveProjectRelativeSrc in videoFrameExtractor for the full why.
332+
srcPath = resolveProjectRelativeSrc(element.src, baseDir, compiledDir);
335333
}
336334

337335
if (isHttpUrl(srcPath)) {

packages/engine/src/services/videoFrameExtractor.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
parseImageElements,
1010
extractAllVideoFrames,
1111
createFrameLookupTable,
12+
resolveProjectRelativeSrc,
1213
type VideoElement,
1314
type ExtractedFrames,
1415
} from "./videoFrameExtractor.js";
@@ -23,6 +24,67 @@ import { runFfmpeg } from "../utils/runFfmpeg.js";
2324
// synthesized VFR fixture.
2425
const HAS_FFMPEG = spawnSync("ffmpeg", ["-version"]).status === 0;
2526

27+
// Regression: a long-standing footgun where `<video src="../assets/foo">`
28+
// inside a sub-composition silently dropped the video from extraction. The
29+
// browser's URL resolver clamps `..` at the served origin's root (so the
30+
// page renders fine in the studio), but `path.join(projectDir, "../assets/foo")`
31+
// normalizes to <parentOfProjectDir>/assets/foo, which doesn't exist. Result:
32+
// no extracted frames, no per-frame injection, the rendered output shows the
33+
// <video>'s first decoded frame for the whole clip duration. The resolver
34+
// now mirrors browser semantics by stripping leading `..` segments as a
35+
// fallback when the literal join doesn't exist.
36+
describe("resolveProjectRelativeSrc — sub-composition path clamping", () => {
37+
let tmp: string;
38+
39+
beforeAll(() => {
40+
tmp = mkdtempSync(join(tmpdir(), "hf-resolver-"));
41+
mkdirSync(join(tmp, "project", "assets"), { recursive: true });
42+
// Empty file is enough for existsSync — this test is about path resolution.
43+
require("node:fs").writeFileSync(join(tmp, "project", "assets", "foo.mp4"), "");
44+
});
45+
afterAll(() => {
46+
rmSync(tmp, { recursive: true, force: true });
47+
});
48+
49+
it("returns the literal join when the file exists at projectDir/src", () => {
50+
const projectDir = join(tmp, "project");
51+
expect(resolveProjectRelativeSrc("assets/foo.mp4", projectDir)).toBe(
52+
join(projectDir, "assets/foo.mp4"),
53+
);
54+
});
55+
56+
it("clamps a leading `../` (sub-comp authoring) so `../assets/foo.mp4` resolves to assets/foo.mp4", () => {
57+
const projectDir = join(tmp, "project");
58+
expect(resolveProjectRelativeSrc("../assets/foo.mp4", projectDir)).toBe(
59+
join(projectDir, "assets/foo.mp4"),
60+
);
61+
});
62+
63+
it("clamps multiple leading `../../../` segments", () => {
64+
const projectDir = join(tmp, "project");
65+
expect(resolveProjectRelativeSrc("../../../assets/foo.mp4", projectDir)).toBe(
66+
join(projectDir, "assets/foo.mp4"),
67+
);
68+
});
69+
70+
it("returns the (non-existent) base-dir path on miss so callers get a stable error message", () => {
71+
const projectDir = join(tmp, "project");
72+
expect(resolveProjectRelativeSrc("../assets/missing.mp4", projectDir)).toBe(
73+
join(projectDir, "../assets/missing.mp4"),
74+
);
75+
});
76+
77+
it("prefers compiled-dir over base-dir when the file exists in both", () => {
78+
const projectDir = join(tmp, "project");
79+
const compiledDir = join(tmp, "compiled");
80+
mkdirSync(join(compiledDir, "assets"), { recursive: true });
81+
require("node:fs").writeFileSync(join(compiledDir, "assets", "foo.mp4"), "");
82+
expect(resolveProjectRelativeSrc("assets/foo.mp4", projectDir, compiledDir)).toBe(
83+
join(compiledDir, "assets/foo.mp4"),
84+
);
85+
});
86+
});
87+
2688
describe("parseVideoElements", () => {
2789
it("parses videos without an id or data-start attribute", () => {
2890
const videos = parseVideoElements('<video src="clip.mp4"></video>');

packages/engine/src/services/videoFrameExtractor.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,37 @@ async function convertVfrToCfr(
459459
}
460460
}
461461

462+
/**
463+
* Resolve a relative `<video src>` to a filesystem path the way the browser
464+
* resolves it as a URL. Browsers clamp `..` segments at the served origin's
465+
* root; `path.join(projectDir, "../assets/foo")` does not. So a sub-comp
466+
* `<video src="../assets/foo">` loads in the page (browser clamps to
467+
* `<projectDir>/assets/foo`) but the filesystem-side resolver lands at
468+
* `<parentOfProjectDir>/assets/foo` — file missing, extraction skipped,
469+
* the rendered output shows the video's first frame for the whole clip.
470+
* Mirror the clamp here.
471+
*
472+
* Returns the first existing candidate, or the base-dir join on miss so
473+
* the caller's existsSync check produces a stable error path.
474+
*/
475+
export function resolveProjectRelativeSrc(
476+
src: string,
477+
baseDir: string,
478+
compiledDir?: string,
479+
): string {
480+
const candidates = [
481+
compiledDir && join(compiledDir, src),
482+
join(baseDir, src),
483+
...(src.startsWith("..")
484+
? (() => {
485+
const clamped = src.replace(/^(\.\.[\\/])+/, "");
486+
return [compiledDir && join(compiledDir, clamped), join(baseDir, clamped)];
487+
})()
488+
: []),
489+
].filter(Boolean) as string[];
490+
return candidates.find(existsSync) ?? join(baseDir, src);
491+
}
492+
462493
export async function extractAllVideoFrames(
463494
videos: VideoElement[],
464495
baseDir: string,
@@ -496,9 +527,7 @@ export async function extractAllVideoFrames(
496527
// baseDir and produce duplicated, nonexistent paths
497528
// (e.g. C:\tmp\hf-vfr-test-X\C:\tmp\hf-vfr-test-X\vfr_screen.mp4).
498529
if (!isAbsolute(videoPath) && !isHttpUrl(videoPath)) {
499-
const fromCompiled = compiledDir ? join(compiledDir, videoPath) : null;
500-
videoPath =
501-
fromCompiled && existsSync(fromCompiled) ? fromCompiled : join(baseDir, videoPath);
530+
videoPath = resolveProjectRelativeSrc(video.src, baseDir, compiledDir);
502531
}
503532

504533
if (isHttpUrl(videoPath)) {
@@ -508,6 +537,15 @@ export async function extractAllVideoFrames(
508537
}
509538

510539
if (!existsSync(videoPath)) {
540+
// Loud: silent miss leaves the rendered video frozen at frame 0 with
541+
// no error in stdout — extremely confusing for authors.
542+
process.stderr.write(
543+
`[hyperframes:render] WARNING: video <${video.id}> src="${video.src}" ` +
544+
`could not be resolved on disk (looked for ${videoPath}). ` +
545+
`The rendered output will show this video's first frame for the entire clip duration. ` +
546+
`If your <video> lives inside a sub-composition, prefer project-root-relative paths ` +
547+
`(e.g. src="assets/foo.mp4") over "../assets/foo.mp4".\n`,
548+
);
511549
errors.push({ videoId: video.id, error: `Video file not found: ${videoPath}` });
512550
continue;
513551
}

packages/producer/src/services/renderOrchestrator.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
type EngineConfig,
3434
resolveConfig,
3535
extractAllVideoFrames,
36+
resolveProjectRelativeSrc,
3637
type ExtractedFrames,
3738
type ExtractionPhaseBreakdown,
3839
createFrameLookupTable,
@@ -2312,13 +2313,15 @@ export async function executeRenderJob(
23122313
if (job.config.hdrMode !== "force-sdr" && composition.videos.length > 0) {
23132314
await Promise.all(
23142315
composition.videos.map(async (v) => {
2315-
let videoPath = v.src;
2316-
if (!videoPath.startsWith("/")) {
2317-
const fromCompiled = existsSync(join(compiledDir, videoPath))
2318-
? join(compiledDir, videoPath)
2319-
: join(projectDir, videoPath);
2320-
videoPath = fromCompiled;
2321-
}
2316+
// Use the shared resolver so a `<video src="../assets/foo">` in a
2317+
// sub-composition resolves the same way the browser would (see
2318+
// resolveProjectRelativeSrc in videoFrameExtractor for the full
2319+
// explanation). Without this, the HDR probe silently no-ops on
2320+
// those videos and the downstream extraction bug shows up
2321+
// amplified — videos register but never extract.
2322+
const videoPath = v.src.startsWith("/")
2323+
? v.src
2324+
: resolveProjectRelativeSrc(v.src, projectDir, compiledDir);
23222325
if (!existsSync(videoPath)) return;
23232326
const meta = await extractMediaMetadata(videoPath);
23242327
if (isHdrColorSpace(meta.colorSpace)) {

0 commit comments

Comments
 (0)