mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-02 20:18:35 +00:00
fix(producer): align frame coverage with extraction rounding (#2770)
## What - model expected video-frame counts using the same rounding contract as the extraction branch: - CFR `-vf fps=<fps>`: nearest output-frame boundary - VFR `-fps_mode cfr -r <fps>`: ceil - retain fail-closed ceil behavior when extraction metadata is missing - keep positive sub-frame clips at a minimum of one expected frame - preserve the existing 95% truncation gate and source-duration credit ## Why The coverage gate universally used `ceil(duration * fps)`, but FFmpeg's CFR fps filter rounds to the nearest boundary. This made successfully extracted short CFR clips such as 0.616666s at 30 fps look truncated (18 captured vs 19 expected). In the dashboard window, 124 of 335 video-coverage failures were exactly one frame short. Historical logs do not include `isVFR`, so that is the maximum addressable cohort rather than a guaranteed reduction. Zero-frame and materially truncated extraction failures remain fail-closed. ## Safety - VFR still uses ceil and the existing strict coverage threshold. - Missing extraction metadata still uses ceil. - No retry, fallback, Temporal workflow, or render-plan behavior changes. - Intended rollout is through the producer sidecar canary with explicit internal in-process jobs before dev and production promotion. ## Test - `bunx vitest run packages/producer/src/services/render/videoFrameCoverage.test.ts` (28 passed) - `bun run --filter @hyperframes/producer typecheck` - `bunx oxlint ...videoFrameCoverage.ts ...videoFrameCoverage.test.ts` - `bunx oxfmt --check ...videoFrameCoverage.ts ...videoFrameCoverage.test.ts` - `bunx fallow audit --base origin/main --fail-on-issues` - repository pre-commit gates
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
// fallow-ignore-file code-duplication
|
||||
|
||||
import type { ExtractedFrames, VideoElement, VideoMetadata } from "@hyperframes/engine";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
@@ -35,9 +37,9 @@ function makeVideo(overrides: Partial<VideoElement> & { id: string }): VideoElem
|
||||
function makeExtracted(
|
||||
videoId: string,
|
||||
delivered: number,
|
||||
options: { fps?: number; durationSeconds?: number } = {},
|
||||
options: { fps?: number; durationSeconds?: number; isVFR?: boolean } = {},
|
||||
): ExtractedFrames {
|
||||
const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY } = options;
|
||||
const { fps = 30, durationSeconds = Number.POSITIVE_INFINITY, isVFR = false } = options;
|
||||
const framePaths = new Map<number, string>();
|
||||
for (let i = 0; i < delivered; i += 1) framePaths.set(i, `/tmp/${videoId}/${i}.jpg`);
|
||||
const metadata: VideoMetadata = {
|
||||
@@ -48,7 +50,7 @@ function makeExtracted(
|
||||
fps,
|
||||
videoCodec: "h264",
|
||||
hasAudio: false,
|
||||
isVFR: false,
|
||||
isVFR,
|
||||
hasAlpha: false,
|
||||
colorSpace: null,
|
||||
};
|
||||
@@ -72,9 +74,23 @@ describe("expectedFramesForClip", () => {
|
||||
expect(expectedFramesForClip(2, 1, 30)).toBe(0); // negative window collapses to 0
|
||||
});
|
||||
|
||||
it("ceils fractional-fps windows so a 29.97fps 1s clip demands 30 frames", () => {
|
||||
it("defaults to fail-closed ceil rounding", () => {
|
||||
expect(expectedFramesForClip(0, 1, 29.97)).toBe(30);
|
||||
expect(expectedFramesForClip(0, 5, 30)).toBe(150);
|
||||
expect(expectedFramesForClip(0, 0.616666, 30)).toBe(19);
|
||||
expect(expectedFramesForClip(0, 0.316666, 30)).toBe(10);
|
||||
});
|
||||
|
||||
it("matches the CFR fps filter's nearest-boundary rounding when requested", () => {
|
||||
expect(expectedFramesForClip(0, 0.616666, 30, "nearest")).toBe(18);
|
||||
expect(expectedFramesForClip(0, 0.316666, 30, "nearest")).toBe(9);
|
||||
expect(expectedFramesForClip(0, 0.633333, 30, "nearest")).toBe(19);
|
||||
});
|
||||
|
||||
it("requires one frame for every positive sub-frame clip", () => {
|
||||
expect(expectedFramesForClip(0, 0.001, 30)).toBe(1);
|
||||
expect(expectedFramesForClip(0, 0.001, 30, "nearest")).toBe(1);
|
||||
expect(expectedFramesForClip(1, 1, 30)).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -123,6 +139,43 @@ describe("computeVideoFrameCoverage", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("reports full coverage for a short CFR clip matching FFmpeg boundary rounding", () => {
|
||||
const videos = [makeVideo({ id: "short", start: 0, end: 0.616666 })];
|
||||
const reports = computeVideoFrameCoverage(videos, [makeExtracted("short", 18)], 30);
|
||||
expect(reports[0]).toMatchObject({
|
||||
expectedFrames: 18,
|
||||
capturedFrames: 18,
|
||||
ratio: 1,
|
||||
});
|
||||
expect(() => assertVideoFrameCoverage(reports, 0.95)).not.toThrow();
|
||||
});
|
||||
|
||||
it("keeps ceil coverage for the VFR extraction branch", () => {
|
||||
const videos = [makeVideo({ id: "short-vfr", start: 0, end: 0.616666 })];
|
||||
const reports = computeVideoFrameCoverage(
|
||||
videos,
|
||||
[makeExtracted("short-vfr", 18, { isVFR: true })],
|
||||
30,
|
||||
);
|
||||
expect(reports[0]).toMatchObject({
|
||||
expectedFrames: 19,
|
||||
capturedFrames: 18,
|
||||
ratio: 18 / 19,
|
||||
});
|
||||
expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError);
|
||||
});
|
||||
|
||||
it("still fails closed when a positive sub-frame clip captured zero frames", () => {
|
||||
const videos = [makeVideo({ id: "blank-sub-frame", start: 0, end: 0.001 })];
|
||||
const reports = computeVideoFrameCoverage(videos, [makeExtracted("blank-sub-frame", 0)], 30);
|
||||
expect(reports[0]).toMatchObject({
|
||||
expectedFrames: 1,
|
||||
capturedFrames: 0,
|
||||
ratio: 0,
|
||||
});
|
||||
expect(() => assertVideoFrameCoverage(reports, 0.95)).toThrow(VideoFrameCoverageError);
|
||||
});
|
||||
|
||||
it("reports 0 capturedFrames when a video was never extracted (injection failure)", () => {
|
||||
// Field signal ts=1784139267: later-injected video clips silently drop
|
||||
// out of the extractor under injection-count saturation.
|
||||
|
||||
@@ -71,6 +71,8 @@ export interface VideoFrameCoverageErrorDetails {
|
||||
}
|
||||
|
||||
export class VideoFrameCoverageError extends Error {
|
||||
// Read structurally by isVideoFrameCoverageError and cross-module callers.
|
||||
// fallow-ignore-next-line unused-class-member
|
||||
readonly hyperframesVideoFrameCoverageError = true as const;
|
||||
readonly threshold: number;
|
||||
readonly worst: VideoFrameCoverageReport;
|
||||
@@ -117,16 +119,47 @@ export function resolveVideoCoverageThreshold(
|
||||
}
|
||||
|
||||
/**
|
||||
* Ceil the clip's authored `[start,end)` window at `fps` — the number of
|
||||
* captured render frames whose center-time falls inside the window. Kept
|
||||
* separate so a caller (or a test) can override it if a composition uses
|
||||
* a non-integer fps whose sampling makes the naive count off by one.
|
||||
* Count frames in a clip's authored `[start,end)` window. CFR extraction uses
|
||||
* FFmpeg's `-vf fps=<fps>` and rounds to the nearest output-frame boundary;
|
||||
* VFR extraction uses `-fps_mode cfr -r <fps>` and rounds up. Missing entries
|
||||
* retain the fail-closed `ceil` default. Positive sub-frame clips emit one.
|
||||
*/
|
||||
export function expectedFramesForClip(start: number, end: number, fps: number): number {
|
||||
export function expectedFramesForClip(
|
||||
start: number,
|
||||
end: number,
|
||||
fps: number,
|
||||
rounding: "ceil" | "nearest" = "ceil",
|
||||
): number {
|
||||
if (!Number.isFinite(start) || !Number.isFinite(end) || !Number.isFinite(fps)) return 0;
|
||||
if (fps <= 0) return 0;
|
||||
const duration = Math.max(0, end - start);
|
||||
return Math.ceil(duration * fps);
|
||||
if (duration === 0) return 0;
|
||||
const frameCount =
|
||||
rounding === "nearest" ? Math.round(duration * fps) : Math.ceil(duration * fps);
|
||||
return Math.max(1, frameCount);
|
||||
}
|
||||
|
||||
function expectedFramesForVideo(
|
||||
video: VideoElement,
|
||||
entry: ExtractedFrames | undefined,
|
||||
fps: number,
|
||||
): number {
|
||||
const rounding = entry && !entry.metadata.isVFR ? "nearest" : "ceil";
|
||||
const slotFrames = expectedFramesForClip(video.start, video.end, fps, rounding);
|
||||
if (!entry) return slotFrames;
|
||||
|
||||
// A short source in a longer slot has a legitimate delivery ceiling of
|
||||
// the source portion, not the full slot: a non-looping clip holds its
|
||||
// final decoded frame across the tail (#2516/#2606), and a looping clip
|
||||
// reuses its full source frame set per repeat (#2665). In both cases the
|
||||
// full source *has* been delivered — the same 90 unique source frames
|
||||
// cover the 300-frame slot — so coverage must measure source-source, not
|
||||
// slot-source.
|
||||
const sourceDuration = entry.metadata.durationSeconds - video.mediaStart;
|
||||
if (!Number.isFinite(sourceDuration) || sourceDuration <= 0) return slotFrames;
|
||||
|
||||
const sourceFrames = expectedFramesForClip(0, sourceDuration, fps, rounding);
|
||||
return Math.min(slotFrames, sourceFrames);
|
||||
}
|
||||
|
||||
export function computeVideoFrameCoverage(
|
||||
@@ -140,20 +173,7 @@ export function computeVideoFrameCoverage(
|
||||
const reports: VideoFrameCoverageReport[] = [];
|
||||
for (const video of videos) {
|
||||
const entry = byId.get(video.id);
|
||||
const slotFrames = expectedFramesForClip(video.start, video.end, fps);
|
||||
// A short source in a longer slot has a legitimate delivery ceiling of
|
||||
// the source portion, not the full slot: a non-looping clip holds its
|
||||
// final decoded frame across the tail (#2516/#2606), and a looping clip
|
||||
// reuses its full source frame set per repeat (#2665). In both cases the
|
||||
// full source *has* been delivered — the same 90 unique source frames
|
||||
// cover the 300-frame slot — so coverage must measure source-source, not
|
||||
// slot-source. A missing extraction (no `entry`) still requires the full
|
||||
// slot; there's no delivered set to credit.
|
||||
const sourceDuration = entry ? entry.metadata.durationSeconds - video.mediaStart : NaN;
|
||||
const hasUsableSourceDuration = Number.isFinite(sourceDuration) && sourceDuration > 0;
|
||||
const sourceFrames =
|
||||
entry && hasUsableSourceDuration ? expectedFramesForClip(0, sourceDuration, fps) : slotFrames;
|
||||
const expectedFrames = entry ? Math.min(slotFrames, sourceFrames) : slotFrames;
|
||||
const expectedFrames = expectedFramesForVideo(video, entry, fps);
|
||||
// framePaths is a Map — `size` is the number of distinct captured frames
|
||||
// delivered to the runtime injector, which is the load-bearing count
|
||||
// (some extractors report a total that includes cache-hit-skipped frames
|
||||
|
||||
Reference in New Issue
Block a user