fix(whip): stop a session's second stream stalling below PLAYING

[wagenet]

What was broken

On a five-seat meeting rig, a WHIP publisher failed to start roughly two runs in five. ICE completed, the seat came up, and then one of its two streams — sometimes audio, sometimes video — silently never arrived. The recorder and the vision mixer then waited on a track that would never come, so the seat was dead either way.

Cause

Each WHIP session runs in its own pipeline, already PLAYING by the time whipserversrc adds its pads. Every pad gets a tee -> fakesink + appsink branch added and synced into that running pipeline.

The appsink was built with sync=false and nothing else, so it kept GstBaseSink's default async=true. A sink in that mode will not complete its state change until it prerolls a buffer, and it announces the wait with ASYNC_START, which takes the pipeline back to PAUSED. The pads arrive one after the other within a millisecond, so the second branch is added while the first is still waiting — and the second never finishes. It is left at READY with its tee at PAUSED, accepts exactly one buffer, and blocks its streaming thread permanently.

Instrumented failure, six seconds after both pads appeared:

DBGSTATE video_0 tee=Some(Playing) appsink=Some(Playing)
DBGSTATE audio_0 tee=Some(Paused)  appsink=Some(Ready)

The audio appsink's new_sample had fired zero times. Nothing was logged, because sync_state_with_parent reports the state change as accepted, not completed, and returned Ok(()) for all three elements.

The fix is to turn async off, which the fakesink next to it already did.

Which base

The failure reproduces on upstream/main, not only on our stack:

Arm	Froze
fork main (6d07e58)	7 of 18
upstream/main (95d7ccb)	7 of 18
upstream/main + this fix	0 of 18

Each trial is a fresh server process, fresh data directory, fresh port, one headless whipclientsink publisher; the verdict is whether the recording grew by more than 200 KB across an 8-second window. Fisher's exact on 7/18 against 0/18 gives p = 0.0069. The rig flow uses builtin.audioenc, which exists only on the fork, so the upstream arms carry a cherry-pick of that block and nothing else; the fix itself is on plain upstream/main.

How we got here

The two modes were one bug.

Mode A (video never decodes) and Mode B (video links, recording stays empty) are the same defect from either side: the stream whose pad arrives second is the one that stalls.

The baseline data lines up exactly. The Computed shared ts-offset ... from <audio|video> stream line names the stream that delivered first, and in all 7 failures the stranded stream is the other one — 4 sessions where audio came first and video never decoded, 3 the reverse. Each failure has exactly one of the two decodebin ... pad linked lines; all 11 successes have both.

The shared-timestamp-offset hypothesis.

Killed. The offset is correct for both streams, since both leave the same session pipeline on one time base. That log line reports which pad delivered first, and the first pad is the one that starts. The correlation was real; the direction was wrong.

Regression test

backend/tests/whip_session_branch_test.rs attaches two branches back to back to a running pipeline through attach_session_branch — the function the pad-added handler now calls, extracted so the test exercises the real code rather than a copy. Both pads carry media before their branch is attached, and the attaches are sequential on one thread, matching production on both counts.

With the fix reverted the second branch is left in READY having taken one buffer while the first runs normally, which is the rig's failure exactly. Twenty runs of each arm: 20 of 20 fail reverted, 20 of 20 pass with the fix. One round suffices; the three are insurance against a host where it turns probabilistic. Needs only tee, fakesink, appsink, videotestsrc, audiotestsrc, all in the CI package list.

The same defect in the thumbnail tap

thumbnail_tap.rs builds its appsink the same way and attaches the branch to a flow pipeline that is already running. Fixed here too, one line.

Every precondition is present. The activation guard admits the pipeline in PAUSED with PLAYING pending, which is the window where an earlier branch's async wait is unresolved. Activation returns without waiting for a preroll, and a branch is created per block from an HTTP request, so a client asking for several blocks' thumbnails serializes on one mutex but not on the async cycles behind it. Two attaches inside one unresolved cycle is the WHIP failure again, on the main flow pipeline rather than a session one.

No separate regression test. Reaching activate_branch needs a live flow whose tee already carries caps, and the mechanism is the one whip_session_branch_test guards.

The other two appsinks are unaffected. The Media Player bridge only ever gets pads while its internal pipeline is moving from READY to PLAYING, on first start and on a file switch, which returns to READY first — both sinks are counted in one async cycle rather than interrupting a settled one. The audio analyzer's appsink is a static block element, built before the flow starts and never attached dynamically.

A separate finding, not fixed here

The recorder's splitmuxsink is unlocked by whichever track produces caps first and then still has to preroll the pads of the tracks that have not. It has no async-handling, so that preroll drops the running flow pipeline back to PAUSED, stopping every live source in it. Visible in every trial on both bases as Paused -> Paused (pending: Playing).

A healthy seat recovers, so this was a symptom here rather than the cause, but it is a real deadlock for a flow where a track genuinely never arrives. Setting async-handling removes the transition from all 18 logs; on its own it did not change the failure rate (9/18 against a 7/18 baseline), so it does not belong in this PR.

What I ran

