moq-lite-05: REANNOUNCE, timestamps, and datagrams#23
Conversation
- Adds `restart` ANNOUNCE status (2) for atomic UNANNOUNCE+ANNOUNCE when origin or hop path changes without interrupting the broadcast. - Adds `Publisher Timescale` to SUBSCRIBE_OK and per-frame `Timestamp Delta` (zigzag varint) to FRAME, enabling timestamp-based expiration instead of wall-clock arrival time. - Adds QUIC datagram delivery for single-frame groups, routed via existing Subscribe IDs (no separate control stream). Datagram bodies are capped at 1200 bytes to fit within the minimum QUIC path MTU. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 55 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis draft updates moq-lite's protocol semantics and wire format by introducing explicit per-frame timestamp support with Timescale-based expiration, adding datagram delivery as an alternative to stream-based group transport, and extending ANNOUNCE messaging with an atomic 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@draft-lcurley-moq-lite.md`:
- Line 730: Replace the phrase "An application specific payload." with the
hyphenated compound adjective "An application-specific payload." — locate the
exact sentence in draft-lcurley-moq-lite.md (the line containing "An application
specific payload.") and update it to use the hyphenated form for consistency and
readability.
- Around line 390-393: The spec currently treats datagram field Timestamp=0 as
“unspecified,” which prevents representing a true absolute timestamp of value 0
for Tracks with non‑zero Timescale; update the Timestamp definition (the
datagram/field named "Timestamp" in the Track description) so that “unspecified”
is not conflated with the numeric zero—either make unspecified conditional on
Timescale=0 (i.e., if Timescale==0 then Timestamp MUST be 0 to mean unspecified)
or add an explicit validity flag/parameter (e.g., a TimestampValid bit)
alongside Timestamp to indicate whether the numeric value is meaningful; change
the prose and any examples to reflect the new rule and ensure all references to
Timestamp and Timescale in the document use the new semantics.
- Around line 472-476: Update the ANNOUNCE state-machine alternation rule so it
explicitly allows the `restart` status alongside `active` and `ended`: modify
the sentence that currently requires publishers to “MUST ONLY alternate ...
active to ended or vice versa” to mention `restart` (e.g., allow transitions
between `active`, `ended`, and `restart` or state that `restart` may replace an
`active` announcement atomically). Ensure references to `restart`, `active`, and
`ended` in the ANNOUNCE section and any normative MUST language are updated so
the rules are consistent with the `restart` definition in the `restart` (2)
paragraph.
- Around line 332-336: The expiration definition is ambiguous for Groups that
contain zero FRAME messages; update the text around the Group/Frame/Max Latency
explanation so that when a Group has no FRAMEs its “first frame” timestamp is
treated as the group arrival/queue time (subscriber arrival or publisher queue
time) for the purpose of computing the delta used by Track (respecting Track
negotiation of Timescale), i.e., if Timescale is non-zero but the group is
empty, fall back to arrival/queue time; ensure the new normative sentence
references Group, Frame, Max Latency, Track and Timescale and clarifies this
fallback rather than leaving it undefined or exempting empty groups.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15a5e7d0-a2bf-456f-9add-608867a569c8
📒 Files selected for processing (1)
draft-lcurley-moq-lite.md
- Define expiration fallback for zero-frame groups (use wall-clock arrival/queue time regardless of Timescale, so empty keep-alive or gap-marker groups don't stall expiration). - Datagram Timestamp field: clarify that 0 is only "unspecified" when the Track's Timescale is 0; otherwise 0 is a legitimate absolute timestamp. - Resolve ANNOUNCE alternation-rule contradiction: the rule now allows `restart` alongside `active`, matching the `restart` (2) definition. - Hyphenate "application-specific" in FRAME payload description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Three additions for moq-lite-05:
1. REANNOUNCE (atomic re-announcement)
A new
restart(2) value forAnnounce Statuson the existing ANNOUNCE message. Semantically equivalent toendedimmediately followed byactivefor the same path, but signals that the broadcast was never actually unavailable — useful when only the origin or hop path changes (relay failover, upstream restart). The Hop ID list may differ from the prior announcement.Alternation rules updated: a broadcast cycles
ended↔ (active|restart). Arestartwithout a prioractiveis a protocol violation.2. Timestamps and per-track timescale
Based on moq-dev/moq#1439.
Publisher Timescale (i)added to SUBSCRIBE_OK. Units per second;0means unspecified. Fixed for the lifetime of the subscription.Timestamp Delta (i)prepended to every FRAME — zigzag-encoded signed delta from the previous frame's timestamp (first frame deltas from0).3. QUIC datagram delivery
Based on moq-dev/moq#1374, but with a different shape per discussion: no separate DATAGRAMS control stream. A publisher MAY send any group as one QUIC datagram routed via the existing Subscribe ID, so datagrams and streams coexist on a single subscription.
Datagram body:
Subscribe ID (i) | Group Sequence (i) | Timestamp (i) | Payload (b). Each datagram is a complete group with exactly one frame; not cached, not retransmitted. Total body capped at 1200 bytes to fit within the minimum QUIC path MTU without IP fragmentation — oversize payloads must be sent as a Group Stream instead. Subscribers must deduplicate by group sequence when receiving both delivery paths.Open design choices worth a second look
restartvsreannounce— happy to rename.Timestamp(PR 1374 omits it) so timestamp-based expiration also works for datagram-only groups.Test plan
make draft-lcurley-moq-lite.txtbuilds cleanly.restartnaming and the datagram-MTU cap.🤖 Generated with Claude Code