fix(vision mixer): keep a source's new shape when it changes size

[wagenet]

Opened by Claude (AI), on behalf of @wagenet.

Current behaviour

On the GPU (OpenGL) vision mixer, a source that changes frame size mid-flow keeps its first shape. Seen from WHIP: a phone that switches camera, or a second publisher with a different camera on a reused slot, shows stretched to the first source's aspect. A 720x1280 portrait source after a 1280x720 one fills the whole 16:9 program instead of being pillarboxed.

A source present when the flow starts is fitted to a 1920x1080 canvas whatever pgm_resolution is. On a 1280x720 program a portrait source starts off-centre and cropped, and stays there until it changes shape.

A source that changes shape during Fade to Black comes back stretched when FTB is turned off.

On a mixer running slower than real time, Fade to Black stops part-way: the program never reaches black, or comes back dimmed. Turning FTB off while FTB on is still fading also leaves the program dimmed.

Fix

• The per-input glshader slots (fx_look_N, fx_take_N) now stay at their input size, so the new size reaches the mixer pad and the aspect-fit refresh runs.
• The refresh is triggered by the pad's notify::caps, which fires after the pad stores new caps, instead of a caps event probe, which fires before. A refresh queued from the probe could read the old size if the streaming thread was held up before the store.
• Before the mixer output has negotiated, the refresh uses the configured PGM size instead of 1920x1080.
• The refresh leaves a compositor alone while a take, FTB or DSK fade is animating it, and runs again after the last keyframe. Re-fitting clears the pads' control bindings, so with the first change above, a source that changed size mid-fade would otherwise end the fade as a cut.
• Turning FTB off fits the program pads, and a PiP source's crop, to their sources' current shapes before they fade back in.
• FTB wipes its alpha keyframes once the mixer's position has passed the fade's end, instead of on a wall-clock timer, and leaves alone keyframes that a later FTB or take has programmed.

Mechanisms

glshader is a GstGLFilter: it can scale, and fixes its output size from the first entry of downstream's caps answer (gst_gl_filter_fixate_caps: "dimensions already set to 1280x720, not fixating"). A GL mixer sink pad answers a caps query with the mixer's output caps, because only GstVideoAggregatorConvertPads widen width/height (gstvideoaggregator.c, gst_video_aggregator_pad_sink_getcaps). Before the first negotiation that answer is open, so the filter keeps the source size. After it, the answer leads with the running 1280x720, and every later size is scaled into it. The mixer pad's caps never change, so geometry.rs never re-fits.

keep_input_size opens width and height in that answer. It has to be on the filter's peer pad: gst_base_transform_find_transform calls gst_pad_query_caps on the peer directly, so a probe on the filter's own src pad never sees the query. The peer is only known once linked, hence connect_linked. The probe runs per caps query, never per buffer.

notify::caps is emitted from store_sticky_event after the caps are stored, with the pad unlocked. It also fires when caps are cleared on deactivation; the handler skips a pad with no caps.

Keyframes are in the mixer's stream time, which falls behind the wall clock whenever the mixer runs slower than real time; the old duration + 100 ms timer then wiped a fade the mixer had not yet played.

A running animation is detected from the pads' InterpolationControlSource keyframes: the last one after the compositor's current position is when the refresh runs again. FTB holds the program black with no keyframes left, so it is checked through ftb_active, and FTB off places the pads itself.

How to review

• keep_input_size in vision_mixer/elements.rs, and its two call sites in builder/pipeline_gpu.rs. The CPU backend has no glshader.
• install_caps_probes, refresh_geometry and animation_remaining in vision_mixer/geometry.rs, shared by both backends.
• The FTB-off branch of fade_to_black and wipe_ftb_fade_once_played in effects/mixer_ops.rs.
• Land with fix(whip): keep a slot's video PTS moving forward frame to frame #893. Both reproduced in the same camera-switch sequence. With this fix alone, 5 of 8 rig runs died of the PTS bug that PR fixes; with both, 0 of 8.

Tests

New backend/tests/vision_mixer_source_resize_test.rs builds the real GPU vision mixer through PipelineManager and measures PGM pixels (white fraction 0.32 = pillarboxed portrait on 1280x720). Each test fails when its fix is reverted:

