fix(worker): count retimed frames in the progress total (#134)

CLAUDE.md

@@ -1103,7 +1103,11 @@ than fail cleanly.
   frame it would merely copy (an exact source position, the held tail) is
   taken from the source by `FrameEval`, so photographed frames are bit-exact.
   The count is `floor(n * num / den)` of the reduced ratio — exactly
-  `FrameMap::Retime`. `sc=True` with `misc.SCDetect` holds the frame across a
+  `FrameMap::Retime`. That ratio needs the source rate, which the app does
+  **not** send: `VideoJob::effective_pipeline` fills `sourceFpsNum/Den` from
+  the job's input rate, and the ratio is taken against the rate *at the pass*
+  (×2 after double-rate deinterlacing). Without that the progress total stays
+  at the source count and pins at 100% with no ETA (issue #134). `sc=True` with `misc.SCDetect` holds the frame across a
   cut rather than blending two shots.
 - **Upscaling: models, factors, tiles.** Five of vs-mlrt's ONNX conversions
   ship (`gpu_plugin::UPSCALE_MODELS`): Real-ESRGAN animevideo 2x/4x and

app/test/integration_frame_mapping_test.dart

@@ -22,13 +22,16 @@
 ///   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)
+///   Fanout then Retime to 25  31 -> 62 -> 31 (the retime converts from the
+///                             doubled 50p, not the source's 25)
 ///
 /// 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:convert';
 import 'dart:io';
 import 'dart:typed_data';
 
@@ -115,6 +118,26 @@ Future<int> _countFrames(String path) async {
   return n!;
 }
 
+/// Every progress total the worker reported — what the app shows as
+/// "frame / total" and derives the percentage and ETA from. The last one is
+/// always the frames written (the worker sends a final 100% with the real
+/// count), so only the ones before it say whether the total was right while
+/// the encode ran. A clip this short may finish before the first poll, leaving
+/// none; `video_job.rs`'s frame_rate_progress_tests are the deterministic
+/// guard, this one checks the real pipeline agrees whenever it can.
+List<int> _progressTotals(JobResult r) {
+  final totals = <int>[];
+  for (final line in r.logs) {
+    if (!line.startsWith('{')) continue;
+    try {
+      final json = jsonDecode(line) as Map<String, dynamic>;
+      final total = json['totalFrames'];
+      if (json['type'] == 'progress' && total is int) totals.add(total);
+    } catch (_) {/* not a worker message */}
+  }
+  return totals;
+}
+
 /// One frame of a video as packed rgb24, by absolute frame index.
 Future<Uint8List> _frameRgb(String path, int index) async {
   final out = File(p.join(Directory.systemTemp.path,
@@ -260,11 +283,11 @@ void main() {
         input: _interlaced,
         pipeline: const ProcessingPipeline(
           deinterlace: QTGMCParameters(enabled: false),
+          // No sourceFpsNum/Den: the app never sends them (issue #134), so
+          // the worker must work the ratio out from the job's input rate.
           frameRate: FrameRateParameters(
             enabled: true,
             target: FrameRateTarget.film23976,
-            sourceFpsNum: 25,
-            sourceFpsDen: 1,
           ),
         ),
         width: 720,
@@ -284,6 +307,46 @@ void main() {
               '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.');
+      print('  progress totals: ${_progressTotals(r).toSet()}');
+      expect(_progressTotals(r), everyElement(n),
+          reason: 'issue #134: a total left at the source count pins '
+              'progress at 100% with no ETA for the rest of the encode');
+    });
+
+    test('a frame rate after double-rate QTGMC converts from the doubled rate',
+        () async {
+      // 25i doubled to 50p, then to 25: 50 -> 25 halves, so 31 -> 62 -> 31.
+      // Taking the ratio against the source's 25 would claim 62.
+      const start = 10, end = 40;
+      const sourceFrames = end - start + 1; // 31
+      final job = _job(
+        'double_rate_retime',
+        input: _interlaced,
+        pipeline: const ProcessingPipeline(
+          deinterlace: QTGMCParameters(
+              preset: QTGMCPreset.superFast, tff: true, fpsDivisor: 1),
+          frameRate: FrameRateParameters(
+            enabled: true,
+            target: FrameRateTarget.pal25,
+          ),
+        ),
+        width: 720,
+        height: 576,
+        pixFmt: 'yuv422p',
+        fps: _interlacedFps,
+        totalFrames: sourceFrames,
+        startFrame: start,
+        endFrame: end,
+      );
+
+      final r = await WorkerHarness.runJob(job.toJson(),
+          label: 'double_rate_retime');
+      expect(r.success, isTrue, reason: r.error);
+      final n = await _countFrames(r.outputPath!);
+      print('  $sourceFrames source -> $n output (expected $sourceFrames)');
+      expect(n, sourceFrames);
+      print('  progress totals: ${_progressTotals(r).toSet()}');
+      expect(_progressTotals(r), everyElement(n));
     });
   });
 
@@ -396,8 +459,6 @@ void main() {
             enabled: true,
             target: FrameRateTarget.pal50,
             method: FrameRateMethod.rife,
-            sourceFpsNum: 25,
-            sourceFpsDen: 1,
           ),
         );
 

worker/src/models/frame_rate_parameters.rs

@@ -105,9 +105,10 @@ pub struct FrameRateParameters {
     #[serde(default = "default_overlap")]
     pub overlap: i32,
 
-    /// The source rate, supplied by the app from ffprobe. The pipeline needs it
-    /// to report a correct `FrameMap::Retime`, and cannot see the job — the
-    /// same reason `inputSar` is carried rather than looked up.
+    /// The source rate. The pipeline needs it to report a correct
+    /// `FrameMap::Retime`, and cannot see the job, so
+    /// `VideoJob::effective_pipeline` fills it from the job's input rate when
+    /// absent — the app never sends it (issue #134).
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub source_fps_num: Option<u32>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -160,14 +161,9 @@ impl FrameRateParameters {
     fn as_fraction(&self) -> (u32, u32) {
         self.target.as_fraction()
     }
-
-    /// The retime ratio using the carried source rate, if the app supplied one.
-    pub fn ratio(&self) -> Option<(u32, u32)> {
-        self.ratio_against(self.source_fps_num?, self.source_fps_den?)
-    }
 }
 
-fn gcd(a: u64, b: u64) -> u64 {
+pub(super) fn gcd(a: u64, b: u64) -> u64 {
     if b == 0 { a.max(1) } else { gcd(b, a % b) }
 }
 

worker/src/models/processing_pipeline.rs

@@ -2,6 +2,8 @@
 
 use serde::{Deserialize, Serialize};
 
+use super::frame_rate_parameters::gcd;
+
 use super::{
     AntiAliasParameters, GeometryParameters, GrainParameters, StabilizeParameters,
     ChromaDenoiseParameters, ChromaFixParameters, ColorCorrectionParameters, CropResizeParameters,
@@ -67,6 +69,17 @@ impl FrameMap {
         }
     }
 
+    /// How the map scales the frame rate, as num/den: a pass that emits more
+    /// frames over the same duration raises the rate by the same factor.
+    pub fn rate_factor(&self) -> (u64, u64) {
+        match *self {
+            FrameMap::Identity { .. } => (1, 1),
+            FrameMap::Fanout { factor, .. } => (factor as u64, 1),
+            FrameMap::Decimate { cycle, keep } => (keep as u64, cycle as u64),
+            FrameMap::Retime { num, den, .. } => (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 {
@@ -542,15 +555,15 @@ impl ProcessingPipeline {
 
     /// Frame mapping for the frame rate pass.
     ///
-    /// The source rate is not known to the pipeline, so this reports the
-    /// identity and the executor substitutes the real ratio once vspipe has
-    /// reported the input rate. Reporting a wrong fixed ratio here would make
-    /// the progress total and the preview seek disagree with the output.
+    /// Needs the source rate, which `VideoJob::effective_pipeline` fills in;
+    /// a pipeline built without one reports the identity. Reporting a wrong
+    /// fixed ratio here would make the progress total and the preview seek
+    /// disagree with the output.
     fn frame_rate_frame_map(&self) -> FrameMap {
         // Without a source rate we cannot know the ratio, and a wrong fixed
         // ratio is worse than none: it would make the progress total and the
         // preview index disagree with what the encoder actually receives.
-        match self.frame_rate.ratio() {
+        match self.frame_rate_input_fps().and_then(|(n, d)| self.frame_rate.ratio_against(n, d)) {
             Some((num, den)) => FrameMap::Retime {
                 num,
                 den,
@@ -563,6 +576,29 @@ impl ProcessingPipeline {
         }
     }
 
+    /// The rate the frame rate pass converts *from*: the source rate as
+    /// changed by every pass ahead of it. Double-rate deinterlacing hands it
+    /// 50p from a 25i source, and its target is absolute, so the ratio has to
+    /// be taken against that, not against the source.
+    fn frame_rate_input_fps(&self) -> Option<(u32, u32)> {
+        let (mut num, mut den) = (
+            self.frame_rate.source_fps_num? as u64,
+            self.frame_rate.source_fps_den? as u64,
+        );
+        for pass in self.enabled_passes() {
+            if pass == PassType::FrameRate {
+                break;
+            }
+            let (f_num, f_den) = self.frame_map_for(pass).rate_factor();
+            num *= f_num;
+            den *= f_den;
+            let g = gcd(num, den);
+            num /= g;
+            den /= g;
+        }
+        Some((u32::try_from(num).ok()?, u32::try_from(den).ok()?))
+    }
+
     /// 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

worker/src/models/video_job.rs

@@ -127,11 +127,58 @@ impl VideoJob {
 
     /// Get the effective processing pipeline.
     /// Uses processing_pipeline if set, otherwise creates one from legacy qtgmc_parameters.
+    ///
+    /// The frame rate pass needs the source rate to know its retime ratio, and
+    /// the pipeline cannot see the job, so it is filled in here from the same
+    /// rate the script declares for the clip. Without it the pass maps as the
+    /// identity: the progress total stays at the source count while RIFE or
+    /// FlowFPS emit more frames, so progress pins at 100% with no ETA for the
+    /// rest of the encode (issue #134), and the preview seeks the wrong frame.
     pub fn effective_pipeline(&self) -> ProcessingPipeline {
-        self.processing_pipeline
+        let mut pipeline = self
+            .processing_pipeline
             .clone()
-            .unwrap_or_else(|| ProcessingPipeline::from_legacy(&self.qtgmc_parameters))
+            .unwrap_or_else(|| ProcessingPipeline::from_legacy(&self.qtgmc_parameters));
+        let fr = &mut pipeline.frame_rate;
+        if fr.source_fps_num.is_none() || fr.source_fps_den.is_none() {
+            let (num, den) = self.input_fps_rational();
+            fr.source_fps_num = Some(num);
+            fr.source_fps_den = Some(den);
+        }
+        pipeline
+    }
+
+    /// The source rate as the script declares it to VapourSynth, 29.97 when
+    /// the job does not say.
+    pub fn input_fps_rational(&self) -> (u32, u32) {
+        let (num, den) = fps_to_rational(self.input_frame_rate.unwrap_or(29.97));
+        (num.max(1) as u32, den.max(1) as u32)
+    }
+}
+
+/// Convert a floating-point frame rate to a rational number (num/den).
+pub fn fps_to_rational(fps: f64) -> (i32, i32) {
+    // Common frame rates
+    let common = [
+        (23.976, 24000, 1001),
+        (24.0, 24, 1),
+        (25.0, 25, 1),
+        (29.97, 30000, 1001),
+        (30.0, 30, 1),
+        (50.0, 50, 1),
+        (59.94, 60000, 1001),
+        (60.0, 60, 1),
+    ];
+
+    for (rate, num, den) in common {
+        if (fps - rate).abs() < 0.01 {
+            return (num, den);
+        }
     }
+
+    // Fallback: multiply by 1000 and simplify
+    let num = (fps * 1000.0).round() as i32;
+    (num, 1000)
 }
 
 /// Subtitle generation settings using Whisper AI.
@@ -1079,6 +1126,76 @@ impl FieldOrder {
     }
 }
 
+#[cfg(test)]
+mod frame_rate_progress_tests {
+    use super::*;
+    use crate::models::{DeinterlaceMethod, FrameRateMethod, FrameRateParameters, FrameRateTarget};
+
+    fn job(fps: f64, deinterlace: bool, frame_rate: FrameRateParameters) -> VideoJob {
+        let mut job: VideoJob = serde_json::from_value(serde_json::json!({
+            "id": "00000000-0000-0000-0000-000000000000",
+            "inputPath": "in.mkv",
+            "outputPath": "out.mkv",
+            "qtgmcParameters": {},
+            "encodingSettings": {},
+        }))
+        .unwrap();
+        job.input_frame_rate = Some(fps);
+        let mut pipeline = ProcessingPipeline::default();
+        pipeline.deinterlace.enabled = deinterlace;
+        pipeline.deinterlace.method = DeinterlaceMethod::Qtgmc;
+        pipeline.frame_rate = frame_rate;
+        job.processing_pipeline = Some(pipeline);
+        job
+    }
+
+    fn rife_to(target: FrameRateTarget) -> FrameRateParameters {
+        FrameRateParameters {
+            enabled: true,
+            target,
+            method: FrameRateMethod::Rife,
+            ..Default::default()
+        }
+    }
+
+    /// Issue #134: the app never sends the frame rate pass its source rate, so
+    /// the progress total used to stay at the source count while RIFE doubled
+    /// it — 100% at the halfway point and no ETA for the rest.
+    #[test]
+    fn the_progress_total_counts_the_retimed_frames_without_an_app_supplied_rate() {
+        let j = job(23.976, false, rife_to(FrameRateTarget::Ntsc5994));
+        // 24000/1001 -> 60000/1001 is exactly 5/2.
+        assert_eq!(j.effective_pipeline().output_count(1775), 1775 * 5 / 2);
+    }
+
+    #[test]
+    fn a_rate_the_app_did_supply_is_kept() {
+        let mut fr = rife_to(FrameRateTarget::Pal50);
+        fr.source_fps_num = Some(25);
+        fr.source_fps_den = Some(1);
+        // The job claims 29.97; the carried rate wins.
+        assert_eq!(job(29.97, false, fr).effective_pipeline().output_count(100), 200);
+    }
+
+    /// The target is absolute, so it converts from the rate deinterlacing
+    /// hands it, not the source's: 25i double-rate is 50p, and 50p -> 25 halves.
+    #[test]
+    fn the_ratio_is_taken_after_double_rate_deinterlacing() {
+        let j = job(25.0, true, rife_to(FrameRateTarget::Pal25));
+        assert_eq!(j.effective_pipeline().output_count(100), 100);
+        let j = job(25.0, true, rife_to(FrameRateTarget::Ntsc5994));
+        // 50 -> 59.94 is 1200/1001.
+        assert_eq!(j.effective_pipeline().output_count(100), 200 * 1200 / 1001);
+    }
+
+    #[test]
+    fn the_preview_inverse_uses_the_same_ratio() {
+        let j = job(25.0, false, rife_to(FrameRateTarget::Pal50));
+        let span = j.effective_pipeline().invert(10);
+        assert_eq!(span.start, 5);
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;