fix(whip): keep a slot's video PTS moving forward frame to frame

[wagenet]

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

Current behaviour

A WHIP publisher joining a slot, including a reconnect or a second device on a reused slot, can take the whole flow down. With a GPU vision mixer downstream, the flow stops with Internal data stream error from appsrc_overlay, queue_overlay and the slot's appsrc_video_0. The GStreamer log shows the cause:

ERROR videoaggregator ...:<vmix:mixer:sink_0> Need timestamped buffers!

On a local rig (browser publisher, same flow shape as the production that hit it, with the aspect fix #894 applied) this happened in 5 of 8 runs of "publish, switch camera, leave, second publisher joins".

Fix

When the slot bridge forwards a video packet that starts a new RTP frame, and its PTS is at most 1 ms behind the previous frame's, the packet gets the previous frame's PTS plus 1 ns. Packets of one frame keep sharing a PTS, and any other PTS passes through unchanged. With the fix, the same rig had 0 of 8 deaths.

Why frames lose their timestamp

• When a session starts, its jitterbuffer can release a backlog of packets from different frames (their RTP timestamps advance about 3000 ticks, i.e. 33 ms at 90 kHz) all carrying one and the same PTS. Captured on the rig with rtpbasedepayload:6: seqnums 22859-22882, eight RTP timestamps, one PTS 0:00:21.537999057.
• rtph264depay passes that PTS on to every access unit. h264parse (baseparse) takes an input PTS only when it differs from the previous one (gstbaseparse.c, prev_pts != pts), so every later frame of the run leaves the parser with no PTS.
• Browser H.264 carries no frame timing (framerate=0/1), so the decoder cannot interpolate one. vtdec passed PTS NONE through (videodecoder:6: chain PTS 99:99:99.999999999 for every frame after the second), and glvideomixer returns GST_FLOW_ERROR on the first frame without a timestamp.

A standalone check (x264 → RTP → appsink/appsrc bridge → rtph264depay ! h264parse), with frames 10-25 forced onto one PTS: 15 of 60 frames leave h264parse without a PTS; with the ordering rule, 0.

Only a PTS at most 1 ms behind the previous frame's is moved. A larger step back is real timing and passes through: a publisher whose encoder uses B-frames sends a frame after the one it precedes. Browsers never send B-frames, but other WHIP clients can. A frame far ahead does not hold back the frames after it.

How to review

• VideoPtsOrder and the reworked tail of forward_sample_to_slot in backend/src/blocks/builtin/whip.rs. This runs per RTP packet on the session's appsink thread: atomics and a read-only RTP header map, no lock and no allocation beyond the buffer copy the bridge already made when rebasing.
• SlotMedia replaces the media: &str argument so the video ordering state can travel with it (clippy's argument limit).
• Overlap with fix(whip): stop an unstamped buffer at the session bridge before qtmux sees it #759, which moves this function into gst::pipeline_bridge::SessionBridge and drops buffers that arrive without a PTS. This PR covers buffers that arrive with one and lose it inside the slot's decodebin; the production rig runs with fix(whip): stop an unstamped buffer at the session bridge before qtmux sees it #759 included and still hit this. Whichever lands second needs a rebase; VideoPtsOrder would move into the bridge. refactor(whip): split whip.rs and whip_session_manager.rs into sub-modules #854 (generated split) regenerates after either.
• Related draft: the vision mixer aspect fix (fix(vision mixer): keep a source's new shape when it changes size #894) reproduced alongside this and should land with it: with only that fix, the rig died in 5 of 8 runs, more often than earlier upstream runs (about 1 in 4, not tabulated).

Tests

• New whip::tests::video_frames_on_one_pts_reach_the_slot_in_order drives forward_sample_to_slot with real RTP packets. With the ordering bypassed it fails ([1000000000, 1000000000, 1000000000, 1000000000, 1100000000]); with it, it passes.
• New video_b_frames_keep_their_pts and video_frame_far_ahead_does_not_hold_back_later_frames fail if every late frame is moved past the previous one (the rule without the 1 ms bound).
• cargo test --workspace on this branch, macOS: 1063 passed, 0 failed.
• Rig: 8 alternating runs each, fresh server per run, three-shape sequence (1280x720 → switch to 720x1280 → leave → 640x480 publisher). Both arms had fix(vision mixer): keep a source's new shape when it changes size #894 applied: without this fix 5 of 8 flows died, with it 0 of 8. The rig is Chrome publishing canvas tracks through the real /player/whip-ingest page. Not run: a phone over cellular.

🤖 Generated with Claude Code

[srperens]

Verdict: Approve — the mechanism is traced end to end (one PTS across several frames, then baseparse keeps only a PTS that changed). The fix is bounded to that one shape, and B-frames and outliers keep their timing. The three new tests drive the real forward_sample_to_slot and all three ran in CI. It changes the same function as #759; either order works, see Overlaps.

Non-blocking notes

1. The RTP read fails silently: backend/src/blocks/builtin/whip.rs:1361 — adjusted = order.order(rtp.timestamp(), adjusted); runs only if the header maps. That is right as long as the bridge carries RTP, which the rig and decode=false's own description (outputs RTP (application/x-rtp)) both say it does. But backend/src/blocks/builtin/whip.rs:1201 — // whipserversrc autoplugs RTP depayloaders inside its own bin, so this says the opposite. If that comment is stale, correcting it here would stop the next reader doubting this fix. If an upstream change ever depayloads before the bridge, the fix turns into a silent no-op.
2. The body says the packets of one frame "keep sharing a PTS". The code enforces that: backend/src/blocks/builtin/whip.rs:1271 — if last_rtptime == u64::from(rtptime) { returns the first packet's PTS for every later packet of the frame. With rtpbin's default slave mode these already match, so in practice nothing changes. It is still a behaviour worth one line in the doc comment.

Claims

Claim	Verdict	Evidence
Only a step back of at most 1 ms is moved; B-frames and far-ahead frames pass	CONFIRMED	backend/src/blocks/builtin/whip.rs:1275 — last_rtptime != u64::MAX && pts <= last_pts && last_pts - pts <= MAX_PTS_NUDGE_NS;, and the tests video_b_frames_keep_their_pts and video_frame_far_ahead_does_not_hold_back_later_frames pin both sides
The regression test fails with the ordering bypassed	CONFIRMED	video_frames_on_one_pts_reach_the_slot_in_order asserts out[2] > out[1] for packets (4000, 1000) after (1000, 1000). Without order() both leave at 1000 ms
No lock and no new allocation on the per-packet path	CONFIRMED	Relaxed atomics plus a read-only RTPBuffer map. The copy happens only when the PTS changed, as it already did whenever offset_ns != 0
baseparse ignores a repeated PTS; vtdec/glvideomixer fail on PTS NONE	EXTERNAL	upstream behaviour, taken from the body's traces

Diagnosis — Right layer. The bridge is the last point that still sees RTP timestamps before h264parse loses the PTS. Deriving PTS from RTP-time deltas would be the obvious alternative, and it is worse: a backlog released at once would be stamped ahead of real time, and the frames after it would then step backwards. The +1 ns nudge avoids that. Coverage is BOUNDED to runs of at most ~1e6 frames on one PTS, which in practice is every run.

Radius — LOCAL: the video bridge of every WHIP Input slot, codec-agnostic, for decode=true and decode=false. Audio is untouched. VideoPtsOrder is created per session pad, so a reused slot starts clean.

Overlaps — #759 (Approve at 9ba9941) changes the same code path: it moves forward_sample_to_slot into SessionBridge. Landing #759 first is the smaller rebase, because VideoPtsOrder moves as one unit. #854 (draft) regenerates after either. #901 restarts the slot decodebin, which is a different mechanism: a fresh h264parse still drops the repeated PTS.

Tests & CI — All Linux, WASM and API checks green at e90f52b. The job log shows all three new whip::tests::video_* tests ok, and pipeline_lifecycle_test ran. macOS/Windows skipped; nothing here is platform-specific.

Confidence: HIGH

[srperens]

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. Your commits are in it with you as author.

Adapted: VideoPtsOrder now runs on the session PTS before #759's SessionBridge rebases it, and copies the buffer only when the PTS actually moves. The bridge offset is constant, so the result is the same. All three of your tests are in, and each fails with its behaviour reverted. #894 is on the integration branch next to it for a rig run.

Suggest closing this once #957 merges.

[srperens]

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