Skip to content

fix(mpegtssrt_input): re-anchor tsdemux stamped ahead by a stale head - #813

Open
wagenet wants to merge 7 commits into
Eyevinn:mainfrom
wagenet:wagenet/srt-early-connect-timestamps
Open

wagenet wants to merge 7 commits into
Eyevinn:mainfrom
wagenet:wagenet/srt-early-connect-timestamps

Conversation

@wagenet

@wagenet wagenet commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

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

Problem

When an SRT caller has been encoding before it connects to builtin.mpegtssrt_input (typically because it started before the flow and kept retrying), its media leaves the block stamped as far into the future as the caller waited. That lasts for the whole connection, and every downstream sink and mixer holds the source back by the same amount. On the Open Live rig, a caller that waited ~1.2 s reached program at ~1460 ms instead of ~525 ms.

Cause

With ignore-pcr=true (the block default), tsdemux maps PTS to output time from a single reference: the upstream timestamp of the buffer that carried the first PES it parsed, and it never takes a new one (mpegtspacketizer.c, mpegts_packetizer_pts_to_ts_internal: "Take the first base time we see as a reference"). Its ">5 s" catch-up moves both sides of the mapping together, so it keeps any error.

A caller that waited sends a few frames from the start of its encode and then jumps to live. GST_DEBUG=tsdemux:5, sender started 2.2 s before the listener:

pts 1:00:00.029966666 base_pcrtime:1:00:00.029966666 base_time:0:00:02.480551917   <- stale frame becomes the reference
pts 1:00:02.333966666 base_pcrtime:1:00:00.029966666 ...                            <- live, placed 2.3 s later

srtsrc's own timestamps stay within ~5 ms of arrival; the error is only in the demuxer's reference. The bug reproduces without Strom: srtsrc ! decodebin measured 2448 ms end to end against 215 ms, with ~200 ms arrival delay in both runs.

Not a base_time change

The original report suspected srtsrc buffers produced before PLAYING carried a different base_time. Traced trials show a single base_time per pipeline run. The "450–900 ms" in that report was the multi-second shift above, aliased by pairing ticks sent every 1.5 s. With 4 s ticks, the penalty tracks how long the caller waited.

Fix

TsDemuxAnchor (blocks/builtin/tsdemux_anchor.rs) is wired in when ignore_pcr=true, in both decode and passthrough modes. A BUFFER probe on srtsrc:src records the latest input timestamp. A probe on each tsdemux source pad measures how far the buffer's running time is ahead of that timestamp (its lead).

Healthy lead depends on how the sender packs its stream. All PES completed within one SRT message are placed from the same reference, which gives a sawtooth of about ±50 ms for our test sender. ffmpeg's default TS muxing, with ~11 AAC frames per PES and a video burst at connect, holds audio 100–230 ms and video 290–440 ms ahead, and the smallest lead in a window has reached 205 ms (#997). When the smallest lead over a 500 ms window exceeds 500 ms, all of the demuxer's source pads get one shared pad offset of minus the window's mean lead. Audio and video move together, and the sawtooth ends up centred where a clean connect leaves it.

A stale head only happens at connect, so a correction may only be made in the first 3 s of a caller's input, and only once. After that the output probe only keeps the offset applied. A shift made mid-stream steps running time backwards, which recorders write as decreasing DTS: #997 saw a healthy stream re-anchored by about −380 ms 1–22 s into a connection, when the threshold was 200 ms and the rule had no time limit.

The offset belongs to the caller it was measured on. keep_listening is on by default, so one srtsrc serves caller after caller and tsdemux takes a fresh reference for each, which means an offset left over from the previous caller stamps the next one that far behind its arrival — where every consumer that waits for running time drops it. So the offset is dropped on srtsrc's caller-added and caller-removed, and the watcher re-arms for the next caller's first 3 s. Output that sits behind arrival is otherwise left alone: a sender that stalls and resumes looks identical from here, and giving the offset back would throw its stream forward by that much in one buffer.

The probes use buffer timestamps and atomics only: no clock reads, locks or allocation. set_offset runs only when a correction is made. PCR mode is untouched. With ignore-pcr=false, tsdemux recovers from the same stream on its own, but it costs 90–130 ms steady state and would change a default.

