mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-30 21:16:52 -06:00
M4.12: never crash: hostile input in New Comp from Selection, an empty work area in Cache Frames When Idle
Following AGENTS.md's Never crash rules for the M3 / M4 additions: - Cache Frames When Idle panicked (`clamp` with min > max) when the work area was inverted (start set past the end); an empty work area now has nothing to cache. - `file.newCompFromSelection`: the sequence overlap times the layer count overflowed for a huge `overlapDuration` (wrapping to a wrong comp length without overflow checks); negative or non-finite values fall back to 1 s and the product saturates. The lead item is read with `get()` instead of indexing. - Layer Settings' enablement reads the first selected layer with `first()`. - Regression tests for both (each failed before the fix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
339a05d771
commit
b21e04b301
@@ -176,7 +176,7 @@ fn new_comp_from_selection(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let sequence = b_p(p, "sequence").unwrap_or(false).then(|| {
|
||||
json!({
|
||||
"overlap": b_p(p, "overlap").unwrap_or(false),
|
||||
"duration": f_p(p, "overlapDuration").unwrap_or(1.0),
|
||||
"duration": f_p(p, "overlapDuration").filter(|d| d.is_finite() && *d >= 0.0).unwrap_or(1.0),
|
||||
"transition": str_p(p, "transition").unwrap_or("off"),
|
||||
})
|
||||
});
|
||||
@@ -187,7 +187,7 @@ fn new_comp_from_selection(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
super::app_more::grouped(s, "New Comp from Selection", |s| {
|
||||
let mut made = vec![];
|
||||
for g in groups {
|
||||
let lead = proj.item(if single { g[from] } else { g[0] }).ok_or(EngineError::NoComp)?;
|
||||
let lead = g.get(if single { from } else { 0 }).and_then(|i| proj.item(*i)).ok_or(EngineError::NoComp)?;
|
||||
let (w, h) = lead.dimensions().unwrap_or((1920, 1080));
|
||||
let rate = lead.frame_rate().unwrap_or(FrameRate::FPS_29_97);
|
||||
let pixel_aspect = match &lead.kind {
|
||||
@@ -199,7 +199,7 @@ fn new_comp_from_selection(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let overlap =
|
||||
sequence.as_ref().filter(|q| q["overlap"] == true).map_or(Tick::ZERO, |q| Tick::from_seconds_f64(q["duration"].as_f64().unwrap_or(1.0)));
|
||||
let dur = match &sequence {
|
||||
Some(_) if g.len() > 1 => g.iter().map(|i| duration(*i)).fold(Tick::ZERO, |a, d| a + d) - Tick(overlap.0 * (g.len() as i64 - 1)),
|
||||
Some(_) if g.len() > 1 => g.iter().map(|i| duration(*i)).fold(Tick::ZERO, |a, d| a + d) - Tick(overlap.0.saturating_mul(g.len() as i64 - 1)),
|
||||
_ => g.iter().map(|i| duration(*i)).max().unwrap_or(Tick::from_seconds_f64(still)),
|
||||
}
|
||||
.max(rate.frame_duration());
|
||||
|
||||
@@ -1197,7 +1197,7 @@ fn layer_settings(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
/// Layer Settings applies to solids (and adjustment layers), nulls, cameras and lights.
|
||||
fn has_layer_settings(s: &Session) -> std::result::Result<(), String> {
|
||||
has_layers(s)?;
|
||||
let l = s.active_comp().and_then(|c| c.layer(s.state.selected_layers[0])).ok_or("select a layer first")?;
|
||||
let l = s.active_comp().zip(s.state.selected_layers.first()).and_then(|(c, l)| c.layer(*l)).ok_or("select a layer first")?;
|
||||
match l.source {
|
||||
LayerSource::Solid { .. } | LayerSource::Null | LayerSource::Camera | LayerSource::Light { .. } => Ok(()),
|
||||
_ => Err("this kind of layer has no settings".into()),
|
||||
|
||||
@@ -832,3 +832,34 @@ fn comp_settings_preserve_frame_rate_and_resolution() {
|
||||
s.execute("edit.undo", json!({})).unwrap();
|
||||
assert!(!s.active_comp().unwrap().preserve_frame_rate && !s.active_comp().unwrap().preserve_resolution);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_comp_from_selection_survives_hostile_sequence_numbers() {
|
||||
let mut s = Session::default();
|
||||
let still = |s: &mut Session, name: &str| {
|
||||
let f = effectcraft_project::Footage {
|
||||
kind: effectcraft_project::FootageKind::Still,
|
||||
width: 64,
|
||||
height: 64,
|
||||
pixel_aspect: 1.0,
|
||||
has_video: true,
|
||||
..Default::default()
|
||||
};
|
||||
std::sync::Arc::make_mut(&mut s.project).add_item(name, effectcraft_color::Label::None, None, ItemKind::Footage(f))
|
||||
};
|
||||
let a = still(&mut s, "a.png");
|
||||
let b = still(&mut s, "b.png");
|
||||
let c = still(&mut s, "c.png");
|
||||
for overlap in [json!(1e30), json!(-5), json!(f64::MAX)] {
|
||||
s.state.project_selection = vec![a, b, c];
|
||||
// A huge overlap overflowed the duration arithmetic (a panic with overflow checks, a
|
||||
// wrapped, too long comp without them).
|
||||
let r = s
|
||||
.execute("file.newCompFromSelection", json!({"single": true, "sequence": true, "overlap": true, "overlapDuration": overlap, "duration": 2}))
|
||||
.unwrap();
|
||||
let c = s.project.comp(effectcraft_project::ItemId(r["comps"][0].as_u64().unwrap())).unwrap();
|
||||
assert!(c.duration.seconds() <= 6.0 && c.duration > effectcraft_time::Tick::ZERO, "{overlap}: {}", c.duration.seconds());
|
||||
}
|
||||
s.state.project_selection = vec![a, b];
|
||||
assert!(s.execute("file.newCompFromSelection", json!({"single": true, "dimensionsFrom": 99})).is_err());
|
||||
}
|
||||
|
||||
@@ -942,8 +942,12 @@ impl EffectcraftApp {
|
||||
ctx.request_repaint_after(std::time::Duration::from_secs_f64(1.0 - quiet));
|
||||
return;
|
||||
}
|
||||
let cur = comp.frame_rate.frame_at(self.session.time()).clamp(wa, wb);
|
||||
// An empty work area has nothing to cache.
|
||||
let n = wb - wa + 1;
|
||||
if n < 1 {
|
||||
return;
|
||||
}
|
||||
let cur = comp.frame_rate.frame_at(self.session.time()).clamp(wa, wb);
|
||||
let fit = self.frames_that_fit(comp, series.scale as f64 / 1000.0).min(n);
|
||||
let slots = (self.frames_parallelism() / 2).max(1);
|
||||
let mut queued = self.frames.inflight();
|
||||
|
||||
@@ -257,3 +257,17 @@ fn cache_frames_when_idle_fills_the_work_area() {
|
||||
}
|
||||
assert_eq!(n, 240, "the work area is cached while idle");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_frames_when_idle_survives_an_inverted_work_area() {
|
||||
let mut h = harness();
|
||||
// Start set past the end: an empty work area (it panicked in `clamp` / `rem_euclid`).
|
||||
invoke(&mut h, "comp.workArea", json!({"end": 2.0}));
|
||||
invoke(&mut h, "comp.workArea", json!({"start": 5.0}));
|
||||
invoke(&mut h, "playback.cacheWhenIdle", json!({"value": true}));
|
||||
for _ in 0..150 {
|
||||
h.step();
|
||||
std::thread::sleep(std::time::Duration::from_millis(2));
|
||||
}
|
||||
assert!(!h.state().playback.playing);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user