Skip to content

fix(moq-mux): anchor the TS table cadence to the media timeline - #2825

Open
kixelated wants to merge 1 commit into
mainfrom
claude/github-issue-2779-ea990b
Open

fix(moq-mux): anchor the TS table cadence to the media timeline#2825
kixelated wants to merge 1 commit into
mainfrom
claude/github-issue-2779-ea990b

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Two exporters of one broadcast should render the same packets. Today the MPEG-TS exporter decides when to re-emit a table by measuring forward from its own last emission, so the cadence is anchored to whenever that process started rather than to the media timeline. Start a second exporter of the same broadcast 20 seconds later and the two put PAT/PMT (and SDT/NIT) on different frames, forever.

That breaks the case in #2779: a SMPTE ST 2022-7 pair needs its two legs packet-identical so a receiver can merge them, and a leg restarted for maintenance has to come back rendering exactly what its partner is rendering.

Root cause

due() asked "has interval elapsed since the last emission", which carries the phase of whenever the exporter first ran. It now asks "has the timestamp crossed into a later floor(timestamp / interval) slot", which is a property of the broadcast alone. Two exporters that last emitted at different points inside the same slot agree on every frame after that.

A program with video hid this for PAT/PMT, because the tables are also re-emitted at every video keyframe and that re-anchors both legs to a boundary they share. The two cases where it does not:

  • an audio-only program, which has no keyframe to re-anchor on, so PAT/PMT drifts;
  • the standalone SI PIDs (SDT 0x0011, NIT 0x0010) in any program, which are never re-anchored at keyframes, so they land on different frames rather than merely being renumbered.

A slot boundary also re-emits after a backwards timestamp jump (a rewound timeline), which the elapsed-time version sat out until the old interval came back around.

What this does not fix

The continuity counter, which is the headline of #2779, is still numbered from process state. Making it a function of the broadcast requires each group's per-PID packet count to be a multiple of 16 (a counter that is both globally continuous and independent of the join point forces that), which means padding, which is a much larger and opt-in-shaped change. The details are in #2779; this is the part that is unambiguous and free.

Tests

late_join_matches_a_running_exporter{,_without_video} produce one broadcast, export it twice with the second exporter joining halfway in, and assert the only bytes that disagree in the overlap are continuity counters. The audio-only case fails without this change (one leg renders 752 bytes where the other renders 376: two extra packets, PAT and PMT). The video case passes either way and is there to catch the next field someone mints per process; it is the topology in the report.

author_dts_is_join_independent_at_a_peak pins the one piece of carried state that is not a divergence: the monotonic decode-clock bump cannot fire on a frame whose PTS leads everything decoded before it, which every keyframe is, and export only ever tunes in on a keyframe.

Cross-package sync

None. No wire format, no public API, no CLI surface, and the MPEG-TS exporter has no JS counterpart.

Fixes part of #2779.

🤖 Generated with Claude Code

(written by Opus 5)

The MPEG-TS exporter decided when to re-emit a table by measuring forward
from its own last emission, so the cadence carried the phase of whenever
that process started. Two exporters of one broadcast that did not start
together then put PAT/PMT (and SDT/NIT) on different frames, forever,
which breaks a redundant pair that compares its two legs packet by packet.

`due` now asks whether the timestamp crossed into a later
floor(timestamp / interval) slot, which is a property of the broadcast
alone. A program with video hid this for PAT/PMT, since the tables are
also re-emitted at every video keyframe and that re-anchors both legs to a
boundary they share; an audio-only program has no such boundary, and the
standalone SI PIDs are never re-anchored in any program.

The continuity counter is still numbered from process state. Making it a
function of the broadcast forces each group's per-PID packet count to be a
multiple of 16, which is a much larger change; the new tests assert that
it is the *only* remaining difference across a late join.

Refs #2779

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@kixelated, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f2316e66-7e6a-4c94-8865-960eec7c7c84

📥 Commits

Reviewing files that changed from the base of the PR and between f91e3bb and f821c6b.

📒 Files selected for processing (2)
  • rs/moq-mux/src/container/ts/export.rs
  • rs/moq-mux/src/container/ts/export_test.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f821c6b245

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Duration::from(timestamp)
.checked_sub(Duration::from(last))
.is_some_and(|elapsed| elapsed >= interval)
slot(timestamp, interval) != slot(last, interval)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Ignore older slots unless the whole timeline rewinds

When PTS moves backward across a slot boundary, as can happen with B-frames in decode order or when one live track temporarily lags another, != treats the older slot as newly due and stores that older timestamp. The next forward frame then triggers another emission, so PAT/PMT and every SI PID can repeatedly bounce between slots; different inter-track arrival timing also makes redundant exporters produce different packet layouts. Compare against a monotonic high-water slot, or distinguish an actual stream rewind from ordinary timestamp reordering. (Written by GPT-5.6 Sol)