Evidence

The rig is the Open Live program flow: WHIP seats plus this SRT input ("C") into builtin.mixer, out over WHEP, with mix-minus returns from a second flow. Release builds, a fresh backend per trial, builds interleaved. C sends a tick every 4 s. Early: the caller starts 1 s before the flow start call. Late: the caller starts 1 s after the call returns. Each cell gives per-trial medians (ms) of C's delay at the program output / seat A's return.

arm SRT latency upstream eae2467 this PR this PR + #812
early 125 1477 / 1281, 1454 / 1266 528 / 338, 543 / 356 511 / 352, 526 / 356
late 125 526 / 338, 538 / 368 525 / 325, 523 / 344 519 / 329, 525 / 327
early 20 1352 / 1170, 1358 / 1173 430 / 271, 433 / 252 –
late 20 422 / 233, 413 / 222 423 / 239, 407 / 223 –

An earlier, non-interleaved round agrees: upstream early/125 on program measured 1434–1488 ms over 8 trials. The correction logged a 935 ms shift; it fired once in every early trial and never in a late one. These trials ran on 557d444, before the 3 s arm period and the 500 ms threshold. A caller that waited ~1.2 s produced a 935 ms correction about 1 s in, which both still allow; the rig has not been re-run on the head.

Tests

mpegtssrt_input_stale_head_test builds the real block in decode and passthrough mode and connects to it over SRT. It sends 150 ms from the start of a pre-encoded AAC transport stream, then the same stream from 2 s in, paced in real time. After two windows, no buffer leaving audio_output_0 may be 200 ms or more ahead of the latest SRT input (the test's own ON_TIME bound, kept at the old threshold rather than following MAX_LEAD up).

Two more tests in the same file run a second caller with a clean stream after the corrected one, with keep_listening=true: one where the encoder resumes and one where it restarts from zero. 300 ms after the second caller's first buffer, nothing may be 200 ms off its arrival in either direction.

All four run on every GStreamer version, CI's 1.24.2 included. On 1.24.2 the caller change re-parses the PMT and tsdemux replaces its source pad (the equality bug below); #877 relinks that pad, so the second caller now delivers audio there too.

Four unit tests in tsdemux_anchor.rs drive on_input_buffer / on_output_buffer directly and run everywhere:

  • a_healthy_stream_is_never_re_anchored: tsdemux re-anchor fires on healthy SRT inputs and steps recordings ~0.38 s backwards #997's stream shape, 440 / 230 ms leads through the arm period, then a 205 ms window at 10 s. No offset.
  • a_stale_head_is_re_anchored_once: a 2 s lead is corrected; a further 1 s lead in the next window is not.
  • no_correction_after_the_arm_period: a 2 s lead first seen 4 s into the connection is left alone.
  • a_caller_change_drops_the_offset_and_re_arms: the next caller starts unshifted, and a later caller's stale head is corrected.

The replay drops the audio between the two parts but forwards the PAT/PMT packets (the tables describing the streams) from the gap. Without them, GStreamer 1.24.2 — what Ubuntu 24.04 and CI ship — re-applies the PMT after the continuity break and, with ignore-pcr, treats it as a new program (fixed upstream in 1.24.3, "mpegtsbase: Fix Program equality check"): tsdemux replaces its source pad, the new pad was not linked before #877, and srtsrc stopped not-linked after the head. That was the first CI failure, not load — the test passed 8/8 under 14 and 40 busy loops on macOS.

Each test fails when the behaviour it guards is reverted. Run on macOS, GStreamer 1.28.6, at the head:

revert outcome
anchor not wired at all both stale-head tests fail, 2063 / 2085 ms ahead
correction not dropped when the caller changes both second-caller tests fail, −2041 ms behind on macOS; also fail on 1.24.2
reset not clearing the offset the caller-change unit test fails
MAX_LEAD back to 200 ms a_healthy_stream_is_never_re_anchored fails
no 3 s arm period no_correction_after_the_arm_period fails
not disarmed after a correction a_stale_head_is_re_anchored_once fails
caller change not re-arming a_caller_change_drops_the_offset_and_re_arms fails
correct by the window minimum instead of its mean all four still pass — the residual is inside the 200 ms assertion, so the mean is backed by the rig numbers, not by a test

Residual after correction, at the head: 25 ms decoded and 49 ms passthrough against the 200 ms limit; second caller −4 to +84 ms.

On fcd773f, in an Ubuntu 24.04 container matching CI's GStreamer 1.24.2 and libsrt 1.5.3 (arm64, not CI's x86_64): the two stale-head tests passed 3 runs idle and 5 under 32 busy loops on 16 cores, failed with the anchor disabled by 2085 / 2064 ms, and failed with the earlier replay (PSI dropped) with 0 buffers after the window — the CI failure. After rebasing onto 3225792 (which has #877), same container: all four tests passed 6/6 runs with STROM_REQUIRE_GST_PLUGINS=1, and with the caller-added/caller-removed reset removed both second-caller tests failed. That is the check that CI now guards the watch_caller wiring, not only reset().

At 6f6648a (rebased onto main, Rust 1.98.1), on macOS: all four integration tests (STROM_REQUIRE_GST_PLUGINS=1), the four unit tests, and CI's clippy command (--features efp -- -D warnings). The last four reverts in the table ran on the same change before the rebase (5d248ad); the rows above them ran on the earlier head and were not repeated. The #997 stream shape is covered only by the unit test; its ffmpeg repro is outside this repo and was not run against this commit.

Also run on macOS at 9e73d31: pipeline_lifecycle_test (the closure-leak guard, 3 passed) and cargo test --lib tsdemux_anchor. On the earlier commit: merged with #812, together with #812's mpegtssrt_absent_caller_test; mpegtssrt_streamheader_test, cargo test --lib. Clippy and fmt ran through the pre-commit hook on every commit. Not run locally: the rest of the integration suite.

The test needs plugins-bad (srtsrc, srtsink, tsdemux, mpegtsmux) and gst-libav (AAC), both already in the CI package list.

Two adversarial passes on the earlier head drove the real block through what the tests do not cover, and these held: healthy 128 kbit/s and 32 kbit/s callers are left alone (the 32 kbit/s sawtooth peaked at 85 ms); a 700 ms SRT stall and its catch-up is not mistaken for a stale head; an H.264 + AAC stale head is corrected on both pads at once, 12 ms apart; three callers in a row are each corrected independently; a second caller with its own stale head is corrected rather than inheriting anything; a stray second SRT connection disturbs nothing. They also found the two failures fixed above.

Risks for review

  • A stale head under 500 ms is not corrected. The threshold has to clear healthy lead, which reaches 440 ms on video at connect with ffmpeg's default muxing (tsdemux re-anchor fires on healthy SRT inputs and steps recordings ~0.38 s backwards #997). A caller that waited less than 500 ms runs up to that much late for its connection; a false correction instead writes backwards timestamps into every recording.
  • Nothing is corrected after the first 3 s of a connection. A sender that jumps ahead mid-stream, or a fast sender clock, now goes uncorrected and its output runs late downstream. Before, either was re-anchored each time it crossed the threshold, and tsdemux re-anchor fires on healthy SRT inputs and steps recordings ~0.38 s backwards #997 showed that rule firing on healthy streams.
  • The first second of a stale-head connection is still emitted stamped ahead, because the correction needs two windows to decide. Measured through a queue ! fakesink sync=true, the shape of every consuming block: delivery peaks 909 ms (decoded) and 974 ms (passthrough) of wall clock behind arrival, recovers to 16–102 ms once the correction lands, and srtsrc delivers nothing for about that second. Upstream, without the fix, the same measurement peaks at 956 ms and never recovers. So the transient is not removed, only bounded — under a 125 ms SRT receive buffer it is a window where loss is possible. A faster decision on a large jump would close it and is not in this PR.
  • Only a caller change drops the offset, so mode=caller is not covered. Dialling out to a remote listener produces no caller-added, so if that peer restarts after a correction has been made, its stream stays shifted and runs late until the flow restarts. Undoing the shift whenever the output sits behind arrival was tried and removed: it cannot tell that case from a sender that stalled, and it made a 900 ms sender hiccup jump the output 852 ms forward in one buffer and left a recovering SRT stall parked ~200 ms early. A signal that distinguishes them is follow-up work.
  • Timestamps that fall behind on their own are not corrected, only a shift this code applied is dropped.
  • fix(mpegtssrt_input): keep an absent SRT peer from holding the flow #812 touches different hunks of mpegtssrt_input.rs and merges cleanly. fix(mpegtssrt_input): keep an absent SRT peer from holding the flow #812 decides when the decodebin starts; this PR corrects timestamps once tsdemux exists. The merged build is in the table.
  • Seen, not addressed.
    • With the caller disconnected first, stopping srtsrc (keep-listening=true) hung once in ~10 test runs, in gst_pad_stop_task while srtsrc waited for a new caller. The test stops the receiver first.

Also fixes #997, which only occurred with this PR's earlier code and is closed.

🤖 Generated with Claude Code

@srperens srperens mentioned this pull request Sep 14, 2026
@srperens

Copy link
Copy Markdown
Collaborator

Noted as a draft, so I am holding off on a full review until it is marked ready for review. If you want one before then, say so here or request me as a reviewer.

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Request changes. The mechanism is right and the per-buffer path is clean. CI only guards half of the fix, though. The caller-change reset (c8a8767) has no test that runs in CI, and main has since landed what the skip gate was waiting for.

Requested changes

  1. Rebase onto main, then re-test the version gate. The branch is 83 commits behind (merge-base eae2467). The gate's reason is #856: backend/tests/mpegtssrt_input_stale_head_test.rs:401 — (major, minor, micro) >= (1, 24, 3) — "tsdemux then replaces its source pad, which the block does not relink". #877 (b28be77) is now on main, and it relinks exactly that replacement (link_to_output, "tsdemux adds the new pad first"). If both second-caller tests pass on 1.24.2 after the rebase, drop the gate. git merge-tree origin/main pr813 merges clean.
  2. If the gate has to stay, guard the wiring another way. a_caller_change_drops_the_offset calls reset() directly (backend/src/blocks/builtin/tsdemux_anchor.rs:343 — anchor.state.reset();). Deleting watch_caller would fail nothing that runs in CI. A unit test can build a real srtsrc, apply a correction, emit_by_name("caller-added", ...), and assert correction == 0. That would cover backend/src/blocks/builtin/tsdemux_anchor.rs:125 — state.reset();.

Claims

Claim Verdict Evidence
Probes do atomics only, no lock or allocation on the per-buffer path CONFIRMED input probe: backend/src/blocks/builtin/tsdemux_anchor.rs:111 — state.last_input.store(pts.nseconds(), Ordering::Relaxed);. In the output probe, only the correction branch leaves atomics: info! and backend/src/blocks/builtin/tsdemux_anchor.rs:281 — pad.set_offset(offset);, which takes the pad's object lock. That happens once per correction, not per buffer
No strong element reference in a signal closure CONFIRMED backend/src/blocks/builtin/tsdemux_anchor.rs:135 — let anchor = self.clone(); clones an Arc<State>. Every handler captures that state, never an element
The stale-head correction is guarded in CI CONFIRMED Check (Linux) job 107386718642: stale_head_is_not_stamped_ahead_{passthrough,decoded} ... ok over ~3.6 s, so they ran
The offset reaches the buffer that triggered it EXTERNAL assumes gst_pad_push_data re-sends the sticky SEGMENT after buffer probes. The 25/49 ms residual is consistent with that

Repo rules. CLAUDE.md: "A test that requires a GStreamer element must be able to run in CI. Tests that skip ... pass green and guard nothing". In the same job, both second_caller_* tests print ok within 1 ms of the binary starting. That is the gate's early return. The body says so; requested change 2 closes it.

Diagnosis. The body traces the root cause into tsdemux and reproduces it without Strom, so this is the right layer. Coverage is BOUNDED, and the body names the bounds: ignore_pcr=true only, listener mode only for the reset, and a first ~1 s still stamped ahead. CLOSED loop: on the healthy path, atomics per buffer and no set_offset.

Radius. LOCAL: one block. It is active on every mpegtssrt_input with the default ignore_pcr, in both decode and passthrough.

Overlaps.

  • #812 changes the same file in different hunks. Pairwise merge is clean, and the body's table shows the two combined. Either can land first.
  • #936 consolidates SRT test port helpers. This PR adds another srt_port(), so whichever lands second should adopt the shared one.

Tests & CI. Eight checks green at 8bcd79b, on a base 83 commits old. Both tsdemux_anchor unit tests and both stale-head tests ran. macOS and Windows show skipping, and nothing here is platform-specific.

Confidence: HIGH

@wagenet
wagenet force-pushed the wagenet/srt-early-connect-timestamps branch from 8bcd79b to 9e73d31 Compare October 2, 2026 13:53
@wagenet

wagenet commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Claude (AI), on behalf of @wagenet.

Addressed both requested changes:

  1. Rebased onto main (3225792, includes fix(mpegtssrt_input): relink an output when the demuxer replaces its pad #877), clean. The 1.24.3 gate is gone (9e73d31). In an Ubuntu 24.04 container on GStreamer 1.24.2, as CI, all four tests in mpegtssrt_input_stale_head_test passed 6/6 runs with STROM_REQUIRE_GST_PLUGINS=1.
  2. Not needed now that the gate is gone: with the caller-added/caller-removed handler body stubbed out (so watch_caller connects but never calls reset()), both second_caller_* tests fail on 1.24.2. CI now guards the wiring, not only reset().

#936 has not landed, so the local srt_port() stays; whichever lands second adopts the shared one. The PR body is updated.

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This supersedes my Request changes review of 8bcd79b. Both requested changes are done. The verdict moves to Comment, not Approve, because #997 was filed since that review, and it reports this mechanism firing on healthy streams.

Verdict: Comment. The rebase, the removed gate and CI coverage are all correct. Whether this lands as it is now, or after #997 settles the trigger rule, is a maintainer's call.

Options for a maintainer

  1. Land as is. #997 then becomes a follow-up against main.
  2. Wait for #997 and change nothing here. #997 owns the threshold, per-pad measurement and the "never step backwards mid-stream" rule, and all three are in this PR's code.
  3. Narrow it here first. For example, only correct during an initial window after caller-added, which is when the stale head this PR targets occurs. A late re-anchor is what #997 sees going wrong.

Claims

Claim Verdict Evidence
The reconnect tests now run on CI's 1.24.2 and guard the caller-change wiring CONFIRMED 9e73d31 removes the version gate. In Check (Linux) job 110867228063, second_caller_resuming_is_not_stamped_behind and second_caller_restarting_is_not_stamped_behind report ok about 3.2 s after the stale-head pair, so they ran rather than returned early.
The rest of the series is unchanged since my last review CONFIRMED git range-diff shows commits 1 to 4 patch-identical to 557d444..8bcd79b. The probe-cost and WeakRef rows stay CONFIRMED as in my review of 2026-10-01.
A correction can fire at any point in a connection, across both pads, and step running time backwards CONFIRMED backend/src/blocks/builtin/tsdemux_anchor.rs:259 — if min_lead <= MAX_LEAD.nseconds() as i64 { is the only gate, and nothing limits it to the start of a connection. The window's minimum is taken over every demuxer pad's samples together. The shift is backend/src/blocks/builtin/tsdemux_anchor.rs:266 — let correction = correction - mean_lead;.
Healthy callers are never re-anchored UNVERIFIED The body tested 128 and 32 kbit/s AAC. #997 reports video holding 290–440 ms of lead from its first-PES burst, and audio PES of about 11 AAC frames (ffmpeg's default TS muxing) at 100–230 ms. That occasionally gives a window minimum of 201–205 ms against backend/src/blocks/builtin/tsdemux_anchor.rs:49 — pub const MAX_LEAD: gst::ClockTime = gst::ClockTime::from_mseconds(200);. The result is a mid-stream step of about −380 ms and decreasing DTS in mp4mux. That reproduction ran outside this repo. The PR's tests do not cover this stream shape.

Diagnosis. The stale-head cause and the fix layer stand as in my earlier review. Coverage is BOUNDED, by the same bounds the body names. #997 adds a cost on the healthy path that the body does not list: the loop is CLOSED, but its trigger is a fixed 200 ms on the combined minimum, and the healthy lead range for some senders reaches it. #997 says it was seen on "Strom 68632494 (main)", but tsdemux_anchor.rs is not on this repo's main, so that build must have carried this branch.

Radius. LOCAL. It is one block, active on every mpegtssrt_input with the default ignore_pcr.

Overlaps.

  • #997 owns the design decision on when this anchor may fire (see the options above).
  • #812 changes the same file in different hunks, and either can land first.
  • #936: whichever of the two lands second adopts the shared srt_port().

Tests & CI. At 9e73d31, Check (Linux), both Linux builds, WASM, API Contract Check and Static JS Tests pass. macOS and Windows were skipped, and nothing here is platform-specific. The branch is 31 commits behind main, and git merge-tree against main is clean. Nothing was built or run here.

Confidence: HIGH

@srperens
srperens dismissed their stale review October 6, 2026 08:08

Superseded by my review of 9e73d31: the requested changes are addressed.

@wagenet

wagenet commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Posted by Claude (Anthropic's AI assistant) on behalf of @wagenet.

#997 is fixed in this PR as of 5d248ad, and the issue is closed. It only happened with this PR's code: the "main" build it was seen on was the fork's integration branch, which carried this PR.

The fix follows option 3 from the last review:

a_healthy_stream_is_never_re_anchored covers #997's stream shape and fails with the old threshold. Each new rule has a unit test that fails when the rule is reverted. The PR body's Fix, Tests and Risks sections are updated. The rig and #997's ffmpeg repro were not re-run on this commit.

wagenet and others added 7 commits October 6, 2026 14:25
With ignore-pcr, tsdemux maps PTS to output time from the first PES it
parses and never takes a new reference. An SRT caller that encoded while
it waited for the listener sends a few frames from the start of its encode,
then live frames. The stale frame becomes the reference, so live audio and
video are stamped as far into the future as the caller waited, and every
downstream sink and mixer holds the source back by that much for the whole
connection.

TsDemuxAnchor compares each demuxed buffer's running time with the latest
srtsrc timestamp (arrival less SRT transit). When the smallest lead over a
500 ms window exceeds 200 ms, it moves all of the demuxer's source pads
back by the window's mean lead with one shared pad offset. The probes use
buffer timestamps and atomics only. PCR mode is left alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The replay dropped every packet between the head and the live part, which
broke the table continuity counters as well as the media. GStreamer 1.24.2,
which Ubuntu 24.04 and CI ship, then re-applies the PMT and, with ignore-pcr,
mistakes it for a new program (fixed upstream in 1.24.3). tsdemux replaces
its source pad, the new pad is unlinked, and srtsrc stops with not-linked
after the head, so no buffers reached the correction window.

Forward the table packets from the gap and drop only the media. The PTS jump
the test guards is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
srtsrc keeps listening after a caller leaves and tsdemux takes a fresh time
reference for the next one, but the pad offset stayed. A second caller with a
clean stream left the block as far behind its arrival as the first caller's
stale head had been ahead of it (-2041 ms measured), where every consumer that
waits for running time drops it as late. Nothing brought it back, because the
offset only ever moved further back.

Drop the correction on srtsrc's caller-added and caller-removed. Output that
sits behind arrival is still left alone: a sender that stalls and resumes looks
the same from here, and giving the offset back would throw its stream forward
by that much in one buffer.

The replay test gains a second caller in both shapes, resuming and restarting,
and the module gains unit tests for what a caller change and a window spent
behind arrival each do to the offset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Before 1.24.3 a re-parsed PMT reads as a new program under ignore-pcr, and a
caller change re-parses it. tsdemux replaces its source pad, the block does not
relink it (Eyevinn#856), and the second caller delivers no audio at all -- so the
reconnect tests cannot pass on Ubuntu 24.04 or CI, whatever the anchor does.

Skip them below 1.24.3 with the version in the message. They run on the release
image, which ships 1.26, and on development machines. CI keeps the two
stale-head tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eyevinn#877 relinks the pad tsdemux swaps in when 1.24.2 re-parses the PMT, so a
second caller now delivers audio there too. Both reconnect tests pass on
1.24.2 (Ubuntu 24.04, as CI) and fail there when the caller-change reset is
removed, so CI guards it.

This reverts commit de6cd99.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A healthy stream runs ahead of arrival by what the sender packs together:
multi-frame audio PES and the video burst at connect. Windows of such a
stream reached a 205 ms minimum lead, crossed the 200 ms threshold
mid-stream, and the shift stepped running time back about 380 ms, so
recorders wrote decreasing DTS.

A stale head only occurs at connect, so a correction may now be made only
in the first 3 s of a caller's input, at most once, and only above 500 ms.
A caller change re-arms the watcher. The integration tests keep their own
200 ms on-time bound instead of borrowing MAX_LEAD.

Fixes Eyevinn#997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rust 1.98's clippy denies chunks_exact with a constant size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wagenet
wagenet force-pushed the wagenet/srt-early-connect-timestamps branch from 5d248ad to 6f6648a Compare October 6, 2026 21:29
wagenet added a commit to wagenet/strom that referenced this pull request Oct 9, 2026

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This supersedes my Comment review of 9e73d31. The verdict moves to Approve. The author implemented option 3 from that review, correcting only at connect, and #997, which owned the trigger rule, is now closed.

Verdict: Approve. A correction can now fire only within 3 s of a caller's first input, and only once per caller. The threshold of 500 ms sits above the healthy lead that #997 measured. Guard tests cover each part, and all of them ran green at this head.

Claims

Claim Verdict Evidence
A correction is gated to the arm period and disarms after it fires CONFIRMED The gate at line 267, quoted under the table, returns before any measurement. :275 disarms once input passes the deadline, and :311 disarms after a correction
The threshold clears #997's healthy lead CONFIRMED backend/src/blocks/builtin/tsdemux_anchor.rs:57 — pub const MAX_LEAD: gst::ClockTime = gst::ClockTime::from_mseconds(500); against the 440 ms video lead at connect. a_healthy_stream_is_never_re_anchored replays that shape and would fail on the old 200 ms rule, whose minimum of 230 ms exceeds 200
The probes stay within CLAUDE.md's BUFFER-probe rules CONFIRMED The input probe adds one load and one store (backend/src/blocks/builtin/tsdemux_anchor.rs:253 — if state.arm_deadline.load(Ordering::Relaxed) == ARM_PENDING {). Once disarmed, the output probe returns after two loads. The only formatting is the info! on the single correction, as before
A caller change re-arms the watcher, and CI guards that wiring CONFIRMED reset() stores ARM_PENDING. In Check (Linux) job 112511813527 at 6f6648a, second_caller_resuming_is_not_stamped_behind, second_caller_restarting_is_not_stamped_behind and a_caller_change_drops_the_offset_and_re_arms all report ok

The arm gate: backend/src/blocks/builtin/tsdemux_anchor.rs:267 — if deadline == DISARMED || deadline == ARM_PENDING {

Diagnosis. The right layer, and the loop is now CLOSED with a bound: on the healthy path the probe pays two atomic loads per buffer after 3 s. Coverage is BOUNDED, and the body names all three bounds:

  • a stale head under 500 ms goes uncorrected;
  • nothing is corrected after the first 3 s;
  • mode=caller never re-arms, because srtsrc emits no caller-added there.

The earlier rule shows why a narrower trigger is the better trade: a false correction writes backwards timestamps into every recording, while a missed one only leaves a source late. The rejected "give the offset back when behind" alternative is in the body with its measured failure.

Overlaps. #997 is closed, so no open item owns this decision. git merge-tree is clean against current main (35 commits ahead of this branch), against #812 and against #936.

Radius. LOCAL: one block, active on every mpegtssrt_input with the default ignore_pcr.

Tests & CI. At 6f6648a, Check (Linux), both Linux builds, WASM, API Contract and Static JS pass, and all four unit tests and four integration tests ran ok there. macOS and Windows were skipped, and nothing here is platform-specific. The CI base is 35 commits behind main. The merge is clean but has not been built.

Confidence: HIGH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsdemux re-anchor fires on healthy SRT inputs and steps recordings ~0.38 s backwards

2 participants