From a3a5533c78072bd409c80b20b75fd8ee7f9963f6 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Mon, 5 Oct 2026 21:11:59 +1100 Subject: [PATCH 1/3] fix(pipeline): run Sharpen after the resize, and drop a false order warning (#109) The Sharpen pass warned that it "runs before Noise Reduction, so the denoiser will soften much of this again". It never did: Sharpen already ran after every clean-up pass. Remove the warning and pin the order the remaining advice talks about against enabledPasses. Sharpen was not last, though. It ran ahead of Chroma Fixes, Colour Correction, Stabilize and the resize, so a downscale softened the sharpened edges again and an upscale enlarged the halos. Move it to after Crop & Resize and before Film Grain, in both templates, the Rust and Dart pass orders and the pass list. Presets and saved jobs that combine Sharpen with a resize render slightly differently, and Sharpen now works at the output resolution. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN --- README.md | 4 +- app/lib/models/pass_advice.dart | 33 +++--- app/lib/models/processing_pipeline.dart | 12 +- app/lib/views/pass_list/pass_list_panel.dart | 3 +- app/test/integration_new_passes_test.dart | 30 +++++ app/test/pass_advice_test.dart | 49 +++++++-- app/test/pass_list_stages_test.dart | 11 +- docs/ENGINEERING_NOTES.md | 10 ++ worker/src/models/processing_pipeline.rs | 13 ++- worker/templates/pipeline_template.vpy | 110 ++++++++++--------- worker/templates/preview_template.vpy | 110 ++++++++++--------- worker/tests/filter_integration_test.rs | 17 ++- 12 files changed, 251 insertions(+), 151 deletions(-) diff --git a/README.md b/README.md index 27fab7f..b36de6f 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ Twenty-one filters, each switchable independently, applied in a fixed order. Mos | **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs last before cropping, so a small crop removes the edges it exposes. | | **Film Grain** | Grain added back after denoising, so the picture is not left plastic — and to hide banding in skies and fades. | | **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. | -| **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. | +| **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. Runs after the resize, on the picture that is actually delivered, and before any added grain. | | **Chroma Fixes** | Colour that sits sideways from the picture (corrected automatically or by hand), bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. Each repair has its own switch, and its settings appear only once it is on. | | **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. Levels and white balance can each be measured automatically or set by hand. | | **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. | @@ -123,7 +123,7 @@ Each filter leads with a plain-language summary and a **More** expander describi The list also reacts to the file you dropped in. Filters that match what was detected in your source are marked **Suggested** with the reason — "source is hard telecine (3:2 pulldown)", "anamorphic source (10:11) — check pixel aspect" — and ones that can't apply say so, such as deinterlacing a progressive file. Nothing is switched on or off for you; detection is sometimes wrong, so it stays a hint. Filters whose problems can't be spotted from the file alone — dirt, scratches, grain, halos — say nothing either way. -Where two filters work against each other, the one that loses out says so when you open it: sharpening ahead of a denoiser that will undo it, for instance. +Where two filters work against each other, the one that loses out says so when you open it: sharpening that would put back the halos Dehalo has just removed, for instance. ## Details diff --git a/app/lib/models/pass_advice.dart b/app/lib/models/pass_advice.dart index 5c083fe..bb94c4e 100644 --- a/app/lib/models/pass_advice.dart +++ b/app/lib/models/pass_advice.dart @@ -16,8 +16,8 @@ class PassAdvice { /// /// At a dozen-odd passes the complexity that actually costs users is not the /// length of the list, it is *interaction*: two denoisers stacked until the -/// picture is plastic, sharpening applied before the denoiser eats it again, -/// grain re-added before the deband that will smooth it away. None of that is +/// picture is plastic, sharpening that puts back the halos Dehalo just removed, +/// sharpening that exaggerates the grain Deband added. None of that is /// an error — every combination here produces a valid render — so none of it can /// be caught by validation. It has to be said out loud at the point the user is /// looking. @@ -29,12 +29,16 @@ class PassAdvice { /// app's job is to make sure they meant it. /// - **Only fires on enabled passes.** Advice about a pass nobody has turned on /// is noise. -/// - **Says what to do, not just what is wrong.** "Sharpen runs before Noise -/// Reduction" is a fact; the useful half is that the denoiser will undo it. +/// - **Says what to do, not just what is wrong.** "Dehalo runs before Sharpen" +/// is a fact; the useful half is that sharpening can put the halos back. +/// - **Never states an order the pipeline does not have.** Every "runs +/// before/after" below is pinned against `enabledPasses` in +/// `pass_advice_test.dart`, because a wrong one reads as a design flaw in the +/// app rather than as a typo (issue #109). /// /// Pass order is fixed (see `PassListPanel.stages` and `script_generator.rs`), /// which is what makes the ordering advice statable at all: Sharpen genuinely -/// always runs before Color Correction, so there is no case to qualify. +/// always runs after Noise Reduction, so there is no case to qualify. List adviseOn(ProcessingPipeline pipeline) { final advice = []; @@ -49,18 +53,13 @@ List adviseOn(ProcessingPipeline pipeline) { // combination. Deliberately silent. } - // --- Sharpening fights the denoiser, and loses --- - // Sharpen runs *before* Noise Reduction in the pipeline, so a denoiser set - // strongly enough to matter will remove most of what was just sharpened, and - // amplified noise is what survives. - if (on(PassType.sharpen) && on(PassType.noiseReduction)) { - advice.add(const PassAdvice( - PassType.sharpen, - 'Sharpen runs before Noise Reduction, so the denoiser will soften much ' - 'of this again — and sharpened noise is what it has to work on. ' - 'Consider denoising alone first and judging the result.', - )); - } + // --- Sharpening after the denoiser --- + // Deliberately silent, and asserted so in pass_advice_test.dart. Sharpen runs + // *after* Noise Reduction — after every clean-up pass and the resize, in + // fact, with only Grain and Frame Rate behind it — which is the order a + // restoration chain wants: there is nothing to warn about. This used to claim + // the reverse and told users the denoiser would undo their sharpening + // (issue #109) — it never did. // --- Deband after grain --- // Deband runs after the denoiser but the f3kdb grain it adds back is applied diff --git a/app/lib/models/processing_pipeline.dart b/app/lib/models/processing_pipeline.dart index f15e23a..ffd9072 100644 --- a/app/lib/models/processing_pipeline.dart +++ b/app/lib/models/processing_pipeline.dart @@ -254,7 +254,7 @@ class ProcessingPipeline { /// Get the ordered list of enabled passes. List get enabledPasses { final passes = []; - // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, sharpen, chroma, color, resize last + // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, anti-alias, chroma, color, resize, then sharpen, grain, frame rate if (cropResize.enabled && cropResize.cropEnabled) { passes.add(PassType.cropResize); // Pre-crop } @@ -304,9 +304,6 @@ class ProcessingPipeline { if (antiAlias.enabled) { passes.add(PassType.antiAlias); } - if (sharpen.enabled) { - passes.add(PassType.sharpen); - } if (chromaFixes.enabled) { passes.add(PassType.chromaFixes); } @@ -330,6 +327,13 @@ class ProcessingPipeline { passes.add(PassType.cropResize); } } + // Sharpening follows the resize (issue #109), so it works on the delivered + // pixels: sharpened earlier, a downscale softens the edges again and an + // upscale enlarges the halos. It still follows anti-aliasing, and precedes + // the grain it would otherwise exaggerate. + if (sharpen.enabled) { + passes.add(PassType.sharpen); + } // Grain goes last of the video passes: added before the resize it is // resampled away, before the deband it is smoothed away. if (grain.hasEffect) { diff --git a/app/lib/views/pass_list/pass_list_panel.dart b/app/lib/views/pass_list/pass_list_panel.dart index dc4c843..cb7b290 100644 --- a/app/lib/views/pass_list/pass_list_panel.dart +++ b/app/lib/views/pass_list/pass_list_panel.dart @@ -58,7 +58,6 @@ class PassListPanel extends StatelessWidget { title: 'Detail & Color', passes: [ PassType.antiAlias, - PassType.sharpen, PassType.chromaFixes, PassType.colorCorrection, ], @@ -69,7 +68,7 @@ class PassListPanel extends StatelessWidget { ), ( title: 'Finishing', - passes: [PassType.grain, PassType.frameRate], + passes: [PassType.sharpen, PassType.grain, PassType.frameRate], ), ( title: 'Post-Processing', diff --git a/app/test/integration_new_passes_test.dart b/app/test/integration_new_passes_test.dart index ace9d2f..2a7cd4d 100644 --- a/app/test/integration_new_passes_test.dart +++ b/app/test/integration_new_passes_test.dart @@ -23,6 +23,7 @@ import 'package:vapourbox/models/chroma_denoise_parameters.dart'; import 'package:vapourbox/models/dehalo_parameters.dart'; import 'package:vapourbox/models/chroma_fix_parameters.dart'; import 'package:vapourbox/models/color_correction_parameters.dart'; +import 'package:vapourbox/models/crop_resize_parameters.dart'; import 'package:vapourbox/models/descratch_parameters.dart'; import 'package:vapourbox/models/deblock_parameters.dart'; import 'package:vapourbox/models/grain_parameters.dart'; @@ -235,6 +236,35 @@ void main() { await _expectValidVideo(result); }, timeout: const Timeout(Duration(minutes: 6))); + // Issue #109: Sharpen moved to after the resize. Every method has to run + // on a clip the resize has already changed the size of, in the encode — the + // script-only tests prove the order, not that the plugins accept it. + for (final method in SharpenMethod.values) { + test('sharpen: ${method.name} runs after a resize', () async { + final label = 'sharpen_after_resize_${method.name}'; + final job = _baseJob( + label, + pipeline: ProcessingPipeline( + deinterlace: const QTGMCParameters(enabled: false), + cropResize: const CropResizeParameters( + enabled: true, + resizeEnabled: true, + targetWidth: 640, + targetHeight: 480, + ), + sharpen: SharpenParameters(enabled: true, method: method), + ), + ); + final result = await WorkerHarness.runJob(job.toJson(), label: label); + await _expectValidVideo(result); + final v = await WorkerHarness.firstStream(result.outputPath!, + selector: 'v:0', entries: ['width', 'height']); + // 720x576 fitted inside 640x480 with the stored aspect kept: 600x480. + expect(v?['width'], 600, reason: 'sharpening must not undo the resize'); + expect(v?['height'], 480); + }, timeout: const Timeout(Duration(minutes: 6))); + } + test('dehalo: HQDeringmod runs end-to-end', () async { final job = _baseJob( 'dehalo_hqderingmod', diff --git a/app/test/pass_advice_test.dart b/app/test/pass_advice_test.dart index e7cd3fc..b1df7cb 100644 --- a/app/test/pass_advice_test.dart +++ b/app/test/pass_advice_test.dart @@ -50,22 +50,24 @@ void main() { }); }); - group('sharpening against the denoiser', () { - test('warns on the Sharpen pass, naming the consequence', () { - final advice = adviceFor( - PassType.sharpen, - ProcessingPipeline( + group('sharpening with the denoiser', () { + // Issue #109: this pair used to warn that "Sharpen runs before Noise + // Reduction, so the denoiser will soften much of this again". It does not + // — Sharpen runs after it — so the warning described a flaw the pipeline + // does not have. + test('is silent, since Sharpen already runs after Noise Reduction', () { + expect( + adviseOn(ProcessingPipeline( deinterlace: const QTGMCParameters(enabled: false), noiseReduction: NoiseReductionParameters.fromPreset(NoiseReductionPreset.heavy), sharpen: const SharpenParameters(enabled: true), - ), + )), + isEmpty, ); - expect(advice, isNotNull); - expect(advice, contains('denoiser')); }); - test('silent when only one of the two is on', () { + test('silent when only Sharpen is on', () { expect( adviceFor( PassType.sharpen, @@ -79,6 +81,35 @@ void main() { }); }); + group('the order the advice talks about is the order the pipeline has', () { + // The advice states pass order as fact. These pin each statement to + // enabledPasses, so the text and the pipeline cannot drift apart again. + final order = ProcessingPipeline( + deinterlace: const QTGMCParameters(enabled: false), + noiseReduction: + NoiseReductionParameters.fromPreset(NoiseReductionPreset.moderate), + chromaDenoise: const ChromaDenoiseParameters(enabled: true), + dehalo: const DehaloParameters(enabled: true), + deband: const DebandParameters(enabled: true), + sharpen: const SharpenParameters(enabled: true), + ).enabledPasses; + + test('Sharpen runs after every clean-up pass', () { + final sharpen = order.indexOf(PassType.sharpen); + expect(sharpen, isNonNegative); + for (final earlier in const [ + PassType.noiseReduction, + PassType.chromaDenoise, + PassType.dehalo, + PassType.deband, + ]) { + expect(order.indexOf(earlier), isNonNegative, reason: '$earlier'); + expect(order.indexOf(earlier), lessThan(sharpen), + reason: '$earlier must run before Sharpen'); + } + }); + }); + group('dehalo against sharpening', () { test('warns on Sharpen, since Dehalo runs first', () { final advice = adviceFor( diff --git a/app/test/pass_list_stages_test.dart b/app/test/pass_list_stages_test.dart index 247c35c..88c8b93 100644 --- a/app/test/pass_list_stages_test.dart +++ b/app/test/pass_list_stages_test.dart @@ -48,12 +48,12 @@ void main() { PassType.deblock, PassType.deband, PassType.antiAlias, - PassType.sharpen, PassType.chromaFixes, PassType.colorCorrection, PassType.stabilize, PassType.geometry, PassType.cropResize, + PassType.sharpen, PassType.grain, PassType.frameRate, PassType.subtitles, @@ -72,6 +72,15 @@ void main() { lessThan(indexOf(PassType.sharpen))); }); + test('sharpening runs after the resize and before the grain', () { + // Issue #109. Sharpened before the resize, a downscale softens the edges + // again and an upscale enlarges the halos; sharpened after the grain, the + // grain is exaggerated. + expect(indexOf(PassType.sharpen), + greaterThan(indexOf(PassType.cropResize))); + expect(indexOf(PassType.sharpen), lessThan(indexOf(PassType.grain))); + }); + test('rotation settles before any framing decision', () { // A quarter turn swaps width and height, so crop, resize and the aspect // declaration must all see the final shape. diff --git a/docs/ENGINEERING_NOTES.md b/docs/ENGINEERING_NOTES.md index b980c3b..76c08f5 100644 --- a/docs/ENGINEERING_NOTES.md +++ b/docs/ENGINEERING_NOTES.md @@ -884,6 +884,16 @@ Two orderings are load-bearing and asserted from both sides - **Anti-aliasing runs before Sharpen.** Sharpening a stair-stepped edge makes the stepping more visible, not less. +- **Sharpen runs after Crop/Resize and before Grain** (added 2026-10-05, issue + #109 — it used to sit straight after Anti-Aliasing, ahead of Chroma Fixes, + Colour Correction, Stabilize and the resize). Resampling a sharpened picture + softens it again on a downscale and enlarges the sharpening halos on an + upscale, and Stabilize's sub-pixel shifts soften it too. Consequences worth + knowing: Sharpen now works at the **output** resolution, so it costs more + when upscaling and less when downscaling, and existing presets and saved + jobs that combine Sharpen with a resize render slightly differently than + before. The same issue also removed a `pass_advice.dart` warning that claimed + Sharpen ran *before* Noise Reduction — it never did. - **Stabilize runs last before Crop/Resize.** It shifts the picture within the frame and exposes thin empty edges, so a crop afterwards removes them. diff --git a/worker/src/models/processing_pipeline.rs b/worker/src/models/processing_pipeline.rs index 827caf5..6e570e1 100644 --- a/worker/src/models/processing_pipeline.rs +++ b/worker/src/models/processing_pipeline.rs @@ -337,7 +337,7 @@ impl ProcessingPipeline { pub fn enabled_passes(&self) -> Vec { let mut passes = Vec::new(); - // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, sharpen, chroma, color, resize last + // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, anti-alias, chroma, color, resize, then sharpen, grain, frame rate if self.crop_resize.enabled && self.crop_resize.crop_enabled { passes.push(PassType::CropResize); // Pre-crop } @@ -390,9 +390,6 @@ impl ProcessingPipeline { if self.anti_alias.enabled { passes.push(PassType::AntiAlias); } - if self.sharpen.enabled { - passes.push(PassType::Sharpen); - } if self.chroma_fixes.enabled { passes.push(PassType::ChromaFixes); } @@ -417,6 +414,14 @@ impl ProcessingPipeline { } } + // Sharpening follows the resize (issue #109), so it works on the + // delivered pixels: sharpened earlier, a downscale softens the edges + // again and an upscale enlarges the halos. It still follows + // anti-aliasing, and precedes the grain it would otherwise exaggerate. + if self.sharpen.enabled { + passes.push(PassType::Sharpen); + } + // Grain goes last of the video passes. Added before the resize it is // resampled away; before the deband it is smoothed away. Measured, and // the reason this pass sits after framing rather than with the other diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index d7b4ec0..f28075e 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -1247,59 +1247,6 @@ clip = haf.santiag( {{/AA_SANTIAG}} {{/ANTI_ALIAS}} -# ============================================================================ -# PASS 7: SHARPEN -# ============================================================================ -{{#SHARPEN}} - -{{#SHARPEN_LSFMOD}} -# LSFmod - Limited Sharpening with overshoot control -clip = haf.LSFmod( - clip, -{{#SHARPEN_STRENGTH}} - strength={{SHARPEN_STRENGTH}}, -{{/SHARPEN_STRENGTH}} -{{#SHARPEN_OVERSHOOT}} - overshoot={{SHARPEN_OVERSHOOT}}, -{{/SHARPEN_OVERSHOOT}} -{{#SHARPEN_UNDERSHOOT}} - undershoot={{SHARPEN_UNDERSHOOT}}, -{{/SHARPEN_UNDERSHOOT}} -{{#SHARPEN_SOFT_EDGE}} - soft={{SHARPEN_SOFT_EDGE}}, -{{/SHARPEN_SOFT_EDGE}} -) -{{/SHARPEN_LSFMOD}} - -{{#SHARPEN_CAS}} -# CAS - Contrast Adaptive Sharpening -clip = core.cas.CAS( - clip, -{{#SHARPEN_CAS_SHARPNESS}} - sharpness={{SHARPEN_CAS_SHARPNESS}}, -{{/SHARPEN_CAS_SHARPNESS}} -) -{{/SHARPEN_CAS}} -{{#SHARPEN_AWARPSHARP2}} -# aWarpSharp2 - sharpens by warping pixels toward edges rather than raising -# local contrast, so it produces no halos. -# -# `chroma` and `planes` are deliberately not passed. This VapourSynth port takes -# chroma as 0 or 1 (NOT Avisynth's 0-6, where 4 means "warp chroma with the luma -# mask") and rejects anything else outright, and by default it processes luma -# only — the conventional use as a sharpener. Passing a guessed value here is how -# this block first shipped broken; expose it deliberately, with a test, or leave -# the plugin's own default alone. -clip = core.warp.AWarpSharp2( - clip, - thresh={{SHARPEN_WARP_THRESH}}, - blur={{SHARPEN_WARP_BLUR}}, - type={{SHARPEN_WARP_TYPE}}, - depth={{SHARPEN_WARP_DEPTH}}, -) -{{/SHARPEN_AWARPSHARP2}} -{{/SHARPEN}} - # ============================================================================ # PASS 8: CHROMA FIXES # ============================================================================ @@ -1954,6 +1901,63 @@ _aspect_box_h = _even(_box_h) if _box_h > 0 else 0 {{/RESIZE_STANDARD}} {{/RESIZE}} +# ============================================================================ +# PASS: SHARPEN +# ============================================================================ +# Sharpening runs after the resize (issue #109), on the pixels that are actually +# delivered. Sharpened before it, the resampling softens the edges again on a +# downscale and enlarges the sharpening halos on an upscale. It stays ahead of +# the grain, which it would otherwise exaggerate. +{{#SHARPEN}} + +{{#SHARPEN_LSFMOD}} +# LSFmod - Limited Sharpening with overshoot control +clip = haf.LSFmod( + clip, +{{#SHARPEN_STRENGTH}} + strength={{SHARPEN_STRENGTH}}, +{{/SHARPEN_STRENGTH}} +{{#SHARPEN_OVERSHOOT}} + overshoot={{SHARPEN_OVERSHOOT}}, +{{/SHARPEN_OVERSHOOT}} +{{#SHARPEN_UNDERSHOOT}} + undershoot={{SHARPEN_UNDERSHOOT}}, +{{/SHARPEN_UNDERSHOOT}} +{{#SHARPEN_SOFT_EDGE}} + soft={{SHARPEN_SOFT_EDGE}}, +{{/SHARPEN_SOFT_EDGE}} +) +{{/SHARPEN_LSFMOD}} + +{{#SHARPEN_CAS}} +# CAS - Contrast Adaptive Sharpening +clip = core.cas.CAS( + clip, +{{#SHARPEN_CAS_SHARPNESS}} + sharpness={{SHARPEN_CAS_SHARPNESS}}, +{{/SHARPEN_CAS_SHARPNESS}} +) +{{/SHARPEN_CAS}} +{{#SHARPEN_AWARPSHARP2}} +# aWarpSharp2 - sharpens by warping pixels toward edges rather than raising +# local contrast, so it produces no halos. +# +# `chroma` and `planes` are deliberately not passed. This VapourSynth port takes +# chroma as 0 or 1 (NOT Avisynth's 0-6, where 4 means "warp chroma with the luma +# mask") and rejects anything else outright, and by default it processes luma +# only — the conventional use as a sharpener. Passing a guessed value here is how +# this block first shipped broken; expose it deliberately, with a test, or leave +# the plugin's own default alone. +clip = core.warp.AWarpSharp2( + clip, + thresh={{SHARPEN_WARP_THRESH}}, + blur={{SHARPEN_WARP_BLUR}}, + type={{SHARPEN_WARP_TYPE}}, + depth={{SHARPEN_WARP_DEPTH}}, +) +{{/SHARPEN_AWARPSHARP2}} +{{/SHARPEN}} + # ============================================================================ # PASS: FILM GRAIN # ============================================================================ diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 191afee..91d9ae1 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -1178,59 +1178,6 @@ clip = haf.santiag( {{/AA_SANTIAG}} {{/ANTI_ALIAS}} -# ============================================================================ -# PASS 7: SHARPEN -# ============================================================================ -{{#SHARPEN}} - -{{#SHARPEN_LSFMOD}} -# LSFmod - Limited Sharpening with overshoot control -clip = haf.LSFmod( - clip, -{{#SHARPEN_STRENGTH}} - strength={{SHARPEN_STRENGTH}}, -{{/SHARPEN_STRENGTH}} -{{#SHARPEN_OVERSHOOT}} - overshoot={{SHARPEN_OVERSHOOT}}, -{{/SHARPEN_OVERSHOOT}} -{{#SHARPEN_UNDERSHOOT}} - undershoot={{SHARPEN_UNDERSHOOT}}, -{{/SHARPEN_UNDERSHOOT}} -{{#SHARPEN_SOFT_EDGE}} - soft={{SHARPEN_SOFT_EDGE}}, -{{/SHARPEN_SOFT_EDGE}} -) -{{/SHARPEN_LSFMOD}} - -{{#SHARPEN_CAS}} -# CAS - Contrast Adaptive Sharpening -clip = core.cas.CAS( - clip, -{{#SHARPEN_CAS_SHARPNESS}} - sharpness={{SHARPEN_CAS_SHARPNESS}}, -{{/SHARPEN_CAS_SHARPNESS}} -) -{{/SHARPEN_CAS}} -{{#SHARPEN_AWARPSHARP2}} -# aWarpSharp2 - sharpens by warping pixels toward edges rather than raising -# local contrast, so it produces no halos. -# -# `chroma` and `planes` are deliberately not passed. This VapourSynth port takes -# chroma as 0 or 1 (NOT Avisynth's 0-6, where 4 means "warp chroma with the luma -# mask") and rejects anything else outright, and by default it processes luma -# only — the conventional use as a sharpener. Passing a guessed value here is how -# this block first shipped broken; expose it deliberately, with a test, or leave -# the plugin's own default alone. -clip = core.warp.AWarpSharp2( - clip, - thresh={{SHARPEN_WARP_THRESH}}, - blur={{SHARPEN_WARP_BLUR}}, - type={{SHARPEN_WARP_TYPE}}, - depth={{SHARPEN_WARP_DEPTH}}, -) -{{/SHARPEN_AWARPSHARP2}} -{{/SHARPEN}} - # ============================================================================ # PASS 8: CHROMA FIXES # ============================================================================ @@ -1885,6 +1832,63 @@ _aspect_box_h = _even(_box_h) if _box_h > 0 else 0 {{/RESIZE_STANDARD}} {{/RESIZE}} +# ============================================================================ +# PASS: SHARPEN +# ============================================================================ +# Sharpening runs after the resize (issue #109), on the pixels that are actually +# delivered. Sharpened before it, the resampling softens the edges again on a +# downscale and enlarges the sharpening halos on an upscale. It stays ahead of +# the grain, which it would otherwise exaggerate. +{{#SHARPEN}} + +{{#SHARPEN_LSFMOD}} +# LSFmod - Limited Sharpening with overshoot control +clip = haf.LSFmod( + clip, +{{#SHARPEN_STRENGTH}} + strength={{SHARPEN_STRENGTH}}, +{{/SHARPEN_STRENGTH}} +{{#SHARPEN_OVERSHOOT}} + overshoot={{SHARPEN_OVERSHOOT}}, +{{/SHARPEN_OVERSHOOT}} +{{#SHARPEN_UNDERSHOOT}} + undershoot={{SHARPEN_UNDERSHOOT}}, +{{/SHARPEN_UNDERSHOOT}} +{{#SHARPEN_SOFT_EDGE}} + soft={{SHARPEN_SOFT_EDGE}}, +{{/SHARPEN_SOFT_EDGE}} +) +{{/SHARPEN_LSFMOD}} + +{{#SHARPEN_CAS}} +# CAS - Contrast Adaptive Sharpening +clip = core.cas.CAS( + clip, +{{#SHARPEN_CAS_SHARPNESS}} + sharpness={{SHARPEN_CAS_SHARPNESS}}, +{{/SHARPEN_CAS_SHARPNESS}} +) +{{/SHARPEN_CAS}} +{{#SHARPEN_AWARPSHARP2}} +# aWarpSharp2 - sharpens by warping pixels toward edges rather than raising +# local contrast, so it produces no halos. +# +# `chroma` and `planes` are deliberately not passed. This VapourSynth port takes +# chroma as 0 or 1 (NOT Avisynth's 0-6, where 4 means "warp chroma with the luma +# mask") and rejects anything else outright, and by default it processes luma +# only — the conventional use as a sharpener. Passing a guessed value here is how +# this block first shipped broken; expose it deliberately, with a test, or leave +# the plugin's own default alone. +clip = core.warp.AWarpSharp2( + clip, + thresh={{SHARPEN_WARP_THRESH}}, + blur={{SHARPEN_WARP_BLUR}}, + type={{SHARPEN_WARP_TYPE}}, + depth={{SHARPEN_WARP_DEPTH}}, +) +{{/SHARPEN_AWARPSHARP2}} +{{/SHARPEN}} + # ============================================================================ # PASS: FILM GRAIN # ============================================================================ diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index f6808da..1f715f7 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -4128,11 +4128,13 @@ fn test_109_lut_derainbow_with_ten_bit_guard() { #[test] fn test_110_new_passes_run_in_the_documented_order() { - // The order the passes appear in the script is the order they run, and two - // of these placements are deliberate: anti-aliasing BEFORE sharpening - // (sharpening stair-stepped edges makes the stepping worse), and - // stabilisation LAST before framing (so a crop can remove the borders it - // shifts into view). + // The order the passes appear in the script is the order they run, and + // three of these placements are deliberate: anti-aliasing BEFORE sharpening + // (sharpening stair-stepped edges makes the stepping worse), stabilisation + // LAST before framing (so a crop can remove the borders it shifts into + // view), and sharpening AFTER the resize (issue #109: resampling a + // sharpened picture softens it again on a downscale and enlarges the halos + // on an upscale). create_output_dir(); let mut job = create_base_job("test_110_order"); job.qtgmc_parameters.enabled = false; @@ -4159,8 +4161,11 @@ fn test_110_new_passes_run_in_the_documented_order() { let aa = script.find("haf.daa(").expect("daa should be present"); let sharp = script.find("core.cas.CAS(").expect("CAS should be present"); let stab = script.find("haf.Stab(").expect("Stab should be present"); + let resize = script.find("height=target_h").expect("resize should be present"); assert!(aa < sharp, "anti-aliasing must run before sharpening"); - assert!(sharp < stab, "stabilisation runs after the detail passes"); + assert!(aa < stab, "stabilisation runs after the detail passes"); + assert!(stab < resize, "stabilisation runs before framing"); + assert!(resize < sharp, "sharpening must run after the resize"); } // ============================================================================ From 5cfde257b54b82550ef21d7ad305ad9291868ff2 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Mon, 5 Oct 2026 21:19:32 +1100 Subject: [PATCH 2/3] docs(readme): list the filters in the order they run (#109) The filter table was in no particular order, so the README never said what the pipeline order is. Number the rows in run order, explain the parts a table cannot show (the crop runs first, the resize late; custom code, colour conversion and borders follow the filters), and pin the table to PassListPanel.stages with a test. Also correct the Stabilize row, which said a crop afterwards removes the edges it exposes. The crop is applied before Deinterlace. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN --- CLAUDE.md | 3 ++ README.md | 57 +++++++++++++----------- app/test/readme_pipeline_order_test.dart | 52 +++++++++++++++++++++ 3 files changed, 87 insertions(+), 25 deletions(-) create mode 100644 app/test/readme_pipeline_order_test.dart diff --git a/CLAUDE.md b/CLAUDE.md index efb1d02..aeb410b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -319,6 +319,9 @@ Adding a filter touches many files. Missing any step causes silent failures (fil `pass_list_stages_test.dart` fails if one is missed. Stages are labels over the **existing pipeline order**, so a pass goes in the stage its position already falls in; never reorder rows to suit a grouping. + The README's "The filter pipeline" table is the user-facing statement of the + same order — add the pass's row at its position there too; + `readme_pipeline_order_test.dart` fails if the table and `stages` disagree. - `app/lib/views/pass_list/pass_list_item.dart` — add icon in `_getIconForPass()` - `app/lib/views/pass_settings/pass_settings_inline.dart` — add case in `_getFilterId()` - `app/lib/viewmodels/main_viewmodel.dart` — add case in BOTH `_convertToParams()` AND `_updatePipelineFromDynamic()` diff --git a/README.md b/README.md index b36de6f..3643ba9 100644 --- a/README.md +++ b/README.md @@ -93,31 +93,38 @@ GPU-accelerated deinterlacing (NNEDI3CL) needs your GPU's OpenCL driver installe ## The filter pipeline -Twenty-one filters, each switchable independently, applied in a fixed order. Most sources need none or a few. - -| Filter | What it addresses | -|--------|-------------------| -| **Deinterlace** | Comb-like jagged edges on moving objects. QTGMC for interlaced video, IVTC to recover the original film frames from telecined DVD, or Bwdif when you want it done in a fraction of the time. | -| **Edge Repair** | The dirty rows and columns at the very edge of a tape capture — rebuilt from the picture just inside, instead of cropped away. | -| **Ghost Removal** | A faint second copy of the picture shifted sideways, left behind by an aerial or a long cable run. | -| **Deflicker** | Brightness pulsing between frames, which is what scanned cine film almost always has. | -| **DeScratch** | Vertical scratch lines on scanned film. | -| **SpotLess** | Dust, dirt and single-frame specks. RemoveDirt is the faster choice — around six times the speed for about 60% of the removal. | -| **Noise Reduction** | Grain and video noise across the whole frame. Motion-compensated by default, with mClean as a gentler alternative that keeps more detail; DFTTest, FFT3DFilter, TTempSmooth, FluxSmooth, STPresso, TemporalDegrain2 and a large-window median are available under advanced options for noise the default handles badly. | -| **Chroma Denoise** | Blotchy, smeared color — common on VHS captures and old camcorder footage. Leaves luma detail untouched. | -| **Dehalo** | Bright outlines around edges, ringing, and residual ghosting left by a deinterlacer. HQDeringmod targets ringing specifically. | -| **Deblock** | Square blocking from heavy compression, and the ringing around edges that comes with it. | -| **Deband** | Visible steps in gradients and skies. | -| **Anti-Aliasing** | Stair-stepping on diagonal edges, left by deinterlacing or upscaling. Runs before sharpening, which would otherwise make the steps more visible. | -| **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs last before cropping, so a small crop removes the edges it exposes. | -| **Film Grain** | Grain added back after denoising, so the picture is not left plastic — and to hide banding in skies and fades. | -| **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. | -| **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. Runs after the resize, on the picture that is actually delivered, and before any added grain. | -| **Chroma Fixes** | Colour that sits sideways from the picture (corrected automatically or by hand), bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. Each repair has its own switch, and its settings appear only once it is on. | -| **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. Levels and white balance can each be measured automatically or set by hand. | -| **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. | -| **Frame Rate** | Converting between PAL and NTSC rates, for a tape that was already converted once and now plays at the wrong speed. | -| **Subtitles** | Whisper AI speech-to-text — written alongside the video as `.srt`, embedded as a selectable track, burnt into the picture, or a combination. | +Twenty-one filters, each switchable independently. Most sources need none or a few. They always run in the order below, top to bottom, whichever ones are switched on — the same order the list in the app shows them in. The order cannot be rearranged. + +| # | Filter | What it addresses | +|---|--------|-------------------| +| 1 | **Deinterlace** | Comb-like jagged edges on moving objects. QTGMC for interlaced video, IVTC to recover the original film frames from telecined DVD, or Bwdif when you want it done in a fraction of the time. | +| 2 | **Edge Repair** | The dirty rows and columns at the very edge of a tape capture — rebuilt from the picture just inside, instead of cropped away. | +| 3 | **Ghost Removal** | A faint second copy of the picture shifted sideways, left behind by an aerial or a long cable run. | +| 4 | **Deflicker** | Brightness pulsing between frames, which is what scanned cine film almost always has. | +| 5 | **DeScratch** | Vertical scratch lines on scanned film. | +| 6 | **SpotLess** | Dust, dirt and single-frame specks. RemoveDirt is the faster choice — around six times the speed for about 60% of the removal. | +| 7 | **Noise Reduction** | Grain and video noise across the whole frame. Motion-compensated by default, with mClean as a gentler alternative that keeps more detail; DFTTest, FFT3DFilter, TTempSmooth, FluxSmooth, STPresso, TemporalDegrain2 and a large-window median are available under advanced options for noise the default handles badly. | +| 8 | **Chroma Denoise** | Blotchy, smeared color — common on VHS captures and old camcorder footage. Leaves luma detail untouched. | +| 9 | **Dehalo** | Bright outlines around edges, ringing, and residual ghosting left by a deinterlacer. HQDeringmod targets ringing specifically. | +| 10 | **Deblock** | Square blocking from heavy compression, and the ringing around edges that comes with it. | +| 11 | **Deband** | Visible steps in gradients and skies. | +| 12 | **Anti-Aliasing** | Stair-stepping on diagonal edges, left by deinterlacing or upscaling. Runs before sharpening, which would otherwise make the steps more visible. | +| 13 | **Chroma Fixes** | Colour that sits sideways from the picture (corrected automatically or by hand), bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. Each repair has its own switch, and its settings appear only once it is on. | +| 14 | **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. Levels and white balance can each be measured automatically or set by hand. | +| 15 | **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs once the picture has been cleaned and colour-corrected, just before it is turned and resized. | +| 16 | **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. | +| 17 | **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. | +| 18 | **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. Runs after the resize, on the picture that is actually delivered, and before any added grain. | +| 19 | **Film Grain** | Grain added back after denoising, so the picture is not left plastic — and to hide banding in skies and fades. | +| 20 | **Frame Rate** | Converting between PAL and NTSC rates, for a tape that was already converted once and now plays at the wrong speed. | +| 21 | **Subtitles** | Whisper AI speech-to-text — written alongside the video as `.srt`, embedded as a selectable track, burnt into the picture, or a combination. | + +A few things about the order are worth knowing: + +- **Cropping happens first, resizing near the end.** Crop & Resize is one entry in the list but two steps: the crop is applied before Deinterlace, so nothing wastes time on pixels that are being thrown away, and the resize takes the place shown in the table. Crop values therefore refer to the source as it was captured, before any rotation. +- **Clean up, then frame, then finish.** Damage and noise are removed first, colour is corrected next, the picture is stabilised, turned and resized, and only then sharpened. Sharpening earlier would have its work softened again by the resize. +- **Grain goes on after sharpening**, so it is neither sharpened into grit nor resampled away, and the frame rate is converted last of all, so every other filter works on real frames rather than invented ones. +- **After the filters:** any Custom VapourSynth code runs next, then the conversion to the output colour format, then added borders — last, so that nothing touches the bars. Subtitles are transcribed from the source and added once the video has been encoded. Each filter leads with a plain-language summary and a **More** expander describing what it does and when it's the right choice, so the settings can be understood in place rather than looked up elsewhere. diff --git a/app/test/readme_pipeline_order_test.dart b/app/test/readme_pipeline_order_test.dart new file mode 100644 index 0000000..9e15f6e --- /dev/null +++ b/app/test/readme_pipeline_order_test.dart @@ -0,0 +1,52 @@ +// The README's "The filter pipeline" table is where users are told what order +// the filters run in. It is prose, so nothing stops it drifting from the +// pipeline — and a documented order that is wrong is worse than none: issue +// #109 was a user reasoning from an in-app message that misstated the order. +// +// This pins the table, row for row, to PassListPanel.stages, which +// pass_list_stages_test.dart in turn pins to the pipeline. +// +// Run with: flutter test test/readme_pipeline_order_test.dart + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/models/processing_pipeline.dart'; +import 'package:vapourbox/views/pass_list/pass_list_panel.dart'; + +/// The README writes a couple of names differently from the pass list. +const _readmeNames = { + PassType.cropResize: 'Crop & Resize', +}; + +void main() { + final readme = File('../README.md').readAsLinesSync(); + final row = RegExp(r'^\| (\d+) \| \*\*(.+?)\*\* \|'); + + final start = readme.indexOf('## The filter pipeline'); + final rows = [ + for (final line in readme.skip(start + 1).takeWhile((l) => !l.startsWith('## '))) + if (row.firstMatch(line) case final m?) (int.parse(m.group(1)!), m.group(2)!), + ]; + + test('the README has a filter pipeline table', () { + expect(start, isNonNegative); + expect(rows, isNotEmpty); + }); + + test('the table lists every filter in the order it runs', () { + final expected = [ + for (final stage in PassListPanel.stages) + for (final pass in stage.passes) + _readmeNames[pass] ?? pass.displayName, + ]; + expect(rows.map((r) => r.$2).toList(), expected, + reason: 'README.md "The filter pipeline" must list the filters in ' + 'pipeline order — see PassListPanel.stages'); + }); + + test('the rows are numbered 1..n without gaps', () { + expect(rows.map((r) => r.$1).toList(), + [for (var i = 1; i <= rows.length; i++) i]); + }); +} From 5a651a94449421061a4cc24482e886b48246e84b Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Mon, 5 Oct 2026 21:43:20 +1100 Subject: [PATCH 3/3] fix(pipeline): crop with the resize, after Stabilize and Rotate / Flip (#109) The crop was a separate block at the top of both templates, ahead of Deinterlace, while the comments, the in-app descriptions and the README all said Stabilize runs last before cropping so that a crop can remove the edges it exposes. It could not. Move the crop to directly before the resize, so the order is Stabilize, Rotate / Flip, Crop, Resize, Sharpen. The pass now appears once in the pass order, at that position. Behaviour changes: crop sides refer to the picture after a rotation, every earlier pass works on the uncropped frame, Edge Repair rebuilds the uncropped edge, and the automatic measurements (levels, white balance, chroma alignment) see the area the crop will remove. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN --- README.md | 4 +-- app/lib/models/processing_pipeline.dart | 18 ++++++------- app/test/integration_new_passes_test.dart | 31 +++++++++++++++++++++++ docs/ENGINEERING_NOTES.md | 14 ++++++++++ worker/src/models/processing_pipeline.rs | 19 +++++++------- worker/src/script_generator.rs | 9 ++++--- worker/templates/pipeline_template.vpy | 20 ++++++++++----- worker/templates/preview_template.vpy | 20 ++++++++++----- worker/tests/filter_integration_test.rs | 21 +++++++++++++-- 9 files changed, 116 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 3643ba9..5de2655 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ Twenty-one filters, each switchable independently. Most sources need none or a f | 12 | **Anti-Aliasing** | Stair-stepping on diagonal edges, left by deinterlacing or upscaling. Runs before sharpening, which would otherwise make the steps more visible. | | 13 | **Chroma Fixes** | Colour that sits sideways from the picture (corrected automatically or by hand), bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. Each repair has its own switch, and its settings appear only once it is on. | | 14 | **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. Levels and white balance can each be measured automatically or set by hand. | -| 15 | **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs once the picture has been cleaned and colour-corrected, just before it is turned and resized. | +| 15 | **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs last before cropping, so a small crop removes the edges it exposes. | | 16 | **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. | | 17 | **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. | | 18 | **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. Runs after the resize, on the picture that is actually delivered, and before any added grain. | @@ -121,7 +121,7 @@ Twenty-one filters, each switchable independently. Most sources need none or a f A few things about the order are worth knowing: -- **Cropping happens first, resizing near the end.** Crop & Resize is one entry in the list but two steps: the crop is applied before Deinterlace, so nothing wastes time on pixels that are being thrown away, and the resize takes the place shown in the table. Crop values therefore refer to the source as it was captured, before any rotation. +- **Crop & Resize is one step, cropping first.** It runs after Stabilize and Rotate / Flip, so a small crop removes the thin edges stabilising exposes, and the four crop sides are the sides of the picture after any rotation. Everything before it works on the uncropped frame. - **Clean up, then frame, then finish.** Damage and noise are removed first, colour is corrected next, the picture is stabilised, turned and resized, and only then sharpened. Sharpening earlier would have its work softened again by the resize. - **Grain goes on after sharpening**, so it is neither sharpened into grit nor resampled away, and the frame rate is converted last of all, so every other filter works on real frames rather than invented ones. - **After the filters:** any Custom VapourSynth code runs next, then the conversion to the output colour format, then added borders — last, so that nothing touches the bars. Subtitles are transcribed from the source and added once the video has been encoded. diff --git a/app/lib/models/processing_pipeline.dart b/app/lib/models/processing_pipeline.dart index ffd9072..66f3bcd 100644 --- a/app/lib/models/processing_pipeline.dart +++ b/app/lib/models/processing_pipeline.dart @@ -254,10 +254,9 @@ class ProcessingPipeline { /// Get the ordered list of enabled passes. List get enabledPasses { final passes = []; - // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, anti-alias, chroma, color, resize, then sharpen, grain, frame rate - if (cropResize.enabled && cropResize.cropEnabled) { - passes.add(PassType.cropResize); // Pre-crop - } + // Order: deinterlace, damage, noise, dehalo, deblock, deband, anti-alias, + // chroma, color, stabilize, rotate, crop + resize, then sharpen, grain, + // frame rate. if (deinterlace.enabled) { passes.add(PassType.deinterlace); } @@ -321,11 +320,12 @@ class ProcessingPipeline { if (geometry.hasEffect) { passes.add(PassType.geometry); } - if (cropResize.enabled && cropResize.resizeEnabled) { - // Resize (post-processing) - if not already added for crop - if (!passes.contains(PassType.cropResize)) { - passes.add(PassType.cropResize); - } + // Crop and resize are one pass and run together, crop first. The crop used + // to run ahead of everything else, which meant it could not remove the + // edges Stabilize exposes (issue #109). + if (cropResize.enabled && + (cropResize.cropEnabled || cropResize.resizeEnabled)) { + passes.add(PassType.cropResize); } // Sharpening follows the resize (issue #109), so it works on the delivered // pixels: sharpened earlier, a downscale softens the edges again and an diff --git a/app/test/integration_new_passes_test.dart b/app/test/integration_new_passes_test.dart index 2a7cd4d..576ec97 100644 --- a/app/test/integration_new_passes_test.dart +++ b/app/test/integration_new_passes_test.dart @@ -265,6 +265,37 @@ void main() { }, timeout: const Timeout(Duration(minutes: 6))); } + // Issue #109: the crop used to run before everything else. It now runs + // with the resize, after Stabilize and Rotate / Flip, so its four sides + // are the sides of the turned picture. The frame size tells the two orders + // apart: 720x576 turned is 576x720, and cropping 16 off each side and 8 off + // top and bottom leaves 544x704. Cropped before the turn it was 560x688. + test('crop: runs after stabilize and rotation', () async { + const label = 'crop_after_rotation'; + final job = _baseJob( + label, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + stabilize: StabilizeParameters(enabled: true), + geometry: GeometryParameters(enabled: true, rotation: Rotation.cw90), + cropResize: CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: 16, + cropRight: 16, + cropTop: 8, + cropBottom: 8, + ), + ), + ); + final result = await WorkerHarness.runJob(job.toJson(), label: label); + await _expectValidVideo(result); + final v = await WorkerHarness.firstStream(result.outputPath!, + selector: 'v:0', entries: ['width', 'height']); + expect(v?['width'], 544); + expect(v?['height'], 704); + }, timeout: const Timeout(Duration(minutes: 6))); + test('dehalo: HQDeringmod runs end-to-end', () async { final job = _baseJob( 'dehalo_hqderingmod', diff --git a/docs/ENGINEERING_NOTES.md b/docs/ENGINEERING_NOTES.md index 76c08f5..b4ebe6e 100644 --- a/docs/ENGINEERING_NOTES.md +++ b/docs/ENGINEERING_NOTES.md @@ -896,6 +896,20 @@ Two orderings are load-bearing and asserted from both sides Sharpen ran *before* Noise Reduction — it never did. - **Stabilize runs last before Crop/Resize.** It shifts the picture within the frame and exposes thin empty edges, so a crop afterwards removes them. + **This was not true until 2026-10-05 (issue #109).** The *resize* ran after + Stabilize, but the crop was a separate `PRE_CROP` block at the top of both + templates, ahead of Deinterlace — so the reason given for the placement + described something the pipeline did not do, in this file, the README, the + model comments and `pass_list_stages_test.dart` alike. The crop is now a + `CROP` block directly before `RESIZE`, after Stabilize and Rotate / Flip. + What that costs, deliberately: every earlier pass works on the uncropped + frame (slower, by the cropped area), Edge Repair rebuilds the edge of the + *uncropped* frame, and the automatic measurements (levels, white balance, + chroma alignment) see whatever the crop was going to remove — head-switching + noise along the bottom of a VHS capture included. Crop values now refer to + the picture *after* a rotation, where they used to refer to the source. + `test_110` pins the script order; `integration_new_passes_test.dart` tells + the two orders apart by frame size on a rotated, cropped encode. > **`santiag`'s `type` is pinned to `nnedi3`.** havsfunc also accepts `eedi2` > and `sangnom`; **neither is in the deps bundle**, and naming an absent one diff --git a/worker/src/models/processing_pipeline.rs b/worker/src/models/processing_pipeline.rs index 6e570e1..6de91b5 100644 --- a/worker/src/models/processing_pipeline.rs +++ b/worker/src/models/processing_pipeline.rs @@ -337,10 +337,9 @@ impl ProcessingPipeline { pub fn enabled_passes(&self) -> Vec { let mut passes = Vec::new(); - // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, anti-alias, chroma, color, resize, then sharpen, grain, frame rate - if self.crop_resize.enabled && self.crop_resize.crop_enabled { - passes.push(PassType::CropResize); // Pre-crop - } + // Order: deinterlace, damage, noise, dehalo, deblock, deband, anti-alias, + // chroma, color, stabilize, rotate, crop + resize, then sharpen, grain, + // frame rate. if self.deinterlace_enabled() { passes.push(PassType::Deinterlace); } @@ -407,11 +406,13 @@ impl ProcessingPipeline { if self.geometry.has_effect() { passes.push(PassType::Geometry); } - if self.crop_resize.enabled && self.crop_resize.resize_enabled { - // Resize (post-processing) - if not already added for crop - if !passes.contains(&PassType::CropResize) { - passes.push(PassType::CropResize); - } + // Crop and resize are one pass and run together, crop first. The crop + // used to run ahead of everything else, which meant it could not + // remove the edges Stabilize exposes (issue #109). + if self.crop_resize.enabled + && (self.crop_resize.crop_enabled || self.crop_resize.resize_enabled) + { + passes.push(PassType::CropResize); } // Sharpening follows the resize (issue #109), so it works on the diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index 411107b..99a3540 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -362,19 +362,20 @@ impl ScriptGenerator { let params = &pipeline.deinterlace; // ==================================================================== - // PRE-CROP PASS + // CROP PASS (its block sits after Stabilize and Rotate / Flip, directly + // before the resize — the template's position is what orders it) // ==================================================================== let crop = &pipeline.crop_resize; if crop.enabled && crop.crop_enabled && (crop.crop_left > 0 || crop.crop_right > 0 || crop.crop_top > 0 || crop.crop_bottom > 0) { - script = script.replace("{{#PRE_CROP}}", ""); - script = script.replace("{{/PRE_CROP}}", ""); + script = script.replace("{{#CROP}}", ""); + script = script.replace("{{/CROP}}", ""); script = script.replace("{{CROP_LEFT}}", &crop.crop_left.to_string()); script = script.replace("{{CROP_RIGHT}}", &crop.crop_right.to_string()); script = script.replace("{{CROP_TOP}}", &crop.crop_top.to_string()); script = script.replace("{{CROP_BOTTOM}}", &crop.crop_bottom.to_string()); } else { - script = remove_block("{{#PRE_CROP}}", "{{/PRE_CROP}}", script); + script = remove_block("{{#CROP}}", "{{/CROP}}", script); } // ==================================================================== diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index f28075e..06c084a 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -84,13 +84,6 @@ def _expr(clips, expr, **kwargs): return _akarin_expr(clips, expr, **kwargs) return core.std.Expr(clips, expr, **kwargs) -# ============================================================================ -# PASS 1: PRE-CROP (before deinterlacing to reduce processing area) -# ============================================================================ -{{#PRE_CROP}} -clip = core.std.Crop(clip, left={{CROP_LEFT}}, right={{CROP_RIGHT}}, top={{CROP_TOP}}, bottom={{CROP_BOTTOM}}) -{{/PRE_CROP}} - # ============================================================================ # PASS 2: DEINTERLACING # ============================================================================ @@ -1707,6 +1700,19 @@ if clip.format.id != _geom_src_format.id: clip = core.resize.Spline36(clip, format=_geom_src_format.id) {{/GEOMETRY}} +# ============================================================================ +# PASS: CROP +# ============================================================================ +# Cropping runs here, with the resize, and not at the top of the script where it +# used to (issue #109). Stabilize shifts the picture and exposes thin edges, and +# a quarter turn swaps width and height, so the crop has to follow both: it then +# removes what Stabilize exposed, and its four sides mean the sides of the +# picture as it will be delivered. The cost is that every earlier pass works on +# the uncropped frame. +{{#CROP}} +clip = core.std.Crop(clip, left={{CROP_LEFT}}, right={{CROP_RIGHT}}, top={{CROP_TOP}}, bottom={{CROP_BOTTOM}}) +{{/CROP}} + # ============================================================================ # PASS 10: RESIZE / UPSCALE # ============================================================================ diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 91d9ae1..73be6d7 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -76,13 +76,6 @@ def _expr(clips, expr, **kwargs): return _akarin_expr(clips, expr, **kwargs) return core.std.Expr(clips, expr, **kwargs) -# ============================================================================ -# PASS 1: PRE-CROP (before deinterlacing to reduce processing area) -# ============================================================================ -{{#PRE_CROP}} -clip = core.std.Crop(clip, left={{CROP_LEFT}}, right={{CROP_RIGHT}}, top={{CROP_TOP}}, bottom={{CROP_BOTTOM}}) -{{/PRE_CROP}} - # ============================================================================ # PASS 2: DEINTERLACING # ============================================================================ @@ -1638,6 +1631,19 @@ if clip.format.id != _geom_src_format.id: clip = core.resize.Spline36(clip, format=_geom_src_format.id) {{/GEOMETRY}} +# ============================================================================ +# PASS: CROP +# ============================================================================ +# Cropping runs here, with the resize, and not at the top of the script where it +# used to (issue #109). Stabilize shifts the picture and exposes thin edges, and +# a quarter turn swaps width and height, so the crop has to follow both: it then +# removes what Stabilize exposed, and its four sides mean the sides of the +# picture as it will be delivered. The cost is that every earlier pass works on +# the uncropped frame. +{{#CROP}} +clip = core.std.Crop(clip, left={{CROP_LEFT}}, right={{CROP_RIGHT}}, top={{CROP_TOP}}, bottom={{CROP_BOTTOM}}) +{{/CROP}} + # ============================================================================ # PASS 10: RESIZE / UPSCALE # ============================================================================ diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 1f715f7..d418b3a 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -4134,7 +4134,9 @@ fn test_110_new_passes_run_in_the_documented_order() { // LAST before framing (so a crop can remove the borders it shifts into // view), and sharpening AFTER the resize (issue #109: resampling a // sharpened picture softens it again on a downscale and enlarges the halos - // on an upscale). + // on an upscale). The crop runs with the resize, AFTER stabilisation and + // rotation — it used to run before deinterlacing, where it could not remove + // the edges stabilisation exposes. create_output_dir(); let mut job = create_base_job("test_110_order"); job.qtgmc_parameters.enabled = false; @@ -4147,8 +4149,16 @@ fn test_110_new_passes_run_in_the_documented_order() { ..Default::default() }, stabilize: StabilizeParameters { enabled: true, ..Default::default() }, + geometry: GeometryParameters { + enabled: true, + flip_horizontal: true, + ..Default::default() + }, crop_resize: CropResizeParameters { enabled: true, + crop_enabled: true, + crop_left: 8, + crop_right: 8, resize_enabled: true, target_width: Some(640), target_height: Some(480), @@ -4158,13 +4168,20 @@ fn test_110_new_passes_run_in_the_documented_order() { }); let script = script_text(&job); + let crop = script + .find("core.std.Crop(clip, left=8, right=8") + .expect("crop should be present"); + let flip = script.find("FlipHorizontal").expect("flip should be present"); let aa = script.find("haf.daa(").expect("daa should be present"); let sharp = script.find("core.cas.CAS(").expect("CAS should be present"); let stab = script.find("haf.Stab(").expect("Stab should be present"); let resize = script.find("height=target_h").expect("resize should be present"); assert!(aa < sharp, "anti-aliasing must run before sharpening"); assert!(aa < stab, "stabilisation runs after the detail passes"); - assert!(stab < resize, "stabilisation runs before framing"); + assert!(stab < flip, "stabilisation runs before the picture is turned"); + assert!(flip < crop, "the crop must see the turned picture"); + assert!(stab < crop, "the crop must run after stabilisation"); + assert!(crop < resize, "the crop runs before the resize"); assert!(resize < sharp, "sharpening must run after the resize"); }