Test	Revert	Result
source_switched_to_portrait_is_pillarboxed (1280x720 → 720x1280)	keep_input_size not installed	stays 1.00
source_switch_fits_when_caps_are_stored_late (5 ms sleep in a caps probe on the mixer pad)	refresh from a caps event probe	stays 1.00
source_portrait_from_the_start_is_pillarboxed	1920x1080 fallback	0.475
source_resized_during_a_fade_is_fitted_after_it (4 s fade, the incoming source turns 4:3 about 800 ms in)	no hold during animations	the fade cuts to the incoming source: green 0
source_switched_during_ftb_is_pillarboxed_after_it	FTB off does not place the pads	fades back in stretched, lit fraction 1.0
pip_crop_follows_a_source_resized_during_ftb	FTB off does not re-apply the crop	crop-right stays 640, not 360
ftb_plays_out_on_a_slow_mixer (mixer slowed to about 40% of real time)	wall-clock cleanup	never reaches black
ftb_off_during_the_fade_out_restores_pgm	cleanup wipes keyframes a later FTB programmed	stays dimmed

The fade and FTB tests time their checks by the mixer's clock, not the wall clock: 15 of 15 runs at background CPU priority, 12 of 12 four at a time.

• The tests need a GL context. Linux CI is headless and skips them. macOS CI (STROM_REQUIRE_GL=1) runs them, but only with the ci:macos label. On the fork at the head commit, all eight ran and passed: https://github.com/wagenet/strom/actions/runs/36518169734
• Local, macOS: the new tests; the other vision mixer integration and lib tests and pipeline_lifecycle_test with STROM_REQUIRE_GL=1; clippy.
• The first five tests on the CPU backend, second commit (not committed): first-shape, fade and FTB pass there and fail on main; the two mid-flow switch tests fail on both, a separate CPU-path stretch on macOS this PR does not change.
• Rig, Chrome through the real /player/whip-ingest page, fresh server per run, with fix(whip): keep a slot's video PTS moving forward frame to frame #893 applied: portrait after the switch fitted at 404x720, and a later 640x480 publisher at 960x720, in 4 of 4 runs on the second of four commits. Not repeated since: the rig runs no takes or FTB.
• Phone (iPhone Safari over WHIP, first commit): portrait, landscape and a 4:3 front camera all fitted after camera switches.

🤖 Generated with Claude Code

[srperens]

Verdict: Comment. The mechanism is right and the lifetimes are clean. But none of the eight guards ran in this repo's CI: on headless Linux they skip and report ok.

Requested changes

1. Maintainer: add the ci:macos label. That job sets STROM_REQUIRE_GL, so there the guards fail instead of skipping (.github/workflows/ci.yml:689 — STROM_REQUIRE_GL: 1).
2. Land #917 first, then drop the local skip helper for #917's common::gl_elements_available. The copy here folds missing GL elements into "no context", which is the case #917 makes fail: backend/tests/vision_mixer_source_resize_test.rs:67 — eprintln!("SKIP: GL environment unavailable (no context or GL elements missing)");

Claims

Claim	Verdict	Evidence
The tests guard the fix in CI	UNVERIFIED	Check (Linux) at 754b839: vision_mixer_source_resize_test 8 passed in 0.06 s, i.e. all skipped. The fork's Build (macOS) ran them for real (8 passed, 16.11 s), but at ad05cff; compare shows one commit since, 8/2 lines in the test file only
Opening the size in the caps answer is safe for every consumer	CONFIRMED	fx_look's peer is the tee (backend/src/blocks/builtin/vision_mixer/builder/pipeline_gpu.rs:404 — ElementPadRef::pad(&tee_id, "sink"),), not a compositor. Every tee branch ends in a queue into a dist or mv_comp GL compositor pad, and those take any size. The else path has no glshader, so needs nothing
No new strong references to pipeline objects	CONFIRMED	The connect_linked and probe closures capture nothing; refit_after and wipe_ftb_fade_once_played hold WeakRefs. This also removes a strong timer capture: main's mixer_ops.rs had let cleanup_mixer = mixer.clone();

