From 3c42b81937778ee17868a2e34612a83befa2fc7d Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 28 Sep 2026 11:37:45 -0700 Subject: [PATCH 1/9] fix(vision mixer): keep a source's new shape when it changes size The GPU mixer's per-input glshader (fx_look, fx_take) takes its output size from the first entry of downstream's caps. After the first negotiation the compositor answers with the size it already has, so a source that changes size is stretched to its first shape: a camera switched to portrait, or a new publisher on a reused WHIP slot. The mixer never sees a caps change and never re-fits the aspect. Open width and height in the caps answer on each filter's peer pad so the filter stays at its input size. Co-Authored-By: Claude Opus 5.5 --- .../vision_mixer/builder/pipeline_gpu.rs | 8 +- .../blocks/builtin/vision_mixer/elements.rs | 50 ++++ .../tests/vision_mixer_source_resize_test.rs | 252 ++++++++++++++++++ 3 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 backend/tests/vision_mixer_source_resize_test.rs diff --git a/backend/src/blocks/builtin/vision_mixer/builder/pipeline_gpu.rs b/backend/src/blocks/builtin/vision_mixer/builder/pipeline_gpu.rs index ae2f314fa..e7a33bc9d 100644 --- a/backend/src/blocks/builtin/vision_mixer/builder/pipeline_gpu.rs +++ b/backend/src/blocks/builtin/vision_mixer/builder/pipeline_gpu.rs @@ -392,7 +392,9 @@ pub(super) fn build_gpu_pipeline( // pixelate, ...). Sits before the tee so the look follows the // source everywhere: PGM, PVW, thumbnails and PiPs. let fx_look_id = p.id(&format!("fx_look_{}", i)); - elems.push((fx_look_id.clone(), elements::make_glshader(&fx_look_id)?)); + let fx_look = elements::make_glshader(&fx_look_id)?; + elements::keep_input_size(&fx_look); + elems.push((fx_look_id.clone(), fx_look)); links.push(( ElementPadRef::pad(&cc_id, "src"), ElementPadRef::pad(&fx_look_id, "sink"), @@ -423,7 +425,9 @@ pub(super) fn build_gpu_pipeline( // shader transitions. Dist branch only — multiview thumbnails // and PVW stay clean during a wipe. let fx_take_id = p.id(&format!("fx_take_{}", i)); - elems.push((fx_take_id.clone(), elements::make_glshader(&fx_take_id)?)); + let fx_take = elements::make_glshader(&fx_take_id)?; + elements::keep_input_size(&fx_take); + elems.push((fx_take_id.clone(), fx_take)); links.push(( ElementPadRef::pad(&q_dist_id, "src"), ElementPadRef::pad(&fx_take_id, "sink"), diff --git a/backend/src/blocks/builtin/vision_mixer/elements.rs b/backend/src/blocks/builtin/vision_mixer/elements.rs index 8e5552039..16fa8c953 100644 --- a/backend/src/blocks/builtin/vision_mixer/elements.rs +++ b/backend/src/blocks/builtin/vision_mixer/elements.rs @@ -183,6 +183,56 @@ pub fn make_glshader(name: &str) -> Result { Ok(elem) } +/// Keep a per-input GL filter at the frame size it is given. +/// +/// `glshader` can scale, and takes its output size from the first entry of +/// downstream's caps. A running compositor answers with the size it already +/// has, so when a source changes size the filter stretches the new frames to +/// the old size: a camera switched to portrait, or a new publisher on a +/// reused WHIP slot, keeps the first source's shape, and the mixer never sees +/// a caps change to aspect-fit. Opening width and height in that answer keeps +/// the filter at its input size; the compositor pads take any size. +/// +/// The filter asks its peer pad directly (`gst_pad_query_caps` on the peer), +/// so the answer can only be rewritten on the peer, which is not known until +/// the filter's src pad is linked. +pub fn keep_input_size(filter: &gst::Element) { + let Some(src) = filter.static_pad("src") else { + return; + }; + src.connect_linked(|_src, peer| { + open_caps_answer_size(peer); + }); +} + +fn open_caps_answer_size(pad: &gst::Pad) { + // PULL: runs once the pad has answered a caps query. Never per buffer. + pad.add_probe( + gst::PadProbeType::QUERY_DOWNSTREAM | gst::PadProbeType::PULL, + |_pad, info| { + let Some(query) = info.query_mut() else { + return gst::PadProbeReturn::Ok; + }; + let gst::QueryViewMut::Caps(q) = query.view_mut() else { + return gst::PadProbeReturn::Ok; + }; + let Some(mut result) = q.result_owned() else { + return gst::PadProbeReturn::Ok; + }; + for s in result.make_mut().iter_mut() { + s.set("width", gst::IntRange::new(1, i32::MAX)); + s.set("height", gst::IntRange::new(1, i32::MAX)); + s.remove_field("pixel-aspect-ratio"); + } + if let Some(filter) = q.filter() { + result = filter.intersect_with_mode(&result, gst::CapsIntersectMode::First); + } + q.set_result(&result); + gst::PadProbeReturn::Ok + }, + ); +} + /// Create a simple GStreamer element by factory name. pub fn make_element(factory: &str, name: &str) -> Result { gst::ElementFactory::make(factory) diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs new file mode 100644 index 000000000..7a8f5535d --- /dev/null +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -0,0 +1,252 @@ +//! Regression test: a source that changes shape mid-stream must be +//! aspect-fitted in its new shape on the GPU (OpenGL) vision mixer. +//! +//! Builds a real vision mixer flow through `PipelineManager` with a white +//! 1280x720 source on input 0, switches that source to 720x1280, and +//! measures the white area of PGM. A portrait source on a 1280x720 canvas is +//! pillarboxed to about a third of the width; stretched to the first source's +//! size, PGM stays entirely white. + +use gstreamer::prelude::*; +use std::collections::HashMap; +use strom::blocks::BlockRegistry; +use strom::events::EventBroadcaster; +use strom::gst::pipeline::PipelineManager; +use strom_types::{Flow, PropertyValue as PV}; +use tempfile::NamedTempFile; + +const BLOCK_ID: &str = "vmresize"; + +/// Same probe as `vision_mixer_fx_test`: a trivial GL run must reach EOS. +/// Having the GL plugins installed is not enough on headless runners. +fn gl_environment_available() -> bool { + if gstreamer::ElementFactory::find("glvideomixerelement").is_none() + || gstreamer::ElementFactory::find("glshader").is_none() + || gstreamer::ElementFactory::find("gltestsrc").is_none() + { + return false; + } + let Ok(pipeline) = gstreamer::parse::launch( + "gltestsrc num-buffers=3 ! video/x-raw(memory:GLMemory),format=RGBA,width=64,height=64,framerate=30/1 ! fakesink sync=false", + ) else { + return false; + }; + let Ok(pipeline) = pipeline.downcast::() else { + return false; + }; + if pipeline.set_state(gstreamer::State::Playing).is_err() { + return false; + } + let bus = pipeline.bus().expect("pipeline has a bus"); + let ok = matches!( + bus.timed_pop_filtered( + gstreamer::ClockTime::from_seconds(20), + &[gstreamer::MessageType::Eos, gstreamer::MessageType::Error], + ), + Some(msg) if matches!(msg.view(), gstreamer::MessageView::Eos(_)) + ); + let _ = pipeline.set_state(gstreamer::State::Null); + ok +} + +/// Skip unless GL works; `STROM_REQUIRE_GL=1` (set on the macOS CI job) +/// turns the skip into a failure. +fn gl_available_or_required() -> bool { + if gl_environment_available() { + return true; + } + assert!( + strom_types::env::var_opt("STROM_REQUIRE_GL").is_none(), + "STROM_REQUIRE_GL is set but no GL context could be created — this platform \ + is supposed to render, so a skip here would hide a GL regression" + ); + eprintln!("SKIP: GL environment unavailable (no context or GL elements missing)"); + false +} + +fn elem(id: &str, ty: &str, props: Vec<(&str, PV)>) -> strom_types::Element { + strom_types::Element { + id: id.to_string(), + element_type: ty.to_string(), + properties: props.into_iter().map(|(k, v)| (k.to_string(), v)).collect(), + position: [0.0, 0.0].into(), + pad_properties: HashMap::new(), + } +} + +fn build_flow() -> Flow { + let mut flow = Flow::new("vm_source_resize"); + flow.blocks.push(strom_types::BlockInstance { + id: BLOCK_ID.to_string(), + block_definition_id: "builtin.vision_mixer".to_string(), + name: None, + properties: { + let mut p = HashMap::new(); + p.insert( + "compositor_preference".to_string(), + PV::String("gpu".into()), + ); + p.insert("num_inputs".to_string(), PV::UInt(2)); + p.insert("pgm_resolution".to_string(), PV::String("1280x720".into())); + p.insert( + "multiview_resolution".to_string(), + PV::String("640x360".into()), + ); + // Download PGM to system memory so the appsink can map pixels. + p.insert("gl_download".to_string(), PV::Bool(true)); + p + }, + position: strom_types::block::Position { x: 100.0, y: 100.0 }, + runtime_data: None, + computed_external_pads: None, + }); + flow.elements.push(elem( + "src0", + "videotestsrc", + vec![ + ("pattern", PV::String("white".into())), + ("is-live", PV::Bool(true)), + ], + )); + flow.elements.push(elem( + "caps0", + "capsfilter", + vec![( + "caps", + PV::String("video/x-raw,width=1280,height=720,framerate=30/1".into()), + )], + )); + flow.elements.push(elem( + "pgmsink", + "appsink", + vec![ + ("sync", PV::Bool(false)), + ("max-buffers", PV::UInt(1)), + ("drop", PV::Bool(true)), + ], + )); + for (from, to) in [ + ("src0:src", "caps0:sink"), + ("caps0:src", "vmresize:video_in_0"), + ("vmresize:pgm_out", "pgmsink:sink"), + ] { + flow.links.push(strom_types::Link { + from: from.to_string(), + to: to.to_string(), + }); + } + flow +} + +/// Fraction of PGM pixels that are white. +fn white_fraction(sample: &gstreamer::Sample) -> f64 { + let caps = sample.caps().expect("caps"); + let s = caps.structure(0).unwrap(); + let w = s.get::("width").unwrap() as usize; + let h = s.get::("height").unwrap() as usize; + let format = s.get::<&str>("format").unwrap().to_string(); + assert!( + matches!(format.as_str(), "RGBA" | "RGBx" | "BGRA" | "BGRx"), + "unexpected PGM format {}", + format + ); + let buffer = sample.buffer().expect("buffer"); + let map = buffer.map_readable().expect("map"); + let mut white = 0u64; + let mut total = 0u64; + // Flat colour fields: every 4th pixel gives the same fraction, faster. + for px in map.chunks_exact(4).take(w * h).step_by(4) { + if px[0] > 200 && px[1] > 200 && px[2] > 200 { + white += 1; + } + total += 1; + } + white as f64 / total.max(1) as f64 +} + +/// Pull PGM frames until one satisfies `done`, or fail after 30 s. +fn wait_for_pgm(appsink: &gstreamer_app::AppSink, what: &str, done: impl Fn(f64) -> bool) -> f64 { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + let mut last = None; + while std::time::Instant::now() < deadline { + if let Some(s) = appsink.try_pull_sample(gstreamer::ClockTime::from_mseconds(500)) { + let f = white_fraction(&s); + if done(f) { + return f; + } + last = Some(f); + } + } + panic!( + "PGM never showed {} within 30s, last white fraction {:?}", + what, last + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn source_switched_to_portrait_is_pillarboxed() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + + // The mixer re-fits a pad's rect from an idle callback on the default main + // context, so it only runs while a main loop does. The server has one. + let main_loop = gstreamer::glib::MainLoop::new(None, false); + let main_loop_thread = { + let ml = main_loop.clone(); + std::thread::spawn(move || ml.run()) + }; + + let flow = build_flow(); + let temp_file = NamedTempFile::new().unwrap(); + let registry = BlockRegistry::new(temp_file.path()); + let events = EventBroadcaster::with_capacity(10); + let mut manager = PipelineManager::new( + &flow, + events, + ®istry, + vec![], + "all".to_string(), + None, + std::env::temp_dir(), + std::sync::Arc::new(std::sync::Mutex::new(HashMap::new())), + ) + .expect("build GPU vision mixer pipeline"); + manager.start().expect("start GPU vision mixer pipeline"); + + let appsink = manager + .pipeline() + .by_name("pgmsink") + .expect("appsink in pipeline") + .downcast::() + .expect("appsink type"); + + // A 16:9 source fills the 16:9 canvas. + let before = wait_for_pgm(&appsink, "the landscape source filling PGM", |f| f > 0.95); + eprintln!("landscape: white={:.2}", before); + + // The source turns portrait. Aspect-fitted, 720x1280 on 1280x720 covers + // 405/1280 of the width: about 0.32 of the frame. + manager + .pipeline() + .by_name("caps0") + .expect("caps0") + .set_property( + "caps", + gstreamer::Caps::builder("video/x-raw") + .field("width", 720i32) + .field("height", 1280i32) + .field("framerate", gstreamer::Fraction::new(30, 1)) + .build(), + ); + let after = wait_for_pgm(&appsink, "the portrait source pillarboxed", |f| { + (0.25..0.40).contains(&f) + }); + eprintln!("portrait: white={:.2}", after); + + manager.stop().expect("stop"); + drop(manager); + main_loop.quit(); + main_loop_thread.join().expect("main loop thread"); +} From 7cb895f524223d555982159bcbaf5531b3c2d46c Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 28 Sep 2026 12:07:26 -0700 Subject: [PATCH 2/9] fix(vision mixer): re-fit an input only once its new caps are stored The caps probe queues a geometry refresh on the main loop, which runs on its own thread. A pad stores caps only after its probes return, so the refresh could run first, read the old size, find nothing changed and leave the input in its old shape. Requeue the refresh until the pad holds the caps that triggered it. Co-Authored-By: Claude Opus 5.5 --- .../blocks/builtin/vision_mixer/geometry.rs | 71 +++++++++++++------ 1 file changed, 49 insertions(+), 22 deletions(-) diff --git a/backend/src/blocks/builtin/vision_mixer/geometry.rs b/backend/src/blocks/builtin/vision_mixer/geometry.rs index 0439d4190..abca2975b 100644 --- a/backend/src/blocks/builtin/vision_mixer/geometry.rs +++ b/backend/src/blocks/builtin/vision_mixer/geometry.rs @@ -55,30 +55,24 @@ pub fn install_caps_probes( let block_id = block_id.to_string(); let mixer_weak = mixer.downgrade(); let mv_weak = mv_comp.downgrade(); - pad.add_probe(gst::PadProbeType::EVENT_DOWNSTREAM, move |_pad, info| { + pad.add_probe(gst::PadProbeType::EVENT_DOWNSTREAM, move |pad, info| { if let Some(gst::PadProbeData::Event(ev)) = &info.data { - if ev.type_() == gst::EventType::Caps { + if let gst::EventView::Caps(caps_event) = ev.view() { // Defer the refresh to the glib main loop instead of - // running it inline: sticky caps are stored on the pad - // only AFTER the probes return, and sibling branches - // negotiate concurrently on their own streaming threads — - // an inline refresh races both, and two probes of the - // same input firing simultaneously can each miss the - // other's caps, leaving that input's thumbnail stretched - // forever. Serialized on the main context, the last - // refresh always sees every negotiated pad. - let block_id = block_id.clone(); - let mixer_weak = mixer_weak.clone(); - let mv_weak = mv_weak.clone(); - gst::glib::idle_add_once(move || { - // Pipeline teardown in progress → nothing to refresh. - let (Some(mixer), Some(mv_comp)) = - (mixer_weak.upgrade(), mv_weak.upgrade()) - else { - return; - }; - refresh_geometry(&block_id, &mixer, &mv_comp); - }); + // running it inline: sibling branches negotiate + // concurrently on their own streaming threads, and two + // probes of the same input firing simultaneously can each + // miss the other's caps, leaving that input's thumbnail + // stretched forever. Serialized on the main context, the + // last refresh always sees every negotiated pad. + refresh_once_stored( + block_id.clone(), + mixer_weak.clone(), + mv_weak.clone(), + pad.downgrade(), + caps_event.caps_owned(), + MAX_CAPS_STORE_WAITS, + ); } } gst::PadProbeReturn::Ok @@ -90,6 +84,39 @@ pub fn install_caps_probes( ); } +/// Main-loop turns to wait for a pad to store caps it was sent. +const MAX_CAPS_STORE_WAITS: u32 = 1000; + +/// Refresh geometry once `pad` holds `caps`. +/// +/// A pad stores caps only after its probes return, and the main loop runs on +/// its own thread, so a refresh queued from the probe can run first. It would +/// read the old size, find no input changed, and leave the rect in the old +/// shape. Requeue until the caps are stored; if they never are (refused), +/// refresh anyway after `waits` turns. +fn refresh_once_stored( + block_id: String, + mixer_weak: gst::glib::WeakRef, + mv_weak: gst::glib::WeakRef, + pad_weak: gst::glib::WeakRef, + caps: gst::Caps, + waits: u32, +) { + gst::glib::idle_add_once(move || { + // Pipeline teardown in progress → nothing to refresh. + let (Some(mixer), Some(mv_comp), Some(pad)) = + (mixer_weak.upgrade(), mv_weak.upgrade(), pad_weak.upgrade()) + else { + return; + }; + if waits > 0 && pad.current_caps().as_ref() != Some(&caps) { + refresh_once_stored(block_id, mixer_weak, mv_weak, pad_weak, caps, waits - 1); + return; + } + refresh_geometry(&block_id, &mixer, &mv_comp); + }); +} + /// Read the UNCROPPED source `(width, height)` feeding a sink pad. On the /// CPU backend the pad's own caps are post-videocrop; this walks past it. fn pad_caps_dims(element: &gst::Element, pad_name: &str) -> Option<(i32, i32)> { From cfe0342c1cf85d06966362844c67a0663630671f Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 28 Sep 2026 15:16:24 -0700 Subject: [PATCH 3/9] fix(vision mixer): re-fit from stored caps and the configured PGM size Refresh an input's geometry from the pad's notify::caps, which fires after the pad stores new caps, instead of a caps event probe that fires before. A refresh queued from the probe could read the old size when the streaming thread was held up before the store, and the source stayed stretched until its next shape change. This replaces the bounded requeue. Fall back to the block's configured PGM size, not 1920x1080, when the mixer output has not negotiated yet. The first refresh runs before it has, so a source present at start was fitted to a 1080p canvas on any other PGM size and stayed misplaced. Co-Authored-By: Claude Opus 5.5 --- .../blocks/builtin/vision_mixer/geometry.rs | 96 +++---- .../tests/vision_mixer_source_resize_test.rs | 235 ++++++++++++------ 2 files changed, 194 insertions(+), 137 deletions(-) diff --git a/backend/src/blocks/builtin/vision_mixer/geometry.rs b/backend/src/blocks/builtin/vision_mixer/geometry.rs index abca2975b..bfe54f49c 100644 --- a/backend/src/blocks/builtin/vision_mixer/geometry.rs +++ b/backend/src/blocks/builtin/vision_mixer/geometry.rs @@ -23,12 +23,12 @@ use crate::gst::pipeline::effects::{ }; use crate::gst::underlay::UnderlayCtx; -/// Install CAPS event probes on every input video sink pad of the dist and -/// multiview compositors. Each caps arrival/change triggers a full geometry -/// refresh from the current overlay state. +/// Watch the caps of every input video sink pad of the dist and multiview +/// compositors. Each caps arrival/change triggers a full geometry refresh +/// from the current overlay state. /// /// Must run after linking (request pads exist). Elements are captured as -/// `WeakRef`s in the probe closures — pads own their probes, and a strong +/// `WeakRef`s in the handlers — pads own their handlers, and a strong /// element reference would create a cycle that leaks the pipeline. pub fn install_caps_probes( block_id: &str, @@ -55,68 +55,38 @@ pub fn install_caps_probes( let block_id = block_id.to_string(); let mixer_weak = mixer.downgrade(); let mv_weak = mv_comp.downgrade(); - pad.add_probe(gst::PadProbeType::EVENT_DOWNSTREAM, move |pad, info| { - if let Some(gst::PadProbeData::Event(ev)) = &info.data { - if let gst::EventView::Caps(caps_event) = ev.view() { - // Defer the refresh to the glib main loop instead of - // running it inline: sibling branches negotiate - // concurrently on their own streaming threads, and two - // probes of the same input firing simultaneously can each - // miss the other's caps, leaving that input's thumbnail - // stretched forever. Serialized on the main context, the - // last refresh always sees every negotiated pad. - refresh_once_stored( - block_id.clone(), - mixer_weak.clone(), - mv_weak.clone(), - pad.downgrade(), - caps_event.caps_owned(), - MAX_CAPS_STORE_WAITS, - ); - } + // `notify::caps` fires once the pad has stored the new caps; a caps + // EVENT probe fires before, so a refresh queued from one can read the + // old size and leave the rect in the old shape. + pad.connect_notify(Some("caps"), move |pad, _| { + // Caps are cleared on deactivation; nothing to fit. + if pad.current_caps().is_none() { + return; } - gst::PadProbeReturn::Ok + // Defer the refresh to the glib main loop instead of running it + // inline: sibling branches negotiate concurrently on their own + // streaming threads, and two notifies of the same input firing + // simultaneously can each miss the other's caps, leaving that + // input's thumbnail stretched forever. Serialized on the main + // context, the last refresh always sees every negotiated pad. + let block_id = block_id.clone(); + let mixer_weak = mixer_weak.clone(); + let mv_weak = mv_weak.clone(); + gst::glib::idle_add_once(move || { + // Pipeline teardown in progress → nothing to refresh. + let (Some(mixer), Some(mv_comp)) = (mixer_weak.upgrade(), mv_weak.upgrade()) else { + return; + }; + refresh_geometry(&block_id, &mixer, &mv_comp); + }); }); } debug!( - "Vision mixer {}: installed {} caps probes for reactive geometry", + "Vision mixer {}: watching caps on {} pads for reactive geometry", block_id, installed ); } -/// Main-loop turns to wait for a pad to store caps it was sent. -const MAX_CAPS_STORE_WAITS: u32 = 1000; - -/// Refresh geometry once `pad` holds `caps`. -/// -/// A pad stores caps only after its probes return, and the main loop runs on -/// its own thread, so a refresh queued from the probe can run first. It would -/// read the old size, find no input changed, and leave the rect in the old -/// shape. Requeue until the caps are stored; if they never are (refused), -/// refresh anyway after `waits` turns. -fn refresh_once_stored( - block_id: String, - mixer_weak: gst::glib::WeakRef, - mv_weak: gst::glib::WeakRef, - pad_weak: gst::glib::WeakRef, - caps: gst::Caps, - waits: u32, -) { - gst::glib::idle_add_once(move || { - // Pipeline teardown in progress → nothing to refresh. - let (Some(mixer), Some(mv_comp), Some(pad)) = - (mixer_weak.upgrade(), mv_weak.upgrade(), pad_weak.upgrade()) - else { - return; - }; - if waits > 0 && pad.current_caps().as_ref() != Some(&caps) { - refresh_once_stored(block_id, mixer_weak, mv_weak, pad_weak, caps, waits - 1); - return; - } - refresh_geometry(&block_id, &mixer, &mv_comp); - }); -} - /// Read the UNCROPPED source `(width, height)` feeding a sink pad. On the /// CPU backend the pad's own caps are post-videocrop; this walks past it. fn pad_caps_dims(element: &gst::Element, pad_name: &str) -> Option<(i32, i32)> { @@ -160,12 +130,10 @@ fn refresh_geometry(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element } } - // Dist canvas size from the mixer's negotiated output caps. - let (cw, ch) = pad_caps_dims_src(mixer).unwrap_or_else(|| { - strom_types::parse_resolution_string(vision_mixer::DEFAULT_PGM_RESOLUTION) - .map(|(w, h)| (w as i32, h as i32)) - .expect("DEFAULT_PGM_RESOLUTION must be valid") - }); + // Dist canvas size from the mixer's negotiated output caps, or the PGM + // size it is built to produce: the first refresh can run before the + // output has negotiated, and no later one runs unless an input changes. + let (cw, ch) = pad_caps_dims_src(mixer).unwrap_or((state.pgm_w as i32, state.pgm_h as i32)); let fallback = if ch > 0 { cw as f64 / ch as f64 } else { diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs index 7a8f5535d..b1204dde5 100644 --- a/backend/tests/vision_mixer_source_resize_test.rs +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -2,10 +2,9 @@ //! aspect-fitted in its new shape on the GPU (OpenGL) vision mixer. //! //! Builds a real vision mixer flow through `PipelineManager` with a white -//! 1280x720 source on input 0, switches that source to 720x1280, and -//! measures the white area of PGM. A portrait source on a 1280x720 canvas is -//! pillarboxed to about a third of the width; stretched to the first source's -//! size, PGM stays entirely white. +//! source on input 0 and measures the white area of PGM. A 720x1280 source on +//! a 1280x720 canvas is pillarboxed to about a third of the width; stretched, +//! PGM is entirely white. use gstreamer::prelude::*; use std::collections::HashMap; @@ -15,8 +14,6 @@ use strom::gst::pipeline::PipelineManager; use strom_types::{Flow, PropertyValue as PV}; use tempfile::NamedTempFile; -const BLOCK_ID: &str = "vmresize"; - /// Same probe as `vision_mixer_fx_test`: a trivial GL run must reach EOS. /// Having the GL plugins installed is not enough on headless runners. fn gl_environment_available() -> bool { @@ -74,10 +71,27 @@ fn elem(id: &str, ty: &str, props: Vec<(&str, PV)>) -> strom_types::Element { } } -fn build_flow() -> Flow { +const LANDSCAPE: (i32, i32) = (1280, 720); +const PORTRAIT: (i32, i32) = (720, 1280); + +fn source_caps((width, height): (i32, i32)) -> gstreamer::Caps { + gstreamer::Caps::builder("video/x-raw") + .field("width", width) + .field("height", height) + .field("framerate", gstreamer::Fraction::new(30, 1)) + .build() +} + +/// A 720x1280 source aspect-fitted on 1280x720 covers 405/1280 of the width. +fn pillarboxed(white: f64) -> bool { + (0.25..0.40).contains(&white) +} + +/// Mixer state is global per block id, so each test needs its own. +fn build_flow(block_id: &str, first_shape: (i32, i32)) -> Flow { let mut flow = Flow::new("vm_source_resize"); flow.blocks.push(strom_types::BlockInstance { - id: BLOCK_ID.to_string(), + id: block_id.to_string(), block_definition_id: "builtin.vision_mixer".to_string(), name: None, properties: { @@ -111,10 +125,7 @@ fn build_flow() -> Flow { flow.elements.push(elem( "caps0", "capsfilter", - vec![( - "caps", - PV::String("video/x-raw,width=1280,height=720,framerate=30/1".into()), - )], + vec![("caps", PV::String(source_caps(first_shape).to_string()))], )); flow.elements.push(elem( "pgmsink", @@ -126,14 +137,11 @@ fn build_flow() -> Flow { ], )); for (from, to) in [ - ("src0:src", "caps0:sink"), - ("caps0:src", "vmresize:video_in_0"), - ("vmresize:pgm_out", "pgmsink:sink"), + ("src0:src".to_string(), "caps0:sink".to_string()), + ("caps0:src".to_string(), format!("{block_id}:video_in_0")), + (format!("{block_id}:pgm_out"), "pgmsink:sink".to_string()), ] { - flow.links.push(strom_types::Link { - from: from.to_string(), - to: to.to_string(), - }); + flow.links.push(strom_types::Link { from, to }); } flow } @@ -183,70 +191,151 @@ fn wait_for_pgm(appsink: &gstreamer_app::AppSink, what: &str, done: impl Fn(f64) ); } +/// A running GPU vision mixer flow and the glib main loop its geometry +/// refresh needs: the mixer re-fits a pad's rect from an idle callback on the +/// default main context, which only runs while a main loop does. The server +/// has one. +struct Running { + manager: PipelineManager, + appsink: gstreamer_app::AppSink, + main_loop: gstreamer::glib::MainLoop, + main_loop_thread: std::thread::JoinHandle<()>, + _registry_file: NamedTempFile, +} + +impl Running { + fn start(flow: Flow) -> Running { + let main_loop = gstreamer::glib::MainLoop::new(None, false); + let main_loop_thread = { + let ml = main_loop.clone(); + std::thread::spawn(move || ml.run()) + }; + let registry_file = NamedTempFile::new().unwrap(); + let registry = BlockRegistry::new(registry_file.path()); + let events = EventBroadcaster::with_capacity(10); + let mut manager = PipelineManager::new( + &flow, + events, + ®istry, + vec![], + "all".to_string(), + None, + std::env::temp_dir(), + std::sync::Arc::new(std::sync::Mutex::new(HashMap::new())), + ) + .expect("build GPU vision mixer pipeline"); + manager.start().expect("start GPU vision mixer pipeline"); + let appsink = manager + .pipeline() + .by_name("pgmsink") + .expect("appsink in pipeline") + .downcast::() + .expect("appsink type"); + Running { + manager, + appsink, + main_loop, + main_loop_thread, + _registry_file: registry_file, + } + } + + fn element(&self, name: &str) -> gstreamer::Element { + self.manager + .pipeline() + .by_name(name) + .unwrap_or_else(|| panic!("{name} in pipeline")) + } + + fn set_source_shape(&self, shape: (i32, i32)) { + self.element("caps0") + .set_property("caps", source_caps(shape)); + } + + fn stop(mut self) { + self.manager.stop().expect("stop"); + drop(self.manager); + self.main_loop.quit(); + self.main_loop_thread.join().expect("main loop thread"); + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn source_switched_to_portrait_is_pillarboxed() { gstreamer::init().unwrap(); if !gl_available_or_required() { return; } - - // The mixer re-fits a pad's rect from an idle callback on the default main - // context, so it only runs while a main loop does. The server has one. - let main_loop = gstreamer::glib::MainLoop::new(None, false); - let main_loop_thread = { - let ml = main_loop.clone(); - std::thread::spawn(move || ml.run()) - }; - - let flow = build_flow(); - let temp_file = NamedTempFile::new().unwrap(); - let registry = BlockRegistry::new(temp_file.path()); - let events = EventBroadcaster::with_capacity(10); - let mut manager = PipelineManager::new( - &flow, - events, - ®istry, - vec![], - "all".to_string(), - None, - std::env::temp_dir(), - std::sync::Arc::new(std::sync::Mutex::new(HashMap::new())), - ) - .expect("build GPU vision mixer pipeline"); - manager.start().expect("start GPU vision mixer pipeline"); - - let appsink = manager - .pipeline() - .by_name("pgmsink") - .expect("appsink in pipeline") - .downcast::() - .expect("appsink type"); + let running = Running::start(build_flow("vmresize", LANDSCAPE)); // A 16:9 source fills the 16:9 canvas. - let before = wait_for_pgm(&appsink, "the landscape source filling PGM", |f| f > 0.95); + let before = wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); eprintln!("landscape: white={:.2}", before); - // The source turns portrait. Aspect-fitted, 720x1280 on 1280x720 covers - // 405/1280 of the width: about 0.32 of the frame. - manager - .pipeline() - .by_name("caps0") - .expect("caps0") - .set_property( - "caps", - gstreamer::Caps::builder("video/x-raw") - .field("width", 720i32) - .field("height", 1280i32) - .field("framerate", gstreamer::Fraction::new(30, 1)) - .build(), - ); - let after = wait_for_pgm(&appsink, "the portrait source pillarboxed", |f| { - (0.25..0.40).contains(&f) - }); + running.set_source_shape(PORTRAIT); + let after = wait_for_pgm( + &running.appsink, + "the portrait source pillarboxed", + pillarboxed, + ); eprintln!("portrait: white={:.2}", after); - manager.stop().expect("stop"); - drop(manager); - main_loop.quit(); - main_loop_thread.join().expect("main loop thread"); + running.stop(); +} + +/// The first fit runs before the mixer's output has negotiated its size, so +/// it must use the configured PGM size, not a default canvas. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn source_portrait_from_the_start_is_pillarboxed() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let running = Running::start(build_flow("vmresize_first", PORTRAIT)); + let white = wait_for_pgm( + &running.appsink, + "the portrait source pillarboxed", + pillarboxed, + ); + eprintln!("portrait from the start: white={:.2}", white); + running.stop(); +} + +/// The mixer pad stores new caps only after its event probes return. A +/// streaming thread held up there must not leave the old shape in place. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn source_switch_fits_when_caps_are_stored_late() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmresize_late"; + let running = Running::start(build_flow(block_id, LANDSCAPE)); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + + running + .element(&format!("{block_id}:mixer")) + .static_pad("sink_0") + .expect("mixer sink_0") + .add_probe(gstreamer::PadProbeType::EVENT_DOWNSTREAM, |_, info| { + if let Some(gstreamer::PadProbeData::Event(ev)) = &info.data { + if ev.type_() == gstreamer::EventType::Caps { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + } + gstreamer::PadProbeReturn::Ok + }); + + running.set_source_shape(PORTRAIT); + let white = wait_for_pgm( + &running.appsink, + "the portrait source pillarboxed", + pillarboxed, + ); + eprintln!("portrait, caps stored late: white={:.2}", white); + running.stop(); } From b2a4262a101284e420f43b0d6eb8acd55b083083 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 28 Sep 2026 19:44:25 -0700 Subject: [PATCH 4/9] fix(vision mixer): hold a re-fit until a take or FTB is over Re-fitting an input clears the pad's control bindings, so a source that changed size during a fade or wipe ended the transition as a cut. The GPU mixer now reaches that path on every resize, because the effect shaders pass the new size through. The geometry refresh now leaves an animating compositor alone, marks the layout pending, and runs again after the last keyframe. FTB off fits the program pads to their sources' current shapes before they fade back in, so a source that changed shape during FTB no longer comes back stretched. Co-Authored-By: Claude Opus 5.5 --- .../blocks/builtin/vision_mixer/geometry.rs | 79 +++++++- .../blocks/builtin/vision_mixer/overlay.rs | 4 + backend/src/gst/pipeline/effects/mixer_ops.rs | 35 +++- .../tests/vision_mixer_source_resize_test.rs | 180 +++++++++++++++++- 4 files changed, 280 insertions(+), 18 deletions(-) diff --git a/backend/src/blocks/builtin/vision_mixer/geometry.rs b/backend/src/blocks/builtin/vision_mixer/geometry.rs index bfe54f49c..1fc026069 100644 --- a/backend/src/blocks/builtin/vision_mixer/geometry.rs +++ b/backend/src/blocks/builtin/vision_mixer/geometry.rs @@ -14,6 +14,10 @@ use gstreamer as gst; use gstreamer::prelude::*; +use gstreamer_controller::prelude::*; +use gstreamer_controller::InterpolationControlSource; +use std::sync::atomic::Ordering; +use std::time::Duration; use strom_types::vision_mixer::{self, aspect_fit_rect, SourceAspects}; use tracing::debug; @@ -119,10 +123,28 @@ fn refresh_geometry(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element // refreshing on each of those events would clear control bindings and // kill the very animation producing them. Source dims are read upstream // of videocrop, so crop-induced caps events never count as changes. - if !state.update_input_dims(&dims) { + let dims_changed = state.update_input_dims(&dims); + if !dims_changed && !state.layout_refit_pending.load(Ordering::Relaxed) { return; } + // A take, FTB or DSK fade animates pad properties through control + // bindings, and re-applying a layout wipes them: the animation would + // end as a cut. Leave an animating compositor alone and re-fit it once + // the animation is over. FTB off re-fits the dist mixer itself. + let ftb = state.ftb_active.load(Ordering::Relaxed); + let dist_animation = animation_remaining(mixer); + let mv_animation = animation_remaining(mv_comp); + let dist_busy = ftb || dist_animation.is_some(); + let mv_busy = mv_animation.is_some(); + state + .layout_refit_pending + .store(dist_busy || mv_busy, Ordering::Relaxed); + let retry = [dist_animation, mv_animation].into_iter().flatten().max(); + if let Some(wait) = retry { + refit_after(block_id, mixer, mv_comp, wait + REFIT_MARGIN); + } + let mut aspects = SourceAspects::new(); for (i, d) in dims.iter().enumerate() { if let Some((w, h)) = d { @@ -150,8 +172,8 @@ fn refresh_geometry(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element scale: state.layout.pvw_rect.w / pgm_w, }); - // --- Dist compositor (PGM) — skip while FTB drives the alphas. --- - if !state.ftb_active.load(std::sync::atomic::Ordering::Relaxed) { + // --- Dist compositor (PGM). --- + if !dist_busy { if let Some(p) = state.pgm_pip() { apply_pip_layout_to_region( mixer, @@ -196,6 +218,10 @@ fn refresh_geometry(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element pad.set_property("height", h); } + if mv_busy { + return; + } + // --- Multiview PVW big region. --- let r = &state.layout.pvw_rect; let pvw_region = (r.x as i32, r.y as i32, r.w as i32, r.h as i32); @@ -261,6 +287,53 @@ fn refresh_geometry(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element ); } +/// Pad properties that takes, FTB and layout code animate. +const ANIMATED_PAD_PROPS: [&str; 6] = ["alpha", "xpos", "ypos", "width", "height", "zorder"]; + +/// Slack after an animation's last keyframe before re-fitting. +const REFIT_MARGIN: Duration = Duration::from_millis(50); + +/// How long until the last keyframe programmed on any of `compositor`'s sink +/// pads, or `None` when none lies ahead. Keyframes are in the compositor's +/// stream time, which is what its position query reports. +fn animation_remaining(compositor: &gst::Element) -> Option { + let now = compositor.query_position::()?; + compositor + .sink_pads() + .iter() + .flat_map(|pad| { + ANIMATED_PAD_PROPS + .iter() + .filter_map(move |prop| last_keyframe(pad, prop)) + }) + .max() + .filter(|end| *end > now) + .map(|end| Duration::from_nanos((end - now).nseconds())) +} + +fn last_keyframe(pad: &gst::Pad, prop: &str) -> Option { + let cs = pad + .control_binding(prop)? + .property::>("control-source")? + .downcast::() + .ok()?; + cs.list_control_points().last().map(|tv| tv.timestamp()) +} + +/// Run a geometry refresh after `wait`, on the main loop. +fn refit_after(block_id: &str, mixer: &gst::Element, mv_comp: &gst::Element, wait: Duration) { + let block_id = block_id.to_string(); + let mixer_weak = mixer.downgrade(); + let mv_weak = mv_comp.downgrade(); + gst::glib::timeout_add_once(wait, move || { + // Pipeline teardown in progress → nothing to refresh. + let (Some(mixer), Some(mv_comp)) = (mixer_weak.upgrade(), mv_weak.upgrade()) else { + return; + }; + refresh_geometry(&block_id, &mixer, &mv_comp); + }); +} + /// Read `(width, height)` from an element's `src` pad caps. fn pad_caps_dims_src(element: &gst::Element) -> Option<(i32, i32)> { let pad = element.static_pad("src")?; diff --git a/backend/src/blocks/builtin/vision_mixer/overlay.rs b/backend/src/blocks/builtin/vision_mixer/overlay.rs index db1bf6306..d96a859ad 100644 --- a/backend/src/blocks/builtin/vision_mixer/overlay.rs +++ b/backend/src/blocks/builtin/vision_mixer/overlay.rs @@ -82,6 +82,9 @@ pub struct VisionMixerOverlayState { pub num_inputs: usize, /// Whether Fade to Black is active. pub ftb_active: AtomicBool, + /// An input changed size while a take or FTB was animating the layout, + /// so the layout still has to be re-fitted once the animation is over. + pub layout_refit_pending: AtomicBool, /// Multiview overlay alpha (0.0–1.0), stored as f64 bits. overlay_alpha: AtomicU64, /// DSK enabled states (one per DSK input, max 4). @@ -199,6 +202,7 @@ impl VisionMixerOverlayState { num_pips: pip.num_pips, num_inputs, ftb_active: AtomicBool::new(false), + layout_refit_pending: AtomicBool::new(false), overlay_alpha: AtomicU64::new(1.0f64.to_bits()), dsk_enabled: (0..num_dsk_inputs) .map(|_| AtomicBool::new(false)) diff --git a/backend/src/gst/pipeline/effects/mixer_ops.rs b/backend/src/gst/pipeline/effects/mixer_ops.rs index ed8390c13..0454bff53 100644 --- a/backend/src/gst/pipeline/effects/mixer_ops.rs +++ b/backend/src/gst/pipeline/effects/mixer_ops.rs @@ -359,7 +359,7 @@ impl PipelineManager { base, scale: cw as f64 / state.pgm_w.max(1) as f64, }); - pads_for_source( + let targets = pads_for_source( &state, state.pgm_pip(), pgm, @@ -370,12 +370,33 @@ impl PipelineManager { src_aspect, &self.vision_mixer_source_aspects(block_instance_id, state.num_inputs), dist_underlay, - ) - .into_iter() - // Restore bordered zone sources' underlay pads along with their - // content pads. - .flat_map(|t| std::iter::once(t.pad_idx).chain(t.underlay.map(|u| u.pad_idx))) - .collect() + ); + // Fit the pads to their sources' current shapes before they fade + // back in: a source that changed shape during FTB was not re-fitted. + for (pad_idx, (x, y, w, h)) in targets.iter().flat_map(|t| { + std::iter::once((t.pad_idx, (t.x, t.y, t.w, t.h))).chain( + t.underlay + .as_ref() + .map(|u| (u.pad_idx, (u.x, u.y, u.w, u.h))), + ) + }) { + if let Some(pad) = find_pad(mixer, &format!("sink_{}", pad_idx)) { + crate::gst::control_bindings::wipe_control_bindings( + pad.upcast_ref(), + &["xpos", "ypos", "width", "height"], + ); + pad.set_property("xpos", x); + pad.set_property("ypos", y); + pad.set_property("width", w); + pad.set_property("height", h); + } + } + targets + .into_iter() + // Restore bordered zone sources' underlay pads along with their + // content pads. + .flat_map(|t| std::iter::once(t.pad_idx).chain(t.underlay.map(|u| u.pad_idx))) + .collect() }; // Use mixer position for stream-time (same as transitions). diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs index b1204dde5..7bbbb0da2 100644 --- a/backend/tests/vision_mixer_source_resize_test.rs +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -127,6 +127,20 @@ fn build_flow(block_id: &str, first_shape: (i32, i32)) -> Flow { "capsfilter", vec![("caps", PV::String(source_caps(first_shape).to_string()))], )); + // Input 1, on PVW: a red source for takes. + flow.elements.push(elem( + "src1", + "videotestsrc", + vec![ + ("pattern", PV::String("red".into())), + ("is-live", PV::Bool(true)), + ], + )); + flow.elements.push(elem( + "caps1", + "capsfilter", + vec![("caps", PV::String(source_caps(LANDSCAPE).to_string()))], + )); flow.elements.push(elem( "pgmsink", "appsink", @@ -139,6 +153,8 @@ fn build_flow(block_id: &str, first_shape: (i32, i32)) -> Flow { for (from, to) in [ ("src0:src".to_string(), "caps0:sink".to_string()), ("caps0:src".to_string(), format!("{block_id}:video_in_0")), + ("src1:src".to_string(), "caps1:sink".to_string()), + ("caps1:src".to_string(), format!("{block_id}:video_in_1")), (format!("{block_id}:pgm_out"), "pgmsink:sink".to_string()), ] { flow.links.push(strom_types::Link { from, to }); @@ -148,6 +164,26 @@ fn build_flow(block_id: &str, first_shape: (i32, i32)) -> Flow { /// Fraction of PGM pixels that are white. fn white_fraction(sample: &gstreamer::Sample) -> f64 { + pixel_fraction(sample, |px| px[0] > 200 && px[1] > 200 && px[2] > 200) +} + +/// Fraction of PGM pixels that are black. +fn black_fraction(sample: &gstreamer::Sample) -> f64 { + pixel_fraction(sample, |px| px[0] < 40 && px[1] < 40 && px[2] < 40) +} + +/// Mean green level of PGM: 255 for white, 0 for red or black. +fn mean_green(sample: &gstreamer::Sample) -> f64 { + let buffer = sample.buffer().expect("buffer"); + let map = buffer.map_readable().expect("map"); + let (sum, n) = map + .chunks_exact(4) + .step_by(4) + .fold((0u64, 0u64), |(sum, n), px| (sum + px[1] as u64, n + 1)); + sum as f64 / n.max(1) as f64 +} + +fn pixel_fraction(sample: &gstreamer::Sample, matches: impl Fn(&[u8]) -> bool) -> f64 { let caps = sample.caps().expect("caps"); let s = caps.structure(0).unwrap(); let w = s.get::("width").unwrap() as usize; @@ -160,25 +196,35 @@ fn white_fraction(sample: &gstreamer::Sample) -> f64 { ); let buffer = sample.buffer().expect("buffer"); let map = buffer.map_readable().expect("map"); - let mut white = 0u64; + let mut hits = 0u64; let mut total = 0u64; // Flat colour fields: every 4th pixel gives the same fraction, faster. for px in map.chunks_exact(4).take(w * h).step_by(4) { - if px[0] > 200 && px[1] > 200 && px[2] > 200 { - white += 1; + if matches(px) { + hits += 1; } total += 1; } - white as f64 / total.max(1) as f64 + hits as f64 / total.max(1) as f64 } -/// Pull PGM frames until one satisfies `done`, or fail after 30 s. +/// Pull PGM frames until one's white fraction satisfies `done`, or fail +/// after 30 s. fn wait_for_pgm(appsink: &gstreamer_app::AppSink, what: &str, done: impl Fn(f64) -> bool) -> f64 { + wait_for_pgm_by(appsink, what, white_fraction, done) +} + +fn wait_for_pgm_by( + appsink: &gstreamer_app::AppSink, + what: &str, + measure: impl Fn(&gstreamer::Sample) -> f64, + done: impl Fn(f64) -> bool, +) -> f64 { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); let mut last = None; while std::time::Instant::now() < deadline { if let Some(s) = appsink.try_pull_sample(gstreamer::ClockTime::from_mseconds(500)) { - let f = white_fraction(&s); + let f = measure(&s); if done(f) { return f; } @@ -186,7 +232,7 @@ fn wait_for_pgm(appsink: &gstreamer_app::AppSink, what: &str, done: impl Fn(f64) } } panic!( - "PGM never showed {} within 30s, last white fraction {:?}", + "PGM never showed {} within 30s, last measured {:?}", what, last ); } @@ -248,7 +294,11 @@ impl Running { } fn set_source_shape(&self, shape: (i32, i32)) { - self.element("caps0") + self.set_input_shape(0, shape); + } + + fn set_input_shape(&self, input: usize, shape: (i32, i32)) { + self.element(&format!("caps{input}")) .set_property("caps", source_caps(shape)); } @@ -339,3 +389,117 @@ async fn source_switch_fits_when_caps_are_stored_late() { eprintln!("portrait, caps stored late: white={:.2}", white); running.stop(); } + +/// A source that changes shape during a fade must not cut the fade short, +/// and must be fitted in its new shape once the fade is over. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn source_resized_during_a_fade_is_fitted_after_it() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmresize_fade"; + let running = Running::start(build_flow(block_id, LANDSCAPE)); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + + // Fade from white (input 0) to red (input 1) over 2 s. The red source + // turns 4:3 at 600 ms. + running + .manager + .trigger_transition(block_id, 0, 1, "fade", 2000) + .expect("fade"); + running + .manager + .update_vision_mixer_after_take(block_id, Some(1), Some(0), 2) + .expect("after take"); + let start = std::time::Instant::now(); + let mut resized = false; + let mut mid_fade = Vec::new(); + while start.elapsed() < std::time::Duration::from_millis(1000) { + if !resized && start.elapsed() > std::time::Duration::from_millis(600) { + running.set_input_shape(1, (960, 720)); + resized = true; + } + if let Some(s) = running + .appsink + .try_pull_sample(gstreamer::ClockTime::from_mseconds(200)) + { + if start.elapsed() > std::time::Duration::from_millis(750) { + mid_fade.push(mean_green(&s)); + } + } + } + // Still blending: some white left. A cut shows red and black only. + assert!( + !mid_fade.is_empty() && mid_fade.iter().all(|g| *g > 30.0), + "the fade was cut short after the resize, mean green {:?}", + mid_fade + ); + + // 4:3 red on 16:9: pillarbox bars cover a quarter of PGM. + let bars = wait_for_pgm_by( + &running.appsink, + "the 4:3 source pillarboxed after the fade", + black_fraction, + |f| (0.20..0.30).contains(&f), + ); + eprintln!("after the fade: black={:.2}", bars); + running.stop(); +} + +/// A source that changes shape during Fade to Black comes back fitted. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn source_switched_during_ftb_is_pillarboxed_after_it() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmresize_ftb"; + let running = Running::start(build_flow(block_id, LANDSCAPE)); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + + running + .manager + .fade_to_black(block_id, 200) + .expect("FTB on"); + wait_for_pgm(&running.appsink, "black", |f| f < 0.05); + running.set_source_shape(PORTRAIT); + std::thread::sleep(std::time::Duration::from_millis(500)); + running + .manager + .fade_to_black(block_id, 1000) + .expect("FTB off"); + + // The source is grey while it fades back in, so count lit pixels: every + // frame of the fade-in must already show it pillarboxed, not stretched. + let start = std::time::Instant::now(); + let mut fade_in = Vec::new(); + while start.elapsed() < std::time::Duration::from_millis(900) { + if let Some(s) = running + .appsink + .try_pull_sample(gstreamer::ClockTime::from_mseconds(200)) + { + let lit = 1.0 - black_fraction(&s); + if lit > 0.05 { + fade_in.push(lit); + } + } + } + assert!( + !fade_in.is_empty() && fade_in.iter().all(|lit| *lit < 0.45), + "the source faded in stretched, lit fractions {:?}", + fade_in + ); + + let white = wait_for_pgm( + &running.appsink, + "the portrait source pillarboxed", + pillarboxed, + ); + eprintln!("after FTB: white={:.2}", white); + running.stop(); +} From ad05cffc34efde18f3dfef87bc385f7ce4d2f60a Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 28 Sep 2026 20:40:01 -0700 Subject: [PATCH 5/9] fix(vision mixer): let Fade to Black play out on a slow mixer FTB wiped its alpha keyframes on a wall-clock timer, duration + 100 ms. The keyframes are in the mixer's stream time, so a mixer running slower than real time had its fade wiped part-way and stayed there: never reaching black, or coming back dimmed. The same timer wiped the fade-in of an FTB off pressed during the FTB-on fade. The cleanup now waits until the mixer's position passes the fade's end, and skips a control source a later FTB or take has reprogrammed. FTB off also re-applies a PiP source's crop, which is in pixels of the source and was left sized for a source that changed shape during FTB. The fade and FTB tests now time their checks by the mixer's clock instead of the wall clock. Co-Authored-By: Claude Opus 5.5 --- backend/src/gst/pipeline/effects/mixer_ops.rs | 72 ++++- .../tests/vision_mixer_source_resize_test.rs | 280 +++++++++++++++--- 2 files changed, 302 insertions(+), 50 deletions(-) diff --git a/backend/src/gst/pipeline/effects/mixer_ops.rs b/backend/src/gst/pipeline/effects/mixer_ops.rs index 0454bff53..5b13ed849 100644 --- a/backend/src/gst/pipeline/effects/mixer_ops.rs +++ b/backend/src/gst/pipeline/effects/mixer_ops.rs @@ -373,11 +373,14 @@ impl PipelineManager { ); // Fit the pads to their sources' current shapes before they fade // back in: a source that changed shape during FTB was not re-fitted. - for (pad_idx, (x, y, w, h)) in targets.iter().flat_map(|t| { - std::iter::once((t.pad_idx, (t.x, t.y, t.w, t.h))).chain( + // Content pads (dist pad index = input index) also get their crop + // again, which is in pixels of the source's size. + let transforms = state.pgm_pip().map(|p| state.pip_transforms(p)); + for (pad_idx, (x, y, w, h), content) in targets.iter().flat_map(|t| { + std::iter::once((t.pad_idx, (t.x, t.y, t.w, t.h), true)).chain( t.underlay .as_ref() - .map(|u| (u.pad_idx, (u.x, u.y, u.w, u.h))), + .map(|u| (u.pad_idx, (u.x, u.y, u.w, u.h), false)), ) }) { if let Some(pad) = find_pad(mixer, &format!("sink_{}", pad_idx)) { @@ -389,6 +392,13 @@ impl PipelineManager { pad.set_property("ypos", y); pad.set_property("width", w); pad.set_property("height", h); + if content { + let crop = transforms + .as_ref() + .and_then(|t| t.get(&pad_idx).copied()) + .unwrap_or_default(); + set_pad_crop(&pad, &crop); + } } } targets @@ -472,19 +482,11 @@ impl PipelineManager { // Once the fade completes, neutralize the alpha bindings (keyframe // wipe) so later direct alpha writes (DSK toggles, takes) stick. if !control_sources.is_empty() { - let cleanup_mixer = mixer.clone(); - let cleanup_duration = duration_ms + 100; // small margin - gst::glib::timeout_add_once( - std::time::Duration::from_millis(cleanup_duration), - move || { - for pad in cleanup_mixer.sink_pads() { - crate::gst::control_bindings::wipe_control_binding( - pad.upcast_ref(), - "alpha", - ); - } - drop(control_sources); - }, + wipe_ftb_fade_once_played( + mixer.downgrade(), + end_time, + control_sources, + std::time::Duration::from_millis(duration_ms + 100), ); } @@ -1042,3 +1044,41 @@ impl PipelineManager { Ok(()) } } + +/// Wipe an FTB fade's alpha keyframes once the mixer has played past `end`, +/// so later direct alpha writes (DSK toggles, takes) stick. Keyframes are in +/// the mixer's stream time, which lags the wall clock whenever the mixer runs +/// slower than real time, so `wait` is only when to look first. A control +/// source whose last keyframe is no longer `end` was reprogrammed by a later +/// FTB or take, and is left alone. +fn wipe_ftb_fade_once_played( + mixer: gst::glib::WeakRef, + end: gst::ClockTime, + sources: Vec, + wait: std::time::Duration, +) { + use gstreamer_controller::prelude::*; + gst::glib::timeout_add_once(wait, move || { + // Pipeline teardown in progress → nothing to clean up. + let Some(m) = mixer.upgrade() else { + return; + }; + if let Some(pos) = m.query_position::() { + if pos < end { + let behind = std::time::Duration::from_nanos((end - pos).nseconds()); + wipe_ftb_fade_once_played( + mixer, + end, + sources, + behind + std::time::Duration::from_millis(50), + ); + return; + } + } + for cs in &sources { + if cs.list_control_points().last().map(|p| p.timestamp()) == Some(end) { + cs.unset_all(); + } + } + }); +} diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs index 7bbbb0da2..8bf9c127d 100644 --- a/backend/tests/vision_mixer_source_resize_test.rs +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -1,5 +1,6 @@ -//! Regression test: a source that changes shape mid-stream must be -//! aspect-fitted in its new shape on the GPU (OpenGL) vision mixer. +//! Regression tests: a source that changes shape mid-stream must be +//! aspect-fitted in its new shape on the GPU (OpenGL) vision mixer, also +//! across takes and Fade to Black, and Fade to Black must play out in full. //! //! Builds a real vision mixer flow through `PipelineManager` with a white //! source on input 0 and measures the white area of PGM. A 720x1280 source on @@ -302,6 +303,42 @@ impl Running { .set_property("caps", source_caps(shape)); } + /// The dist mixer's position: the clock its keyframes and PGM frame + /// times are in. + fn position(&self, block_id: &str) -> gstreamer::ClockTime { + self.element(&format!("{block_id}:mixer")) + .query_position::() + .expect("mixer position") + } + + /// Wait until the dist mixer's pad for `input` has stored caps `width` + /// wide, and return the mixer's position then. + fn wait_for_input_width( + &self, + block_id: &str, + input: usize, + width: i32, + ) -> gstreamer::ClockTime { + let pad = self + .element(&format!("{block_id}:mixer")) + .static_pad(&format!("sink_{input}")) + .expect("mixer sink pad"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + loop { + let stored = pad + .current_caps() + .and_then(|c| c.structure(0).and_then(|s| s.get::("width").ok())); + if stored == Some(width) { + return self.position(block_id); + } + assert!( + std::time::Instant::now() < deadline, + "input {input} never reached the mixer {width} wide, caps width {stored:?}" + ); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + fn stop(mut self) { self.manager.stop().expect("stop"); drop(self.manager); @@ -404,38 +441,50 @@ async fn source_resized_during_a_fade_is_fitted_after_it() { f > 0.95 }); - // Fade from white (input 0) to red (input 1) over 2 s. The red source - // turns 4:3 at 600 ms. + // Fade from white (input 0) to red (input 1) over 4 s. The red source + // turns 4:3 about a second in, by the mixer's clock: a throttled mixer + // catches up in bursts and runs ahead of the wall clock. + let start = running.position(block_id); running .manager - .trigger_transition(block_id, 0, 1, "fade", 2000) + .trigger_transition(block_id, 0, 1, "fade", 4000) .expect("fade"); running .manager .update_vision_mixer_after_take(block_id, Some(1), Some(0), 2) .expect("after take"); - let start = std::time::Instant::now(); - let mut resized = false; - let mut mid_fade = Vec::new(); - while start.elapsed() < std::time::Duration::from_millis(1000) { - if !resized && start.elapsed() > std::time::Duration::from_millis(600) { - running.set_input_shape(1, (960, 720)); - resized = true; + while running.position(block_id) < start + gstreamer::ClockTime::from_mseconds(800) { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + running.set_input_shape(1, (960, 720)); + let resized = running.wait_for_input_width(block_id, 1, 960); + + // A cut drops green (white to red) from over 200 to 0 between two + // frames; the 4 s fade moves it a few levels a frame. Frames are + // compared with each other, not placed on the wall clock, so the check + // holds however far the mixer drifts from real time. + let mut greens = Vec::new(); + while let Some(s) = running + .appsink + .try_pull_sample(gstreamer::ClockTime::from_seconds(5)) + { + if s.buffer().and_then(|b| b.pts()).expect("PGM pts") <= resized { + continue; } - if let Some(s) = running - .appsink - .try_pull_sample(gstreamer::ClockTime::from_mseconds(200)) - { - if start.elapsed() > std::time::Duration::from_millis(750) { - mid_fade.push(mean_green(&s)); - } + let green = mean_green(&s); + greens.push(green); + if green < 20.0 { + break; } } - // Still blending: some white left. A cut shows red and black only. + let largest_step = greens + .windows(2) + .map(|w| w[0] - w[1]) + .fold(0.0f64, f64::max); assert!( - !mid_fade.is_empty() && mid_fade.iter().all(|g| *g > 30.0), + greens.len() > 2 && greens[0] > 60.0 && largest_step < 40.0, "the fade was cut short after the resize, mean green {:?}", - mid_fade + greens ); // 4:3 red on 16:9: pillarbox bars cover a quarter of PGM. @@ -466,9 +515,10 @@ async fn source_switched_during_ftb_is_pillarboxed_after_it() { .manager .fade_to_black(block_id, 200) .expect("FTB on"); - wait_for_pgm(&running.appsink, "black", |f| f < 0.05); + wait_for_pgm_by(&running.appsink, "black", black_fraction, |f| f > 0.95); running.set_source_shape(PORTRAIT); - std::thread::sleep(std::time::Duration::from_millis(500)); + running.wait_for_input_width(block_id, 0, PORTRAIT.0); + let start = running.position(block_id); running .manager .fade_to_black(block_id, 1000) @@ -476,17 +526,18 @@ async fn source_switched_during_ftb_is_pillarboxed_after_it() { // The source is grey while it fades back in, so count lit pixels: every // frame of the fade-in must already show it pillarboxed, not stretched. - let start = std::time::Instant::now(); + let fade_end = start + gstreamer::ClockTime::from_seconds(1); let mut fade_in = Vec::new(); - while start.elapsed() < std::time::Duration::from_millis(900) { - if let Some(s) = running - .appsink - .try_pull_sample(gstreamer::ClockTime::from_mseconds(200)) - { - let lit = 1.0 - black_fraction(&s); - if lit > 0.05 { - fade_in.push(lit); - } + while let Some(s) = running + .appsink + .try_pull_sample(gstreamer::ClockTime::from_seconds(5)) + { + if s.buffer().and_then(|b| b.pts()).expect("PGM pts") > fade_end { + break; + } + let lit = 1.0 - black_fraction(&s); + if lit > 0.05 { + fade_in.push(lit); } } assert!( @@ -503,3 +554,164 @@ async fn source_switched_during_ftb_is_pillarboxed_after_it() { eprintln!("after FTB: white={:.2}", white); running.stop(); } + +/// Run the mixer at about 40% of real time: each output frame takes 80 ms +/// instead of 33. +fn slow_down_mixer(running: &Running, block_id: &str) { + running + .element(&format!("{block_id}:mixer")) + .static_pad("src") + .expect("mixer src") + .add_probe(gstreamer::PadProbeType::BUFFER, |_, _| { + std::thread::sleep(std::time::Duration::from_millis(80)); + gstreamer::PadProbeReturn::Ok + }); +} + +/// FTB keyframes are in the mixer's stream time. On a mixer slower than real +/// time, both fades must still play out in full. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ftb_plays_out_on_a_slow_mixer() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmftb_slow"; + let running = Running::start(build_flow(block_id, LANDSCAPE)); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + slow_down_mixer(&running, block_id); + + running + .manager + .fade_to_black(block_id, 200) + .expect("FTB on"); + wait_for_pgm_by(&running.appsink, "black", black_fraction, |f| f > 0.95); + running + .manager + .fade_to_black(block_id, 1000) + .expect("FTB off"); + wait_for_pgm(&running.appsink, "the source back at full level", |f| { + f > 0.95 + }); + running.stop(); +} + +/// FTB off pressed while FTB on is still fading out brings PGM fully back. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ftb_off_during_the_fade_out_restores_pgm() { + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmftb_abort"; + let running = Running::start(build_flow(block_id, LANDSCAPE)); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + + running + .manager + .fade_to_black(block_id, 500) + .expect("FTB on"); + std::thread::sleep(std::time::Duration::from_millis(250)); + running + .manager + .fade_to_black(block_id, 500) + .expect("FTB off"); + // FTB on's cleanup comes due in the middle of FTB off's fade. Stuck + // part-way, PGM stays grey. + wait_for_pgm(&running.appsink, "the source back at full level", |f| { + f > 0.95 + }); + running.stop(); +} + +/// A cropped PiP source that changes shape during FTB comes back with its +/// crop re-derived from the new width: half of 720 px, not of 1280. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn pip_crop_follows_a_source_resized_during_ftb() { + use strom_types::vision_mixer::{PipTransforms, SourceCrop, Zone}; + gstreamer::init().unwrap(); + if !gl_available_or_required() { + return; + } + let block_id = "vmftb_pipcrop"; + let mut flow = build_flow(block_id, LANDSCAPE); + flow.blocks[0] + .properties + .insert("num_pips".to_string(), PV::String("1".into())); + let running = Running::start(flow); + wait_for_pgm(&running.appsink, "the landscape source filling PGM", |f| { + f > 0.95 + }); + + // PiP on PGM: red background (input 1), input 0 in one zone with its + // right half cropped off. + let mut transforms = PipTransforms::new(); + transforms.insert( + 0, + SourceCrop { + left: 0.0, + top: 0.0, + right: 0.5, + bottom: 0.0, + }, + ); + running + .manager + .apply_vision_mixer_pip_config( + block_id, + 0, + Some(1), + vec![Zone { + rect: None, + capacity: None, + sources: vec![0], + border: None, + }], + transforms, + ) + .expect("PiP config"); + running + .manager + .select_vision_mixer_pip_for_preview(block_id, 0) + .expect("PiP to PVW"); + running + .manager + .trigger_transition(block_id, 0, 0, "cut", 0) + .expect("take the PiP"); + let pad = running + .element(&format!("{block_id}:mixer")) + .static_pad("sink_0") + .expect("mixer sink_0"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while pad.property::("crop-right") != 640 { + assert!( + std::time::Instant::now() < deadline, + "PiP crop never applied, crop-right {}", + pad.property::("crop-right") + ); + std::thread::sleep(std::time::Duration::from_millis(50)); + } + + running + .manager + .fade_to_black(block_id, 200) + .expect("FTB on"); + wait_for_pgm_by(&running.appsink, "black", black_fraction, |f| f > 0.95); + running.set_source_shape(PORTRAIT); + running.wait_for_input_width(block_id, 0, PORTRAIT.0); + running + .manager + .fade_to_black(block_id, 200) + .expect("FTB off"); + + assert_eq!( + pad.property::("crop-right"), + PORTRAIT.0 / 2, + "the crop after FTB off is still sized for the old source" + ); + running.stop(); +} From 21a3f107c64ed9f27e57423dfbc422ef518daf6e Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Tue, 29 Sep 2026 08:01:58 -0700 Subject: [PATCH 6/9] test(gl): fail instead of skipping when the GPU mixer breaks vision_mixer_fx_test printed SKIP and passed when the GPU vision mixer failed to build or start, even after the GL probe had proven the environment renders and even under STROM_REQUIRE_GL. A GPU mixer that stopped building passed green on the macOS job. Those paths now fail. Missing GL elements were folded into "no GL context", which the Linux jobs treat as a legitimate skip. Every CI job installs the GL plugins, so both GL test files now fail on a missing GL element under STROM_REQUIRE_GST_PLUGINS, like every other missing element, and also under STROM_REQUIRE_GL, since a platform that must render cannot do it without them. The two fx tests shared block ID "vmfx". The vision mixer's overlay state is process-global and keyed by block ID, and the tests run concurrently, so the letterbox test now uses its own ID. Co-Authored-By: Claude Opus 5.5 --- backend/tests/shader_validation_test.rs | 36 +++++++--- backend/tests/vision_mixer_fx_test.rs | 95 ++++++++++++++----------- 2 files changed, 80 insertions(+), 51 deletions(-) diff --git a/backend/tests/shader_validation_test.rs b/backend/tests/shader_validation_test.rs index bc674b357..6f26bd150 100644 --- a/backend/tests/shader_validation_test.rs +++ b/backend/tests/shader_validation_test.rs @@ -104,14 +104,37 @@ fn gl_environment_available() -> bool { } } +/// Are the GL elements installed? Every CI job installs them, so a missing one +/// is a broken install, not a headless runner: `STROM_REQUIRE_GST_PLUGINS` +/// turns the skip into a failure, as it does for every other missing element. +fn gl_elements_available() -> bool { + let missing: Vec<&str> = ["glshader", "gltestsrc"] + .into_iter() + .filter(|e| gst::ElementFactory::find(e).is_none()) + .collect(); + if missing.is_empty() { + return true; + } + // Either variable forbids the skip: a missing element is a broken install, + // and a platform that must render cannot render without them. + if let Some(var) = ["STROM_REQUIRE_GST_PLUGINS", "STROM_REQUIRE_GL"] + .into_iter() + .find(|v| strom_types::env::var_opt(v).is_some()) + { + panic!( + "{var} is set but these elements are missing: {}", + missing.join(", ") + ); + } + eprintln!("SKIP: GL elements missing: {}", missing.join(", ")); + false +} + #[test] fn all_shader_fragments_compile() { gst::init().expect("gst init"); - if gst::ElementFactory::find("glshader").is_none() - || gst::ElementFactory::find("gltestsrc").is_none() - { - eprintln!("SKIP: GStreamer GL elements not available"); + if !gl_elements_available() { return; } @@ -152,10 +175,7 @@ fn all_shader_fragments_compile() { fn runtime_fragment_swap_takes_effect() { gst::init().expect("gst init"); - if gst::ElementFactory::find("glshader").is_none() - || gst::ElementFactory::find("gltestsrc").is_none() - { - eprintln!("SKIP: GStreamer GL elements not available"); + if !gl_elements_available() { return; } if !gl_environment_available() { diff --git a/backend/tests/vision_mixer_fx_test.rs b/backend/tests/vision_mixer_fx_test.rs index c912ebd8c..cb3bf238c 100644 --- a/backend/tests/vision_mixer_fx_test.rs +++ b/backend/tests/vision_mixer_fx_test.rs @@ -16,6 +16,9 @@ use strom_types::Flow; use tempfile::NamedTempFile; const BLOCK_ID: &str = "vmfx"; +/// The vision mixer's overlay state is process-global and keyed by block ID, +/// and the tests in this binary run concurrently, so each test needs its own. +const LETTERBOX_BLOCK_ID: &str = "vmfx-letterbox"; /// Probe whether this environment can actually render through GL: the GL /// plugins being installed is not enough — on headless CI runners the @@ -26,12 +29,6 @@ const BLOCK_ID: &str = "vmfx"; /// not skip it). fn gl_environment_available() -> bool { use gstreamer::prelude::*; - if gstreamer::ElementFactory::find("glvideomixerelement").is_none() - || gstreamer::ElementFactory::find("glshader").is_none() - || gstreamer::ElementFactory::find("gltestsrc").is_none() - { - return false; - } let Ok(pipeline) = gstreamer::parse::launch( "gltestsrc num-buffers=3 ! video/x-raw(memory:GLMemory),format=RGBA,width=64,height=64,framerate=30/1 ! fakesink sync=false", ) else { @@ -66,7 +63,30 @@ fn gl_environment_available() -> bool { /// `STROM_REQUIRE_GL=1` turns the skip into a failure. CI sets it on the macOS job, /// which is the one platform whose runner renders, so that job cannot quietly stop /// exercising the FX engine. +/// +/// Missing GL elements are a different failure: every CI job installs them, so +/// they go through `STROM_REQUIRE_GST_PLUGINS` like any other missing element, +/// rather than being folded into "no GL context" where the Linux jobs skip. fn gl_available_or_required() -> bool { + let missing: Vec<&str> = ["glvideomixerelement", "glshader", "gltestsrc"] + .into_iter() + .filter(|e| gstreamer::ElementFactory::find(e).is_none()) + .collect(); + if !missing.is_empty() { + // Either variable forbids the skip: a missing element is a broken install, + // and a platform that must render cannot render without them. + if let Some(var) = ["STROM_REQUIRE_GST_PLUGINS", "STROM_REQUIRE_GL"] + .into_iter() + .find(|v| strom_types::env::var_opt(v).is_some()) + { + panic!( + "{var} is set but these elements are missing: {}", + missing.join(", ") + ); + } + eprintln!("SKIP: GL elements missing: {}", missing.join(", ")); + return false; + } if gl_environment_available() { return true; } @@ -75,7 +95,7 @@ fn gl_available_or_required() -> bool { "STROM_REQUIRE_GL is set but no GL context could be created — this platform \ is supposed to render, so a skip here would hide a GL regression" ); - eprintln!("SKIP: GL environment unavailable (no context or GL elements missing)"); + eprintln!("SKIP: GL environment unavailable (no context could be created)"); false } @@ -121,7 +141,7 @@ async fn vision_mixer_fx_engine_end_to_end() { let flow = build_vm_flow(); - let mut manager = match PipelineManager::new( + let mut manager = PipelineManager::new( &flow, events, ®istry, @@ -130,19 +150,12 @@ async fn vision_mixer_fx_engine_end_to_end() { None, media_path, std::sync::Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), - ) { - Ok(m) => m, - Err(e) => { - // GL context creation can fail in truly headless environments. - eprintln!("SKIP: could not build GPU pipeline ({})", e); - return; - } - }; + ) + // GL was proven to render above, so a build or start failure here is the + // GPU mixer breaking, not the environment. It must fail, not skip. + .expect("GPU vision mixer pipeline builds"); - if let Err(e) = manager.start() { - eprintln!("SKIP: could not start GPU pipeline ({})", e); - return; - } + manager.start().expect("GPU vision mixer pipeline starts"); // Wait until the mixer actually produces output (its position query // answers). A fixed sleep is not enough on cold software-GL CI runners, @@ -293,7 +306,7 @@ async fn wipe_between_letterboxed_sources_animates() { let mut flow = Flow::new("vm_letterbox_wipe"); flow.blocks.push(strom_types::BlockInstance { - id: BLOCK_ID.to_string(), + id: LETTERBOX_BLOCK_ID.to_string(), block_definition_id: "builtin.vision_mixer".to_string(), name: None, properties: { @@ -381,10 +394,10 @@ async fn wipe_between_letterboxed_sources_animates() { )); for (from, to) in [ ("src0:src", "caps0:sink"), - ("caps0:src", "vmfx:video_in_0"), + ("caps0:src", "vmfx-letterbox:video_in_0"), ("src1:src", "caps1:sink"), - ("caps1:src", "vmfx:video_in_1"), - ("vmfx:pgm_out", "pgmsink:sink"), + ("caps1:src", "vmfx-letterbox:video_in_1"), + ("vmfx-letterbox:pgm_out", "pgmsink:sink"), ] { flow.links.push(strom_types::Link { from: from.to_string(), @@ -396,7 +409,7 @@ async fn wipe_between_letterboxed_sources_animates() { let registry = BlockRegistry::new(temp_file.path()); let events = EventBroadcaster::with_capacity(10); - let mut manager = match PipelineManager::new( + let mut manager = PipelineManager::new( &flow, events, ®istry, @@ -405,17 +418,10 @@ async fn wipe_between_letterboxed_sources_animates() { None, std::env::temp_dir(), std::sync::Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), - ) { - Ok(m) => m, - Err(e) => { - eprintln!("SKIP: could not build GPU pipeline ({})", e); - return; - } - }; - if let Err(e) = manager.start() { - eprintln!("SKIP: could not start GPU pipeline ({})", e); - return; - } + ) + // GL was proven to render above: fail, do not skip. + .expect("GPU vision mixer pipeline builds"); + manager.start().expect("GPU vision mixer pipeline starts"); // Let caps probes settle so pads get their aspect-fitted rects. tokio::time::sleep(tokio::time::Duration::from_millis(1500)).await; @@ -468,7 +474,7 @@ async fn wipe_between_letterboxed_sources_animates() { }; // Debug aid: verify the source branches are actually linked. - for name in ["vmfx:queue_0", "vmfx:queue_1"] { + for name in ["vmfx-letterbox:queue_0", "vmfx-letterbox:queue_1"] { let q = manager.pipeline().by_name(name).expect(name); let linked = q.static_pad("sink").map(|p| p.is_linked()).unwrap_or(false); eprintln!("{} sink linked: {}", name, linked); @@ -540,13 +546,13 @@ async fn wipe_between_letterboxed_sources_animates() { // --- classic orientation: 2.40:1 -> 2.34:1 (outgoing does not cover) --- manager - .trigger_transition(BLOCK_ID, 0, 1, "wipe_left", 2000) + .trigger_transition(LETTERBOX_BLOCK_ID, 0, 1, "wipe_left", 2000) .expect("wipe 0->1"); // Mirror the API handler: persist the PGM/PVW swap after the take — // trigger_transition reads the authoritative bus state from overlay // state, so without this the next take would re-run the same pair. manager - .update_vision_mixer_after_take(BLOCK_ID, Some(1), Some(0), 2) + .update_vision_mixer_after_take(LETTERBOX_BLOCK_ID, Some(1), Some(0), 2) .expect("after take 0->1"); let (animated, w_end, r_end) = observe_wipe(true); eprintln!( @@ -566,10 +572,10 @@ async fn wipe_between_letterboxed_sources_animates() { // --- inverted orientation: 2.34:1 -> 2.40:1 (outgoing covers) --- manager - .trigger_transition(BLOCK_ID, 1, 0, "wipe_left", 2000) + .trigger_transition(LETTERBOX_BLOCK_ID, 1, 0, "wipe_left", 2000) .expect("wipe 1->0"); manager - .update_vision_mixer_after_take(BLOCK_ID, Some(0), Some(1), 2) + .update_vision_mixer_after_take(LETTERBOX_BLOCK_ID, Some(0), Some(1), 2) .expect("after take 1->0"); let (animated2, w_end2, r_end2) = observe_wipe(false); eprintln!( @@ -577,7 +583,10 @@ async fn wipe_between_letterboxed_sources_animates() { animated2, w_end2, r_end2 ); // Debug: pad + fx state at the broken end state. - let mixer = manager.pipeline().by_name("vmfx:mixer").expect("mixer"); + let mixer = manager + .pipeline() + .by_name("vmfx-letterbox:mixer") + .expect("mixer"); for i in 0..2 { let pad = mixer.static_pad(&format!("sink_{}", i)).unwrap(); eprintln!( @@ -594,7 +603,7 @@ async fn wipe_between_letterboxed_sources_animates() { for i in 0..2 { let fx = manager .pipeline() - .by_name(&format!("vmfx:fx_take_{}", i)) + .by_name(&format!("vmfx-letterbox:fx_take_{}", i)) .unwrap(); let u = fx.property::>("uniforms"); eprintln!("fx_take_{} uniforms: {:?}", i, u.map(|s| s.to_string())); From 7d9968ccfa5915ba860ecce92e649826e4cb4a44 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Wed, 30 Sep 2026 22:05:38 -0700 Subject: [PATCH 7/9] test(gl): route missing GL elements through the shared test helper Both GL test files hand-rolled the STROM_REQUIRE_GST_PLUGINS check that common::require_or_skip already owns. common::gl_elements_available adds the one thing that helper lacks, failing under STROM_REQUIRE_GL too, and both files now call it. Co-Authored-By: Claude Opus 5.5 --- backend/tests/common/mod.rs | 12 +++++++++ backend/tests/shader_validation_test.rs | 34 +++++-------------------- backend/tests/vision_mixer_fx_test.rs | 22 +++------------- 3 files changed, 23 insertions(+), 45 deletions(-) diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index cddec03a1..15df71af1 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -43,6 +43,18 @@ pub fn plugins_available(required: &[&str]) -> bool { require_or_skip(&missing_elements(required)) } +/// [`plugins_available`] for GL elements. A missing one also fails under +/// `STROM_REQUIRE_GL`: a platform that must render cannot do it without them. +pub fn gl_elements_available(required: &[&str]) -> bool { + let missing = missing_elements(required); + assert!( + missing.is_empty() || strom_types::env::var_opt("STROM_REQUIRE_GL").is_none(), + "STROM_REQUIRE_GL is set but these GL elements are missing: {}", + missing.join(", ") + ); + require_or_skip(&missing) +} + /// Fail outright if any element in `required` is missing, whatever /// `STROM_REQUIRE_GST_PLUGINS` says. For tests that must never skip. pub fn require_elements(required: &[&str]) { diff --git a/backend/tests/shader_validation_test.rs b/backend/tests/shader_validation_test.rs index 6f26bd150..a9ee99adf 100644 --- a/backend/tests/shader_validation_test.rs +++ b/backend/tests/shader_validation_test.rs @@ -9,6 +9,8 @@ //! The whole test skips when the environment cannot create a GL context at //! all (probed with the identity fragment, which is trivially valid GLSL). +pub mod common; + use gstreamer as gst; use gstreamer::prelude::*; use gstreamer_app as gst_app; @@ -104,37 +106,15 @@ fn gl_environment_available() -> bool { } } -/// Are the GL elements installed? Every CI job installs them, so a missing one -/// is a broken install, not a headless runner: `STROM_REQUIRE_GST_PLUGINS` -/// turns the skip into a failure, as it does for every other missing element. -fn gl_elements_available() -> bool { - let missing: Vec<&str> = ["glshader", "gltestsrc"] - .into_iter() - .filter(|e| gst::ElementFactory::find(e).is_none()) - .collect(); - if missing.is_empty() { - return true; - } - // Either variable forbids the skip: a missing element is a broken install, - // and a platform that must render cannot render without them. - if let Some(var) = ["STROM_REQUIRE_GST_PLUGINS", "STROM_REQUIRE_GL"] - .into_iter() - .find(|v| strom_types::env::var_opt(v).is_some()) - { - panic!( - "{var} is set but these elements are missing: {}", - missing.join(", ") - ); - } - eprintln!("SKIP: GL elements missing: {}", missing.join(", ")); - false -} +/// The GL elements this test needs. Every CI job installs them, so a missing +/// one is a broken install, not a headless runner, and must not skip silently. +const GL_ELEMENTS: &[&str] = &["glshader", "gltestsrc"]; #[test] fn all_shader_fragments_compile() { gst::init().expect("gst init"); - if !gl_elements_available() { + if !common::gl_elements_available(GL_ELEMENTS) { return; } @@ -175,7 +155,7 @@ fn all_shader_fragments_compile() { fn runtime_fragment_swap_takes_effect() { gst::init().expect("gst init"); - if !gl_elements_available() { + if !common::gl_elements_available(GL_ELEMENTS) { return; } if !gl_environment_available() { diff --git a/backend/tests/vision_mixer_fx_test.rs b/backend/tests/vision_mixer_fx_test.rs index cb3bf238c..d3fca6597 100644 --- a/backend/tests/vision_mixer_fx_test.rs +++ b/backend/tests/vision_mixer_fx_test.rs @@ -7,6 +7,8 @@ //! context (probed like `shader_validation_test`; merely having the GL //! plugins installed is not enough, as headless CI runners show). +pub mod common; + use std::collections::HashMap; use strom::blocks::BlockRegistry; use strom::events::EventBroadcaster; @@ -65,26 +67,10 @@ fn gl_environment_available() -> bool { /// exercising the FX engine. /// /// Missing GL elements are a different failure: every CI job installs them, so -/// they go through `STROM_REQUIRE_GST_PLUGINS` like any other missing element, +/// they fail under `STROM_REQUIRE_GST_PLUGINS` like any other missing element, /// rather than being folded into "no GL context" where the Linux jobs skip. fn gl_available_or_required() -> bool { - let missing: Vec<&str> = ["glvideomixerelement", "glshader", "gltestsrc"] - .into_iter() - .filter(|e| gstreamer::ElementFactory::find(e).is_none()) - .collect(); - if !missing.is_empty() { - // Either variable forbids the skip: a missing element is a broken install, - // and a platform that must render cannot render without them. - if let Some(var) = ["STROM_REQUIRE_GST_PLUGINS", "STROM_REQUIRE_GL"] - .into_iter() - .find(|v| strom_types::env::var_opt(v).is_some()) - { - panic!( - "{var} is set but these elements are missing: {}", - missing.join(", ") - ); - } - eprintln!("SKIP: GL elements missing: {}", missing.join(", ")); + if !common::gl_elements_available(&["glvideomixerelement", "glshader", "gltestsrc"]) { return false; } if gl_environment_available() { From 754b839c33798064616b997181b421799b257f43 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Thu, 1 Oct 2026 13:14:43 -0700 Subject: [PATCH 8/9] test(vision mixer): pass take indices through a helper that fits either signature #806 changes trigger_transition's input indices from usize to Option. idx() converts into whichever the method takes, so these tests compile whether #806 lands before or after this PR. Co-Authored-By: Claude Opus 5.5 --- backend/tests/vision_mixer_source_resize_test.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs index 8bf9c127d..f4ed36aaf 100644 --- a/backend/tests/vision_mixer_source_resize_test.rs +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -15,6 +15,12 @@ use strom::gst::pipeline::PipelineManager; use strom_types::{Flow, PropertyValue as PV}; use tempfile::NamedTempFile; +/// A take index for `trigger_transition`, which takes `usize` on main and +/// `Option` once #806 lands. Either signature accepts this. +fn idx>(i: usize) -> T { + T::from(i) +} + /// Same probe as `vision_mixer_fx_test`: a trivial GL run must reach EOS. /// Having the GL plugins installed is not enough on headless runners. fn gl_environment_available() -> bool { @@ -447,7 +453,7 @@ async fn source_resized_during_a_fade_is_fitted_after_it() { let start = running.position(block_id); running .manager - .trigger_transition(block_id, 0, 1, "fade", 4000) + .trigger_transition(block_id, idx(0), idx(1), "fade", 4000) .expect("fade"); running .manager @@ -680,7 +686,7 @@ async fn pip_crop_follows_a_source_resized_during_ftb() { .expect("PiP to PVW"); running .manager - .trigger_transition(block_id, 0, 0, "cut", 0) + .trigger_transition(block_id, idx(0), idx(0), "cut", 0) .expect("take the PiP"); let pad = running .element(&format!("{block_id}:mixer")) From e3a30cca877fb96300382bed045b36a6d8ea78db Mon Sep 17 00:00:00 2001 From: Per Enstedt Date: Fri, 2 Oct 2026 16:29:13 +0200 Subject: [PATCH 9/9] test(vision mixer): route missing GL elements through the shared test helper The resize tests had their own copy of the GL skip check, which folded missing GL elements into "no GL context". Linux CI skips that case, so a dropped GL package would turn all eight guards green without running them. Check the elements with common::gl_elements_available from #917 first, and keep the context probe for the legitimate headless skip. Co-Authored-By: Claude Opus 5.5 --- backend/tests/vision_mixer_source_resize_test.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/tests/vision_mixer_source_resize_test.rs b/backend/tests/vision_mixer_source_resize_test.rs index f4ed36aaf..f8e775cd8 100644 --- a/backend/tests/vision_mixer_source_resize_test.rs +++ b/backend/tests/vision_mixer_source_resize_test.rs @@ -7,6 +7,8 @@ //! a 1280x720 canvas is pillarboxed to about a third of the width; stretched, //! PGM is entirely white. +pub mod common; + use gstreamer::prelude::*; use std::collections::HashMap; use strom::blocks::BlockRegistry; @@ -24,12 +26,6 @@ fn idx>(i: usize) -> T { /// Same probe as `vision_mixer_fx_test`: a trivial GL run must reach EOS. /// Having the GL plugins installed is not enough on headless runners. fn gl_environment_available() -> bool { - if gstreamer::ElementFactory::find("glvideomixerelement").is_none() - || gstreamer::ElementFactory::find("glshader").is_none() - || gstreamer::ElementFactory::find("gltestsrc").is_none() - { - return false; - } let Ok(pipeline) = gstreamer::parse::launch( "gltestsrc num-buffers=3 ! video/x-raw(memory:GLMemory),format=RGBA,width=64,height=64,framerate=30/1 ! fakesink sync=false", ) else { @@ -54,8 +50,12 @@ fn gl_environment_available() -> bool { } /// Skip unless GL works; `STROM_REQUIRE_GL=1` (set on the macOS CI job) -/// turns the skip into a failure. +/// turns the skip into a failure. Missing GL elements go through +/// `common::gl_elements_available`, so they are not folded into "no context". fn gl_available_or_required() -> bool { + if !common::gl_elements_available(&["glvideomixerelement", "glshader", "gltestsrc"]) { + return false; + } if gl_environment_available() { return true; } @@ -64,7 +64,7 @@ fn gl_available_or_required() -> bool { "STROM_REQUIRE_GL is set but no GL context could be created — this platform \ is supposed to render, so a skip here would hide a GL regression" ); - eprintln!("SKIP: GL environment unavailable (no context or GL elements missing)"); + eprintln!("SKIP: GL environment unavailable (no context could be created)"); false }