Repository navigation
Conversation
A WHIP session asked its publisher for a keyframe only until video first decoded. After that, nothing asked again. A browser sends a keyframe only when asked, so a loss the jitterbuffer could not repair left the slot's decoder broken for the rest of the session: smeared pictures, or every frame rejected with an error flag, until the watchdog tore the session down and the guest rejoined. The slot's decodebin now reports damage through a per-slot flag: - a gap: the depayloader marks the first access unit after a sequence gap DISCONT; damaged until a keyframe with no gap in front of it leaves the depayloader (a repair keyframe can lose a packet too). - a silent decoder: data lost with no gap (the depayloader drops a fragment it cannot place) makes the decoder reject every frame without a bus message. 30 access units in with no picture out counts as damage, cleared when pictures come out again. Both are buffer probes that touch atomics only. A per-session thread on the session side reads the flag and sends the keyframe request on the whipserversrc pad, where it reaches the publisher: at once, then at most once a second, 10 times per damage episode, and never on healthy video. Each episode logs its recovery time and request count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wagenet
marked this pull request as ready for review
October 2, 2026 02:18
This was referenced Oct 2, 2026
Collaborator
|
Written by Claude (AI) on behalf of @srperens. This fix is now part of #957, which consolidates the WHIP Input fixes into one series against current main. You are the author. Only the conflicts with #756 and #901 were resolved: a restarted Suggest closing this once #957 merges. |
Collaborator
|
Closing in favour of #957, which carries this work (see the comment above). |
srperens
added a commit
that referenced
this pull request
Oct 2, 2026
… and #943 (#957) * fix(whip): stop an unstamped buffer at the session bridge before qtmux sees it A WHIP session's appsink -> appsrc bridge forwarded buffers with no PTS. A recorder downstream answers one with "Buffer has no PTS" from qtmux and GST_FLOW_ERROR, which takes the whole flow down: on a live rig, a second participant joining killed the first. The slot bridge now goes through gst::pipeline_bridge::SessionBridge: a buffer with no PTS is dropped and counted (warned on the first and every hundredth), the shared A/V offset is set once per session with compare_exchange and taken from the first stamped buffer, DTS is shifted with PTS, and a sample without caps no longer panics. From #759 (cfc4518, f62e777, 9ba9941), adapted to main: - The slot callback keeps the session_finished gate from #753 and the ingress liveness stamp from #756, wrapped in install_slot_bridge so the wiring test drives the real callback. - The Media Player half of #759 is left out. #945 replaced that bridge's timing with its own, so SessionBridge no longer fits there. Co-Authored-By: Per Enstedt <per.enstedt@eyevinn.se> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(whip): keep a slot's video PTS moving forward frame to frame A WHIP publisher joining a slot can take the whole flow down with a GPU vision mixer downstream: "Need timestamped buffers!" from videoaggregator. The session's jitterbuffer can release a run of packets from different frames on one PTS; h264parse takes a PTS only when it differs from the previous one, so the later frames of the run leave it with none, and a browser stream has no framerate to derive one from. A video packet that starts a new RTP frame and whose PTS is at most 1 ms behind the previous frame's now gets that PTS plus 1 ns. Packets of one frame keep sharing a PTS, and any other PTS (a B-frame, a step in the sender's clock, a frame far ahead) passes through. On the rig, 5 of 8 runs of publish / switch camera / leave / rejoin died before, 0 of 8 after. From #893 (745ce02, e90f52b, 2540522), adapted to the SessionBridge from #759: the ordering runs on the session PTS before the bridge rebases it, and copies the buffer only when the PTS moves. Co-Authored-By: Per Enstedt <per.enstedt@eyevinn.se> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(whip): stop a session's second stream stalling below PLAYING A WHIP session pipeline is already PLAYING when whipserversrc adds its pads, and each pad gets a `tee -> fakesink + appsink` branch synced into it. The appsink ran with `GstBaseSink`'s default `async=true`, so it answered that state change with ASYNC and waited for a preroll. The pads arrive one after the other within a millisecond, so the second branch lands while the first is still waiting, and the second is left below PLAYING: it takes a single buffer and then blocks its streaming thread for good. That seat's audio or video then never reaches the slot's appsrc, so its decodebin exposes no pad, and the flow's recorder and mixer wait on a track that will never arrive. Nothing in the log says so — `sync_state_with_parent` reports the state change as accepted either way. Turn `async` off, as the fakesink beside it already does. Measured on a five-seat meeting rig, 18 fresh-server trials per arm: 7/18 seats failed to start before, 0/18 after. The branch construction moves into `attach_session_branch` so the regression test builds the real thing rather than a copy of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(thumbnail): turn off async on the thumbnail tap appsink The thumbnail branch is attached to a flow pipeline that is already running, and its appsink ran with GstBaseSink's default async=true, so it answered that state change with ASYNC and waited for a preroll. The activation guard admits the pipeline in PAUSED with PLAYING pending, which is that wait, and a branch attached inside one is left below PLAYING: it takes a single buffer and blocks its streaming thread. 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. Same defect and same fix as the WHIP session branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(whip): give a reused slot a fresh video decoder A WHIP Input slot keeps its decode chain while publishers come and go. The video decoder that decodebin plugged for the first session kept that session's reference frames, reorder queue and configuration, and a later session with equal caps sends none to reconfigure it. On macOS, VideoToolbox then decoded a second Safari publisher at a few frames per second and flagged every frame of a third as an error, so the slot showed no video while its audio played. When a session claims a slot whose video decodebin has already run, restart that decodebin from an IDLE probe on the slot appsrc: unlink, set it to NULL, relink so the sticky events are sent again, and sync it with the pipeline. The existing pad-added handler links the new chain when data arrives. Audio keeps its running chain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(whip): guard a codec change on a reused slot The test reuses a slot with a VP8 publisher after an H.264 one and asserts the VP8 session decodes. With the restart disabled, the kept decoder refuses the VP8 caps and the slot's source stops with not-negotiated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(whip): ask for a keyframe when a running session's video breaks A WHIP session asked its publisher for a keyframe only until video first decoded. After that, nothing asked again. A browser sends a keyframe only when asked, so a loss the jitterbuffer could not repair left the slot's decoder broken for the rest of the session: smeared pictures, or every frame rejected with an error flag, until the watchdog tore the session down and the guest rejoined. The slot's decodebin now reports damage through a per-slot flag: - a gap: the depayloader marks the first access unit after a sequence gap DISCONT; damaged until a keyframe with no gap in front of it leaves the depayloader (a repair keyframe can lose a packet too). - a silent decoder: data lost with no gap (the depayloader drops a fragment it cannot place) makes the decoder reject every frame without a bus message. 30 access units in with no picture out counts as damage, cleared when pictures come out again. Both are buffer probes that touch atomics only. A per-session thread on the session side reads the flag and sends the keyframe request on the whipserversrc pad, where it reaches the publisher: at once, then at most once a second, 10 times per damage episode, and never on healthy video. Each episode logs its recovery time and request count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(whip): guard that a reused slot reports its video as decoding The keyframe requester stops once the slot's video_decoding flag is set, and that flag is set from decodebin's pad-added. Before the decoder restart on reuse, pad-added fired once per flow, so every reconnecting publisher was asked for the full request budget while its video decoded fine (#805). The restart brings pad-added back for each session. The reuse test now re-arms the flag as a new session does and asserts the second session sets it again. With the restart disabled it fails with "the second session decoded, but the slot never reported its video as decoding". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Peter Wagenet <peter.wagenet@gmail.com> Co-authored-by: Claude Opus 5.5 <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
2 participants
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.
Written by Claude (AI) on behalf of @wagenet.
A WHIP session asks its publisher for a keyframe only until video first decodes. After that, a loss the jitterbuffer cannot repair (an outage longer than its 400 ms, a retransmission that came too late) leaves the slot's decoder broken for the rest of the session. A browser sends a keyframe only when asked, so the picture stays smeared, or every frame is rejected, until the guest rejoins.
The slot's decodebin now raises a per-slot damage flag, and a thread on the session side asks for a keyframe while it is up.
max-errorsis -1). 30 access units in with no picture out counts as damage, cleared when pictures come out again.GstForceKeyUniton the session'swhipserversrcpad, where it becomes a PLI/FIR to the publisher; sent from the main pipeline it would die at the slot'sappsrc. At once, then at most once a second, at most 10 per episode, never on healthy video. Each episode logs its recovery time and request count.Both detectors are buffer probes touching atomics only. The repair thread holds the pad weakly and ends with the session's cleanup flag.
Rig results
Receiver-side loss injected on the session's
nicesrc(a measurement-only patch, not in this PR), fresh Strom per trial, arms alternated, macOS (vtdec), load average 2–24. The random-loss rows count only runs with both detectors in place; the last batch ran on this head.Tests
New:
whip_video_damage_test(3 tests, real slot chain frombuild_whipserversrcfed H.264 RTP with packets removed) and 9 unit tests ingst::keyframe_requestfor the flag and the request policy. Every one fails when its behaviour is reverted:a_lost_packet_damages_...: timed out waiting for the loss to damage the slota_lost_packet_damages_...: timed out waiting for the keyframe to repairhealthy_video_never_damages_the_slotasserts the flag never rose)a_keyframe_that_lost_a_packet_does_not_repair_the_sloton vtdec and avdec_h264; unita_keyframe_with_a_gap_in_front_does_not_repairRun locally on macOS with
STROM_REQUIRE_GST_PLUGINS=1:--lib whip,--lib keyframe_request,whip_video_damage_test(16/16 repeated runs: 8 undertaskpolicy -b, 8 withavdec_h264forced as on Linux CI),pipeline_lifecycle_test,whip_idle_slot_test,whip_slot_caps_reuse_test,whip_whep_ice_transport_policy_test,openapi_test. The slot test needsx264enc,rtph264pay/depayand an H.264 decoder, all in CI's package list. Not run: the full suite, Linux, Windows.Not verified
#805 touches the same import line in
keyframe_request.rs; the conflict is one line.🤖 Generated with Claude Code