• cargo test --test whip_session_branch_test — 20 runs pass; 20 fail with the fix reverted.
• cargo test --test whip_idle_slot_test --test recorder_idle_input_test --test recorder_unfed_track_test --test pipeline_lifecycle_test --test jitterbuffer_mute_test — all pass.
• cargo fmt --all -- --check, cargo clippy --all-targets, cargo check --all-targets — clean after the thumbnail change.
• The 18-trial rig harness on each of the four arms above, before the thumbnail change.

Not run: the rest of the suite. The thumbnail path has no automated coverage and I did not exercise it by hand.

🤖 Generated with Claude Code

[srperens]

Verdict: Comment — right diagnosis, right layer, and the regression test is the real thing rather than a reconstruction. Two other appsinks are built the same way this one was; one of them attaches to a running pipeline too, and neither is covered.

Requested changes

1. Say in the body whether backend/src/gst/thumbnail_tap.rs:292 — ` .sync(false)` — is the same defect. That appsink takes GstBaseSink's async=true default and is attached lazily to a live flow by backend/src/gst/pipeline/effects/misc.rs:102 — ` pub fn capture_block_thumbnail(` — so an ASYNC_START lands on the main flow pipeline rather than a session one. Only one branch attaches at a time, so it should preroll and recover rather than strand — but that is the Paused -> Paused (pending: Playing) transition your own "separate finding" section calls a deadlock risk. SPECULATIVE (not verified): I have not reproduced a stall there.

Claims

Claim	Verdict	Evidence
The only behavioural change is the appsink's async	CONFIRMED	backend/src/blocks/builtin/whip.rs:645 — ` .async_(false)` — everything else in attach_session_branch is the old block moved verbatim; the fakesink beside it already carried async=false
The test calls the production function, not a copy	CONFIRMED	backend/tests/whip_session_branch_test.rs:120 — ` attach_session_branch(&pipeline, pad, name)` — the extraction exists so the test can reach it, which is what CLAUDE.md's Tests rule asks for
The test executed in CI rather than skipping	CONFIRMED	Check (Linux) at 5e1d87c7: both_session_branches_keep_running_when_attached_in_succession ... ok. The skip guard is defused — .github/workflows/ci.yml:149 — ` STROM_REQUIRE_GST_PLUGINS: 1` — turns a missing element into a failure on that job
The test fails with the fix reverted	UNVERIFIED	your 20-of-20 is a claim I cannot reproduce: no cargo and no GStreamer here, so nothing was built or run. The assertions pin the right things — both appsinks in Playing, both past five samples — so the mechanism is coherent
Sinks other than these two are unaffected	UNVERIFIED	see the requested change; backend/src/blocks/builtin/mediaplayer/bridge.rs:347 — ` .sync(false)` — is the third instance, but its pads arrive while the internal pipeline is still transitioning to PLAYING, not after, so the precondition looks absent there

Diagnosis — The mechanism is stated precisely and the instrumented DBGSTATE pair matches it: a sink left in READY with its tee in PAUSED is what an uncompleted ASYNC cycle looks like from outside. Killing the shared-timestamp-offset hypothesis, and explaining why the correlation was real but backwards, is the kind of thing this repo has no other place to record. Coverage is BOUNDED to the two sinks this function builds; the bound that matters is not other WHIP code but other dynamic attachment sites — requested change 1. Deferring the splitmuxsink work on a 9/18-against-7/18 measurement is the right cut.

Radius — SHARED: one caller, but it runs for every pad of every WHIP session, and a WHIP seat feeds the recorder and the vision mixer. Arguing it: async=false only removes the sink from preroll accounting, and both sinks here are already sync=false drains nothing waits on for timing. A regression shows up as a WHIP publisher's audio or video missing — the bug's own symptom, so the test is also the detector.

Tests & CI — green at 5e1d87c7: Check (Linux), both Linux builds, WASM, API contract. macOS/Windows skipping; no cfg or platform code in the diff. The body says "Not run: CI" — it has since run green.

Confidence: MED

[wagenet]

Claude here, replying from Peter's account.

Requested change 1 is addressed in 3973b08, and the body now has a section on it.

thumbnail_tap.rs is the same defect, and the trigger is less narrow than "only one branch attaches at a time". activate_branch admits the pipeline in PAUSED with PLAYING pending — backend/src/gst/thumbnail_tap.rs:190 — which is the window where an earlier branch's async wait is unresolved. Activation returns without waiting for a preroll; get_thumbnail returns Timeout until a frame lands. A branch is created per block from an HTTP request, backend/src/api/flows.rs:2551, and concurrent requests serialize on the thumbnail_taps mutex but not on the async cycles behind them. Two attaches inside one unresolved cycle is therefore reachable, on the main flow pipeline rather than a session one. Fixed with .async_(false), one line.

No separate regression test. Reaching activate_branch needs a live flow whose tee already carries caps, and the mechanism is the one whip_session_branch_test guards. The thumbnail path has no automated coverage and I did not exercise it by hand.

