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); + } +}