fix(whip): flush a released slot's appsrc so a new session gets none of the old one's frames

[srperens]

Closes #914
Refs #906 (related, not fixed)

Problem

When a WHIP Input slot's chain falls behind, its appsrc queues the publisher's frames, up to APPSRC_MAX_TIME. If the publisher leaves with frames queued and the next session claims the slot, those frames drain into the new session. They reach the decoder restarted for it and come out first, with the previous session's timestamps. They also set video_decoding, which stops the new session's keyframe requester before its publisher has sent anything. If the queue holds H.264 and the new publisher negotiated VP8, the slot fails not-negotiated.

Change

• The release flushes the slot's appsrcs. release_slot flushes both appsrcs with a flush-start, then a flush-stop (reset_time = false). A temporary probe on the appsrc's src pad stops the flush events, so they never reach the decoder or the flow's consumers downstream of the slot's tee. The probe returns Handled, not Drop, for the same reason as fix(pipeline): drop sink QoS events with Handled, not Drop #1011: with Drop, GStreamer before 1.26 unrefs NULL and logs a critical.
  • The flush runs at release, not at the claim. WHIP Input: a reused slot's new session receives frames queued by the previous one #914 records that flushing inside the claim's restart broke VP8-after-H.264 reuse: flush-stop restarts the appsrc's task, which renegotiates with the last caps. At release, that renegotiation reaches the old decoder, which already has those caps.
• One ordered work queue per input (SlotInputWork). The release's flush and the claim's decoder restart share it, so they never run concurrently. Work that piles up while the pad is busy runs together, flush first.
• The work runs on a short-lived thread of its own, behind a real block. An IDLE probe on the appsrc's src pad starts the worker, installs a BLOCK_DOWNSTREAM probe and hands the block's id to the worker. The worker runs the work and then removes the block. The approaches rejected along the way:
  • Running the work inside the IDLE callback (the first version). The callback can fire on the appsrc's streaming thread right after an event push that basesrc makes holding its live lock. The flush-start takes that same lock and the slot's streaming thread hangs. Fired from add_probe on an idle pad, the callback also runs on the caller: a tokio worker in release_slot that holds the endpoint's slot lock.
  • Keeping the IDLE probe installed (returning Ok) to block the pad while a worker runs (the second version). An IDLE probe only holds the pad while its callback runs. The appsrc pushed on during the decoder restart, a push landed between the unlink and the relink, failed not-linked, and stopped the slot's input for good. The kept probe was also called again after every push, which started a second worker that removed the same probe twice (a GLib critical).
  • A mutex could deadlock against the stream lock that the streaming thread holds.
• The decoder restart is queued on the video input directly. Only the video decoder is restarted on reuse. Looking its input up through the decoder's current peer skipped the restart silently when that peer was missing.
• The restart clears the appsrc's reconfigure flag after relinking (check_reconfigure). Without it, the appsrc's next loop would renegotiate with the previous session's caps and configure the fresh decoder for them before the new publisher's caps arrive.

Evidence

All new tests are in backend/tests/whip_slot_flush_test.rs. They build the slot through the real WHIPInputBuilder and drive allocate_slot and release_slot.

• Queued frames no longer reach the next session: a_new_session_gets_none_of_the_frames_its_predecessor_left_queued. The consumer is slowed to 10 fps, the slot is released with about 40 frames queued, and the new session sends nothing. The test counts both the buffers that leave the appsrc and the frames that leave the slot's tee after the claim.
  • On main, 5 of 5 runs failed: 36 of the previous session's frames (of 41 queued) came out as the new session's output.
  • With the release flush disabled on this branch, 10 of 10 runs failed: 36 of the previous session's frames (of 40 queued) left the slot's appsrc after the next session claimed it.
  • With the fix, it passed 30 of 30.
• A claim while the appsrc is pushing keeps the slot running: a_claim_while_the_input_is_pushing_keeps_the_slot_running. On the IDLE-only worker version, 10 of 10 runs failed: the slot's input stopped after the claim: 4 frame(s) in 3 s. With the fix, it passed 20 of 20.
• No GLib criticals from the probe ids: none in 30 runs of the flush test with the fix. The IDLE-only worker version logged pad ... has no probe with id 1.
• No GStreamer criticals from the flush: the flush test now counts them. The first CI run of this PR, still with Drop, logged 8 gst_mini_object_unref: assertion 'mini_object != NULL' failed in whip_slot_flush_test and 8 in whip_slot_decoder_reuse_test. That run was on Ubuntu 24.04 with GStreamer 1.24.2, one critical per flush event. Locally (GStreamer 1.26) there are none either way, so the CI run on the Handled commit is the check.
• Unverified: check_reconfigure. No test fails without it. Removing it left whip_slot_decoder_reuse_test, whip_slot_caps_reuse_test and whip_slot_flush_test passing 15 of 15. It was earlier suspected of causing a flaky reused_slot_accepts_a_different_codec. That test passed 20 of 20 on main and 20 of 20 on this branch, so this PR makes no claim about that flake.

Tests

Ran on macOS (arm64), GStreamer from Homebrew, rebased on current main:

