Repository navigation
fix(mux): start a live export at the publisher's largest group - #5947
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A live export latched onto the newest group announced when its first frame arrived. Behind a backlog, the budget serves groups oldest first, so the backlog's oldest group can land alone: the export read it, then skipped the groups after it once the next PES moved the live edge a budget past them. That is TS validation's MP2 hole: one frame, then 96 ms missing. The latch now takes the publisher's largest (SUBSCRIBE_START on lite-07) when it names a group past what has arrived. A relay answered with only the newest group it held, which right after its upstream subscribed can be that same oldest group, so a lite-07 relay's SUBSCRIBE_START now forwards its source's largest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Quest outcome: I reproduced the MP2 join hole deterministically and fixed it at its cause, the live latch starting at the backlog's oldest group. The fix has two parts: the export latches on the publisher's largest, and lite-07 relays forward their source's largest. All local checks pass, including One open decision keeps this a draft: should a relay's SUBSCRIBE_START (Written by Claude Opus 5.5) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Decision settled by the maintainer: option 1. A relay's SUBSCRIBE_START proxies its upstream's largest group rather than reporting only what its cache holds. The cache is a performance optimization, so a relay should report and behave the same with a warm or cold cache. That guideline is now in (Written by Claude Opus 5.5) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit details
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 800565dd9e
ℹ️ 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".
Mirror the Rust relay: a lite-07 SUBSCRIBE_START names the larger of the newest buffered frame and the largest the source reported, so a relay holding only the oldest group of a backlog still points a live joiner at the newest group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging with maintainer approval. A relay's lite-07 SUBSCRIBE_START reports its source's largest group, in Rust and JS, and the export's live start uses it, fixing the mp2 join hole; rs/moq-net/AGENTS.md gains the warm/cold cache rule. Codex approved this head; CI green. (Written by Claude Opus 5.5) |
#5947 added a `peekLargest` hook returning a location, which the lite publisher reads for a subscriber's start. This branch already had one by that name returning the largest and whether it can still advance. Keep the branch's shape, answer it from the serving copy for a resumed reader as Rust `peek_largest` does, and have the publisher read its `largest`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
peek_largest answers SUBSCRIBE_START, a wire answer, so it reads the track's largest regardless of the reader's local bounds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
TS validation's "TS full multiplex at its own rate" step still flaked with an MP2 hole right after the export's live join, even with #5854's budget fix: run 39042472016 logged
media skipped upstream ... track="2.mp2" at=2.389977s skipped=120ms. The capture has the first frame of one PES (2.390 s), then nothing until the next PES (2.510 s).The cause is the live latch from #5854. It starts at the newest group announced when the first frame arrives. Behind a backlog, the publisher serves the budget's groups oldest first, so the oldest in-budget group can land alone. The export starts there and reads it. Then the next PES moves the live edge a full budget (450 ms) past the groups after it, and the budget skips them: a gap after the first frame.
The CI timings fit this. The source's PCR puts PES 2.39 right at the edge of the 450 ms budget when the export joined, and PES 2.87 arrived about 20 ms later. That expires exactly 2.414 to 2.486 and keeps 2.510, which are the four missing frames.
Reproduced deterministically. A memory session holds back every group stream after the first, the way a network can deliver one stream ahead of the rest. On main, the export reads
[1656000, 1800000, ...]: the same one frame followed by a 96 ms hole.Approach
track::Subscriber::poll_largest). On lite-07 that comes from SUBSCRIBE_START, before the newest group's stream lands. The latch starts at whichever is newer: that largest group or the newest announced. Nothing is read below it, so dropping the backlog is not a gap.poll_live). Right after its upstream subscribes, that can be the same lone oldest group. The relay already records its source's answer as the cache's largest hint (report_largest), soTrackRun::startnow reports the larger of the two. A new crate-privatetrack::Subscriber::peek_largestreads that hint.bufferedLargest, which only sees arrived groups. It is now the later of that and the track's largest, which already holds the source's report; a new internalpeekLargesthook reads it.This also removes #5854's trade-off, where some joins started one video GOP back. Locally,
just test ts --fullnow starts video, MP2 and AC-3 at the newest group (3.08 s) in all three runs, with no holes.Tests:
a_live_start_behind_a_backlog_starts_at_the_newest_group(moq-mux) covers a direct link and a relay. Without the consumer change the direct case fails. Without the relay change the relay case fails.lite07_start_reports_the_sources_largest(moq-net) fails without the relay change (largest: 0.0instead of4.0).lite draft-07 reports the source's largest position past what a relay holds(js/net) is its JS mirror, and fails without the js/net change (group 0 instead of 4).container::test_util::Link, which test(mux): reproduce the live export's burst loss over a session #5857'sa_live_start_loses_no_group_the_publisher_burstsnow uses too.Link::heldgates the uni streams a link accepts.Impact
track::Subscriber::peek_largestispub(crate), and the JShooks.peekLargestis package-internal.Largestnow carries its source's reported largest when that is past what the relay holds. The draft's Largest definition says so.Decision
The relay forwards its source's largest in SUBSCRIBE_START rather than only the newest group its cache holds. The cache is a performance optimization, so a relay reports and behaves the same warm or cold;
rs/moq-net/AGENTS.mdnow says so. The rejected options were latching only the export on the hint (the publisher, relay, export topology keeps flaking) and splitting the moq-net change into its own PR.Alternatives
Follow-ups
live_edgein Rust,liveEdgein js/net), so they have the same relay limitation. This is low priority, since moq-lite comes first.quest/m2/audio-join-budget.mdnow points atLink::heldfor its repro.Checks
just check: passes. One earlier run hit the knownbursts_cross_a_transport_peer::impairedflake, which fix(shaper): open the ingress gate on offered bytes, not loss survivors #5943 fixes; a rerun passed.just drafts check: passes.just test interop --all: passes.just test ts --full: 3 of 3 passed locally, 0 holes on every PID.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code