From c4f8e50201bcbbcd44aaf1634d16277973079afb Mon Sep 17 00:00:00 2001 From: bflatastic Date: Wed, 7 Oct 2026 12:28:12 +0700 Subject: [PATCH] CLI: --format hevc / av1 write HEVC / AV1; an unknown comp is named (#154, #155) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #154: renderQueue.add (and render.addOutputModule) applied `format`, then dropped it before applying the output path, so `--format hevc --out x.mp4` saw the .mp4 extension and switched the module back to H.264 (and ProRes / VP9 for .mov / .webm). An explicit format now wins and a mismatched extension follows it (x.webm → x.mp4 for AV1), as choosing a format in the Output Module renames Output To; an extension that agrees with the format keeps the name as given. The format error lists hevc and av1 everywhere (one FORMATS string). #155: a `comp` that names no composition (name or id) failed with "no active composition". comp_id now reports it as `no composition "Nope"` (EngineError::NoSuchComp), and comp_for_queue passes that on instead of collapsing it. Tests: an_explicit_format_wins_over_the_output_extension and an_unknown_comp_is_named_in_the_error (engine). Checked with the CLI: the files are hevc/hvc1 and av1/av01 and decode cleanly with ffmpeg. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/mod.rs | 5 ++- crates/engine/src/commands/render_queue.rs | 45 +++++++++++---------- crates/engine/src/lib.rs | 3 ++ crates/engine/src/render_queue.rs | 6 +-- crates/engine/src/tests_codec_options.rs | 46 ++++++++++++++++++++++ 5 files changed, 77 insertions(+), 28 deletions(-) diff --git a/crates/engine/src/commands/mod.rs b/crates/engine/src/commands/mod.rs index 099133a1..9594b8a4 100644 --- a/crates/engine/src/commands/mod.rs +++ b/crates/engine/src/commands/mod.rs @@ -442,9 +442,10 @@ pub(crate) fn merge_p(p: &Value) -> Option<&str> { /// `comp`: item id or name; default the active comp. pub(crate) fn comp_id(s: &Session, p: &Value) -> Result { + let missing = |v: &Value| EngineError::NoSuchComp(v.to_string()); match p.get("comp") { - Some(Value::Number(n)) => n.as_u64().map(ItemId).filter(|id| s.project.comp(*id).is_some()).ok_or(EngineError::NoComp), - Some(Value::String(name)) => s.project.items.values().find(|i| &i.name == name && i.as_comp().is_some()).map(|i| i.id).ok_or(EngineError::NoComp), + Some(v @ Value::Number(n)) => n.as_u64().map(ItemId).filter(|id| s.project.comp(*id).is_some()).ok_or_else(|| missing(v)), + Some(v @ Value::String(name)) => s.project.items.values().find(|i| &i.name == name && i.as_comp().is_some()).map(|i| i.id).ok_or_else(|| missing(v)), _ => s.active_comp_id().ok_or(EngineError::NoComp), } } diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index c0eedaec..a7841e7e 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -19,7 +19,7 @@ use crate::{EngineError, Result, Session, cmd, query}; fn can_add(s: &Session) -> std::result::Result<(), String> { not_rendering(s)?; - if s.comp_for_queue(&Value::Null).is_some() { Ok(()) } else { Err("select or open a composition".into()) } + if s.comp_for_queue(&Value::Null).is_ok() { Ok(()) } else { Err("select or open a composition".into()) } } fn not_rendering(s: &Session) -> std::result::Result<(), String> { if s.is_rendering() { Err("the render queue is rendering".into()) } else { Ok(()) } @@ -202,6 +202,9 @@ fn apply_om_template(om: &mut OutputModule, t: &OutputModule) { om.name = t.name.clone(); } +/// The `format` parameter's values. +const FORMATS: &str = "format: h264|hevc|av1|prores|webm|png|jpeg|tiff|exr|gif|wav|aiff"; + /// Apply Output Module parameters (`template` first). `roi`: the viewer's region of interest /// (captured by `crop.useRoi`). fn apply_output(om: &mut OutputModule, templates: &RenderTemplates, roi: Option<[f64; 4]>, p: &Value, cmd: &str) -> Result { @@ -213,7 +216,7 @@ fn apply_output(om: &mut OutputModule, templates: &RenderTemplates, roi: Option< } let before = om.clone(); if let Some(f) = str_p(p, "format") { - let f = OutputFormat::from_name(f).ok_or_else(|| bad(cmd, "format: h264|hevc|av1|prores|webm|png|jpeg|tiff|exr|gif|wav|aiff"))?; + let f = OutputFormat::from_name(f).ok_or_else(|| bad(cmd, FORMATS))?; om.set_format(f); } if let Some(c) = str_p(p, "channels") { @@ -357,13 +360,17 @@ fn apply_output(om: &mut OutputModule, templates: &RenderTemplates, roi: Option< } if let Some(o) = str_p(p, "output").or(str_p(p, "path")) { om.output = o.to_string(); - if p.get("format").is_none() - && let Some(f) = OutputFormat::from_path(o).filter(|f| *f != om.format && !o.contains("[fileExtension]")) - { - // `out.mov` with an H.264 module → ProRes, like picking a file type in Output To. - let keep = om.output.clone(); - om.set_format(f); - om.output = keep; + if let Some(f) = OutputFormat::from_path(o).filter(|f| *f != om.format && !o.contains("[fileExtension]")) { + if p.get("format").is_some() { + // An explicit format wins and the extension follows it, as choosing a format in + // the Output Module renames Output To (#154: `--format hevc --out x.mp4`). + om.set_format(om.format); + } else { + // `out.mov` with an H.264 module → ProRes, like picking a file type in Output To. + let keep = om.output.clone(); + om.set_format(f); + om.output = keep; + } } } if *om != before { @@ -496,24 +503,20 @@ fn item_json(s: &Session, it: &RenderQueueItem, index: usize) -> Value { } fn add(s: &mut Session, p: &Value) -> Result { - let cid = s.comp_for_queue(p).ok_or(EngineError::NoComp)?; + let cid = s.comp_for_queue(p)?; let mut it = RenderQueueItem::new(0, cid); // New items start from the Movie Default templates (Edit ▸ Templates). let templates = s.project.render_templates.clone(); it.settings = templates.default_render_settings(TemplateSlot::Movie); it.output = templates.default_output_module(TemplateSlot::Movie); if let Some(f) = str_p(p, "format") { - let f = OutputFormat::from_name(f).ok_or_else(|| bad("renderQueue.add", "format: h264|prores|webm|png|jpeg|tiff|exr|gif|wav|aiff"))?; + let f = OutputFormat::from_name(f).ok_or_else(|| bad("renderQueue.add", FORMATS))?; if f != it.output.format { it.output = OutputModule::for_format(f); } } apply_settings(&mut it.settings, &templates, p, "renderQueue.add")?; - let mut op = p.clone(); - if let Some(o) = op.as_object_mut() { - o.remove("format"); - } - apply_output(&mut it.output, &templates, s.state.region_of_interest, &op, "renderQueue.add")?; + apply_output(&mut it.output, &templates, s.state.region_of_interest, p, "renderQueue.add")?; if let Some(l) = enum_p(p, "log", "renderQueue.add", RenderLog::parse, "errorsOnly|plusSettings|plusPerFrameInfo")? { it.log = l; } @@ -704,7 +707,7 @@ fn add_output_module(s: &mut Session, p: &Value) -> Result { let n = it.extra_outputs.len() + 2; let mut om = it.output.clone(); if let Some(f) = str_p(p, "format") { - let f = OutputFormat::from_name(f).ok_or_else(|| bad("render.addOutputModule", "format: h264|prores|webm|png|jpeg|tiff|exr|gif|wav|aiff"))?; + let f = OutputFormat::from_name(f).ok_or_else(|| bad("render.addOutputModule", FORMATS))?; om = OutputModule::for_format(f); } if str_p(p, "output").is_none() && str_p(p, "path").is_none() { @@ -714,11 +717,7 @@ fn add_output_module(s: &mut Session, p: &Value) -> Result { None => format!("{}_{n}", om.output), }; } - let mut op = p.clone(); - if let Some(o) = op.as_object_mut() { - o.remove("format"); - } - apply_output(&mut om, &s.project.render_templates, s.state.region_of_interest, &op, "render.addOutputModule")?; + apply_output(&mut om, &s.project.render_templates, s.state.region_of_interest, p, "render.addOutputModule")?; s.edit("Add Output Module", None, |proj, _| { let it = &mut proj.render_queue[i]; it.extra_outputs.push(om); @@ -733,7 +732,7 @@ fn add_output_module(s: &mut Session, p: &Value) -> Result { /// Composition ▸ Pre-render…: queue the comp with a lossless-with-alpha module whose post-render /// action imports the result and replaces the comp's uses. fn pre_render(s: &mut Session, p: &Value) -> Result { - let cid = s.comp_for_queue(p).ok_or(EngineError::NoComp)?; + let cid = s.comp_for_queue(p)?; let mut it = RenderQueueItem::new(0, cid); let templates = s.project.render_templates.clone(); it.settings = templates.default_render_settings(TemplateSlot::PreRender); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e690021b..8c5b4816 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -81,6 +81,9 @@ pub enum EngineError { BadParams { cmd: String, msg: String }, #[error("no active composition")] NoComp, + /// A `comp` reference (name or id) that names no composition. + #[error("no composition {0}")] + NoSuchComp(String), #[error("{0}")] Project(#[from] effectcraft_project::ProjectError), #[error("{0}")] diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index fb871d53..0ea4aac9 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -640,11 +640,11 @@ impl Session { } /// The comp to add: `comp` param, the active comp, or the first comp selected in the Project panel. - pub(crate) fn comp_for_queue(&self, p: &serde_json::Value) -> Option { + pub(crate) fn comp_for_queue(&self, p: &serde_json::Value) -> crate::Result { if p.get("comp").is_some() { - return crate::commands::comp_id(self, p).ok(); + return crate::commands::comp_id(self, p); } let sel = self.state.project_selection.iter().copied().find(|i| self.project.comp(*i).is_some()); - self.active_comp_id().or(sel) + self.active_comp_id().or(sel).ok_or(crate::EngineError::NoComp) } } diff --git a/crates/engine/src/tests_codec_options.rs b/crates/engine/src/tests_codec_options.rs index 21dc7e45..757f7c43 100644 --- a/crates/engine/src/tests_codec_options.rs +++ b/crates/engine/src/tests_codec_options.rs @@ -51,3 +51,49 @@ fn hevc_and_av1_output_module_options() { assert!(f["hevcLevels"].as_array().unwrap().contains(&json!("4.1"))); assert_eq!(f["webmCodecs"], json!(["vp9", "av1"])); } + +/// An explicit format wins over the output file's extension, which follows it (#154: +/// `effectcraft-cli render --format hevc --out x.mp4` wrote H.264). Without a format, the extension +/// still picks one. +#[test] +fn an_explicit_format_wins_over_the_output_extension() { + let mut s = session(); + let cases = [ + ("hevc", "/out/x.mp4", OutputFormat::Hevc, "/out/x.mp4"), + ("av1", "/out/x.mp4", OutputFormat::Av1, "/out/x.mp4"), + ("hevc", "/out/x.mov", OutputFormat::Hevc, "/out/x.mp4"), + ("av1", "/out/x.webm", OutputFormat::Av1, "/out/x.mp4"), + ("prores", "/out/x.mp4", OutputFormat::ProRes, "/out/x.mov"), + ]; + for (k, (format, out, want, path)) in cases.into_iter().enumerate() { + s.execute("renderQueue.add", json!({"format": format, "output": out})).unwrap(); + let om = &s.project.render_queue[k].output; + assert_eq!((om.format, om.output.as_str()), (want, path), "{format} → {out}"); + // A second output module of the item behaves the same. + s.execute("render.addOutputModule", json!({"index": k + 1, "format": format, "output": out})).unwrap(); + let extra = &s.project.render_queue[k].extra_outputs[0]; + assert_eq!((extra.format, extra.output.as_str()), (want, path), "addOutputModule {format} → {out}"); + } + // No format: the extension picks it, as before. A format the extension agrees with keeps + // the name as given (the CLI passes the extension as the format). + s.execute("renderQueue.add", json!({"output": "/out/y.mov"})).unwrap(); + assert_eq!(s.project.render_queue.last().unwrap().output.format, OutputFormat::ProRes); + s.execute("renderQueue.add", json!({"format": "png", "output": "/out/still.png"})).unwrap(); + let om = &s.project.render_queue.last().unwrap().output; + assert_eq!((om.format, om.output.as_str()), (OutputFormat::PngSequence, "/out/still.png")); +} + +/// A `comp` that names no composition says so, rather than "no active composition" (#155). +#[test] +fn an_unknown_comp_is_named_in_the_error() { + let mut s = session(); + for (cmd, p) in [ + ("renderQueue.add", json!({"comp": "Nope", "output": "/out/x.mp4"})), + ("layer.newSolid", json!({"comp": "Nope"})), + ("comp.settings", json!({"comp": 99_999, "width": 10})), + ] { + let e = s.execute(cmd, p.clone()).unwrap_err().to_string(); + let name = p["comp"].to_string(); + assert!(e.contains(&format!("no composition {name}")), "{cmd}: {e}"); + } +}