#3364: warn about one-frame In/Out exports (#3402)

* #3364: warn about one-frame In/Out exports

Show the marked range duration and an advisory in Export, command results, and CLI stderr. Preserve export contents and explicit-range behavior. Add engine, CLI, and UI regressions and document the warning.

* #3364: retain one-frame warnings in queued exports

Collect range advisories in export.queue.add while preserving queued ranges and existing response fields. Cover default and explicit queued ranges plus CLI --queue warning output.

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
Michael Butler
2026-10-30 01:20:44 -04:00
committed by GitHub
parent e2f5fb0f21
commit cc57ebb95b
8 changed files with 236 additions and 13 deletions
+3
View File
@@ -507,6 +507,9 @@ async fn cli_export(a: &Args) {
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()));
}
+31
View File
@@ -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()));
+105
View File
@@ -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();
+37 -12
View File
@@ -346,17 +346,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() {
@@ -375,7 +392,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
@@ -551,12 +569,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);
}
@@ -754,6 +773,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();
@@ -761,7 +781,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);
@@ -790,7 +812,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 {
@@ -921,14 +943,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> {
@@ -936,7 +959,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()))?;
@@ -949,6 +973,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,
+10
View File
@@ -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).
@@ -50,7 +50,7 @@ pub(super) fn preview_column(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Re
);
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();
@@ -61,7 +61,24 @@ pub(super) fn preview_column(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Re
drag(ui, &mut reg, "export.range.start", &mut ex.custom_start, DragOptions { range: 0.0..=86_400.0, speed: 0.04, suffix: " s", decimals: 2 });
drag(ui, &mut reg, "export.range.end", &mut ex.custom_end, DragOptions { range: 0.0..=86_400.0, speed: 0.04, suffix: " s", decimals: 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));
+24
View File
@@ -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");
+8
View File
@@ -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.