Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -939,14 +939,21 @@ mirrors the script's fitting). Rules:
samples were not gamut-converted. A gamut conversion moved a 75% green by
55 levels on a player that does not colour-manage
(docs/reports/2026-11-crop-detect-709).
- **Always needs the pass on, not its geometry.** With Cropping and Resize
both off, an enabled Crop & Resize still converts, at the source's size
(choosing Always in the panel switches the pass on). So `enabled_passes()` /
`enabledPasses` list Crop & Resize whenever it is on: the app's subtitle-only
test (`ProcessingPipeline.isSubtitleOnly`) reads that list, and a pass
missing from it is skipped with the whole encode. `test_219`.
- **R78's resizers let frame properties override `*_in_s` arguments**, so the
`{{#COLOR_709}}` block (both templates, right after `{{/RESIZE}}`) removes
`_Matrix`/`_Range` first. Any new block that converts colour with explicit
`*_in_s` arguments needs the same.
- `integration_bt709_conversion_test.dart` (heavy) checks the pixels: every
patch within 3 levels (measured ≤ 0.67) of the source decoded as 601,
through the tags and when read as 709 regardless, preview = encode, and Off
stays 601 and bit-identical in YUV.
stays 601 and bit-identical in YUV; with no crop or resize, Always converts
at SD size and Auto / Off are byte-identical to the source.

