fix(engine): catch low-contrast drawElement self-verify misses - #3941
Open
santhiprakash wants to merge 1 commit into
Open
santhiprakash wants to merge 1 commit into
santhiprakash wants to merge 1 commit into
Conversation
Whole-frame magnitude-weighted PSNR cannot see a fully-missing low-contrast element: the issue-3345 repro (a ~4/255 band over ~15% of the frame) scores 44dB, 12dB clear of the 32dB floor, and ships a wrong MP4 with exit code 0. Add regionShift(): one ffmpeg pass (grainextract + area downscale) reporting the worst ~96px region's mean signed channel shift in 1/255 units. Encoder noise cancels inside a region while an absent element leaves its whole footprint shifted by its own delta, so a missing region can no longer be diluted by a correct background. Both the streaming drain guard and the parallel disk-path verify now require PSNR >= HF_DE_VERIFY_MIN_DB AND region shift <= HF_DE_VERIFY_MAX_SHIFT (default 3/255). Either breach throws DrawElementVerificationError (new kind "shift"), which the orchestrator already converts into the never-wrong screenshot fallback — matching the videoFrameCoverage fail-closed precedent. Closes heygen-com#3345
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a complementary region-presence gate to the drawElement self-verify path. The existing whole-frame PSNR check stays as the magnitude baseline; a new
regionShift()metric reports the worst ~96px region's mean signed channel shift (1/255 units) via one extra ffmpeg pass (grainextract+ area downscale). A sample now fails ifdb < HF_DE_VERIFY_MIN_DBorshift > HF_DE_VERIFY_MAX_SHIFT(default 3/255). Either breach throwsDrawElementVerificationErrorwith a newkind: "shift", which the orchestrator already converts into the screenshot fallback — fail-closed, matching thevideoFrameCoverage.tsprecedent called out in the issue thread.Closes #3345.
Why
The magnitude-weighted PSNR gate cannot detect a fully-missing low-contrast element. Reproduced from the issue: a ~4/255 luma band covering ~15% of the frame scores 44.1 dB — 12 dB clear of the 32 dB floor — so a wrong MP4 ships with exit code 0. PSNR is the right answer to "how far off are the pixels on average"; the self-verify question is "did any content go missing", which needs a localized signal that a correct background cannot dilute.
How
packages/engine/src/utils/psnr.ts— newregionShift()(signed diff centered on mid-gray bygrainextract, downscaled to coarse cells by area average; the max |cell−128| is the worst region's mean shift) andresolveDeVerifyMaxShift()(envHF_DE_VERIFY_MAX_SHIFT, clamped to [1, 64], default 3).packages/producer/src/services/render/stages/captureStreamingStage.ts— streaming drain guard computes both metrics and throwskind: "shift"when only the region gate trips.packages/engine/src/services/parallelCoordinator.ts— same two-gate check on the parallel disk-verify path (assertDiskSampleVerified, exported for tests).packages/engine/src/services/frameCapture.ts—DrawElementVerificationDetails.kindgains"shift"withfailedShift/verifyMaxShiftstructured fields (no message parsing).renderOrchestrator.ts/observability.ts— telemetry union widened so a shift trip reportsde_fallback_reason = "shift"with the frame index.Measured on the repro pair (1080p variant): missing band PSNR 44.1 dB / shift 5, identical pair shift 0, independent encoder noise ≤2, saturated damage 127 (clamped). Chroma-heavy damage is caught too (rgb24 diff, not luma-only).
Test plan
New regression coverage, all passing locally:
packages/engine/src/utils/psnr.test.ts(new): real-ffmpeg repro of drawElement self-verify PSNR gate cannot detect missing low-contrast content #3345 — the missing low-contrast element passes the old 32 dB gate (44 dB) but trips the region gate; identical/noisy pairs stay under the ceiling; saturated damage reads far above it. Plus mocked-execFileunit tests for the rawvideo scan and both env resolvers.frameCapture.test.ts:kind: "shift"details survivegetDrawElementVerificationDetailsround-trip without colliding with the dB fields.parallelCoordinator.test.ts:assertDiskSampleVerifiedthrowskind: "shift"at db=44/shift=13, still throwskind: "psnr"below the floor, and passes a clean sample.captureStreamingStage.test.ts: end-to-end drain-guard tests — a verified frame with PSNR 44 / shift 13 is rejected with the new error, and PSNR 48 / shift 1 is accepted.Out of scope: the issue's second hole — verification only samples truth frames, so unsampled frames stay unchecked — is unchanged and worth a follow-up.
AI-assisted. I wrote and verified this change.