Merge pull request #24 from storytold/pr/never-crash-engine

engine, edit: never crash (commands without expect/unwrap/panic)
This commit is contained in:
Brandon Thomas 2026-10-04 16:18:29 -04:00 committed by GitHub
commit 0171afa89d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 113 additions and 71 deletions

View File

@ -105,8 +105,9 @@ pub fn merge_captions(seq: &mut Sequence, ids: &[ClipId]) -> Result<ClipId> {
if idx.windows(2).any(|w| w[1] != w[0] + 1) {
return Err(EditError::Other("captions to merge must be next to each other".into()));
}
let first = idx[0];
let last = *idx.last().expect("non-empty");
let (Some(&first), Some(&last)) = (idx.first(), idx.last()) else {
return Err(EditError::Nothing);
};
let end = tr.captions[last].end();
let text: Vec<String> = idx.iter().map(|&i| tr.captions[i].text.trim().to_string()).filter(|t| !t.is_empty()).collect();
let keep = tr.captions[first].id;
@ -205,7 +206,7 @@ pub fn move_captions(seq: &mut Sequence, ids: &[ClipId], delta: Tick) -> Result<
return Err(EditError::Nothing);
}
for tid in tracks {
let tr = seq.caption_track_mut(tid).expect("checked");
let Some(tr) = seq.caption_track_mut(tid) else { continue };
for c in tr.captions.iter_mut().filter(|c| ids.contains(&c.id)) {
c.start += d;
}

View File

@ -15,6 +15,8 @@
//!
//! Keyframes are stored in media time, so trims and splits never need to move them.
#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
pub mod captions;
pub mod multicam;
pub mod through;

View File

@ -344,7 +344,9 @@ pub fn caption_blocks(words: &[SeqWord], rules: &CaptionRules, rate: FrameRate)
continue;
}
if fits_line {
*lines.last_mut().expect("non-empty") += 1 + len;
if let Some(l) = lines.last_mut() {
*l += 1 + len;
}
} else {
lines.push(len);
}
@ -362,7 +364,7 @@ pub fn caption_blocks(words: &[SeqWord], rules: &CaptionRules, rate: FrameRate)
for g in groups {
let mut text_lines: Vec<String> = vec![String::new()];
for w in &words[g.clone()] {
let l = text_lines.last_mut().expect("non-empty");
let Some(l) = text_lines.last_mut() else { break };
if l.is_empty() {
l.push_str(&w.text);
} else if l.chars().count() + 1 + w.text.chars().count() <= max_chars {

View File

@ -1097,7 +1097,7 @@ fn subclip_plan(s: &Session, p: &Value) -> Result<(ItemId, TimeRange, String, La
} else {
let item = item_p(p, "item").or_else(|| subclip_source(s)).ok_or_else(|| bad("clip.makeSubclip", "open a clip in the Source monitor"))?;
let (root, m, sub) = media_root(&s.project, item).ok_or_else(|| bad("clip.makeSubclip", "only media clips can be subclipped"))?;
let pi = s.project.item(item).expect("resolved");
let pi = s.project.item(item).ok_or_else(|| EngineError::Other("no such item".into()))?;
let rate = m.frame_rate();
let still = s.prefs.timeline.still_duration(rate);
let is_still = matches!(m.info.kind, filmcraft_media::MediaKind::Still);
@ -1246,7 +1246,7 @@ fn audio_channels(s: &mut Session, p: &Value) -> Result<Value> {
let mut out = Vec::new();
let mut maps = Vec::new();
for i in &items {
let m = s.project.item(*i).and_then(|it| it.as_media()).expect("filtered");
let Some(m) = s.project.item(*i).and_then(|it| it.as_media()) else { continue };
let n = m.info.audio.as_ref().map_or(2, |a| a.channels) as u16;
let fmt = format.unwrap_or_else(|| m.interpret.audio_channels.as_ref().map_or(AudioChannels::Stereo, |a| a.format));
let map = match &custom {
@ -1603,7 +1603,7 @@ fn extract_audio(s: &mut Session, p: &Value) -> Result<Value> {
})
.ok_or_else(|| EngineError::Other("save the project first: extracted audio is written next to it".into()))?,
};
let pi = s.project.item(item).expect("resolved");
let pi = s.project.item(item).ok_or_else(|| EngineError::Other("no such item".into()))?;
let name = pi.name.clone();
let stem = std::path::Path::new(&name).file_stem().map(|x| x.to_string_lossy().into_owned()).unwrap_or_else(|| name.clone());
let src = s.media.full_res_source(&s.project, root, &*s.services).ok_or_else(|| EngineError::Other(format!("{name}: media is offline")))?;

View File

@ -362,7 +362,7 @@ pub(crate) fn place_item(
let frame = (seq.settings.width, seq.settings.height);
let mut placements = Vec::new();
let link = if has_v && has_a { Some(p.alloc_id()) } else { None };
if has_v {
if let Some(vdest) = vdest.filter(|_| has_v) {
let mut v = p.make_track_item(item, TrackKind::Video, at, range, rate).ok_or_else(|| bad(label, "bad item"))?;
v.link = link;
for e in &mut v.effects {
@ -371,10 +371,9 @@ pub(crate) fn place_item(
if is_media && src_size != frame {
crate::settings::apply_media_scaling(&mut v, &scaling, frame, src_size);
}
placements.push((vdest.expect("checked"), v));
placements.push((vdest, v));
}
if has_a {
let adest = adest.expect("checked");
if let Some(adest) = adest.filter(|_| has_a) {
let (arange, aat) = audio_split.unwrap_or((range, at));
let mut a = p.make_track_item(item, TrackKind::Audio, aat, arange, rate).ok_or_else(|| bad(label, "bad item"))?;
// Modify ▸ Audio Channels with several audio clips: one per clip, on the tracks below
@ -675,27 +674,27 @@ fn build() -> Vec<CommandSpec> {
Ok(id) => ids.push(id.0),
Err(e) => errors.push(format!("{path}: {e}")),
},
Ok(b) if crate::captions::detect(&path, &b).is_some() => {
let fmt = crate::captions::detect(&path, &b).expect("detected");
match crate::captions::import(s, &path, &b, fmt, None) {
Ok(r) => reports.push(r),
Err(e) => errors.push(format!("{path}: {e}")),
}
}
Ok(b) if crate::interchange::detect(&path, &b).is_some() => {
let fmt = crate::interchange::detect(&path, &b).expect("detected");
match crate::interchange::import(s, &path, &b, fmt) {
Ok(r) => {
sequences.extend(r["sequences"].as_array().cloned().unwrap_or_default());
reports.push(r);
Ok(b) => {
if let Some(fmt) = crate::captions::detect(&path, &b) {
match crate::captions::import(s, &path, &b, fmt, None) {
Ok(r) => reports.push(r),
Err(e) => errors.push(format!("{path}: {e}")),
}
} else if let Some(fmt) = crate::interchange::detect(&path, &b) {
match crate::interchange::import(s, &path, &b, fmt) {
Ok(r) => {
sequences.extend(r["sequences"].as_array().cloned().unwrap_or_default());
reports.push(r);
}
Err(e) => errors.push(format!("{path}: {e}")),
}
} else {
match import_bytes(s, &path, b.into(), bin) {
Ok(id) => ids.push(id.0),
Err(e) => errors.push(format!("{path}: {e}")),
}
Err(e) => errors.push(format!("{path}: {e}")),
}
}
Ok(b) => match import_bytes(s, &path, b.into(), bin) {
Ok(id) => ids.push(id.0),
Err(e) => errors.push(format!("{path}: {e}")),
},
Err(e) => errors.push(format!("{path}: {e}")),
}
}
@ -1280,7 +1279,7 @@ fn build() -> Vec<CommandSpec> {
let p = p.clone();
s.edit("Sequence Settings", |pr, _| {
if let Some(n) = str_p(&p, "name") {
pr.item_mut(id).expect("seq").name = n.to_string();
pr.item_mut(id).ok_or(EngineError::NoSequence)?.name = n.to_string();
}
let q = pr.sequence_mut(id).ok_or(EngineError::NoSequence)?;
if let Some(w) = u64_p(&p, "width") {
@ -2454,7 +2453,7 @@ fn nest(s: &mut Session, p: &Value) -> Result<Value> {
s.edit("Nest", |pr, st| {
let nid = pr.new_sequence(&name, q.settings.clone(), q.video_tracks.len(), q.audio_tracks.len(), None);
{
let nq = pr.sequence_mut(nid).expect("new");
let nq = pr.sequence_mut(nid).ok_or(EngineError::NoSequence)?;
for (k, ti, it) in &items {
let mut it = it.clone();
it.start -= start;
@ -2465,11 +2464,15 @@ fn nest(s: &mut Session, p: &Value) -> Result<Value> {
}
}
let rate = q.settings.frame_rate;
let mut v = pr.make_track_item(nid, TrackKind::Video, start, TimeRange::new(Tick::ZERO, end - start), rate).expect("item");
let mut v = pr
.make_track_item(nid, TrackKind::Video, start, TimeRange::new(Tick::ZERO, end - start), rate)
.ok_or_else(|| EngineError::Other("cannot place the nested sequence".into()))?;
for e in &mut v.effects {
resolve_auto_points(e, (q.settings.width, q.settings.height), (q.settings.width, q.settings.height));
}
let a = pr.make_track_item(nid, TrackKind::Audio, start, TimeRange::new(Tick::ZERO, end - start), rate).expect("item");
let a = pr
.make_track_item(nid, TrackKind::Audio, start, TimeRange::new(Tick::ZERO, end - start), rate)
.ok_or_else(|| EngineError::Other("cannot place the nested sequence".into()))?;
let link = pr.alloc_id();
let seq = pr.sequence_mut(seq_id).ok_or(EngineError::NoSequence)?;
edit::delete_items(seq, &sel);

View File

@ -44,6 +44,15 @@ pub fn add_generator(p: &mut Project, pool: &MediaPool, src: GeneratorSource, na
/// Build the demo project. Returns (project, main sequence).
pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
build_demo(pool).unwrap_or_else(|| {
// Only reachable if the built-in items could not be placed: an empty demo, not a crash.
let mut p = Project::new("FilmCraft Demo");
let seq = p.new_sequence("Main Edit", SequenceSettings::default(), 3, 3, None);
(p, seq)
})
}
fn build_demo(pool: &MediaPool) -> Option<(Project, ItemId)> {
let mut p = Project::new("FilmCraft Demo");
let footage = p.add_bin("Footage", None);
let audio_bin = p.add_bin("Audio", None);
@ -108,8 +117,8 @@ pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
for (ci, sin, dur) in cuts {
let src_range = TimeRange::new(r.snap(Tick::from_seconds_f64(sin)), r.snap(Tick::from_seconds_f64(dur)));
let link = p.alloc_id();
let mut v = p.make_track_item(clips[ci], TrackKind::Video, t, src_range, r).expect("item");
let mut a = p.make_track_item(clips[ci], TrackKind::Audio, t, src_range, r).expect("item");
let mut v = p.make_track_item(clips[ci], TrackKind::Video, t, src_range, r)?;
let mut a = p.make_track_item(clips[ci], TrackKind::Audio, t, src_range, r)?;
v.link = Some(link);
a.link = Some(link);
for e in &mut v.effects {
@ -118,14 +127,14 @@ pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
a.effect_mut("volume").map(|e| e.params.get_mut("level").map(|l| l.value = ParamValue::Float(-8.0)));
let dur_t = v.duration;
placed.push((v.id, a.id, t, dur_t));
let s = p.sequence_mut(seq).expect("seq");
let s = p.sequence_mut(seq)?;
s.video_tracks[0].items.push(v);
s.audio_tracks[0].items.push(a);
t += dur_t;
}
// A lower-third-ish overlay on V2: the plasma clip scaled down in the corner with a drop shadow.
let ov_range = TimeRange::new(Tick::ZERO, r.snap(Tick::from_seconds_f64(4.0)));
let mut ov = p.make_track_item(clips[4], TrackKind::Video, r.snap(Tick::from_seconds_f64(6.0)), ov_range, r).expect("item");
let mut ov = p.make_track_item(clips[4], TrackKind::Video, r.snap(Tick::from_seconds_f64(6.0)), ov_range, r)?;
for e in &mut ov.effects {
resolve_auto_points(e, (1920, 1080), (1920, 1080));
}
@ -152,18 +161,18 @@ pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
lum.params.get_mut("temperature").map(|v| v.value = ParamValue::Float(18.0));
lum.params.get_mut("contrast").map(|v| v.value = ParamValue::Float(22.0));
lum.params.get_mut("vignette_amount").map(|v| v.value = ParamValue::Float(-1.4));
let s = p.sequence_mut(seq).expect("seq");
let s = p.sequence_mut(seq)?;
if let Some(it) = s.video_tracks[0].items.get_mut(3) {
it.effects.insert(0, lum);
}
}
let s = p.sequence_mut(seq).expect("seq");
let s = p.sequence_mut(seq)?;
s.video_tracks[1].items.push(ov);
// Transitions: cross dissolve between shots 1-2, dip to black 3-4, push 5-6, fade in at start.
let tr_len = r.tick_of(24);
let mk = |id: u64, eff: &str, from: Option<filmcraft_project::ClipId>, to: Option<filmcraft_project::ClipId>, start: Tick| Transition {
id: TransitionId(id),
effect: find_effect(eff).expect("fx").instance(),
effect: crate::presets::effect_instance(eff),
start,
duration: tr_len,
from,
@ -181,7 +190,7 @@ pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
s.video_tracks[0].transitions = trs;
s.audio_tracks[0].transitions.push(Transition {
id: TransitionId(900_010),
effect: find_effect("constant_power").expect("fx").instance(),
effect: crate::presets::effect_instance("constant_power"),
start: cut(1) - tr_len.mul_ratio(1, 2),
duration: tr_len,
from: Some(placed[0].1),
@ -207,13 +216,13 @@ pub fn demo_project(pool: &MediaPool) -> (Project, ItemId) {
});
}
// Music on A2 (under the whole edit), ducked -14 dB.
let mut m = p.make_track_item(music, TrackKind::Audio, Tick::ZERO, TimeRange::new(Tick::ZERO, end), r).expect("music");
let mut m = p.make_track_item(music, TrackKind::Audio, Tick::ZERO, TimeRange::new(Tick::ZERO, end), r)?;
m.effect_mut("volume").map(|e| e.params.get_mut("level").map(|l| l.value = ParamValue::Float(-14.0)));
m.label = Label::Caribbean;
let s = p.sequence_mut(seq).expect("seq");
let s = p.sequence_mut(seq)?;
s.audio_tracks[1].items.push(m);
for tr in s.all_tracks_mut() {
tr.sort();
}
(p, seq)
Some((p, seq))
}

View File

@ -157,15 +157,15 @@ pub fn thumbnail(t: &GraphicsTemplate, w: u32) -> (u32, u32, Vec<u8>) {
let mut p = Project::new("thumb");
let rate = filmcraft_time::FrameRate::FPS_30;
let g = p.add_item("G", Label::Rose, ItemKind::Graphic { width: t.canvas[0], height: t.canvas[1], rate }, None);
let mut it = p
.make_track_item(
g,
filmcraft_project::TrackKind::Video,
Tick::ZERO,
filmcraft_time::TimeRange::new(Tick::ZERO, t.duration.max(rate.frame_duration())),
rate,
)
.expect("item");
let Some(mut it) = p.make_track_item(
g,
filmcraft_project::TrackKind::Video,
Tick::ZERO,
filmcraft_time::TimeRange::new(Tick::ZERO, t.duration.max(rate.frame_duration())),
rate,
) else {
return (w, h, vec![0; w as usize * h as usize * 4]);
};
it.effects.extend(t.layers.iter().cloned());
it.graphic = Some(Box::new(meta));
// a moment into the clip, so intro animations have settled a little

View File

@ -551,11 +551,11 @@ macro_rules! arrange_cmd {
fn new_shape_cmd(s: &mut Session, p: &Value, shape: &str) -> Result<Value> {
let mut q = p.clone();
q["shape"] = json!(shape);
(find_spec("graphics.newShape").run)(s, &q)
(find_spec("graphics.newShape")?.run)(s, &q)
}
fn find_spec(id: &str) -> CommandSpec {
commands().into_iter().find(|c| c.id == id).expect("graphics command")
fn find_spec(id: &str) -> Result<CommandSpec> {
commands().into_iter().find(|c| c.id == id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))
}
/// Graphics menu commands (New Layer shapes, Align/Distribute/Arrange/Select submenus, resets).
@ -576,7 +576,7 @@ fn menu_commands() -> Vec<CommandSpec> {
|s, p| {
let mut p = if p.is_object() { p.clone() } else { json!({}) };
p["vertical"] = json!(true);
(find_spec("graphics.newText").run)(s, &p)
(find_spec("graphics.newText")?.run)(s, &p)
},
),
spec(
@ -731,7 +731,7 @@ fn menu_commands() -> Vec<CommandSpec> {
let rate = q.settings.frame_rate;
let want = rate.snap_nearest(Tick::from_seconds_f64(seconds)).max(rate.frame_duration());
let tr = q.video_tracks.iter_mut().find(|tr| tr.items.iter().any(|i| i.id == clip)).ok_or(filmcraft_edit::EditError::NoItem(clip))?;
let i = tr.items.iter().position(|i| i.id == clip).expect("found");
let i = tr.items.iter().position(|i| i.id == clip).ok_or(filmcraft_edit::EditError::NoItem(clip))?;
let start = tr.items[i].start;
let limit = tr.items.get(i + 1).map_or(want, |n| n.start - start);
let d = want.min(limit).max(rate.frame_duration());

View File

@ -8,6 +8,8 @@
//! The project is an `Arc<Project>` edited copy-on-write; undo keeps whole-project snapshots (cheap
//! thanks to structural sharing of untouched items).
#![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable))]
pub mod aaf_omf;
pub mod autosave;
pub mod captions;

View File

@ -554,7 +554,7 @@ fn merge_clips(s: &mut Session, p: &Value) -> Result<Value> {
a.link = Some(link);
a_items.push(a);
}
let q = pr.sequence_mut(sid).expect("new sequence");
let q = pr.sequence_mut(sid).ok_or(EngineError::NoSequence)?;
for (t, it) in q.video_tracks.iter_mut().zip(v_items) {
t.items.push(it);
}
@ -616,7 +616,7 @@ fn create_multicam(s: &mut Session, p: &Value) -> Result<Value> {
let (mut vi, mut ai) = (0usize, 0usize);
let mut placed: Vec<(TrackKind, usize, TrackItem)> = Vec::new();
let (vtracks, atracks): (Vec<TrackId>, Vec<TrackId>) = {
let q = pr.sequence(sid).expect("new sequence");
let q = pr.sequence(sid).ok_or(EngineError::NoSequence)?;
(q.video_tracks.iter().map(|t| t.id).collect(), q.audio_tracks.iter().map(|t| t.id).collect())
};
for (k, c) in clips.iter().enumerate() {
@ -640,7 +640,7 @@ fn create_multicam(s: &mut Session, p: &Value) -> Result<Value> {
cameras.push(cam);
}
{
let q = pr.sequence_mut(sid).expect("new sequence");
let q = pr.sequence_mut(sid).ok_or(EngineError::NoSequence)?;
for (kind, idx, it) in placed {
q.tracks_mut(kind)[idx].items.push(it);
}
@ -892,10 +892,14 @@ fn record_start(s: &mut Session, p: &Value) -> Result<Value> {
fn apply_pass(s: &mut Session, end: Tick) -> Result<usize> {
let Some(seq_id) = s.mcrec.seq else { return Ok(0) };
let merging = s.history.merge_key.as_deref() == Some(s.mcrec.key.as_str()) && s.history.undo.last().is_some_and(|u| u.0 == RECORD_LABEL);
if !merging || s.mcrec.base.is_none() {
s.mcrec.base = Some(s.project.clone());
}
let base = s.mcrec.base.clone().expect("set above");
let base = match s.mcrec.base.clone() {
Some(b) if merging => b,
_ => {
let b = s.project.clone();
s.mcrec.base = Some(b.clone());
b
}
};
let (tracks, cuts, key) = (s.mcrec.tracks.clone(), s.mcrec.cuts.clone(), s.mcrec.key.clone());
let Some(source) = s.mcrec.source else { return Ok(0) };
let media = s.media.clone();

View File

@ -151,8 +151,22 @@ fn kf(t_secs: f64, v: ParamValue) -> Keyframe {
Keyframe::new(Tick((t_secs * TICKS_PER_SECOND as f64).round() as i64), v)
}
/// A fresh instance of a built-in effect (a bare instance named `id` if the registry lacks it,
/// which the preset tests rule out).
pub(crate) fn effect_instance(id: &str) -> EffectInstance {
filmcraft_project::find_effect(id).map(|d| d.instance()).unwrap_or_else(|| EffectInstance {
effect: id.to_string(),
enabled: true,
params: Default::default(),
masks: Vec::new(),
post_fader: false,
essential: false,
layer: None,
})
}
fn effect(id: &str, params: &[(&str, ParamValue)]) -> EffectInstance {
let mut e = filmcraft_project::find_effect(id).unwrap_or_else(|| panic!("built-in preset uses unknown effect {id}")).instance();
let mut e = effect_instance(id);
for (k, v) in params {
if let Some(p) = e.params.get_mut(*k) {
p.value = v.clone();

View File

@ -381,7 +381,10 @@ fn make_subsequence(s: &mut Session, p: &Value) -> Result<Value> {
let offset = range.map(|r| r.start).unwrap_or_else(|| items.iter().map(|i| i.2.start).min().unwrap_or_default());
let name = match str_p(p, "name") {
Some(n) => n.to_string(),
None => (1..).map(|k| format!("{seq_name}_Sub_{k:02}")).find(|n| !s.project.items.values().any(|i| &i.name == n)).expect("free name"),
None => (1..)
.map(|k| format!("{seq_name}_Sub_{k:02}"))
.find(|n| !s.project.items.values().any(|i| &i.name == n))
.unwrap_or_else(|| format!("{seq_name}_Sub")),
};
let id = s.edit("Make Subsequence", |pr, st| {
let nid = pr.new_sequence(&name, q.settings.clone(), q.video_tracks.len(), q.audio_tracks.len(), None);

View File

@ -441,7 +441,9 @@ fn bindable(c: &CommandSpec) -> bool {
}
_ => {}
}
fields.last_mut().expect("field").push(ch);
if let Some(f) = fields.last_mut() {
f.push(ch);
}
}
fields.iter().all(|f| f.contains('?') || f.contains('='))
}
@ -505,7 +507,7 @@ impl Shortcuts {
dir: None,
session_start: None,
};
s.bindings = s.builtin(DEFAULT_PRESET).expect("default preset");
s.bindings = s.builtin(DEFAULT_PRESET).unwrap_or_default();
s
}

View File

@ -433,7 +433,7 @@ pub fn shuttle(s: &mut Session, p: &Value) -> Result<Value> {
tick(s, clock)?;
}
let rate = s.sequence_rate();
let d = s.trim_play.dynamic.as_mut().expect("dynamic trim");
let d = s.trim_play.dynamic.as_mut().ok_or_else(|| EngineError::Other("dynamic trimming is not active".into()))?;
let speed = if slow {
0.25 * dir
} else if d.speed * dir > 0.0 && !d.at_limit {