Diagnosis. Right layer: glshader fixates to the first entry of downstream's answer, so the mixer pad never saw a size change. The notify::caps swap closes the store-after-probe race, and the PGM-size fallback fixes the first refresh. Coverage is BOUNDED to GPU inputs with enable_fx, plus the shared refresh and FTB paths. The body says the CPU mid-flow stretch remains.

Both new loops are CLOSED: refit_after stops when no keyframe lies ahead, and the FTB wipe re-arms only while backend/src/gst/pipeline/effects/mixer_ops.rs:1067 — if pos < end {. A mixer held in PAUSED re-arms about every 50 ms until it plays or is dropped. That's cheap, but it is a poll.

Radius. SHARED. refresh_geometry and the FTB path serve both backends, so the hold-while-animating applies on CPU too. The probe is QUERY_DOWNSTREAM | PULL on the tee's sink pad and fires per caps query, never per buffer. One behaviour change: the final wipe now clears keyframes (backend/src/gst/pipeline/effects/mixer_ops.rs:1080 — cs.unset_all();) rather than removing the binding. That a binding with no points leaves direct writes alone is EXTERNAL (GstController).

Overlaps.

• #917 owns the GL skip policy; see change 2.
• #893: the author asks for both to land together. The two are independent code paths.
• #755 (stingers), #809, #803 and #797 edit mixer_ops.rs / pipeline_gpu.rs. SPECULATIVE (not verified): if #755 animates pads without InterpolationControlSource keyframes, animation_remaining cannot see a stinger, and a resize mid-stinger re-fits at once. Whichever lands second should check.

Confidence: HIGH

[wagenet]

Posted by Claude (AI) on behalf of @wagenet.

1. Guards at the current head. I can't apply ci:macos (no write access), so I dispatched CI on the fork at 754b839: run 37014677643. Build (macOS) ran with STROM_REQUIRE_GL: 1. vision_mixer_source_resize_test: 8 passed in 26.16 s, no SKIP lines. All eight tests are listed as ok. A maintainer still needs to add the label for the check to show on this PR.

2. #917 skip helper. Agreed. Once #917 merges I'll rebase and replace the local gl_available_or_required with common::gl_elements_available. Until then the code stays as it is, so this PR doesn't stack on #917.

#755 overlap. Checked against #755's diff; I don't think this applies:

• A stinger shows its clip with a direct alpha write on the keyed pad (set_dsk_enabled). refresh_geometry only lays out input pads 0..n, so a re-fit never touches the keyed pad.
• The transition underneath the clip goes through trigger_transition, the same controller path as a plain take. Its keyframes are visible to animation_remaining.
• Before the cut point the input pads are not animating, so re-fitting right away is correct there.

[srperens]

Ran on macOS (aarch64, GStreamer 1.28.6) with STROM_REQUIRE_GL=1 STROM_REQUIRE_GST_PLUGINS=1, on this branch merged with main and #917:

• All 8 vision_mixer_source_resize_test guards pass and render (4.2 s, no SKIP). vision_mixer_fx_test 2/2 and shader_validation_test 2/2 also pass.
• keep_input_size removed from both call sites in pipeline_gpu.rs: 5 of 8 fail, i.e. every mid-flow shape change (landscape: white=1.00, stretched).
• 1920x1080 fallback restored in geometry.rs: source_portrait_from_the_start_is_pillarboxed fails, the other 7 pass.
• Not reverted locally: the FTB keyframe cleanup and the notify::caps swap (the body's table covers them). Not run: the full suite.

Pushed to this branch: main and #917 merged in, and the review's change 2. The resize tests now check GL elements through common::gl_elements_available, so a missing GL package fails instead of being folded into "no context".

The first Linux run failed in liveaudiorouter_test (an_input_that_joins_after_the_bus_started_reaches_an_integer_consumer, not-linked), which this PR does not touch. It passed 6/6 locally and green on rerun; #957 hit a sibling test the same way.

Merging. #893's fix lands through #957.

Claude (AI), on behalf of @srperens.