video: handle unknown/short durations in thumb + preview gen
`get_duration_seconds` now returns `Option<f64>` and falls back from `format=duration` to `stream=duration`. Empty stdout no longer parse-panics with "cannot parse float from empty string", which was poisoning the preview-clip row with status=failed and re-queueing every full scan (notably for GoPro LRV files). `generate_preview_clip` handles the unknown-duration case by transcoding the whole file (capped at 10s). `generate_video_thumbnail` seeks to ~50% of the probed duration instead of a hardcoded `-ss 3`, with a first-frame fallback when the probe returns nothing. Fixes the loop where short Snapchat clips (<3s) got "missing thumbnail" logged on every scan because ffmpeg exited 0 without writing a frame, and never wrote the .unsupported sentinel either. Adds unit tests for `parse_ffprobe_duration` covering the empty-output, N/A, multi-line, non-positive, and non-finite cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,7 +2,7 @@ use crate::database::PreviewDao;
|
||||
use crate::is_video;
|
||||
use crate::libraries::Library;
|
||||
use crate::otel::global_tracer;
|
||||
use crate::video::ffmpeg::generate_preview_clip;
|
||||
use crate::video::ffmpeg::{generate_preview_clip, get_duration_seconds_blocking};
|
||||
use actix::prelude::*;
|
||||
use log::{debug, error, info, trace, warn};
|
||||
use opentelemetry::KeyValue;
|
||||
@@ -108,6 +108,13 @@ pub async fn create_playlist(video_path: &str, playlist_file: &str) -> Result<Ch
|
||||
}
|
||||
|
||||
pub fn generate_video_thumbnail(path: &Path, destination: &Path) -> std::io::Result<()> {
|
||||
// Probe duration up front and seek to ~50% — gives a more
|
||||
// representative frame than a fixed offset (skipping title cards on
|
||||
// long videos, landing inside the clip on 1–2s Snapchat MP4s) and
|
||||
// sidesteps the seek-past-EOF class of bug entirely. When duration
|
||||
// probing fails (LRV files, fragmented MP4s, ffprobe missing) fall
|
||||
// back to the first frame: ugly but reliable.
|
||||
//
|
||||
// -vf scale + -c:v mjpeg mirrors `generate_image_thumbnail_ffmpeg`. The
|
||||
// filter chain matters as much as the scale does: without it, ffmpeg
|
||||
// hands the decoded frame straight to the mjpeg encoder, which rejects
|
||||
@@ -115,10 +122,14 @@ pub fn generate_video_thumbnail(path: &Path, destination: &Path) -> std::io::Res
|
||||
// filter chain lets ffmpeg auto-insert the pix_fmt converter the
|
||||
// encoder needs, which is how the image-thumbnail path already handles
|
||||
// the same class of source.
|
||||
let output = Command::new("ffmpeg")
|
||||
.arg("-y")
|
||||
.arg("-ss")
|
||||
.arg("3")
|
||||
let seek = get_duration_seconds_blocking(path).map(|d| format!("{:.3}", d / 2.0));
|
||||
|
||||
let mut cmd = Command::new("ffmpeg");
|
||||
cmd.arg("-y");
|
||||
if let Some(s) = &seek {
|
||||
cmd.arg("-ss").arg(s);
|
||||
}
|
||||
let output = cmd
|
||||
.arg("-i")
|
||||
.arg(path)
|
||||
.arg("-vframes")
|
||||
@@ -139,6 +150,18 @@ pub fn generate_video_thumbnail(path: &Path, destination: &Path) -> std::io::Res
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)));
|
||||
}
|
||||
// ffmpeg can exit 0 without writing a frame for malformed files where
|
||||
// the probe duration lies. Confirm a non-empty file actually landed —
|
||||
// returning Err makes the caller write the `.unsupported` sentinel so
|
||||
// we stop re-detecting on every scan.
|
||||
let wrote = std::fs::metadata(destination)
|
||||
.map(|m| m.len() > 0)
|
||||
.unwrap_or(false);
|
||||
if !wrote {
|
||||
return Err(std::io::Error::other(
|
||||
"ffmpeg exited successfully but produced no thumbnail output",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -223,20 +223,83 @@ impl Ffmpeg {
|
||||
}
|
||||
|
||||
/// Get video duration in seconds as f64 for precise interval calculation.
|
||||
async fn get_duration_seconds(input_file: &str) -> Result<f64> {
|
||||
Command::new("ffprobe")
|
||||
.args(["-i", input_file])
|
||||
.args(["-show_entries", "format=duration"])
|
||||
///
|
||||
/// Returns `Ok(None)` when ffprobe runs successfully but the container has no
|
||||
/// readable duration (notably GoPro `LRV` low-res preview files, some
|
||||
/// fragmented MP4s, and short Snapchat clips with stripped headers). Callers
|
||||
/// can fall back to a duration-agnostic encode rather than treating this as
|
||||
/// a hard failure — previously the `parse::<f64>` on empty stdout produced
|
||||
/// "cannot parse float from empty string" and poisoned the preview-clip row
|
||||
/// with status=failed, which the watcher would re-queue every full scan.
|
||||
async fn get_duration_seconds(input_file: &str) -> Result<Option<f64>> {
|
||||
if let Some(d) = probe_duration(input_file, "format=duration").await? {
|
||||
return Ok(Some(d));
|
||||
}
|
||||
// Fall back to the per-stream duration — populated for some MP4s where
|
||||
// the format-level duration tag is missing.
|
||||
probe_duration(input_file, "stream=duration").await
|
||||
}
|
||||
|
||||
/// Synchronous cousin of `get_duration_seconds`, for callers running on
|
||||
/// blocking thread pools (Rayon). Same fallback strategy: tries
|
||||
/// `format=duration`, then `stream=duration`. Returns `None` for any
|
||||
/// failure — ffprobe missing, container without a duration tag, parse
|
||||
/// error — so callers can pick a duration-agnostic default.
|
||||
pub fn get_duration_seconds_blocking(input_file: &std::path::Path) -> Option<f64> {
|
||||
if let Some(d) = probe_duration_blocking(input_file, "format=duration") {
|
||||
return Some(d);
|
||||
}
|
||||
probe_duration_blocking(input_file, "stream=duration")
|
||||
}
|
||||
|
||||
fn probe_duration_blocking(input_file: &std::path::Path, show_entries: &str) -> Option<f64> {
|
||||
let out = std::process::Command::new("ffprobe")
|
||||
.args(["-v", "quiet"])
|
||||
.args(["-show_entries", show_entries])
|
||||
.args(["-of", "csv=p=0"])
|
||||
.arg("-i")
|
||||
.arg(input_file)
|
||||
.output()
|
||||
.await
|
||||
.map(|out| String::from_utf8_lossy(&out.stdout).trim().to_string())
|
||||
.and_then(|duration_str| {
|
||||
duration_str
|
||||
.parse::<f64>()
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))
|
||||
})
|
||||
.ok()?;
|
||||
let raw = String::from_utf8_lossy(&out.stdout);
|
||||
parse_ffprobe_duration(&raw)
|
||||
}
|
||||
|
||||
async fn probe_duration(input_file: &str, show_entries: &str) -> Result<Option<f64>> {
|
||||
let out = Command::new("ffprobe")
|
||||
.args(["-v", "quiet"])
|
||||
.args(["-show_entries", show_entries])
|
||||
.args(["-of", "csv=p=0"])
|
||||
.args(["-i", input_file])
|
||||
.output()
|
||||
.await?;
|
||||
let raw = String::from_utf8_lossy(&out.stdout);
|
||||
Ok(parse_ffprobe_duration(&raw))
|
||||
}
|
||||
|
||||
/// Parse ffprobe's `csv=p=0` duration output. Returns the first valid
|
||||
/// positive finite duration, or `None` when there isn't one.
|
||||
///
|
||||
/// Stream-level queries (`-show_entries stream=duration`) emit one value per
|
||||
/// stream, one per line; format-level queries emit a single line. The shape
|
||||
/// also varies — `N/A` for streams without a known duration, empty string
|
||||
/// for containers without the tag at all, and (rarely) `0`/`-1` for
|
||||
/// fragmented MP4s. All of those have to map to `None` so the caller can
|
||||
/// fall back to a duration-agnostic encode.
|
||||
fn parse_ffprobe_duration(stdout: &str) -> Option<f64> {
|
||||
for line in stdout.lines() {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() || trimmed == "N/A" {
|
||||
continue;
|
||||
}
|
||||
if let Ok(d) = trimmed.parse::<f64>()
|
||||
&& d.is_finite()
|
||||
&& d > 0.0
|
||||
{
|
||||
return Some(d);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Generate a preview clip from a video file.
|
||||
@@ -268,28 +331,39 @@ pub async fn generate_preview_clip(input_file: &str, output_file: &str) -> Resul
|
||||
|
||||
cmd.arg("-i").arg(input_file);
|
||||
|
||||
if duration < 1.0 {
|
||||
// Very short video (<1s): transcode the whole thing to 480p MP4
|
||||
// format=yuv420p ensures 10-bit sources are converted to 8-bit for h264_nvenc
|
||||
// Branch on duration. `None` means ffprobe couldn't tell us — we treat
|
||||
// it like the <1s case and just transcode the whole file. The selected
|
||||
// clip-duration we report back is computed alongside, so callers don't
|
||||
// need to re-probe.
|
||||
let clip_duration = match duration {
|
||||
None => {
|
||||
warn!(
|
||||
"Unknown duration for '{}', transcoding whole file as preview",
|
||||
input_file
|
||||
);
|
||||
cmd.args(["-vf", "scale=-2:480,format=yuv420p"]);
|
||||
} else {
|
||||
let segment_count = if duration < 10.0 {
|
||||
duration.floor() as u32
|
||||
} else {
|
||||
10
|
||||
};
|
||||
let interval = duration / segment_count as f64;
|
||||
|
||||
// format=yuv420p ensures 10-bit sources are converted to 8-bit for h264_nvenc
|
||||
// Cap the encode at 10s so a long video with stripped duration
|
||||
// metadata doesn't spend forever generating a "preview".
|
||||
cmd.args(["-t", "10"]);
|
||||
10.0
|
||||
}
|
||||
Some(d) if d < 1.0 => {
|
||||
cmd.args(["-vf", "scale=-2:480,format=yuv420p"]);
|
||||
d
|
||||
}
|
||||
Some(d) => {
|
||||
let segment_count = if d < 10.0 { d.floor() as u32 } else { 10 };
|
||||
let interval = d / segment_count as f64;
|
||||
let vf = format!(
|
||||
"select='lt(mod(t,{:.4}),1)',setpts=N/FRAME_RATE/TB,fps=30,scale=-2:480,format=yuv420p",
|
||||
interval
|
||||
);
|
||||
let af = format!("aselect='lt(mod(t,{:.4}),1)',asetpts=N/SR/TB", interval);
|
||||
|
||||
cmd.args(["-vf", &vf]);
|
||||
cmd.args(["-af", &af]);
|
||||
if d < 10.0 { d.floor() } else { 10.0 }
|
||||
}
|
||||
};
|
||||
|
||||
// Force 30fps output so high-framerate sources (60fps) don't play back
|
||||
// at double speed due to select/setpts timestamp mismatches.
|
||||
@@ -320,14 +394,6 @@ pub async fn generate_preview_clip(input_file: &str, output_file: &str) -> Resul
|
||||
let metadata = std::fs::metadata(output_file)?;
|
||||
let file_size = metadata.len();
|
||||
|
||||
let clip_duration = if duration < 1.0 {
|
||||
duration
|
||||
} else if duration < 10.0 {
|
||||
duration.floor()
|
||||
} else {
|
||||
10.0
|
||||
};
|
||||
|
||||
info!(
|
||||
"Generated preview clip '{}' ({:.1}s, {} bytes) in {:?}",
|
||||
output_file,
|
||||
@@ -338,3 +404,69 @@ pub async fn generate_preview_clip(input_file: &str, output_file: &str) -> Resul
|
||||
|
||||
Ok((clip_duration, file_size))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::parse_ffprobe_duration;
|
||||
|
||||
#[test]
|
||||
fn empty_output_returns_none() {
|
||||
// The original bug: ffprobe -show_entries format=duration returned
|
||||
// "" for some GoPro LRV files, and `parse::<f64>` panicked with
|
||||
// "cannot parse float from empty string".
|
||||
assert_eq!(parse_ffprobe_duration(""), None);
|
||||
assert_eq!(parse_ffprobe_duration("\n"), None);
|
||||
assert_eq!(parse_ffprobe_duration(" \n \n"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn na_returns_none() {
|
||||
// ffprobe emits "N/A" for streams without a known duration.
|
||||
assert_eq!(parse_ffprobe_duration("N/A"), None);
|
||||
assert_eq!(parse_ffprobe_duration("N/A\nN/A\n"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_simple_duration() {
|
||||
assert_eq!(parse_ffprobe_duration("12.345"), Some(12.345));
|
||||
assert_eq!(parse_ffprobe_duration("12.345\n"), Some(12.345));
|
||||
assert_eq!(parse_ffprobe_duration("0.5"), Some(0.5));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_non_positive_durations() {
|
||||
// Fragmented MP4s and broken containers occasionally report 0 or a
|
||||
// negative duration. Treat as "unknown" so the caller falls back to
|
||||
// whole-file transcoding rather than dividing by zero downstream.
|
||||
assert_eq!(parse_ffprobe_duration("0"), None);
|
||||
assert_eq!(parse_ffprobe_duration("0.0"), None);
|
||||
assert_eq!(parse_ffprobe_duration("-1.5"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_non_finite_durations() {
|
||||
assert_eq!(parse_ffprobe_duration("inf"), None);
|
||||
assert_eq!(parse_ffprobe_duration("nan"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn first_valid_line_wins_for_stream_query() {
|
||||
// `-show_entries stream=duration` emits one value per stream. For a
|
||||
// video file the video stream is first; we accept it and ignore
|
||||
// any audio-stream values that follow.
|
||||
assert_eq!(parse_ffprobe_duration("12.5\n8.3\n"), Some(12.5));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_leading_na_and_blank_lines() {
|
||||
// Stream queries can put N/A first (e.g. data stream before the
|
||||
// video stream); the parser should keep scanning.
|
||||
assert_eq!(parse_ffprobe_duration("N/A\n\n7.25\n"), Some(7.25));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_garbage() {
|
||||
assert_eq!(parse_ffprobe_duration("not a number"), None);
|
||||
assert_eq!(parse_ffprobe_duration("12.5abc"), None);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user