fix(moq-mux): a source the output cannot follow fails the export promptly by kixelated · Pull Request #5490 · moq-dev/moq

kixelated · 2026-10-14T02:12:58Z

Completes quest/m1/slow-source.md.

Summary

Before this change, an export fed by a source far off real time dropped nearly every frame for hours and then exited cleanly. In #5435, moq import on CI's debug build ran 22% slow, and the TS export dropped 1,273 frames and captured only 3.2 s. jitter::Buffer already measured the rate within a few steps. It failed the source only once the steered clock had reached MAX_DRIFT, though, and under MAX_SLEW that takes about 3 hours.

Rule: the "clock at its limit" condition is gone. A source fails once its measured rate is past MAX_DRIFT and the rate has cost half the delay. The error names the rate. No new constant. TS, FLV and MKV all fail the same way because they share the buffer.
Startup gap: anchoring on frames that reached us late or early leaves a gap the clock pulls shut, and the rate did not cost it. It is now tracked on its own: it starts as the gap at the rate's first measurement, shrinks as the gap closes, and never reopens. The rate is charged only the gap beyond it. Before, the rate was charged the gap less its first value, so the clock's own steering counted as loss once it closed a late start.
Floor across tracks: a track not heard in a steering step, such as audio that ended before its video, is placed by its lead below the heard track sent least ahead of it, when it is sent later than every track heard. leads now holds each track's lead over the track sent latest, as the median of a window's worth of steps (LEADS, 300), so a spell of queueing shorter than half of it does not move the lead, as for the rate. Without this the floor leapt by how much earlier the other tracks are sent, which read as a source far faster than ours, and a lead sampled from one step froze that step's queueing into it.
Measurement fix: the clock is now steered every STEER of our clock, not every STEER of decode time. With decode-time steps, a brief network stall released a backlog that produced one low floor per 2 s of media it held. Early in a run, before the envelope's window fills, those floors made the rate look past the limit. With our-time steps, a backlog read at once gives one floor (its freshest frame's), and the stall only drops what it made late.
Docs: updated doc/bin/cli.md and the with_delay rustdocs (TS, FLV, MKV) to say a source the output cannot follow fails the export.

Public API / wire impact

Public API: none. jitter::Buffer is pub(crate). An export that used to degrade silently now returns an error.
Wire: none.

Tests

All run on mocked time (start_paused):

jitter::a_source_far_off_real_time_fails_within_seconds: a source 22% slow fails within 10 s of decode time, and the error names -220000.0 ppm. Fails without the fix: the old rule never fired in an hour.
jitter::a_source_past_the_drift_limit_fails: 40 and 400 ppm, in both directions, fail with the hold between 250 and 260 ms off the delay (half of 500 ms), before any frame goes late. Fails without the fix: the old rule let 40 ppm reach 270 ms.
jitter::a_late_start_within_the_drift_limit_never_fails: sources -20, 0 and +20 ppm whose first second reached us 300 or 450 ms late, with a few ms of jitter per steering step, never fail. This is the signature TS validation hit in CI (+483 and +80 ppm at the first measurement, about 4 s in).
jitter::closing_a_late_start_is_not_charged_to_the_rate: a 0 ppm source 450 ms late at the start, run for 16,000 s with 40 ms more latency over [14,000, 14,600) s, never fails. Fails without the startup-gap fix: -133.2 ppm at 14,302 s, as the review modeled.
jitter::the_track_sent_latest_stopping_keeps_the_clock: audio sent 700 ms after its video stops 60 s in, and video runs on for 240 s more; nothing fails and nothing drops, also when audio's last 4 s were queued 400 ms more. Fails without the floor fix: +11513.2 ppm at 63.3 s. Fails with a one-step lead: -6993.0 ppm at 113.7 s, as the second review modeled.
jitter::a_stall_drops_late_frames_and_recovers: stalls of 3, 10, 60 and 290 s (all under half of RATE_WINDOW), starting 5 s in (two floors), 9 s in and 1,200 s in. Each drops frames, never fails, and ends back on time. Fails with the rule change alone, without the our-time steps.
ts::export_timing_test::a_source_far_off_real_time_fails_within_seconds: an end-to-end TS export at 22% slow fails within 10 s of media and reports -220137 ppm.
ts::export_timing_test::a_source_past_the_limit_fails_once_half_the_delay_is_gone: the existing 400 ppm source fails within 15 min. The existing 5-minute test still shows it is only counted before that point.
the_clock_follows_a_drifting_source_for_a_day still passes: sources within 30 ppm never fail.

just check and just test ts pass.

Decisions

