From 74b95b0a9dcea5ebc5e69423bc039f7efb8218de Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 11 Oct 2026 18:49:54 +0200 Subject: [PATCH] #886: say why footage the decoders can't read shows no picture Problem: a movie whose profile the software decoders lack (H.264 High 4:2:2 / 10-bit) probes fine, then every frame read fails; MediaPool logged the error per frame and returned None, so the layer rendered black with no sign why in the viewer, Project panel or for agents. Fix: MediaPool records the reason once per file (codec from the probe plus the decoder's text without error wrappers; I/O errors stay with missing-footage handling), logs it once, and exposes it through the new FootageSource::problem. The Composition viewer shows it on the affected layer, the Project panel marks the item with a warning whose tooltip is the reason, and project.summary / footage.info carry a "problem" field. Cleared when the file is forgotten; only set when decoding really failed, so a hardware decoder that handles the file shows nothing. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/commands/footage_panel.rs | 6 +- crates/engine/src/commands/query.rs | 7 +++ crates/engine/src/tests_panels.rs | 33 ++++++++++ crates/media/src/pool.rs | 52 ++++++++++++++- crates/media/tests/footage.rs | 29 +++++++++ crates/render/src/lib.rs | 7 +++ crates/ui-egui/src/panels/project.rs | 16 ++++- crates/ui-egui/src/panels/viewer.rs | 24 +++++++ crates/ui-egui/tests/ui_unreadable_footage.rs | 63 +++++++++++++++++++ 9 files changed, 232 insertions(+), 5 deletions(-) create mode 100644 crates/ui-egui/tests/ui_unreadable_footage.rs diff --git a/crates/engine/src/commands/footage_panel.rs b/crates/engine/src/commands/footage_panel.rs index f984d53f..0dd42f68 100644 --- a/crates/engine/src/commands/footage_panel.rs +++ b/crates/engine/src/commands/footage_panel.rs @@ -206,8 +206,12 @@ fn info(s: &mut Session, _: &Value) -> Result { let item = s.project.item(v.item); let name = item.map(|i| i.name.clone()).unwrap_or_default(); let frame = fr.frame_at(v.time); + let problem = match item.map(|i| &i.kind) { + Some(ItemKind::Footage(f)) => s.footage.problem(f), + _ => None, + }; Ok( - json!({"item": v.item.0, "name": name, "time": v.time.seconds(), "frame": frame, "timecode": timecode(item, fr, frame), "in": v.in_point.map(|t| t.seconds()), "out": v.out_point.map(|t| t.seconds()), "duration": dur.map(|d| d.seconds()), "frameRate": fr.as_f64()}), + json!({"item": v.item.0, "name": name, "time": v.time.seconds(), "frame": frame, "timecode": timecode(item, fr, frame), "in": v.in_point.map(|t| t.seconds()), "out": v.out_point.map(|t| t.seconds()), "duration": dur.map(|d| d.seconds()), "frameRate": fr.as_f64(), "problem": problem}), ) } diff --git a/crates/engine/src/commands/query.rs b/crates/engine/src/commands/query.rs index 56139b6c..b9175afa 100644 --- a/crates/engine/src/commands/query.rs +++ b/crates/engine/src/commands/query.rs @@ -46,6 +46,13 @@ fn summary(s: &mut Session, _: &Value) -> Result { o["layers"] = json!(c.layers.len()); o["frameRate"] = json!(c.frame_rate.as_f64()); } + // Footage that shows no picture although its file is there (a codec or profile the + // decoders lack): why, so an agent can tell the user. + if let ItemKind::Footage(f) = &i.kind + && let Some(why) = s.footage.problem(f) + { + o["problem"] = json!(why); + } o }) .collect(); diff --git a/crates/engine/src/tests_panels.rs b/crates/engine/src/tests_panels.rs index b77c7e9b..c48dc3db 100644 --- a/crates/engine/src/tests_panels.rs +++ b/crates/engine/src/tests_panels.rs @@ -629,3 +629,36 @@ fn media_browser_over_a_virtual_tree() { assert_eq!(*b.actions.lock().unwrap(), ["openFolder"]); assert!(s.execute("mediaBrowser.action", json!({})).is_err()); } + +/// Footage the source can't decode is reported with its reason by `project.summary` and +/// `footage.info`, so an agent can tell the user why the picture is black. +#[test] +fn footage_that_cannot_be_decoded_reports_why() { + struct Broken; + impl FootageSource for Broken { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + None + } + fn problem(&self, f: &Footage) -> Option { + Some(format!("{} isn't supported by the built-in decoder", f.codec)) + } + } + let mut s = Session { footage: Arc::new(Broken), ..Default::default() }; + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 36, "duration": 2})).unwrap(); + let f = Footage { + kind: FootageKind::Video, + path: "a.mp4".into(), + codec: "H.264 (High 4:2:2)".into(), + width: 64, + height: 36, + has_video: true, + ..Default::default() + }; + let item = std::sync::Arc::make_mut(&mut s.project).add_item("Clip", Default::default(), None, ItemKind::Footage(f)); + let sum = s.execute("project.summary", json!({})).unwrap(); + let it = sum["items"].as_array().unwrap().iter().find(|i| i["id"] == json!(item.0)).unwrap(); + assert_eq!(it["problem"], json!("H.264 (High 4:2:2) isn't supported by the built-in decoder")); + assert!(sum["items"].as_array().unwrap().iter().filter(|i| i["id"] != json!(item.0)).all(|i| i.get("problem").is_none()), "only footage reports one"); + s.execute("footage.open", json!({"item": item.0})).unwrap(); + assert_eq!(s.execute("footage.info", json!({})).unwrap()["problem"], it["problem"]); +} diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index ce980c56..b08c83d0 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -206,6 +206,9 @@ struct Inner { /// audio is read from the decoder instead of trying again on every read. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] conform_failed: Mutex>, + /// Movies that failed to open or decode (path to a readable reason; see + /// [`FootageSource::problem`]). The first failure of a file is logged, later ones are not. + problems: Mutex, String>>, } /// Decodes footage for the renderer: stills, image sequences and movies (FilmCraft codecs), plus @@ -273,6 +276,7 @@ impl MediaPool { conform: Mutex::default(), conforming: Mutex::default(), conform_failed: Mutex::default(), + problems: Mutex::default(), }), } } @@ -320,6 +324,7 @@ impl MediaPool { pub fn clear(&self) { self.clear_frames(); lock(&self.inner.sources).clear(); + lock(&self.inner.problems).clear(); } /// Forget a file (after it changed on disk, or to retry a file that failed to open). @@ -327,6 +332,7 @@ impl MediaPool { lock(&self.inner.sources).remove(path); lock(&self.inner.models).remove(path); lock(&self.inner.gifs).remove(path); + lock(&self.inner.problems).remove(path); let mut c = lock(&self.inner.cache); let keys: Vec = c.lru.map.keys().filter(|k| &*k.path == path).cloned().collect(); for k in keys { @@ -355,8 +361,11 @@ impl MediaPool { } match Inner::frame_at(&self.inner, footage, t, true, size) { Ok(img) => Some(img), + // (a movie's failure is logged once per file, when it is recorded) Err(e) => { - log::warn!("media: frame of {} at {:?}: {e}", footage.path, t); + if lock(&self.inner.problems).get(footage.path.as_str()).is_none() { + log::warn!("media: frame of {} at {:?}: {e}", footage.path, t); + } None } } @@ -582,7 +591,7 @@ impl Inner { match opened { Ok(s) => Some(s), Err(e) => { - log::warn!("media: cannot open {path}: {e}"); + self.note_problem(path, "", &e); None } } @@ -590,6 +599,19 @@ impl Inner { .clone() } + /// Remember why the movie at `path` cannot be shown (`codec` is the footage's, when known) + /// and log it the first time. I/O errors (missing or unreadable file) are not decoder + /// problems and are left to the missing-footage handling. + fn note_problem(&self, path: &Arc, codec: &str, e: &MediaError) { + if matches!(e, MediaError::Io(_)) { + return; + } + let reason = describe_problem(codec, e); + if lock(&self.problems).insert(path.clone(), reason.clone()).is_none() { + log::warn!("media: {path}: {reason}"); + } + } + /// The decoded frames of the animated GIF at `path`. The first caller decodes it, outside the /// map's lock; callers asking for the same file meanwhile wait for that. fn gif(&self, path: &Arc) -> Result> { @@ -752,7 +774,10 @@ impl Inner { } Some(mt) => { let src = self.source(path).ok_or_else(|| MediaError::Io(format!("{path}: cannot open")))?; - let vf = src.video_frame(FrameRequest::full(filmcraft_time::Tick(mt.0))).map_err(MediaError::from)?; + let vf = src + .video_frame(FrameRequest::full(filmcraft_time::Tick(mt.0))) + .map_err(MediaError::from) + .inspect_err(|e| self.note_problem(path, &footage.codec, e))?; // A reduced-resolution render's frame, made at its size (a frame of the footage's // own size only, as the renderer resamples only those). let (w, h) = loc.key.size; @@ -798,6 +823,13 @@ impl FootageSource for MediaPool { self.frame_sized(item, footage, t, Some((width, height))) } + fn problem(&self, footage: &Footage) -> Option { + if footage.missing || footage.kind != FootageKind::Video { + return None; + } + lock(&self.inner.problems).get(footage.path.as_str()).cloned() + } + fn vector_frame(&self, _item: ItemId, footage: &Footage, scale: f64) -> Option> { if footage.missing || !effectcraft_render::is_vector_footage(footage) { return None; @@ -845,6 +877,20 @@ fn read_stereo(src: &SharedSource, s0: i64, frames: usize, rate: u32) -> std::re Ok(out) } +/// The one-line reason a movie shows no picture: its `codec` (the probe's, e.g. "H.264 (High +/// 4:2:2)") and what the decoder said, without the error wrappers it passed through. +fn describe_problem(codec: &str, e: &MediaError) -> String { + let (MediaError::Io(s) | MediaError::Unsupported(s) | MediaError::Decode(s)) = e; + let mut detail = s.as_str(); + let mut unsupported = matches!(e, MediaError::Unsupported(_)); + while let Some(rest) = ["decode error: ", "unsupported media: ", "unsupported: ", "I/O error: "].iter().find_map(|p| detail.strip_prefix(p)) { + unsupported |= detail.starts_with("unsupported"); + detail = rest; + } + let what = if codec.is_empty() { "This video" } else { codec }; + if unsupported { format!("{what} isn't supported by the built-in decoder: {detail}") } else { format!("{what} can't be decoded: {detail}") } +} + fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index d26b1a1d..8afc5bc7 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -674,3 +674,32 @@ fn conformed_flac_audio_reads_back_like_the_decoder() { assert_eq!(pool.audio_samples(&f, at, 4800, 48_000), decoded); let _ = std::fs::remove_dir_all(&folder); } + +/// H.264 High 4:2:2 10-bit: the software decoder lacks it; the pool says why, once, and the +/// footage reports it (not a black frame without a word). +#[test] +fn an_unsupported_stream_reports_why_it_shows_no_picture() { + let Some(p) = ffmpeg_fixture( + "h264_high422_10bit.mp4", + &["-f", "lavfi", "-i", "testsrc=size=320x180:rate=24:duration=1", "-c:v", "libx264", "-profile:v", "high422", "-pix_fmt", "yuv422p10le"], + ) else { + return; + }; + let f = probe(&p).expect("probe"); + let pool = MediaPool::new(); + assert_eq!(pool.problem(&f), None, "nothing is wrong before a frame was asked for"); + if pool.frame(ItemId(1), &f, Tick::ZERO).is_some() { + // a decoder that handles 4:2:2 (FilmCraft's, or a hardware one): no notice + assert_eq!(pool.problem(&f), None); + return; + } + let why = pool.problem(&f).expect("a failed frame leaves a reason"); + assert!(why.contains("4:2:2"), "names the stream: {why}"); + assert!(why.contains("isn't supported"), "says it is unsupported: {why}"); + assert!(!why.contains("decode error"), "no error wrappers: {why}"); + // it stays for the other frames, and is dropped when the file is forgotten + assert!(pool.frame(ItemId(1), &f, Tick::from_seconds_f64(0.5)).is_none()); + assert_eq!(pool.problem(&f).as_deref(), Some(why.as_str())); + pool.forget(&f.path); + assert_eq!(pool.problem(&f), None); +} diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index f4a5a98a..8084a2f6 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -85,6 +85,13 @@ pub trait FootageSource: Send + Sync { fn vector_frame(&self, _item: ItemId, _footage: &Footage, _scale: f64) -> Option> { None } + /// Why `footage` shows no picture although its file is there: a read of it failed because + /// no decoder handles its codec, profile, chroma format or bit depth, or the stream is damaged + /// (e.g. "H.264 (High 4:2:2) isn't supported by the built-in decoder: ..."). `None` while it + /// decodes, and for sources that don't track failures. Stays set until the file is forgotten. + fn problem(&self, _footage: &Footage) -> Option { + None + } } /// Footage that can be rasterised at any scale (SVG). diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 21f84297..6c8d3b8e 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -441,7 +441,21 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { actions.push(("file.useProxy".into(), json!({"item": id.0}))); } } - let name_clip = Rect::from_min_max(pos2(x0 + 30.0, r.min.y), pos2(r.min.x + name_w + 4.0, r.max.y)).intersect(list); + // Footage that can't be decoded (no decoder for its codec / profile): a warning marker + // at the end of the name cell, the reason on hover. + let problem = match &it.kind { + ItemKind::Footage(f) => app.session.footage.problem(f), + _ => None, + }; + let name_clip = + Rect::from_min_max(pos2(x0 + 30.0, r.min.y), pos2(r.min.x + name_w + 4.0 - if problem.is_some() { 16.0 } else { 0.0 }, r.max.y)).intersect(list); + if let Some(why) = &problem { + let m = Rect::from_center_size(pos2(r.min.x + name_w - 8.0, r.center().y), vec2(13.0, 12.0)); + lp.add(egui::Shape::convex_polygon(vec![m.center_top(), m.right_bottom(), m.left_bottom()], t.warning, Stroke::NONE)); + lp.text(m.center() + vec2(0.0, 1.5), Align2::CENTER_CENTER, "!", Tokens::semibold(9.0), Color32::BLACK); + ui.interact(m.expand(2.0).intersect(list), egui::Id::new(("pproblem", id.0)), Sense::hover()).on_hover_text(why.as_str()); + app.auto.add(&format!("project.item.{}.problem", id.0), m, why); + } let name_rect = Rect::from_min_max(pos2(x0 + 28.0, r.min.y + 2.0), pos2(r.min.x + name_w + 2.0, r.max.y - 2.0)); // Inline edits (rename / comment). let cell_edit = |app: &mut EffectcraftApp, ui: &mut egui::Ui, actions: &mut Vec<(String, serde_json::Value)>, field: &str, cell: Rect| -> bool { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 5fe7697c..17e03e75 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -1045,6 +1045,30 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { app.auto.add("viewer.warpBanner", r, &text); } + // Footage that can't be decoded (no decoder for its codec / profile): say why on the layer + // instead of leaving a black picture. + let mut notices = 0; + for layer in comp.layers.iter().filter(|l| l.is_active_at(time) && l.switches.video) { + let effectcraft_engine::project::LayerSource::Footage { item } = &layer.source else { continue }; + let Some(effectcraft_engine::project::ItemKind::Footage(f)) = app.session.project.item(*item).map(|i| &i.kind) else { continue }; + let Some(why) = app.session.footage.problem(f) else { continue }; + if notices >= 3 { + break; + } + let (m, _) = l2c(&ectx, layer); + let (w, h) = effectcraft_engine::render::source_size(&app.session.project, layer); + let c = m.apply(effectcraft_engine::geom::vec2(w as f64 / 2.0, h as f64 / 2.0)); + let at = map.to_screen([c.x, c.y]); + let text = format!("{}: {}", layer.name, why); + let galley = painter.layout(text.clone(), Tokens::medium(12.0), Color32::WHITE, (map.area.width() - 40.0).clamp(80.0, 420.0)); + let r = Rect::from_center_size(at + vec2(0.0, notices as f32 * 4.0), galley.size() + vec2(24.0, 16.0)).intersect(map.area); + painter.rect_filled(r, 3.0, Color32::from_rgba_unmultiplied(0x4a, 0x1a, 0x14, 0xe6)); + painter.rect_stroke(r, 3.0, Stroke::new(1.0, t.warning), StrokeKind::Inside); + painter.galley(pos2(r.min.x + 12.0, r.center().y - galley.size().y / 2.0), galley, Color32::WHITE); + app.auto.add(&format!("viewer.footageProblem.{}", layer.id.0), r, &text); + notices += 1; + } + // Free transform box of a path (double-click a path with the Selection tool). let ft = ov::draw_free_transform(app, &ctx, &painter, &map, &paths); diff --git a/crates/ui-egui/tests/ui_unreadable_footage.rs b/crates/ui-egui/tests/ui_unreadable_footage.rs new file mode 100644 index 00000000..9b48b953 --- /dev/null +++ b/crates/ui-egui/tests/ui_unreadable_footage.rs @@ -0,0 +1,63 @@ +//! Footage whose codec the decoders lack says so: a warning marker in the Project panel and a +//! notice on the layer in the Composition viewer, instead of a black picture with no reason. + +use std::sync::Arc; + +use effectcraft_engine::Session; +use effectcraft_engine::project::{Footage, FootageKind, ItemId, ItemKind}; +use effectcraft_engine::render::FootageSource; +use effectcraft_render::Image; +use effectcraft_time::Tick; +use effectcraft_ui_egui::EffectcraftApp; +use egui_kittest::Harness; +use serde_json::json; + +const WHY: &str = "H.264 (High 4:2:2) isn't supported by the built-in decoder: chroma_format_idc 2 (only 4:2:0 is implemented)"; + +/// A source that decodes nothing and, for `broken` movies, says why. +struct Stub(bool); + +impl FootageSource for Stub { + fn frame(&self, _: ItemId, _: &Footage, _: Tick) -> Option> { + None + } + fn problem(&self, _: &Footage) -> Option { + self.0.then(|| WHY.to_string()) + } +} + +fn harness(broken: bool) -> (Harness<'static, EffectcraftApp>, ItemId) { + let mut s = Session { footage: Arc::new(Stub(broken)), ..Default::default() }; + s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "duration": 4})).unwrap(); + let footage = Footage { + kind: FootageKind::Video, + path: "clip.mp4".into(), + width: 64, + height: 36, + has_video: true, + duration: Tick::from_seconds_f64(2.0), + ..Default::default() + }; + let clip = std::sync::Arc::make_mut(&mut s.project).add_item("Clip", Default::default(), None, ItemKind::Footage(footage)); + s.execute("layer.addItem", json!({"item": clip.0})).unwrap(); + s.execute("project.select", json!({"items": []})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(4); + (h, clip) +} + +#[test] +fn undecodable_footage_is_marked_in_the_project_panel_and_the_viewer() { + let (h, clip) = harness(true); + let marker = h.state().auto.find(&format!("project.item.{}.problem", clip.0)).expect("a warning on the footage item").clone(); + assert!(marker.label.contains("4:2:2"), "{marker:?}"); + let notice = h.state().auto.query("viewer.footageProblem.").first().copied().expect("a notice in the viewer").clone(); + assert!(notice.label.contains(WHY), "{notice:?}"); +} + +#[test] +fn readable_footage_has_no_notice() { + let (h, clip) = harness(false); + assert!(h.state().auto.find(&format!("project.item.{}.problem", clip.0)).is_none()); + assert!(h.state().auto.query("viewer.footageProblem.").is_empty()); +}