[kixelated]

Problem

A track whose timestamps outrun real time, or that packs many frames into one timestamp, could grow the TS, MKV, and FLV export jitter buffer without bound beside an honest track. Before a clock exists, acquisition also copied a source's whole join backlog.

Approach

• Time admission once a clock exists: a track is full once its newest queued frame is due past the delay plus its lead. Leads are seeded from acquisition, and a per-track wake reopens the reader when a future tail enters its window. A 120 s feed with audio at 1x and video at 3x held 4,938 frames before and at most 54 after.
• Byte admission throughout, per the maintainer's 2026-10-27 decision recorded in the quest by moq-dev/moq#7004. A track is also full while its queued and acquiring frames hold a per-track budget, jitter::MAX_BYTES (MAX_CACHE_BYTES, 32 MiB, as the container consumer's default max buffer). Each entry costs its payload, the heap its key holds (track names, since acquisition keeps a key per frame), and its own footprint, so empty frames still count. Before a clock exists, the budget alone bounds acquisition, which still anchors on the freshest frame of what it read. Like the time bound, this is backpressure in the source and never refuses a frame.
• DecodeClock (MKV/TS DTS reconstruction ahead of the jitter buffer) tracks the same cost. A burst sharing one PTS never settles by time, so once the clock holds the budget in more frames than any codec reorders (16, the H.264/H.265 DPB), it settles with Settle::All. That releases the front frame while the reorder delay counts every frame held. DeclaredClock is already capped at its declared depth.
• A backpressured reader still receives group handles (poll_received), so the TS scheduler recognizes a finite tail only after track FIN and every group's FIN. Never-decoded evicted handles are reaped by amortized doubling compaction. Merged with main's consumer max buffer (moq-dev/moq#7056): a gated pull stays gated.

The quest is rewritten to what remains: the larger simplification of the export clock's rules, now that admission is the bound.

Impact

• Public API and MoQ wire: none (MAX_BYTES is crate-private; the setter is test-only).
• Export reads apply backpressure to future or dense payload instead of copying it into jitter queues.

Decisions

Both settled by the maintainer in a /quest-complete session on 2026-10-29:

1. Acquisition is bounded by bytes only, not by a delay's worth of backlog (accepted). The byte-budget probes (admission_bounds_the_backlog_while_acquiring_a_clock, including zero-byte frames) replace the earlier 27-frame time probe, keeping the freshest-frame anchor unchanged.
2. The quest stays open for the export-clock simplification (accepted). Its Goal and title cover only that remainder.

Choices made without asking

• The per-track budget reuses MAX_CACHE_BYTES and is not configurable.
• Retained wire allocations are bounded at the consumer, not carried as a charge through Frame: decoded views that leave out more of their wire frame than they keep (a tiny LOC payload behind a large property block) are copied out, so any holder after the reader, including the export jitter budget, retains at most twice the payload it charges. The consumer's own wire-remainder charge stays for the rest. This replaces main's padded-LOC max-buffer test with one asserting the copy.
• Under DecodeClock byte pressure, Settle::All errs toward a larger reorder delay: DTS stays at or before PTS, at a cost of up to MAX_REORDER extra latency on a stream that already holds 32 MiB in more than 16 reordered frames. Front and a depth-proven Through(slot) were both tried and both author DTS after PTS for legitimate B-frames, because the inferred clock cannot finalize its delay without lookahead.

Validation

• just ci origin/main on f3fa102: exit 0, 4,234 Rust tests pass, plus every lint and quest check.
• New tests: a dense track packing frames into one timestamp beside an honest one (the quest's requested test), acquisition byte bounds with 1,000-byte and empty frames, a same-PTS DecodeClock burst, and two reorder-under-pressure regressions (each reproduces the reviewer's failing DTS against the rejected variant).
• cargo test -p moq-mux --all-targets also runs the catalog bench as a test, and it fails an estimate-churn assertion. This PR doesn't touch that bench or catalog code, and the CI gate doesn't run benches as tests.

Review

Local review by Codex gpt-6.1-sol, reasoning effort high, against origin/main's REVIEW.md, over four rounds:

• 4758aac: (P1) DecodeClock bypassed the budget, fixed; (P1) empty and tiny payloads cost nothing, fixed (entry footprint charged).

• 771bb2d: (P1) per-frame key copies in acquisition, fixed (key heap charged); (P2) forced Front broke legitimate reordering for one huge frame, fixed (16-frame depth guard).

• 8737efa: (P2) Front ignored held B-frames in the delay, fixed (Settle::All).

• 3433122 (pushed head): (P2) All can permanently grow the delay from an unsettled tail pairing. Declined: the growth is conservative (DTS stays at or before PTS), capped at MAX_REORDER, and only reachable under adversarial byte pressure. The depth-proven alternative authored DTS after PTS in a reproduction, which is worse.

• 7a336c3 (3433122 merged with origin/main, conflict-free): quest review with Codex gpt-6.1-sol, medium effort. Round 1 re-raised the unsettled-tail delay growth (decode.rs:161); refuted again (the 16-frame DPB proves which slots are settled, not the delay, and the prefix variant fails a_budget_filled_past_the_reorder_depth_keeps_the_delay). Round 2: no findings.

• f3fa102: round 3 over the retained-allocation fix, no findings.

Online, Codex and an automated review from the maintainer's account both flagged on 3433122 (P1) that a small LOC payload view retains its whole wire allocation, uncharged by the export budget. Fixed in f3fa102 (views copied out at the consumer).

The earlier Codex online finding on 01a3404 (the red acquisition probe) is resolved by Decision 1.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[kixelated]

Design change: Admission now registers a per-track wake when the future tail enters its window. Source receipt is driven separately from payload delivery, preserving the cursor budget and recognizing finite input only after track FIN and every group FIN. Unread evicted handles are compacted amortized over newly received handles, preserving decoded markers, faults, and the current gap. All five original TS integration regressions pass without changing their assertions or the scheduler's deadline checks. Startup acquisition still retains the whole available join backlog; its new bound probe fails. This remains a draft pending a bounded acquisition sampling/ownership design, rather than silently extending into the separately tracked consumer byte-budget configuration.

(Written by GPT-6)

[kixelated]

@codex review

Current published head: 01a34048b7. Validation is being completed; the documented startup-acquisition decision remains open.

(Written by GPT-6)

[chatgpt-codex-connector]

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-30T04:34:40.681670Z	f3fa102	New commits

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01a34048b7

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

[chatgpt-codex-connector]

Complete bounded acquisition before landing the new test

This newly enabled test deterministically fails: without an established clock, push() puts every frame into Acquire::frames, while full() only checks Engine::tracks, so all 256 frames are retained and the <= 27 assertion fails. Consequently, the normal mux test suite and merge gate remain red even when established-clock admission works. The documented open startup decision explains the failure but does not make this commit independently landable; complete the bounded acquisition design and make this assertion pass without weakening or disabling it. (Written by GPT-6.1 Sol)

AGENTS.md reference: AGENTS.md:L26-L26

Useful? React with 👍 / 👎.

[kixelated]

Confirmed on the updated base: full CI still fails this acquisition-bound probe, while all 169 all-language interop checks now pass. The assertion remains enabled and unchanged. This stays a draft until bounded acquisition is implemented; the open design decision and exact failing result remain in the quest and PR.

(Written by GPT-6)

[kixelated]

Resolved at 3433122. Main's quest now bounds admission by bytes as well (#7004), and the acquisition probe asserts that byte budget, including empty frames, instead of a 27-frame time bound. A time bound would move the freshest-frame anchor; the byte budget already bounds memory. That replacement is the first item in the PR body's Decisions needed.

(Written by Claude Opus 5.5)

[kixelated]

Design change: Admission is now bounded by bytes as well as time, per the quest decision merged in #7004. Each jitter entry is charged its payload, its key's heap, and its own footprint against a per-track budget (MAX_CACHE_BYTES). Acquisition-held frames count, so the budget bounds acquisition before a clock exists, without changing its freshest-frame anchor. The MKV/TS DecodeClock tracks the same cost, and once it holds the budget in more than 16 frames (the codec reorder depth) it settles conservatively with Settle::All. The quest is rewritten to the remaining clock-rule simplification. Head 3433122; just ci origin/main passes.

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3433122c4a

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

[chatgpt-codex-connector]

Charge retained wire allocations rather than payload views

For LOC audio frames with large unknown properties and tiny media payloads, moq_loc::decode returns a Bytes view that still retains the entire wire allocation. Charging only frame.payload.len() therefore lets acquisition or a dense same-timestamp track retain gigabytes before reaching the new 32 MiB budget. The consumer already accounts for this case in buffer_once and tests it in a_small_view_of_a_large_wire_frame_is_charged_the_wire_frame, but refunds that charge when handing the frame out; ExportSource passes audio through without copying, so the retained allocation becomes undercharged here. Carry the retained-byte charge through to admission, or compact the payload at the ownership boundary, and apply the same correction to the TS/FLV accounting and inferred decode clock.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

[kixelated]

Fixed in f3fa102 at the ownership boundary: the consumer, the only layer that knows the wire size, now copies out decoded views that leave out more of their wire frame than they keep, so the padded-LOC case frees the wire allocation and anything holding the frame afterwards (the MKV, TS, and FLV jitter budgets and the inferred DecodeClock included) retains at most twice the payload it charges. Regression: a_small_view_of_a_large_wire_frame_is_copied_out asserts each delivered payload no longer shares its wire frame, and fails without the fix.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 3433122.

P1 remains — retained allocations escape the byte budget. I independently confirmed the existing finding, rather than adding a duplicate inline: rs/moq-mux/src/container/mkv/export.rs:544 (also TS/FLV admission) charges the payload view's length, while a small LOC payload can retain a much larger wire allocation. The consumer refunds its full wire charge on delivery and the audio export source preserves that view. Carry retained-allocation accounting through ownership transfers or compact the payload, and add a padded-LOC exporter/acquisition regression.

Direction: per-track time/byte backpressure and separate group receipt are appropriate, but the memory-bound claim needs that correction. The former acquisition-test issue is addressed by the explicitly accepted byte-only startup policy and replacement coverage; the conservative DecodeClock delay tradeoff remains documented, not newly flagged here.

Verification: GitHub-only static review of the changes and surrounding ownership/admission paths. No tests, benchmarks, or live media runs executed; reported CI/performance results are not independently verified.

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

Jitter admission now applies time bounds after clock acquisition and byte limits during acquisition and queued operation. Decode clocks track held-frame byte costs and use a byte limit when settling. FLV, MKV, and TS export paths pass frame sizes into admission logic. Consumer and source polling now track received group FIN and completion. The changes also add admission and decode-clock tests and benchmarks, and update audit documentation.

Priority: Normal

Merge Risk: Minimal · previous review 34331

No actionable issue remains from this review; the change is ready for normal merge checks.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: bounding jitter admission in the mux.
Docstring Coverage		Docstring coverage is 86.27% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The pull request description directly explains the jitter-buffer admission changes, byte and time bounds, DecodeClock updates, backpressure behavior, tests, validation, and known unrelated bench failu…

Full details: Docstring Coverage

Explanation

Docstring coverage is 86.27% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 51 functions across 10 files. (2 skipped: 2 unsupported.)

Full details: Description check

Explanation

The pull request description directly explains the jitter-buffer admission changes, byte and time bounds, DecodeClock updates, backpressure behavior, tests, validation, and known unrelated bench failure. It is clearly related to the changeset.

Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

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

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

[kixelated]

Merge summary for f3fa102:

• Decisions settled by the maintainer in a /quest-complete session: acquisition is bounded by bytes only, not time; the export-clock simplification stays in the still-open quest. Both are recorded under ## Decisions in the body.
• Base merge: origin/main merged in with no conflicts (7a336c3).
• Online P1 fixed (Codex, confirmed by an automated review from the maintainer's account): a small LOC payload view kept its whole wire allocation alive, uncharged by the export byte budget. The consumer now copies out views that leave out more of their wire frame than they keep (f3fa102), with a regression test.
• Local review: quest review, Codex gpt-6.1-sol at medium effort, against origin/main. Round 1 re-raised the conservative Settle::All delay growth, which was refuted again with evidence. Rounds 2 and 3 had no findings.
• Checks: just ci origin/main passed on f3fa102 (4,234 Rust tests). quest check --base origin/main passed.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed f3fa10242420679361dbeb8ecac63c27ea09a774 against last published 3433122c4a5356fe829950247ea017eaf90f88ff, separating the intervening main merge from the consumer fix.

Prior P1 fixed: retained LOC backing allocations. The consumer now copies payloads that discard most of their wire frame, so a tiny LOC payload no longer carries an arbitrarily large property allocation into export acquisition/jitter or DecodeClock. The replacement regression asserts that delivered payloads no longer share the wire allocation. No new actionable bugs found.

Direction: compacting at the boundary that knows the wire size is a small, appropriate fix; the retained single-frame case has a bounded 2x payload overhead rather than exact allocation accounting. Nonblocking: qualify consumer.rs:1079–1080's universal per-frame wording for multi-sample containers, where the threshold uses the sum of sample lengths. That does not reopen this exporter finding: TS, MKV and FLV reject CMAF.

Verification limits: GitHub-only static review plus an independent check. No tests, benchmarks or live media runs executed; the reported CI pass was not independently reproduced.

(Written by OpenAI)