mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-10 22:20:14 +00:00
fix(producer): decode percent-encoded video src in HDR pre-extract (#2759)
* fix(producer): decode percent-encoded video src in HDR pre-extract (PRINFRA-349) * fix(producer): decode percent-encoded src in HDR image probe The HDR image probe still hand-rolled the path join the video probe had already delegated to resolveProjectRelativeSrc, so a percent-encoded non-ASCII `<img src>` (`图1.png` -> `%E5%9B%BE1.png`) never resolved: the image never entered nativeHdrImageIds, resolveEffectiveHdrMode saw no HDR sources, and the composition rendered through the SDR fallback with wrong color -- silently, unlike the video path which errored at ffmpeg. Both probes now call resolveProjectRelativeSrc directly, with no isAbsolute() pre-check. The resolver already returns an absolute path that exists and otherwise treats a leading slash as a browser origin-root URL, so a pre-check would hand back `/assets/%E5%9B%BE1.png` undecoded and re-open the same bug for root-relative srcs. This matches planHdrResources, so the two halves of the fix can no longer disagree. Widening resolution also makes previously-unresolvable files reachable for the first time, including truncated or 0-byte assets on which ffprobe exits non-zero. These probes run inside a bare Promise.all, so an unguarded throw aborted the whole render over one unreadable image; probeColorSpaceSafely now logs and treats such a source as SDR. Tests cover percent-encoded CJK, origin-root percent-encoded CJK, compiledDir-over-projectDir precedence, and existing-absolute passthrough, with distinct projectDir/compiledDir so the precedence is actually pinned. Fault-injection verified: reintroducing the isAbsolute short-circuit fails the origin-root test. Refs PRINFRA-349 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(producer): restore the vitest runner import in extractVideosStage tests The rebase merged the new `node:fs` / `node:os` / `node:path` imports into line 1 and took the incoming side, so `import { describe, expect, it } from "vitest"` was replaced rather than kept alongside. The file still uses all three, and `bun run test:classification` regex-matches `/\bfrom\s+["']vitest["']/` to route each test file to a runner — so the file matched neither and hard-failed the gate, taking Producer unit + integration and the required Test check with it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7563b644a2
commit
13c867267e
@@ -26,11 +26,13 @@ import {
|
||||
estimateHdrExtractionBytes,
|
||||
extractHdrVideoFrames,
|
||||
getHdrExtractionReservedBytes,
|
||||
planHdrResources,
|
||||
reserveHdrExtractionBytes,
|
||||
resolveHdrExtractionActiveBudgetBytes,
|
||||
resolveHdrExtractionBudgetBytes,
|
||||
resolveHdrExtractionWindow,
|
||||
} from "./captureHdrResources.js";
|
||||
import type { CompositionMetadata } from "../shared.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
@@ -115,6 +117,57 @@ function hdrExtractionFixture(videos: VideoElement[], framesDir: string) {
|
||||
};
|
||||
}
|
||||
|
||||
function videoComposition(src: string): CompositionMetadata {
|
||||
return {
|
||||
duration: 5,
|
||||
width: 1920,
|
||||
height: 1080,
|
||||
audios: [],
|
||||
images: [],
|
||||
videos: [{ id: "a-roll", src, start: 0, end: 5, mediaStart: 0, loop: false, hasAudio: false }],
|
||||
};
|
||||
}
|
||||
|
||||
describe("planHdrResources non-ASCII src resolution (PRINFRA-349)", () => {
|
||||
it("decodes a percent-encoded CJK <video src> back to the real on-disk path", () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-hdr-cjk-"));
|
||||
try {
|
||||
const realName = "视频1.mp4";
|
||||
writeFileSync(join(projectDir, realName), "x");
|
||||
// The compiled DOM carries the URL-encoded attribute value.
|
||||
const encoded = encodeURIComponent(realName); // %E8%A7%86%E9%A2%911.mp4
|
||||
const prep = planHdrResources({
|
||||
composition: videoComposition(encoded),
|
||||
nativeHdrVideoIds: new Set(["a-roll"]),
|
||||
nativeHdrImageIds: new Set(),
|
||||
projectDir,
|
||||
compiledDir: projectDir,
|
||||
});
|
||||
// Must be the decoded filesystem path ffmpeg can open, not the %-encoded string.
|
||||
expect(prep.hdrVideoSrcPaths.get("a-roll")).toBe(join(projectDir, realName));
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves an ASCII src untouched", () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-hdr-ascii-"));
|
||||
try {
|
||||
writeFileSync(join(projectDir, "clip.mp4"), "x");
|
||||
const prep = planHdrResources({
|
||||
composition: videoComposition("clip.mp4"),
|
||||
nativeHdrVideoIds: new Set(["a-roll"]),
|
||||
nativeHdrImageIds: new Set(),
|
||||
projectDir,
|
||||
compiledDir: projectDir,
|
||||
});
|
||||
expect(prep.hdrVideoSrcPaths.get("a-roll")).toBe(join(projectDir, "clip.mp4"));
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("estimateHdrExtractionBytes", () => {
|
||||
it("sums 6 bytes per pixel per frame across videos", () => {
|
||||
// 10s @ 30fps of 1920x1080 = 300 frames * 1920*1080*6
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
queryElementStacking,
|
||||
resampleRgb48leObjectFit,
|
||||
resolveFinalFrameExtractionWindow,
|
||||
resolveProjectRelativeSrc,
|
||||
resolveVideoExtractionWindow,
|
||||
runFfmpeg,
|
||||
type TimelineExtractionWindow,
|
||||
@@ -78,7 +79,6 @@ export function planHdrResources(args: {
|
||||
nativeHdrImageIds: Set<string>;
|
||||
projectDir: string;
|
||||
compiledDir: string;
|
||||
existsSync: (p: string) => boolean;
|
||||
}): HdrResourcePrep {
|
||||
const { composition, nativeHdrVideoIds, nativeHdrImageIds, projectDir, compiledDir } = args;
|
||||
const hdrVideoIds = composition.videos
|
||||
@@ -87,12 +87,14 @@ export function planHdrResources(args: {
|
||||
const hdrVideoSrcPaths = new Map<string, string>();
|
||||
for (const v of composition.videos) {
|
||||
if (!hdrVideoIds.includes(v.id)) continue;
|
||||
let srcPath = v.src;
|
||||
if (!srcPath.startsWith("/")) {
|
||||
const fromCompiled = join(compiledDir, srcPath);
|
||||
srcPath = args.existsSync(fromCompiled) ? fromCompiled : join(projectDir, srcPath);
|
||||
}
|
||||
hdrVideoSrcPaths.set(v.id, srcPath);
|
||||
// Resolve via the shared SDR resolver so a percent-encoded `<video src>`
|
||||
// (`视频1.mp4` → `%E8%A7%86%E9%A2%911.mp4` in the compiled DOM URL) decodes
|
||||
// back to the real on-disk filename before ffmpeg sees it. The old
|
||||
// hand-rolled join passed the encoded string straight through, so HDR
|
||||
// pre-extraction failed with "No such file" on non-ASCII media
|
||||
// (PRINFRA-349 symptom c). resolveProjectRelativeSrc also handles query
|
||||
// strings, origin-root URLs, and `..` traversal identically to the SDR path.
|
||||
hdrVideoSrcPaths.set(v.id, resolveProjectRelativeSrc(v.src, projectDir, compiledDir));
|
||||
}
|
||||
const hdrVideoStartTimes = new Map<string, number>();
|
||||
for (const v of composition.videos) {
|
||||
|
||||
@@ -202,7 +202,6 @@ export async function runCaptureHdrStage(
|
||||
nativeHdrImageIds,
|
||||
projectDir,
|
||||
compiledDir,
|
||||
existsSync,
|
||||
});
|
||||
|
||||
const domSession = await createCaptureSession(
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type {
|
||||
ExtractedFrames,
|
||||
ExtractionResult,
|
||||
VideoElement,
|
||||
VideoExtractionFailure,
|
||||
} from "@hyperframes/engine";
|
||||
import { resolveProjectRelativeSrc } from "@hyperframes/engine";
|
||||
import {
|
||||
appendAutoDetectedVideoAudio,
|
||||
assertVideoExtractionSucceeded,
|
||||
@@ -121,6 +125,74 @@ describe("appendAutoDetectedVideoAudio", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// The HDR probes in this stage resolve `<video>`/`<img>` srcs with
|
||||
// resolveProjectRelativeSrc and NO isAbsolute() pre-check. These pin the src
|
||||
// shapes that a pre-check would silently break — an earlier revision of the
|
||||
// PRINFRA-349 fix short-circuited on isAbsolute and left root-relative srcs
|
||||
// percent-encoded, so the HDR image never resolved and the render shipped SDR.
|
||||
describe("HDR probe src resolution (PRINFRA-349)", () => {
|
||||
it("decodes a percent-encoded CJK src to the real on-disk path", () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-probe-cjk-"));
|
||||
const compiledDir = mkdtempSync(join(tmpdir(), "hf-probe-compiled-"));
|
||||
try {
|
||||
const realName = "图1.png";
|
||||
writeFileSync(join(projectDir, realName), "x");
|
||||
// The compiled DOM carries the URL-encoded attribute value.
|
||||
const encoded = encodeURIComponent(realName); // %E5%9B%BE1.png
|
||||
expect(resolveProjectRelativeSrc(encoded, projectDir, compiledDir)).toBe(
|
||||
join(projectDir, realName),
|
||||
);
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
rmSync(compiledDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("decodes a percent-encoded CJK src served from a browser origin-root URL", () => {
|
||||
// Regression guard: `isAbsolute("/assets/%E5%9B%BE1.png")` is true on POSIX,
|
||||
// so a pre-check would return it verbatim, existsSync would fail, and the
|
||||
// image would never enter nativeHdrImageIds — a silent SDR render.
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-probe-root-"));
|
||||
try {
|
||||
mkdirSync(join(projectDir, "assets"));
|
||||
const realName = "图1.png";
|
||||
writeFileSync(join(projectDir, "assets", realName), "x");
|
||||
const rootRelative = `/assets/${encodeURIComponent(realName)}`;
|
||||
expect(resolveProjectRelativeSrc(rootRelative, projectDir, projectDir)).toBe(
|
||||
join(projectDir, "assets", realName),
|
||||
);
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("prefers compiledDir over projectDir when both hold the asset", () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-probe-proj-"));
|
||||
const compiledDir = mkdtempSync(join(tmpdir(), "hf-probe-comp-"));
|
||||
try {
|
||||
writeFileSync(join(projectDir, "clip.mp4"), "x");
|
||||
writeFileSync(join(compiledDir, "clip.mp4"), "x");
|
||||
expect(resolveProjectRelativeSrc("clip.mp4", projectDir, compiledDir)).toBe(
|
||||
join(compiledDir, "clip.mp4"),
|
||||
);
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
rmSync(compiledDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("returns an existing absolute path unchanged", () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "hf-probe-abs-"));
|
||||
try {
|
||||
const abs = join(projectDir, "clip.mp4");
|
||||
writeFileSync(abs, "x");
|
||||
expect(resolveProjectRelativeSrc(abs, projectDir, projectDir)).toBe(abs);
|
||||
} finally {
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldCopyExtractedFrames", () => {
|
||||
it("copies frames on Windows (symlinkSync throws EPERM without Developer Mode)", () => {
|
||||
expect(shouldCopyExtractedFrames("win32")).toBe(true);
|
||||
|
||||
@@ -29,7 +29,7 @@
|
||||
*/
|
||||
|
||||
import { existsSync } from "node:fs";
|
||||
import { isAbsolute, join } from "node:path";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
type CaptureVideoMetadataHint,
|
||||
type EngineConfig,
|
||||
@@ -247,6 +247,34 @@ function applyVideoExtractionFailurePolicy(
|
||||
return policy.failureMode === "enforce" ? error : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Probe a media file's color space, returning null when it can't be read.
|
||||
*
|
||||
* Both HDR probes below widened which files they resolve (PRINFRA-349: the
|
||||
* shared resolver percent-decodes non-ASCII names, so `%E5%9B%BE1.png` now
|
||||
* finds `图1.png`). Files that used to silently fail to resolve are therefore
|
||||
* reachable for the first time — including truncated / 0-byte assets, on which
|
||||
* ffprobe exits non-zero and `extractMediaMetadata` throws. These probes run
|
||||
* inside a bare `Promise.all`, so an unguarded throw aborts the whole render
|
||||
* over one unreadable image. A probe that can't read a file must skip it, not
|
||||
* kill the render.
|
||||
*/
|
||||
async function probeColorSpaceSafely(
|
||||
path: string,
|
||||
log: ProducerLogger | undefined,
|
||||
): Promise<VideoColorSpace | null> {
|
||||
try {
|
||||
const meta = await extractMediaMetadata(path);
|
||||
return meta.colorSpace;
|
||||
} catch (error) {
|
||||
log?.warn("HDR color-space probe failed; treating source as SDR", {
|
||||
path,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function runExtractVideosStage(
|
||||
input: ExtractVideosStageInput,
|
||||
): Promise<ExtractVideosStageResult> {
|
||||
@@ -282,14 +310,14 @@ export async function runExtractVideosStage(
|
||||
log?.info("Probing video color spaces...", { videoCount: composition.videos.length });
|
||||
const probeFailures = await Promise.all(
|
||||
composition.videos.map(async (v) => {
|
||||
// Use the shared resolver so a `<video src="../assets/foo">` in a
|
||||
// sub-composition resolves the same way the browser would (see
|
||||
// resolveProjectRelativeSrc in videoFrameExtractor for the full
|
||||
// explanation). isAbsolute (not `startsWith("/")`) so Windows
|
||||
// absolute paths like `C:\...` skip the join correctly.
|
||||
const videoPath = isAbsolute(v.src)
|
||||
? v.src
|
||||
: resolveProjectRelativeSrc(v.src, projectDir, compiledDir);
|
||||
// Shared resolver so a `<video src="../assets/foo">` in a sub-composition
|
||||
// resolves the same way the browser would, and a percent-encoded
|
||||
// non-ASCII name decodes to its real on-disk path. Called with no
|
||||
// isAbsolute() pre-check: the resolver already returns an absolute path
|
||||
// that exists, and otherwise treats a leading slash as a browser
|
||||
// origin-root URL — pre-checking would hand back `/assets/%E5%9B%BE1.png`
|
||||
// undecoded and re-open PRINFRA-349 for root-relative srcs.
|
||||
const videoPath = resolveProjectRelativeSrc(v.src, projectDir, compiledDir);
|
||||
if (!existsSync(videoPath)) return null;
|
||||
try {
|
||||
// Retries are separately opt-in from the failure gate. With the
|
||||
@@ -338,21 +366,19 @@ export async function runExtractVideosStage(
|
||||
if (job.config.hdrMode !== "force-sdr" && composition.images.length > 0) {
|
||||
const probed = await Promise.all(
|
||||
composition.images.map(async (img) => {
|
||||
let imgPath = img.src;
|
||||
if (!imgPath.startsWith("/")) {
|
||||
const fromCompiled = existsSync(join(compiledDir, imgPath))
|
||||
? join(compiledDir, imgPath)
|
||||
: join(projectDir, imgPath);
|
||||
imgPath = fromCompiled;
|
||||
}
|
||||
// Same shared resolver as the video probe above — a percent-encoded
|
||||
// non-ASCII `<img src>` must decode to the on-disk path, or the HDR image
|
||||
// never enters nativeHdrImageIds and the composition silently renders
|
||||
// through the SDR fallback with wrong color (PRINFRA-349 symptom c).
|
||||
const imgPath = resolveProjectRelativeSrc(img.src, projectDir, compiledDir);
|
||||
if (!existsSync(imgPath)) return null;
|
||||
const meta = await extractMediaMetadata(imgPath);
|
||||
if (isHdrColorSpace(meta.colorSpace)) {
|
||||
const colorSpace = await probeColorSpaceSafely(imgPath, log);
|
||||
if (isHdrColorSpace(colorSpace)) {
|
||||
nativeHdrImageIds.add(img.id);
|
||||
imageTransfers.set(img.id, detectTransfer(meta.colorSpace));
|
||||
imageTransfers.set(img.id, detectTransfer(colorSpace));
|
||||
hdrImageSrcPaths.set(img.id, imgPath);
|
||||
}
|
||||
return meta.colorSpace;
|
||||
return colorSpace;
|
||||
}),
|
||||
);
|
||||
imageColorSpaces.push(...probed);
|
||||
|
||||
Reference in New Issue
Block a user