Repository navigation
fix(mux): put a live export's budget on its first SUBSCRIBE - #5854
Conversation
A live export source subscribed with a zero budget to land on the newest group, then raised the budget after reading its first frame. Until that update reached every hop, each one dropped any group a newer one superseded. MP2 imported from TS is a group per frame, written five at a time per PES, so a joiner lost the groups right after its start: the hole `just test ts --full` reported in the MP2 PID. Subscribe with the real budget from the start and pick the live edge locally: the consumer starts at the newest group announced and discards the older ones the budget also delivers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ffprobe stops describing streams after 5 MB. A capture whose first audio PES lands past that gets its stream type's guess (mp3 for MP2), and the second frame of that PES repeats the first's timestamp, which holes.py read as a 24 ms hole that the capture never had. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (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 4 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 |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
A deterministic repro of this loss, stacked on this branch: #5857. It runs a lite-07 session over (written by Claude Opus 5.5) |
Fixes the flaky
FAIL PID 0x101 mp2: ... 1 holesin TS validation's "TS full multiplex at its own rate" step. Two separate causes produce that one failure line, and this PR fixes both.1. Real loss: a live export's zero-budget SUBSCRIBE (96 ms holes)
ExportSource::live()subscribed withmax_delay = 0to land on the newest group, then raised the budget withset_max_delayafter reading its first frame. Until that SUBSCRIBE_UPDATE reached the relay and the publisher, each hop applied the zero budget and dropped every group a newer one superseded. That is the rule the existinglatency_is_sent_with_the_initial_subscriptiontest already states: a budget raised after SUBSCRIBE_OK cannot recover what the publisher skipped.MP2 imported from TS is one group per 24 ms frame, and the importer writes the five frames of each PES at once. A joiner whose start group is the first frame of a PES lost the next four groups: a 120 ms step, 96 ms missing, right after the capture's first MP2 frame. That matches the CI signature (
media skipped upstream ... track="2.mp2" at=2.846s skipped=120ms). Instrumenting the consumer showed the start group finished whole and sequences N+1..N+4 never arrived. Because the start group was whole, #5713's "cut start group starts at the next group" rule does not apply: these are whole groups missing after one that ended.Fix: subscribe with the real budget from the start, so no hop drops anything, and pick the live edge locally.
container::Consumergains a crate-privatelive()that latches its cursor onto the newest group announced when the first frame arrives, then discards the older groups the budget also delivered.ExportSourceno longer callsset_max_delayon its first read.export ts --passthroughstarts itsm2tstrack through the samelive(), so it gets the same change.Repro: one 30 s
--fullclip published through a local relay, with 24 subscribers joining at staggered offsets (export ts --delay 900ms), on this box under load.2. Ghost hole: ffprobe's 5 MB probe (24 ms holes)
The 24 ms failures (runs 39002260268 and 39002594653) logged no gap from the exporter, and the capture has no hole. Its first MP2 PES starts about 5.9 MB into the file, past ffprobe's default 5 MB probe. ffprobe never describes the stream (it reports
mp3from the stream type alone), so the second frame of that PES repeats the first's timestamp, andholes.pyread that as a 48 ms step. Reading the PES headers directly shows every frame present.holes.pynow probes the whole file. The newProbetest inholes.test.pybuilds such a TS with ffmpeg; it fails without the change and passes with it.Trade-off: some joins start one video GOP back
With the budget on the wire, a join within the budget (delay / 2) after a video keyframe also gets the previous GOP. Its first frame can arrive before the new GOP's stream is announced, so the local latch starts there. In each 24-join run, the end of a fixed 5 s capture was 0.29 to 0.58 s behind main in 5 to 7 joins: extra latency, bounded by the budget plus a GOP boundary, and no loss. Audio starts at its newest group as before. Making the latch exact needs the publisher's newest position before the first frame, for example the lite-07 SUBSCRIBE_START
largestthatquest/m1/largest.mdplans to expose. Whether this trade is acceptable is the maintainer's call.Public API and wire impact
ExportSource::live()keeps its signature with updated docs.container::Consumer::live()ispub(crate).Checks
just check: passes.cargo nextest run -p moq-mux --lib container::: 903 passed.latency_is_sent_with_the_initial_subscriptionnow also covers a live source and fails on main (left: 0ns, right: 10s). Newa_live_start_reads_from_the_newest_groupcovers the local latch.just test ts --full(the whole recipe, includingholes.test.py): 6/6 passed locally under load. 4 of the 6 joined mid-GOP (video from 2.080 s, MP2 after it), the phase every CI failure had; main's 8 local runs all joined after the 3.08 s keyframe and never reached it.🤖 Generated with Claude Code