Merge pull request #2141 from Silvertree2010/fix/h1878-cli-render-silently-renders-fram

Reject non-numeric --seconds and --scale in CLI render
This commit is contained in:
Brandon Thomas
2026-10-18 03:20:07 -04:00
committed by GitHub
2 changed files with 33 additions and 2 deletions
+12 -2
View File
@@ -465,8 +465,8 @@ async fn cli() {
usage("render is headless; with --bridge use the MCP `render_frame` tool or `exec` ui commands");
}
let Backend::Local(mut s) = Backend::open(&a) else { usage("render is headless") };
let secs: f64 = a.opt("--seconds").and_then(|v| v.parse().ok()).unwrap_or(0.0);
let scale: f32 = a.opt("--scale").and_then(|v| v.parse().ok()).unwrap_or(1.0);
let secs: f64 = finite_opt(&a, "--seconds").unwrap_or(0.0);
let scale: f32 = finite_opt(&a, "--scale").unwrap_or(1.0);
let out = a.opt("--out").unwrap_or("frame.png");
s.set_playhead(filmcraft_time::Tick::from_seconds_f64(secs));
let t0 = std::time::Instant::now();
@@ -551,6 +551,16 @@ fn check_offline(a: &Args, offline: &[Value], what: &str) {
}
}
/// A numeric option, or a usage error when it is not a finite number (so a typo never silently
/// falls back to a default).
fn finite_opt<T: std::str::FromStr + Copy + Into<f64>>(a: &Args, name: &str) -> Option<T> {
let v = a.opt(name)?;
match v.parse::<T>() {
Ok(n) if n.into().is_finite() => Some(n),
_ => usage(format!("{name}: expected a finite number, got `{v}`")),
}
}
/// The active sequence at the playhead with the offline items it drew, or the reason it cannot be
/// rendered (no sequence, bad scale).
fn render_at_playhead(s: &Session, scale: f32) -> Result<(filmcraft_engine::render::Image, Vec<filmcraft_engine::media_pool::OfflineMedia>), String> {
+21
View File
@@ -295,6 +295,27 @@ fn closed_stdout_still_runs_the_script_and_saves() {
let _ = std::fs::remove_dir_all(&dir);
}
/// `render` with a non-numeric or non-finite `--seconds` / `--scale` is a usage error (exit 2) and
/// writes nothing, instead of silently rendering frame 0 at full scale.
#[test]
fn render_rejects_bad_numbers() {
let dir = std::env::temp_dir().join(format!("fc-cli-render-nan-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let still = dir.join("s.png").to_str().unwrap().to_string();
for (opt, bad) in
[("--seconds", "1s"), ("--seconds", "1,5"), ("--seconds", "abc"), ("--seconds", "NaN"), ("--seconds", "inf"), ("--scale", "xyz"), ("--scale", "NaN")]
{
let (secs, scale) = if opt == "--seconds" { (bad, "0.1") } else { ("1", bad) };
let o = cli(&["--demo", "render", "--seconds", secs, "--scale", scale, "--out", &still]);
let err = String::from_utf8_lossy(&o.stderr);
assert_eq!(o.status.code(), Some(2), "{opt} {bad}: {err}");
assert!(err.contains("filmcraft-cli:") && err.contains(opt), "{opt} {bad}: {err}");
assert!(!std::path::Path::new(&still).exists(), "{opt} {bad} wrote a file");
}
let _ = std::fs::remove_dir_all(&dir);
}
/// #1054: exporting or rendering a project whose media is missing fails (exit 1) and names the
/// offline items on stderr and in the result's `offlineMedia`, unless `--allow-offline` accepts the
/// placeholder frames; then it still warns.