From 9dfd0b1c64d3931249643c0fcc0931c3521f2606 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Sun, 14 Jun 2026 20:19:03 +1000 Subject: [PATCH] worker: probe input frame count when the job omits total_frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pipe_source builds a fixed-length clip from TOTAL_FRAMES, and the worker defaulted that to 1 when a job had no total_frames — so direct callers (the integration tests, CLI) produced single-frame output, breaking every frame-count / frame-diff / baseline heavy test. (The Flutter app probes and sets total_frames, so production was unaffected.) This looked like a "lagarith won't decode" bug but the codec was a red herring — the count was just 1. - DependencyLocator::probe_frame_count() counts decodable frames via ffprobe -count_frames (accurate; header nb_frames is wrong for AVI/lossless — the lagarith clip reports 79 but decodes 75). - main.rs resolves total_frames when absent, using the *post-trim* count (start/end_frame) so it matches the decoder's range logic and pipe_source's clip length. Reuses one DependencyLocator for this and the OpenCL probe. - integration_audio_conversion_test: hash raw PCM (s16le), not WAV — a WAV header varies by source container so a passthrough copy hashed differently from its source (same fix already applied to the filter_pipeline helper). Heavy suite locally: 123 passed, 1 skipped (whisper add-on absent), 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration_audio_conversion_test.dart | 8 +++-- worker/src/dependency_locator.rs | 30 +++++++++++++++++ worker/src/main.rs | 32 ++++++++++++++++--- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/app/test/integration_audio_conversion_test.dart b/app/test/integration_audio_conversion_test.dart index cdbf8e3..5af3f3f 100644 --- a/app/test/integration_audio_conversion_test.dart +++ b/app/test/integration_audio_conversion_test.dart @@ -109,15 +109,19 @@ Future getAudioStreamInfo(String videoPath) async { Future extractAudioHash(String videoPath) async { final ffmpegPath = WorkerHarness.ffmpegPath; final tempDir = Directory.systemTemp; - final audioPath = '${tempDir.path}/audio_${DateTime.now().millisecondsSinceEpoch}.wav'; + final audioPath = '${tempDir.path}/audio_${DateTime.now().millisecondsSinceEpoch}.raw'; try { - // Extract audio as raw PCM WAV (lossless extraction for comparison) + // Extract headerless raw PCM (s16le), NOT a WAV. A WAV header varies by + // source container metadata even when the samples are identical, which makes + // a passthrough copy hash differently from its source. Raw PCM hashes the + // audio content only. await Process.run( ffmpegPath, [ '-i', videoPath, '-vn', // No video + '-f', 's16le', // Headerless raw PCM container '-acodec', 'pcm_s16le', // Convert to PCM for consistent comparison '-ar', '48000', // Resample to 48kHz for consistency '-ac', '2', // Stereo diff --git a/worker/src/dependency_locator.rs b/worker/src/dependency_locator.rs index 1a6bd10..09863b9 100644 --- a/worker/src/dependency_locator.rs +++ b/worker/src/dependency_locator.rs @@ -313,6 +313,36 @@ impl DependencyLocator { String::new() } + /// Count the decodable video frames in `input` via ffprobe. Returns None if + /// ffprobe is unavailable or the count can't be determined. Uses + /// `-count_frames` (decodes) for an accurate count that matches what the + /// decoder will actually pipe — header `nb_frames` is often wrong for AVI / + /// lossless codecs (e.g. our lagarith test clip reports 79 but decodes 75). + pub fn probe_frame_count(&self, input: &str) -> Option { + let ffprobe = self.ffprobe_path().ok()?; + let out = Command::new(&ffprobe) + .args([ + "-v", "error", + "-select_streams", "v:0", + "-count_frames", + "-show_entries", "stream=nb_read_frames", + "-of", "default=nokey=1:noprint_wrappers=1", + input, + ]) + .envs(self.build_environment()) + .stderr(Stdio::null()) + .output() + .ok()?; + if !out.status.success() { + return None; + } + String::from_utf8_lossy(&out.stdout) + .trim() + .parse::() + .ok() + .filter(|&n| n > 0) + } + /// Get the path to ffmpeg executable. pub fn ffmpeg_path(&self) -> Result { let exe_name = if cfg!(windows) { "ffmpeg.exe" } else { "ffmpeg" }; diff --git a/worker/src/main.rs b/worker/src/main.rs index f491976..6940499 100644 --- a/worker/src/main.rs +++ b/worker/src/main.rs @@ -280,7 +280,7 @@ fn run_worker( reporter.send_log(models::LogLevel::Info, "Loading job configuration..."); let config_content = std::fs::read_to_string(config_path) .with_context(|| format!("Failed to read config file: {:?}", config_path))?; - let job: VideoJob = serde_json::from_str(&config_content) + let mut job: VideoJob = serde_json::from_str(&config_content) .with_context(|| "Failed to parse job configuration")?; reporter.send_log( @@ -301,13 +301,37 @@ fn run_worker( job.qtgmc_parameters.preset.as_str()), ); + // Resolve deps once for the pre-generation probes below. + let deps = dependency_locator::DependencyLocator::new().ok(); + + // Resolve the frame count if the caller didn't supply one. The Flutter app + // probes and sets total_frames; direct callers (tests, CLI) may omit it, and + // the script's pipe_source needs an exact length (it builds a fixed-size + // clip). Without this the worker defaults to 1 frame and the whole output is + // a single frame. total_frames must be the *post-trim* count the decoder + // will actually pipe (trimming is decoder-side: -ss + -frames:v), matching + // the decoder's own range logic in pipeline_executor. + if job.total_frames.is_none() { + if let Some(full) = deps.as_ref().and_then(|d| d.probe_frame_count(&job.input_path)) { + let effective = match (job.start_frame, job.end_frame) { + (Some(s), Some(e)) => (e - s + 1).max(0), + (None, Some(e)) => e + 1, + (Some(s), None) => (full - s).max(0), + (None, None) => full, + }; + reporter.send_log( + models::LogLevel::Debug, + &format!("Probed input frame count: {} (effective after trim: {})", full, effective), + ); + job.total_frames = Some(effective); + } + } + // Generate VapourSynth script reporter.send_log(models::LogLevel::Info, "Generating VapourSynth script..."); // Detect OpenCL availability so QTGMC falls back to CPU NNEDI3 on machines // without a usable OpenCL device (headless CI, VMs, remote desktop). - let opencl_available = dependency_locator::DependencyLocator::new() - .map(|d| d.opencl_available()) - .unwrap_or(true); + let opencl_available = deps.as_ref().map(|d| d.opencl_available()).unwrap_or(true); let script_generator = ScriptGenerator::new()?.with_opencl_available(opencl_available); let script_path = script_generator .generate(&job)