diff --git a/apps/filmcraft-cli/src/main.rs b/apps/filmcraft-cli/src/main.rs --- a/apps/filmcraft-cli/src/main.rs +++ b/apps/filmcraft-cli/src/main.rs @@ -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())); } diff --git a/apps/filmcraft-cli/tests/it/cli.rs b/apps/filmcraft-cli/tests/it/cli.rs --- a/apps/filmcraft-cli/tests/it/cli.rs +++ b/apps/filmcraft-cli/tests/it/cli.rs @@ -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())); diff --git a/crates/engine/src/export_tests.rs b/crates/engine/src/export_tests.rs --- a/crates/engine/src/export_tests.rs +++ b/crates/engine/src/export_tests.rs @@ -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(); diff --git a/crates/engine/src/export_tools.rs b/crates/engine/src/export_tools.rs --- a/crates/engine/src/export_tools.rs +++ b/crates/engine/src/export_tools.rs @@ -339,17 +339,34 @@ pub fn sequence_param(s: &Session, p: &Value, cmd: &str) -> Result { /// 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> { +/// 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, 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, 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 { 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 { 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 { 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 { 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 { 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 { @@ -929,7 +952,8 @@ fn resolve(s: &mut Session, p: &Value) -> Result { 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 { "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, diff --git a/crates/ui-egui/README.md b/crates/ui-egui/README.md --- a/crates/ui-egui/README.md +++ b/crates/ui-egui/README.md @@ -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). diff --git a/crates/ui-egui/src/panels/export_mode.rs b/crates/ui-egui/src/panels/export_mode.rs --- a/crates/ui-egui/src/panels/export_mode.rs +++ b/crates/ui-egui/src/panels/export_mode.rs @@ -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)); diff --git a/crates/ui-egui/tests/it/export_ui.rs b/crates/ui-egui/tests/it/export_ui.rs --- a/crates/ui-egui/tests/it/export_ui.rs +++ b/crates/ui-egui/tests/it/export_ui.rs @@ -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"); diff --git a/docs/agents.md b/docs/agents.md --- a/docs/agents.md +++ b/docs/agents.md @@ -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.