Repository navigation
feat(lite): lite-07 carries an untimed track untimed - #5461
Conversation
|
Local results on b7c477e:
Open decisions for the maintainer (both kept as recommended in the PR body): (Written by Claude Opus 5.5) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On moq-lite-07 an untimed track declares a Timescale of 0 in TRACK_INFO, and its FRAME messages and datagrams omit the timestamp, so absence survives a lite relay hop. lite-05/06 keep declaring milliseconds and stamping send times, the documented downgrade for old peers. Rust and JS share one rule, `wire_timescale` / `wireTimescale`, for the timescale a version carries. The datagram decoder asks the subscription whether a timestamp follows, since the body does not say. 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 3 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 |
66546fa to
e5dadbc
Compare
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: 66546fac7b
ℹ️ 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".
| w.varint(self.subscribe)?; | ||
| w.varint(self.sequence)?; | ||
| w.varint(self.timestamp)?; | ||
| if let Some(timestamp) = self.timestamp { |
There was a problem hiding this comment.
When timestamp is None with Lite05 or Lite06, this encoder silently omits a field that those versions require, so a compliant receiver consumes the first payload bytes as the timestamp and corrupts the payload or rejects it. The new lite_varint_interop test exercises exactly this invalid Lite06 encoding, and the mirrored JS encoder accepts it too, so the round trip hides the malformed wire data. Reject None before Lite07 rather than serializing it.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8703633. Both encoders (Rust lite::Datagram and JS Datagram) now refuse a missing timestamp on lite-05/06, with a unit test each, and lite_varint_interop only exchanges untimed datagram and GROUP bytes on lite-07, asserting the lite-06 encode is refused.
(Written by Claude Opus 5.5)
| while (!stop) { | ||
| producer.appendDatagram(undefined, new Uint8Array([7])); | ||
| await new Promise((resolve) => setTimeout(resolve, 2)); | ||
| } |
There was a problem hiding this comment.
On a slow or loaded CI worker, this test repeatedly sleeps for 2 ms until subscription setup happens and has no bound if no datagram arrives. A regression can therefore leave the pump running indefinitely instead of producing a deterministic failure, and scheduler timing can make the test unnecessarily flaky. Synchronize subscription establishment or use mocked scheduling instead of polling wall-clock time.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8703633. The test now writes one group and waits for the subscriber to read it, which proves the subscription is live end to end, then sends a single datagram. No timer or loop remains.
(Written by Claude Opus 5.5)
lite-05/06 always carry a datagram timestamp, so encoding one without it produced bytes a receiver would misread. Both encoders now refuse it, and the byte interop only exchanges untimed bytes on lite-07. The JS untimed datagram test now waits for a group to prove the subscription is live instead of pumping datagrams on a wall-clock timer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge summary for 8703633:
(Written by Claude Opus 5.5) |
Quest:
quest/m1/lite-untimed.md(deleted here, references removed).Summary
On moq-lite-07-wip an untimed track now crosses the wire untimed in Rust and JS, so absence survives a lite relay hop:
Timescaleis0for an untimed track.0was already invalid, so no shift or new field is needed.Timestamp Deltaand the DATAGRAM body omitsTimestampon an untimed track. The datagram body does not say which, so the decoder asks the subscription named bySubscribe ID, the same way FRAME already relies on TRACK_INFO.1000and stamp each payload with its send time. A0there is still refused.Rust (
lite::Version::wire_timescale) and JS (wireTimescale) share one rule for the timescale a version carries. TRACK_INFO, SUBSCRIBE serving, FETCH serving, and datagrams all go through it, replacing the scatteredunwrap_or(MILLI)fallbacks.Draft:
draft-lcurley-moq-lite.mddescribes the untimed encoding and the lite-05/06 downgrade, marks[Timestamp Delta]and[Timestamp]optional, drops "any varint value (including 0) is valid" and the "every frame carries a presentation timestamp" text, says an untimed group never expires, and adds lite-07 changelog entries.doc/concept/moq-lite.mdis updated to match.Public API and wire impact
lite::TrackInfo,lite::Datagram, and the JS lite wire classes are crate/package-internal.Tests
rs/moq-net/tests/untimed.rsnow expects lite-07 to be faithful for frames, datagrams, and FETCH, plus a lite-07 to lite-07 relay chain.js/net/src/untimed.test.tsflips lite-07 to unstamped and adds an untimed datagram round trip on lite-06 and lite-07.0on lite-07 and milliseconds on lite-05/06,0is refused on lite-05/06, and an untimed datagram has no timestamp bytes.lite_varint_interop) now also round-trips an untimed datagram, an untimed GROUP with its frames, and an untimed TRACK_INFO on lite-06 and lite-07, in both directions.just checkpasses.just drafts checkpasses.just test interop --allpasses on 8703633: 63 cells, 0 failures.Decisions
Timescale = 0in TRACK_INFO is the untimed signal (maintainer accepted). It was already an invalid value and costs no bytes; no separate flag.Subscribe ID, matching FRAME, rather than spending a flag byte on every datagram.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)