From 117e6b17b827c95a450198cd1416ec83a256feff Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Sat, 26 Sep 2026 18:40:58 +1000 Subject: [PATCH] Crop & Resize: Add Borders to a fixed canvas, in a chosen colour (#86) Adds a Borders section to Crop & Resize: pad the picture out to a fixed canvas (PAL/NTSC DVD, 720p, 1080p presets, or an exact size in advanced mode) without rescaling it, independent of Resize, with a black/grey/ white/custom fill colour. All padding, including the existing Pad to Fill, now happens in one BORDERS step at the very end of both templates, after grain and the output format conversion, so nothing touches the bars. The fill is converted into the clip's own format via resize with the source's matrix and range, which also fixes Pad to Fill's bars: AddBorders' default fill is luma 0, below video black on limited-range sources. Offsets stay on the chroma grid, and on the field grid when the picture is still interlaced; a picture larger than the canvas raises rather than producing the wrong frame size. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014GLXdGLfPwgYjW1AkonGqN --- CLAUDE.md | 25 ++ README.md | 2 +- app/assets/filters/core/crop_resize.json | 141 ++++++++- app/lib/models/crop_resize_parameters.dart | 50 ++++ app/lib/models/parameter_converter.dart | 15 + app/test/integration_borders_test.dart | 269 ++++++++++++++++++ .../integration_filter_parameters_test.dart | 34 +++ app/test/parameter_converter_test.dart | 30 ++ worker/src/models/color_metadata.rs | 61 +++- worker/src/models/crop_resize_parameters.rs | 150 ++++++++++ worker/src/script_generator.rs | 54 +++- worker/templates/pipeline_template.vpy | 99 ++++++- worker/templates/preview_template.vpy | 99 ++++++- worker/tests/filter_integration_test.rs | 173 +++++++++++ 14 files changed, 1167 insertions(+), 35 deletions(-) create mode 100644 app/test/integration_borders_test.dart diff --git a/CLAUDE.md b/CLAUDE.md index ad22d58..8a5635e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -698,6 +698,31 @@ Three things decide the shape of the output, and they live in different places: else so a typo falls back to the source's own aspect rather than reaching ffmpeg as a broken filter argument. +### Borders (issue #86) + +Two ways to add bars, sharing one step at the **very end of both templates** +(`{{#BORDERS}}`, after `CHROMA_CONVERT`): **Add Borders** (`padEnabled`/ +`padWidth`/`padHeight`) pads to a fixed canvas without rescaling, independent +of Resize; **Pad to Fill** (`padToAspect`) pads to the resize target, and its +block inside `RESIZE_STANDARD` only *records* the box (`_aspect_box_w/h`). + +- **Last, on purpose.** Grain after the bars would noise them; a format + conversion after them would resample their edge. Nothing may be added after + `BORDERS` that touches pixels. +- **Never call `AddBorders` without `color=`.** Its default is luma 0 — + below video black on a limited-range clip. `_border_color()` converts the + chosen RGB into the clip's own format via `resize` with the source's matrix and + range (`ColorMetadata::zimg_matrix` / `is_full_range`), so depth, range and + matrix come out right from one path. +- **Offsets stay on the chroma grid, and on the field grid if still + interlaced** (`{{BORDER_FIELD_ALIGN}}`: deinterlacing off + a detected field + order). A picture bigger than the canvas **raises** rather than being + cropped or left short — a wrong frame size is the one thing authoring can't + take. + +`integration_borders_test.dart` (heavy) checks bar levels at 8 and 10-bit, and +that the picture inside the bars is not rescaled. + ### Colour metadata: read it, carry it, re-stamp it Same shape as the SAR handling above, same fix site. Colour tags (`color_space`, diff --git a/README.md b/README.md index 15a9dfc..2b24582 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ Twenty-one filters, each switchable independently, applied in a fixed order. Mos | **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. | | **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. | +| **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. | diff --git a/app/assets/filters/core/crop_resize.json b/app/assets/filters/core/crop_resize.json index 6d71007..026d2da 100644 --- a/app/assets/filters/core/crop_resize.json +++ b/app/assets/filters/core/crop_resize.json @@ -1,10 +1,10 @@ { "$schema": "https://vapourbox.app/schemas/filter-v1.json", "id": "crop_resize", - "version": "1.2.0", + "version": "1.3.0", "name": "Crop & Resize", - "description": "Crop borders and resize or upscale video", - "longDescription": "Trims unwanted borders and changes the output resolution. Cropping happens first, then resizing.\n\nUse crop to cut the head-switching noise along the bottom of VHS captures and the black overscan edges of broadcast material — otherwise the encoder spends bitrate on them. Use resize for a target resolution, or the NNEDI3 upscaler for a much better 2x/4x enlargement than a plain kernel. Keep crop values even so they stay aligned with chroma subsampling.", + "description": "Crop borders, resize or upscale, and add bars to a target size", + "longDescription": "Trims unwanted borders, changes the output resolution, and adds bars out to a target frame size. Cropping happens first, then resizing, then the bars.\n\nUse crop to cut the head-switching noise along the bottom of VHS captures and the black overscan edges of broadcast material — otherwise the encoder spends bitrate on them. Use resize for a target resolution, or the NNEDI3 upscaler for a much better 2x/4x enlargement than a plain kernel. Keep crop values even so they stay aligned with chroma subsampling.\n\nUse Add Borders to bring a cropped picture back to an exact frame size for authoring (720x576 for PAL DVD, 720x480 for NTSC) without rescaling it: the picture is centred and the rest filled with bars.", "category": "transform", "icon": "crop", "order": 9, @@ -37,6 +37,11 @@ "customSar", "displayAspect", "padToAspect", + "padEnabled", + "padWidth", + "padHeight", + "padColor", + "padCustomColor", "useIntegerUpscale", "upscaleMethod", "upscaleFactor", @@ -609,11 +614,83 @@ "default": false, "ui": { "label": "Pad to Fill", - "description": "Letterbox or pillarbox out to the exact target size instead of leaving the picture smaller than it in one direction", + "description": "Letterbox or pillarbox out to the exact target size instead of leaving the picture smaller than it in one direction. The bars use the Border Colour below", "visibleWhen": { "resizeEnabled": true } } + }, + "padEnabled": { + "type": "boolean", + "default": false, + "sinceAppVersion": "1.2.0", + "ui": { + "label": "Add Borders", + "description": "Add bars around the picture to bring it to an exact frame size, without rescaling it. Runs after cropping and resizing, so crop off a dirty edge and border back out to the size you need" + } + }, + "padWidth": { + "type": "integer", + "default": 720, + "min": 64, + "max": 7680, + "step": 2, + "sinceAppVersion": "1.2.0", + "ui": { + "label": "Canvas Width", + "description": "Output frame width in pixels. Must be at least the picture's width after cropping and resizing", + "widget": "number", + "visibleWhen": { + "padEnabled": true + } + } + }, + "padHeight": { + "type": "integer", + "default": 576, + "min": 64, + "max": 4320, + "step": 2, + "sinceAppVersion": "1.2.0", + "ui": { + "label": "Canvas Height", + "description": "Output frame height in pixels. Must be at least the picture's height after cropping and resizing", + "widget": "number", + "visibleWhen": { + "padEnabled": true + } + } + }, + "padColor": { + "type": "enum", + "default": "black", + "options": [ + "black", + "grey", + "white", + "custom" + ], + "sinceAppVersion": "1.2.0", + "ui": { + "label": "Border Colour", + "description": "Colour of the bars, for Add Borders and Pad to Fill. Black is video black, not the below-black level bars used to get", + "widget": "dropdown" + } + }, + "padCustomColor": { + "type": "string", + "default": "#000000", + "sinceAppVersion": "1.2.0", + "ui": { + "label": "Custom Colour", + "description": "Bar colour as #RRGGBB (e.g. #202020 for dark grey). Anything else falls back to black", + "widget": "textfield", + "visibleWhen": { + "padColor": [ + "custom" + ] + } + } } }, "parameterPresets": { @@ -681,6 +758,36 @@ "targetHeight": 2160 } } + }, + "borderPreset": { + "label": "Canvas Preset", + "description": "Add bars to bring the picture to a standard frame size, without rescaling it", + "default": "None", + "options": { + "None": { + "padEnabled": false + }, + "PAL DVD (720x576)": { + "padEnabled": true, + "padWidth": 720, + "padHeight": 576 + }, + "NTSC DVD (720x480)": { + "padEnabled": true, + "padWidth": 720, + "padHeight": 480 + }, + "720p (1280x720)": { + "padEnabled": true, + "padWidth": 1280, + "padHeight": 720 + }, + "1080p (1920x1080)": { + "padEnabled": true, + "padWidth": 1920, + "padHeight": 1080 + } + } } }, "ui": { @@ -720,6 +827,17 @@ ], "expanded": true }, + { + "title": "Borders", + "parameters": [ + "padEnabled", + "padWidth", + "padHeight", + "padColor", + "padCustomColor" + ], + "expanded": true + }, { "title": "Upscale", "description": "Enlarge using an edge-directed interpolator rather than a plain resize. Doubles at a time, so pick the factor that reaches or exceeds your target and set a Resize below to land on it exactly.", @@ -791,6 +909,21 @@ "useIntegerUpscale": true, "upscaleMethod": "spline36" } + }, + { + "function": "core.std.AddBorders", + "role": "Pad to Fill's letterbox/pillarbox bars, in the border colour", + "activeWhen": { + "resizeEnabled": true, + "padToAspect": true + } + }, + { + "function": "core.std.AddBorders", + "role": "bars out to the canvas, in the border colour (last step, after the output format conversion)", + "activeWhen": { + "padEnabled": true + } } ] } diff --git a/app/lib/models/crop_resize_parameters.dart b/app/lib/models/crop_resize_parameters.dart index 1998ef6..8cf2bb5 100644 --- a/app/lib/models/crop_resize_parameters.dart +++ b/app/lib/models/crop_resize_parameters.dart @@ -51,6 +51,20 @@ enum PixelAspectMode { custom, } +/// Fill colour for letterbox/pillarbox bars (issue #86). +enum BorderColor { + @JsonValue('black') + black, + @JsonValue('grey') + grey, + @JsonValue('white') + white, + + /// An `#RRGGBB` value from [CropResizeParameters.padCustomColor]. + @JsonValue('custom') + custom, +} + /// Crop/resize preset options. enum CropResizePreset { @JsonValue('off') @@ -135,6 +149,23 @@ class CropResizeParameters { /// fitted image smaller than it in one axis. final bool padToAspect; + // --- Borders (issue #86) --- + + /// Add bars out to a fixed canvas size, without rescaling the picture. + final bool padEnabled; + + /// Canvas width. Null leaves the width at the picture's own. + final int? padWidth; + + /// Canvas height. Null leaves the height at the picture's own. + final int? padHeight; + + /// Fill colour for every bar this pass adds, Pad to Fill's included. + final BorderColor padColor; + + /// `#RRGGBB` used when [padColor] is [BorderColor.custom]. + final String? padCustomColor; + // --- Upscale Parameters (for integer scaling) --- /// Whether to use integer upscaling (2x, 4x) instead of arbitrary resize. @@ -204,6 +235,12 @@ class CropResizeParameters { this.customSar, this.displayAspect, this.padToAspect = false, + // Border defaults + this.padEnabled = false, + this.padWidth, + this.padHeight, + this.padColor = BorderColor.black, + this.padCustomColor, // Upscale defaults this.useIntegerUpscale = false, this.upscaleMethod = UpscaleMethod.nnedi3Rpow2, @@ -295,6 +332,11 @@ class CropResizeParameters { String? customSar, String? displayAspect, bool? padToAspect, + bool? padEnabled, + int? padWidth, + int? padHeight, + BorderColor? padColor, + String? padCustomColor, bool? useIntegerUpscale, UpscaleMethod? upscaleMethod, int? upscaleFactor, @@ -329,6 +371,11 @@ class CropResizeParameters { customSar: customSar ?? this.customSar, displayAspect: displayAspect ?? this.displayAspect, padToAspect: padToAspect ?? this.padToAspect, + padEnabled: padEnabled ?? this.padEnabled, + padWidth: padWidth ?? this.padWidth, + padHeight: padHeight ?? this.padHeight, + padColor: padColor ?? this.padColor, + padCustomColor: padCustomColor ?? this.padCustomColor, useIntegerUpscale: useIntegerUpscale ?? this.useIntegerUpscale, upscaleMethod: upscaleMethod ?? this.upscaleMethod, upscaleFactor: upscaleFactor ?? this.upscaleFactor, @@ -378,6 +425,9 @@ class CropResizeParameters { if (useIntegerUpscale) { parts.add('${upscaleFactor}x'); } + if (padEnabled && (padWidth != null || padHeight != null)) { + parts.add('Border ${padWidth ?? "?"}x${padHeight ?? "?"}'); + } if (pixelAspect == PixelAspectMode.square) parts.add('Square px'); if (displayAspect != null && displayAspect!.isNotEmpty) { parts.add('DAR $displayAspect'); diff --git a/app/lib/models/parameter_converter.dart b/app/lib/models/parameter_converter.dart index d23352c..188bee7 100644 --- a/app/lib/models/parameter_converter.dart +++ b/app/lib/models/parameter_converter.dart @@ -847,6 +847,13 @@ class ParameterConverter { 'maintainAspect': params.maintainAspect, 'pixelAspect': params.pixelAspect.name, 'padToAspect': params.padToAspect, + 'padEnabled': params.padEnabled, + // A canvas that was never set shows the PAL DVD size, as the resize + // target shows 1080p — so ticking Add Borders does something at once. + 'padWidth': params.padWidth ?? 720, + 'padHeight': params.padHeight ?? 576, + 'padColor': params.padColor.name, + 'padCustomColor': params.padCustomColor ?? '#000000', 'useIntegerUpscale': params.useIntegerUpscale, 'upscaleMethod': params.upscaleMethod.name, 'upscaleFactor': params.upscaleFactor, @@ -1443,6 +1450,14 @@ class ParameterConverter { customSar: v['customSar'] as String?, displayAspect: v['displayAspect'] as String?, padToAspect: v['padToAspect'] as bool? ?? false, + padEnabled: v['padEnabled'] as bool? ?? false, + padWidth: _asInt(v['padWidth']) ?? 720, + padHeight: _asInt(v['padHeight']) ?? 576, + padColor: BorderColor.values.firstWhere( + (c) => c.name == (v['padColor'] as String? ?? 'black'), + orElse: () => BorderColor.black, + ), + padCustomColor: v['padCustomColor'] as String?, useIntegerUpscale: v['useIntegerUpscale'] as bool? ?? false, upscaleMethod: UpscaleMethod.values.firstWhere( (m) => m.name.toLowerCase() == (v['upscaleMethod'] as String? ?? 'nnedi3Rpow2').toLowerCase(), diff --git a/app/test/integration_borders_test.dart b/app/test/integration_borders_test.dart new file mode 100644 index 0000000..0bc7f19 --- /dev/null +++ b/app/test/integration_borders_test.dart @@ -0,0 +1,269 @@ +/// Full-encode tests for Add Borders (issue #86). +/// +/// Two things only a real encode can show. The picture must be *bordered*, not +/// rescaled to fill the canvas, so the pixels inside the bars have to be the +/// source's own. And the bars must be the right level in the clip's own format: +/// Pad to Fill used to leave AddBorders at its default fill, luma 0, which is +/// below video black on a limited-range source. Both are checked at 8-bit and +/// 10-bit, since a level that is right at one depth and wrong at the other is +/// exactly the class of bug these tests exist for. +/// +/// Heavy (full-encode) — runs in the nightly workflow, not the push gate. +@Tags(['heavy']) +library; + +// ignore_for_file: avoid_print — these tests print diagnostics to the test log. + +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:uuid/uuid.dart'; + +import 'package:vapourbox/models/crop_resize_parameters.dart'; +import 'package:vapourbox/models/encoding_settings.dart'; +import 'package:vapourbox/models/processing_pipeline.dart'; +import 'package:vapourbox/models/qtgmc_parameters.dart'; +import 'package:vapourbox/models/video_job.dart'; + +import 'support/worker_harness.dart'; + +String get _outDir => '${WorkerHarness.outputDir}/borders'; + +const _w = 720; +const _h = 576; + +/// A flat mid-grey 720x576 source in [pixFmt], untagged. Flat so that "is this +/// pixel picture or bar" has one unambiguous answer anywhere in the frame. +Future _fixture(String pixFmt) async { + final path = '$_outDir/src_grey_$pixFmt.mkv'; + final result = await Process.run( + WorkerHarness.ffmpegPath, + [ + '-hide_banner', '-loglevel', 'error', '-y', + '-f', 'lavfi', + '-i', 'color=c=0x808080:size=${_w}x$_h:rate=25:duration=0.4', + '-c:v', 'ffv1', '-pix_fmt', pixFmt, + path, + ], + environment: WorkerHarness.ffmpegEnv, + ); + expect(result.exitCode, 0, reason: 'fixture: ${result.stderr}'); + return path; +} + +Map _job(String name, String input, String pixFmt, + CropResizeParameters cropResize) { + return VideoJob( + id: const Uuid().v4(), + inputPath: input, + outputPath: '$_outDir/$name.mkv', + processingPipeline: ProcessingPipeline( + deinterlace: const QTGMCParameters(enabled: false), + cropResize: cropResize, + ), + encodingSettings: const EncodingSettings( + codec: VideoCodec.ffv1, + container: ContainerFormat.mkv, + audioMode: AudioMode.none, + ), + inputWidth: _w, + inputHeight: _h, + inputPixelFormat: pixFmt, + inputFrameRate: 25.0, + startFrame: 0, + endFrame: 4, + ).toJson(); +} + +/// Crop 8px off every side, then border back out to the full 720x576. +CropResizeParameters _cropAndBorder({ + BorderColor color = BorderColor.black, + String? custom, +}) => + CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: 8, + cropRight: 8, + cropTop: 8, + cropBottom: 8, + padEnabled: true, + padWidth: _w, + padHeight: _h, + padColor: color, + padCustomColor: custom, + ); + +/// One decoded frame's planes, in the file's own format. +class _Planes { + final int width; + final int height; + final int bytesPerSample; + final int chromaShiftW; + final Uint8List raw; + + _Planes(this.width, this.height, this.bytesPerSample, this.chromaShiftW, this.raw); + + int _at(int offset) => bytesPerSample == 1 + ? raw[offset] + : raw[offset] | (raw[offset + 1] << 8); + + int y(int row, int col) => _at((row * width + col) * bytesPerSample); + + /// Chroma for 4:2:0 / 4:2:2, addressed in luma coordinates. + int u(int row, int col, {required bool verticallySubsampled}) { + final cw = width >> chromaShiftW; + final r = verticallySubsampled ? row >> 1 : row; + final base = width * height; + return _at((base + r * cw + (col >> chromaShiftW)) * bytesPerSample); + } + + int v(int row, int col, {required bool verticallySubsampled}) { + final cw = width >> chromaShiftW; + final ch = verticallySubsampled ? height >> 1 : height; + final r = verticallySubsampled ? row >> 1 : row; + final base = width * height + cw * ch; + return _at((base + r * cw + (col >> chromaShiftW)) * bytesPerSample); + } +} + +Future<_Planes> _firstFrame(String path, String pixFmt) async { + final stream = await WorkerHarness.firstStream(path, + selector: 'v:0', entries: ['width', 'height', 'pix_fmt']); + expect(stream, isNotNull); + expect(stream!['pix_fmt'], pixFmt, reason: 'output format must be the source\'s'); + final width = int.parse(stream['width'].toString()); + final height = int.parse(stream['height'].toString()); + + final raw = File('$path.frame0.raw'); + final r = await Process.run( + WorkerHarness.ffmpegPath, + ['-y', '-v', 'error', '-i', path, '-frames:v', '1', '-f', 'rawvideo', raw.path], + environment: WorkerHarness.ffmpegEnv, + ); + expect(r.exitCode, 0, reason: 'decoding $path: ${r.stderr}'); + final bytes = await raw.readAsBytes(); + await raw.delete(); + return _Planes(width, height, pixFmt.contains('10') ? 2 : 1, 1, bytes); +} + +Future _encode(String name, String input, String pixFmt, + CropResizeParameters params) async { + final result = await WorkerHarness.runJob(_job(name, input, pixFmt, params), label: name); + final tail = result.logs.length > 25 + ? result.logs.sublist(result.logs.length - 25) + : result.logs; + expect(result.success, isTrue, + reason: '${result.error}\n--- worker log (tail) ---\n${tail.join('\n')}'); + return result.outputPath!; +} + +void main() { + late String src8; + late String src10; + + setUpAll(() async { + await WorkerHarness.ensureReady(); + await Directory(_outDir).create(recursive: true); + src8 = await _fixture('yuv420p'); + src10 = await _fixture('yuv422p10le'); + }); + + group('Add Borders (full encode)', () { + // [depth label, fixture, pix_fmt, vertically subsampled, scale from 8-bit] + for (final (label, pixFmt, subV, scale) in [ + ('8-bit 4:2:0', 'yuv420p', true, 1), + ('10-bit 4:2:2', 'yuv422p10le', false, 4), + ]) { + test('$label: crop then border back to the canvas, unscaled, in video black', + () async { + final input = pixFmt == 'yuv420p' ? src8 : src10; + final out = await _encode('borders_black_$pixFmt', input, pixFmt, _cropAndBorder()); + final f = await _firstFrame(out, pixFmt); + + expect((f.width, f.height), (_w, _h), reason: 'output must be the canvas size'); + + // Video black, scaled to the depth: 16/128 at 8-bit, 64/512 at 10-bit. + // AddBorders' own default would have been luma 0. + for (final (row, col) in [(0, 0), (7, 360), (300, 7), (568, 360), (300, 712)]) { + expect(f.y(row, col), 16 * scale, reason: 'bar luma at ($row,$col)'); + expect(f.u(row, col, verticallySubsampled: subV), 128 * scale, + reason: 'bar Cb at ($row,$col)'); + expect(f.v(row, col, verticallySubsampled: subV), 128 * scale, + reason: 'bar Cr at ($row,$col)'); + } + + // The picture starts exactly 8px in and is the flat grey it was — a + // rescale to fill the canvas would have left no bar at all. + final grey = f.y(288, 360); + expect(grey, isNot(16 * scale)); + for (final (row, col) in [(8, 360), (300, 8), (567, 360), (300, 711)]) { + expect(f.y(row, col), grey, reason: 'picture luma at ($row,$col)'); + } + }, timeout: const Timeout(Duration(minutes: 5))); + } + + test('a custom colour is converted into the clip\'s format', () async { + // White at 10-bit limited is Y=940 — not 1023, and not 235. + final out = await _encode('borders_white_10bit', src10, 'yuv422p10le', + _cropAndBorder(color: BorderColor.custom, custom: '#FFFFFF')); + final f = await _firstFrame(out, 'yuv422p10le'); + expect(f.y(0, 0), 940); + expect(f.u(0, 0, verticallySubsampled: false), 512); + expect(f.v(0, 0, verticallySubsampled: false), 512); + }, timeout: const Timeout(Duration(minutes: 5))); + + test('Pad to Fill bars are video black too', () async { + // 720x576 fitted into a 1024x576 box leaves 152px pillars each side. + final out = await _encode( + 'borders_pad_to_fill', + src8, + 'yuv420p', + const CropResizeParameters( + enabled: true, + resizeEnabled: true, + targetWidth: 1024, + targetHeight: 576, + maintainAspect: true, + padToAspect: true, + ), + ); + final f = await _firstFrame(out, 'yuv420p'); + expect((f.width, f.height), (1024, 576)); + expect(f.y(288, 0), 16, reason: 'was 0 before issue #86'); + expect(f.y(288, 1023), 16); + expect(f.y(288, 512), isNot(16), reason: 'the picture itself'); + }, timeout: const Timeout(Duration(minutes: 5))); + + test('a picture bigger than the canvas fails with a clear message', () async { + final result = await WorkerHarness.runJob( + _job('borders_too_big', src8, 'yuv420p', const CropResizeParameters( + enabled: true, + padEnabled: true, + padWidth: 704, + padHeight: 576, + )), + label: 'borders_too_big', + ); + expect(result.success, isFalse, + reason: 'silently producing the wrong frame size is the failure to avoid'); + final all = '${result.error}\n${result.logs.join('\n')}'; + expect(all, contains('larger than the 704x576 canvas')); + }, timeout: const Timeout(Duration(minutes: 5))); + + test('the preview shows the bars too', () async { + // Preview and encode are separate scripts; assert both. + final preview = await WorkerHarness.runPreview( + _job('borders_preview', src8, 'yuv420p', _cropAndBorder()), + frame: 2); + expect(preview.success, isTrue, reason: preview.error); + final rgb = await WorkerHarness.imageToRgb24(preview.png!, label: 'borders'); + expect(rgb.length, _w * _h * 3, reason: 'preview must be the canvas size'); + // Video black decodes to RGB 0; the grey picture does not. + expect(rgb.sublist(0, 3), [0, 0, 0]); + final centre = (288 * _w + 360) * 3; + expect(rgb[centre], greaterThan(64)); + }, timeout: const Timeout(Duration(minutes: 5))); + }); +} diff --git a/app/test/integration_filter_parameters_test.dart b/app/test/integration_filter_parameters_test.dart index 6b841cb..daacc70 100644 --- a/app/test/integration_filter_parameters_test.dart +++ b/app/test/integration_filter_parameters_test.dart @@ -411,6 +411,40 @@ void main() { print(' PASS'); }, timeout: const Timeout(Duration(minutes: 2))); + // Issue #86: Add Borders. Starts from the panel's own dynamic values, so + // the schema keys, the converter and the Rust field names are all on the + // trip — a name mismatch anywhere and the canvas silently stays off. + test('crop_resize: Add Borders pads to the canvas in the chosen colour', () async { + final schema = loadSchema('crop_resize'); + for (final key in ['padEnabled', 'padWidth', 'padHeight', 'padColor', 'padCustomColor']) { + expect(schema.parameters.containsKey(key), isTrue, reason: '$key missing from schema'); + } + final dyn = ParameterConverter.fromCropResize(const CropResizeParameters(enabled: true)) + .withValues({ + 'cropEnabled': true, + 'cropLeft': 8, + 'cropRight': 8, + 'padEnabled': true, + 'padWidth': 720, + 'padHeight': 480, + 'padColor': 'custom', + 'padCustomColor': '#204060', + }); + final job = buildJob( + testName: 'add_borders', + cropResize: ParameterConverter.toCropResize(dyn), + ); + print(' Generating Add Borders script...'); + final script = await generateScriptViaWorker(job); + expect(script, contains('core.std.AddBorders(')); + expect(script, contains('color=list((32, 64, 96))')); + expect(script, contains('_canvas_w = 720')); + expect(script, contains('_canvas_h = 480')); + // A canvas, not a resize: the picture is never rescaled to fill it. + expect(script, isNot(contains('width=target_w'))); + print(' PASS'); + }, timeout: const Timeout(Duration(minutes: 2))); + // Issue #50: temperature/tint white balance. U carries blue-yellow and V // carries red-cyan, so warm is -U/+V; a sign error here is invisible until diff --git a/app/test/parameter_converter_test.dart b/app/test/parameter_converter_test.dart index 4332775..eaedfc0 100644 --- a/app/test/parameter_converter_test.dart +++ b/app/test/parameter_converter_test.dart @@ -346,6 +346,36 @@ void main() { expect(dynamic.values['upscaleMethod'], 'nnedi3Rpow2'); expect(dynamic.values['upscaleFactor'], 2); }); + + test('border parameters survive a round trip', () { + const params = CropResizeParameters( + enabled: true, + padEnabled: true, + padWidth: 720, + padHeight: 480, + padColor: BorderColor.custom, + padCustomColor: '#102030', + ); + final dynamic = ParameterConverter.fromCropResize(params); + expect(dynamic.values['padEnabled'], true); + expect(dynamic.values['padColor'], 'custom'); + + final back = ParameterConverter.toCropResize(dynamic); + expect(back.padEnabled, true); + expect(back.padWidth, 720); + expect(back.padHeight, 480); + expect(back.padColor, BorderColor.custom); + expect(back.padCustomColor, '#102030'); + }); + + test('an unset canvas shows a usable size, so ticking Add Borders works', () { + final dynamic = ParameterConverter.fromCropResize( + const CropResizeParameters(enabled: true)); + expect(dynamic.values['padEnabled'], false); + expect(dynamic.values['padWidth'], 720); + expect(dynamic.values['padHeight'], 576); + expect(dynamic.values['padColor'], 'black'); + }); }); group('fromPipeline', () { diff --git a/worker/src/models/color_metadata.rs b/worker/src/models/color_metadata.rs index a111ce9..165bcb0 100644 --- a/worker/src/models/color_metadata.rs +++ b/worker/src/models/color_metadata.rs @@ -107,6 +107,37 @@ impl ColorMetadata { args } + /// Whether the source declares full range. Anything else is limited, which + /// is what an untagged SD capture almost always is. + pub fn is_full_range(&self) -> bool { + matches!(self.range.as_deref(), Some("pc") | Some("jpeg") | Some("full")) + } + + /// The zimg `matrix_s` for converting an RGB colour into the source's YUV — + /// used for border fills (issue #86), the only place the script turns RGB + /// into YUV. Nothing re-matrixes the picture, so the samples still carry the + /// source's matrix; untagged, it is the usual guess by frame height. + /// + /// Neutral fills (black/grey/white) come out identical under every matrix, + /// so this only matters for a custom colour. + pub fn zimg_matrix(&self, source_height: Option) -> &'static str { + match self.matrix.as_deref() { + Some("bt709") => "709", + Some("fcc") => "fcc", + Some("bt470bg") => "470bg", + Some("smpte170m") => "170m", + Some("smpte240m") => "240m", + Some("ycgco") => "ycgco", + Some("bt2020nc") => "2020ncl", + Some("bt2020c") => "2020cl", + Some("chroma-derived-nc") => "chromancl", + Some("chroma-derived-c") => "chromacl", + Some("ictcp") => "ictcp", + _ if source_height.is_some_and(|h| h >= 720) => "709", + _ => "170m", + } + } + /// swscale input options for the preview's YUV→RGB conversion. /// /// The preview's second stage reads a Y4M pipe, which carries no colour @@ -128,7 +159,7 @@ impl ColorMetadata { } // Anything but an explicit full-range tag is treated as limited, which // is what an untagged SD capture almost always is. - let full = matches!(self.range.as_deref(), Some("pc") | Some("jpeg") | Some("full")); + let full = self.is_full_range(); opts.push(format!("in_range={}", if full { "pc" } else { "tv" })); opts } @@ -202,6 +233,34 @@ mod tests { ); } + #[test] + fn zimg_matrix_follows_the_tag_then_the_frame_height() { + let tagged = ColorMetadata::from_raw(Some("bt470bg"), None, None, None); + assert_eq!(tagged.zimg_matrix(Some(1080)), "470bg"); + let untagged = ColorMetadata::default(); + assert_eq!(untagged.zimg_matrix(Some(1080)), "709"); + assert_eq!(untagged.zimg_matrix(Some(576)), "170m"); + assert_eq!(untagged.zimg_matrix(None), "170m"); + // The identity matrix is RGB; guess as if untagged rather than feed + // zimg an RGB->RGB matrix for a YUV clip. + let rgb = ColorMetadata::from_raw(Some("rgb"), None, None, None); + assert_eq!(rgb.zimg_matrix(Some(480)), "170m"); + } + + #[test] + fn every_recognised_matrix_maps_to_a_zimg_name() { + // Only the RGB identity, and SMPTE 2085 (which zimg does not implement), + // may fall through to the height guess. + for m in MATRICES.iter().filter(|m| !matches!(**m, "rgb" | "smpte2085")) { + let c = ColorMetadata::from_raw(Some(m), None, None, None); + assert_ne!( + (c.zimg_matrix(Some(1080)), c.zimg_matrix(Some(480))), + ("709", "170m"), + "{m} is not mapped" + ); + } + } + #[test] fn preview_drops_the_rgb_identity_matrix() { // swscale has no in_color_matrix entry for it. diff --git a/worker/src/models/crop_resize_parameters.rs b/worker/src/models/crop_resize_parameters.rs index f1f4116..5478bf5 100644 --- a/worker/src/models/crop_resize_parameters.rs +++ b/worker/src/models/crop_resize_parameters.rs @@ -85,6 +85,30 @@ pub enum PixelAspectMode { Custom, } +/// Fill colour for letterbox/pillarbox bars (issue #86). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub enum BorderColor { + #[default] + Black, + Grey, + White, + /// An `#RRGGBB` value from `pad_custom_color`. + Custom, +} + +/// Parse `#RRGGBB` (the `#` optional) into 8-bit RGB. `None` for anything else, +/// so a typo falls back to black rather than reaching the script. +pub fn parse_hex_color(text: &str) -> Option<[u8; 3]> { + let hex = text.trim(); + let hex = hex.strip_prefix('#').unwrap_or(hex); + if hex.len() != 6 || !hex.is_ascii() { + return None; + } + let channel = |i: usize| u8::from_str_radix(&hex[i..i + 2], 16).ok(); + Some([channel(0)?, channel(2)?, channel(4)?]) +} + /// Crop/resize preset options. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "camelCase")] @@ -186,6 +210,28 @@ pub struct CropResizeParameters { #[serde(default)] pub pad_to_aspect: bool, + // --- Borders (issue #86) --- + + /// Add bars out to a fixed canvas size, without rescaling the picture. + #[serde(default)] + pub pad_enabled: bool, + + /// Canvas width. `None` leaves the width at the picture's own. + #[serde(default)] + pub pad_width: Option, + + /// Canvas height. `None` leaves the height at the picture's own. + #[serde(default)] + pub pad_height: Option, + + /// Fill colour for every bar this pass adds, Pad to Fill's included. + #[serde(default)] + pub pad_color: BorderColor, + + /// `#RRGGBB` used when `pad_color` is `Custom`. + #[serde(default)] + pub pad_custom_color: Option, + // --- Upscale Parameters (for integer scaling) --- /// Whether to use integer upscaling (2x, 4x) instead of arbitrary resize. @@ -267,6 +313,11 @@ impl Default for CropResizeParameters { custom_sar: None, display_aspect: None, pad_to_aspect: false, + pad_enabled: false, + pad_width: None, + pad_height: None, + pad_color: BorderColor::default(), + pad_custom_color: None, bicubic_b: None, bicubic_c: None, lanczos_taps: None, @@ -383,6 +434,45 @@ impl CropResizeParameters { } } + /// Whether Pad to Fill applies: it pads out to the resize target, so there + /// is nothing to pad to without one. + pub fn pads_to_aspect(&self) -> bool { + self.enabled && self.resize_enabled && self.pad_to_aspect + } + + /// The fixed canvas to pad out to, as (width, height) with `-1` for an axis + /// left at the picture's own size — or `None` when not padding to a canvas. + /// + /// Odd sizes are rounded up to even: every subsampled format needs it, and + /// rounding up means the picture still fits. + pub fn pad_canvas(&self) -> Option<(i32, i32)> { + if !(self.enabled && self.pad_enabled) { + return None; + } + let axis = |v: Option| v.filter(|&v| v > 0).map_or(-1, |v| (v + 1) & !1); + let (w, h) = (axis(self.pad_width), axis(self.pad_height)); + (w > 0 || h > 0).then_some((w, h)) + } + + /// Whether any bars are added at all. + pub fn adds_borders(&self) -> bool { + self.pads_to_aspect() || self.pad_canvas().is_some() + } + + /// The bar colour as 8-bit RGB. An unparseable custom colour is black. + pub fn border_rgb(&self) -> [u8; 3] { + match self.pad_color { + BorderColor::Black => [0, 0, 0], + BorderColor::Grey => [128, 128, 128], + BorderColor::White => [255, 255, 255], + BorderColor::Custom => self + .pad_custom_color + .as_deref() + .and_then(parse_hex_color) + .unwrap_or([0, 0, 0]), + } + } + /// Get total horizontal crop. pub fn total_horizontal_crop(&self) -> i32 { self.crop_left + self.crop_right @@ -512,6 +602,66 @@ mod tests { assert_eq!(params.aspect_declaration(Some("10:11")), AspectDeclaration::None); } + #[test] + fn test_parse_hex_color() { + assert_eq!(parse_hex_color("#FF8000"), Some([255, 128, 0])); + assert_eq!(parse_hex_color(" 0a0B0c "), Some([10, 11, 12])); + for junk in ["", "#FFF", "#GGGGGG", "#FF80001", "red", "#ÿÿÿ"] { + assert_eq!(parse_hex_color(junk), None, "{junk:?}"); + } + } + + #[test] + fn test_border_rgb_falls_back_to_black() { + let mut params = CropResizeParameters::default(); + assert_eq!(params.border_rgb(), [0, 0, 0]); + params.pad_color = BorderColor::White; + assert_eq!(params.border_rgb(), [255, 255, 255]); + params.pad_color = BorderColor::Custom; + params.pad_custom_color = Some("#102030".to_string()); + assert_eq!(params.border_rgb(), [16, 32, 48]); + params.pad_custom_color = Some("blue".to_string()); + assert_eq!(params.border_rgb(), [0, 0, 0]); + } + + #[test] + fn test_pad_canvas() { + let params = CropResizeParameters { + enabled: true, + pad_enabled: true, + pad_width: Some(720), + pad_height: Some(575), + ..CropResizeParameters::default() + }; + // Odd rounds up, so the picture still fits. + assert_eq!(params.pad_canvas(), Some((720, 576))); + assert!(params.adds_borders()); + + // One axis only: the other stays at the picture's own size. + let one_axis = CropResizeParameters { pad_width: None, ..params.clone() }; + assert_eq!(one_axis.pad_canvas(), Some((-1, 576))); + + // No size at all, the section off, or the pass off: no canvas. + let no_size = CropResizeParameters { pad_width: None, pad_height: Some(0), ..params.clone() }; + assert_eq!(no_size.pad_canvas(), None); + let off = CropResizeParameters { pad_enabled: false, ..params.clone() }; + assert_eq!(off.pad_canvas(), None); + let pass_off = CropResizeParameters { enabled: false, ..params }; + assert_eq!(pass_off.pad_canvas(), None); + assert!(!pass_off.adds_borders()); + } + + #[test] + fn test_pad_to_aspect_needs_a_resize_target() { + let params = CropResizeParameters { + enabled: true, + pad_to_aspect: true, + ..CropResizeParameters::default() + }; + assert!(!params.pads_to_aspect()); + assert!(CropResizeParameters { resize_enabled: true, ..params }.pads_to_aspect()); + } + #[test] fn test_serialization() { let params = CropResizeParameters::default(); diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index 3bcbcea..87bd4b9 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -2031,7 +2031,8 @@ impl ScriptGenerator { } // Padding only means anything when there is a box to pad out to. - if resize.pad_to_aspect && resize.resize_enabled { + // This only records the box; the BORDERS step adds the bars. + if resize.pads_to_aspect() { script = script.replace("{{#ASPECT_PAD}}", ""); script = script.replace("{{/ASPECT_PAD}}", ""); } else { @@ -2080,6 +2081,57 @@ impl ScriptGenerator { } } + // ==================================================================== + // BORDERS (issue #86) + // ==================================================================== + // Last in the script, after the output format conversion, so the bars + // are exact values in the encoded format. Pad to Fill's box was + // recorded by the resize step; the fixed canvas is independent of it. + if resize.adds_borders() { + script = script.replace("{{#BORDERS}}", ""); + script = script.replace("{{/BORDERS}}", ""); + + let [r, g, b] = resize.border_rgb(); + script = script.replace("{{BORDER_RGB}}", &format!("({r}, {g}, {b})")); + let color = job.color_metadata(); + script = script.replace("{{BORDER_MATRIX}}", color.zimg_matrix(job.input_height)); + script = script.replace( + "{{BORDER_RANGE}}", + if color.is_full_range() { "full" } else { "limited" }, + ); + + // A picture that is still interlaced must move down by whole field + // pairs. Conservative: it is only off-centre by a line or two if + // something else (IVTC) already made it progressive. + let still_interlaced = !pipeline.deinterlace.enabled + && Self::field_based_for(job, pipeline).is_some(); + script = script.replace( + "{{BORDER_FIELD_ALIGN}}", + if still_interlaced { "2" } else { "1" }, + ); + + if resize.pads_to_aspect() { + script = script.replace("{{#BORDER_ASPECT_BOX}}", ""); + script = script.replace("{{/BORDER_ASPECT_BOX}}", ""); + } else { + script = remove_block("{{#BORDER_ASPECT_BOX}}", "{{/BORDER_ASPECT_BOX}}", script); + } + + match resize.pad_canvas() { + Some((w, h)) => { + script = script.replace("{{#BORDER_CANVAS}}", ""); + script = script.replace("{{/BORDER_CANVAS}}", ""); + script = script.replace("{{PAD_WIDTH}}", &w.to_string()); + script = script.replace("{{PAD_HEIGHT}}", &h.to_string()); + } + None => { + script = remove_block("{{#BORDER_CANVAS}}", "{{/BORDER_CANVAS}}", script); + } + } + } else { + script = remove_block("{{#BORDERS}}", "{{/BORDERS}}", script); + } + script } } diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index 17a2b0d..0e02935 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -1955,20 +1955,11 @@ clip = core.resize.{{RESIZE_KERNEL}}( ) {{#ASPECT_PAD}} -# Letterbox/pillarbox out to the requested box instead of leaving the fitted -# image smaller than it. Borders are split evenly and kept even for chroma. -if _box_w > clip.width or _box_h > clip.height: - _pad_w = max(0, _even(_box_w) - clip.width) - _pad_h = max(0, _even(_box_h) - clip.height) - _left = (_pad_w // 2) & ~1 - _top = (_pad_h // 2) & ~1 - clip = core.std.AddBorders( - clip, - left=_left, - right=_pad_w - _left, - top=_top, - bottom=_pad_h - _top, - ) +# Pad to Fill: remember the box, so the BORDERS step at the very end can +# letterbox/pillarbox out to it. The bars go on last so that nothing after this +# (grain in particular) touches them. An axis with no target stays as it is. +_aspect_box_w = _even(_box_w) if _box_w > 0 else 0 +_aspect_box_h = _even(_box_h) if _box_h > 0 else 0 {{/ASPECT_PAD}} {{/RESIZE_STANDARD}} {{/RESIZE}} @@ -2109,6 +2100,86 @@ if clip.format.id != target_format: dither_type="error_diffusion") {{/CHROMA_CONVERT}} +# ============================================================================ +# BORDERS (issue #86) +# +# Letterbox/pillarbox bars: Pad to Fill's box from the resize step, then Add +# Borders' fixed canvas. Deliberately the very last step — after grain (which +# would put noise on the bars), after custom code, and after the output format +# conversion, so the bars are exact values in the format actually encoded +# rather than resampled along with the picture. Identical in both templates. +# ============================================================================ +{{#BORDERS}} +def _border_color(c): + # The fill is chosen as RGB and converted into the clip's own format by the + # same resizer as everything else, so bit depth, range and matrix all come + # out right from one path. AddBorders' own default is luma 0: below video + # black on a limited-range clip, which legalisers and authoring tools flag. + fmt = c.format + swatch = core.std.BlankClip( + width=4 << fmt.subsampling_w, + height=4 << fmt.subsampling_h, + format=vs.RGB24, + color=list({{BORDER_RGB}}), + length=1, + ) + if fmt.color_family == vs.RGB: + swatch = core.resize.Point(swatch, format=fmt.id) + else: + swatch = core.resize.Point(swatch, format=fmt.id, matrix_s="{{BORDER_MATRIX}}", range_s="{{BORDER_RANGE}}") + frame = swatch.get_frame(0) + return [frame[p][0, 0] for p in range(fmt.num_planes)] + + +def _pad_to(c, canvas_w, canvas_h): + pad_w = canvas_w - c.width + pad_h = canvas_h - c.height + if pad_w < 0 or pad_h < 0: + raise vs.Error( + f"Add Borders: the picture is {c.width}x{c.height} after cropping " + f"and resizing, larger than the {canvas_w}x{canvas_h} canvas. Crop " + "more, resize smaller, or make the canvas bigger." + ) + if pad_w == 0 and pad_h == 0: + return c + fmt = c.format + step_w = 1 << fmt.subsampling_w + step_h = 1 << fmt.subsampling_h + if pad_w % step_w or pad_h % step_h: + raise vs.Error( + f"Add Borders: a {canvas_w}x{canvas_h} canvas cannot hold this " + f"{fmt.name} picture - the canvas must be a multiple of " + f"{step_w}x{step_h} for its chroma." + ) + # Centred, each offset on the chroma grid. A still-interlaced picture also + # moves down by a whole number of field pairs (of luma and of chroma rows), + # or its fields swap over. + align_h = step_h * {{BORDER_FIELD_ALIGN}} + left = (pad_w // 2) // step_w * step_w + top = (pad_h // 2) // align_h * align_h + return core.std.AddBorders( + c, + left=left, + right=pad_w - left, + top=top, + bottom=pad_h - top, + color=_border_color(c), + ) + + +{{#BORDER_ASPECT_BOX}} +# Pad to Fill: out to the resize target, never smaller than the picture. +clip = _pad_to(clip, max(_aspect_box_w, clip.width), max(_aspect_box_h, clip.height)) +{{/BORDER_ASPECT_BOX}} +{{#BORDER_CANVAS}} +# Add Borders: a fixed canvas, the picture centred and never rescaled. An axis +# given as -1 stays at the picture's own size. +_canvas_w = {{PAD_WIDTH}} +_canvas_h = {{PAD_HEIGHT}} +clip = _pad_to(clip, _canvas_w if _canvas_w > 0 else clip.width, _canvas_h if _canvas_h > 0 else clip.height) +{{/BORDER_CANVAS}} +{{/BORDERS}} + # ============================================================================ # OUTPUT # ============================================================================ diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 2642ab7..5866a36 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -1883,20 +1883,11 @@ clip = core.resize.{{RESIZE_KERNEL}}( ) {{#ASPECT_PAD}} -# Letterbox/pillarbox out to the requested box instead of leaving the fitted -# image smaller than it. Borders are split evenly and kept even for chroma. -if _box_w > clip.width or _box_h > clip.height: - _pad_w = max(0, _even(_box_w) - clip.width) - _pad_h = max(0, _even(_box_h) - clip.height) - _left = (_pad_w // 2) & ~1 - _top = (_pad_h // 2) & ~1 - clip = core.std.AddBorders( - clip, - left=_left, - right=_pad_w - _left, - top=_top, - bottom=_pad_h - _top, - ) +# Pad to Fill: remember the box, so the BORDERS step at the very end can +# letterbox/pillarbox out to it. The bars go on last so that nothing after this +# (grain in particular) touches them. An axis with no target stays as it is. +_aspect_box_w = _even(_box_w) if _box_w > 0 else 0 +_aspect_box_h = _even(_box_h) if _box_h > 0 else 0 {{/ASPECT_PAD}} {{/RESIZE_STANDARD}} {{/RESIZE}} @@ -2034,6 +2025,86 @@ if clip.format.id != target_format: dither_type="error_diffusion") {{/CHROMA_CONVERT}} +# ============================================================================ +# BORDERS (issue #86) +# +# Letterbox/pillarbox bars: Pad to Fill's box from the resize step, then Add +# Borders' fixed canvas. Deliberately the very last step — after grain (which +# would put noise on the bars), after custom code, and after the output format +# conversion, so the bars are exact values in the format actually encoded +# rather than resampled along with the picture. Identical in both templates. +# ============================================================================ +{{#BORDERS}} +def _border_color(c): + # The fill is chosen as RGB and converted into the clip's own format by the + # same resizer as everything else, so bit depth, range and matrix all come + # out right from one path. AddBorders' own default is luma 0: below video + # black on a limited-range clip, which legalisers and authoring tools flag. + fmt = c.format + swatch = core.std.BlankClip( + width=4 << fmt.subsampling_w, + height=4 << fmt.subsampling_h, + format=vs.RGB24, + color=list({{BORDER_RGB}}), + length=1, + ) + if fmt.color_family == vs.RGB: + swatch = core.resize.Point(swatch, format=fmt.id) + else: + swatch = core.resize.Point(swatch, format=fmt.id, matrix_s="{{BORDER_MATRIX}}", range_s="{{BORDER_RANGE}}") + frame = swatch.get_frame(0) + return [frame[p][0, 0] for p in range(fmt.num_planes)] + + +def _pad_to(c, canvas_w, canvas_h): + pad_w = canvas_w - c.width + pad_h = canvas_h - c.height + if pad_w < 0 or pad_h < 0: + raise vs.Error( + f"Add Borders: the picture is {c.width}x{c.height} after cropping " + f"and resizing, larger than the {canvas_w}x{canvas_h} canvas. Crop " + "more, resize smaller, or make the canvas bigger." + ) + if pad_w == 0 and pad_h == 0: + return c + fmt = c.format + step_w = 1 << fmt.subsampling_w + step_h = 1 << fmt.subsampling_h + if pad_w % step_w or pad_h % step_h: + raise vs.Error( + f"Add Borders: a {canvas_w}x{canvas_h} canvas cannot hold this " + f"{fmt.name} picture - the canvas must be a multiple of " + f"{step_w}x{step_h} for its chroma." + ) + # Centred, each offset on the chroma grid. A still-interlaced picture also + # moves down by a whole number of field pairs (of luma and of chroma rows), + # or its fields swap over. + align_h = step_h * {{BORDER_FIELD_ALIGN}} + left = (pad_w // 2) // step_w * step_w + top = (pad_h // 2) // align_h * align_h + return core.std.AddBorders( + c, + left=left, + right=pad_w - left, + top=top, + bottom=pad_h - top, + color=_border_color(c), + ) + + +{{#BORDER_ASPECT_BOX}} +# Pad to Fill: out to the resize target, never smaller than the picture. +clip = _pad_to(clip, max(_aspect_box_w, clip.width), max(_aspect_box_h, clip.height)) +{{/BORDER_ASPECT_BOX}} +{{#BORDER_CANVAS}} +# Add Borders: a fixed canvas, the picture centred and never rescaled. An axis +# given as -1 stays at the picture's own size. +_canvas_w = {{PAD_WIDTH}} +_canvas_h = {{PAD_HEIGHT}} +clip = _pad_to(clip, _canvas_w if _canvas_w > 0 else clip.width, _canvas_h if _canvas_h > 0 else clip.height) +{{/BORDER_CANVAS}} +{{/BORDERS}} + # ============================================================================ # OUTPUT - select the exact target frame for preview. # The worker decodes a window of source frames around the requested frame and diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index cec5d92..25de7da 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -5786,3 +5786,176 @@ fn test_155_a_bundle_without_the_split_still_autoloads_zsmooth() { } } } + +/// A crop-then-border job: 8px off each side, bordered back to 720x576. +fn borders_job(output_name: &str, crop_resize: CropResizeParameters) -> VideoJob { + let mut job = create_base_job(output_name); + job.input_width = Some(720); + job.input_height = Some(576); + job.processing_pipeline = Some(ProcessingPipeline { + deinterlace: QTGMCParameters { enabled: false, ..QTGMCParameters::default() }, + crop_resize, + ..ProcessingPipeline::default() + }); + job +} + +/// The template's header comment mentions `{{...}}`, so look for this +/// feature's own tags rather than any brace pair. +fn assert_no_border_tags(name: &str, script: &str) { + for tag in ["{{#BORDER", "{{/BORDER", "{{BORDER_", "{{PAD_", "{{#ASPECT_PAD", "{{/ASPECT_PAD"] { + assert!(!script.contains(tag), "{name} script left {tag} unsubstituted"); + } +} + +fn canvas_720x576() -> CropResizeParameters { + CropResizeParameters { + enabled: true, + crop_enabled: true, + crop_left: 8, + crop_right: 8, + crop_top: 8, + crop_bottom: 8, + pad_enabled: true, + pad_width: Some(720), + pad_height: Some(576), + ..CropResizeParameters::default() + } +} + +#[test] +fn test_156_add_borders_to_a_fixed_canvas() { + // Issue #86: crop a dirty edge, then border back out to the authoring + // size — with no resize, so the picture is never rescaled to fill the box. + create_output_dir(); + let job = borders_job( + "test_156_add_borders", + CropResizeParameters { + pad_color: BorderColor::Custom, + pad_custom_color: Some("#FF8000".to_string()), + ..canvas_720x576() + }, + ); + run_job_and_verify(&job, "Add Borders", &[ + "core.std.AddBorders(", + "color=_border_color(c)", + "color=list((255, 128, 0))", + "_canvas_w = 720", + "_canvas_h = 576", + ]) + .unwrap(); + + let (encode, preview) = generate_both_scripts(&job); + for (name, script) in [("encode", &encode), ("preview", &preview)] { + assert!(script.contains("_pad_to(clip, _canvas_w"), "{name} script must pad to the canvas"); + assert!(!script.contains("width=target_w"), + "{name} script must not resize: the picture is bordered, not rescaled"); + assert!(!script.contains("_aspect_box_w, clip.width"), "{name}: Pad to Fill is off"); + assert_no_border_tags(name, script); + } +} + +#[test] +fn test_157_borders_are_the_last_thing_the_script_does() { + // Grain after the bars would noise them, and the output format conversion + // after them would resample their edge. They go on after both. + create_output_dir(); + let mut job = borders_job("test_157_borders_last", canvas_720x576()); + job.encoding_settings.chroma_subsampling = ChromaSubsampling::Yuv420; + if let Some(p) = job.processing_pipeline.as_mut() { + p.grain = GrainParameters { enabled: true, ..GrainParameters::default() }; + } + + let (encode, preview) = generate_both_scripts(&job); + for (name, script) in [("encode", &encode), ("preview", &preview)] { + let pad = script.find("clip = _pad_to(clip").expect("pads"); + let convert = script.find("target_format = ").expect("converts"); + let grain = script.find("core.grain.Add(").expect("grains"); + assert!(pad > convert, "{name}: bars must go on after the format conversion"); + assert!(pad > grain, "{name}: bars must go on after grain"); + } +} + +#[test] +fn test_158_border_colour_follows_the_source_matrix_and_range() { + create_output_dir(); + // Untagged SD: 601 limited. + let job = borders_job("test_158_sd", canvas_720x576()); + let (encode, preview) = generate_both_scripts(&job); + for script in [&encode, &preview] { + assert!(script.contains(r#"matrix_s="170m", range_s="limited""#)); + assert!(script.contains("color=list((0, 0, 0))"), "black by default"); + } + + // A tagged full-range 709 source. + let mut job = borders_job("test_158_hd", canvas_720x576()); + job.input_color_matrix = Some("bt709".to_string()); + job.input_color_range = Some("pc".to_string()); + let (encode, preview) = generate_both_scripts(&job); + for script in [&encode, &preview] { + assert!(script.contains(r#"matrix_s="709", range_s="full""#)); + } +} + +#[test] +fn test_159_borders_keep_an_interlaced_picture_field_aligned() { + // Not deinterlacing a TFF source: the picture must move down by whole + // field pairs, or its fields swap. + create_output_dir(); + let job = borders_job("test_159_interlaced", canvas_720x576()); + let (encode, preview) = generate_both_scripts(&job); + for script in [&encode, &preview] { + assert!(script.contains("align_h = step_h * 2")); + } + + // Deinterlaced, it is progressive by the time the bars go on. + let mut job = borders_job("test_159_progressive", canvas_720x576()); + if let Some(p) = job.processing_pipeline.as_mut() { + p.deinterlace = QTGMCParameters { enabled: true, ..QTGMCParameters::default() }; + } + let (encode, preview) = generate_both_scripts(&job); + for script in [&encode, &preview] { + assert!(script.contains("align_h = step_h * 1")); + } +} + +#[test] +fn test_160_pad_to_fill_uses_the_border_step_and_its_colour() { + // Pad to Fill used to call AddBorders with no colour — luma 0, below video + // black. It now records its box and shares the BORDERS step. + create_output_dir(); + let job = borders_job( + "test_160_pad_to_fill", + CropResizeParameters { + enabled: true, + resize_enabled: true, + target_width: Some(1920), + target_height: Some(1080), + pad_to_aspect: true, + pad_color: BorderColor::White, + ..CropResizeParameters::default() + }, + ); + let (encode, preview) = generate_both_scripts(&job); + for (name, script) in [("encode", &encode), ("preview", &preview)] { + assert!(script.contains("_aspect_box_w = _even(_box_w)"), "{name}"); + assert!(script.contains("max(_aspect_box_w, clip.width)"), "{name}"); + assert!(script.contains("color=list((255, 255, 255))"), "{name}"); + assert!(!script.contains("_canvas_w = "), "{name}: no fixed canvas was asked for"); + assert_eq!(script.matches("core.std.AddBorders(").count(), 1, "{name}: one padding site"); + assert_no_border_tags(name, script); + } + + // Nothing asked for: no border code at all. + let job = borders_job("test_160_none", CropResizeParameters { + enabled: true, + crop_enabled: true, + crop_left: 8, + ..CropResizeParameters::default() + }); + let (encode, preview) = generate_both_scripts(&job); + for script in [&encode, &preview] { + assert!(!script.contains("_pad_to(")); + assert_no_border_tags("either", script); + } +}