• cargo fmt --all -- --check, cargo clippy --workspace --all-targets --features efp,nvidia -- -D warnings: clean (pre-commit hook).
• cargo test --lib -- whip state::: 81 passed.
• STROM_REQUIRE_GST_PLUGINS=1 cargo test --test <t> for every backend/tests/whip_*.rs and pipeline_lifecycle_test: all passed. The counts were slot_flush 2, idle_slot 3, session_branch 2, slot_caps_reuse 3, slot_decoder_reuse 2, video_damage 3, whip_whep_ice_transport_policy 9 and pipeline_lifecycle 4.
• Repeat counts are in Evidence. No extra CPU load was applied.
• Not run: Linux and Windows locally. The flush test needs x264enc, avdec_h264 and nicesrc, which CI's Linux job installs (gstreamer1.0-plugins-ugly, gstreamer1.0-libav, gstreamer1.0-nice) and runs with STROM_REQUIRE_GST_PLUGINS=1.

For the reviewer

• The flush test pins the software decoder. It sets the vtdec ranks to NONE. VideoToolbox outputs decoded frames from a thread of its own, so the few frames it already holds at the release keep draining until the claim restarts the decoder. That made the output count flaky on macOS: 19 of 20 runs passed with the fix. That latency is the decoder's, not the appsrc backlog this PR removes.
• The output count alone did not guard the fix with the software decoder. On the second version the restart killed the input, so nothing leaked either way. The appsrc-level count is the part that fails on revert.
• A new short-lived thread per scheduled batch. That means at most one per release and one per claim on each input. Nothing on the hot path changes: the block probe is installed only while work runs.

Out of scope

• WHIP Input: a stalled video chain is not restarted when a new session claims the slot #906: a chain stalled downstream never goes idle, so the queued work waits, as before.
• A sample the old session's bridge was already pushing at the release can still land after the flush. SessionActivity's grace covers it.
• VideoToolbox's output queue at the release: a few frames already inside the decoder can drain until the claim's restart.

🤖 Generated with Claude Code

[srperens]

Verdict: Comment. The flush is in the right place and the right layer, the work queue serializes it correctly against the claim's restart, and both new tests ran in CI. One line has no test, and the body says so itself. The guard is cheap to add, or the line can come out.

Requested changes

1. check_reconfigure has no test. Add one to backend/tests/whip_slot_decoder_reuse_test.rs that fails without it, or remove it:
   backend/src/whip_session_manager.rs:318 — let _ = src.check_reconfigure();
   The body reports that three binaries pass 15 of 15 without it. As things stand, that line changes how every reused slot negotiates its first caps, and no test notices. If the H.264→VP8 case it targets cannot be made to fail, say so in a comment on the line.

Claims

Claim	Verdict	Evidence
The release's flush and the claim's restart never run concurrently	CONFIRMED	armed is swapped before a probe is installed. run re-arms itself before looping, so only one worker executes run at a time. A late schedule either piggybacks or arms a fresh IDLE probe
Flush events stop at the appsrc's pad	CONFIRMED	EVENT_FLUSH probe returning Handled, removed straight after the flush-stop. Check (Linux) counts 0 gst_mini_object_unref criticals in whip_slot_flush_test and whip_slot_decoder_reuse_test (125 of the job's 139 come from output_refusal_test, unrelated)
Queued frames no longer reach the next session	CONFIRMED	whip_slot_flush_test::a_new_session_gets_none_of_the_frames_its_predecessor_left_queued ... ok, binary 2 passed in 5.87 s on Check (Linux). It drives allocate_slot/release_slot on the real builder. The revert counts are the author's
check_reconfigure keeps the fresh decoder off the old caps	UNVERIFIED	See requested change 1

backend/src/whip_session_manager.rs:417 — self.schedule_input_work(slot, SlotMedium::Audio, INPUT_FLUSH);

Diagnosis. Coverage is BOUNDED, and the body states both bounds. A chain stalled downstream never goes idle, so the flush waits (#906). A sample already in flight from the old bridge can land after the flush, and SessionActivity's grace covers that. Flushing at release instead of at claim is argued from #914's own failed attempt, and the reasoning holds. The flush-stop's renegotiation reaches a decoder that already has those caps.

Lifetime. Each batch spawns one short-lived thread. Every release_slot call queues two batches, one per medium. That includes the six POST error paths in backend/src/api/whip_ingest.rs, where the pad is idle and the work is a no-op flush. The worker holds a strong Pad and an upgraded AppSrc only while it runs. The IDLE closure keeps the appsrc weak.

SPECULATIVE (not verified): if a flow stops while a worker is mid-flush, the strong refs delay finalization by that long. In that window, stop_flow's survives-drop check could log a false ERROR. The window is milliseconds.

Radius. SHARED: every WHIP Input slot, on every release (two callers in the session manager, six in whip_ingest.rs). The decoder restart moves from its own IDLE probe to the shared queue, so reuse behaviour changes for every codec. whip_slot_decoder_reuse_test and whip_slot_caps_reuse_test pass.

Overlaps. #1025 (draft) adds a WhipEndpointConfig field in the same struct and in new(), and #854 (draft) splits both files. Both will conflict textually. Land this first; it is ready and they are not. #906 stays open by design.

Tests & CI. At e8f2939: Linux, ARM64, WASM, API contract and JS are green, and pipeline_lifecycle_test 4 passed. macOS and Windows are skipped, and nothing here is platform-specific.

Confidence: HIGH