feat(vision-mixer): zone borders as underlay pads, PiP layout presets, transition fixes - #625
Merged
Merged
Conversation
Mixer-state-driven graphics belong in the mixer: borders frame boxes
whose geometry only the mixer knows in real time (morphs, takes and
punch-ins drive the compositor pads via control bindings), so no
external graphics source could stay in sync. External graphics (lower
thirds, bugs) remain on DSK.
- Zone gains border: Option<ZoneBorder> { color: #RRGGBB[AA], width }
(serde default — additive API). The border belongs to the BOX, not the
source: a capacity-1 swap zone keeps its frame whoever is pushed in.
Borders draw around the actual rendered source boxes (aspect-fitted
rects), hugging the picture rather than the layout rect.
- Width is resolution-normalized: expressed in PGM canvas pixels and
scaled by region_width / pgm_width at every render target, so 4 px on
air looks proportionally identical on the PVW display and PiP tiles.
- New PGM overlay (pgm_overlay.rs): cairo → appsrc → one pad on the
dist compositor below the DSK stack (zorder 90), mirroring the
multiview overlay pattern. Geometry is read LIVE from the compositor
sink pads each timer tick (property reads, never a buffer probe) —
borders track animations frame by frame and fade with their box's
alpha. Dirty-hash skips redraws when nothing moved.
- Multiview parity: the existing mv overlay draws the same borders on
PiP tiles and the PVW big display (PGM big shows the dist output and
carries them already), with a live-geometry hash in its dirty check
so borders follow morphs there too.
- Pad-loop audit: the classic-take reset and FTB restore leave the new
PGM overlay pad alone / restore it (it sits beyond the DSK range).
- Zone editor: color swatch + width field on the active-zone control
row (v2.21). Operator guide updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mpling time Borders trailed their boxes during morphs/takes: the overlay renderer sampled the pads' current property values, but an overlay frame pushed now is composited 1-3 output frames later — by which time the control bindings driving the animation have moved the box further. Evaluate the bindings at (compositor position + 66 ms lead) instead via GstControlBinding::get_value, so the border is drawn where the box WILL be when the frame composites. Properties without an active binding fall back to plain reads. Applied to both the PGM overlay and the multiview border pass (shared pad_geometry_at helper). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The trailing fix evaluated border geometry 2 frame periods ahead, which overshot — borders led their boxes during morphs. One period matches reality: query_position() reports the frame currently being produced and a buffer pushed now lands in the next one. Confirmed empirically (0 frames trails, 2 leads, 1 tracks). The lead derives from each output's actual framerate — PGM and multiview can differ and each renderer uses its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…PU format Self-review findings on the zone-borders branch: - Build the PGM overlay machinery only when num_pips > 0: zones (and thus borders) cannot exist without PiPs, and the default mixer config has none — skipping it saves a compositor pad, an appsrc chain and a render thread on every plain mixer. The FTB/take pad-index guards remain valid either way (the pad index simply never matches). - Timer restart race (also fixed in the pre-existing multiview timer): a fast flow restart can re-register a NEW renderer under the same block id before the old timer thread observes the unregistration — the presence check alone would keep the old thread (and its strong AppSrc ref into the dead pipeline) alive forever. Both timers now verify registry identity (Arc::ptr_eq) instead. - CPU backend: match the forced output format with a capsfilter after videoconvert_pgm_overlay, mirroring the DSK and multiview overlay chains. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… alpha restore on take Three zone-border fixes: - Derive the border geometry look-ahead from the compositor's aggregation latency (the block's `latency` property) instead of one output frame period. The two happened to coincide at the 20 ms default / 50 fps, but a mixer configured with e.g. 500 ms latency drew borders ~480 ms behind their boxes. - Resolve border style (color/width) once per tick in border_pad_set (new MvBorder struct) and include it in the renderer dirty-hash, so a color- or width-only edit re-renders the multiview immediately instead of waiting for the next geometry change. - On take with FTB auto-cancel, strip the FTB alpha binding from the PGM graphics overlay pad and pin it back to full alpha (both the classic and PiP-aware take paths) — borders no longer stay frozen at whatever alpha a mid-flight fade last wrote. Frontend: keep an API-set #RRGGBBAA border color's alpha when re-committing border edits through the 6-digit color picker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions
Backend: new GET /api/flows/{flow_id}/blocks/{block_id}/pip/{pip_idx}
returning the PiP's live composition (bg, zones, transforms) as PipState —
the export half of save/restore; restore is the existing PUT on the same
path. 404 when the pipeline is not running or the index is out of range.
Registered in openapi.json.
Frontend (v2.24): Layout preset row in the PiP editor — dropdown plus
Save/Load/Delete against a browser-wide localStorage key, so a preset
saved on one PiP/flow can be loaded on any other.
A preset is a full composition snapshot: zones (rect, capacity, border,
sources by input index), background, and every non-default crop — including
retained crops for sources currently outside the PiP, mirroring strom's
own swap-zone semantics. Loading is best-effort: input indices that do not
exist on the target mixer are silently skipped, the bg double-booked as a
zone source is stripped, and cross-zone duplicates are deduped — each of
which would otherwise fail the whole PUT with 400.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… cairo overlays
A zone border is now a solid-color compositor pad sitting in z-order
directly beneath its content pad, sized to the content rect inflated
outward by the region-scaled border width. This replaces the cairo-drawn
borders (the PGM graphics overlay renderer and the multiview overlay's
border pass) and fixes two structural problems with drawing borders on
top of the mix:
- Morph sync: the underlay is animated by the same control-binding
timeline as its content pad, so the compositor evaluates both at the
same composite time. The whole look-ahead chain (border_lead, the
latency-derived lead, the per-tick geometry polling and dirty-hash,
the cairo re-render, the FTB alpha special-casing in the take engine)
is deleted — it existed to chase the compositor's clock from outside.
- Z-order: zone slots use a doubled scheme (content at
overlay_zorder + 2k + 1, underlay one below), so an overlapping
higher zone covers a lower zone's border exactly like a stacked
framed card, instead of all borders rendering on top of all content.
Each underlay pad is fed by a 16x16 videotestsrc (pattern=solid-color,
non-live so it adds no latency); the border color is written to its
foreground-color. Pads exist only when PiPs are configured: dist input
i's underlay at sink_{N+DSK+i} (replacing the PGM overlay pad), and on
the multiview after the overlay pad (PVW big + per-tile candidates).
Borders now also fade with their box (FTB, capacity-1 cross-fades) and
clamp to their region, so a tile border no longer bleeds into the
neighboring multiview tile. The stroke now sits fully outside the video
edge (it never covers content pixels) instead of half over it.
PadTarget carries an optional UnderlayTarget; plan_transition still
decides actions from content geometry alone, and the controller drives
each pad's underlay in lockstep with whatever action the content pad
got. Verified live on the CPU backend: geometry/zorder/color on dist
and multiview pads, FTB on/off, cut/fade takes input<->PiP, zone-rect
morphs, and two overlapping bordered zones interleaving correctly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slide and push transitions computed the incoming pad's destination from
the OUTGOING pad's position — correct back when every pad was fullscreen,
wrong since explicit per-pad aspect-fit geometry: taking between a 16:9
and a 2.39:1 source landed the incoming picture at the outgoing picture's
letterbox offset (wrong y). The classic-take reset already positions every
pad at its own aspect-fitted rect before the transition runs, so the
incoming pad's own xpos/ypos IS the destination — read that instead.
Slide also gets end-of-slide handling per the planner's rule ("what's
visible needs a transition; what's hidden doesn't"): when the incoming
rect fully covers the outgoing one, the old picture snaps off at the end
as before (it's hidden under the new one). With mismatched aspect ratios
the incoming rect can leave parts of the old picture visible — those now
fade out over the slide instead of popping away at the end. Push needs
neither: the outgoing picture always exits a full canvas dimension.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…transition picker Three UI improvements: - The source chips now act as checkboxes for the ACTIVE zone: checked = in the selected zone; a source sitting in another zone of the same PiP shows with a dashed outline and clicking it MOVES it to the active zone (previously the chips showed "in any zone" and clicking a taken source removed it from wherever it was). Tooltips spell out remove/move/push per chip. - Selecting a zone (zone buttons, canvas click, + Zone, preset load) points the Crop/Zoom panel at the zone's first source — selecting a box usually means "work on this box". Manually picking another crop target stays sticky until the next zone switch. - The transition picker now exposes all engine variants: MIX, DIP, PUSH and SLIDE with a direction row (left/right/up/down) for the latter two. Previously only fade, dip and push_left were reachable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er remove bindings
Production segfault (GP fault, confirmed via core dump): GStreamer's
gst_object_sync_values() iterates an object's control_bindings GList
WITHOUT any lock — the lock is deliberately disabled upstream ("FIXME:
this deadlocks" in gstobject.c, present in 1.24 and current master).
gst_object_remove_control_binding() mutates and frees list nodes under
the object lock from whatever thread calls it. Removing a binding while
a streaming thread is mid-iteration is therefore a use-after-free.
We hit it through videocrop, which syncs control values on EVERY buffer:
rapid PiP updates (dragging a zone rect fires many PUTs per second) had
the API thread removing + re-adding crop bindings while the videocrop
streaming thread walked the list — the core dump shows the iteration
following a dangling node->next into freed memory. Compositor pads sync
per output frame and carry the same race, just with a narrower window.
Fix: persistent bindings (new gst::control_bindings module). Each
(object, property) pair gets ONE DirectControlBinding with ONE
InterpolationControlSource, attached on first use and never removed —
first-time attachment appends, which the unlocked reader tolerates;
removal is what frees memory under it. "Clearing" a binding now wipes
the control source's keyframes (unset_all — guarded by the source's
internal lock), which makes the binding inert: get_value returns FALSE,
the property is left alone, and plain set_property writes behave exactly
as before. Reprogramming reuses the same source (unset_all + set_mode +
new keyframes). All former add/remove sites migrated: the transition
controller, set_pad_crop (both backends), the layout appliers, the
classic-take reset, and FTB (whose cleanup timeout now wipes instead of
removes, keeping DSK toggles and takes writable after a fade).
Verified live on the CPU backend: 300 sequential rapid PiP updates plus
3 parallel update hammers with concurrent fade/push takes — previously
crashed within seconds of dragging; now survives with the API healthy.
Note: volume_ramp.rs still uses add/remove on the volume element (same
race class, lower frequency) — migrate separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
srperens
changed the title
…ions, zone-aware editor Catch the guide up with the branch: full transition palette (slide/push directions + dip_to_black) with mixed-aspect behavior, the broadened PiP downgrade rule, the composited zone-border semantics (outside the picture edge, stacks like framed cards, fades with its box), zone-aware source chips, crop-follows-zone, layout presets, and the PiP composition export in the controls reference + glossary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, underlay churn Three fixes from the pre-merge review pass: - CPU underlay sources always produce RGBA instead of inheriting the forced output_format: an alpha-less format (I420/NV12) silently dropped the alpha of #RRGGBBAA border colors (opaque on CPU, correct on GPU), and exotic forced formats risked failing videotestsrc negotiation. The compositor's convert pads handle per-pad conversion. - TRANSITION_FOREGROUND_ZORDER raised 50 → 60: the doubled zone z-order scheme tops out at 21 + 2·14 + 1 = 50 on a multiview tile at MAX_PIP_OVERLAYS, exactly colliding with the old lift base — a pad lifted during a concurrent morph could swap stacking order with a fully-loaded zone's top source. Lifted ceilings still clear DSK (100) on dist and the overlay (200) on the multiview. - Underlay videotestsrc framerate 30 → 5 fps (the color only changes on border edits; the mixer keeps compositing the latest buffer between pushes), and the preset localStorage parse hoisted out of the per-PiP panel loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch grew from "draw zone borders" into a small architecture arc — the cairo-drawing approach was built, shipped, hardened, and then replaced once its structural limits showed in practice. The final state:
Zone borders as compositor underlay pads
A zone border is a solid-color compositor pad sitting in z-order directly beneath its content pad, sized to the content rect inflated outward by the region-scaled border width (
gst/underlay.rs). Compared to the cairo-overlay approach this branch started with:overlay_zorder + 2k + 1, underlay one below), so an overlapping higher zone covers a lower zone's border like a stacked framed card.Per-PiP layout presets
GET /api/flows/{id}/blocks/{bid}/pip/{idx}exports one PiP's live composition (restore = existingPUTon the same path). The vision mixer UI gets a localStorage-backed preset manager (save/load/delete named layouts; full composition incl. sources, bg and retained crops; best-effort load silently skips input indices that don't exist on the target mixer).Transition fixes + full picker
Use-after-free fix in control binding handling
Rapid PiP updates (zone dragging) segfaulted in
gst_object_sync_values: GStreamer iterates the bindings list unlocked (upstream "FIXME: this deadlocks", no fix in master) whileremove_control_bindingfrees nodes from the API thread — confirmed via core dump (videocrop syncs per buffer). Newgst/control_bindings.rsmodule establishes the persistent-binding protocol: one binding per (object, property), attached once, never removed; clearing wipes the control source's keyframes instead. All former add/remove sites migrated. (volume_ramp.rscarries the same pattern at lower frequency — separate follow-up.)UI polish
Source chips act as checkboxes for the selected zone (dashed = taken by another zone, click moves it here); selecting a zone points Crop/Zoom at its first source.
Test plan
🤖 Generated with Claude Code
Review pass (pre-merge)
A 7-angle review of the full branch diff surfaced and fixed (92b89bd): CPU border alpha vs forced output_format, a z-order ceiling collision at MAX_PIP_OVERLAYS, and underlay buffer churn (30→5 fps).
Deferred follow-ups (cleanup, non-blocking):
UnderlayCtxconstructions into shared helpers.active_transitionskeep-alive map is dead weight after persistent bindings (the binding owns its control source) — remove it.cloneZone()helper in vision-mixer.html (zone deep-copy duplicated ×4) and a sharedUnderlayTargetbuilder (duplicated between pads_for_source and apply_pip_layout_to_region).volume_ramp.rsindependently implements the same persistent-binding protocol (withnew_absolute) — unify withgst::control_bindingswhen touched.