mirror of
https://github.com/storytold/filmcraft.git
synced 2026-10-14 19:26:59 -04:00
CLI: a closed stdout no longer panics filmcraft-cli, and the work still finishes (#285)
`filmcraft-cli commands | head -c 10` panicked with "failed printing to stdout: Broken pipe (os error 32)". Output now goes through `outln!` / `out!`. When stdout fails, later output is dropped but the program does not stop: the rest of `exec`, `run` and `import` (later script lines, `--save`, `--save-as`) still runs and the exit status still reports a failing command. Exiting at the first failed print would report success while skipping the save, e.g. `filmcraft-cli run edits.jsonl --project p.fcproj --save | head -1`. A closed pipe is not an error; any other stdout write error is reported and makes the exit status 1. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
This commit is contained in:
1 parent
5231852443
commit
44248c060f
4 files changed
+119
-9
No files matched your search
@@ -159,6 +159,8 @@ Status: ✅ done · 🟡 in progress · ⬜ not started. Estimates are remaining
|
||||
|
||||
## Log
|
||||
|
||||
- **2026-10-08 (#285):** `filmcraft-cli` no longer panics with "Broken pipe" when its stdout is closed early (`filmcraft-cli commands | head`): later output is dropped, the rest of the work (script lines, `--save`, `--save-as`) still runs, and the exit status still reports failures. Other stdout write errors are reported and make the exit status 1. No change to the checklist or estimates.
|
||||
|
||||
- **2026-10-06:** 21 community PRs landed (#19, #28, #38, #39, #41–#60). Highlights: hardware decoding is now actually on in the desktop app (#41: `register()` sat inside a `log::info!` that never ran, so #33's VideoToolbox path only worked in the CLI and benchmarks); decode memory on long timelines cut sharply (frame pool, shared GOP-cache budget, positional file reads); exports are byte-identical across machines (#19); crash fix for odd track names over MCP (#44); ripple trim, ripple delete and speed changes move split-edit clips once (#56–#58); Japanese interface using an installed system font (#38; no fonts bundled); MCP annotations, resources, export progress and cancellation (#28).
|
||||
|
||||
- **2026-10-05:** honest assessment added (checklist ~87% vs ready for real work ~50–60%). Hardware acceleration started (#30): blend modes on the GPU (#32); VideoToolbox decode + `platform` crate, the one crate allowed `unsafe` (#33). Community fixes landed: AIFF import, export start/end ranges, transitions following ripple trims, Slide moving linked audio (#5–#8).
|
||||
|
||||
@@ -4,6 +4,32 @@
|
||||
|
||||
#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
|
||||
|
||||
/// `println!` that doesn't panic when stdout is closed (`filmcraft-cli commands | head`) with
|
||||
/// "failed printing to stdout: Broken pipe (os error 32)". Once stdout has failed, later output is
|
||||
/// dropped and the work goes on: see `stdout_failed`.
|
||||
macro_rules! outln {
|
||||
($($arg:tt)*) => {{
|
||||
use std::io::Write as _;
|
||||
if !$crate::stdout_gone() {
|
||||
if let Err(e) = writeln!(std::io::stdout(), $($arg)*) {
|
||||
$crate::stdout_failed(e);
|
||||
}
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
/// `print!` counterpart of `outln!`.
|
||||
macro_rules! out {
|
||||
($($arg:tt)*) => {{
|
||||
use std::io::Write as _;
|
||||
if !$crate::stdout_gone() {
|
||||
if let Err(e) = write!(std::io::stdout(), $($arg)*) {
|
||||
$crate::stdout_failed(e);
|
||||
}
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
mod args;
|
||||
mod probe;
|
||||
|
||||
@@ -11,6 +37,28 @@ use args::{Args, parse_value};
|
||||
use filmcraft_automation::BridgeClient;
|
||||
use filmcraft_engine::Session;
|
||||
use serde_json::{Value, json};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
/// Set once a write to stdout has failed; later output is dropped.
|
||||
static STDOUT_GONE: AtomicBool = AtomicBool::new(false);
|
||||
/// Set when stdout failed for a reason other than a closed reader: the exit status becomes 1.
|
||||
static STDOUT_ERROR: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
fn stdout_gone() -> bool {
|
||||
STDOUT_GONE.load(Ordering::Relaxed)
|
||||
}
|
||||
|
||||
/// stdout went away. The program doesn't stop there: the rest of `exec`/`run`/`import` (later
|
||||
/// script lines, `--save`, `--save-as`) still runs and the exit status still tells how it went, so
|
||||
/// `exec … --save | head -1` keeps the edit. A reader that stopped early (a closed pipe) is not an
|
||||
/// error, as in ripgrep; any other write error is reported and makes the exit status 1.
|
||||
fn stdout_failed(e: std::io::Error) {
|
||||
if STDOUT_GONE.swap(true, Ordering::Relaxed) || e.kind() == std::io::ErrorKind::BrokenPipe {
|
||||
return;
|
||||
}
|
||||
eprintln!("filmcraft-cli: can't write to stdout: {e}");
|
||||
STDOUT_ERROR.store(true, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
const HELP: &str = "\
|
||||
filmcraft-cli: drive FilmCraft from the shell (headless engine, or the live app with --bridge)
|
||||
@@ -134,7 +182,7 @@ impl Backend {
|
||||
|
||||
fn print(a: &Args, v: &Value) {
|
||||
let s = if a.flag("--compact") { serde_json::to_string(v) } else { serde_json::to_string_pretty(v) };
|
||||
println!("{}", s.unwrap_or_default());
|
||||
outln!("{}", s.unwrap_or_default());
|
||||
}
|
||||
|
||||
/// Export format from the output file's extension.
|
||||
@@ -162,16 +210,23 @@ fn register_hardware_decoders() -> filmcraft_platform::Availability {
|
||||
|
||||
#[tokio::main]
|
||||
async fn main() {
|
||||
cli().await;
|
||||
if STDOUT_ERROR.load(Ordering::Relaxed) {
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
async fn cli() {
|
||||
if matches!(std::env::args().nth(1).as_deref(), Some("--version" | "-V")) {
|
||||
println!("filmcraft-cli {}", env!("CARGO_PKG_VERSION"));
|
||||
outln!("filmcraft-cli {}", env!("CARGO_PKG_VERSION"));
|
||||
return;
|
||||
}
|
||||
register_hardware_decoders();
|
||||
let a = Args::parse(std::env::args().skip(1));
|
||||
let Some(cmd) = a.pos(0) else { usage("missing subcommand") };
|
||||
match cmd {
|
||||
"help" | "--help" | "-h" => print!("{HELP}"),
|
||||
"version" => println!("filmcraft-cli {}", env!("CARGO_PKG_VERSION")),
|
||||
"help" | "--help" | "-h" => out!("{HELP}"),
|
||||
"version" => outln!("filmcraft-cli {}", env!("CARGO_PKG_VERSION")),
|
||||
"probe" => {
|
||||
let path = a.pos(1).unwrap_or_else(|| usage("probe <media> [--image-sequence]"));
|
||||
match probe::probe(path, a.flag("--image-sequence")) {
|
||||
@@ -189,11 +244,11 @@ async fn main() {
|
||||
src.video_frame(filmcraft_media::FrameRequest::full(rate.tick_of(f))).unwrap_or_else(|e| fail(e));
|
||||
}
|
||||
let dt = t0.elapsed().as_secs_f64();
|
||||
println!("{n} frames in {dt:.2}s → {:.1} fps (sequential, via Mp4Source)", n as f64 / dt);
|
||||
outln!("{n} frames in {dt:.2}s → {:.1} fps (sequential, via Mp4Source)", n as f64 / dt);
|
||||
let t1 = std::time::Instant::now();
|
||||
let f = src.video_frame(filmcraft_media::FrameRequest::full(rate.tick_of(n / 2))).unwrap_or_else(|e| fail(e));
|
||||
let (w, h, _) = f.to_linear_f32_decimated(2);
|
||||
println!("½-res linear conversion {w}x{h}: {:.1} ms", t1.elapsed().as_secs_f64() * 1000.0);
|
||||
outln!("½-res linear conversion {w}x{h}: {:.1} ms", t1.elapsed().as_secs_f64() * 1000.0);
|
||||
}
|
||||
"commands" | "describe" => {
|
||||
let mut b = Backend::open(&a);
|
||||
@@ -219,7 +274,7 @@ async fn main() {
|
||||
} else {
|
||||
for c in &list {
|
||||
let s = |k: &str| c[k].as_str().unwrap_or("").to_string();
|
||||
println!("{:<32} {:<34} {:<14} {}", s("id"), s("label"), s("shortcut"), s("params"));
|
||||
outln!("{:<32} {:<34} {:<14} {}", s("id"), s("label"), s("shortcut"), s("params"));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -246,9 +301,9 @@ async fn main() {
|
||||
let v: Value = serde_json::from_str(line).unwrap_or_else(|e| usage(format!("line {}: {e}", n + 1)));
|
||||
let id = v["id"].as_str().unwrap_or_else(|| usage(format!("line {}: missing \"id\"", n + 1)));
|
||||
match b.exec(id, v.get("params").cloned().unwrap_or(json!({}))).await {
|
||||
Ok(r) => println!("{}", json!({"line": n + 1, "id": id, "ok": true, "result": r})),
|
||||
Ok(r) => outln!("{}", json!({"line": n + 1, "id": id, "ok": true, "result": r})),
|
||||
Err(e) => {
|
||||
println!("{}", json!({"line": n + 1, "id": id, "ok": false, "error": e}));
|
||||
outln!("{}", json!({"line": n + 1, "id": id, "ok": false, "error": e}));
|
||||
failed += 1;
|
||||
if !a.flag("--keep-going") {
|
||||
std::process::exit(1);
|
||||
|
||||
@@ -185,3 +185,53 @@ fn export_with_presets() {
|
||||
assert!(String::from_utf8_lossy(&bad.stderr).contains("No Such Preset"));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// `filmcraft-cli commands | head -c 10` panicked with "failed printing to stdout: Broken pipe
|
||||
/// (os error 32)" and exit status 101 (#285).
|
||||
#[test]
|
||||
fn closed_stdout_ends_quietly() {
|
||||
use std::io::Read;
|
||||
for args in [&["commands"][..], &["help"], &["--demo", "commands", "--json"]] {
|
||||
let mut child = Command::new(env!("CARGO_BIN_EXE_filmcraft-cli")).args(args).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
|
||||
let mut first = [0u8; 10];
|
||||
child.stdout.as_mut().unwrap().read_exact(&mut first).unwrap();
|
||||
drop(child.stdout.take());
|
||||
let out = child.wait_with_output().unwrap();
|
||||
let err = String::from_utf8_lossy(&out.stderr);
|
||||
assert!(out.status.success() && err.is_empty(), "{args:?}: {:?}: {err}", out.status);
|
||||
}
|
||||
}
|
||||
|
||||
/// `run -` with stdout already closed: `run` reads its script to the end before it prints, so its
|
||||
/// first write fails for sure. The rest of the script and the save still happen, and the exit status
|
||||
/// still reports a failing command.
|
||||
fn run_with_closed_stdout(args: &[&str], script: &[u8]) -> Output {
|
||||
let mut child =
|
||||
Command::new(env!("CARGO_BIN_EXE_filmcraft-cli")).args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
|
||||
drop(child.stdout.take());
|
||||
child.stdin.take().unwrap().write_all(script).unwrap();
|
||||
child.wait_with_output().unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn closed_stdout_still_runs_the_script_and_saves() {
|
||||
let dir = std::env::temp_dir().join(format!("fc-cli-pipe-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let proj = dir.join("p.fcproj");
|
||||
let p = proj.to_str().unwrap();
|
||||
let script = b"{\"id\":\"file.newBin\",\"params\":{\"name\":\"Piped One\"}}\n{\"id\":\"file.newBin\",\"params\":{\"name\":\"Piped Two\"}}\n";
|
||||
let out = run_with_closed_stdout(&["--demo", "--save-as", p, "run", "-"], script);
|
||||
assert!(out.status.success(), "{:?}: {}", out.status, String::from_utf8_lossy(&out.stderr));
|
||||
let tree = json_out(&cli(&["--project", p, "inspect", "project"])).to_string();
|
||||
assert!(tree.contains("Piped One") && tree.contains("Piped Two"), "{tree}");
|
||||
|
||||
// a failing command is still a failure, whether or not anyone reads stdout
|
||||
let out = run_with_closed_stdout(&["--demo", "run", "-"], b"{\"id\":\"no.such.command\"}\n");
|
||||
assert_eq!(out.status.code(), Some(1));
|
||||
let out = run_with_closed_stdout(
|
||||
&["--demo", "--keep-going", "run", "-"],
|
||||
b"{\"id\":\"no.such.command\"}\n{\"id\":\"file.newBin\",\"params\":{\"name\":\"x\"}}\n",
|
||||
);
|
||||
assert_eq!(out.status.code(), Some(1));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
@@ -166,6 +166,9 @@ Notes:
|
||||
|
||||
Every command is also one shell call away. Options go anywhere; output is JSON; exit status is 0 on
|
||||
success, 1 when a command fails and 2 on a usage error. `filmcraft-cli help` prints the reference.
|
||||
If the reader of stdout closes the pipe early (`filmcraft-cli commands | head`), the CLI drops the rest
|
||||
of its output but still finishes the work, saves included, and the exit status still reports
|
||||
failures; any other stdout write error is reported and makes the status 1.
|
||||
|
||||
```sh
|
||||
filmcraft-cli commands razor # find ids (add --json for machine output)
|
||||
|
||||
Reference in new issue
Block a user