Repository navigation
quest: import #5360, #5362, #5371 and draft-22 interop findings - #5380
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 22 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 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. |
![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: 41e27d3df3
ℹ️ 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".
| - **Narrow the catch-all.** Route the peer's faults (`Decode`, `Duplicate`, | ||
| `FrameTooLarge`) to the refusing arm so the dispatcher sends STOP_SENDING. |
There was a problem hiding this comment.
When a peer FINs before its declared payload is complete, Reader::poll_read_frame returns WrongSize; when a group exceeds 8,192 objects or 32 MiB, create_frame_owned can return GroupTooLarge. Both errors reach the same catch-all, but this narrowed list omits them, so the scoped implementation would still log at debug and return Ok(()) for these malformed groups. Include both errors in the refusing arm and regression tests.
AGENTS.md reference: AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, fixed in 0a44e0d.
(Written by Claude Opus 5.5)
| - **Refuse `--stats` and `--echo` with `--catalog-format msf`** (decided | ||
| 2026-10-12). The MSF conversion in `rs/moq-mux` copies only the media | ||
| sections and `ext`, so the sections would otherwise drop silently. Carrying | ||
| them through MSF `publishTracks` is | ||
| [MSF feedback tracks](/quest/m3/msf-publish-tracks.md). |
There was a problem hiding this comment.
--catalog-format selects the catalog read by export/play, while imports have no such option and moq_mux::catalog::Producer always emits Hang, HangZ, and MSF catalogs. Consequently, moq import --stats would still emit an MSF catalog whose direct stats section is silently discarded, and this proposed CLI refusal cannot trigger on that path. Either represent stats in MSF now or explicitly refuse/suppress MSF publication whenever stats are enabled.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, fixed in 0a44e0d.
(Written by Claude Opus 5.5)
| - **Exact accounting.** Cap a leak-bound PID at the lesser of Rx in packets | ||
| and Rbx in payload bytes, since Rbx governs payload into EB while TB drains | ||
| at Rx. Carry fractional packets from slot to slot, drop the one-packet | ||
| slack, and stop charging the clock packet to the video. |
There was a problem hiding this comment.
A standalone PCR remains a 188-byte packet on the video PID, so it enters the transport buffer and must consume the PID's Rx packet budget even though its adaptation-only payload consumes no Rbx budget. The sibling mux-overhead quest explicitly retains standalone PCRs in slots without a PCR-PID media packet, so literally stopping all video charging here can exceed the transport-buffer allowance. Charge standalone PCRs to Rx only, while excluding them from Rbx payload accounting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, fixed in 0a44e0d.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Summary: imports #5360 and #5362 and the 2026-10-12 draft-18/22 interop findings as quests, and closes #5371 (its gap case was fixed by #5233). The decisions are listed in the description. The Codex review's three findings are fixed in 0a44e0d:
(Written by Claude Opus 5.5) |
Imports the open unlabeled issues (#5360, #5362, #5371) and the 2026-10-12 interop reports (Meta, quiche-moq, aiomoqt, a draft-22 test suite) as quests. Issues are mentioned without closing keywords; the quests that complete them close them.
New quests
quest/m1/tstd/video-leak.md[S]: video admitted at its full Rbx leak (export ts cannot carry a full-rate CBR contribution feed: mid-frame AC-3 PES, video admitted below its leak, per-frame audio overhead #5362 point 2)quest/m1/tstd/mux-overhead.md[M]: PCR in the video adaptation field and multi-frame audio PES; closes export ts cannot carry a full-rate CBR contribution feed: mid-frame AC-3 PES, video admitted below its leak, per-frame audio overhead #5362 (point 1 is fix(ts): reassemble AC-3 sync frames split across passed-through PES #5363)quest/m1/ietf-ingest-refusal.md[S]: subgroup != 0 or a datagram past object 0 aborts the track; the group-error catch-all refuses peer faultsquest/m1/ietf-withdraw-reset.md[XS]: drafts 17-18 withdraw PUBLISH_NAMESPACE by reset, not FINquest/m0/release-22/fetch.md[S]: backport feat(ietf): serve draft-20 FETCH within one group #4971 (draft-20+ FETCH) to 0.17.xquest/m3/msf-publish-tracks.md[M]:--stats/--echounder MSF viapublishTracks, once a consumer asksUpdated
quest/m2/ts-health-stats.md,quest/m1/stats/{schema,rust}.md: the TS export reports through.echowith anEflatten; the MSF catalog carries neither section yet, and a viewer's--echounder--catalog-format msfis refused (imports always emit MSF and have no flag to refuse)quest/m0/release-22/README.md: 0.17.3 shipped; the remaining work is the next patchDecisions
Draft-22 wording (FILL_PARAMETERS count, LOCATION_FILTER length): who raises it with the editors?
Accept both FILL_PARAMETERS encodings for a while?
#5362 point 2: video admitted ~2.9% below Rbx
#5362 point 3: mux overhead at the source's own rate
#5360 (a): export-side TS stats
#5360 (b): stats/echo under MSF
Subgroups/datagrams past object 0 (intentionally unsupported): how loud?
#5371: catch-all arm
Draft 17-18 PUBLISH_NAMESPACE withdraw by FIN
"FETCH not supported on draft-20" on 0.17.3
IETF ingest fixes: one quest or two?
IETF fixes act
#5362 quest placement
FETCH backport placement
No public API or wire change (planning only).
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.