From 1b70a6f0b46fe63a3b2a9721f1976c4d90de559d Mon Sep 17 00:00:00 2001 From: Cameron Date: Sat, 16 May 2026 21:03:21 -0400 Subject: [PATCH 1/3] video: probe frame rate via ffprobe and return on /video/generate Adds frame_rate to GenerateVideoResponse so the mobile scrubber can step at the source's real fps instead of a hardcoded 30. probe_video_stream_meta gains a frame_rate field (avg_frame_rate preferred, r_frame_rate fallback, nonsense values rejected) and is now pub so the handler can reuse it. Cost is one ffprobe per /video/generate call; degrades silently to None on probe failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/handlers/video.rs | 17 +++++++++++++- src/video/actors.rs | 52 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/handlers/video.rs b/src/handlers/video.rs index 1be529f..42bc204 100644 --- a/src/handlers/video.rs +++ b/src/handlers/video.rs @@ -25,7 +25,9 @@ use crate::files::is_valid_full_path; use crate::libraries; use crate::otel::{extract_context_from_request, global_tracer}; use crate::state::AppState; -use crate::video::actors::{GeneratePreviewClipMessage, QueueVideosMessage, VideoToQueue}; +use crate::video::actors::{ + GeneratePreviewClipMessage, QueueVideosMessage, VideoToQueue, probe_video_stream_meta, +}; use crate::video::hls_paths; /// Response body for `POST /video/generate`. Clients consume @@ -46,6 +48,11 @@ struct GenerateVideoResponse { /// transcode was queued; clients should retry the URL after a short /// delay (or rely on HLS.js's own retry policy). ready: bool, + /// Source-video frame rate in Hz, probed via ffprobe. `None` when the + /// probe failed or ffprobe couldn't parse either rate field — clients + /// fall back to their own default (typically 30) for frame stepping. + #[serde(skip_serializing_if = "Option::is_none")] + frame_rate: Option, } #[post("/video/generate")] @@ -182,11 +189,19 @@ pub async fn generate_video( hls_paths::PLAYLIST_FILENAME ); + // Probe the source for frame rate so the mobile scrubber can step at + // the right interval. Cheap (~tens of ms) and only runs once per video + // open. Probe failures degrade silently — clients have a fallback. + let frame_rate = probe_video_stream_meta(&full_path.to_string_lossy()) + .await + .frame_rate; + span.set_status(Status::Ok); HttpResponse::Ok().json(GenerateVideoResponse { playlist_url, content_hash: content_hash_str, ready, + frame_rate, }) } diff --git a/src/video/actors.rs b/src/video/actors.rs index 9f2df1b..27d599b 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -122,16 +122,21 @@ pub fn generate_image_thumbnail_ffmpeg(path: &Path, destination: &Path) -> std:: /// Video stream metadata needed to pick HLS encode settings. Populated by /// a single ffprobe call to avoid spawning multiple subprocesses per video. #[derive(Debug, Default)] -struct VideoStreamMeta { - is_h264: bool, +pub struct VideoStreamMeta { + pub is_h264: bool, /// Rotation in degrees (0/90/180/270). Checks both the legacy `rotate` /// stream tag and the modern display-matrix side data. - rotation: i32, + pub rotation: i32, + /// Frames per second. Prefers `avg_frame_rate` (handles VFR better than + /// `r_frame_rate`, which lies on variable-framerate sources). `None` + /// when ffprobe couldn't parse either field — caller picks a fallback. + pub frame_rate: Option, } /// Probe video stream metadata in one ffprobe call. Returns default (codec -/// unknown, rotation 0) on any failure — callers fall back to transcoding. -async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta { +/// unknown, rotation 0, fps None) on any failure — callers fall back to +/// transcoding / a default framerate. +pub async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta { let output = tokio::process::Command::new("ffprobe") .arg("-v") .arg("error") @@ -140,7 +145,7 @@ async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta { .arg("-print_format") .arg("json") .arg("-show_entries") - .arg("stream=codec_name:stream_tags=rotate:side_data_list") + .arg("stream=codec_name,r_frame_rate,avg_frame_rate:stream_tags=rotate:side_data_list") .arg(video_path) .output() .await; @@ -191,12 +196,41 @@ async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta { }) .unwrap_or(0); + // ffprobe reports frame rates as rational strings like "30000/1001". + // Prefer avg_frame_rate (handles VFR) and fall back to r_frame_rate. + // Reject 0/0 (unknown), negative numbers, and anything wildly out of + // range so a malformed probe can't poison the scrubber's step size. + let parse_rational = |s: &str| -> Option { + let (num, den) = s.split_once('/')?; + let num: f32 = num.parse().ok()?; + let den: f32 = den.parse().ok()?; + if den.abs() < f32::EPSILON { + return None; + } + let v = num / den; + (v.is_finite() && v > 0.0 && v < 1000.0).then_some(v) + }; + let frame_rate = stream + .get("avg_frame_rate") + .and_then(|v| v.as_str()) + .and_then(parse_rational) + .or_else(|| { + stream + .get("r_frame_rate") + .and_then(|v| v.as_str()) + .and_then(parse_rational) + }); + debug!( - "Probed {}: codec_h264={}, rotation={}°", - video_path, is_h264, rotation + "Probed {}: codec_h264={}, rotation={}°, fps={:?}", + video_path, is_h264, rotation, frame_rate ); - VideoStreamMeta { is_h264, rotation } + VideoStreamMeta { + is_h264, + rotation, + frame_rate, + } } /// Probe the max keyframe interval (GOP) in the first ~30s of a video. -- 2.49.1 From bd61e101580c27ec10fc5370933e812a56ada5fb Mon Sep 17 00:00:00 2001 From: Cameron Date: Sat, 16 May 2026 21:13:06 -0400 Subject: [PATCH 2/3] chore: add .gitattributes + unit tests for ffprobe rational parser LF normalization across OSes; *.sql pinned to LF for stable diffs. Tests cover the rational frame-rate parser (NTSC 29.97, integer fps, slow-mo 240, ffprobe's 0/0 unknown sentinel, malformed and out-of-range inputs). Extracted the closure into a free fn for the test seam. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitattributes | 9 ++++++ src/video/actors.rs | 79 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..7ed1241 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,9 @@ +# Normalize line endings in the repo to LF. Windows checkouts can still +# present working-copy files as CRLF; this just keeps the committed history +# stable so contributors on any OS don't see whitespace-only diffs every +# time someone touches a file. +* text=auto eol=lf + +# Migrations and SQL must be LF — SQLite parsers don't care, but diffing +# is much cleaner with stable endings. +*.sql text eol=lf diff --git a/src/video/actors.rs b/src/video/actors.rs index 27d599b..8d04ea1 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -133,6 +133,21 @@ pub struct VideoStreamMeta { pub frame_rate: Option, } +/// Parse ffprobe's rational frame-rate strings (`"30000/1001"`, +/// `"60/1"`, `"0/0"`). Rejects 0/0 (ffprobe's "unknown" sentinel), +/// non-positive results, and anything wildly out of range so a malformed +/// probe can't poison the scrubber's step size. +fn parse_ffprobe_rational(s: &str) -> Option { + let (num, den) = s.split_once('/')?; + let num: f32 = num.parse().ok()?; + let den: f32 = den.parse().ok()?; + if den.abs() < f32::EPSILON { + return None; + } + let v = num / den; + (v.is_finite() && v > 0.0 && v < 1000.0).then_some(v) +} + /// Probe video stream metadata in one ffprobe call. Returns default (codec /// unknown, rotation 0, fps None) on any failure — callers fall back to /// transcoding / a default framerate. @@ -198,27 +213,15 @@ pub async fn probe_video_stream_meta(video_path: &str) -> VideoStreamMeta { // ffprobe reports frame rates as rational strings like "30000/1001". // Prefer avg_frame_rate (handles VFR) and fall back to r_frame_rate. - // Reject 0/0 (unknown), negative numbers, and anything wildly out of - // range so a malformed probe can't poison the scrubber's step size. - let parse_rational = |s: &str| -> Option { - let (num, den) = s.split_once('/')?; - let num: f32 = num.parse().ok()?; - let den: f32 = den.parse().ok()?; - if den.abs() < f32::EPSILON { - return None; - } - let v = num / den; - (v.is_finite() && v > 0.0 && v < 1000.0).then_some(v) - }; let frame_rate = stream .get("avg_frame_rate") .and_then(|v| v.as_str()) - .and_then(parse_rational) + .and_then(parse_ffprobe_rational) .or_else(|| { stream .get("r_frame_rate") .and_then(|v| v.as_str()) - .and_then(parse_rational) + .and_then(parse_ffprobe_rational) }); debug!( @@ -820,3 +823,51 @@ impl Handler for PreviewClipGenerator { }) } } + +#[cfg(test)] +mod tests { + use super::parse_ffprobe_rational; + + #[test] + fn parses_common_rational_framerates() { + // NTSC 29.97 fps + assert!((parse_ffprobe_rational("30000/1001").unwrap() - 29.970_03).abs() < 1e-3); + // Plain integer fps + assert!((parse_ffprobe_rational("30/1").unwrap() - 30.0).abs() < 1e-6); + assert!((parse_ffprobe_rational("60/1").unwrap() - 60.0).abs() < 1e-6); + // iPhone slow-mo + assert!((parse_ffprobe_rational("240/1").unwrap() - 240.0).abs() < 1e-6); + } + + #[test] + fn rejects_ffprobe_unknown_sentinel() { + // 0/0 is ffprobe's way of saying "I don't know" — must not be + // interpreted as 0 fps. + assert_eq!(parse_ffprobe_rational("0/0"), None); + } + + #[test] + fn rejects_malformed_input() { + assert_eq!(parse_ffprobe_rational(""), None); + assert_eq!(parse_ffprobe_rational("30"), None); + assert_eq!(parse_ffprobe_rational("/1"), None); + assert_eq!(parse_ffprobe_rational("30/"), None); + assert_eq!(parse_ffprobe_rational("abc/def"), None); + } + + #[test] + fn rejects_non_positive_results() { + // Negative numerator -> negative fps; meaningless. + assert_eq!(parse_ffprobe_rational("-30/1"), None); + // Zero numerator -> zero fps; also meaningless for frame stepping. + assert_eq!(parse_ffprobe_rational("0/1"), None); + } + + #[test] + fn rejects_out_of_range() { + // Anything > 1000 fps is almost certainly garbage probe output, + // not a real source. (Real high-speed capture maxes near 1 kHz.) + assert_eq!(parse_ffprobe_rational("999999/1"), None); + } +} + -- 2.49.1 From acdffc1558167cb83eeb1813f3bdc8a6ff37eb97 Mon Sep 17 00:00:00 2001 From: Cameron Date: Sat, 16 May 2026 21:14:30 -0400 Subject: [PATCH 3/3] cargo fmt: drop trailing blank line in actors.rs Co-Authored-By: Claude Opus 4.7 (1M context) --- src/video/actors.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/video/actors.rs b/src/video/actors.rs index 8d04ea1..faad727 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -870,4 +870,3 @@ mod tests { assert_eq!(parse_ffprobe_rational("999999/1"), None); } } - -- 2.49.1