diff --git a/CLAUDE.md b/CLAUDE.md index 17f8fd4..b5035c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,7 +116,7 @@ VapourBox/ | `worker/src/pixel_format.rs` | Source `pix_fmt` → pipe format (see "Source Pixel Formats") | | `worker/templates/stabilize.py` | Stabilize pass's vertical pitch correction (see "Stabilize") | | `worker/src/cpu.rs` | The x86 deps CPU tier (`v3`/`v2`) — the single decision; see "CPU tiers" | -| `worker/src/source_decode.rs` | Decoder ffmpeg args for encode + preview: full stored frame, size-guarded (see "Source Frame Size") | +| `worker/src/source_decode.rs` | Decoder ffmpeg args for encode + preview: full stored frame, size-guarded, invalid colour tags cleared (see "Source Frame Size", "Colour metadata") | | `worker/src/source_timing.rs` | A variable-frame-rate source's stored frames placed on the constant-rate grid (see "Variable Frame Rate Sources") | | `worker/src/gpu_plugin.rs` | The ncnn-on-Vulkan plugins (RIFE, AI upscaling): file layout, RIFE's model add-on, the `--probe-rife` / `--probe-upscale` probe (on stand-in models, no add-on needed), and the user-facing reason/remedy when they can't run (see "GPU Plugins") | | `worker/src/animejanai.rs` | AnimeJaNai's add-on models: where they live, which one a job runs, the refusal when they're missing (see "Add-ons") | @@ -159,6 +159,7 @@ VapourBox/ | `app/lib/models/ai_runtime_status.dart` | The worker's `--probe-ai-runtime` result | | `app/lib/models/face_restore_parameters.dart` | Face Restoration's parameters and the add-ons each model needs (`faceRestoreAddonIds`) | | `app/lib/models/colorize_parameters.dart` | Colourisation's parameters and the add-on each model needs (`colorizeAddonSelectedBy`) | +| `app/lib/models/color_tags.dart` | FFmpeg's accepted colour-tag lists (twin of `color_metadata.rs`) and `decodeNeutraliser`, the filter that clears invalid tags before the app's decodes | | `app/lib/models/hdr_display.dart` | The display tone-map for the app's own source decodes (twin of `ColorMetadata::display_tonemap_filter`) and Dolby Vision detection | | `app/lib/models/gpu_plugin_status.dart` | The worker's GPU-plugin probe results, and the panels' notice decision | | `app/assets/filters/core/*.json` | Built-in filter schema definitions | @@ -920,6 +921,20 @@ The preview path needs the same source tags fed to swscale (`swscale_input_opts()`) or it guesses the matrix and shows a colour shift the real encode doesn't have. +**A tag FFmpeg rejects (ProRes's `reserved`) is cleared before any decode +converts, and only that tag.** FFmpeg 9's scaler fails a frame tagged +`reserved` outright, so every decode of the source that may convert its pixel +format puts `ColorMetadata::decode_tag_neutraliser` (worker, via +`source_decode::decoder_filter`: encode, preview and every detector) or its +byte-identical twin `ColorTags.decodeNeutraliser` (app: before frame and +thumbnails, via `PreviewGenerator.beforeFrameArgs`/`thumbnailArgs`) **first** +in its one `-vf`. Never clear a valid tag (a valid matrix moves yuv → gray16le +by up to 28 levels), which is why the job carries ffprobe's **raw** tags +(`colorTag` forwards `reserved`) and `from_raw` drops them only for the +encoder's flags. A new decode of the source must use the same helper. +`integration_reserved_colour_tags_test.dart` (heavy) checks each one against +an untagged twin; see docs/reports/2026-11-reserved-colour-tags. + **BT.601 → BT.709 on SD → HD (app 1.7.0) is the one place the script converts colour.** Crop & Resize's `convertToBt709` (`Bt709Conversion`: `auto` default, `on`, `off`) converts a BT.601 source (tagged diff --git a/Tests/TestResources/generate_reserved_colour_tags_test.sh b/Tests/TestResources/generate_reserved_colour_tags_test.sh new file mode 100755 index 0000000..8a90852 --- /dev/null +++ b/Tests/TestResources/generate_reserved_colour_tags_test.sh @@ -0,0 +1,73 @@ +#!/bin/bash +# Generate the two ProRes clips tagged with `reserved` colour primaries and +# transfer (docs/reports/2026-11-reserved-colour-tags). +# +# FFmpeg 9's scaler refuses to convert a frame tagged `reserved`, so before the +# fix the app's "before" frame, its thumbnails, Detect borders and Descale's +# Detect all failed on such a source, and so did the encode whenever the +# decoder converts to the pipe format. +# +# The tags come from the prores_metadata bitstream filter: its `unknown` +# constant writes code 0, which is Apple's "unknown" and ISO 23091's +# `reserved` — FFmpeg's decoder reports the latter. That is exactly what the +# user's clip carries (color_primaries=reserved, color_transfer=reserved, +# color_space=unknown). Stream copy only, so the pictures are untouched. +# +# reserved_colour_tags.mov ProRes 422 yuv422p10le, 15 frames of +# prores422_10bit_telecine.mov; laid out like +# the user's clip: pcm_s24le audio as stream +# 0, video as stream 1, a 1/11988 timescale. +# reserved_colour_tags_4444a.mov ProRes 4444 with alpha (yuva444p12le when +# decoded), 320x240, 10 frames: not a pipe +# format, so the ENCODE decoder converts it. +# +# Usage: ./generate_reserved_colour_tags_test.sh [output_dir] +# FFMPEG / FFPROBE override the tools (default: ffmpeg / ffprobe on PATH). + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +OUTPUT_DIR="${1:-$SCRIPT_DIR}" +FFMPEG="${FFMPEG:-ffmpeg}" +FFPROBE="${FFPROBE:-ffprobe}" +SOURCE="$SCRIPT_DIR/prores422_10bit_telecine.mov" +OUT_422="$OUTPUT_DIR/reserved_colour_tags.mov" +OUT_4444="$OUTPUT_DIR/reserved_colour_tags_4444a.mov" +RESERVED="prores_metadata=color_primaries=unknown:color_trc=unknown" + +if [ ! -f "$SOURCE" ]; then + echo "Missing $SOURCE — run ./generate_prores422_10bit_test.sh first." >&2 + exit 1 +fi + +"$FFMPEG" -v error -y \ + -i "$SOURCE" \ + -f lavfi -i anullsrc=r=48000:cl=stereo \ + -map 1:a -map 0:v \ + -frames:v 15 -shortest \ + -c:a pcm_s24le \ + -c:v copy -bsf:v "$RESERVED" \ + -video_track_timescale 11988 \ + "$OUT_422" + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT +"$FFMPEG" -v error -y \ + -f lavfi -i "testsrc2=s=320x240:r=30000/1001:d=1" \ + -frames:v 10 \ + -c:v prores_ks -profile:v 4 -vendor apl0 -pix_fmt yuva444p10le \ + "$TMP/4444a.mov" +"$FFMPEG" -v error -y -i "$TMP/4444a.mov" -c:v copy -bsf:v "$RESERVED" "$OUT_4444" + +# The fixtures are only worth anything if they reproduce the tags. +for f in "$OUT_422" "$OUT_4444"; do + tags="$("$FFPROBE" -v error -select_streams v:0 \ + -show_entries stream=color_primaries,color_transfer -of csv=p=0 "$f")" + if [ "$tags" != "reserved,reserved" ]; then + echo "$f has tags '$tags', not 'reserved,reserved'" >&2 + exit 1 + fi + echo "Wrote $f ($(wc -c < "$f" | tr -d ' ') bytes): $("$FFPROBE" -v error \ + -select_streams v:0 -show_entries stream=pix_fmt,color_primaries,color_transfer,color_space \ + -of compact=p=0 "$f")" +done diff --git a/Tests/TestResources/reserved_colour_tags.mov b/Tests/TestResources/reserved_colour_tags.mov new file mode 100644 index 0000000..880a628 Binary files /dev/null and b/Tests/TestResources/reserved_colour_tags.mov differ diff --git a/Tests/TestResources/reserved_colour_tags_4444a.mov b/Tests/TestResources/reserved_colour_tags_4444a.mov new file mode 100644 index 0000000..8bb2976 Binary files /dev/null and b/Tests/TestResources/reserved_colour_tags_4444a.mov differ diff --git a/app/lib/models/color_tags.dart b/app/lib/models/color_tags.dart new file mode 100644 index 0000000..125cad6 --- /dev/null +++ b/app/lib/models/color_tags.dart @@ -0,0 +1,62 @@ +/// Which colour tags FFmpeg accepts, and the filter that clears the ones it +/// doesn't before the app's own decodes convert a frame. +/// +/// The twin of `decode_tag_neutraliser` in +/// worker/src/models/color_metadata.rs: the lists are that file's `MATRICES`, +/// `PRIMARIES` and `TRANSFERS`, and the filter must be byte-identical for the +/// same tags. `color_tags_test.dart` reads the Rust source and checks both. +library; + +class ColorTags { + ColorTags._(); + + /// `-colorspace` values (worker `MATRICES`). + static const matrices = [ + 'rgb', 'bt709', 'fcc', 'bt470bg', 'smpte170m', 'smpte240m', 'ycgco', + 'bt2020nc', 'bt2020c', 'smpte2085', 'chroma-derived-nc', + 'chroma-derived-c', 'ictcp', + ]; + + /// `-color_primaries` values (worker `PRIMARIES`). + static const primaries = [ + 'bt709', 'bt470m', 'bt470bg', 'smpte170m', 'smpte240m', 'film', 'bt2020', + 'smpte428', 'smpte431', 'smpte432', 'jedec-p22', 'ebu3213', + ]; + + /// `-color_trc` values (worker `TRANSFERS`). + static const transfers = [ + 'bt709', 'gamma22', 'gamma28', 'smpte170m', 'smpte240m', 'linear', + 'log100', 'log316', 'iec61966-2-4', 'bt1361e', 'iec61966-2-1', + 'bt2020-10', 'bt2020-12', 'smpte2084', 'smpte428', 'arib-std-b67', + ]; + + static bool _isInvalid(String? value, List allowed, + [List alsoValid = const []]) { + final v = value?.trim().toLowerCase(); + if (v == null || v.isEmpty || v == 'unknown' || v == 'n/a') return false; + return !allowed.contains(v) && !alsoValid.contains(v); + } + + /// The `setparams` that goes FIRST in the `-vf` of every app decode of the + /// source that converts its pixel format (the "before" frame, timeline + /// thumbnails), or null when every tag is usable. Takes ffprobe's RAW + /// values — `colorTag` forwards `reserved` for exactly this. + /// + /// FFmpeg 9's scaler refuses a frame tagged `reserved` (ProRes writes code + /// 0 for "unknown", which FFmpeg reads as reserved), so without this the + /// decode fails and the Original side shows "No preview". Only an invalid + /// tag is cleared, each on its own: clearing a valid matrix changes pixels. + /// `gbr` (ffprobe's name for the RGB identity matrix) is left alone. + static String? decodeNeutraliser({ + String? matrix, + String? primaries, + String? transfer, + }) { + final params = [ + if (_isInvalid(primaries, ColorTags.primaries)) 'color_primaries=unknown', + if (_isInvalid(transfer, transfers)) 'color_trc=unknown', + if (_isInvalid(matrix, matrices, const ['gbr'])) 'colorspace=unknown', + ]; + return params.isEmpty ? null : 'setparams=${params.join(':')}'; + } +} diff --git a/app/lib/services/field_order_detector.dart b/app/lib/services/field_order_detector.dart index 8f20ae1..4362901 100644 --- a/app/lib/services/field_order_detector.dart +++ b/app/lib/services/field_order_detector.dart @@ -631,6 +631,13 @@ enum ScanType { /// command line. Anything unrecognised is dropped again worker-side, in /// `ColorMetadata::from_raw`, so a surprising value cannot reach ffmpeg. /// `PreviewGenerator` reads the tags with this too. +/// +/// A value FFmpeg rejects, such as ProRes's `reserved`, is **forwarded on +/// purpose**, not dropped here: FFmpeg's decoder reads the tag from the stream +/// whatever the job says, so every decode that converts the pixel format has +/// to know the tag is invalid to clear it first (`ColorTags.decodeNeutraliser`, +/// and the worker's `VideoJob::decode_tag_neutraliser` from the job's tags). +/// The encoder's tags still drop it, in `ColorMetadata::from_raw`. String? colorTag(Map stream, String key) { final value = (stream[key] as String?)?.trim(); if (value == null || value.isEmpty || value == 'unknown' || value == 'N/A') { diff --git a/app/lib/services/preview_generator.dart b/app/lib/services/preview_generator.dart index 7f882ec..80fc189 100644 --- a/app/lib/services/preview_generator.dart +++ b/app/lib/services/preview_generator.dart @@ -6,6 +6,7 @@ import 'dart:typed_data'; import 'package:path/path.dart' as path; import 'package:uuid/uuid.dart'; +import '../models/color_tags.dart'; import '../models/encoding_settings.dart'; import '../models/hdr_display.dart'; import '../models/preview_frame_info.dart'; @@ -94,6 +95,73 @@ class PreviewGenerator { range: _colorRange, ); + /// Clears a colour tag FFmpeg's scaler refuses (`reserved`), or null when + /// every tag is usable. See [ColorTags.decodeNeutraliser]. + String? get _tagFix => ColorTags.decodeNeutraliser( + matrix: _colorMatrix, + primaries: _colorPrimaries, + transfer: _colorTransfer, + ); + + /// The ffmpeg arguments for the "before" frame: source frame [frameIndex] + /// written to [outputPath] (JPEG in the app; the tests ask for PNG). + /// + /// Seeks to the midpoint of the interval *before* the target frame + /// ((index - 0.5)/fps) so the first decoded frame is exactly [frameIndex], + /// as the worker's preview does. [tagFix] goes first in the one `-vf`: the + /// JPEG/PNG encode converts the pixel format, which fails on a `reserved` + /// tag. The integration tests run exactly these arguments. + static List beforeFrameArgs({ + required String videoPath, + required int frameIndex, + required double frameRate, + String? toneMap, + String? tagFix, + required String outputPath, + }) { + final frame = frameIndex < 0 ? 0 : frameIndex; + final seekTime = (frame - 0.5) / frameRate; + final ss = seekTime < 0 ? 0.0 : seekTime; + final vf = [if (tagFix != null) tagFix, if (toneMap != null) toneMap]; + return [ + '-y', + '-ss', ss.toStringAsFixed(6), + ...sourceDecodeOptions, + '-i', videoPath, + '-frames:v', '1', + if (vf.isNotEmpty) ...['-vf', vf.join(',')], + '-q:v', '2', + outputPath, + ]; + } + + /// The ffmpeg arguments for one timeline thumbnail at [time] seconds. + /// [tagFix] precedes `scale`, which is the step that converts. + static List thumbnailArgs({ + required String videoPath, + required double time, + String? toneMap, + String? tagFix, + required String outputPath, + }) { + return [ + '-y', + '-ss', time.toStringAsFixed(3), + ...sourceDecodeOptions, + '-i', videoPath, + '-vframes', '1', + // Scaled first: tone-mapping a 4K frame in float costs far more + // than the 160-pixel thumbnail it ends up as. + '-vf', [ + if (tagFix != null) tagFix, + 'scale=160:-1', + if (toneMap != null) toneMap, + ].join(','), + '-q:v', '5', + outputPath, + ]; + } + /// Log messages from preview generation (stderr output). final List _previewLog = []; @@ -229,22 +297,18 @@ class PreviewGenerator { // Extract frame final outputPath = '$_tempDir/frame_$frame.jpg'; - final seekTime = ((frame - 0.5) / _frameRate); - final ss = seekTime < 0 ? 0.0 : seekTime; try { final result = await Process.run( _ffmpegPath!, - [ - '-y', - '-ss', ss.toStringAsFixed(6), - ...sourceDecodeOptions, - '-i', _currentVideoPath!, - '-frames:v', '1', - if (_toneMap case final toneMap?) ...['-vf', toneMap], - '-q:v', '2', - outputPath, - ], + beforeFrameArgs( + videoPath: _currentVideoPath!, + frameIndex: frame, + frameRate: _frameRate, + toneMap: _toneMap, + tagFix: _tagFix, + outputPath: outputPath, + ), ); if (result.exitCode == 0 && await File(outputPath).exists()) { @@ -625,21 +689,13 @@ class PreviewGenerator { try { final result = await Process.run( _ffmpegPath!, - [ - '-y', - '-ss', time.toStringAsFixed(3), - ...sourceDecodeOptions, - '-i', videoPath, - '-vframes', '1', - // Scaled first: tone-mapping a 4K frame in float costs far more - // than the 160-pixel thumbnail it ends up as. - '-vf', [ - 'scale=160:-1', - if (_toneMap case final toneMap?) toneMap, - ].join(','), - '-q:v', '5', - outputPath, - ], + thumbnailArgs( + videoPath: videoPath, + time: time, + toneMap: _toneMap, + tagFix: _tagFix, + outputPath: outputPath, + ), ); if (result.exitCode == 0 && await File(outputPath).exists()) { diff --git a/app/test/color_metadata_test.dart b/app/test/color_metadata_test.dart index 207c33e..cd69070 100644 --- a/app/test/color_metadata_test.dart +++ b/app/test/color_metadata_test.dart @@ -25,6 +25,14 @@ void main() { expect(colorTag(const {}, 'color_space'), isNull); }); + test('forwards a value FFmpeg rejects, such as ProRes\'s reserved', () { + // Deliberate: the worker needs the raw tag to know the decoder must + // clear it (VideoJob::decode_tag_neutraliser). The encoder's tags drop + // it in ColorMetadata::from_raw. + expect(colorTag({'color_primaries': 'reserved'}, 'color_primaries'), + 'reserved'); + }); + test('trims surrounding whitespace', () { expect(colorTag({'color_space': ' bt709 '}, 'color_space'), 'bt709'); }); diff --git a/app/test/color_tags_test.dart b/app/test/color_tags_test.dart new file mode 100644 index 0000000..7f28d7a --- /dev/null +++ b/app/test/color_tags_test.dart @@ -0,0 +1,128 @@ +// Colour tags FFmpeg's scaler refuses (ProRes's `reserved`) are cleared before +// the app's own decodes convert a frame; see ColorTags.decodeNeutraliser and +// its twin, decode_tag_neutraliser in worker/src/models/color_metadata.rs. + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:vapourbox/models/color_tags.dart'; +import 'package:vapourbox/models/hdr_display.dart'; +import 'package:vapourbox/services/preview_generator.dart'; + +String _repoFile(String rel) => File(p.join( + Directory.current.path.endsWith('app') + ? Directory.current.parent.path + : Directory.current.path, + rel)) + .readAsStringSync() + .replaceAll('\r\n', '\n'); + +/// The `&str` entries of one `const NAME: &[&str] = &[ … ];` in Rust source. +List _rustList(String src, String name) { + final start = src.indexOf('const $name: &[&str] = &['); + expect(start, isNot(-1), reason: '$name not found in color_metadata.rs'); + final body = src.substring(start, src.indexOf('];', start)); + return RegExp(r'"([^"]*)"') + .allMatches(body.substring(body.indexOf('&[') + 2)) + .map((m) => m.group(1)!) + .toList(); +} + +void main() { + group('ColorTags lists', () { + final rust = _repoFile('worker/src/models/color_metadata.rs'); + test('are the worker\'s, value for value', () { + expect(ColorTags.matrices, _rustList(rust, 'MATRICES')); + expect(ColorTags.primaries, _rustList(rust, 'PRIMARIES')); + expect(ColorTags.transfers, _rustList(rust, 'TRANSFERS')); + }); + }); + + group('decodeNeutraliser', () { + String? n(String? m, String? pr, String? t) => + ColorTags.decodeNeutraliser(matrix: m, primaries: pr, transfer: t); + + // The same table as the Rust test `decode_tag_neutraliser_cases`. + test('matches the worker\'s case table', () { + expect(n(null, null, null), isNull); + expect(n('bt709', 'bt709', 'bt709'), isNull); + expect(n('unknown', 'unknown', 'unknown'), isNull); + expect(n('N/A', '', ' '), isNull); + expect(n('unknown', 'reserved', 'reserved'), + 'setparams=color_primaries=unknown:color_trc=unknown'); + expect(n('bt709', 'reserved', null), 'setparams=color_primaries=unknown'); + expect(n(null, 'smpte170m', 'Reserved '), 'setparams=color_trc=unknown'); + for (final m in ['reserved', 'ipt-c2', 'ycgco-re']) { + expect(n(m, null, null), 'setparams=colorspace=unknown', reason: m); + } + expect(n('gbr', null, null), isNull); + expect(n('reserved', 'reserved', 'reserved'), + 'setparams=color_primaries=unknown:color_trc=unknown:colorspace=unknown'); + }); + + test('never clears a value FFmpeg accepts', () { + for (final m in ColorTags.matrices) { + expect(n(m, null, null), isNull, reason: m); + } + for (final pr in ColorTags.primaries) { + expect(n(null, pr, null), isNull, reason: pr); + } + for (final t in ColorTags.transfers) { + expect(n(null, null, t), isNull, reason: t); + } + }); + }); + + group('PreviewGenerator arguments', () { + const fix = 'setparams=color_primaries=unknown:color_trc=unknown'; + List vfOf(List args) { + expect(args.where((a) => a == '-vf').length, 1); + return args[args.indexOf('-vf') + 1].split(','); + } + + test('before frame: clean tags change nothing', () { + final args = PreviewGenerator.beforeFrameArgs( + videoPath: 'in.mov', frameIndex: 10, frameRate: 25, outputPath: 'o.jpg'); + expect(args, [ + '-y', '-ss', '0.380000', '-apply_cropping', 'codec', '-i', 'in.mov', + '-frames:v', '1', '-q:v', '2', 'o.jpg', + ]); + }); + + test('before frame: the fix is the whole -vf, after -i', () { + final args = PreviewGenerator.beforeFrameArgs( + videoPath: 'in.mov', frameIndex: 0, frameRate: 25, tagFix: fix, + outputPath: 'o.jpg'); + expect(vfOf(args), [fix]); + expect(args.indexOf('-vf'), greaterThan(args.indexOf('-i'))); + }); + + test('before frame: the fix precedes the HDR tone map', () { + // Dolby Vision profile 5: PQ transfer, ipt-c2 matrix. + final toneMap = HdrDisplay.toneMapFilter( + matrix: 'ipt-c2', primaries: 'bt2020', transfer: 'smpte2084')!; + final tagFix = ColorTags.decodeNeutraliser( + matrix: 'ipt-c2', primaries: 'bt2020', transfer: 'smpte2084'); + expect(tagFix, 'setparams=colorspace=unknown'); + final args = PreviewGenerator.beforeFrameArgs( + videoPath: 'in.mov', frameIndex: 5, frameRate: 24, toneMap: toneMap, + tagFix: tagFix, outputPath: 'o.jpg'); + expect(args[args.indexOf('-vf') + 1], '$tagFix,$toneMap'); + }); + + test('thumbnail: the fix precedes scale, which converts', () { + final plain = PreviewGenerator.thumbnailArgs( + videoPath: 'in.mov', time: 1, outputPath: 't.jpg'); + expect(vfOf(plain), ['scale=160:-1']); + final fixed = PreviewGenerator.thumbnailArgs( + videoPath: 'in.mov', time: 1, tagFix: fix, outputPath: 't.jpg'); + expect(vfOf(fixed), [...fix.split(','), 'scale=160:-1']); + final hdr = HdrDisplay.toneMapFilter(transfer: 'smpte2084')!; + final both = PreviewGenerator.thumbnailArgs( + videoPath: 'in.mov', time: 1, tagFix: fix, toneMap: hdr, + outputPath: 't.jpg'); + expect(both[both.indexOf('-vf') + 1], '$fix,scale=160:-1,$hdr'); + }); + }); +} diff --git a/app/test/integration_reserved_colour_tags_test.dart b/app/test/integration_reserved_colour_tags_test.dart new file mode 100644 index 0000000..2e60db2 --- /dev/null +++ b/app/test/integration_reserved_colour_tags_test.dart @@ -0,0 +1,275 @@ +/// Sources tagged with `reserved` colour primaries/transfer +/// (docs/reports/2026-11-reserved-colour-tags). +/// +/// FFmpeg 9's scaler refuses to convert such a frame, so before the fix the +/// app's "before" frame and thumbnails, `--detect-crop` and `--detect-native` +/// all failed on a ProRes that writes "unknown" the Apple way (code 0, which +/// FFmpeg reads as `reserved`) — and so did the encode and preview of a ProRes +/// 4444 with alpha, which the decoder converts to the pipe format. +/// +/// Every check compares against an untagged twin: the same file stream-copied +/// with valid primaries/transfer (bt709), the matrix left unknown as in the +/// fixture. Clearing invalid primaries/transfer must give exactly the twin's +/// pictures and reports, not merely succeed. +/// +/// The fixtures come from `Tests/TestResources/generate_reserved_colour_tags_test.sh`. +/// +/// Heavy (runs the worker, encodes) — runs in the nightly workflow. +@Tags(['heavy']) +library; + +import 'dart:convert'; +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:uuid/uuid.dart'; + +import 'package:vapourbox/models/color_tags.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 'package:vapourbox/services/field_order_detector.dart'; +import 'package:vapourbox/services/preview_generator.dart'; + +import 'support/worker_harness.dart'; + +String get _outDir => p.join(WorkerHarness.outputDir, 'reserved_colour_tags'); + +String _resource(String name) => + p.join(WorkerHarness.repoRoot, 'Tests', 'TestResources', name); + +/// What the app reads from ffprobe for a source, through the same `colorTag`. +class _Source { + final String path; + final int width, height, frames; + final double fps; + final String pixFmt; + final String? matrix, primaries, transfer, range; + + _Source(this.path, Map s) + : width = s['width'] as int, + height = s['height'] as int, + frames = int.parse(s['nb_frames'].toString()), + fps = _rate(s['avg_frame_rate'] as String), + pixFmt = s['pix_fmt'] as String, + matrix = colorTag(s, 'color_space'), + primaries = colorTag(s, 'color_primaries'), + transfer = colorTag(s, 'color_transfer'), + range = colorTag(s, 'color_range'); + + static double _rate(String r) { + final parts = r.split('/'); + return double.parse(parts[0]) / double.parse(parts[1]); + } + + static Future<_Source> probe(String path) async { + final json = await WorkerHarness.ffprobeJson([ + '-v', 'error', '-select_streams', 'v:0', '-show_streams', '-of', 'json', + path, + ]); + return _Source(path, (json['streams'] as List).first as Map); + } + + String? get tagFix => ColorTags.decodeNeutraliser( + matrix: matrix, primaries: primaries, transfer: transfer); + + VideoJob job(String name) => VideoJob( + id: const Uuid().v4(), + inputPath: path, + outputPath: p.join(_outDir, '$name.mkv'), + processingPipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + ), + encodingSettings: const EncodingSettings( + codec: VideoCodec.ffv1, + container: ContainerFormat.mkv, + audioMode: AudioMode.none, + ), + totalFrames: frames, + inputFrameRate: fps, + inputWidth: width, + inputHeight: height, + inputPixelFormat: pixFmt, + inputColorMatrix: matrix, + inputColorPrimaries: primaries, + inputColorTransfer: transfer, + inputColorRange: range, + ); +} + +/// [source] stream-copied with valid primaries and transfer. +Future _twin(String source) async { + final out = p.join(_outDir, 'twin_${p.basename(source)}'); + final r = await Process.run( + WorkerHarness.ffmpegPath, + [ + '-v', 'error', '-y', '-i', source, '-map', '0', '-c', 'copy', + '-bsf:v', 'prores_metadata=color_primaries=bt709:color_trc=bt709', + out, + ], + environment: WorkerHarness.ffmpegEnv, + ); + expect(r.exitCode, 0, reason: 'making the twin: ${r.stderr}'); + return out; +} + +/// Run ffmpeg with [args] (an output path last); the file's bytes, or null +/// with the stderr when it fails. +Future<(Uint8List?, String)> _ffmpeg(List args) async { + final out = File(args.last); + if (out.existsSync()) out.deleteSync(); + final r = await Process.run(WorkerHarness.ffmpegPath, ['-v', 'error', ...args], + environment: WorkerHarness.ffmpegEnv); + if (r.exitCode != 0 || !out.existsSync()) return (null, r.stderr.toString()); + return (out.readAsBytesSync(), ''); +} + +Future> _detect(String flag, VideoJob job) async { + final config = File(p.join(_outDir, '${job.id}.json')); + await config.writeAsString(jsonEncode(job.toJson())); + try { + final r = await Process.run( + WorkerHarness.workerPath, + [flag, '--config', config.path], + environment: WorkerHarness.workerEnv, + workingDirectory: File(WorkerHarness.workerPath).parent.path, + ); + final line = const LineSplitter() + .convert(r.stdout.toString()) + .lastWhere((l) => l.trim().startsWith('{'), orElse: () => '{}'); + final report = jsonDecode(line) as Map; + expect(report, isNotEmpty, reason: '$flag printed no report: ${r.stderr}'); + expect(report['error'], isNull, reason: '$flag: ${report['error']}\n${r.stderr}'); + return report; + } finally { + await config.delete(); + } +} + +/// MD5 of the first video stream's decoded frames, in its own format. +Future _framesMd5(String path) async { + final r = await Process.run( + WorkerHarness.ffmpegPath, + ['-v', 'error', '-i', path, '-map', '0:v:0', '-c:v', 'rawvideo', '-f', 'md5', '-'], + environment: WorkerHarness.ffmpegEnv, + ); + expect(r.exitCode, 0, reason: '${r.stderr}'); + return (r.stdout as String).trim(); +} + +void main() { + late _Source prores422, twin422, prores4444a, twin4444a; + + setUpAll(() async { + await WorkerHarness.ensureReady(); + Directory(_outDir).createSync(recursive: true); + prores422 = await _Source.probe(_resource('reserved_colour_tags.mov')); + prores4444a = await _Source.probe(_resource('reserved_colour_tags_4444a.mov')); + twin422 = await _Source.probe(await _twin(prores422.path)); + twin4444a = await _Source.probe(await _twin(prores4444a.path)); + }); + + test('the fixtures carry reserved tags, and the twins valid ones', () { + for (final s in [prores422, prores4444a]) { + expect((s.primaries, s.transfer, s.matrix), ('reserved', 'reserved', null), + reason: s.path); + expect(s.tagFix, 'setparams=color_primaries=unknown:color_trc=unknown'); + } + for (final s in [twin422, twin4444a]) { + expect((s.primaries, s.transfer, s.matrix), ('bt709', 'bt709', null)); + expect(s.tagFix, isNull); + } + expect(prores4444a.pixFmt, 'yuva444p12le', + reason: 'must not be a pipe format, or the encode decoder never converts'); + }); + + test('before frame: fails without the fix, equals the twin\'s with it', () async { + List args(_Source s, {required bool fix}) => + PreviewGenerator.beforeFrameArgs( + videoPath: s.path, + frameIndex: 7, + frameRate: s.fps, + tagFix: fix ? s.tagFix : null, + outputPath: p.join(_outDir, 'before_${p.basename(s.path)}_$fix.png'), + ); + final (broken, err) = await _ffmpeg(args(prores422, fix: false)); + expect(broken, isNull, reason: 'the fixture no longer reproduces the bug'); + expect(err, contains('Unsupported input')); + + for (final (s, t) in [(prores422, twin422), (prores4444a, twin4444a)]) { + final (fixed, e1) = await _ffmpeg(args(s, fix: true)); + expect(fixed, isNotNull, reason: e1); + final (twin, e2) = await _ffmpeg(args(t, fix: true)); + expect(twin, isNotNull, reason: e2); + expect( + WorkerHarness.meanAbsDiff( + await WorkerHarness.imageToRgb24(fixed!), + await WorkerHarness.imageToRgb24(twin!), + ), + 0, + reason: s.path, + ); + } + }); + + test('thumbnail: fails without the fix, succeeds with it', () async { + List args({String? tagFix}) => PreviewGenerator.thumbnailArgs( + videoPath: prores422.path, + time: 0.2, + tagFix: tagFix, + outputPath: p.join(_outDir, 'thumb_${tagFix != null}.jpg'), + ); + final (broken, _) = await _ffmpeg(args()); + expect(broken, isNull, reason: 'the fixture no longer reproduces the bug'); + final (fixed, err) = await _ffmpeg(args(tagFix: prores422.tagFix)); + expect(fixed, isNotNull, reason: err); + }); + + test('--detect-crop reports exactly what it reports on the twin', () async { + final report = await _detect('--detect-crop', prores422.job('crop')); + final twin = await _detect('--detect-crop', twin422.job('crop_twin')); + expect(report, twin); + }); + + test('--detect-native reports exactly what it reports on the twin', () async { + final report = await _detect('--detect-native', prores422.job('native')); + final twin = await _detect('--detect-native', twin422.job('native_twin')); + expect(report, twin); + }); + + test('--detect-noise-halo reports exactly what it reports on the twin', () async { + final report = await _detect('--detect-noise-halo', prores422.job('nh')); + final twin = await _detect('--detect-noise-halo', twin422.job('nh_twin')); + expect(report, twin); + }); + + test('preview renders the twin\'s picture (422 and converted 4444a)', () async { + for (final (s, t) in [(prores422, twin422), (prores4444a, twin4444a)]) { + final preview = await WorkerHarness.runPreview(s.job('pv').toJson(), frame: 5); + expect(preview.success, isTrue, reason: '${preview.error}\n${preview.logs}'); + final twin = await WorkerHarness.runPreview(t.job('pv_twin').toJson(), frame: 5); + expect(twin.success, isTrue, reason: '${twin.error}\n${twin.logs}'); + expect( + WorkerHarness.meanAbsDiff( + await WorkerHarness.imageToRgb24(preview.png!), + await WorkerHarness.imageToRgb24(twin.png!), + ), + 0, + reason: s.path, + ); + } + }); + + test('a 4444-with-alpha encode (converting decoder) equals the twin\'s', () async { + final job = await WorkerHarness.runJob(prores4444a.job('enc').toJson(), + label: 'encode reserved 4444a'); + expect(job.success, isTrue, reason: '${job.error}\n${job.logs.join('\n')}'); + final twin = await WorkerHarness.runJob(twin4444a.job('enc_twin').toJson(), + label: 'encode twin 4444a'); + expect(twin.success, isTrue, reason: '${twin.error}\n${twin.logs.join('\n')}'); + expect(await _framesMd5(job.outputPath!), await _framesMd5(twin.outputPath!)); + }); +} diff --git a/docs/ENGINEERING_NOTES.md b/docs/ENGINEERING_NOTES.md index 936018b..0bbac5d 100644 --- a/docs/ENGINEERING_NOTES.md +++ b/docs/ENGINEERING_NOTES.md @@ -3256,6 +3256,36 @@ Full report with scripts: The mean alone would miss a one-frame patch (~0.03 points over 40 frames). - `pal-sd-25.mov` is static colour bars: useless for anything motion-related. +### Sources tagged `reserved`: clear invalid colour tags before converting (2026-11-10) + +Full report with scripts: +[docs/reports/2026-11-reserved-colour-tags/](reports/2026-11-reserved-colour-tags/index.html). + +- Reported on the videohelp forum with a ProRes 422 capture: no Original + frame, no thumbnails, Detect borders "No frames could be decoded." Its tags + are `color_primaries=reserved`, `color_transfer=reserved`. FFmpeg's ProRes + decoder reads code 0 (Apple's "unknown") as ISO 23091's `reserved`, and + FFmpeg 9's scaler refuses to convert such a frame + (`Unsupported input … prim:reserved trc:reserved`). +- Every decode that converts the pixel format failed: the app's before frame + and thumbnails, `--detect-crop`, `--detect-native` (not reported), and the + encode/preview of a ProRes 4444 with alpha (`yuva444p12le` is converted to + the pipe format). Decodes in the source's own format were fine. +- Fix: `setparams` sets each *invalid* tag (off `ColorMetadata`'s lists; + `gbr` counts as valid) to `unknown`, first in the decode's one `-vf` + (worker `decode_tag_neutraliser` via `source_decode::decoder_filter`; app + `ColorTags.decodeNeutraliser` via `PreviewGenerator.beforeFrameArgs` / + `thumbnailArgs`). The result is bit-identical to the same file untagged. +- Never clear a valid tag: on a bt709 source, yuv → gray16le with + `colorspace=unknown` differs by up to 28 levels (mean 15). That is also why + the job keeps the raw `reserved` (the app's `colorTag` forwards it). The + worker decides from it rather than clearing whatever `from_raw` dropped. +- Clearing only one of primaries/transfer is not enough when both are + reserved. +- Open: `--detect-native`'s gray16le decode depends on the matrix tag in any + case (the same 28 levels). `--detect-noise-halo` avoids it by reading the + luma plane of the source's own format. + ### Testing a deps change before publishing A PR that changes `deps/` has a chicken-and-egg problem: `ci-test.yml` and diff --git a/docs/reports/2026-11-reserved-colour-tags/index.html b/docs/reports/2026-11-reserved-colour-tags/index.html new file mode 100644 index 0000000..10df726 --- /dev/null +++ b/docs/reports/2026-11-reserved-colour-tags/index.html @@ -0,0 +1,229 @@ + + + + + +Reserved colour tags: before frame, thumbnails and Detect fail + + + +
+

Sources tagged with reserved colour primaries/transfer

+

2026-11-10 · reported on the videohelp forum (thread 419911, posts #123/#140/#150) with a +ProRes capture · measured on an M1 Pro (macOS arm64) against the deps 1.16.0 bundle (FFmpeg 9.0.2)

+ +
+Outcome. FFmpeg 9's scaler refuses to convert a frame tagged +color_primaries=reserved / color_transfer=reserved, and fails the whole +decode. That emptied the app's Original side ("No preview") and timeline and failed Detect borders, as +reported. It also failed Descale's Detect, and the encode and preview of a ProRes 4444 with alpha, +which nobody had reported. Every decode of the source now puts a setparams that sets +only the invalid tags to unknown first in its one -vf. The fixed +decodes are bit-identical to the same file untagged, and a source with usable tags decodes with +exactly the arguments it always had. Built. +
+ +

1. The source and the error

+

The clip is ProRes 422 (apcn), 720x486, yuv422p10le, BFF, 2997/100 fps, time base +1/11988, with pcm_s24le as stream 0 and video as stream 1. ffprobe reports +color_space=unknown, color_primaries=reserved, +color_transfer=reserved, color_range=tv. Any ffmpeg that converts its pixel +format stops with:

+
[swscaler] Unsupported input (Operation not supported): fmt:yuv422p10le csp:unknown prim:reserved trc:reserved -> fmt:yuv422p …
+ +

Where reserved comes from

+

FFmpeg's ProRes decoder reads the header's colour fields as ISO 23091 codes, where 0 is +reserved and "unspecified" is 2. Apple's ProRes spec uses 0 for "unknown". So a ProRes +writer that marks colour as unknown the Apple way produces exactly these tags. FFmpeg's own +prores_metadata bitstream filter does it too: its unknown constant writes +code 0, and it refuses 2 and 3.

+ + + + + +
prores_metadata settingffprobe then reports
color_primaries=unknown:color_trc=unknownprim reserved, trc reserved (the reported clip's tags)
color_primaries=2 / =3refused ("not a valid value")
colorspace=0matrix gbr (on a YUV stream)
+

H.264 is not affected in the same way. Its decoder maps reserved codes to unknown itself: +h264_metadata with primaries/transfer 0 or 3 probed as unknown and converted +fine. The exception is matrix 15, ipt-c2 (Dolby Vision profile 5), which fails with +"Impossible to convert". The fixture and the tests are therefore ProRes.

+ +

2. Which decodes fail

+

From scripts/probe_tag_rules.py on the reported clip, each call site's own arguments +without and with setparams=color_primaries=unknown:color_trc=unknown at the front of its +filter chain. Exit 211 is ffmpeg's failure.

+ + + + + + + + +
DecodeConverts?Before the fixWith the neutraliser
App before frame (PreviewGenerator.getFrameAtIndex, JPEG)yesexit 211OK
App timeline thumbnail (scale=160:-1)yesexit 211OK
Worker --detect-crop (yuv444p)yesexit 211OK
Worker --detect-native (gray16le). Not in the original report.yesexit 211OK
Worker preview / encode decoder, --detect-noise-halo (the source's own format)noOKOK, raw bytes identical
App field-order analysis (idet, no -pix_fmt)noOKunchanged
+ +

The encode is affected too, when its decoder converts

+

The worker's decoders write the pipe format chosen by decode_pixel_format. That is the +source's own format, except when pipe_source cannot read it. ProRes 4444 with alpha +decodes as yuva444p12le, which is converted to yuv444p12le. With reserved tags:

+ + + + +
Encode-decoder conversion yuva444p12le → yuv444p12leResult
as beforeUnsupported input
with the neutraliserOK, bit-identical to the same file untagged
+

So the neutraliser belongs in all three source_decode builders (encode, preview, VFR +timeline), and through them in every detector. It is not only for the detectors. The heavy test checks +that the encode and preview of such a clip fail on the old worker and match an untagged twin on the new one.

+ +

3. What the neutraliser must and must not touch

+

From scripts/probe_tag_rules.py, on testsrc2 ProRes fixtures it builds itself:

+ + + + + + + + + + + + +
MeasurementResult
Reserved primaries + transfer, clear only primariesstill fails (prim:unknown trc:reserved)
… clear only transferstill fails
… clear bothOK
Fixed decode vs the untagged original: yuv444p, gray16le, rgb24identical, all three
bt709 matrix + reserved primaries/transfer, matrix kept, rgb24identical to the untagged file read as 709
A valid bt709 source: clear primaries + transfer (yuv444p, gray16le)identical
A valid bt709 source: clear the matrix, yuv444pidentical
A valid bt709 source: clear the matrix, gray16lediffers: max 27.8 8-bit levels, mean 15.2
RGB source (FFV1 gbrp, probed matrix gbr): clear the matrixidentical (yuv444p, rgb24)
-colorspace gbr as an optionrejected; FFmpeg's option name is rgb
+

Hence the rule:

+
    +
  • Per tag, only where it is invalid, meaning not on the lists +ColorMetadata::from_raw already uses. Untagged values (unknown, +N/A, empty) need nothing. A blanket "set everything to unknown" would change Descale +Detect's input on every tagged source, by up to 28 levels. With an unknown matrix, swscale's yuv→gray +is an exact limited→full expansion; with bt709 it is something else.
    • +
  • gbr counts as valid. It is ffprobe's name for the RGB identity +matrix, swscale handles it, and clearing it changed nothing. Leaving it alone keeps RGB sources' +arguments unchanged.
    • +
  • ipt-c2 is cleared. swscale cannot convert it, and a Dolby Vision +profile 5 picture is warned about, never blocked. With a PQ transfer, the HDR tone-map's own leading +setparams overrides the matrix anyway.
    • +
+ +

4. Decisions

+
    +
  • The raw tag stays in the job. The task first proposed making the app's +colorTag() drop reserved. It doesn't. The decoder reads the tag from the +stream whatever the job says, so the worker has to know the stream's tag is invalid in order to clear +it, and the job is the only place it can learn that without its own ffprobe (which would cost every +preview render). Clearing unconditionally whatever from_raw didn't keep was also rejected +(section 3: it changes gray16le pixels when the job lacks a valid tag the file has). The encoder's tags +still drop reserved, in from_raw.
    • +
  • One decision on each side, byte-identical. In the worker it is +color_metadata::decode_tag_neutraliser (via VideoJob::decode_tag_neutraliser), +placed by source_decode::decoder_filter ahead of the size guard. In the app it is +ColorTags.decodeNeutraliser, placed by PreviewGenerator.beforeFrameArgs / +thumbnailArgs ahead of scale and the HDR tone map. +color_tags_test.dart parses the Rust lists from source and repeats the Rust case table.
    • +
  • Unchanged arguments for usable tags. No tag fix means the -vf is +exactly the size guard, as before (asserted in source_decode.rs).
    • +
+ +

5. The reported clip, after the fix

+

From scripts/real_clip_worker.py, with the job built as the app builds it (ffprobe's raw +tags through colorTag). No frame of the clip is reproduced here or committed: it is +the reporter's footage.

+ + + + + + + + +
RunResult
--detect-cropOK, 1.1 s: crop left 14, right 8, top 0, bottom 0; 24/24 frames usable
--detect-nativeOK, 4.0 s: no clear native resolution (dip 0.75, so not upscaled)
--detect-noise-haloOK, 2.7 s: noise 0.0, no suggestion (see below)
Preview, frame 300OK, 0.3 s
Trimmed encode, frames 100–129 (FFV1)OK, 30 frames
Before frame and thumbnail, the app's arguments with the fixOK (section 2)
+

Noted, no action: --detect-noise-halo reports noise 0.0 on this clip +under any tagging. It is telecined film, and the 3:2 pulldown's repeated fields match each other +exactly, which a motion-compensated difference reads as noiseless. That only withholds a suggestion, +which matches the documented rule that a low score is silent.

+

The forum report also described the Processed side as showing no difference. Part of that was the +one-frame seek offset, a separate bug with its own fix (seek alignment).

+ +

6. Tests

+
    +
  • Fixtures (Tests/TestResources/generate_reserved_colour_tags_test.sh, +stream copy only): reserved_colour_tags.mov is 15 frames of +prores422_10bit_telecine.mov laid out like the reported clip (audio first, 1/11988 time +base), 1.7 MB. reserved_colour_tags_4444a.mov is ProRes 4444 with alpha, 320x240, +10 frames, 0.55 MB. The script fails unless ffprobe reports reserved,reserved.
    • +
  • Heavy (integration_reserved_colour_tags_test.dart): every check +compares against an untagged twin, which is the fixture stream-copied with bt709 primaries/transfer and +the matrix left unknown. The checks: before frame and thumbnail fail without the fix (a guard that the +fixture still reproduces the bug) and with it equal the twin's picture (MAD 0); each of the three +detectors' reports equals the twin's; preview of both fixtures equals the twin's (MAD 0); the 4444a +encode's frames equal the twin's (MD5). +
    Run against the unfixed worker: --detect-crop, --detect-native, +the preview and the encode tests fail, and --detect-noise-halo +passes (no conversion, as predicted). With the fix, all 8 pass.
    • +
  • Unit: the case table and "never touches an accepted value" sweep on both sides; +source_decode keeps one -vf with the fix first, and is unchanged without +one; the Dart lists equal the Rust source's; argument placement with the HDR tone map and with DV5 +(ipt-c2 + PQ).
    • +
+ +

7. Limits and what wasn't tested

+
    +
  • Measured on macOS arm64 with FFmpeg 9.0.2 only. The scaler is shared code and FFmpeg is pinned to +the 9.0 series everywhere, but the nightly heavy run is what checks Windows and Linux.
    • +
  • One real clip. Other writers that produce reserved (a MOV colr atom on +another codec, for example) were not found or tested. The rule does not depend on the codec.
    • +
  • No app screenshot. Every result here is an exit code or a bit-exact comparison, and none is a visual judgement.
    • +
  • An invalid color_range (ffprobe reports only tv/pc/unknown) and tags that change +mid-stream away from the probed ones are not handled.
    • +
  • Found in passing, not fixed here: --detect-native decodes gray16le +through swscale, so Descale Detect's luma depends on the matrix tag (section 3: up to 28 levels between +a bt709-tagged and an untagged copy of the same picture). --detect-noise-halo avoids this +by decoding the source's own format and taking its luma plane. This fix does not make it worse: a +valid tag is never cleared.
    • +
+ +

Scripts

+
    +
  • scripts/run.sh: runs both scripts below. Set DEPS, SCRATCH and, for section 5, CLIP.
    • +
  • scripts/probe_tag_rules.py: sections 1–3, building every fixture from testsrc2; with a clip, the call-site table.
    • +
  • scripts/real_clip_worker.py: section 5, with the worker on a real clip.
    • +
+
+ + diff --git a/docs/reports/2026-11-reserved-colour-tags/scripts/probe_tag_rules.py b/docs/reports/2026-11-reserved-colour-tags/scripts/probe_tag_rules.py new file mode 100755 index 0000000..9569b36 --- /dev/null +++ b/docs/reports/2026-11-reserved-colour-tags/scripts/probe_tag_rules.py @@ -0,0 +1,150 @@ +#!/usr/bin/env python3 +"""Which decodes fail on `reserved` colour tags, and which neutraliser fixes +them without changing pixels. + +Usage: python3 -I probe_tag_rules.py [real_clip.mov] + +Builds every fixture it needs in from ffmpeg's testsrc2 (no repo +files), prints one line per measurement. With a real clip, also runs the +call-site table on it (its own size is read with ffprobe). +""" +import json, os, struct, subprocess, sys + +FF, FP, WD = sys.argv[1:4] +CLIP = sys.argv[4] if len(sys.argv) > 4 else None +os.makedirs(WD, exist_ok=True) +N2 = "setparams=color_primaries=unknown:color_trc=unknown" +N3 = N2 + ":colorspace=unknown" + + +def ff(*args): + r = subprocess.run([FF, "-v", "error", "-y", *args], capture_output=True, text=True) + return r.returncode, r.stderr.strip().splitlines()[0][:110] if r.stderr.strip() else "" + + +def w(name): + return os.path.join(WD, name) + + +def tags(path): + out = subprocess.run([FP, "-v", "error", "-select_streams", "v:0", "-show_entries", + "stream=pix_fmt,width,height,color_space,color_primaries,color_transfer", + "-of", "json", path], capture_output=True, text=True).stdout + return json.loads(out)["streams"][0] + + +def raw(path, fmt, vf=None, frames=1, ss=None): + out = w("probe.raw") + if os.path.exists(out): + os.remove(out) + args = (["-ss", str(ss)] if ss else []) + ["-apply_cropping", "codec", "-i", path, + "-map", "0:v:0", "-frames:v", str(frames)] + if vf: + args += ["-vf", vf] + code, err = ff(*args, "-f", "rawvideo", "-pix_fmt", fmt, out) + return (open(out, "rb").read() if code == 0 and os.path.exists(out) else None), err + + +def retag(src, dst, spec): + return ff("-i", src, "-map", "0", "-c", "copy", "-bsf:v", "prores_metadata=" + spec, dst) + + +print("== FFmpeg:", subprocess.run([FF, "-version"], capture_output=True, text=True).stdout.splitlines()[0]) + +# Fixtures. +ff("-f", "lavfi", "-i", "testsrc2=s=720x486:r=30000/1001:d=0.2", "-c:v", "prores_ks", + "-profile:v", "2", "-pix_fmt", "yuv422p10le", w("base.mov")) +ff("-f", "lavfi", "-i", "testsrc2=s=320x240:r=25:d=0.2", "-c:v", "prores_ks", + "-profile:v", "4", "-pix_fmt", "yuva444p10le", w("base4444a.mov")) + +print("\n== prores_metadata BSF values -> what ffprobe reports") +for spec in ["color_primaries=unknown:color_trc=unknown", "color_primaries=2", "color_primaries=3", + "colorspace=0", "colorspace=1:color_primaries=unknown:color_trc=unknown"]: + dst = w("bsf.mov") + code, err = retag(w("base.mov"), dst, spec) + t = tags(dst) if code == 0 else {} + print(f" {spec:55} -> " + (f"csp={t.get('color_space')} prim={t.get('color_primaries')} trc={t.get('color_transfer')}" + if code == 0 else f"refused ({err})")) + +retag(w("base.mov"), w("reserved.mov"), "color_primaries=unknown:color_trc=unknown") +retag(w("base.mov"), w("v709.mov"), "colorspace=bt709:color_primaries=bt709:color_trc=bt709") +retag(w("base.mov"), w("m709_reserved.mov"), "colorspace=bt709:color_primaries=unknown:color_trc=unknown") +retag(w("base4444a.mov"), w("reserved4444a.mov"), "color_primaries=unknown:color_trc=unknown") + +print("\n== Which neutraliser is enough (reserved primaries + transfer, yuv422p10le -> rgb24)") +for vf in [None, "setparams=color_primaries=unknown", "setparams=color_trc=unknown", N2, N3]: + data, err = raw(w("reserved.mov"), "rgb24", vf) + print(f" {str(vf):70} -> {'OK' if data else 'FAILS: ' + err}") + +print("\n== Fixed decode vs the untagged original (bit-exact?)") +for fmt in ["yuv444p", "gray16le", "rgb24"]: + a, _ = raw(w("reserved.mov"), fmt, N2) + b, _ = raw(w("base.mov"), fmt) + print(f" {fmt:9} identical: {a == b}") + +print("\n== A valid matrix must be kept: bt709 matrix, reserved prim/trc, rgb24") +kept, _ = raw(w("m709_reserved.mov"), "rgb24", N2) +ref709, _ = raw(w("base.mov"), "rgb24", "setparams=colorspace=bt709") +print(f" matrix kept == untagged read as 709: {kept == ref709}") + +print("\n== Clearing a VALID tag (bt709 everywhere): does the decode change?") +for fmt in ["yuv444p", "gray16le"]: + a, _ = raw(w("v709.mov"), fmt) + for vf in ["setparams=color_primaries=unknown:color_trc=unknown", "setparams=colorspace=unknown"]: + b, _ = raw(w("v709.mov"), fmt, vf) + if a == b: + print(f" {fmt:9} {vf:55} identical") + continue + if fmt == "gray16le": + A = struct.unpack(f"<{len(a)//2}H", a) + B = struct.unpack(f"<{len(b)//2}H", b) + d = [abs(x - y) for x, y in zip(A, B)] + print(f" {fmt:9} {vf:55} DIFFERS: max {max(d)}/65535 = {max(d)/257:.1f} 8-bit levels, " + f"mean {sum(d)/len(d)/257:.1f}") + else: + print(f" {fmt:9} {vf:55} DIFFERS") + +print("\n== ProRes 4444 with alpha: the encode decoder converts yuva444p12le -> yuv444p12le") +print(" probed:", tags(w("reserved4444a.mov"))["pix_fmt"]) +for vf in [None, N2]: + data, err = raw(w("reserved4444a.mov"), "yuv444p12le", vf, frames=3) + print(f" {str(vf):55} -> {'OK' if data else 'FAILS: ' + err}") +a, _ = raw(w("reserved4444a.mov"), "yuv444p12le", N2, frames=3) +b, _ = raw(w("base4444a.mov"), "yuv444p12le", None, frames=3) +print(f" fixed == untagged original: {a == b}") + +print("\n== gbr (ffprobe's RGB identity) is handled by swscale: clearing it changes nothing") +ff("-f", "lavfi", "-i", "testsrc2=s=320x240:r=25:d=0.2", "-c:v", "ffv1", "-pix_fmt", "gbrp", w("rgb.mkv")) +print(" probed matrix:", tags(w("rgb.mkv")).get("color_space")) +for fmt in ["yuv444p", "rgb24"]: + a, _ = raw(w("rgb.mkv"), fmt) + b, _ = raw(w("rgb.mkv"), fmt, "setparams=colorspace=unknown") + print(f" {fmt:8} identical: {a == b}") +code, err = ff("-f", "lavfi", "-i", "testsrc2=s=64x64:d=0.04", "-colorspace", "gbr", "-c:v", "ffv1", w("x.mkv")) +print(f" -colorspace gbr accepted as an option: {code == 0} ({err})") + +if CLIP: + t = tags(CLIP) + W, H = t["width"], t["height"] + guard = f"crop=w='if(eq(iw\\,{W})\\,iw\\,0)':h='if(eq(ih\\,{H})\\,ih\\,0)':x=0:y=0:exact=1" + print(f"\n== Real clip ({W}x{H} {t['pix_fmt']} csp={t.get('color_space')} prim={t.get('color_primaries')} " + f"trc={t.get('color_transfer')}): each call site's decode, without and with the fix") + sites = [ + ("app before frame (JPEG)", ["-ss", "10", "-apply_cropping", "codec", "-i", CLIP, "-frames:v", "1"], + None, ["-q:v", "2", w("b.jpg")]), + ("app thumbnail (scale=160:-1)", ["-ss", "10", "-apply_cropping", "codec", "-i", CLIP, "-vframes", "1"], + "scale=160:-1", ["-q:v", "5", w("t.jpg")]), + ("--detect-crop (yuv444p)", ["-ss", "10", "-apply_cropping", "codec", "-i", CLIP, "-map", "0:v:0", "-frames:v", "1"], + guard, ["-f", "rawvideo", "-pix_fmt", "yuv444p", w("c.raw")]), + ("--detect-native (gray16le)", ["-ss", "10", "-apply_cropping", "codec", "-i", CLIP, "-map", "0:v:0", "-frames:v", "1"], + guard, ["-f", "rawvideo", "-pix_fmt", "gray16le", w("n.raw")]), + ("preview/encode/noise-halo (own format)", ["-ss", "10", "-apply_cropping", "codec", "-i", CLIP, "-map", "0:v:0", "-frames:v", "2"], + guard, ["-f", "rawvideo", "-pix_fmt", t["pix_fmt"], w("p.raw")]), + ] + for name, pre, vf, post in sites: + res = [] + for fix in [None, N2]: + chain = ",".join(x for x in [fix, vf] if x) + code, err = ff(*pre, *(["-vf", chain] if chain else []), *post) + res.append("OK" if code == 0 else f"exit {code}") + print(f" {name:42} today: {res[0]:9} with fix: {res[1]}") diff --git a/docs/reports/2026-11-reserved-colour-tags/scripts/real_clip_worker.py b/docs/reports/2026-11-reserved-colour-tags/scripts/real_clip_worker.py new file mode 100755 index 0000000..5c19715 --- /dev/null +++ b/docs/reports/2026-11-reserved-colour-tags/scripts/real_clip_worker.py @@ -0,0 +1,100 @@ +#!/usr/bin/env python3 +"""Run the worker's detectors, a preview and a trimmed encode on a real clip, +the way the app does (job JSON carrying ffprobe's raw colour tags). + +Usage: python3 -I real_clip_worker.py +macOS/Linux environment only (mirrors WorkerHarness.workerEnv). +""" +import json, os, subprocess, sys, time, uuid + +WORKER, DEPS, CLIP, OUT = sys.argv[1:5] +os.makedirs(OUT, exist_ok=True) +FF = os.path.join(DEPS, "ffmpeg", "ffmpeg") +FP = os.path.join(DEPS, "ffmpeg", "ffprobe") + +env = dict(os.environ) +env.update({ + "PYTHONHOME": os.path.join(DEPS, "python"), + "PYTHONPATH": DEPS + ":" + os.path.join(DEPS, "python-packages"), + "PYTHONNOUSERSITE": "1", + "XDG_CONFIG_HOME": os.path.join(DEPS, "config"), + "PATH": ":".join([os.path.join(DEPS, "python", "bin"), os.path.join(DEPS, "vapoursynth"), + os.path.join(DEPS, "ffmpeg"), env.get("PATH", "")]), + "DYLD_LIBRARY_PATH": os.path.join(DEPS, "vapoursynth") + ":" + os.path.join(DEPS, "python", "lib"), + "LD_LIBRARY_PATH": ":".join([os.path.join(DEPS, "vapoursynth"), os.path.join(DEPS, "python", "lib"), + os.path.join(DEPS, "lib")]), +}) + +s = json.loads(subprocess.run([FP, "-v", "error", "-select_streams", "v:0", "-show_streams", "-of", "json", CLIP], + capture_output=True, text=True).stdout)["streams"][0] + + +def tag(k): + v = (s.get(k) or "").strip() + return None if v in ("", "unknown", "N/A") else v # the app's colorTag + + +num, den = map(int, s["avg_frame_rate"].split("/")) +fps = num / den +frames = int(s.get("nb_frames") or round(float(s["duration"]) * fps)) +print(f"clip: {s['codec_tag_string']} {s['width']}x{s['height']} {s['pix_fmt']} {s.get('field_order')} " + f"{s['avg_frame_rate']} tb {s['time_base']} frames {frames}; csp={tag('color_space')} " + f"prim={tag('color_primaries')} trc={tag('color_transfer')} range={tag('color_range')}") + + +def job(**extra): + j = { + "id": str(uuid.uuid4()), "inputPath": CLIP, "outputPath": os.path.join(OUT, "unused.mkv"), + "qtgmcParameters": {"enabled": False}, + "processingPipeline": {"deinterlace": {"enabled": False}}, + "encodingSettings": {"codec": "ffv1", "container": "mkv", "audioMode": "none"}, + "detectedFieldOrder": "bff" if s.get("field_order") in ("bt", "bb") else "tff", + "totalFrames": frames, "inputFrameRate": fps, "inputWidth": s["width"], "inputHeight": s["height"], + "inputPixelFormat": s["pix_fmt"], + "inputColorMatrix": tag("color_space"), "inputColorPrimaries": tag("color_primaries"), + "inputColorTransfer": tag("color_transfer"), "inputColorRange": tag("color_range"), + } + j.update(extra) + path = os.path.join(OUT, j["id"] + ".json") + json.dump(j, open(path, "w")) + return path + + +def run(label, args, stdout_file=None): + t = time.time() + r = subprocess.run([WORKER, *args], env=env, capture_output=True, + cwd=os.path.dirname(WORKER)) + dt = time.time() - t + out = r.stdout + if stdout_file: + open(stdout_file, "wb").write(out) + return r.returncode, out.decode(errors="replace"), r.stderr.decode(errors="replace"), dt + + +for flag in ["--detect-crop", "--detect-native", "--detect-noise-halo"]: + code, out, err, dt = run(flag, [flag, "--config", job()]) + lines = [l for l in out.splitlines() if l.strip().startswith("{")] + rep = json.loads(lines[-1]) if lines else {} + if rep.get("error") or code: + print(f"{flag:20} FAILED ({dt:.1f} s): {rep.get('error') or err[-300:]}") + continue + if flag == "--detect-crop": + summary = f"crop {rep.get('crop')} usable {rep.get('usableFrames')}/{len(rep.get('frames', []))}" + elif flag == "--detect-native": + summary = f"found {rep.get('found')} height {rep.get('height')} kernel {rep.get('kernel')} dip {rep.get('dip')}" + else: + summary = ", ".join(f"{k} {v}" for k, v in rep.items() if k in ("noise", "noiseHigh", "halo", "haloHigh", "usablePairs")) + print(f"{flag:20} OK ({dt:.1f} s): {summary}") + +png = os.path.join(OUT, "preview_300.png") +code, out, err, dt = run("preview", ["--config", job(), "--preview", "--frame", "300"], stdout_file=png) +print(f"{'preview frame 300':20} {'OK' if code == 0 and os.path.getsize(png) > 0 else 'FAILED'} ({dt:.1f} s)" + + ("" if code == 0 else ": " + err[-300:])) + +enc = os.path.join(OUT, "trimmed_100_129.mkv") +code, out, err, dt = run("encode", ["--config", job(outputPath=enc, startFrame=100, endFrame=129, totalFrames=30)]) # totalFrames is the trimmed count, as the app sends +ok = code == 0 and os.path.exists(enc) +n = subprocess.run([FP, "-v", "error", "-count_frames", "-select_streams", "v:0", "-show_entries", + "stream=nb_read_frames", "-of", "csv=p=0", enc], capture_output=True, text=True).stdout.strip() if ok else "-" +print(f"{'trimmed encode':20} {'OK' if ok else 'FAILED'} ({dt:.1f} s), {n} frames" + + ("" if ok else ": " + (out + err)[-400:])) diff --git a/docs/reports/2026-11-reserved-colour-tags/scripts/run.sh b/docs/reports/2026-11-reserved-colour-tags/scripts/run.sh new file mode 100755 index 0000000..d71067f --- /dev/null +++ b/docs/reports/2026-11-reserved-colour-tags/scripts/run.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# Reproduce every number in the report. +# +# DEPS= dir> default: the repo's deps/ (macOS arm64 / Linux x64) +# SCRATCH= default: a fresh temp dir +# CLIP= optional: adds the real-clip runs +# +# The worker must be built first (cd worker && cargo build). +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +REPO="$(cd "$HERE/../../../.." && pwd)" +case "$(uname -s)-$(uname -m)" in + Darwin-arm64) PLATFORM=macos-arm64 ;; Darwin-x86_64) PLATFORM=macos-x64 ;; + Linux-x86_64) PLATFORM=linux-x64 ;; Linux-aarch64) PLATFORM=linux-arm64 ;; +esac +DEPS="${DEPS:-$REPO/deps/$PLATFORM}" +SCRATCH="${SCRATCH:-$(mktemp -d)}" +WORKER="$REPO/worker/target/debug/vapourbox-worker" + +python3 -I "$HERE/probe_tag_rules.py" "$DEPS/ffmpeg/ffmpeg" "$DEPS/ffmpeg/ffprobe" \ + "$SCRATCH/rules" ${CLIP:+"$CLIP"} +if [ -n "${CLIP:-}" ]; then + python3 -I "$HERE/real_clip_worker.py" "$WORKER" "$DEPS" "$CLIP" "$SCRATCH/real" +fi diff --git a/docs/reports/README.md b/docs/reports/README.md index 533e59c..2690233 100644 --- a/docs/reports/README.md +++ b/docs/reports/README.md @@ -66,6 +66,7 @@ to it. | Date | Report | Outcome | |---|---|---| +| 2026-11-10 | [Sources tagged with `reserved` colour primaries/transfer](2026-11-reserved-colour-tags/index.html) | Fixed: FFmpeg 9's scaler refuses ProRes's `reserved` tags (code 0, Apple's "unknown"), which failed the before frame, thumbnails, Detect borders, Descale Detect and a 4444-alpha encode; every converting decode now clears only the invalid tags first (bit-identical to the untagged file). Clearing a valid matrix was rejected: it moves yuv→gray16le by up to 28 levels | | 2026-11-10 | [Issue #148: Frame Rate combined with Noise Reduction and Sharpen](issue-148-frame-rate-combined/index.html) | Not reproduced on macOS arm64 across 5 denoisers x 4 sharpeners x FlowFPS/RIFE x 8/10-bit; no pipeline change; nightly regression test added to cover Windows/MVTools v24 and other GPUs; reporter asked for a clip | | 2026-11-09 | [Noise and halo scores for the pass list](2026-11-noise-halo-scores/index.html) | Built: `--detect-noise-halo` badges Noise Reduction at a motion-compensated noise sigma ≥ 3.5 and Dehalo at a two-sided edge overshoot ≥ 6% of the step; no false positives on 154 fixtures (clean max 2.06 / 1.48), catches white noise from sigma 4 and tape/grain noise from sigma 6, misses sharpening of already-soft edges. VCG's signalstats TOUT and halo ratio did not separate the fixtures; a spatial (Immerkær) noise score was rejected as blind to tape and grain noise | | 2026-11-08 | [Crop & Resize: BT.601 → BT.709 on SD → HD, and Detect borders](2026-11-crop-detect-709/index.html) | Built: matrix-only 601 → 709 conversion when the output is HD (default Auto; patches within 0.67 levels of the source; a gamut conversion rejected: it moved 75% green by 55 levels on players without colour management); `--detect-crop` finds blanking, bars and the head-switching band exactly on real-picture fixtures, and matches a by-eye measurement of a DVB-T broadcast | diff --git a/worker/src/crop_detect.rs b/worker/src/crop_detect.rs index d4b54b9..bb5b4e8 100644 --- a/worker/src/crop_detect.rs +++ b/worker/src/crop_detect.rs @@ -871,12 +871,15 @@ pub fn decode_samples( let env = deps.build_environment(); let (width, height) = frame_size(job)?; let fps = job.input_frame_rate.filter(|f| *f > 0.0).unwrap_or(29.97); + // A `reserved` colour tag fails the conversion to `pix_fmt` outright. + let tag_fix = job.decode_tag_neutraliser(); let decode = |frame: i32| -> Result { let seek = ((frame as f64 - 0.5) / fps).max(0.0); let out = Command::new(&ffmpeg) .args(source_decode::preview_decoder_args( &job.input_path, seek, count, width, height, pix_fmt, "pipe:1", + tag_fix.as_deref(), )) .envs(&env) .stdin(Stdio::null()) diff --git a/worker/src/models/color_metadata.rs b/worker/src/models/color_metadata.rs index 8f4459b..6ddcd88 100644 --- a/worker/src/models/color_metadata.rs +++ b/worker/src/models/color_metadata.rs @@ -73,6 +73,61 @@ fn clean(value: Option<&str>, allowed: &[&str]) -> Option { allowed.contains(&v.as_str()).then_some(v) } +/// Whether a raw ffprobe tag is present but not a value FFmpeg accepts. +/// Untagged (`unknown`, `N/A`, empty, absent) is not invalid: there is +/// nothing to neutralise. +fn is_invalid(value: Option<&str>, allowed: &[&str], also_valid: &[&str]) -> bool { + let Some(v) = value.map(|v| v.trim().to_ascii_lowercase()) else { + return false; + }; + if v.is_empty() || v == "unknown" || v == "n/a" { + return false; + } + !allowed.contains(&v.as_str()) && !also_valid.contains(&v.as_str()) +} + +/// The `setparams` that clears a source's INVALID colour tags before anything +/// converts its pixel format, or `None` when every tag is usable. +/// +/// FFmpeg 9's scaler refuses to convert a frame tagged with a value it has no +/// meaning for — `[swscaler] Unsupported input … prim:reserved trc:reserved` — +/// and fails the whole decode. ProRes is where this happens: Apple's ProRes +/// spec writes code 0 for "unknown", which FFmpeg reads as ISO 23091's +/// `reserved` (so FFmpeg's own `prores_metadata=color_primaries=unknown` +/// produces such a file). Every decode of the source that may convert its +/// pixel format puts this first in its one `-vf`: the app's "before" frame and +/// thumbnails (`ColorTags.decodeNeutraliser`, byte-identical), the detectors, +/// and the encode/preview decoders (a ProRes 4444 with alpha, `yuva444p12le`, +/// is converted to the pipe format there). +/// +/// **Only an invalid tag is touched, each on its own.** Clearing a VALID matrix +/// changes pixels: `yuv → gray16le` on a bt709-tagged picture differs by up to +/// ~28 8-bit levels from the same picture with `colorspace=unknown`. Clearing +/// only the primaries when the transfer is also reserved still fails. The +/// measurements are in docs/reports/2026-11-reserved-colour-tags. +/// +/// `gbr` is ffprobe's name for the RGB identity matrix (`-colorspace` spells it +/// `rgb`, hence `MATRICES`); swscale handles it, so it is left alone. +/// `ipt-c2` (Dolby Vision profile 5) is cleared: swscale cannot convert it, and +/// a DV5 picture is warned about, never blocked. +pub fn decode_tag_neutraliser( + matrix: Option<&str>, + primaries: Option<&str>, + transfer: Option<&str>, +) -> Option { + let mut params = Vec::new(); + if is_invalid(primaries, PRIMARIES, &[]) { + params.push("color_primaries=unknown"); + } + if is_invalid(transfer, TRANSFERS, &[]) { + params.push("color_trc=unknown"); + } + if is_invalid(matrix, MATRICES, &["gbr"]) { + params.push("colorspace=unknown"); + } + (!params.is_empty()).then(|| format!("setparams={}", params.join(":"))) +} + /// The four colour tags, as read from the source and re-declared on the output. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct ColorMetadata { @@ -338,6 +393,58 @@ mod tests { assert!(ColorMetadata::from_raw(None, None, None, None).is_empty()); } + /// The case table `color_tags_test.dart` repeats for `ColorTags. + /// decodeNeutraliser` — change both together. + #[test] + fn decode_tag_neutraliser_cases() { + let n = decode_tag_neutraliser; + assert_eq!(n(None, None, None), None); + assert_eq!(n(Some("bt709"), Some("bt709"), Some("bt709")), None); + assert_eq!(n(Some("unknown"), Some("unknown"), Some("unknown")), None); + assert_eq!(n(Some("N/A"), Some(""), Some(" ")), None); + // The reported ProRes capture's tags. + assert_eq!( + n(Some("unknown"), Some("reserved"), Some("reserved")).as_deref(), + Some("setparams=color_primaries=unknown:color_trc=unknown") + ); + // Each tag on its own; a valid matrix is kept. + assert_eq!( + n(Some("bt709"), Some("reserved"), None).as_deref(), + Some("setparams=color_primaries=unknown") + ); + assert_eq!( + n(None, Some("smpte170m"), Some("Reserved ")).as_deref(), + Some("setparams=color_trc=unknown") + ); + for m in ["reserved", "ipt-c2", "ycgco-re"] { + assert_eq!( + n(Some(m), None, None).as_deref(), + Some("setparams=colorspace=unknown"), + "{m}" + ); + } + assert_eq!(n(Some("gbr"), None, None), None); + assert_eq!( + n(Some("reserved"), Some("reserved"), Some("reserved")).as_deref(), + Some("setparams=color_primaries=unknown:color_trc=unknown:colorspace=unknown") + ); + } + + /// A tag `from_raw` keeps must never be cleared: a valid matrix changes + /// pixels when it is (see `decode_tag_neutraliser`). + #[test] + fn decode_tag_neutraliser_never_touches_an_accepted_value() { + for m in MATRICES { + assert_eq!(decode_tag_neutraliser(Some(m), None, None), None, "{m}"); + } + for p in PRIMARIES { + assert_eq!(decode_tag_neutraliser(None, Some(p), None), None, "{p}"); + } + for t in TRANSFERS { + assert_eq!(decode_tag_neutraliser(None, None, Some(t)), None, "{t}"); + } + } + #[test] fn unrecognised_values_are_dropped_not_forwarded() { // A value we do not know must never reach ffmpeg, or it fails the whole diff --git a/worker/src/models/video_job.rs b/worker/src/models/video_job.rs index e32264c..dec7511 100644 --- a/worker/src/models/video_job.rs +++ b/worker/src/models/video_job.rs @@ -110,6 +110,19 @@ impl VideoJob { ) } + /// The `setparams` every decode of the source puts first in its `-vf`, or + /// `None` when the source's tags are all usable. Read from the job's RAW + /// tags: the app forwards ffprobe's strings (`reserved` included) for + /// exactly this, since the decoder reads the tag from the stream itself. + /// See `color_metadata::decode_tag_neutraliser`. + pub fn decode_tag_neutraliser(&self) -> Option { + super::color_metadata::decode_tag_neutraliser( + self.input_color_matrix.as_deref(), + self.input_color_primaries.as_deref(), + self.input_color_transfer.as_deref(), + ) + } + /// The frame size the pipeline delivers, as far as the worker can tell /// before the script runs: the source, turned by Rotate / Flip, then /// Crop & Resize (`CropResizeParameters::output_size`). `None` when the diff --git a/worker/src/native_res.rs b/worker/src/native_res.rs index 062f4b1..765d1ce 100644 --- a/worker/src/native_res.rs +++ b/worker/src/native_res.rs @@ -195,6 +195,9 @@ pub fn detect(deps: &DependencyLocator, job: &VideoJob) -> Result Result { @@ -211,6 +214,7 @@ pub fn detect(deps: &DependencyLocator, job: &VideoJob) -> Result source_decode::timeline_decoder_args( &job.input_path, @@ -346,6 +349,7 @@ impl PipelineExecutor { pix_fmt, &t.window_for_trim(job.start_frame, job.end_frame), "pipe:1", + tag_fix.as_deref(), ), None => source_decode::encode_decoder_args( &job.input_path, @@ -356,6 +360,7 @@ impl PipelineExecutor { job.start_frame, job.end_frame, job.total_frames, + tag_fix.as_deref(), ), }; @@ -1390,6 +1395,7 @@ impl PipelineExecutor { // Same decode as the encode path: the full stored frame at exactly the // probed size, never resampled — see source_decode.rs. + let tag_fix = job.decode_tag_neutraliser(); let decoder_args = match &window { Some(w) => source_decode::timeline_decoder_args( &job.input_path, @@ -1398,6 +1404,7 @@ impl PipelineExecutor { pix_fmt, w, raw_path.to_string_lossy().as_ref(), + tag_fix.as_deref(), ), None => source_decode::preview_decoder_args( &job.input_path, @@ -1407,6 +1414,7 @@ impl PipelineExecutor { height, pix_fmt, raw_path.to_string_lossy().as_ref(), + tag_fix.as_deref(), ), }; let extract_result = Command::new(&ffmpeg_path) diff --git a/worker/src/source_decode.rs b/worker/src/source_decode.rs index 6cfece0..923955f 100644 --- a/worker/src/source_decode.rs +++ b/worker/src/source_decode.rs @@ -27,7 +27,17 @@ //! no-op) and fails the decode on any other size, including a mid-stream //! resolution change, which with no guard at all would desync into garbage. //! -//! See docs/ENGINEERING_NOTES.md, "Clean aperture: decode the stored frame". +//! 3. **An invalid colour tag is cleared before anything converts.** FFmpeg 9's +//! scaler refuses a frame tagged `reserved` (ProRes writes code 0 for +//! "unknown", which FFmpeg reads as reserved), and the decoder converts +//! whenever the pipe format is not the source's own (ProRes 4444 with alpha, +//! `yuva444p12le`) or a detector asks for another format. Every builder +//! takes the job's `VideoJob::decode_tag_neutraliser` and puts it first in +//! its one `-vf` ([`decoder_filter`]); `None` leaves the args exactly as they +//! were. +//! +//! See docs/ENGINEERING_NOTES.md, "Clean aperture: decode the stored frame", +//! and docs/reports/2026-11-reserved-colour-tags. /// Input option placed before `-i` on every ffmpeg that decodes the source's /// pictures for the pipeline (encode decoder, preview decoder). See the module @@ -54,6 +64,17 @@ pub fn size_guard_filter(width: i32, height: i32) -> String { ) } +/// The decoder's single `-vf`: the colour-tag neutraliser, if the source needs +/// one, then the size guard. One chain, because ffmpeg honours only the last +/// `-vf`. +pub fn decoder_filter(width: i32, height: i32, tag_fix: Option<&str>) -> String { + let guard = size_guard_filter(width, height); + match tag_fix { + Some(fix) => format!("{fix},{guard}"), + None => guard, + } +} + /// If a failed decoder's stderr shows the size guard tripped, the error to /// report instead of ffmpeg's exit code. pub fn explain_decoder_failure(stderr: &str, width: i32, height: i32) -> Option { @@ -85,6 +106,7 @@ pub fn encode_decoder_args( start_frame: Option, end_frame: Option, total_frames: Option, + tag_fix: Option<&str>, ) -> Vec { let mut args: Vec = Vec::new(); @@ -105,7 +127,7 @@ pub fn encode_decoder_args( args.extend([ "-i".to_string(), input_path.to_string(), "-map".to_string(), "0:v:0".to_string(), // only first video stream - "-vf".to_string(), size_guard_filter(width, height), + "-vf".to_string(), decoder_filter(width, height, tag_fix), "-f".to_string(), "rawvideo".to_string(), "-pix_fmt".to_string(), pix_fmt.to_string(), "-v".to_string(), "error".to_string(), @@ -145,6 +167,7 @@ pub fn preview_decoder_args( height: i32, pix_fmt: &str, output_path: &str, + tag_fix: Option<&str>, ) -> Vec { let mut args: Vec = vec!["-ss".to_string(), format!("{:.6}", seek_time)]; args.extend(APPLY_CROPPING.iter().map(|s| s.to_string())); @@ -152,7 +175,7 @@ pub fn preview_decoder_args( "-i".to_string(), input_path.to_string(), "-map".to_string(), "0:v:0".to_string(), "-frames:v".to_string(), num_frames.to_string(), - "-vf".to_string(), size_guard_filter(width, height), + "-vf".to_string(), decoder_filter(width, height, tag_fix), "-f".to_string(), "rawvideo".to_string(), "-pix_fmt".to_string(), pix_fmt.to_string(), "-v".to_string(), "error".to_string(), @@ -177,6 +200,7 @@ pub fn timeline_decoder_args( pix_fmt: &str, window: &crate::source_timing::TimelineWindow, output: &str, + tag_fix: Option<&str>, ) -> Vec { let mut args: Vec = Vec::new(); if let Some(seek) = window.seek_time { @@ -187,7 +211,7 @@ pub fn timeline_decoder_args( args.extend([ "-i".to_string(), input_path.to_string(), "-map".to_string(), "0:v:0".to_string(), - "-vf".to_string(), size_guard_filter(width, height), + "-vf".to_string(), decoder_filter(width, height, tag_fix), "-fps_mode".to_string(), "passthrough".to_string(), "-frames:v".to_string(), window.src_count.to_string(), "-f".to_string(), "rawvideo".to_string(), @@ -204,11 +228,11 @@ mod tests { use super::*; fn encode_args() -> Vec { - encode_decoder_args("in.mov", 720, 576, "yuv422p10le", Some(25.0), None, None, Some(100)) + encode_decoder_args("in.mov", 720, 576, "yuv422p10le", Some(25.0), None, None, Some(100), None) } fn preview_args() -> Vec { - preview_decoder_args("in.mov", 0.38, 11, 720, 576, "yuv422p10le", "frames.raw") + preview_decoder_args("in.mov", 0.38, 11, 720, 576, "yuv422p10le", "frames.raw", None) } fn pos(args: &[String], flag: &str) -> Option { @@ -224,7 +248,7 @@ mod tests { } fn timeline_args() -> Vec { - timeline_decoder_args("in.mov", 720, 576, "yuv422p10le", &timeline_window(), "pipe:1") + timeline_decoder_args("in.mov", 720, 576, "yuv422p10le", &timeline_window(), "pipe:1", None) } /// Every decoder, so every assertion below covers every path. @@ -290,7 +314,7 @@ mod tests { #[test] fn encode_decoder_trims_and_limits_frames() { - let args = encode_decoder_args("in.mov", 720, 576, "yuv420p", Some(25.0), Some(10), Some(19), None); + let args = encode_decoder_args("in.mov", 720, 576, "yuv420p", Some(25.0), Some(10), Some(19), None, None); // Seek before -i, to the midpoint before the start frame. let ss = pos(&args, "-ss").unwrap(); assert!(ss < pos(&args, "-i").unwrap()); @@ -326,10 +350,42 @@ mod tests { assert_eq!(args[ss + 1], "1.250000"); let mut from_start = timeline_window(); from_start.seek_time = None; - let args = timeline_decoder_args("in.mov", 720, 576, "yuv420p", &from_start, "pipe:1"); + let args = timeline_decoder_args("in.mov", 720, 576, "yuv420p", &from_start, "pipe:1", None); assert!(pos(&args, "-ss").is_none()); } + /// Every builder with a tag fix: still one `-vf`, the fix ahead of the size + /// guard (the conversion happens after the chain, so the tags must already + /// be cleared), and an output option. + #[test] + fn decoders_clear_invalid_colour_tags_first() { + let fix = Some("setparams=color_primaries=unknown:color_trc=unknown"); + let all = [ + ("encode", encode_decoder_args("in.mov", 720, 576, "yuv444p12le", Some(25.0), Some(10), Some(19), None, fix)), + ("preview", preview_decoder_args("in.mov", 0.38, 11, 720, 576, "yuv444p12le", "frames.raw", fix)), + ("timeline", timeline_decoder_args("in.mov", 720, 576, "yuv444p12le", &timeline_window(), "pipe:1", fix)), + ]; + for (path, args) in all { + assert_eq!(args.iter().filter(|a| *a == "-vf").count(), 1, "{path}"); + let vf = pos(&args, "-vf").unwrap(); + assert!(vf > pos(&args, "-i").unwrap(), "{path}"); + assert_eq!( + args[vf + 1], + format!("setparams=color_primaries=unknown:color_trc=unknown,{}", size_guard_filter(720, 576)), + "{path}" + ); + } + } + + /// A source with usable tags decodes with exactly the args it always had. + #[test] + fn decoders_without_a_tag_fix_are_unchanged() { + assert_eq!(decoder_filter(720, 576, None), size_guard_filter(720, 576)); + for (path, args) in both() { + assert!(!args.iter().any(|a| a.contains("setparams")), "{path}: {args:?}"); + } + } + #[test] fn guard_failures_are_explained() { let stderr = "[Parsed_crop_0 @ 0x6000] Invalid too big or non positive size for width '0' or height '576'\n\