feat(scripts): flag a capture region that briefly returns to an older picture - #4167
Open
miguel-heygen wants to merge 7 commits into
Open
miguel-heygen wants to merge 7 commits into
miguel-heygen wants to merge 7 commits into
Conversation
… picture check-capture-reversion.mjs compares cropped, downscaled frames and reports a frame that differs between two matching frames within a window. Continuous motion is not flagged. Adds a unit test and a short section in CONTRIBUTING.
Also replaces a stale comment on probeSize that contradicted the code.
A bad option, a missing binary or file, or an undecodable video used to read as clean or crash with exit 1, which the caller cannot tell from a flagged capture. Options are now validated, the frame height is passed to ffmpeg explicitly so it always matches the size the check assumes, and the tests pin the thresholds, window boundary, ordering and mean.
The entry-point guard compared argv[1] with a realpath, so a symlinked path (macOS /tmp) skipped main and exited 0 with no output. It now compares realpaths, checks every video before exiting with the most severe code, and the tests drive the real CLI for exit 0, 1 and 2.
… spinning A crop only two pixels tall made the assumed frame size zero, so the comparison loop never ended. A non-positive frame size now throws, and each video has a wall-clock limit (default 120s, --timeout) covering the ffmpeg decode and the comparison; hitting it exits 2.
The limit is now one budget per video shared by ffprobe, ffmpeg and the comparison, checked on every frame pair so a large --window cannot run past it. A timed-out tool is killed with its whole process group, so a shell-wrapper ffmpeg leaves no orphan. --timeout is capped below the Node timer limit and inherited option names are rejected.
The tool runs in its own process group so a timeout can kill a wrapper script and its children. That left the child running when the checker itself got SIGINT or SIGTERM. Live groups are now killed on either signal, and the checker exits 2.
Fallow audit reportFound 2 findings. Dependencies (1)
Health (1)
Generated by fallow. |
4 tasks
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
1 participant
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
scripts/check-capture-reversion.mjs: checks a screen recording for a region that briefly returns to an older picture (a clip that reappears for one frame, a panel that flashes its previous state). It compares cropped, downscaled frames and reports a frame that differs between two matching frames within a window. Continuous motion is not flagged.Why
A recording attached to a PR can look fine at a glance and still contain a one-frame reversion. Eyeballing does not catch it reliably; a mechanical frame comparison does, and every author can run the same check on their own captures.
How
--crop=W:H:X:Y), scaled to 96px wide, grayscale. A reversion is an A, B, A pattern: frame k nearly equals an earlier frame i while a frame between them clearly differs from both.--timeout, default 120s) shared by ffprobe, ffmpeg and the comparison. A timed-out tool is killed with its whole process group, also when the checker itself gets SIGINT or SIGTERM.Test plan
scripts/check-capture-reversion.test.mjs(node --test), registered intest:scripts. They cover the thresholds, window boundary, ordering, option validation, the real CLI exit codes (0, 1, 2) including a symlinked path, the time limit against a fake hung ffmpeg and ffprobe, and no orphaned child after a timeout or SIGTERM.--cropon the timeline strip.An independent adversarial review ran over four rounds; its findings (exit-code conflation, NaN options reading clean, a hang on a thin crop, a symlink entry-guard silent pass, deadline granularity, orphaned children) are fixed and covered by tests, and I mutation-checked the guards.