fix: unbreak the regression suite on Windows (PSNR filtergraph escaping + console window popups) - #3839
Open
Bforbtay wants to merge 2 commits into
Open
Conversation
Node's `windowsHide` defaults to false, so every child process that is a console application pops a window on the user's desktop. During a render or a regression run that is a stream of black windows stealing focus. The repo already sets `windowsHide: true` across cli, studio-server and most of engine; these were the remaining gaps, all on paths that run during a normal render: - core/mediaGradeAnalyzer: ffprobe + ffmpeg, once per analyzed media file - engine/browserManager: `nvidia-smi` VRAM probe. This one goes through a shell (`execSync`), so it flashes a cmd.exe window even on machines with no NVIDIA GPU, where the command exists only to fail - producer: the regression harness ffmpeg runner, PSNR/audio comparison helpers, fixture synthesis, and the ffprobe call in audioPadTrim No behavior change on POSIX, where the flag is ignored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ph parse
An ffmpeg filtergraph argument is unescaped twice: once when the graph is
split into filters and their options, then again when the option value is
read. `psnrAtFrames` escaped `\` and `:` a single time, which only survives
the first pass.
On POSIX that is invisible, because mkdtemp under tmpdir() produces neither
character. On Windows the stats path is always `C:\Users\...`, so it reaches
the option parser with a bare `:`, which starts a new option and fails the
whole graph:
[AVFilterGraph] No option name near '\Users\...\psnr.log'
Error parsing filterchain '[rv][gv]psnr=...:stats_file=C\:\Users\...'
Every suite that got as far as quality validation died there, so a Windows
regression run reported 0 passes with only 2 genuine compilation failures.
Escaping for both passes fixes it and stays a no-op on POSIX.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Two independent Windows fixes found while setting the repo up on Windows 11:
stats_fileescaping (packages/producer/src/regression-harness.ts) — the regression suite's visual comparison could not run at all on Windows.windowsHide: trueon the 11 remainingchild_processcall sites that lacked it, acrosscore,engineandproducer.Why
PSNR escaping. An ffmpeg filtergraph argument is unescaped twice: once when the graph is split into filters and their options, then again when the option value itself is read.
psnrAtFramesescaped\and:a single time, which only survives the first pass.On POSIX this is invisible, because
mkdtempundertmpdir()produces neither character. On Windows the stats path is alwaysC:\Users\..., so it reaches the option parser with a bare:, which starts a new option and fails the whole graph:Every suite that got as far as quality validation died there. A full Windows run reported 73 total / 0 passed / 73 failed, but only 2 of those failed at compilation and 1 at visual comparison — the other ~70 never got to compare anything.
windowsHide. Node'swindowsHidedefaults tofalse, so every child process that is a console application pops a window on the user's desktop. Over a render — let alone a regression run — that is a steady stream of black windows stealing focus. The repo already sets the flag acrosscli,studio-serverand most ofengine; these were the remaining gaps, all on paths that run during a normal render.Worth calling out one of them:
probeNvidiaVramMbinengine/browserManagerusesexecSync, which goes through a shell, so it flashes acmd.exewindow even on machines with no NVIDIA GPU — where the command exists only to fail.How
PSNR escaping — escape for both passes. Verified against ffmpeg 9.0.1 on Windows with an identical fixture:
C\:\\Users\\...(single escape, previous)C\\:\\\\Users\\\\...(double escape, this PR)C\:/Users/...(forward slashes, single-escaped colon)C\\:/Users/...(forward slashes, double-escaped colon)cwdDouble-escaping was chosen over the
cwdapproach because it is a one-line change that keepsrunFfmpeg's signature and does not require the other path arguments to be absolute. It is a no-op on POSIX.windowsHide— added to the call sites that lacked it:core/mediaGradeAnalyzer— ffprobe + ffmpeg, once per analyzed media fileengine/browserManager— thenvidia-smiVRAM probe described aboveproducer— the regression harness ffmpeg runner, the PSNR/audio comparison helpers, fixture synthesis, and the ffprobe call inaudioPadTrimThe two concerns are separate commits, so either can be dropped independently.
Test plan
Environment: Windows 11 Pro 26200, Node v24.19.0, bun 1.4.2, ffmpeg 9.0.1.
bun run --filter '*' typecheck— all 14 packages exit 0bunx oxlint/bunx oxfmt --checkon the changed files — 0 warnings, 0 errorsNo option name near '\Users\...'bunx tsx src/regression-harness.ts sub-comp-t0completes all 100 checkpoints and passes —Total: 1 | Passed: 1 | Failed: 0, visual PASSED (3 failed frames, threshold 5), audio PASSED (correlation 1.000, lag 0)No POSIX behavior change in either commit, so this needs a look from someone on macOS/Linux only insofar as CI covers it.
Two notes for maintainers, both out of scope here and neither touched by this PR:
gen-os-static.s3.us-east-2.amazonaws.com. That host is sinkholed to0.0.0.0by Cloudflare's malware-filtering resolver (1.1.1.2), which silently stripsdata-end/data-durationand shows up as a golden-snapshot mismatch rather than a network error. Took a while to identify; may be worth a clearer diagnostic.mediaTypeTestFixturesand a couple of extractor tests still pass-vsync, which ffmpeg 9 removed in favour of-fps_mode. Test-only — the production paths already use-fps_mode.🤖 Generated with Claude Code