From 0b7e4e363618ad9a212a69eaebc28fd7717d3070 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Tue, 25 Aug 2026 19:13:40 +1000 Subject: [PATCH] fix(ctmf): pin the CPU dispatch level away from the crashing AVX-512 kernel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTMF r5's AVX-512 kernel for 8-bit input crashes the process outright — a 0xC0000005 access violation, so vspipe dies having printed nothing. The encode then surfaces as the *encoder* ffmpeg failing on an empty Y4M pipe ("Header too large", exit -22) and the preview as a bare "exit code 1" whose log ends after the routine API3 warnings. Nothing names CTMF. It bites every radius except 2 (radius 2 has its own filterRadius2_* kernel) and only at 8 bits (>=10-bit uses the uint16_t helpers). opt=1, 2 and 3 are bit-identical to each other and none of them crash. CTMF r5 (2020) is the newest upstream release, so there is nothing to upgrade to. This is why the nightly Windows job went red on 2026-08-24 against a tree unchanged since 08-20: x264's capability line in the same logs reads "... AVX2" on the three passing nights and "... AVX2 AVX512" on the failing ones. The runner hardware was the variable. The plugin does NOT verify that the CPU supports the level it is handed — opt=3 on a pre-AVX2 machine crashes exactly as opt=4 does here, and ctmfCreate carries only a 0..4 range check. So the value cannot be a constant: script_generator::ctmf_opt queries the CPU and emits 3 where AVX2 exists, else 2 (SSE2 is part of the x86-64 baseline). It lives in the worker rather than the script because it is a property of the machine, not of the clip. Non-x86 builds ignore the argument. Verified on an AVX-512 CPU: the failing nightly case now passes, and removing the pin reproduces the CI error byte for byte. All 20 depth x radius combinations pass with the pin. The other six bundled plugins exposing the same opt dispatch were swept and are clean. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 77 +++++++++++++++++++ .../integration_filter_parameters_test.dart | 13 +++- worker/src/script_generator.rs | 38 +++++++++ worker/templates/pipeline_template.vpy | 16 +++- worker/templates/preview_template.vpy | 16 +++- worker/tests/filter_integration_test.rs | 68 ++++++++++++++++ 6 files changed, 225 insertions(+), 3 deletions(-) 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"); + } +}