collective
Browse / github.com/storytold/filmcraft / patch
#3364: warn about one-frame In/Out exports
Export: an In/Out range of In = Out silently exports a single frame ## Report When a sequence has In and Out on the same frame (easy to leave behind by accident), exporting with the default range uses In/Out and produces a one-frame file without any warning. Scripts calling `file.exportMedia` without `range` get a 0.04 s file and report success. ## Cause `range` defaults to In/Out when they're set ([`range_param`](https://github.com/storytold/filmcraft/blob/8fda2f232198c4c79ff9cfe5b6a347bb1375fa9e/crates/engine/src/export_tools.rs#L340-L372)), and [`export_range`](https://github.com/storytold/filmcraft/blob/8fda2f232198c4c79ff9cfe5b6a347bb1375fa9e/crates/ex
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- thecyberbutler (imported from pull request #3402, open)
- Base
8fda2f232198c4c79ff9cfe5b6a347bb1375fa9e- Size
- +236 −13 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
aef5a587ce80730ec157c95d
How it went
No other operator has reported using it yet.
Diff
raw · apps/filmcraft-cli/src/main.rs apps/filmcraft-cli/tests/it/cli.rs crates/engine/src/export_tests.rs crates/engine/src/export_tools.rs crates/ui-egui/README.md crates/ui-egui/src/panels/export_mode.rs crates/ui-egui/tests/it/export_ui.rs docs/agents.md
@@ -487,6 +487,9 @@ async fn cli() { Ok(v) => { print(&a, &v); let written = written_paths(&v, out).join(", ");+ for warning in v.get("warnings").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_str) {+ diag(format_args!("filmcraft-cli: warning: {warning} (CLI: --range entire)"));+ } check_offline(&a, &offline_media(&v), &format!("export: {written}")); diag(format_args!("exported {written} in {:.1}s", t0.elapsed().as_secs_f64())); }@@ -169,6 +169,37 @@ fn run_waits_for_an_export_on_its_last_line() { /// `export --preset`: a built-in preset by name (dash or en dash), a custom range, a user preset /// from `--data-dir`, `--list-presets`, `--queue`, and a clean failure for an unknown preset.+#[test]+fn single_frame_in_out_export_warns_in_json_and_stderr() {+ let dir = std::env::temp_dir().join(format!("fc-cli-single-frame-{}", std::process::id()));+ std::fs::create_dir_all(&dir).unwrap();+ let project = dir.join("marked.fcproj");+ let script = dir.join("marks.jsonl");+ std::fs::write(&script, "{\"id\":\"markers.markIn\",\"params\":{\"frame\":12}}\n{\"id\":\"markers.markOut\",\"params\":{\"frame\":12}}\n").unwrap();+ let setup = cli(&["--demo", "--save-as", project.to_str().unwrap(), "run", script.to_str().unwrap()]);+ assert!(setup.status.success(), "{}", String::from_utf8_lossy(&setup.stderr));+ for (name, bounds, warned) in [+ ("marked.wav", vec![], true),+ ("explicit.wav", vec!["--start", "0", "--end", "0.04"], false),+ ("queued.wav", vec!["--queue"], true),+ ("queued-explicit.wav", vec!["--queue", "--start", "0", "--end", "0.04"], false),+ ] {+ let path = dir.join(name);+ let mut args = vec!["--project", project.to_str().unwrap(), "export", path.to_str().unwrap()];+ args.extend(bounds);+ let output = cli(&args);+ let v = json_out(&output);+ assert_eq!(v["warnings"].as_array().unwrap().len(), usize::from(warned));+ let stderr = String::from_utf8_lossy(&output.stderr);+ assert_eq!(stderr.contains("In/Out range is 1 frame"), warned, "{stderr}");+ if warned {+ assert!(stderr.contains("--range entire"), "{stderr}");+ }+ assert!(path.exists());+ }+ std::fs::remove_dir_all(dir).unwrap();+}+ #[test] fn export_with_presets() { let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/export-tests").join(format!("cli-{}", std::process::id()));@@ -92,6 +92,111 @@ fn demo() -> Session { s } +#[test]+fn single_frame_in_out_warning_follows_the_selected_range() {+ let mut s = demo();+ for rate in [filmcraft_time::FrameRate::FPS_25, filmcraft_time::FrameRate::FPS_23_976] {+ let seq = s.state.active_sequence.unwrap();+ std::sync::Arc::make_mut(&mut s.project).sequence_mut(seq).unwrap().settings.frame_rate = rate;+ s.execute("markers.markIn", json!({"frame": 12})).unwrap();+ s.execute("markers.markOut", json!({"frame": 12})).unwrap();+ std::sync::Arc::make_mut(&mut s.project).sequence_mut(seq).unwrap().work_area =+ Some(filmcraft_time::TimeRange::new(rate.tick_of(12), rate.frame_duration()));+ for p in [json!({}), json!({"range":null}), json!({"range":"inOut"}), json!({"range":{"mode":"Sequence In/Out"}})] {+ let v = s.execute("export.resolve", p.clone()).unwrap();+ assert_eq!(v["frames"], 1, "{p}: {v}");+ assert_eq!(v["warnings"].as_array().unwrap().len(), 1, "{p}: {v}");+ assert!(v["warnings"][0].as_str().unwrap().contains("range: entire"));+ }+ let custom = json!({"range":"custom", "startFrame":12, "endFrame":13});+ for p in [+ json!({"range":"entire"}),+ json!({"range":"workArea"}),+ custom,+ json!({"startFrame":12, "endFrame":13}),+ json!({"range":{"startFrame":12, "endFrame":13}}),+ ] {+ let v = s.execute("export.resolve", p.clone()).unwrap();+ assert_eq!(v["warnings"], json!([]), "{p}: {v}");+ }+ // Rate conversion does not change the length of the marked sequence range.+ let v = s.execute("export.resolve", json!({"fps":60})).unwrap();+ assert_eq!(v["warnings"].as_array().unwrap().len(), 1);+ s.execute("markers.markOut", json!({"frame":13})).unwrap();+ let v = s.execute("export.resolve", json!({})).unwrap();+ assert_eq!(v["frames"], 2);+ assert_eq!(v["warnings"], json!([]));+ }+ // A one-frame sequence without marks is intentional and must stay quiet.+ let seq = s.state.active_sequence.unwrap();+ let q = std::sync::Arc::make_mut(&mut s.project).sequence_mut(seq).unwrap();+ q.mark_in = None;+ q.mark_out = None;+ q.video_tracks.clear();+ q.audio_tracks.clear();+ let v = s.execute("export.resolve", json!({})).unwrap();+ assert_eq!(v["frames"], 1);+ assert_eq!(v["warnings"], json!([]));+}++#[test]+fn single_frame_in_out_queue_add_retains_warnings_and_ranges() {+ let mut s = demo();+ let dir = Scratch::new("queue-range-warning");+ s.execute("markers.markIn", json!({"frame":12})).unwrap();+ s.execute("markers.markOut", json!({"frame":12})).unwrap();+ let expected = s.execute("export.resolve", json!({})).unwrap()["warnings"].clone();+ let v = s.execute("export.queue.add", json!({"path":dir.path("default.wav"),"format":"wav"})).unwrap();+ assert_eq!(v["warnings"], expected);+ assert_eq!(v["added"].as_array().unwrap().len(), 1);+ assert_eq!(v["items"].as_array().unwrap().len(), 1);+ assert_eq!(v["items"][0]["id"], v["added"][0]);+ assert_eq!(v["items"][0]["status"], "ready");+ assert_eq!(v["items"][0]["range"], Value::Null, "default range is still resolved from the snapshot's marks");+ s.execute("export.queue.clear", json!({"all":true})).unwrap();++ // Each marked range contributes its advisory; later explicit ranges must not overwrite it.+ let ranges = json!(["inOut", {"mode":"inOut"}, {"startFrame":12,"endFrame":13}, "entire"]);+ let v = s.execute("export.queue.add", json!({"path":dir.path("ranges.wav"),"format":"wav","ranges":ranges})).unwrap();+ assert_eq!(v["warnings"], json!([expected[0], expected[0]]));+ assert_eq!(v["added"].as_array().unwrap().len(), 4);+ let items = v["items"].as_array().unwrap();+ assert_eq!(items.len(), 4);+ for (i, range) in ranges.as_array().unwrap().iter().enumerate() {+ let resolved = s.execute("export.resolve", json!({"range":range})).unwrap();+ assert_eq!(items[i]["id"], v["added"][i]);+ assert_eq!(items[i]["range"]["start"], resolved["range"]["start"]);+ assert_eq!(items[i]["range"]["duration"], resolved["range"]["duration"]);+ }+ let quiet = s.execute("export.queue.add", json!({"path":dir.path("custom.wav"),"format":"wav","startFrame":12,"endFrame":13})).unwrap();+ assert_eq!(quiet["warnings"], json!([]), "warnings describe this addition, not previously queued items");+}++#[test]+fn single_frame_in_out_exports_warn_without_changing_the_output() {+ let mut s = demo();+ let dir = Scratch::new("single-frame-warning");+ s.execute("markers.markIn", json!({"frame":12})).unwrap();+ s.execute("markers.markOut", json!({"frame":12})).unwrap();+ for wait in [true, false] {+ let path = dir.path(&format!("marked-{wait}.wav"));+ let v = s.execute("file.exportMedia", json!({"path":path,"format":"wav","wait":wait})).unwrap();+ assert_eq!(v["warnings"].as_array().unwrap().len(), 1);+ if !wait {+ let job = s.jobs.iter().find(|j| Some(j.id) == v["job"].as_u64()).unwrap();+ let start = std::time::Instant::now();+ while !job.progress.finished.load(std::sync::atomic::Ordering::Relaxed) {+ assert!(start.elapsed() < std::time::Duration::from_secs(30));+ std::thread::sleep(std::time::Duration::from_millis(5));+ }+ }+ let explicit = dir.path(&format!("explicit-{wait}.wav"));+ let v = s.execute("file.exportMedia", json!({"path":explicit,"format":"wav","wait":true,"startFrame":12,"endFrame":13})).unwrap();+ assert_eq!(v["warnings"], json!([]));+ assert_eq!(std::fs::read(path).unwrap(), std::fs::read(explicit).unwrap());+ }+}+ #[test] fn export_integer_parameters_cannot_wrap_or_overflow() { let mut s = demo();@@ -339,17 +339,34 @@ pub fn sequence_param(s: &Session, p: &Value, cmd: &str) -> Result<ItemId> { /// Export ▸ Range: `entire` | `inOut` | `workArea` | `custom` (`start…` / `end…` times). /// None = the default (In/Out when set, else the whole sequence); bare `start…` / `end…` times-/// without a `range` mean `custom`.-pub fn range_param(s: &Session, project: &Project, seq: ItemId, v: Option<&Value>, p: &Value, cmd: &str) -> Result<Option<TimeRange>> {+/// without a `range` mean `custom`. Returns the range and any advisories.+pub fn range_param(s: &Session, project: &Project, seq: ItemId, v: Option<&Value>, p: &Value, cmd: &str) -> Result<(Option<TimeRange>, Vec<&'static str>)> {+ range_with_warnings(s, project, seq, v, p, cmd)+}++/// A marked range of one sequence frame is valid, but easily left behind by accident.+/// Explicit custom/work-area/entire ranges must not inherit this warning.+pub fn single_frame_in_out(q: &filmcraft_project::Sequence, range: TimeRange) -> bool {+ let rate = q.settings.frame_rate;+ (q.mark_in.is_some() || q.mark_out.is_some()) && range.duration > Tick::ZERO && rate.frame_at(range.start) == rate.frame_at(range.end() - Tick(1))+}++const SINGLE_FRAME_WARNING: &str = "In/Out range is 1 frame; pass range: entire for the whole sequence";++fn range_with_warnings(s: &Session, project: &Project, seq: ItemId, v: Option<&Value>, p: &Value, cmd: &str) -> Result<(Option<TimeRange>, Vec<&'static str>)> { let q = project.sequence(seq).ok_or(EngineError::NoSequence)?; q.check_bounds().map_err(|e| bad(cmd, e))?; let fd = q.settings.frame_rate.frame_duration(); let whole = TimeRange::from_bounds(Tick::ZERO, q.duration().max(fd)); let mode = match v { None | Some(Value::Null) if time_p(s, p, "start").is_some() || time_p(s, p, "end").is_some() => "custom".to_string(),- None | Some(Value::Null) => return Ok(None),+ None | Some(Value::Null) => {+ let range = filmcraft_export::export_range(project, seq, &ExportSettings::default()).map_err(|e| bad(cmd, e.to_string()))?;+ let warnings = if single_frame_in_out(q, range) { vec![SINGLE_FRAME_WARNING] } else { Vec::new() };+ return Ok((None, warnings));+ } Some(Value::String(m)) => m.to_ascii_lowercase().replace(['-', '_', ' ', '/'], ""),- Some(o @ Value::Object(_)) => return range_param(s, project, seq, o.get("mode").or(Some(&json!("custom"))), o, cmd),+ Some(o @ Value::Object(_)) => return range_with_warnings(s, project, seq, o.get("mode").or(Some(&json!("custom"))), o, cmd), Some(other) => return Err(bad(cmd, format!("range: unexpected {other}"))), }; let range = match mode.as_str() {@@ -368,7 +385,8 @@ pub fn range_param(s: &Session, project: &Project, seq: ItemId, v: Option<&Value other => return Err(bad(cmd, format!("range `{other}`: entire | inOut | workArea | custom"))), }; filmcraft_export::validate_range(range).map_err(|e| bad(cmd, e.to_string()))?;- Ok(Some(range))+ let warnings = if matches!(mode.as_str(), "inout" | "sequenceinout") && single_frame_in_out(q, range) { vec![SINGLE_FRAME_WARNING] } else { Vec::new() };+ Ok((Some(range), warnings)) } /// Where exports go when no path is given: next to the saved project, else ~/Movies, else the@@ -544,12 +562,13 @@ pub fn export_media(s: &mut Session, p: &Value) -> Result<Value> { return Err(bad(cmd, "need `path`")); } let project = s.project.clone();- settings.range = range_param(s, &project, seq, p.get("range"), p, cmd)?;+ let (range, warnings) = range_with_warnings(s, &project, seq, p.get("range"), p, cmd)?;+ settings.range = range; settings.path = output_path(s, p, seq, &settings, None); let wait = bool_p(p, "wait").unwrap_or(false); let path = settings.path.clone(); let id = spawn_export(s, project, seq, settings, format!("Export {}", file_name(&path)), wait)?;- let mut out = json!({"job": id, "path": path});+ let mut out = json!({"job": id, "path": path, "warnings": warnings}); if let Some(pr) = preset { out["preset"] = json!(pr.name); }@@ -747,6 +766,7 @@ fn queue_add(s: &mut Session, p: &Value) -> Result<Value> { let mut ids = Vec::new(); let mut n = 0usize; let mut items = Vec::new();+ let mut warnings = Vec::new(); for &seq in &seqs { for r in &ranges { let mut st = settings.clone();@@ -754,7 +774,9 @@ fn queue_add(s: &mut Session, p: &Value) -> Result<Value> { Some(v) if v.is_object() => *v, _ => p, };- st.range = range_param(s, &project, seq, *r, times, cmd)?;+ let (range, range_warnings) = range_param(s, &project, seq, *r, times, cmd)?;+ st.range = range;+ warnings.extend(range_warnings); // several ranges of one sequence get numbered names; several sequences their own names let suffix = if many && (ranges.len() > 1 || str_p(p, "path").is_some_and(|x| !is_folder(&expand_home(x)))) { Some(n) } else { None }; st.path = output_path(s, p, seq, &st, suffix);@@ -783,7 +805,7 @@ fn queue_add(s: &mut Session, p: &Value) -> Result<Value> { s.export_queue.running = true; pump_queue(s, bool_p(p, "wait").unwrap_or(false)); }- Ok(json!({"added": ids, "items": queue_list_json(s)}))+ Ok(json!({"added": ids, "items": queue_list_json(s), "warnings": warnings})) } fn queue_list_json(s: &Session) -> Value {@@ -914,14 +936,15 @@ fn quick(s: &mut Session, p: &Value) -> Result<Value> { let seq = sequence_param(s, &q, cmd)?; let (preset, mut settings) = settings_from_params(s, &q, cmd)?; let project = s.project.clone();- settings.range = range_param(s, &project, seq, q.get("range"), &q, cmd)?;+ let (range, warnings) = range_with_warnings(s, &project, seq, q.get("range"), &q, cmd)?;+ settings.range = range; settings.path = output_path(s, &q, seq, &settings, None); let wait = bool_p(p, "wait").unwrap_or(false); let path = settings.path.clone(); let id = spawn_export(s, project, seq, settings, format!("Quick Export {}", file_name(&path)), wait)?; let used = preset.map(|x| x.name).unwrap_or(name); s.export_queue.quick_preset = Some(used.clone());- Ok(json!({"job": id, "path": path, "preset": used}))+ Ok(json!({"job": id, "path": path, "preset": used, "warnings": warnings})) } fn resolve(s: &mut Session, p: &Value) -> Result<Value> {@@ -929,7 +952,8 @@ fn resolve(s: &mut Session, p: &Value) -> Result<Value> { let seq = sequence_param(s, p, cmd)?; let (preset, mut settings) = settings_from_params(s, p, cmd)?; let project = s.project.clone();- settings.range = range_param(s, &project, seq, p.get("range"), p, cmd)?;+ let (range, warnings) = range_with_warnings(s, &project, seq, p.get("range"), p, cmd)?;+ settings.range = range; settings.path = output_path(s, p, seq, &settings, None); let q = project.sequence(seq).ok_or(EngineError::NoSequence)?; let range = filmcraft_export::export_range(&project, seq, &settings).map_err(|e| EngineError::Other(e.to_string()))?;@@ -942,6 +966,7 @@ fn resolve(s: &mut Session, p: &Value) -> Result<Value> { "settings": settings, "range": {"start": range.start.0, "duration": range.duration.0, "seconds": range.duration.seconds()}, "frames": f1 - f0,+ "warnings": warnings, "output": {"width": r.width, "height": r.height, "fps": r.rate.label(), "sampleRate": r.sample_rate, "channels": r.channels, "targetKbps": r.target_kbps, "maxKbps": r.max_kbps, "keyframeDistance": r.keyint}, "summary": summary,@@ -3,6 +3,16 @@ The egui frontend draws panels and dialogs, and dispatches project changes through the engine. Interactive controls keep stable automation ids regardless of the interface language. +## Export range warnings++The Export panel shows the selected range's length in sequence frames and seconds beside Range+(`export.range.duration`). When Source In/Out covers just one sequence frame and at least one+mark is set, `export.range.warning` explains how to export the whole sequence instead (#3364).+The warning is advisory: exporting one frame remains valid. Entire Source, Work Area and Custom+ranges do not show this warning. `export_ui::single_frame_in_out_warning_updates_with_range_and_marks`+checks the warning and duration as marks and range selections change; the same test can render+`export-single-frame-warning.png` with `FILMCRAFT_UI_SNAPSHOT_DIR`.+ ## Control input limits The control channel bounds `ui.click` to 10 clicks and `ui.drag` to 1024 movement steps (#2784).@@ -1120,7 +1120,7 @@ fn preview_column(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, seq_id: ); let ex = &mut app.ui.export; let has_work_area = q.work_area.is_some();- child.horizontal(|ui| {+ child.horizontal_wrapped(|ui| { ui.label(egui::RichText::new(tl!("Range")).color(t.text_dim)); let cur = RANGES.iter().find(|(k, _)| *k == ex.range).map(|x| x.1).unwrap_or("Entire Source"); let labels: Vec<(String, bool)> = RANGES.iter().map(|(k, l)| (l.to_string(), *k != "workArea" || has_work_area)).collect();@@ -1131,7 +1131,24 @@ fn preview_column(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect, seq_id: drag(ui, &mut reg, "export.range.start", &mut ex.custom_start, 0.0..=86_400.0, 0.04, " s", 2); drag(ui, &mut reg, "export.range.end", &mut ex.custom_end, 0.0..=86_400.0, 0.04, " s", 2); }+ let (start, end) = range_ticks(ex, &q);+ let span = filmcraft_time::TimeRange::from_bounds(start, end);+ let frames = rate.frame_at(end - Tick(1)).saturating_add(1).saturating_sub(rate.frame_at(start)).max(0);+ let seconds = format!("{:.3}", span.duration.seconds());+ let label = if frames == 1 {+ tlf!("1 frame ({seconds} s)", seconds = seconds)+ } else {+ tlf!("{frames} frames ({seconds} s)", frames = frames, seconds = seconds)+ };+ let resp = ui.label(egui::RichText::new(&label).color(t.text_dim));+ reg.add("export.range.duration", resp.rect, label); });+ let (start, end) = range_ticks(ex, &q);+ if ex.range == "inOut" && filmcraft_engine::export_tools::single_frame_in_out(&q, filmcraft_time::TimeRange::from_bounds(start, end)) {+ let warning = tl!("Source In/Out is only 1 frame. Choose Entire Source to export the whole sequence.");+ let resp = child.add(egui::Label::new(egui::RichText::new(warning).color(t.danger)).wrap());+ reg.add("export.range.warning", resp.rect, warning);+ } if ex.settings.has_video() { child.horizontal(|ui| { ui.label(egui::RichText::new(tl!("Scaling")).color(t.text_dim));@@ -125,6 +125,30 @@ impl Drop for Driver { } } +#[test]+fn single_frame_in_out_warning_updates_with_range_and_marks() {+ let mut d = Driver::new("single-frame-warning");+ d.exec("markers.markIn", json!({"frame":12}));+ d.exec("markers.markOut", json!({"frame":12}));+ d.ok("ui.set", json!({"mode":"export", "export":{"range":"inOut"}}));+ d.frames(3);+ assert!(d.has("export.range.warning"));+ let duration = d.ok("ui.elements", json!({"prefix":"export.range.duration"}));+ assert!(duration[0]["label"].as_str().unwrap().starts_with("1 frame ("), "{duration}");+ d.snapshot("export-single-frame-warning");+ for range in ["entire", "custom"] {+ d.ok("ui.set", json!({"export":{"range":range,"customStart":0.0,"customEnd":0.04}}));+ d.frames(2);+ assert!(!d.has("export.range.warning"));+ }+ d.ok("ui.set", json!({"export":{"range":"inOut"}}));+ d.exec("markers.markOut", json!({"frame":13}));+ d.frames(2);+ assert!(!d.has("export.range.warning"));+ let duration = d.ok("ui.elements", json!({"prefix":"export.range.duration"}));+ assert!(duration[0]["label"].as_str().unwrap().starts_with("2 frames ("), "{duration}");+}+ #[test] fn export_mode_settings_and_summary() { let mut d = Driver::new("settings");@@ -138,6 +138,14 @@ progress and stop it: once: poll `jobs.list` for the job's `progress`, `etaSeconds`, `finished` and `result`, and stop it with `jobs.cancel` (`export.queue.list` lists the Export Queue only, not direct exports, #1544). Other exports (`file.exportFrame`, interchange formats, …) finish quickly and ignore the token.+- `file.exportMedia`, `export.quick`, `export.resolve` and `export.queue.add` also return `warnings` (an array of+ strings, empty when none). If the default or explicitly selected In/Out range covers one+ sequence frame and at least one mark is set, the warning suggests `range: "entire"` (#3364).+ The export still succeeds, including with `wait: true` over MCP or the bridge. Explicit custom,+ work-area and entire-sequence ranges are unchanged and do not warn. Queue additions collect+ warnings from the newly added ranges. `filmcraft-cli export` (including `--queue`)+ also prints the warning on stderr and suggests `--range entire`; the warning does not change+ the exit status. Frame counts here use the sequence rate, even when the export rate differs. - Bridge mode works the same way: the export runs as an app job (the app stays responsive and shows "Exporting… NN%" in its status bar) and the call returns when it is written, however long it takes. `filmcraft-cli --bridge … exec file.exportMedia … wait=true` blocks the same way.