diff --git a/CLAUDE.md b/CLAUDE.md index c79abd2..e2f26cd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -868,6 +868,72 @@ which any documentation would have shown: > is silently lost at 16-bit. Good news: bm3d is never reached, so it needs no > deps addition. Implementable, but effort 3 and blocked on the licence. +### A plugin's own CPU auto-detect is not trustworthy (CTMF, 2026-08-25) + +`ctmf.CTMF`'s AVX-512 kernel for **8-bit** input +(`ctmfHelper_avx512`) crashes the process. Not an exception — a +**0xC0000005 access violation**, so vspipe dies having printed nothing at all. + +That makes the symptom actively misleading. The encode surfaces as the *encoder* +ffmpeg failing to read the empty Y4M pipe: + +``` +[in#0] Header too large. +[in#0] Error opening input: Invalid argument +ffmpeg exited with exit code -22 +``` + +and the preview as a bare `Preview generation failed (exit code 1)` whose log +ends after the routine API3 plugin warnings. **Nothing anywhere names CTMF**, and +"Header too large" reads like a muxer or template bug. + +Scope, measured against the bundle rather than assumed: + +| | 8-bit | ≥10-bit | +|---|---|---| +| `radius=2` | OK | OK | +| every other radius | **crash** | OK | + +Radius 2 escapes because it has its own `filterRadius2_*` kernel; ≥10-bit +escapes because it uses the `uint16_t` helpers. `opt=1` (C), `2` (SSE2) and `3` +(AVX2) are **bit-identical to each other** and none of them crash. + +> **The plugin does not check that the CPU can run the level you ask for.** +> `opt=3` on a pre-AVX2 machine installs the AVX2 kernels and crashes exactly as +> `opt=4` does on an AVX-512 one — there is no guard in `ctmfCreate`, only a +> `0..4` range check. So the fix cannot be a constant. `script_generator::ctmf_opt` +> queries the CPU (`is_x86_feature_detected!`) and emits **3 where AVX2 exists, +> else 2** — SSE2 being part of the x86-64 baseline. Never emit `0`: that is the +> plugin's own auto-detect, and auto-detect is precisely what picks the broken +> kernel. Non-x86 builds compile the dispatch out and ignore the value. +> +> This lives in the worker rather than in the script because it is a property of +> the **machine**, not of the clip — unlike the depth scalings, no preceding pass +> can change the answer. CTMF r5 (2020) is the newest upstream release, so there +> is no fixed build to take instead. + +> **CI turned red with no diff, and the runner hardware was the variable.** +> The nightly Windows job started failing 2026-08-24 against a tree unchanged +> since 08-20. x264's capability line in the same logs is the tell: `... AVX2` on +> the three passing nights, `... AVX2 AVX512` on the failing ones. When a job +> fails with no commit to blame, grep the log for that line before bisecting. +> (These machines also align VapourSynth frames to 64 bytes rather than 32 — a +> 720-wide 8-bit plane gets stride 768.) + +> **Only CTMF is affected.** `cas`, `grain.Add`, `tcanny`, `dfttest`, +> `warp.AWarpSharp2` and `eedi3m` expose the same `opt` parameter and the same +> `instrset_detect()` dispatch; all six were swept at `opt=0` against `opt=3` on +> an AVX-512 CPU, at 8-bit and 16-bit, and are clean. Do not pin them +> pre-emptively — an unnecessary pin costs throughput and hides a real +> regression later. + +`test_152`/`test_153` (Rust) assert both generated scripts carry the pin and that +the value is one the CPU actually has; the Dart twin is in +`integration_filter_parameters_test.dart`. Both matter: the Rust test would pass +against a value no CPU here can run, and the heavy end-to-end +`integration_new_passes_test` CTMF case is the only level that proves vspipe +survives. + ### Third filter batch (2026-08-15): the first deps change **fluxsmooth** is the first plugin this work has *added* to the bundle rather @@ -2545,6 +2611,17 @@ version skew between platforms would change chroma per-OS. it. The app-side counterpart is in `worker_manager.dart`: exactly one completion event per job, so a worker that exits without reporting a result surfaces as a failure instead of leaving the UI on "processing" forever. +11. **"Header too large" / vspipe exits with no message**: not a template or + muxer bug — that is a **native crash inside a plugin**. vspipe dies before + writing the Y4M header, so the encoder ffmpeg reports `Header too large` / + `Error opening input` and a preview reports a bare `exit code 1` with the log + ending after the routine API3 warnings. A Python-level fault would print a + traceback instead, so *absence* of an error is the diagnostic. Bisect by + generating the script (`--config`, keep the `.vpy`) and running it under + vspipe with passes commented out; on Windows confirm with `$LASTEXITCODE` + (`0xC0000005` = access violation, `0xC000001D` = illegal instruction, which + means the binary needs a CPU feature this machine lacks). See "A plugin's own + CPU auto-detect is not trustworthy" for the CTMF case. ## Platform-Specific Notes diff --git a/app/test/integration_filter_parameters_test.dart b/app/test/integration_filter_parameters_test.dart index 6395f74..6b841cb 100644 --- a/app/test/integration_filter_parameters_test.dart +++ b/app/test/integration_filter_parameters_test.dart @@ -1194,7 +1194,7 @@ void main() { // --- Batch four ------------------------------------------------------- - test('noise reduction: CTMF, with its 9-bit guard and pinned memsize', + test('noise reduction: CTMF, with its 9-bit guard, pinned memsize and pinned dispatch', () async { loadSchema('noise_reduction'); final job = buildJob( @@ -1217,6 +1217,17 @@ void main() { // At the plugin default, 16-bit radius 3 measures 0.79 fps against 42 // here, for bit-identical output. expect(actual['memsize'], '16777216'); + // CTMF r5's AVX-512 kernel for 8-bit input crashes the process — vspipe + // dies with an access violation and prints nothing, so the failure + // arrives as ffmpeg reading an empty pipe, naming no filter. `opt=0` is + // the plugin's own auto-detect and is what selects AVX-512, so the worker + // always pins a level it has confirmed the CPU can run (2 = SSE2 or + // 3 = AVX2, which are bit-identical to each other). + expect(actual['opt'], anyOf('2', '3'), + reason: 'CTMF must be pinned to SSE2 or AVX2'); + expect(script, isNot(contains('opt=0')), + reason: 'auto-detect is what picks the AVX-512 kernel that crashes'); + expect(script, isNot(contains('opt=4'))); print(' PASS'); }, timeout: const Timeout(Duration(minutes: 2))); diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index c3d703f..d8d68e1 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -17,6 +17,43 @@ use crate::models::{ FieldOrder, }; +/// CPU dispatch level to hand `ctmf.CTMF`'s `opt` argument. +/// +/// CTMF r5's AVX-512 kernel for 8-bit input crashes the process outright — an +/// access violation with no message, so the encode surfaces as ffmpeg reading +/// an empty pipe and the preview as a bare "exit code 1". `opt=0` (the plugin's +/// default) auto-detects and picks AVX-512 wherever the CPU has it, so it is +/// never safe to emit. The CTMF block in `pipeline_template.vpy` carries the +/// full measurements. +/// +/// The plugin does **not** verify that the CPU can run the level it is handed, +/// so this has to be a real capability query rather than a constant: 3 (AVX2) +/// where the CPU has it, otherwise 2 (SSE2, which every x86-64 CPU has by +/// definition). The three non-AVX-512 levels are bit-identical, so this costs +/// throughput and nothing else. Non-x86 builds of the plugin compile the +/// dispatch out and ignore the value. +/// +/// This belongs in the worker rather than in the script because it is a +/// property of the machine, not of the clip — unlike the depth scalings, no +/// preceding pass can change the answer. +pub fn ctmf_opt() -> u8 { + if cpu_has_avx2() { + 3 + } else { + 2 + } +} + +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +fn cpu_has_avx2() -> bool { + std::is_x86_feature_detected!("avx2") +} + +#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))] +fn cpu_has_avx2() -> bool { + false +} + /// Generates VapourSynth scripts from templates. pub struct ScriptGenerator { template: String, @@ -1023,6 +1060,7 @@ impl ScriptGenerator { &nr.ctmf_effective_radius().to_string(), ); script = script.replace("{{NR_CTMF_PLANES}}", nr.ctmf_planes_literal()); + script = script.replace("{{NR_CTMF_OPT}}", &ctmf_opt().to_string()); } NoiseReductionMethod::QtgmcBuiltin => { // QTGMC built-in denoising is handled in the QTGMC pass itself diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index a3a6f8e..eb1fd4b 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -735,7 +735,7 @@ clip = haf.STPresso( # impulse noise, at a cost that barely moves with radius (measured: radius 1 to # 127 is about 20% apart). # -# Two guards, both measured against the bundle rather than assumed: +# Three guards, all measured against the bundle rather than assumed: # # - 9-BIT IS REJECTED outright ("only constant format 8, 10, 12, 14, 16 bit # integer ... supported"), and 9-bit is reachable here: pixel_format.rs rounds @@ -744,6 +744,19 @@ clip = haf.STPresso( # - memsize is pinned to 16 MiB. At the plugin's 1 MiB default, 16-bit radius 3 # runs at 0.79 fps against 42 fps with this value — a 40x cliff — for # BIT-IDENTICAL output. +# - opt IS PINNED AWAY FROM AVX-512. r5's AVX-512 kernel for 8-bit input +# (ctmfHelper_avx512) crashes the process — vspipe dies with an +# access violation and prints NOTHING, so a job surfaces as ffmpeg reading an +# empty pipe ("Header too large") and a preview as a bare "exit code 1", with +# nothing anywhere naming CTMF. It bites every radius except 2 (radius 2 has +# its own filterRadius2_* kernel) and only at 8 bits (10/12/14/16-bit use the +# uint16_t helpers, which are fine). Measured 2026-08-25; CTMF r5 is the +# newest upstream release, so there is no fixed build to take instead. +# NEVER leave this at 0 — the plugin's own auto-detect is what picks AVX-512. +# Never hardcode 3 either: the plugin does not check that the CPU supports +# the level it is handed, so opt=3 on a pre-AVX2 machine crashes the same +# way. `script_generator::ctmf_opt` queries the CPU and sends 3 or 2, which +# are bit-identical to each other and to the C path. _ctmf_src_format = clip.format if clip.format.bits_per_sample == 9: clip = core.resize.Point(clip, format=clip.format.replace( @@ -753,6 +766,7 @@ clip = core.ctmf.CTMF( radius={{NR_CTMF_RADIUS}}, planes={{NR_CTMF_PLANES}}, memsize=16777216, + opt={{NR_CTMF_OPT}}, ) if _ctmf_src_format.bits_per_sample == 9: clip = core.resize.Point(clip, format=_ctmf_src_format.id) diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 90ff208..f85d56d 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -677,7 +677,7 @@ clip = haf.STPresso( # impulse noise, at a cost that barely moves with radius (measured: radius 1 to # 127 is about 20% apart). # -# Two guards, both measured against the bundle rather than assumed: +# Three guards, all measured against the bundle rather than assumed: # # - 9-BIT IS REJECTED outright ("only constant format 8, 10, 12, 14, 16 bit # integer ... supported"), and 9-bit is reachable here: pixel_format.rs rounds @@ -686,6 +686,19 @@ clip = haf.STPresso( # - memsize is pinned to 16 MiB. At the plugin's 1 MiB default, 16-bit radius 3 # runs at 0.79 fps against 42 fps with this value — a 40x cliff — for # BIT-IDENTICAL output. +# - opt IS PINNED AWAY FROM AVX-512. r5's AVX-512 kernel for 8-bit input +# (ctmfHelper_avx512) crashes the process — vspipe dies with an +# access violation and prints NOTHING, so a job surfaces as ffmpeg reading an +# empty pipe ("Header too large") and a preview as a bare "exit code 1", with +# nothing anywhere naming CTMF. It bites every radius except 2 (radius 2 has +# its own filterRadius2_* kernel) and only at 8 bits (10/12/14/16-bit use the +# uint16_t helpers, which are fine). Measured 2026-08-25; CTMF r5 is the +# newest upstream release, so there is no fixed build to take instead. +# NEVER leave this at 0 — the plugin's own auto-detect is what picks AVX-512. +# Never hardcode 3 either: the plugin does not check that the CPU supports +# the level it is handed, so opt=3 on a pre-AVX2 machine crashes the same +# way. `script_generator::ctmf_opt` queries the CPU and sends 3 or 2, which +# are bit-identical to each other and to the C path. _ctmf_src_format = clip.format if clip.format.bits_per_sample == 9: clip = core.resize.Point(clip, format=clip.format.replace( @@ -695,6 +708,7 @@ clip = core.ctmf.CTMF( radius={{NR_CTMF_RADIUS}}, planes={{NR_CTMF_PLANES}}, memsize=16777216, + opt={{NR_CTMF_OPT}}, ) if _ctmf_src_format.bits_per_sample == 9: clip = core.resize.Point(clip, format=_ctmf_src_format.id) diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 0163b8b..c149aa5 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -5580,3 +5580,71 @@ fn test_151_levels_honour_their_switch_and_yield_to_automatic_levels() { "the manual input point must not be applied on top of the measured one" ); } + +#[test] +fn test_152_ctmf_never_asks_for_the_avx512_kernel() { + // CTMF r5's AVX-512 kernel for 8-bit input crashes the process: vspipe dies + // with an access violation and prints NOTHING, so the job surfaces as + // ffmpeg reading an empty pipe ("Header too large") and the preview as a + // bare "exit code 1" — with nothing anywhere naming the filter. It bites + // every radius except 2 and only at 8 bits. + // + // Reproduced on an AVX-512 CPU 2026-08-25, and in CI the moment GitHub's + // Windows runners gained AVX-512 (the same nightly passed the three nights + // before on runners without it, against an unchanged tree). CTMF r5 is the + // newest upstream release, so there is nothing to upgrade to. + // + // So `opt` must always be emitted, and must never be 0 (the plugin's own + // auto-detect, which is what picks AVX-512) or 4. + create_output_dir(); + let mut job = create_base_job("test_152_ctmf_opt"); + job.qtgmc_parameters.enabled = false; + job.processing_pipeline = Some(ProcessingPipeline { + deinterlace: QTGMCParameters { enabled: false, ..Default::default() }, + noise_reduction: NoiseReductionParameters { + enabled: true, + method: NoiseReductionMethod::Ctmf, + ctmf_radius: 3, + ..Default::default() + }, + ..ProcessingPipeline::default() + }); + + // Both scripts, because a preview that crashes is the half the reporter + // sees first. + let (encode, preview) = generate_both_scripts(&job); + for (name, script) in [("encode", &encode), ("preview", &preview)] { + assert!( + script.contains("core.ctmf.CTMF("), + "{name} script should call CTMF" + ); + assert!( + script.contains(&format!("opt={}", vapourbox_worker::script_generator::ctmf_opt())), + "{name} script must pin CTMF's dispatch level" + ); + assert!( + !script.contains("opt=0") && !script.contains("opt=4"), + "{name} script must never hand CTMF auto-detect or AVX-512" + ); + } +} + +#[test] +fn test_153_ctmf_opt_is_a_level_the_cpu_can_actually_run() { + // The plugin does NOT verify that the CPU supports the level it is handed — + // opt=3 on a pre-AVX2 machine installs the AVX2 kernels and crashes exactly + // as opt=4 does on this one. So this has to stay a real capability query, + // not a constant: 3 only where AVX2 was detected, otherwise 2 (SSE2, which + // every x86-64 CPU has by definition). Both are bit-identical to the C path. + let opt = vapourbox_worker::script_generator::ctmf_opt(); + assert!( + opt == 2 || opt == 3, + "opt must be SSE2 or AVX2, got {opt}" + ); + + #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] + { + let expected = if std::is_x86_feature_detected!("avx2") { 3 } else { 2 }; + assert_eq!(opt, expected, "opt must follow what the CPU actually has"); + } +}