Useful? React with 👍 / 👎.

///
/// A zero interval degenerates to one slot per microsecond, which makes every frame due.
fn slot(timestamp: Timestamp, interval: Duration) -> u128 {
Duration::from(timestamp).as_micros() / interval.as_micros().max(1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve zero-interval emission for every frame

When a custom SI entry specifies Duration::ZERO, mapping the interval to one microsecond does not make every frame due as documented: two tracks can legitimately emit frames at the same timestamp, and the second call receives the same slot and returns false. The previous elapsed-time implementation treated every non-backward frame as due for a zero interval, so this silently skips requested SI repetitions; special-case zero before calculating slots. (Written by GPT-5.6 Sol)

Useful? React with 👍 / 👎.

@t0ms

t0ms commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Reviewed and tested. The diagnosis is right and the fix does what it says — on a source that
carries SI, which is the case the synthetic tests can't construct. One regression that those tests
can't see, and it has a one-character fix.

Setup: one publisher feeding a real DVB contribution stream (CNN International EMEA HD, 9.95 Mbps
CBR, H.264 with B-frames, MP2 + AC-3 + teletext + 3× SCTE-35, carrying SDT and NIT), two
moq export ts subscribers of the same broadcast on one relay, the second joining 20 s late, 45 s
window. Same rig and same clip for all three builds.

1. It works

Byte-comparing the two legs doesn't measure this (§4), so the measurement is where each table lands
in media time: the exporter writes due tables and then the frame's PES packets into one buffer, so
the first PES header after a table gives the PTS of the frame that triggered it. Agreement below is
frames where both legs emitted the table, over frames where either did — 100 % means the emission
points belong to the broadcast.

PAT/PMT SDT NIT
6d3c51d7 (before) 95.51 % 0.00 % 0.00 %
f821c6b2 (this PR) 96.36 % 92.74 % 95.71 %

The standalone SI PIDs go from never agreeing to almost always. That is exactly the mechanism you
described: PAT/PMT were largely saved by the keyframe re-anchor and SDT/NIT had nothing to re-anchor
on. Worth noting the before column is 0.00 %, not "low" — over a 45 s overlap the two legs did not
emit SDT on a single common frame.

2. B-frames re-trigger the cadence, ~25× on this clip

due now returns true whenever the slot index changes, and
due_crosses_an_absolute_slot pins that a backwards timestamp is due ("a rewound timeline, not a
slot that has yet to lapse"). On B-frame content a backwards timestamp is not a rewound timeline —
it is ordinary presentation-order reordering, and it happens constantly. In this clip 39.2 % of
video frames arrive at a PTS below their predecessor (median 60 ms back, max 180 ms).

So every oscillation that straddles a slot boundary re-fires the tables: cross into the new slot,
step back into the old one, cross again. 46.8 % of the PR's PAT emissions occur at a backwards step.

Share of the stream spent on PAT/PMT/SDT/NIT, same clip, same window:

Build PAT PMT NIT SDT Total Share
6d3c51d7 125 125 7 31 288 0.071 %
f821c6b2 2,875 2,875 242 1,191 7,183 1.747 %

Not a conformance failure — TR 101 290 P1 bounds the maximum PSI interval, not the minimum — but it
is 1.7 points of payload spent re-sending tables that did not change, on a lane whose bandwidth
argument is that it strips redundancy rather than adds it.

The tests can't catch this because export_twice writes video at tick * VIDEO_US, which is
strictly monotonic, so no fixture in the suite ever revisits an earlier timestamp.

3. !=> fixes it, and improves the thing the PR is for

slot(timestamp, interval) > slot(last, interval)

last is only updated when due fires, so comparing "strictly later slot" still emits exactly once
per slot, and a reordered frame stepping back into a slot that has already been served no longer
re-fires. Rebuilt and re-ran the same rig:

PAT/PMT emissions (A) PAT/PMT frames in disagreement SDT emissions (A) SDT frames in disagreement
6d3c51d7 87 4 21 43 (nothing shared)
f821c6b2 1,487 55 801 60
with > 113 2 23 3

Strictly better on both axes: a twentieth of the emissions and fewer frames where the legs
disagree. SI/PSI share comes back to 0.089 %.

Exactly one test fails, and it is the intended-rewind assertion:
due_crosses_an_absolute_slot's assert!(due(ms(750), Some(ms(1_000)), PSI_INTERVAL)). Everything
else passes, including both late_join_matches_a_running_exporter cases. So the trade is explicit
and singular: rewind handling costs 25× PSI on any source with B-frames.

If the rewind property is worth keeping, the two are separable by magnitude — reorder depth is
bounded by the codec's reorder buffer and measured at 180 ms max here, where a rewind is a
discontinuity. A threshold well above the reorder depth, or a high-water mark with an explicit
rewind escape, keeps both.

4. The interleave is not a residue on real content, it is the wall

A note on why the numbers above are table anchor points and not byte comparisons. Comparing the two
legs packet by packet, the longest run they share is 32 packets, even with the continuity counter
masked — and the PID census over the aligned span shows why: the legs render different numbers of
video and audio packets over the same media interval.

That is the fourth item from your #2779 comment, poll_next emitting whichever track has a frame in
hand. On a single-track synthetic fixture it is invisible; on real multi-track content over real
sockets it is not a residue sitting behind the continuity counter, it is what stops a byte
comparison working at all. Our own 1+1 measurements only reached 97 % masked-identical because a
downstream CBR groomer re-places every packet onto a stream-clocked grid, which absorbs the
interleave before anything compares the legs.

Two consequences worth having on the record:

  • The residual 2 PAT / 3 SDT disagreements under the > variant are most likely this: near a slot
    boundary the two legs disagree about which frame crosses first.
  • The opt-in deterministic mode you sketched needs the interleave and the counter. With the
    interleave non-deterministic, a byte-identical pair is unreachable no matter what the counter does.

Verdict

Ship it, with > in place of != (or a rewind threshold above the reorder depth). It closes a real
divergence that we can now measure on real content, and the cadence half is a precondition for the
counter work whatever form that takes.

Happy to contribute the two scripts if useful — the anchor-point measurement is a few dozen lines
and is the only way we found to test this property without a byte-identical pair.

(written by Opus 5)

@t0ms

t0ms commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Follow-up with the downstream measurement I owed you, since it turns out this PR closes a gap
we had been attributing elsewhere.

The rig is a 1+1 redundancy pair: one publisher, one relay, two export ts subscribers, each
paced to a constant-rate RTP leg whose packet placement is derived from the stream position
rather than the process clock, with the second subscriber joining 20 s late. The legs are then
compared at equal stream slots with the continuity counter masked, so what is left is only what
the two exporters rendered differently.

On a single-track source (H.264, no B-frames, no audio, 2 Mb/s CBR):

build slots identical, counter masked
main 96.43 %
this PR 100.00 %

So the cadence really was the whole of the remaining difference there, and this PR removes it.

One caveat worth recording: on a source whose keyframe interval divides the PSI interval exactly
(a 2 s GOP against the 500 ms interval), main already scores 100.00 %. The defect only becomes
visible when the two do not divide — at a 1.8 s GOP it is the 96.43 % above. That is presumably
why it went unnoticed: the common broadcast GOPs hide it.

On multi-track content with B-frames the picture is the one from my earlier comment:

build slots identical, counter masked
main 87.75 %
this PR 89.72 %
this PR with !=> in due 95.62 %

The gap between the middle and last rows is the PSI inflation seen through a receiver: over the
compared span this PR emits PAT 1,959 / 1,946 and SDT 818 / 803 across the two legs, where the
monotonic variant emits 111 / 111 and 22 / 22 — the same tables the source has, and the same
number on both legs. So the one-character change is not only about overhead; without it the
inflated tables also land inconsistently between legs.

The residual 4.4 % is not yours and not a late-join effect — with both subscribers started at the
same instant it is 4.82 %, with every table matching exactly and the two legs differing only in how
many audio and video packets they place. That is the interleave, which I have filed separately as
#2829.

@t0ms

t0ms commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Correction to the caveat in my last comment: divisibility is not the explanation, and the defect
is not as easy to miss as I implied.

I went back to a capture from a different host — the same 1+1 rig, a video-only 2 Mb/s source with
a 2 s GOP, so the keyframe interval does divide the 500 ms PSI interval — and main scores
97.10 % there, not 100 %. So a source can divide cleanly and still diverge; whether two legs'
emission points coincide is incidental to where each one's first emission happened to land, which
is the point of the PR.

That capture also shows the mechanism unusually clearly, because it is old enough to predate
anything I changed. Over the shared span the two legs emit exactly the same tables — PAT 123/123,
PMT 123/123, SDT 31/31 — and the residue is:

     2,561  0x0100 (video), same PID, different bytes
       683  0x0100 vs 0x1fff
       668  0x1fff vs 0x0100
        31  0x0011 (SDT) vs 0x0100
        16  0x0000 (PAT) vs 0x1fff
        16  0x1000 (PMT) vs 0x1fff

31 of 31: every single SDT emission lands on a slot where the other leg has video. PAT and PMT
disagree on 16 of 123, which is the keyframe re-emission holding them mostly in step while the
interval-driven ones drift. Each misplaced table then shifts everything after it by a packet,
which is where the 2 561 video packets come from — the two legs are carrying the same media, one
slot apart.

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.

2 participants