Skip to content

Commit 58869f0

Browse files
jrusso1020jrusso1020
authored
fix(producer): accept partial color metadata in plan v2 (#2814)
* fix(producer): accept partial color metadata in plan v2 * test(engine): make partial color probe hermetic * fix(producer): validate plan v2 sentinels in fallback mode
1 parent 98a4cd7 commit 58869f0

8 files changed

Lines changed: 409 additions & 14 deletions

File tree

‎packages/engine/src/utils/ffprobe.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,41 @@ describe("ffprobe missing-binary fallback", () => {
316316
expect(meta.hasAlpha).toBe(true);
317317
});
318318

319+
it("normalizes omitted video color components to empty strings", async () => {
320+
const { spawn } = createSpawnSpy([
321+
{
322+
kind: "exit",
323+
code: 0,
324+
stdout: JSON.stringify({
325+
streams: [
326+
{
327+
codec_type: "video",
328+
codec_name: "h264",
329+
width: 64,
330+
height: 64,
331+
r_frame_rate: "30/1",
332+
avg_frame_rate: "30/1",
333+
pix_fmt: "yuv420p",
334+
color_space: "bt709",
335+
},
336+
],
337+
format: { duration: "1" },
338+
}),
339+
},
340+
]);
341+
vi.resetModules();
342+
vi.doMock("child_process", () => ({ spawn }));
343+
344+
const { extractMediaMetadata: extractMediaMetadataMocked } = await import("./ffprobe.js");
345+
const metadata = await extractMediaMetadataMocked("/tmp/partial-color.mp4");
346+
347+
expect(metadata.colorSpace).toEqual({
348+
colorPrimaries: "",
349+
colorTransfer: "",
350+
colorSpace: "bt709",
351+
});
352+
});
353+
319354
// Regression: newer libavformat builds (and the output of `hyperframes
320355
// remove-background` itself) write the VP9-alpha sidecar tag as
321356
// `ALPHA_MODE` (uppercase). The lowercase-only check classified those

‎packages/producer/src/services/distributed/planV2.test.ts‎

Lines changed: 218 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,24 @@ function createV1Plan(
5353
video?: boolean;
5454
omitVideoMetadata?: boolean;
5555
fpsDen?: number;
56+
videoColorSpace?: unknown;
57+
videoCodec?: string;
58+
videoId?: string;
59+
videoSrcPath?: string;
60+
framePattern?: string;
5661
},
5762
): string {
63+
const {
64+
audio = false,
65+
video = false,
66+
omitVideoMetadata = false,
67+
fpsDen = 1,
68+
videoColorSpace = null,
69+
videoCodec = "h264",
70+
videoId = "hero",
71+
videoSrcPath = "/fixture/hero.mp4",
72+
framePattern = "frame_%05d.jpg",
73+
} = options ?? {};
5874
const planDir = join(root, "v1");
5975
mkdirSync(join(planDir, "compiled"), { recursive: true });
6076
mkdirSync(join(planDir, "meta"), { recursive: true });
@@ -69,7 +85,7 @@ function createV1Plan(
6985
);
7086
writeFileSync(join(planDir, "meta", "encoder.json"), "{}");
7187
writeFileSync(join(planDir, "meta", "composition.json"), "{}");
72-
if (options?.video) {
88+
if (video) {
7389
const framesDir = join(planDir, "video-frames", "hero");
7490
mkdirSync(framesDir, { recursive: true });
7591
writeFileSync(join(framesDir, "frame_00001.jpg"), "frame zero");
@@ -80,7 +96,7 @@ function createV1Plan(
8096
JSON.stringify({
8197
videos: [
8298
{
83-
id: "hero",
99+
id: videoId,
84100
src: "hero.mp4",
85101
start: 0,
86102
end: 1,
@@ -91,9 +107,9 @@ function createV1Plan(
91107
],
92108
extracted: [
93109
{
94-
videoId: "hero",
95-
srcPath: "/fixture/hero.mp4",
96-
framePattern: "frame_%05d.jpg",
110+
videoId,
111+
srcPath: videoSrcPath,
112+
framePattern,
97113
fps: 30,
98114
totalFrames: 3,
99115
metadata: {
@@ -102,35 +118,35 @@ function createV1Plan(
102118
width: 16,
103119
height: 16,
104120
fps: 30,
105-
videoCodec: "h264",
121+
videoCodec,
106122
hasAudio: false,
107123
isVFR: false,
108124
hasAlpha: false,
109-
colorSpace: null,
125+
colorSpace: videoColorSpace,
110126
},
111127
},
112128
],
113129
}),
114130
);
115-
if (options.omitVideoMetadata === true) {
131+
if (omitVideoMetadata) {
116132
rmSync(join(planDir, "meta", "videos.json"));
117133
}
118134
}
119-
if (options?.audio) writeFileSync(join(planDir, "audio.aac"), "assemble-only-audio");
135+
if (audio) writeFileSync(join(planDir, "audio.aac"), "assemble-only-audio");
120136
writeFileSync(
121137
join(planDir, "plan.json"),
122138
JSON.stringify({
123139
protocol: CURRENT_PLAN_PROTOCOL,
124140
planHash: "1".repeat(64),
125141
chunkCount: 2,
126142
totalFrames: 2,
127-
hasAudio: options?.audio === true,
143+
hasAudio: audio,
128144
ffmpegVersion: "ffmpeg fixture",
129145
producerVersion: "0.0.0-test",
130146
fontSnapshotSha: "font-snapshot-fixture",
131147
dimensions: {
132148
fpsNum: 30,
133-
fpsDen: options?.fpsDen ?? 1,
149+
fpsDen,
134150
width: 16,
135151
height: 16,
136152
format: "mp4",
@@ -202,6 +218,87 @@ describe("Plan v2 manifest", () => {
202218
);
203219
});
204220

221+
it("omits the extraction-cache completion sentinel from exact frame dependencies", () => {
222+
const root = tempPath("hf-plan-v2-extraction-sentinel-");
223+
const v1 = createV1Plan(root, { video: true });
224+
writeFileSync(join(v1, "video-frames", "hero", ".hf-complete"), "");
225+
refreshV1PlanHash(v1);
226+
227+
const result = createPlanV2FromV1(v1, join(root, "v2"));
228+
const manifest = readPlanV2Manifest(result.planDir);
229+
230+
expect(result.limitations.videoDependencyMode).toBe("exact-rendered-frames");
231+
expect(manifest.artifacts.some((artifact) => artifact.path.endsWith("/.hf-complete"))).toBe(
232+
false,
233+
);
234+
expect(manifest.artifacts.some((artifact) => artifact.path.endsWith("/frame_00001.jpg"))).toBe(
235+
true,
236+
);
237+
});
238+
239+
it("omits the extraction-cache completion sentinel from the full-source fallback", () => {
240+
const root = tempPath("hf-plan-v2-full-source-extraction-sentinel-");
241+
const v1 = createV1Plan(root, { video: true, omitVideoMetadata: true });
242+
writeFileSync(join(v1, "video-frames", "hero", ".hf-complete"), "");
243+
refreshV1PlanHash(v1);
244+
245+
const result = createPlanV2FromV1(v1, join(root, "v2"));
246+
const manifest = readPlanV2Manifest(result.planDir);
247+
248+
expect(result.limitations.videoDependencyMode).toBe("full-source-pack");
249+
expect(manifest.artifacts.some((artifact) => artifact.path.endsWith("/.hf-complete"))).toBe(
250+
false,
251+
);
252+
expect(manifest.artifacts.some((artifact) => artifact.path.endsWith("/frame_00003.jpg"))).toBe(
253+
true,
254+
);
255+
});
256+
257+
it("keeps non-canonical completion-marker paths over-included in full-source mode", () => {
258+
const root = tempPath("hf-plan-v2-nested-extraction-sentinel-");
259+
const v1 = createV1Plan(root, { video: true, omitVideoMetadata: true });
260+
const nestedDir = join(v1, "video-frames", "hero", "nested");
261+
mkdirSync(nestedDir);
262+
writeFileSync(join(nestedDir, ".hf-complete"), "unknown future artifact");
263+
refreshV1PlanHash(v1);
264+
265+
const result = createPlanV2FromV1(v1, join(root, "v2"));
266+
const manifest = readPlanV2Manifest(result.planDir);
267+
const nestedMarker = manifest.artifacts.find(
268+
(artifact) => artifact.path === "video-frames/hero/nested/.hf-complete",
269+
);
270+
271+
expect(result.limitations.videoDependencyMode).toBe("full-source-pack");
272+
expect(nestedMarker).toEqual(expect.objectContaining({ chunks: "all", assembler: false }));
273+
});
274+
275+
for (const omitVideoMetadata of [false, true]) {
276+
const dependencyMode = omitVideoMetadata ? "full-source" : "exact-dependency";
277+
for (const malformedSentinel of ["non-empty file", "directory", "symlink"]) {
278+
it(`rejects a ${malformedSentinel} extraction sentinel in ${dependencyMode} mode`, () => {
279+
if (malformedSentinel === "symlink" && process.platform === "win32") return;
280+
const root = tempPath(
281+
`hf-plan-v2-malformed-extraction-sentinel-${dependencyMode}-${malformedSentinel}-`,
282+
);
283+
const v1 = createV1Plan(root, { video: true, omitVideoMetadata });
284+
const sentinelPath = join(v1, "video-frames", "hero", ".hf-complete");
285+
if (malformedSentinel === "symlink") {
286+
symlinkSync(join(v1, "compiled", "asset.txt"), sentinelPath);
287+
} else if (malformedSentinel === "directory") {
288+
mkdirSync(sentinelPath);
289+
writeFileSync(join(sentinelPath, "unexpected"), "not cache metadata");
290+
} else {
291+
writeFileSync(sentinelPath, "not cache metadata");
292+
}
293+
refreshV1PlanHash(v1);
294+
295+
expect(() => createPlanV2FromV1(v1, join(root, "v2"))).toThrow(
296+
".hf-complete must be a zero-byte regular file",
297+
);
298+
});
299+
}
300+
}
301+
205302
it("selects audio only for the assembler", () => {
206303
const root = tempPath("hf-plan-v2-targets-");
207304
const result = createPlanV2FromV1(createV1Plan(root, { audio: true }), join(root, "v2"));
@@ -231,6 +328,116 @@ describe("Plan v2 manifest", () => {
231328
);
232329
});
233330

331+
const partialColorSpaceCases = [
332+
{
333+
name: "matrix-only",
334+
colorSpace: { colorTransfer: "", colorPrimaries: "", colorSpace: "bt709" },
335+
},
336+
{
337+
name: "transfer-only",
338+
colorSpace: { colorTransfer: "bt709", colorPrimaries: "", colorSpace: "" },
339+
},
340+
{
341+
name: "primaries-only",
342+
colorSpace: { colorTransfer: "", colorPrimaries: "bt709", colorSpace: "" },
343+
},
344+
];
345+
346+
for (const testCase of partialColorSpaceCases) {
347+
it(`preserves ${testCase.name} video color metadata`, () => {
348+
const root = tempPath(`hf-plan-v2-${testCase.name}-color-`);
349+
const result = createPlanV2FromV1(
350+
createV1Plan(root, { video: true, videoColorSpace: testCase.colorSpace }),
351+
join(root, "v2"),
352+
);
353+
const materializedDir = join(root, "materialized");
354+
materializePlanV2Target(result.planDir, { role: "chunk", chunkIndex: 0 }, materializedDir);
355+
356+
expect(
357+
JSON.parse(readFileSync(join(materializedDir, "meta", "videos.json"), "utf-8")),
358+
).toEqual(
359+
expect.objectContaining({
360+
extracted: [
361+
expect.objectContaining({
362+
metadata: expect.objectContaining({ colorSpace: testCase.colorSpace }),
363+
}),
364+
],
365+
}),
366+
);
367+
});
368+
}
369+
370+
it("preserves null video color metadata", () => {
371+
const root = tempPath("hf-plan-v2-null-color-");
372+
expect(() =>
373+
createPlanV2FromV1(
374+
createV1Plan(root, { video: true, videoColorSpace: null }),
375+
join(root, "v2"),
376+
),
377+
).not.toThrow();
378+
});
379+
380+
const colorComponents = ["colorTransfer", "colorPrimaries", "colorSpace"];
381+
const invalidColorValues: Array<{ name: string; value: unknown }> = [
382+
{ name: "missing", value: undefined },
383+
{ name: "null", value: null },
384+
{ name: "number", value: 709 },
385+
{ name: "object", value: { name: "bt709" } },
386+
];
387+
388+
for (const component of colorComponents) {
389+
for (const invalid of invalidColorValues) {
390+
it(`rejects a ${invalid.name} ${component} color component`, () => {
391+
const root = tempPath(`hf-plan-v2-invalid-${component}-${invalid.name}-`);
392+
const colorSpace: Record<string, unknown> = {
393+
colorTransfer: "bt709",
394+
colorPrimaries: "bt709",
395+
colorSpace: "bt709",
396+
};
397+
colorSpace[component] = invalid.value;
398+
const v1 = createV1Plan(root, { video: true, videoColorSpace: colorSpace });
399+
400+
expect(() => createPlanV2FromV1(v1, join(root, "v2"))).toThrow(
401+
`metadata.colorSpace.${component} must be a string`,
402+
);
403+
});
404+
}
405+
}
406+
407+
const unrelatedEmptyStringCases = [
408+
{
409+
name: "video codec",
410+
options: { videoCodec: "" },
411+
field: "metadata.videoCodec",
412+
},
413+
{
414+
name: "video identifier",
415+
options: { videoId: "" },
416+
field: "videos[0].id",
417+
},
418+
{
419+
name: "video source path",
420+
options: { videoSrcPath: "" },
421+
field: "srcPath",
422+
},
423+
{
424+
name: "frame pattern",
425+
options: { framePattern: "" },
426+
field: "framePattern",
427+
},
428+
];
429+
430+
for (const testCase of unrelatedEmptyStringCases) {
431+
it(`continues to reject an empty ${testCase.name}`, () => {
432+
const root = tempPath(`hf-plan-v2-empty-${testCase.name.replaceAll(" ", "-")}-`);
433+
const v1 = createV1Plan(root, { video: true, ...testCase.options });
434+
435+
expect(() => createPlanV2FromV1(v1, join(root, "v2"))).toThrow(
436+
`${testCase.field} must be a non-empty string`,
437+
);
438+
});
439+
}
440+
234441
it("falls back to the full source frame pack when video metadata is absent", () => {
235442
const root = tempPath("hf-plan-v2-video-fallback-");
236443
const result = createPlanV2FromV1(

0 commit comments

Comments
 (0)