diff --git a/CLAUDE.md b/CLAUDE.md index b00333d..c79abd2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1390,6 +1390,62 @@ Subtitles' `burnInPath`. > either pass loses a real class of bug; both were verified against the actual > defects. +### Frame counts and geometry are verified against the pipeline, not the model + +`FrameMap` was covered only as arithmetic — `output_count`, `inverse`, +`total_radius` — which proves the model is self-consistent and says nothing +about whether the plugins agree with it. `run_job` in the Rust suite cannot +close that gap by design: it generates and inspects the `.vpy` and never runs +`vspipe | ffmpeg`. Two heavy Dart files now do (added 2026-08-20): + +- **`integration_frame_mapping_test.dart`** — encodes and *counts* for every + pass that changes the count: double-rate QTGMC (31 -> 62), single-rate as the + control (31 -> 31), IVTC cycle 5 (90 -> 72), and FlowFPS 25 -> 23.976 + (75 -> 71). Expected values are derived by hand from the FrameMap definitions + rather than read back from the model, so a change to the model cannot quietly + redefine "correct". The retime case is the one that pins why FlowFPS was + chosen over BlockFPS: `n*num/den` exactly, where BlockFPS gives + `floor((n-1)*r)+1` and every retimed job's progress total would be a frame out. +- **Preview/render correspondence** in the same file, and for geometry in + `integration_upscale_resize_test.dart`. `--frame N` is a **source** index (the + worker logs "Preview: source frame N") and the target's output index is + `output_count(local)`, so under double-rate the preview of source frame S must + be output frame 2S. Measured **0.00** mean abs diff at the right frame against + 8-30 at its neighbours, which is bit-exact; and 0.00 for crop, resize and + crop+resize. The geometry half also asserts the size, because a preview + rendered at a different resolution shows up as a buffer-length mismatch in + `meanAbsDiff` rather than as a pixel difference. +- **Crop pixels, not just crop size.** A size assertion cannot tell a correct + crop from one that swaps left with right - both give the requested + dimensions. The offset test crops asymmetrically and compares against the + pipeline's own uncropped output cropped in Dart, so the swapped offsets are + available as negative controls (measured 1.06 correct against 9.65 and 15.97). + +> **`totalFrames` is the POST-TRIM count, and getting that wrong looks like a +> filter bug.** Trimming is decoder-side (`-ss` + `-frames:v`) and +> `pipe_source` builds a fixed-length clip from `totalFrames`, so the number has +> to be what the decoder will actually pipe. The app sets it that way +> (`main_viewmodel.dart`: `effectiveEnd - effectiveStart + 1`) and `main.rs` +> computes the same thing when it is absent ("effective after trim"). Pass the +> **full** source length alongside a trim and the script declares a clip longer +> than the data: pipe_source hits EOF and repeats the last real frame to pad, so +> the output is full-length with a frozen tail. The frame-mapping test was +> written that way first and reported 150 frames for a 31-frame trim, which +> reads exactly like FPSDivisor being ignored. + +> **`select=eq(n,N)` on a source is not the pipeline's frame N.** +> `interlaced_test.avi` declares 79 frames and decodes 75: it carries null +> frames, and the decoder's default CFR mode expands them onto the 25 fps grid +> (measured: the first seven decoded frames come out 0,0,0,0,1,1,2). So `select` +> counts *decoded* frames while the pipeline counts *timeline* positions, and +> the two diverge wherever a null frame sits. That is correct behaviour — +> pipe_source needs a fixed-length CFR clip — but it makes ffmpeg-side frame +> indexing useless as a reference. Compare pipeline output against pipeline +> output, which uses one decoder and one indexing scheme. +> `WorkerHarness.frameRgb24` exists for that and decodes from the start rather +> than seeking, because an input seek lands on the nearest keyframe and would +> silently compare the wrong frame. + ### Suggestions and advice are hints, and must stay hints Two small pure-function models sit beside the pass list, and both are diff --git a/app/test/integration_frame_mapping_test.dart b/app/test/integration_frame_mapping_test.dart new file mode 100644 index 0000000..6216d7f --- /dev/null +++ b/app/test/integration_frame_mapping_test.dart @@ -0,0 +1,381 @@ +/// Frame mapping, verified against what actually comes out of the pipeline. +/// +/// `FrameMap` decides two things that are invisible until they are wrong: +/// `output_count` drives the progress total, and `inverse` decides which output +/// frame a preview shows for a given source frame. Both were covered only as +/// arithmetic — `FrameMap::Retime.output_count(97) == 116` and friends — which +/// proves the model is self-consistent and says nothing about whether the +/// plugins agree with it. +/// +/// That gap is exactly the failure this repo already documents for custom +/// VapourSynth: a real output length that differs from the declared one "makes +/// the progress bar lie *and* makes frame-accurate preview show a different +/// frame than its label, both silently". Custom code is guarded (the script +/// raises if `len(clip)` changed); the built-in passes that change the count on +/// purpose had no equivalent end-to-end check. +/// +/// The counts below are derived by hand from the FrameMap definitions rather +/// than read back from the model, so a change to the model cannot quietly +/// redefine what "correct" means: +/// +/// Fanout{factor:2} 75 -> 150, 31 -> 62 +/// Decimate{cycle:5,keep:4} 90 -> 72 +/// Retime 25 -> 24000/1001 ratio reduces to 960/1001, so 75 -> 71 +/// (75*960/1001 = 71.93, floored) +/// +/// Heavy (full encode + preview) — nightly, not the push gate. +@Tags(['heavy']) +library; + +// ignore_for_file: avoid_print — these tests print diagnostics to the test log. + +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:uuid/uuid.dart'; + +import 'package:vapourbox/models/encoding_settings.dart'; +import 'package:vapourbox/models/frame_rate_parameters.dart'; +import 'package:vapourbox/models/processing_pipeline.dart'; +import 'package:vapourbox/models/qtgmc_parameters.dart'; +import 'package:vapourbox/models/video_job.dart'; + +import 'support/worker_harness.dart'; + +String get _outDir => '${WorkerHarness.outputDir}/frame_mapping'; + +String get _interlaced => WorkerHarness.inputFile; // 720x576 yuv422p, 25 fps, 75 frames +String get _telecine => p.join( + WorkerHarness.repoRoot, 'Tests', 'TestResources', 'hard_telecine_test.avi'); + +/// The fixtures' real properties, asserted in `setUpAll` so a replaced fixture +/// fails naming itself rather than silently changing every expected count. +const _interlacedFrames = 75; +const _interlacedFps = 25.0; +const _telecineFrames = 90; + +VideoJob _job( + String name, { + required String input, + required ProcessingPipeline pipeline, + required int width, + required int height, + required String pixFmt, + required double fps, + required int totalFrames, + int? startFrame, + int? endFrame, + VideoCodec codec = VideoCodec.h264, +}) => + VideoJob( + id: const Uuid().v4(), + inputPath: input, + outputPath: '$_outDir/$name.mkv', + processingPipeline: pipeline, + encodingSettings: EncodingSettings( + codec: codec, + container: ContainerFormat.mkv, + audioMode: AudioMode.none, + ), + // Set explicitly rather than left to the worker's 720x480 / yuv420p + // fallbacks: the preview and the encode must read the source through the + // same geometry or the comparison below is meaningless. + // + // `totalFrames` is the count the decoder will actually pipe, i.e. the + // POST-TRIM count — trimming is decoder-side (`-ss` + `-frames:v`) and + // pipe_source builds a fixed-length clip from this number. The app sets + // it that way (`main_viewmodel.dart`: `effectiveEnd - effectiveStart + 1`) + // and the worker computes the same thing when it is absent + // (`main.rs`: "effective after trim"). + // + // Passing the full source length alongside a trim declares a clip longer + // than the data: pipe_source hits EOF and repeats the last real frame to + // pad, so the output is full-length with a frozen tail and every count + // here is wrong by the padding. That is not a hypothetical — this test + // was written that way first and reported 150 frames for a 31-frame trim. + inputWidth: width, + inputHeight: height, + inputPixelFormat: pixFmt, + inputFrameRate: fps, + totalFrames: totalFrames, + startFrame: startFrame, + endFrame: endFrame, + ); + +/// Frames actually present in a file, counted rather than read from a header. +Future _countFrames(String path) async { + final v = await WorkerHarness.firstStream(path, + selector: 'v:0', entries: ['nb_read_frames'], countFrames: true); + expect(v, isNotNull, reason: 'no video stream in $path'); + final n = int.tryParse(v!['nb_read_frames']?.toString() ?? ''); + expect(n, isNotNull, reason: 'ffprobe returned no frame count for $path'); + return n!; +} + +/// One frame of a video as packed rgb24, by absolute frame index. +Future _frameRgb(String path, int index) async { + final out = File(p.join(Directory.systemTemp.path, + 'vb_fm_${DateTime.now().microsecondsSinceEpoch}_$index.png')); + try { + final r = await Process.run( + WorkerHarness.ffmpegPath, + [ + '-y', '-v', 'error', + '-i', path, + // Decode from the start and pick by index — accurate, unlike an input + // seek, and these clips are short enough that it costs little. + '-vf', 'select=eq(n\\,$index)', + '-frames:v', '1', + out.path, + ], + environment: WorkerHarness.ffmpegEnv, + ); + expect(r.exitCode, 0, reason: 'extracting frame $index: ${r.stderr}'); + expect(await out.exists(), isTrue, reason: 'no frame $index in $path'); + return WorkerHarness.imageToRgb24(await out.readAsBytes(), + label: 'out$index'); + } finally { + if (await out.exists()) await out.delete().catchError((_) => out); + } +} + +void main() { + setUpAll(() async { + await WorkerHarness.ensureReady(); + await Directory(_outDir).create(recursive: true); + + // Every expected number below is a function of the fixtures' lengths, so + // pin those first — otherwise a re-encoded fixture turns these into + // confidently wrong assertions. + expect(await _countFrames(_interlaced), _interlacedFrames, + reason: 'interlaced_test.avi changed length; the expected output ' + 'counts in this file are derived from it'); + expect(await _countFrames(_telecine), _telecineFrames, + reason: 'hard_telecine_test.avi changed length'); + }); + + group('a pass that changes the frame count actually changes it', () { + test('double-rate QTGMC emits two frames per source frame', () async { + // Fanout{factor: 2}. Trimmed to 31 source frames, so 62 out. + const start = 10, end = 40; + const sourceFrames = end - start + 1; // 31 + final job = _job( + 'double_rate', + input: _interlaced, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters( + preset: QTGMCPreset.superFast, tff: true, fpsDivisor: 1), + ), + width: 720, + height: 576, + pixFmt: 'yuv422p', + fps: _interlacedFps, + // Post-trim, per the contract above. + totalFrames: sourceFrames, + startFrame: start, + endFrame: end, + ); + + final r = await WorkerHarness.runJob(job.toJson(), label: 'double_rate'); + expect(r.success, isTrue, reason: r.error); + final n = await _countFrames(r.outputPath!); + print(' $sourceFrames source -> $n output (expected 62)'); + expect(n, sourceFrames * 2, + reason: 'double-rate QTGMC must emit one frame per field. A count ' + 'equal to the source length means FPSDivisor was ignored and ' + 'the progress total is now double the real work.'); + }); + + test('single-rate QTGMC leaves the count alone', () async { + // The control. Without it, a broken assertion that always sees the source + // count would still pass the test above on a pipeline that fanned out + // nothing. + const start = 10, end = 40; + const sourceFrames = end - start + 1; + final job = _job( + 'single_rate', + input: _interlaced, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters( + preset: QTGMCPreset.superFast, tff: true, fpsDivisor: 2), + ), + width: 720, + height: 576, + pixFmt: 'yuv422p', + fps: _interlacedFps, + // Post-trim, per the contract above. + totalFrames: sourceFrames, + startFrame: start, + endFrame: end, + ); + + final r = await WorkerHarness.runJob(job.toJson(), label: 'single_rate'); + expect(r.success, isTrue, reason: r.error); + final n = await _countFrames(r.outputPath!); + print(' $sourceFrames source -> $n output (expected $sourceFrames)'); + expect(n, sourceFrames); + }); + + test('IVTC decimates 5 frames to 4', () async { + // Decimate{cycle: 5, keep: 4}: 90 telecined frames -> 72 film frames. + final job = _job( + 'ivtc', + input: _telecine, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters( + enabled: true, + method: DeinterlaceMethod.ivtc, + tff: true, + ivtcCycle: 5, + ), + ), + width: 720, + height: 480, + pixFmt: 'yuv420p', + fps: 30000 / 1001, + totalFrames: _telecineFrames, + ); + + final r = await WorkerHarness.runJob(job.toJson(), label: 'ivtc'); + expect(r.success, isTrue, reason: r.error); + final n = await _countFrames(r.outputPath!); + print(' $_telecineFrames source -> $n output (expected 72)'); + expect(n, _telecineFrames * 4 ~/ 5, + reason: 'IVTC must remove the pulldown-duplicated frame from every ' + 'cycle of five. The source count means VDecimate did nothing.'); + }); + + test('frame rate conversion retimes by the reduced ratio', () async { + // Retime. 25 -> 24000/1001 reduces to 960/1001, so 75 -> 71. + // + // Deinterlacing is disabled *explicitly*: ProcessingPipeline()'s default + // has QTGMC enabled, which would fan the count out by two and make this + // assertion about the composition rather than about FlowFPS. + const expected = _interlacedFrames * 960 ~/ 1001; // 71 + final job = _job( + 'retime', + input: _interlaced, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + frameRate: FrameRateParameters( + enabled: true, + target: FrameRateTarget.film23976, + sourceFpsNum: 25, + sourceFpsDen: 1, + ), + ), + width: 720, + height: 576, + pixFmt: 'yuv422p', + fps: _interlacedFps, + totalFrames: _interlacedFrames, + ); + + final r = await WorkerHarness.runJob(job.toJson(), label: 'retime'); + expect(r.success, isTrue, reason: r.error); + final n = await _countFrames(r.outputPath!); + print(' $_interlacedFrames source -> $n output (expected $expected)'); + expect(n, expected, + reason: 'FlowFPS was chosen over BlockFPS precisely because its ' + 'output count is n*num/den exactly, where BlockFPS is ' + 'floor((n-1)*r)+1. A count of ${expected + 1} means the ' + 'BlockFPS arithmetic is in play and every retimed job reports a ' + 'progress total one frame out.'); + }); + }); + + group('the preview shows the frame it claims to', () { + // `--frame S` is a SOURCE index (the worker logs "Preview: source frame + // S"), and the target's output index is output_count(local). So under + // double-rate the preview of source frame S must be output frame 2S. + // + // The assertion is deliberately RELATIVE — the correct output frame must be + // closer to the preview than its neighbours are. An absolute tolerance + // would have to absorb the encoder, the colour conversion and the window + // edges, and picking one loose enough to be safe would also be loose enough + // to pass on a preview that is off by a frame. The fixture moves ~12/255 + // per source frame, so neighbours are comfortably distinguishable. + late String encoded; + + setUpAll(() async { + // Untrimmed, so output frame index is exactly output_count(source index) + // with no start offset to reason about. Lossless, so the comparison is + // not measuring H.264. + final job = _job( + 'preview_sync', + input: _interlaced, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters( + preset: QTGMCPreset.superFast, tff: true, fpsDivisor: 1), + ), + width: 720, + height: 576, + pixFmt: 'yuv422p', + fps: _interlacedFps, + totalFrames: _interlacedFrames, + codec: VideoCodec.ffv1, + ); + final r = await WorkerHarness.runJob(job.toJson(), label: 'preview_sync'); + expect(r.success, isTrue, reason: r.error); + encoded = r.outputPath!; + expect(await _countFrames(encoded), _interlacedFrames * 2); + }); + + /// The job used for the preview must match the encode exactly, or any + /// difference measured is the job's, not the mapping's. + VideoJob previewJob() => _job( + 'preview_sync', + input: _interlaced, + pipeline: const ProcessingPipeline( + deinterlace: QTGMCParameters( + preset: QTGMCPreset.superFast, tff: true, fpsDivisor: 1), + ), + width: 720, + height: 576, + pixFmt: 'yuv422p', + fps: _interlacedFps, + totalFrames: _interlacedFrames, + codec: VideoCodec.ffv1, + ); + + for (final sourceFrame in [12, 24, 36]) { + test('source frame $sourceFrame previews as output frame ' + '${sourceFrame * 2}', () async { + final pv = await WorkerHarness.runPreview(previewJob().toJson(), + frame: sourceFrame, label: 'pv$sourceFrame'); + expect(pv.success, isTrue, reason: pv.toString()); + final preview = + await WorkerHarness.imageToRgb24(pv.png!, label: 'pv$sourceFrame'); + + final expectedIndex = sourceFrame * 2; + // ±4 output frames is ±2 source frames — far enough that motion is + // unambiguous, near enough to catch an off-by-one-source-frame error. + final candidates = {}; + for (final offset in [-4, -2, 0, 2, 4]) { + final idx = expectedIndex + offset; + candidates[idx] = WorkerHarness.meanAbsDiff( + preview, await _frameRgb(encoded, idx)); + } + candidates.forEach((idx, diff) => print( + ' output frame $idx: meanAbsDiff ${diff.toStringAsFixed(2)}' + '${idx == expectedIndex ? ' <- expected' : ''}')); + + final best = + candidates.entries.reduce((a, b) => a.value <= b.value ? a : b); + expect(best.key, expectedIndex, + reason: 'the preview of source frame $sourceFrame matched output ' + 'frame ${best.key} more closely than $expectedIndex, so the ' + 'preview and the render disagree about which frame this is'); + + // And it must actually be the same picture, not merely the closest of + // five wrong ones. + expect(candidates[expectedIndex]!, lessThan(6.0), + reason: 'the preview is nearest the right frame but still differs ' + 'from it substantially, which points at the pass rendering ' + 'differently in the two paths rather than at the mapping'); + }, timeout: const Timeout(Duration(minutes: 6))); + } + }); +} diff --git a/app/test/integration_upscale_resize_test.dart b/app/test/integration_upscale_resize_test.dart index 6368186..f9b6d41 100644 --- a/app/test/integration_upscale_resize_test.dart +++ b/app/test/integration_upscale_resize_test.dart @@ -12,6 +12,7 @@ library; // ignore_for_file: avoid_print — these tests print diagnostics to the test log. import 'dart:io'; +import 'dart:typed_data'; import 'package:flutter_test/flutter_test.dart'; import 'package:uuid/uuid.dart'; @@ -71,6 +72,70 @@ Future _expectSize(String name, CropResizeParameters cropResize, expect(int.parse(v['height'].toString()), height); } +/// A lossless job with the source's real pixel format, for comparisons where +/// the encoder must not be part of what is measured. +/// +/// The plain [_job] above encodes H.264 and leaves `inputPixelFormat` unset, so +/// the decoder normalises to yuv420p. Fine for "is the output 640x480", useless +/// for "is this the same picture" — the codec and the chroma round-trip would +/// both land in the difference. +VideoJob _losslessJob( + String name, + CropResizeParameters cropResize, { + int startFrame = 0, + int endFrame = 6, +}) => + VideoJob( + id: const Uuid().v4(), + inputPath: WorkerHarness.inputFile, + outputPath: '$_outDir/$name.mkv', + processingPipeline: ProcessingPipeline( + deinterlace: const QTGMCParameters(enabled: false), + cropResize: cropResize, + ), + encodingSettings: const EncodingSettings( + codec: VideoCodec.ffv1, + container: ContainerFormat.mkv, + audioMode: AudioMode.none, + ), + inputWidth: _srcWidth, + inputHeight: _srcHeight, + inputPixelFormat: 'yuv422p', + inputFrameRate: 25.0, + totalFrames: endFrame - startFrame + 1, + startFrame: startFrame, + endFrame: endFrame, + ); + +/// Crop a packed rgb24 frame, in Dart. +/// +/// The reference a crop is judged against is built by cropping the pipeline's +/// OWN uncropped output rather than by asking ffmpeg to crop the source, and +/// that is not incidental: `interlaced_test.avi` declares 79 frames but only 75 +/// decode — it carries null frames, which the decoder's default CFR mode expands +/// onto the 25 fps grid. So `select=eq(n,N)` on the source counts decoded frames +/// while the pipeline counts timeline positions, and the two diverge wherever a +/// null frame sits. Comparing pipeline output against pipeline output uses one +/// decoder, one indexing scheme, and isolates the crop itself. +Uint8List _cropRgb24( + Uint8List src, { + required int srcWidth, + required int x, + required int y, + required int width, + required int height, +}) { + final srcHeight = src.length ~/ (srcWidth * 3); + expect(x + width, lessThanOrEqualTo(srcWidth), reason: 'crop past right edge'); + expect(y + height, lessThanOrEqualTo(srcHeight), reason: 'crop past bottom'); + final out = Uint8List(width * height * 3); + for (var row = 0; row < height; row++) { + final from = ((y + row) * srcWidth + x) * 3; + out.setRange(row * width * 3, (row + 1) * width * 3, src, from); + } + return out; +} + void main() { setUpAll(() async { await WorkerHarness.ensureReady(); @@ -171,4 +236,223 @@ void main() { }, timeout: const Timeout(Duration(minutes: 5))); } }); + + group('cropping', () { + // Crop was exercised by the pipeline tests but nothing asserted the size it + // produced — they ran a crop and checked the job succeeded, which passes + // just as happily on a crop that did nothing. + test('an asymmetric crop removes exactly the requested edges', () async { + // Deliberately asymmetric on both axes. A symmetric crop cannot tell a + // correct implementation from one that swaps left/right or top/bottom, + // and a size-only assertion cannot tell either regardless. + const l = 16, r = 8, t = 12, b = 4; + const w = 720 - l - r; // 696 + const h = 576 - t - b; // 560 + + await _expectSize( + 'crop_asymmetric', + const CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: l, + cropRight: r, + cropTop: t, + cropBottom: b, + ), + width: w, + height: h, + ); + }); + + test('the crop is taken from the right offset, not just the right size', + () async { + // A size assertion cannot tell a correct crop from one that swaps left + // with right or top with bottom — both produce exactly the requested + // dimensions. This compares the pixels. + const l = 16, r = 8, t = 12, b = 4; + const w = 720 - l - r, h = 576 - t - b; + const frame = 3; + + final uncropped = await WorkerHarness.runJob( + _losslessJob('crop_none', const CropResizeParameters(enabled: false)) + .toJson(), + label: 'crop_none'); + expect(uncropped.success, isTrue, reason: uncropped.error); + + final cropped = await WorkerHarness.runJob( + _losslessJob( + 'crop_offset', + const CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: l, + cropRight: r, + cropTop: t, + cropBottom: b, + ), + ).toJson(), + label: 'crop_offset'); + expect(cropped.success, isTrue, reason: cropped.error); + + final full = await WorkerHarness.frameRgb24(uncropped.outputPath!, frame, + label: 'full'); + final actual = await WorkerHarness.frameRgb24(cropped.outputPath!, frame, + label: 'cropped'); + + double diffAt(int x, int y) => WorkerHarness.meanAbsDiff( + actual, + _cropRgb24(full, + srcWidth: 720, x: x, y: y, width: w, height: h), + ); + + // The requested offset, and the two a swap would produce. + final correct = diffAt(l, t); + final swappedH = diffAt(r, t); + final swappedV = diffAt(l, b); + print(' crop offset diffs — correct ($l,$t): ' + '${correct.toStringAsFixed(2)}, ' + 'left/right swapped ($r,$t): ${swappedH.toStringAsFixed(2)}, ' + 'top/bottom swapped ($l,$b): ${swappedV.toStringAsFixed(2)}'); + + // Not zero, and it cannot be: the two frames are converted to rgb24 at + // different widths, so chroma interpolation at the edges of a 4:2:2 clip + // differs slightly between them. Measured ~1.1 against ~10 and ~16 for + // the swaps, so the margin below carries the assertion and the absolute + // bound is only a backstop. + expect(correct, lessThan(3.0), + reason: 'the cropped output should be the region at ($l, $t) of the ' + 'uncropped output, to within the rgb conversion'); + expect(correct * 3, lessThan(swappedH), + reason: 'the crop matches the left/right-swapped offset almost as ' + 'well as the requested one, so the horizontal edges are suspect'); + expect(correct * 3, lessThan(swappedV), + reason: 'the crop matches the top/bottom-swapped offset almost as ' + 'well as the requested one, so the vertical edges are suspect'); + }); + + test('crop composes with resize, in that order', () async { + // Crop first, then scale the cropped picture to the target — so the + // target size is what comes out, whatever the crop was. If the order were + // reversed the crop would eat into the target and the output would be + // smaller than asked for. + await _expectSize( + 'crop_then_resize', + const CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: 16, + cropRight: 16, + cropTop: 8, + cropBottom: 8, + resizeEnabled: true, + targetWidth: 640, + targetHeight: 480, + maintainAspect: false, + kernel: ResizeKernel.spline36, + ), + width: 640, + height: 480, + ); + }); + }); + + group('the preview shows the same geometry as the render', () { + // The analogue of the frame-mapping preview test in + // integration_frame_mapping_test.dart: crop and resize are computed inside + // the .vpy, and the encode and the preview are separate scripts. A pass + // that reached one template and not the other renders a preview at a + // different size or framing than the file the user gets — silently, since + // both are individually valid pictures. The output-format block was in the + // encode template only for exactly this reason once already. + Future expectPreviewMatchesRender( + String name, + CropResizeParameters cropResize, { + required int width, + required int height, + int sourceFrame = 3, + }) async { + final job = _losslessJob(name, cropResize); + final result = await WorkerHarness.runJob(job.toJson(), label: name); + expect(result.success, isTrue, reason: result.error); + + final v = await WorkerHarness.firstStream(result.outputPath!, + selector: 'v:0', entries: ['width', 'height']); + expect(int.parse(v!['width'].toString()), width); + expect(int.parse(v['height'].toString()), height); + + final pv = await WorkerHarness.runPreview(job.toJson(), + frame: sourceFrame, label: '$name-pv'); + expect(pv.success, isTrue, reason: pv.toString()); + final preview = + await WorkerHarness.imageToRgb24(pv.png!, label: '$name-pv'); + + // No geometry pass changes the frame count, so the preview of source + // frame N is output frame N. meanAbsDiff throws on a size mismatch, which + // is the assertion that matters most here — a preview at a different + // resolution is the failure this test exists to catch, and it surfaces as + // a buffer-length difference rather than a pixel difference. + final rendered = await WorkerHarness.frameRgb24( + result.outputPath!, sourceFrame, + label: '$name-out'); + final diff = WorkerHarness.meanAbsDiff(preview, rendered); + print(' $name: preview vs render meanAbsDiff ${diff.toStringAsFixed(2)} ' + 'at ${width}x$height'); + expect(diff, lessThan(2.0), + reason: 'the preview and the render disagree about the picture at ' + 'the same frame, despite agreeing on the size'); + } + + test('a crop reaches the preview path', () async { + await expectPreviewMatchesRender( + 'pv_crop', + const CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: 16, + cropRight: 8, + cropTop: 12, + cropBottom: 4, + ), + width: 696, + height: 560, + ); + }); + + test('a resize reaches the preview path', () async { + await expectPreviewMatchesRender( + 'pv_resize', + const CropResizeParameters( + enabled: true, + resizeEnabled: true, + targetWidth: 512, + targetHeight: 384, + maintainAspect: false, + kernel: ResizeKernel.spline36, + ), + width: 512, + height: 384, + ); + }); + + test('crop and resize together reach the preview path', () async { + await expectPreviewMatchesRender( + 'pv_crop_resize', + const CropResizeParameters( + enabled: true, + cropEnabled: true, + cropLeft: 16, + cropRight: 16, + cropTop: 8, + cropBottom: 8, + resizeEnabled: true, + targetWidth: 640, + targetHeight: 480, + maintainAspect: false, + kernel: ResizeKernel.spline36, + ), + width: 640, + height: 480, + ); + }); + }); } diff --git a/app/test/support/worker_harness.dart b/app/test/support/worker_harness.dart index 2a765db..cda41bb 100644 --- a/app/test/support/worker_harness.dart +++ b/app/test/support/worker_harness.dart @@ -552,6 +552,43 @@ class WorkerHarness { return total / a.length; } + /// One frame of a video as packed rgb24, addressed by absolute frame index. + /// + /// Decodes from the start and picks by index rather than seeking: an input + /// seek lands on the nearest keyframe, which for these comparisons would + /// silently compare the wrong frame. The fixtures are short enough that the + /// full decode costs little. + /// + /// Pairs with [imageToRgb24] and [meanAbsDiff] to answer "is this the frame + /// the pipeline says it is" rather than merely "did something render". + static Future frameRgb24(String videoPath, int index, + {String label = 'frame'}) async { + final png = File(p.join(Directory.systemTemp.path, + 'vb_${label}_${DateTime.now().microsecondsSinceEpoch}_$index.png')); + try { + final r = await Process.run( + ffmpegPath, + [ + '-y', '-v', 'error', + '-i', videoPath, + '-vf', 'select=eq(n\\,$index)', + '-frames:v', '1', + png.path, + ], + environment: ffmpegEnv, + ); + if (r.exitCode != 0) { + throw Exception('extracting frame $index from $videoPath: ${r.stderr}'); + } + if (!await png.exists()) { + throw Exception('no frame $index in $videoPath (past the end?)'); + } + return imageToRgb24(await png.readAsBytes(), label: '$label$index'); + } finally { + if (await png.exists()) await png.delete().catchError((_) => png); + } + } + /// Extract one frame as PNG and return its md5 hash. static Future frameHash(String videoPath, {int frame = 5}) async { final framePath = p.join(Directory.systemTemp.path,