diff --git a/CLAUDE.md b/CLAUDE.md index 9309ea9..df22f10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,6 +90,7 @@ VapourBox/ |------|---------| | `worker/src/models/video_job.rs` | Job config, EncodingSettings, processing passes | | `worker/src/models/qtgmc_parameters.rs` | All 70+ QTGMC parameters (serde) | +| `worker/src/models/processing_pipeline.rs` | Pipeline passes + `FrameMap` (source↔output frame mapping: `output_count` drives the progress total, `invert`/`total_radius` drive frame-accurate preview) | | `worker/src/script_generator.rs` | Template substitution for .vpy | | `worker/src/pipeline_executor.rs` | vspipe \| ffmpeg execution | | `worker/templates/pipeline_template.vpy` | VapourSynth script template | diff --git a/app/lib/services/frame_math.dart b/app/lib/services/frame_math.dart new file mode 100644 index 0000000..f3657b0 --- /dev/null +++ b/app/lib/services/frame_math.dart @@ -0,0 +1,22 @@ +/// Pure conversions between a normalized scrubber position (0.0-1.0) and an +/// integer source frame index. +/// +/// The scrubber gesture is naturally normalized, but the preview and the +/// step/jump seek controls operate on exact frames. Keeping these conversions +/// pure (and shared) means the rounding behaviour is unit-testable and a +/// position → frame → position round-trip is stable (no drift when stepping). +class FrameMath { + /// Frame index for a normalized [position], given the [totalFrames] count. + /// Returns 0 when the video has 0 or 1 frames. + static int frameForPosition(double position, int totalFrames) { + if (totalFrames <= 1) return 0; + return (position.clamp(0.0, 1.0) * (totalFrames - 1)).round(); + } + + /// Normalized position for a [frame] index, given the [totalFrames] count. + /// Returns 0.0 when the video has 0 or 1 frames. + static double positionForFrame(int frame, int totalFrames) { + if (totalFrames <= 1) return 0.0; + return (frame / (totalFrames - 1)).clamp(0.0, 1.0); + } +} diff --git a/app/lib/services/preview_generator.dart b/app/lib/services/preview_generator.dart index d742eae..a260ae9 100644 --- a/app/lib/services/preview_generator.dart +++ b/app/lib/services/preview_generator.dart @@ -150,28 +150,35 @@ class PreviewGenerator { return thumbnails; } - /// Get a single frame at a specific time position. - Future getFrameAt(double timeSeconds) async { + /// Get the unprocessed source frame at an exact frame index. + /// + /// Frame-accurate: seeks to the midpoint of the interval *before* the target + /// frame ((index - 0.5)/fps) so the first decoded frame (PTS >= seek time) is + /// exactly `frameIndex`. This matches the worker's preview seek, so the + /// before/after comparison always shows the same source frame. + Future getFrameAtIndex(int frameIndex) async { if (_currentVideoPath == null || _ffmpegPath == null) return null; - final frameIndex = (timeSeconds * _frameRate).round(); + final frame = frameIndex < 0 ? 0 : frameIndex; // Check cache - if (_thumbnailCache.containsKey(frameIndex)) { - return _thumbnailCache[frameIndex]; + if (_thumbnailCache.containsKey(frame)) { + return _thumbnailCache[frame]; } // Extract frame - final outputPath = '$_tempDir/frame_${frameIndex}.jpg'; + 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', timeSeconds.toStringAsFixed(3), + '-ss', ss.toStringAsFixed(6), '-i', _currentVideoPath!, - '-vframes', '1', + '-frames:v', '1', '-q:v', '2', outputPath, ], @@ -179,7 +186,7 @@ class PreviewGenerator { if (result.exitCode == 0 && await File(outputPath).exists()) { final bytes = await File(outputPath).readAsBytes(); - _thumbnailCache[frameIndex] = bytes; + _thumbnailCache[frame] = bytes; return bytes; } } catch (e) { @@ -195,7 +202,7 @@ class PreviewGenerator { /// the processed frame. Uses the same code path as actual video processing /// to ensure preview matches the final output. Future generateProcessedPreview({ - required double timeSeconds, + required int frameNumber, required ProcessingPipeline pipeline, required FieldOrder fieldOrder, required EncodingSettings encodingSettings, @@ -205,14 +212,25 @@ class PreviewGenerator { return null; } + // Dimensions come from the ffprobe pass in loadVideo(). If they aren't + // populated yet (probe still running — e.g. a slow first-run Gatekeeper + // assessment of freshly-downloaded ffprobe — or the probe failed), skip + // rather than spawning the worker with a 0x0 job, which fails with a + // misleading "Invalid video dimensions" error. The preview is requested + // again once the video is probed, so this self-heals. + if (_videoWidth <= 0 || _videoHeight <= 0) { + return null; + } + // Cancel any existing preview generation await cancelPreviewGeneration(); if (cancelToken?.isCancelled ?? false) return null; - // Calculate frame number in the SOURCE video (not output) - // We no longer double for FPSDivisor=1 because we're seeking in the source - final frameNumber = (timeSeconds * _frameRate).round(); + // `frameNumber` is the SOURCE frame index, passed straight to the worker + // (no time round-trip). The worker decodes a window centred on it and emits + // the exact output frame it maps to — see generate_preview in the worker. + final frame = frameNumber < 0 ? 0 : frameNumber; final configPath = '$_tempDir/preview_config_${DateTime.now().millisecondsSinceEpoch}.json'; Process? process; @@ -247,7 +265,7 @@ class PreviewGenerator { // Clear log for this preview generation _previewLog.clear(); _lastError = null; - _previewLog.add('[${DateTime.now().toIso8601String()}] Starting preview generation for frame $frameNumber'); + _previewLog.add('[${DateTime.now().toIso8601String()}] Starting preview generation for frame $frame'); // Run worker in preview mode // Use local variable to avoid race conditions when another preview request cancels this one @@ -257,7 +275,7 @@ class PreviewGenerator { [ '--config', configPath, '--preview', - '--frame', frameNumber.toString(), + '--frame', frame.toString(), ], environment: ToolLocator.instance.workerEnvironment, workingDirectory: path.dirname(_workerPath!), diff --git a/app/lib/viewmodels/main_viewmodel.dart b/app/lib/viewmodels/main_viewmodel.dart index a1137af..6882f48 100644 --- a/app/lib/viewmodels/main_viewmodel.dart +++ b/app/lib/viewmodels/main_viewmodel.dart @@ -17,6 +17,7 @@ import '../models/processing_preset.dart'; import '../services/disc_detector.dart'; import '../services/dvd_service.dart'; import '../services/field_order_detector.dart'; +import '../services/frame_math.dart'; import '../services/preset_service.dart'; import '../services/preview_generator.dart'; import '../services/worker_manager.dart'; @@ -268,6 +269,32 @@ class MainViewModel extends ChangeNotifier { List get previewLog => _previewGenerator.previewLog; String? get previewError => _previewGenerator.lastError; + /// Total number of source frames in the loaded video (0 if not loaded). + int get totalFrames => _previewGenerator.totalFrames; + + /// The integer source frame the scrubber currently points at. This is the + /// authoritative unit for the preview and step/jump controls — the normalized + /// scrubberPosition is just the gesture representation. + int get currentFrameIndex => _frameForPosition(scrubberPosition); + + /// Convert a normalized scrubber position (0.0-1.0) to a source frame index. + int _frameForPosition(double position) => + FrameMath.frameForPosition(position, _previewGenerator.totalFrames); + + /// Convert a source frame index to a normalized scrubber position (0.0-1.0). + double _positionForFrame(int frame) => + FrameMath.positionForFrame(frame, _previewGenerator.totalFrames); + + /// Seek to an exact source frame, clamped to the video's range. + Future seekToFrame(int frame) async { + final total = _previewGenerator.totalFrames; + final clamped = total > 0 ? frame.clamp(0, total - 1) : 0; + await setScrubberPosition(_positionForFrame(clamped)); + } + + /// Step the scrubber by a signed number of frames (e.g. -1 / +1). + Future stepFrame(int delta) => seekToFrame(currentFrameIndex + delta); + // Timeline zoom getters (delegate to selected item) double get timelineZoom => selectedItem?.timelineZoom ?? 1.0; double get timelineViewStart => selectedItem?.timelineViewStart ?? 0.0; @@ -487,7 +514,7 @@ class MainViewModel extends ChangeNotifier { if (_selectedItemId == item.id) { try { _queue[index].thumbnails = await _previewGenerator.loadVideo(item.inputPath); - _queue[index].currentFrame = await _previewGenerator.getFrameAt(0); + _queue[index].currentFrame = await _previewGenerator.getFrameAtIndex(0); } catch (e) { _logMessages.add(LogMessage( level: LogLevel.warning, @@ -562,8 +589,8 @@ class MainViewModel extends ChangeNotifier { if (item.thumbnails.isEmpty && item.status != QueueItemStatus.analyzing) { try { item.thumbnails = await _previewGenerator.loadVideo(item.inputPath); - item.currentFrame = await _previewGenerator.getFrameAt( - item.scrubberPosition * _previewGenerator.duration, + item.currentFrame = await _previewGenerator.getFrameAtIndex( + _frameForPosition(item.scrubberPosition), ); notifyListeners(); } catch (e) { @@ -641,9 +668,11 @@ class MainViewModel extends ChangeNotifier { item.scrubberPosition = position.clamp(0.0, 1.0); - // Update current frame immediately - final timeSeconds = item.scrubberPosition * _previewGenerator.duration; - item.currentFrame = await _previewGenerator.getFrameAt(timeSeconds); + // Update the unprocessed source frame immediately, seeking to the exact + // frame the scrubber maps to (same index the processed preview uses, so the + // before/after comparison stays aligned). + item.currentFrame = + await _previewGenerator.getFrameAtIndex(_frameForPosition(item.scrubberPosition)); notifyListeners(); // Debounce the processed preview generation @@ -854,11 +883,11 @@ class MainViewModel extends ChangeNotifier { final cancelToken = CancelToken(); _previewCancelToken = cancelToken; - final timeSeconds = item.scrubberPosition * _previewGenerator.duration; + final frameNumber = _frameForPosition(item.scrubberPosition); try { final preview = await _previewGenerator.generateProcessedPreview( - timeSeconds: timeSeconds, + frameNumber: frameNumber, pipeline: _processingPipeline, fieldOrder: effectiveFieldOrder, encodingSettings: _encodingSettings, diff --git a/app/lib/views/preview_panel.dart b/app/lib/views/preview_panel.dart index 0346408..7da5d0c 100644 --- a/app/lib/views/preview_panel.dart +++ b/app/lib/views/preview_panel.dart @@ -102,6 +102,35 @@ class _PreviewPanelState extends State { _formatTime(viewModel.scrubberPosition * viewModel.videoDuration), style: Theme.of(context).textTheme.bodySmall, ), + // Frame-accurate seek controls: step back/forward one frame + // and a readout of the exact source frame being previewed. + if (viewModel.totalFrames > 0) ...[ + const SizedBox(width: 8), + IconButton( + icon: const Icon(Icons.chevron_left, size: 18), + tooltip: 'Previous frame', + visualDensity: VisualDensity.compact, + constraints: const BoxConstraints(minWidth: 28, minHeight: 28), + padding: EdgeInsets.zero, + onPressed: viewModel.currentFrameIndex > 0 + ? () => viewModel.stepFrame(-1) + : null, + ), + Text( + 'f ${viewModel.currentFrameIndex} / ${viewModel.totalFrames - 1}', + style: Theme.of(context).textTheme.bodySmall, + ), + IconButton( + icon: const Icon(Icons.chevron_right, size: 18), + tooltip: 'Next frame', + visualDensity: VisualDensity.compact, + constraints: const BoxConstraints(minWidth: 28, minHeight: 28), + padding: EdgeInsets.zero, + onPressed: viewModel.currentFrameIndex < viewModel.totalFrames - 1 + ? () => viewModel.stepFrame(1) + : null, + ), + ], const SizedBox(width: 8), // In point button Tooltip( diff --git a/app/test/frame_math_test.dart b/app/test/frame_math_test.dart new file mode 100644 index 0000000..8f77660 --- /dev/null +++ b/app/test/frame_math_test.dart @@ -0,0 +1,38 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/services/frame_math.dart'; + +void main() { + group('FrameMath', () { + test('maps endpoints to first and last frame', () { + expect(FrameMath.frameForPosition(0.0, 1000), 0); + expect(FrameMath.frameForPosition(1.0, 1000), 999); + }); + + test('rounds to the nearest frame', () { + // 0.5 of a 1001-frame clip (last index 1000) -> 500 + expect(FrameMath.frameForPosition(0.5, 1001), 500); + }); + + test('clamps out-of-range positions', () { + expect(FrameMath.frameForPosition(-0.2, 500), 0); + expect(FrameMath.frameForPosition(1.5, 500), 499); + }); + + test('degenerate clips never divide by zero', () { + expect(FrameMath.frameForPosition(0.7, 0), 0); + expect(FrameMath.frameForPosition(0.7, 1), 0); + expect(FrameMath.positionForFrame(5, 0), 0.0); + expect(FrameMath.positionForFrame(5, 1), 0.0); + }); + + test('position <-> frame round-trips without drift', () { + // Stepping frame-by-frame must be stable: frame -> position -> frame == frame. + const total = 2500; + for (final frame in [0, 1, 2, 1249, 1250, 2498, 2499]) { + final pos = FrameMath.positionForFrame(frame, total); + expect(FrameMath.frameForPosition(pos, total), frame, + reason: 'round-trip should be stable for frame $frame'); + } + }); + }); +} diff --git a/worker/src/main.rs b/worker/src/main.rs index c21ee70..8c4948f 100644 --- a/worker/src/main.rs +++ b/worker/src/main.rs @@ -276,11 +276,10 @@ fn run_preview_mode(args: &Args) -> ExitCode { } }; - // Calculate time from frame number - let frame_rate = job.input_frame_rate.unwrap_or(29.97); - let time_seconds = frame as f64 / frame_rate; - - eprintln!("Preview: frame {} at {:.3}s (fps: {:.2})", frame, time_seconds, frame_rate); + // The frame index is passed straight through to the worker — no + // time-conversion round-trip — so the rendered frame is exactly the one the + // caller asked for (see generate_preview's frame-accurate windowing). + eprintln!("Preview: source frame {} (fps: {:.2})", frame, job.input_frame_rate.unwrap_or(29.97)); // Execute preview (extracts frames with ffmpeg, processes with VapourSynth) let executor = match PipelineExecutor::new(ProgressReporter::new()) { @@ -291,7 +290,7 @@ fn run_preview_mode(args: &Args) -> ExitCode { } }; - match executor.generate_preview(&job, time_seconds) { + match executor.generate_preview(&job, frame) { Ok(()) => ExitCode::SUCCESS, Err(e) => { eprintln!("Error generating preview: {}", e); diff --git a/worker/src/models/processing_pipeline.rs b/worker/src/models/processing_pipeline.rs index cad5a2e..3fefb01 100644 --- a/worker/src/models/processing_pipeline.rs +++ b/worker/src/models/processing_pipeline.rs @@ -4,10 +4,110 @@ use serde::{Deserialize, Serialize}; use super::{ ChromaFixParameters, ColorCorrectionParameters, CropResizeParameters, - DebandParameters, DeblockParameters, DehaloParameters, DeScratchParameters, + DebandParameters, DeblockParameters, DehaloParameters, DeinterlaceMethod, DeScratchParameters, SpotLessParameters, SharpenParameters, NoiseReductionParameters, QTGMCParameters, }; +/// Minimum temporal context (frames on each side of the target) a windowed +/// preview decodes, regardless of which filters are enabled. Motion-compensated +/// filters (QTGMC) need neighbours; this floor preserves the old fixed 11-frame +/// (5-per-side) window so preview quality never regresses when a pipeline's +/// declared radius is small. +pub const MIN_PREVIEW_RADIUS: u32 = 5; + +/// A span of *source* (input-side) frames, with whether the mapping that +/// produced it is frame-exact. `exact = false` means the span is the best +/// available estimate (data-dependent decimation, or a synthesized/interpolated +/// output frame with no single source origin) and a consumer should treat the +/// result as approximate. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] +pub struct SourceSpan { + pub start: u64, + pub end: u64, + pub exact: bool, +} + +/// How one pass relates its *input* clip to its *output* clip on the frame +/// timeline. Two operations matter to the rest of the system: a forward count +/// (the progress total) and an inverse window (which source frames produce a +/// given output frame, for preview/seek). See `ProcessingPipeline::output_count` +/// / `invert` for how these compose across a pipeline. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] +pub enum FrameMap { + /// Count and index unchanged. `radius` = temporal neighbours the filter + /// reads to compute one frame (denoise, sharpen, single-rate QTGMC…). + Identity { radius: u32 }, + + /// Each input frame fans out to `factor` contiguous output frames. + /// QTGMC FPSDivisor=1 → factor 2 (one output frame per field). Exact. + Fanout { factor: u32, radius: u32 }, + + /// Drop frames on a fixed cycle: keep `keep` of every `cycle` + /// (VDecimate / IVTC, cycle 5 keep 4 → 30→24). The count is exact, but + /// *which* frame is dropped is data-dependent, so the inverse is approximate. + Decimate { cycle: u32, keep: u32 }, + + /// Rational retime of the timeline (frame-rate conversion / interpolation). + /// `synthesizes` = output frames may have no single source origin (motion + /// interpolation) → inverse is a blended, inexact range. + Retime { num: u32, den: u32, synthesizes: bool, radius: u32 }, +} + +#[allow(dead_code)] +impl FrameMap { + /// Output frame count given an input frame count. (Progress / total.) + pub fn output_count(&self, n_in: u64) -> u64 { + match *self { + FrameMap::Identity { .. } => n_in, + FrameMap::Fanout { factor, .. } => n_in * factor as u64, + FrameMap::Decimate { cycle, keep } => n_in * keep as u64 / cycle as u64, + FrameMap::Retime { num, den, .. } => n_in * num as u64 / den as u64, + } + } + + /// Map an output frame index back to the input-side source frames that + /// produce it. (Preview / seek.) + pub fn inverse(&self, m: u64) -> SourceSpan { + match *self { + FrameMap::Identity { .. } => SourceSpan { start: m, end: m, exact: true }, + + // output frames [k*factor .. k*factor+factor) all come from source k + FrameMap::Fanout { factor, .. } => { + let s = m / factor as u64; + SourceSpan { start: s, end: s, exact: true } + } + + // best-effort: assume kept frames are evenly spaced within each cycle. + // Off by ≤1 from the true kept frame, and the dropped one is a + // near-duplicate, so visually negligible — but mark it inexact. + FrameMap::Decimate { cycle, keep } => { + let c = m / keep as u64; + let i = m % keep as u64; + let s = c * cycle as u64 + i * cycle as u64 / keep as u64; + SourceSpan { start: s, end: s, exact: false } + } + + FrameMap::Retime { num, den, synthesizes, .. } => { + let lo = m * den as u64 / num as u64; + let hi = (m * den as u64 + num as u64 - 1) / num as u64; // ceil + SourceSpan { start: lo, end: hi.max(lo), exact: !synthesizes } + } + } + } + + /// Temporal neighbours (each side) the filter reads to compute one frame. + pub fn radius(&self) -> u32 { + match *self { + FrameMap::Identity { radius } + | FrameMap::Fanout { radius, .. } + | FrameMap::Retime { radius, .. } => radius, + FrameMap::Decimate { .. } => 0, + } + } +} + /// Defines the type of each processing pass. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -220,6 +320,84 @@ impl ProcessingPipeline { count } + /// The frame-timeline map each enabled pass imposes, in application order. + /// Only the deinterlace pass changes the count today; the rest are identity + /// (with a small temporal radius for context-sensitive filters). + pub fn frame_maps(&self) -> Vec { + self.enabled_passes() + .into_iter() + .map(|p| self.frame_map_for(p)) + .collect() + } + + /// The `FrameMap` for a single pass, derived from its parameters. + fn frame_map_for(&self, pass: PassType) -> FrameMap { + match pass { + PassType::Deinterlace => self.deinterlace_frame_map(), + // Temporal filters need neighbours for a correct windowed preview, + // but don't change the frame count. + PassType::NoiseReduction => FrameMap::Identity { radius: 3 }, + PassType::SpotLess => FrameMap::Identity { radius: 2 }, + PassType::DeScratch => FrameMap::Identity { radius: 1 }, + // Purely spatial passes. + _ => FrameMap::Identity { radius: 0 }, + } + } + + /// Frame mapping for the deinterlace pass. QTGMC double-rate (FPSDivisor=1, + /// the default when unset) doubles the frame count; IVTC/soft-telecine + /// decimate on a cycle. Mirrors the count logic the progress reporter and + /// VapourSynth pipeline actually produce. + fn deinterlace_frame_map(&self) -> FrameMap { + let d = &self.deinterlace; + match d.method { + DeinterlaceMethod::Qtgmc => { + if d.fps_divisor.unwrap_or(1) == 1 { + FrameMap::Fanout { factor: 2, radius: 3 } + } else { + FrameMap::Identity { radius: 3 } + } + } + DeinterlaceMethod::Ivtc | DeinterlaceMethod::SoftTelecine => { + let cycle = d.ivtc_cycle.unwrap_or(5).max(2) as u32; + FrameMap::Decimate { cycle, keep: cycle - 1 } + } + } + } + + /// Total output frame count for a given source frame count, composing every + /// enabled pass's forward map. Replaces ad-hoc double-rate / IVTC scaling. + pub fn output_count(&self, source_frames: u64) -> u64 { + self.frame_maps() + .iter() + .fold(source_frames, |n, m| m.output_count(n)) + } + + /// Map a final output frame index back to the source frame span that + /// produces it, composing every enabled pass's inverse in reverse order. + pub fn invert(&self, output_frame: u64) -> SourceSpan { + self.frame_maps().iter().rev().fold( + SourceSpan { start: output_frame, end: output_frame, exact: true }, + |span, m| { + let a = m.inverse(span.start); + let b = m.inverse(span.end); + SourceSpan { + start: a.start, + end: b.end.max(a.start), + exact: span.exact && a.exact && b.exact, + } + }, + ) + } + + /// Conservative temporal padding (frames each side) a windowed preview + /// should decode so every enabled filter has the context it needs. Floored + /// at `MIN_PREVIEW_RADIUS` so motion-compensated filters never starve. + pub fn total_radius(&self) -> u32 { + let declared: u32 = self.frame_maps().iter().map(|m| m.radius()).sum(); + declared.max(MIN_PREVIEW_RADIUS) + } + /// Check if a specific pass is enabled. pub fn is_pass_enabled(&self, pass: PassType) -> bool { match pass { @@ -271,4 +449,77 @@ mod tests { assert!(json.contains("\"noiseReduction\"")); assert!(json.contains("\"colorCorrection\"")); } + + // ---- FrameMap descriptor ---- + + #[test] + fn test_framemap_output_count() { + assert_eq!(FrameMap::Identity { radius: 0 }.output_count(100), 100); + assert_eq!(FrameMap::Fanout { factor: 2, radius: 0 }.output_count(100), 200); + // cycle 5, keep 4 → 30fps→24fps + assert_eq!(FrameMap::Decimate { cycle: 5, keep: 4 }.output_count(100), 80); + // 24→25 retime + assert_eq!(FrameMap::Retime { num: 25, den: 24, synthesizes: false, radius: 0 }.output_count(240), 250); + } + + #[test] + fn test_framemap_inverse() { + assert_eq!(FrameMap::Identity { radius: 0 }.inverse(42), + SourceSpan { start: 42, end: 42, exact: true }); + // double-rate: output 10 and 11 both come from source 5 + assert_eq!(FrameMap::Fanout { factor: 2, radius: 0 }.inverse(10), + SourceSpan { start: 5, end: 5, exact: true }); + assert_eq!(FrameMap::Fanout { factor: 2, radius: 0 }.inverse(11), + SourceSpan { start: 5, end: 5, exact: true }); + // decimation inverse is approximate (data-dependent drop) + assert!(!FrameMap::Decimate { cycle: 5, keep: 4 }.inverse(8).exact); + // interpolation: synthesized frames are inexact, blended span + let s = FrameMap::Retime { num: 2, den: 1, synthesizes: true, radius: 0 }.inverse(7); + assert!(!s.exact); + } + + #[test] + fn test_pipeline_output_count_matches_old_ladder() { + // Single-rate QTGMC (FPSDivisor=2): unchanged count. + let mut p = ProcessingPipeline::default(); + p.deinterlace.enabled = true; + p.deinterlace.method = DeinterlaceMethod::Qtgmc; + p.deinterlace.fps_divisor = Some(2); + assert_eq!(p.output_count(1000), 1000); + + // Double-rate QTGMC (FPSDivisor=1, and the unset default): ×2. + p.deinterlace.fps_divisor = Some(1); + assert_eq!(p.output_count(1000), 2000); + p.deinterlace.fps_divisor = None; + assert_eq!(p.output_count(1000), 2000); + + // IVTC cycle 5: ×(cycle-1)/cycle, matching vspipe_total*(cycle-1)/cycle. + p.deinterlace.method = DeinterlaceMethod::Ivtc; + p.deinterlace.fps_divisor = None; + p.deinterlace.ivtc_cycle = Some(5); + assert_eq!(p.output_count(1000), 800); + + // Disabled deinterlace: identity. + p.deinterlace.enabled = false; + assert_eq!(p.output_count(1000), 1000); + } + + #[test] + fn test_pipeline_invert_double_rate() { + let mut p = ProcessingPipeline::default(); + p.deinterlace.enabled = true; + p.deinterlace.method = DeinterlaceMethod::Qtgmc; + p.deinterlace.fps_divisor = Some(1); // double-rate + // output frame 20 ← source frame 10 + let span = p.invert(20); + assert_eq!(span.start, 10); + assert!(span.exact); + } + + #[test] + fn test_total_radius_floored() { + // Even an all-identity pipeline gets the minimum preview window. + let p = ProcessingPipeline::default(); + assert!(p.total_radius() >= MIN_PREVIEW_RADIUS); + } } diff --git a/worker/src/pipeline_executor.rs b/worker/src/pipeline_executor.rs index b251693..fb40605 100644 --- a/worker/src/pipeline_executor.rs +++ b/worker/src/pipeline_executor.rs @@ -69,6 +69,25 @@ fn is_sigpipe(status: &std::process::ExitStatus) -> bool { } } +/// Decode window for a frame-accurate preview of source frame `target`, given +/// a temporal `radius`. Returns `(first, local, num_frames)`: +/// - `first`: first source frame to decode (clamped so it never goes below 0), +/// - `local`: the target's index *within* the decoded window, +/// - `num_frames`: how many source frames to decode — the window is +/// `[first ..= target + radius]`. +/// +/// Pulled out as a pure function so the clamp-at-start behaviour (which the old +/// "middle of 11 frames" heuristic got wrong near the head of the clip) is unit +/// testable without ffmpeg. +fn preview_window(target: i32, radius: i32) -> (i32, i32, i32) { + let target = target.max(0); + let radius = radius.max(0); + let first = (target - radius).max(0); + let local = target - first; + let num_frames = local + radius + 1; + (first, local, num_frames) +} + use crate::dependency_locator::DependencyLocator; use crate::models::{AudioMode, ContainerFormat, DeinterlaceMethod, EncoderFamily, LogLevel, ProgressInfo, SubtitleOutput, VideoCodec, VideoJob}; use crate::progress_reporter::ProgressReporter; @@ -136,13 +155,16 @@ impl PipelineExecutor { // can seek using the container index) let mut decoder_args: Vec = Vec::new(); - // Seek to start frame if specified (time-based, before -i for fast seek) + // Seek to the start frame if specified (before -i for an accurate, + // fast decode-seek). Target the midpoint of the interval *before* + // `start` so the first kept frame (PTS >= seek time) is exactly `start`, + // not start±1 — the boundary `start/fps` is ambiguous under PTS rounding. if let Some(start) = job.start_frame { if start > 0 { let fps = job.input_frame_rate.unwrap_or(29.97); - let start_time = start as f64 / fps; + let seek_time = ((start as f64 - 0.5) / fps).max(0.0); decoder_args.push("-ss".to_string()); - decoder_args.push(format!("{:.6}", start_time)); + decoder_args.push(format!("{:.6}", seek_time)); } } @@ -284,15 +306,10 @@ impl PipelineExecutor { autoload_failed }); - // Determine how deinterlacing affects output frame count + // How the pipeline remaps the frame count (double-rate QTGMC ×2, IVTC + // decimation ×(cycle-1)/cycle, …). vspipe reports the *source* count via + // INPUT_INFO; output_count() turns it into the encoder's output count. let pipeline = job.effective_pipeline(); - let is_double_rate = pipeline.deinterlace.enabled - && pipeline.deinterlace.method == DeinterlaceMethod::Qtgmc - && pipeline.deinterlace.fps_divisor.unwrap_or(1) == 1; - let is_ivtc = pipeline.deinterlace.enabled - && matches!(pipeline.deinterlace.method, - DeinterlaceMethod::Ivtc | DeinterlaceMethod::SoftTelecine); - let ivtc_cycle = pipeline.deinterlace.ivtc_cycle.unwrap_or(5); // Capture ffmpeg stderr in a background thread (for error messages). // Progress comes from the temp file, not stderr. @@ -377,14 +394,7 @@ impl PipelineExecutor { } if vspipe_total > 0 { - let mut effective_total = if is_double_rate { - vspipe_total * 2 - } else if is_ivtc && ivtc_cycle > 1 { - // IVTC VDecimate removes 1 frame per cycle - vspipe_total * (ivtc_cycle - 1) / ivtc_cycle - } else { - vspipe_total - }; + let mut effective_total = pipeline.output_count(vspipe_total as u64) as i32; // Prevent the total from ever decreasing (avoids progress bar jumping backward) if effective_total > max_effective_total { @@ -871,21 +881,23 @@ impl PipelineExecutor { } } - /// Generate a preview frame as PNG to stdout. + /// Generate a preview of a specific source frame as PNG to stdout. /// - /// Decodes frames around the target time with FFmpeg, pipes raw data through - /// VapourSynth for filtering, then encodes the middle frame as PNG. - pub fn generate_preview(&self, job: &VideoJob, time_seconds: f64) -> Result<()> { + /// Frame-accurate: decodes a window of source frames centred on + /// `frame_index`, pipes the raw data through the full VapourSynth pipeline, + /// then emits the exact output frame that `frame_index` maps to (accounting + /// for any frame-rate change such as QTGMC double-rate). The requested frame + /// is echoed back on stderr as `PREVIEW_INFO:frame=N` so the caller can + /// label the result truthfully. + pub fn generate_preview(&self, job: &VideoJob, frame_index: i32) -> Result<()> { use std::io::Write; let ffmpeg_path = self.deps.ffmpeg_path()?; let vspipe_path = self.deps.vspipe_path()?; let env = self.deps.build_environment(); - // Number of frames to extract (need enough for QTGMC temporal processing) - let num_frames = 11; // Decode 11 frames, VapourSynth uses middle one + let target = frame_index.max(0); let frame_rate = job.input_frame_rate.unwrap_or(29.97); - let frame_duration = 1.0 / frame_rate; let pix_fmt = job.input_pixel_format.as_deref().unwrap_or("yuv420p"); let width = job.input_width.unwrap_or(720); let height = job.input_height.unwrap_or(480); @@ -901,10 +913,28 @@ impl PipelineExecutor { ); } - // Calculate start time (go back half the frames for temporal context) - let start_time = (time_seconds - (num_frames as f64 / 2.0) * frame_duration).max(0.0); - - eprintln!("Decoding {} frames starting at {:.3}s ({}x{} {})", num_frames, start_time, width, height, pix_fmt); + // Decode a window [first .. target+radius] of source frames so every + // temporal filter has context, then emit exactly the target frame. + let pipeline = job.effective_pipeline(); + let radius = pipeline.total_radius() as i32; + let (first, local, num_frames) = preview_window(target, radius); + + // Seek so the FIRST decoded frame is exactly `first`. With `-ss` before + // `-i` ffmpeg does an accurate (decode-and-discard) seek that keeps the + // first frame whose PTS >= the seek time. Targeting the midpoint of the + // interval *before* `first` (i.e. (first - 0.5)/fps) lands squarely on + // `first` for CFR sources, avoiding the ±1-frame ambiguity of seeking to + // the frame boundary itself. + let seek_time = ((first as f64 - 0.5) / frame_rate).max(0.0); + + // Output frame index the target source frame maps to after the pipeline + // (e.g. ×2 for QTGMC double-rate). The template clamps defensively. + let output_index = pipeline.output_count(local as u64) as i32; + + eprintln!( + "Preview: source frame {} (window {}..{}, local {}, output index {}) seek {:.6}s ({}x{} {})", + target, first, target + radius, local, output_index, seek_time, width, height, pix_fmt + ); // Determine field order for interlaced content let field_based = if job.qtgmc_parameters.tff == Some(true) { @@ -927,6 +957,7 @@ impl PipelineExecutor { fps_num, fps_den, field_based, + output_index, }; let script_path = script_generator.generate_preview(job, &preview_params)?; @@ -942,7 +973,7 @@ impl PipelineExecutor { let extract_result = Command::new(&ffmpeg_path) .args([ - "-ss", &format!("{:.6}", start_time), + "-ss", &format!("{:.6}", seek_time), "-i", &job.input_path, "-map", "0:v:0", "-frames:v", &num_frames.to_string(), @@ -1052,6 +1083,10 @@ impl PipelineExecutor { bail!("ffmpeg encoder exited with {}", format_exit_status(&output.status)); } + // Echo the frame we actually rendered so the caller can label it (the + // PNG itself is the only thing on stdout, so this goes to stderr). + eprintln!("PREVIEW_INFO:frame={}", target); + // Write PNG to stdout std::io::stdout().write_all(&output.stdout)?; std::io::stdout().flush()?; @@ -1085,6 +1120,29 @@ mod tests { use crate::models::{AudioCodec, AudioQuality, EncodingSettings, QTGMCParameters, VideoCodec}; use uuid::Uuid; + #[test] + fn test_preview_window_centered() { + // Mid-clip: window is symmetric, target sits at `radius` within it. + assert_eq!(preview_window(100, 5), (95, 5, 11)); + } + + #[test] + fn test_preview_window_clamped_near_start() { + // Near the head the window can't extend below frame 0, so `first` + // clamps and the target's local index shrinks accordingly — the case + // the old "middle of 11" heuristic rendered as the wrong frame. + assert_eq!(preview_window(2, 5), (0, 2, 8)); + assert_eq!(preview_window(0, 5), (0, 0, 6)); + } + + #[test] + fn test_preview_window_zero_radius() { + // Degenerate radius still yields a single-frame window at the target. + assert_eq!(preview_window(42, 0), (42, 0, 1)); + // Negative inputs are clamped, never panicking. + assert_eq!(preview_window(-3, 5), (0, 0, 6)); + } + #[test] fn test_is_autoload_skip_line() { // The two real VapourSynth phrasings seen in the wild (missing namespace diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index bad9b85..9a6bbdf 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -46,6 +46,9 @@ pub struct PreviewParams { pub fps_den: i32, /// Field order: 1 = BFF, 2 = TFF pub field_based: i32, + /// Output frame index to emit as the preview (after the pipeline runs on + /// the decoded window). Accounts for any frame-rate change in the pipeline. + pub output_index: i32, } impl ScriptGenerator { @@ -107,6 +110,7 @@ impl ScriptGenerator { script = script.replace("{{TOTAL_FRAMES}}", &preview_params.num_frames.to_string()); script = script.replace("{{FPS_NUM}}", &preview_params.fps_num.to_string()); script = script.replace("{{FPS_DEN}}", &preview_params.fps_den.to_string()); + script = script.replace("{{PREVIEW_OUTPUT_INDEX}}", &preview_params.output_index.to_string()); // Field-based: only set when deinterlacing is enabled if pipeline.deinterlace.enabled { script = script.replace("{{#SET_FIELD_BASED}}", ""); diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 201f2ca..faed109 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -791,8 +791,11 @@ clip = core.resize.Bilinear(clip, width=target_w, height=target_h) {{/RESIZE}} # ============================================================================ -# OUTPUT - select the middle frame for preview +# OUTPUT - select the exact target frame for preview. +# The worker decodes a window of source frames around the requested frame and +# passes the deterministic output index it maps to (after any frame-rate change +# such as QTGMC double-rate). Clamp defensively in case the decode came up short. # ============================================================================ -middle_frame = clip.num_frames // 2 -clip = clip[middle_frame] +target_frame = max(0, min({{PREVIEW_OUTPUT_INDEX}}, clip.num_frames - 1)) +clip = clip[target_frame] clip.set_output() diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 1489df5..ae2fb16 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -7,7 +7,7 @@ use uuid::Uuid; // Import the worker's models use vapourbox_worker::models::*; -use vapourbox_worker::script_generator::ScriptGenerator; +use vapourbox_worker::script_generator::{PreviewParams, ScriptGenerator}; fn get_test_input() -> PathBuf { let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); @@ -1901,3 +1901,79 @@ fn test_54_qtgmc_knlmeanscl_falls_back_to_dfttest() { script ); } + +#[test] +fn test_55_preview_selects_exact_frame() { + // Frame-accurate preview: the generated preview script must emit the exact + // output index the worker computed for the requested frame, NOT the old + // "middle of the decoded window" heuristic. + create_output_dir(); + + let mut job = create_base_job("test_55_preview_frame"); + job.qtgmc_parameters.enabled = true; + job.qtgmc_parameters.tff = Some(true); + job.processing_pipeline = Some(ProcessingPipeline { + deinterlace: job.qtgmc_parameters.clone(), + ..ProcessingPipeline::default() + }); + + let generator = ScriptGenerator::new().expect("create generator"); + let params = PreviewParams { + width: 720, + height: 576, + pix_fmt: "yuv420p".to_string(), + num_frames: 11, + fps_num: 25, + fps_den: 1, + field_based: 2, + output_index: 7, + }; + let script_path = generator + .generate_preview(&job, ¶ms) + .expect("generate preview script"); + let script = std::fs::read_to_string(&script_path).expect("read preview script"); + + assert!( + script.contains("min(7, clip.num_frames - 1)"), + "preview must select the exact computed output index, script was:\n{}", + script + ); + assert!( + !script.contains("num_frames // 2"), + "preview must not fall back to the old middle-frame heuristic" + ); + let _ = std::fs::remove_file(&script_path); +} + +#[test] +fn test_56_frame_count_mapping() { + // The progress reporter derives the output frame total from output_count(); + // verify the composed map reproduces the deinterlace count transforms. + let mut p = ProcessingPipeline::default(); + p.deinterlace.enabled = true; + p.deinterlace.method = DeinterlaceMethod::Qtgmc; + + // Double-rate QTGMC (FPSDivisor=1, and the unset default) doubles frames. + p.deinterlace.fps_divisor = Some(1); + assert_eq!(p.output_count(1000), 2000); + p.deinterlace.fps_divisor = None; + assert_eq!(p.output_count(1000), 2000); + + // Single-rate QTGMC leaves the count unchanged. + p.deinterlace.fps_divisor = Some(2); + assert_eq!(p.output_count(1000), 1000); + + // IVTC cycle 5 keeps 4 of every 5 frames (30→24). + p.deinterlace.method = DeinterlaceMethod::Ivtc; + p.deinterlace.fps_divisor = None; + p.deinterlace.ivtc_cycle = Some(5); + assert_eq!(p.output_count(1000), 800); + + // A double-rate output frame inverts back to its source frame, exactly. + p.deinterlace.method = DeinterlaceMethod::Qtgmc; + p.deinterlace.fps_divisor = Some(1); + p.deinterlace.ivtc_cycle = None; + let span = p.invert(20); + assert_eq!(span.start, 10); + assert!(span.exact); +}