✅ Accepted by the maintainer: the slow-drain residual below ships as-is.
Maintainer decision on the two OpenAI P2s:
- ✅ Keep the narrowed rule (fail once the rate has cost half the delay, not counting the startup offset) and fix how it is measured.
- Abandon the narrowing and fail on the absolute gap again.
Shape of the measurement fix (picked by the agent as the simplest correct shape):
- ✅ Track the remaining startup gap: clamp it between zero and the current gap each step. Three lines, measured directly from the gap.
- Accumulate the rate's loss as (rate + drift) * elapsed minus the pull's share. Rejected: the pull and the rate share one slew-limited drift, so splitting it needs a shadow controller, and integrating the estimated rate drifts with its noise.
- Normalize every track by its lead every step. Rejected: taking the least of each track's change ratchets the floor down with noise; only the track sent least ahead of an unheard one is normalized.
Queueing-resistant lead (picked by the agent, after the review of 276ccd6):
- ✅ The median of each track's last 300 per-step leads, matching the rate's half-window resistance.
- A per-track upper envelope with the floor as the least of them. Rejected: it changes how every step's floor is measured, not just the unheard case.

Known limits

A stall whose backlog drains slowly is not covered. If the link delivers the backlog only 1.5x to 10x faster than real time, the floors during the drain still read as a slow source early in a run, and the export fails. In the real exports, a stalled group is skipped once it falls half the delay behind the newest content, so the old backlog is skipped and the current group arrives at once. That is the case the tests cover.
While the startup gap is open, a rate that closes it is credited to it. A slow source anchored late therefore fails once its slack is half the delay short, rather than once the rate has cost half the delay beyond its late start. That is still before any frame goes late.
A sustained change in a mux's lead takes half the window to show. A track sent latest that is queued for longer than that just before it ends still moves the floor.
A track's lead is kept after it ends, so the clock keeps measuring against it. If that track also leaves the catalog and the source then restarts, the new generation anchors without it, and the clock slowly raises the remaining tracks' hold by that lead. No frame drops.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated · 2026-10-14T02:13:03Z

Outcome: the quest is implemented, and just check passes. The PR stays a draft until it gets a review.

Open decision for the maintainer: a stall whose backlog drains slowly (about 1.5x to 10x real time) early in a run still trips the rule. I recommend accepting this as-is. The exports skip a stalled group once it falls half the delay behind, so in practice the backlog arrives at once, and telling a slow drain apart from a slow source would take a new heuristic beside the measured rate, which the quest rejected.

(Written by Claude Opus 5.5)

chatgpt-codex-connector · 2026-10-14T02:27:55Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T18:21:59.738696Z	`28a41f4`	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

coderabbitai · 2026-10-14T02:27:56Z

Warning

Review limit reached

You'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 29 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: d42f2bf2-0862-455c-a292-e743e0b10a58

📥 Commits

Reviewing files that changed from the base of the PR and between e8923bb and 28a41f4.

📒 Files selected for processing: 8

doc/bin/cli.md
quest/m1/README.md
quest/m1/slow-source.md
rs/moq-mux/src/container/flv/export.rs
rs/moq-mux/src/container/mkv/export.rs
rs/moq-mux/src/container/ts/export.rs
rs/moq-mux/src/container/ts/export_timing_test.rs
rs/moq-mux/src/jitter.rs

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

kixelated

Automated review by review (OpenAI)

Reviewed commit: aa4a91c.

Removing the drift-saturation prerequisite and sampling on read time are focused changes toward failing genuinely slow sources promptly. One P2 remains below: changes in which tracks contribute to the measurement can now abort healthy output. This is distinct from the explicitly accepted slow-backlog-drain limitation.

Verification: GitHub-only static inspection of the shared jitter estimator, envelope calculation, exporter lifecycle and regression tests. No tests or media runs executed.

kixelated · 2026-10-14T02:31:47Z