### HDR Sources and Dolby Vision (issue #133)

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Most sources need only a few of these, and the presets pick them for you. Filter
| 16 | **Color Correction** | Brightness, contrast, saturation, levels, white balance, shadow detail and local contrast, set by hand or measured |
| 17 | **Stabilize** | Shake and weave from telecine, film scans or a handheld camera, and the frame-height "breathing" of pull-through film scanners (8mm, Super 8): measured from the footage, or read per frame from a factor file (`reel.pitch.txt` next to `reel.mp4` is picked up automatically) |
| 18 | **Rotate / Flip** | Sideways, upside-down or mirrored footage |
| 19 | **Crop & Resize** | Black borders, tape blanking and head-switching noise (measured for you by **Detect borders**), scaling and upscaling (edge-directed or AI) with SD colour converted to HD's BT.709, and borders to reach an exact frame size |
| 19 | **Crop & Resize** | Black borders, tape blanking and head-switching noise (measured for you by **Detect borders**), scaling and upscaling (edge-directed or AI) with SD colour converted to BT.709 (for HD, or always), and borders to reach an exact frame size |
| 20 | **Face Restoration** | Blurry, low-resolution faces, redrawn by AI at the output resolution (optional download) |
| 21 | **Sharpen** | Soft pictures, sharpened after the resize so the result isn't softened again |
| 22 | **Film Grain** | Grain added back after denoising, so the picture doesn't look plastic |
Expand Down
4 changes: 2 additions & 2 deletions app/assets/filters/core/crop_resize.json
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@
"on": "Always",
"off": "Never"
},
"description": "An SD video's colour is BT.601. Upscaled to HD it is still tagged 601, which is correct, but many players and TVs ignore the tag on an HD picture and decode it as BT.709, which shifts greens and reds. Auto converts the colour to BT.709 when the output is at least 720 lines (or 1280 wide), so every player shows it the same; Always converts at any size. The source's primaries are kept and tagged. Never leaves the colour and its tags as they are."
"description": "An SD video's colour is BT.601. Upscaled to HD it is still tagged 601, which is correct, but many players and TVs ignore the tag on an HD picture and decode it as BT.709, which shifts greens and reds. Auto converts the colour to BT.709 when the output is at least 720 lines (or 1280 wide), so every player shows it the same. Always converts at any size, with or without a crop or resize: choosing it switches this pass on, and it converts even with Cropping and Resize both off (switching the pass off switches the conversion off too). The source's primaries are kept and tagged. Never leaves the colour and its tags as they are."
}
},
"useIntegerUpscale": {
Expand Down Expand Up @@ -1114,7 +1114,7 @@
},
{
"function": "core.resize.Spline36",
"role": "BT.601 to BT.709 colour (the matrix only), after the resize, when the output is HD",
"role": "BT.601 to BT.709 colour (the matrix only), after any resize: under Auto when the output is HD, under Always at any size",
"activeWhen": {
"convertToBt709": [
"auto",
Expand Down
6 changes: 5 additions & 1 deletion app/lib/models/crop_resize_parameters.dart
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,11 @@ class CropResizeParameters {
if (displayAspect != null && displayAspect!.isNotEmpty) {
parts.add('DAR $displayAspect');
}
return parts.isEmpty ? 'Custom' : parts.join(' ');
if (parts.isEmpty) {
// Converting colour is all the pass does (it needs no crop or resize).
return convertToBt709 == Bt709Conversion.on ? 'BT.709' : 'Custom';
}
return parts.join(' ');
}

factory CropResizeParameters.fromJson(Map<String, dynamic> json) =>
Expand Down
15 changes: 12 additions & 3 deletions app/lib/models/processing_pipeline.dart
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,11 @@ class ProcessingPipeline {
}
// Crop and resize are one pass and run together, crop first. The crop used
// to run ahead of everything else, which meant it could not remove the
// edges Stabilize exposes (issue #109).
if (cropResize.enabled &&
(cropResize.cropEnabled || cropResize.resizeEnabled)) {
// edges Stabilize exposes (issue #109). Listed whenever it is on, not only
// when it crops or resizes: it also upscales, pads, squares pixels and
// converts BT.601 to BT.709 at any size, and [isSubtitleOnly] reads this
// list. In step with the worker's `enabled_passes`.
if (cropResize.enabled) {
passes.add(PassType.cropResize);
}
// Faces are restored at the delivered resolution, after the resize, and
Expand Down Expand Up @@ -438,6 +440,13 @@ class ProcessingPipeline {
return count;
}

/// Whether a job only transcribes: subtitles on, and nothing that touches
/// the picture or the soundtrack. The worker then skips the encode entirely,
/// so a pass missing from [enabledPasses] would be silently dropped. The one
/// test, used for both the job and its output path.
bool get isSubtitleOnly =>
subtitles.enabled && enabledPasses.isEmpty && !audio.hasEffect;

/// Get count of enabled video processing passes (excludes audio
/// restoration and subtitles).
int get videoPassCount {
Expand Down
8 changes: 2 additions & 6 deletions app/lib/viewmodels/main_viewmodel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1753,9 +1753,7 @@ class MainViewModel extends ChangeNotifier {
// Subtitle-only mode: no video processing passes enabled, only subtitles
// Audio restoration needs the full job (it renders the output's
// soundtrack), so it rules out subtitle-only mode.
final isSubtitleOnly = _processingPipeline.subtitles.enabled &&
_processingPipeline.enabledPasses.isEmpty &&
!_processingPipeline.audio.hasEffect;
final isSubtitleOnly = _processingPipeline.isSubtitleOnly;
_lastJobSubtitleOnly = isSubtitleOnly &&
_processingPipeline.subtitles.output == SubtitleOutput.srtFile;

Expand Down Expand Up @@ -2128,9 +2126,7 @@ class MainViewModel extends ChangeNotifier {
final outputFilename = _encodingSettings.generateOutputFilename(inputBaseName);

// Subtitle-only with SRT output: use .srt extension
final isSubtitleOnly = _processingPipeline.subtitles.enabled &&
_processingPipeline.videoPassCount == 0 &&
!_processingPipeline.audio.hasEffect;
final isSubtitleOnly = _processingPipeline.isSubtitleOnly;
if (isSubtitleOnly && _processingPipeline.subtitles.output == SubtitleOutput.srtFile) {
return path.join(outputDir, '$outputFilename.srt');
}
Expand Down
58 changes: 58 additions & 0 deletions app/test/crop_resize_bt709_only_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Crop & Resize used only for its BT.601 -> BT.709 conversion (forum #154 /
// #155): "Always" converts with no crop or resize, as long as the pass is on.
// The pass must then count as a pass, or a job with subtitles on is sent as
// subtitle-only and the worker never encodes the picture, so the conversion
// is silently lost.
import 'package:flutter_test/flutter_test.dart';
import 'package:vapourbox/models/crop_resize_parameters.dart';
import 'package:vapourbox/models/processing_pipeline.dart';
import 'package:vapourbox/models/qtgmc_parameters.dart';
import 'package:vapourbox/models/subtitle_parameters.dart';

void main() {
ProcessingPipeline pipeline(CropResizeParameters cropResize, {bool subtitles = false}) =>
ProcessingPipeline(
deinterlace: const QTGMCParameters(enabled: false),
cropResize: cropResize,
subtitles: SubtitleParameters(enabled: subtitles),
);

const colourOnly = CropResizeParameters(enabled: true, convertToBt709: Bt709Conversion.on);

test('an enabled Crop & Resize with no geometry is a pass', () {
expect(colourOnly.cropEnabled, isFalse);
expect(colourOnly.resizeEnabled, isFalse);
final p = pipeline(colourOnly);
expect(p.enabledPasses, [PassType.cropResize]);
expect(p.videoPassCount, 1, reason: 'the two counts agree');
expect(pipeline(const CropResizeParameters(enabled: true, padEnabled: true)).enabledPasses,
[PassType.cropResize], reason: 'borders only');
expect(pipeline(const CropResizeParameters(enabled: false, convertToBt709: Bt709Conversion.on))
.enabledPasses, isEmpty, reason: 'a switched-off pass is not one');
});

test('subtitles on: a colour-only Crop & Resize is not subtitle-only', () {
expect(pipeline(colourOnly, subtitles: true).isSubtitleOnly, isFalse);
expect(pipeline(const CropResizeParameters(enabled: false), subtitles: true).isSubtitleOnly,
isTrue, reason: 'control: nothing touches the picture');
expect(pipeline(colourOnly).isSubtitleOnly, isFalse, reason: 'no subtitles');
});

test('the pass row names the conversion when it is all the pass does', () {
expect(colourOnly.summary, 'BT.709');
expect(const CropResizeParameters(enabled: true).summary, 'Custom',
reason: 'Auto with no geometry does nothing on its own');
expect(const CropResizeParameters(enabled: false, convertToBt709: Bt709Conversion.on).summary,
'Off');
expect(
const CropResizeParameters(
enabled: true,
resizeEnabled: true,
targetWidth: 1920,
targetHeight: 1080,
convertToBt709: Bt709Conversion.on,
).summary,
'1920x1080',
reason: 'geometry still describes the pass');
});
}
68 changes: 66 additions & 2 deletions app/test/integration_bt709_conversion_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ void main() {
});

VideoJob job(String name, String input, Bt709Conversion mode,
{int width = _srcW, int height = _srcH, bool sourceTagged = true, int targetH = 1080}) {
{int width = _srcW,
int height = _srcH,
bool sourceTagged = true,
int targetH = 1080,
bool resize = true}) {
return VideoJob(
id: const Uuid().v4(),
inputPath: input,
Expand All @@ -168,7 +172,8 @@ void main() {
deinterlace: const QTGMCParameters(enabled: false),
cropResize: CropResizeParameters(
enabled: true,
resizeEnabled: true,
// Off: the pass is on for the conversion alone (forum #154/#155).
resizeEnabled: resize,
targetWidth: targetH * 16 ~/ 9,
targetHeight: targetH,
maintainAspect: true,
Expand Down Expand Up @@ -292,4 +297,63 @@ void main() {
print(' preview vs encode mean abs diff: ${diff.toStringAsFixed(3)}/255');
expect(diff, lessThan(1.0), reason: 'the preview shows different colours from the encode');
}, timeout: const Timeout(Duration(minutes: 5)));
// Always with no crop or resize (forum #154/#155): the pass is on for the
// conversion alone, so the picture stays SD and only its colour changes.
test('Always with no crop or resize: SD stays SD, converted', () async {
final out = await encode(job('always_nogeom.mkv', tagged, Bt709Conversion.on, resize: false));
final t = await tags(out);
print(' always, no geometry: $t');
expect(t['width'], _srcW);
expect(t['height'], _srcH);
expect(t['color_space'], 'bt709');
expect(t['color_transfer'], 'bt709');
expect(t['color_range'], 'tv');
expect(t['color_primaries'], 'smpte170m',
reason: 'only the matrix is converted, so the primaries stay the source\'s');
final viaTags = _patchRgb(await _decode(out, _swsMatrix(t['color_space'] as String?)));
final as709 = _patchRgb(await _decode(out, 'bt709'));
for (final name in _patches.keys) {
final d = _maxDelta(viaTags[name]!, source601[name]!);
print(' $name: max delta ${d.toStringAsFixed(2)}');
expect(d, lessThanOrEqualTo(_tolerance), reason: '$name through the output\'s tags');
expect(_maxDelta(as709[name]!, source601[name]!), lessThanOrEqualTo(_tolerance),
reason: '$name decoded as 709');
}
}, timeout: const Timeout(Duration(minutes: 5)));

test('Always with no crop or resize: the preview shows what the encode stores', () async {
final j = job('always_nogeom_preview.mkv', tagged, Bt709Conversion.on, resize: false);
final out = await encode(j);
const frame = 2;
final preview = await WorkerHarness.runPreview(j.toJson(), frame: frame);
expect(preview.success, isTrue, reason: '$preview');
final previewRgb = await WorkerHarness.imageToRgb24(preview.png!, label: 'pv709ng');
final encoded = await WorkerHarness.frameRgb24(out, frame, label: 'enc709ng');
final diff = WorkerHarness.meanAbsDiff(previewRgb, encoded);
print(' preview vs encode mean abs diff: ${diff.toStringAsFixed(3)}/255');
expect(diff, lessThan(1.0), reason: 'the preview shows different colours from the encode');
}, timeout: const Timeout(Duration(minutes: 5)));

test('Never and Auto with no crop or resize: bit-identical to the source', () async {
Future<List<int>> yuv(String path, String name) async {
final raw = '$_outDir/$name.yuv';
await _ffmpeg(['-i', path, '-f', 'rawvideo', '-pix_fmt', 'yuv420p', raw]);
return File(raw).readAsBytes();
}

final source = await yuv(tagged, 'nogeom_source');
for (final mode in [Bt709Conversion.off, Bt709Conversion.auto]) {
final out = await encode(job('nogeom_${mode.name}.mkv', tagged, mode, resize: false));
final t = await tags(out);
expect(t['color_space'], 'smpte170m', reason: '${mode.name}: tags stay 601');
expect(t['color_primaries'], 'smpte170m');
final output = await yuv(out, 'nogeom_${mode.name}');
expect(output.length, source.length, reason: '${mode.name}: same frames');
var differing = 0;
for (var i = 0; i < source.length; i++) {
if (source[i] != output[i]) differing++;
}
expect(differing, 0, reason: '${mode.name}: every sample is the source\'s');
}
}, timeout: const Timeout(Duration(minutes: 5)));
}
33 changes: 33 additions & 0 deletions app/test/integration_filter_parameters_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,39 @@ void main() {
print(' PASS');
}, timeout: const Timeout(Duration(minutes: 2)));

// Always with no crop or resize (forum #154/#155), by the panel's own
// route: the pass starts switched off, the user picks Always, and editing
// a value switches the pass on (enabledByEditFrom). It then converts the
// SD picture at its own size, with no resize in the script.
test('crop_resize: BT.709 Always converts with no crop or resize', () async {
final off = ParameterConverter.fromCropResize(const CropResizeParameters())
.withEnabled(false);
expect(off.values['cropEnabled'], isFalse);
expect(off.values['resizeEnabled'], isFalse);
Future<String> scriptFor(String mode) async {
// Auto is the default, so "choosing" it changes nothing; switch the
// pass on by hand to test Auto with the pass on and no geometry.
final edited = mode == 'auto'
? off.withEnabled(true)
: off.withValue('convertToBt709', mode).enabledByEditFrom(off);
final params = ParameterConverter.toCropResize(edited);
expect(params.enabled, isTrue, reason: 'choosing an option switches the pass on');
final job = buildJob(testName: 'bt709_nogeom_$mode', cropResize: params);
return generateScriptViaWorker(job);
}

const block = 'RemoveFrameProps(clip, props=["_Matrix", "_Range", "_ColorRange"])';
final on = await scriptFor('on');
expect(on, contains(block));
expect(on, contains('matrix_in_s="170m"'));
expect(on, contains('matrix_s="709"'));
expect(on, isNot(contains('width=target_w')), reason: 'no resize block without a resize');
expect(await scriptFor('auto'), isNot(contains(block)),
reason: 'Auto leaves an SD output alone');
expect(await scriptFor('off'), isNot(contains(block)));
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
// you look at the picture.
Expand Down
7 changes: 7 additions & 0 deletions worker/src/models/crop_resize_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,13 @@ mod tests {
assert!(!on.converts_to_bt709(false, (720, 576)));
let off = CropResizeParameters { convert_to_bt709: Bt709Conversion::Off, ..p.clone() };
assert!(!off.converts_to_bt709(true, (1920, 1080)));
// No crop or resize: an enabled pass still converts on On, at SD size.
let colour_only = CropResizeParameters { enabled: true, ..CropResizeParameters::default() };
assert!(!colour_only.crop_enabled && !colour_only.resize_enabled);
assert_eq!(colour_only.output_size(720, 576, 1.0), (720, 576));
assert!(CropResizeParameters { convert_to_bt709: Bt709Conversion::On, ..colour_only.clone() }
.converts_to_bt709(true, (720, 576)));
assert!(!colour_only.converts_to_bt709(true, (720, 576)), "Auto leaves SD alone");
let disabled = CropResizeParameters { enabled: false, convert_to_bt709: Bt709Conversion::On, ..p };
assert!(!disabled.converts_to_bt709(true, (1920, 1080)), "a disabled pass converts nothing");
}
Expand Down
23 changes: 19 additions & 4 deletions worker/src/models/processing_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,10 +490,11 @@ impl ProcessingPipeline {
}
// Crop and resize are one pass and run together, crop first. The crop
// used to run ahead of everything else, which meant it could not
// remove the edges Stabilize exposes (issue #109).
if self.crop_resize.enabled
&& (self.crop_resize.crop_enabled || self.crop_resize.resize_enabled)
{
// remove the edges Stabilize exposes (issue #109). Listed whenever it
// is on, not only when it crops or resizes: it also upscales, pads,
// squares pixels and converts BT.601 to BT.709 at any size, and the
// app decides subtitle-only mode from this list.
if self.crop_resize.enabled {
passes.push(PassType::CropResize);
}

Expand Down Expand Up @@ -1121,6 +1122,20 @@ mod tests {
assert!(!passes.contains(&PassType::ChromaFixes));
}

#[test]
fn crop_resize_is_a_pass_whenever_it_is_on() {
let mut pipeline = ProcessingPipeline::default();
pipeline.deinterlace.enabled = false;
pipeline.crop_resize.enabled = true;
pipeline.crop_resize.convert_to_bt709 = crate::models::Bt709Conversion::On;
assert_eq!(pipeline.enabled_passes(), vec![PassType::CropResize], "colour conversion only");
pipeline.crop_resize.convert_to_bt709 = crate::models::Bt709Conversion::Auto;
pipeline.crop_resize.pad_enabled = true;
assert_eq!(pipeline.enabled_passes(), vec![PassType::CropResize], "borders only");
pipeline.crop_resize.enabled = false;
assert!(pipeline.enabled_passes().is_empty());
}

#[test]
fn test_serialization() {
let pipeline = ProcessingPipeline::default();
Expand Down
Loading
Loading