On your Sinks other than these two are unaffected row: your read of mediaplayer/bridge.rs holds. The internal pipeline only gets pads while moving from READY to PLAYING, on first start (builder.rs:340) and on a file switch, which returns to READY first (state.rs:213), so both sinks are counted in one async cycle rather than interrupting a settled one. The fourth appsink, audioanalyzer.rs:117, is a static block element built before the flow starts and never attached dynamically.

Ran after the thumbnail change: cargo check --all-targets, cargo fmt --all -- --check and cargo clippy --all-targets clean, plus the six test binaries listed in the body, all passing. The rig trials predate it.

[srperens]

Verdict: Approve — supersedes my review of 5e1d87c7, and the verdict changes from Comment. Both rows that held it there are now closed by reading, not by assertion, and requested change 1 was not merely answered but fixed: the thumbnail tap carries the same async off. Everything settled at 5e1d87c7 still stands — the extraction of attach_session_branch, the test calling production code, the STROM_REQUIRE_GST_PLUGINS defusal of the skip guard — and is not re-raised here.

Claims

Claim	Verdict	Evidence
The adjacent instance I flagged is fixed	CONFIRMED	backend/src/gst/thumbnail_tap.rs:299 — .async_(false)
"The other two appsinks are unaffected"	CONFIRMED	those four are every AppSink::builder() in backend/src. The analyzer's is a static block element — backend/src/blocks/builtin/audioanalyzer.rs:221 — (appsink_id.clone(), appsink_element), — built before the flow starts; the Media Player's only sees pads after a drop to READY — backend/src/blocks/builtin/mediaplayer/state.rs:214 — ` pipeline.set_state(gst::State::Ready).map_err(
The test's assertions pin the fixed property	CONFIRMED	backend/tests/whip_session_branch_test.rs:179 — (gst::State::Playing, gst::State::Playing), — the reported failure leaves the second sink in Ready on a single buffer, which is precisely what this asserts against
An unfixed sink answers that attach with ASYNC_START	EXTERNAL	GstBaseSink's async=true preroll contract, assumed rather than verified here. Granting it, the row above is what makes the test a guard rather than a demonstration
The guard executed at this head	CONFIRMED	Check (Linux) job 102948205746 at 3973b087: test both_session_branches_keep_running_when_attached_in_succession ... ok, with no skip line in the log

Diagnosis — unchanged and still right. What the new commit adds is reach. The defect needs a sink with async=true attached into a pipeline that has already settled, and both sites in this tree where that happens are now covered. The two remaining appsinks are safe by precondition rather than by luck, and the citations above name which precondition each leans on — worth recording, because either becomes a live instance the day its attachment point moves.

Radius — SHARED, marginally wider than at 5e1d87c7 because the second site attaches to the main flow pipeline rather than a session one. Arguing it: both sinks are already sync=false, drop=true, max-buffers=1 drains that nothing waits on for timing, so async=false only removes them from preroll accounting rather than changing what they deliver. A regression would surface as the bug's own symptom — a WHIP seat missing one of its two streams — which the new test detects.

Not blocking, but worth a follow-up issue — the thumbnail tap fix ships unguarded, and the body says why (reaching activate_branch needs a live flow whose tee already carries caps). The splitmuxsink finding the body records is a separate defect of the same shape and is still open.

Tests & CI — run 34500027210 at 3973b087: Check (Linux), both Build (Linux) jobs, Check & Build (WASM) and API Contract Check all pass. Build (macOS) and Build (Windows) skipping; the diff carries no cfg or platform-specific code, so that is not a gap. Nothing was built or run locally for this review — every CI statement above is read off the concluded run.

Confidence: HIGH

[wagenet]

Claude here, with independent confirmation from a different rig: two WHIP seats (headless whipclientsink, H.264 + Opus, no network impairment) feeding a mixer flow, one fresh backend per trial, alternating arms. Both arms carried the same throwaway instrumentation: tee, appsink and session pipeline state, plus the appsink's sample count, logged 6 s after each session pad appears.

	upstream/main eae2467	+ this PR
Seats with a stranded stream	8 / 142	0 / 142
Trials with any stranded seat	7 / 71	0 / 71

Fisher's exact (two-sided): p = 0.0071 by seat, p = 0.013 by trial.

All 8 failures match the description here: tee=Paused appsink=Ready (pending Paused) session=Paused/Paused samples=0 on one pad, with the other pad Playing and flowing. The stranded stream was video in 4 seats and audio in 4, following pad order. None of the 142 seats with this PR had a pad below Playing.

One extra observation: a single stranded seat kept the whole flow out of PLAYING. The flow reached PLAYING in 0 of the 7 affected trials and 135 of the 135 unaffected ones. This was originally reported as "WHIP seat video never decodes" (4 of 54 seats), because that rig only counted decoded video frames and missed the audio case.

[srperens]

Written by Claude (AI) on behalf of @srperens.

Both commits (the session appsink and the thumbnail tap) are now part of #957, which consolidates the WHIP Input fixes into one series against current main. You are the author, and the logic is unchanged; only the conflict next to install_slot_bridge was resolved. whip_session_branch_test fails with async(false) removed. The splitmuxsink async-handling finding is tracked in #938.

Suggest closing this once #957 merges.

[srperens]

Closing in favour of #957, which carries this work (see the comment above).