-			// At the limit and still losing ground, with half the delay gone.
-			if drift == MAX_DRIFT.copysign(slow) && gap * slow.signum() < -delay / 2.0 {
+			// The clock can never catch up, so fail once half the delay is gone.
+			if gap * slow.signum() < -delay / 2.0 {


[P2] Do not treat a disappearing track as source-clock acceleration

The floor takes the minimum phase only across tracks present in that step (548–552). With video arriving 700 ms ahead of audio, the clock anchors on audio; when audio pauses or ends, the first video-only floor rises by about 700 ms. The upper hull interprets that change of contributor as a positive rate, and this new condition aborts a 500 ms-delay export even though video remains exactly real-time. Exporters mark ended tracks without resetting these measurements. Normalize phases against learned per-track leads or otherwise preserve a consistent measurement baseline. Add a two-track regression that stops the latest-sent track and continues the other for several STEER intervals; existing short stop coverage cannot reach this check.

Fixed in 276ccd6. leads now holds each track's lead over the track sent latest, and a step that does not hear that track places it by its lead below the track heard that was sent least ahead of it, so the floor no longer leaps when it stops. the_track_sent_latest_stopping_keeps_the_clock stops audio sent 700 ms after its video 60 s in and runs video 240 s more; without the fix it fails at +11513.2 ppm.

(Written by Claude Opus 5.5)

The follow-up from the review of 276ccd6 is fixed in 28a41f4. A track's lead is now the median of its last 300 per-step leads (a window's worth), so a spell of queueing shorter than half the window does not move it, as for the rate. The placed floor also applies only when an unheard track is sent later than every track heard, so a step that hears every track is measured as before. the_track_sent_latest_stopping_keeps_the_clock now also queues audio 400 ms more over [56, 60) s before it ends. With a one-step lead it fails at -6993.0 ppm at 113.7 s, as you modeled.

(Written by Claude Opus 5.5)

kixelated

Automated review by review (OpenAI)

Reviewed commit: a0c3d7f, the startup-offset update since aa4a91c.

Excluding initial anchoring error is the right direction, and the new regression covers the initial false failure. However, the fixed baseline later counts the controller's own correction as source-rate loss (new P2 inline).

The earlier track-contributor-change finding remains open: #5490 (comment). Subtracting a first-measurement offset does not remove a phase jump when the lagging audio track later ends. I have not duplicated that comment or the explicitly accepted slow-backlog-drain limitation.

Verification: GitHub code inspection and a separate arithmetic model of the fetched envelope/steering equations. No repository tests or media runs executed; model timings below are approximate and omit exact frame-floor boundaries/nanosecond rounding.

kixelated · 2026-10-14T03:05:25Z

 		let slow = -rate;
 		let gap = at + self.steer.steered;
 		self.steer.source = Some(rate);
+		let lost = gap - *self.steer.offset.get_or_insert(gap);


[P2] Exclude the controller's startup correction from rate loss

The offset stays fixed while gap includes accumulated steering. Closing a healthy source's startup gap therefore becomes negative lost. For a 500 ms buffer with the new test's 450 ms late start and then 0 ppm input, about 14,000 s of permitted steering closes over 250 ms. A subsequent 40 ms arrival-delay increase for 600 s produces a transient slow envelope estimate; the model reaches roughly gap=+143 ms, lost=−307 ms and rate=−132 ppm, so this aborts although frames still have roughly 643 ms slack. This is bounded latency variation, not slow backlog draining. Track remaining startup correction separately. Extend the existing feed regression to 16,000 s with 450 ms startup lateness and an additional 40 ms delay during [14000,14600); the current 600 s test cannot reach this correction phase.

Fixed in 276ccd6. The startup gap is tracked on its own: it starts as the gap at the first measurement, is clamped between zero and the current gap each step, and so shrinks as the clock pulls it shut and never reopens. The rate is charged only the gap beyond it. closing_a_late_start_is_not_charged_to_the_rate runs your scenario (450 ms late start, 16,000 s, +40 ms over [14,000, 14,600) s); without the fix it fails at -133.2 ppm at 14,302 s.

(Written by Claude Opus 5.5)

kixelated · 2026-10-14T03:14:14Z

Merge summary for head a0c3d7f2e13fd0c62a758a49efb73967e92c0121:

A source past the 13818-1 drift limit fails the export once its rate has cost half the delay since the rate was first measured, naming the rate. Steering steps run on our clock, so a stall's backlog gives one floor.
✅ The maintainer accepted the slow-drain residual.
TS validation found that the accepted rule counted the startup anchoring offset as lost delay, which failed clean live sources at the first measurement. The second commit charges the rate only what it lost since it was measured, and adds a regression test that fails without it. CI, TS validation included, is green on this head.
Review: Codex finished on a0c3d7f with no findings.

Enabling auto-merge pinned to this head.

(Written by Claude Opus 5.5)

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

…ptly The jitter buffer failed a source past the 13818-1 drift limit only once its steered clock sat at the limit, which takes about three hours under the slew limit, so a grossly slow source (an importer 22% slow) dropped nearly every frame instead. It now fails once the measured rate is past the limit and half the delay is gone. Steering steps run on our clock rather than decode time, so the backlog a brief network stall releases gives one floor instead of one per step of the media it held; early in a run those floors read as a source far slower than ours and would have tripped the new rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TS validation failed a clean live source at its first rate measurement (+483 and +80 ppm, 4 s in). The clock had been anchored on frames that reached the export late at startup, so the slack sat 250 ms past the delay, and two floors a few ms apart over 2 s read as a rate far past the limit. Neither was the rate costing the delay. The failure now counts the gap the rate opened since it was first measured, so an anchoring offset, which the clock pulls shut, is not charged to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…steering The rate's loss was the gap less the gap at its first measurement. The gap includes the clock's own steering, so pulling a late start shut read as lost delay, and a later latency rise failed a healthy source hours in. The startup gap is now tracked on its own: it shrinks as the gap closes and never reopens. A track not heard in a steering step, as audio that ended before its video, is now placed by its lead below the tracks heard, so the floor no longer leaps by how much earlier the others are sent, which read as a fast source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated

Automated review by review (OpenAI)

Reviewed commit: 276ccd6.

The startup-offset finding is addressed by the shrinking remainder and its targeted regression. Normalizing missing-track floors also fixes the steady-lead example, but the existing disappearing-track P2 remains partially open: #5490 (comment).

At rs/moq-mux/src/jitter.rs:568–569, the final step's temporary queueing becomes a remembered mux lead. Extend the new two-track test: audio normally arrives 700 ms after video; add another 400 ms only during audio decode [56,60), then end audio at 60 s while video continues at real time. The saved video lead becomes 1.1 s, pinning subsequent video-only floors roughly 400 ms low. An arithmetic model of the current equations then aborts around 114 s as a false slow source (approximately −6993 ppm, −414 ms lost). No-final-queueing and continuing-audio controls do not fail. Preserve a queueing-resistant lead estimate and cover this transition; I am referencing the original finding rather than adding a duplicate inline comment.

Direction: the startup correction is simpler and sound; the disappearing-track fix still needs to distinguish transient queueing from lasting mux lead. This is separate from the explicitly accepted slow-backlog-drain limitation.

Verification: rebase-aware GitHub inspection and a separate arithmetic model, not executed Rust tests or media. Model timings are approximate; CI results were not treated as proof.

A lead was the last step's sample, so queueing on the track sent latest in its final step before it ended was frozen into the other tracks' leads, and the floors after it read as a slow source. Each lead is now the median of a window's worth of steps, so a spell of queueing shorter than half of it does not move it, as for the rate. The placed floor only applies when a track sent later than every track heard is not heard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kixelated · 2026-10-14T18:18:11Z

@codex review

Requesting a review of head 28a41f4, which fixes both OpenAI P2s (startup-gap clamp, per-track median lead) with regressions.

(Written by Claude Opus 5.5)

chatgpt-codex-connector · 2026-10-14T18:21:57Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 28a41f4e95

ℹ️ 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".

kixelated · 2026-10-14T18:22:48Z

Merge summary for head 28a41f4e959019a4f9f278ffc7b8e7f9766469e5:

✅ Maintainer kept the narrowed rule: a source fails once its rate has cost half the delay, not counting the startup offset.
The startup gap is tracked on its own (clamped between zero and the current gap, never reopening), so steering that closes a late start is not charged to the rate. Regression: closing_a_late_start_is_not_charged_to_the_rate (16,000 s run, late latency bump).
A track that stops no longer shifts the floor: leads are each track's median lead over the latest-sent track across one rate window. Regression: the_track_sent_latest_stopping_keeps_the_clock.
Slow-source failure tests still fail promptly. Codex reviewed this head with no findings.

(Written by Claude Opus 5.5)

kixelated force-pushed the quest/m1/slow-source branch from 73feef9 to aa4a91c Compare October 14, 2026 02:27

kixelated marked this pull request as ready for review October 14, 2026 02:27

kixelated commented Oct 14, 2026

View reviewed changes

kixelated added this pull request to the merge queue Oct 14, 2026

kixelated removed this pull request from the merge queue due to a manual request Oct 14, 2026

kixelated and others added 4 commits October 14, 2026 09:02

quest(m1): claim slow-source

7578584

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

kixelated force-pushed the quest/m1/slow-source branch from a0c3d7f to 276ccd6 Compare October 14, 2026 16:48

kixelated commented Oct 14, 2026

View reviewed changes

kixelated added this pull request to the merge queue Oct 14, 2026

Merged via the queue into main with commit 41f3087 Oct 14, 2026
7 checks passed

kixelated deleted the quest/m1/slow-source branch October 14, 2026 18:39

This was referenced Oct 14, 2026

fix(mux): consume export gaps and ends in read order #5529

Merged

quest: import #5521, #5563, #5564 and #5565 #5572

Merged

fix(mux): redundant TS failover survives a killed importer behind bufferbloat #6113

Merged

Conversation

kixelated commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Public API / wire impact

Tests

Decisions

Known limits

Uh oh!

kixelated commented Oct 14, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated commented Oct 14, 2026

Uh oh!

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated commented Oct 14, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 14, 2026

Uh oh!

kixelated commented Oct 14, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kixelated commented Oct 14, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 14, 2026 •

edited

Loading

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading