docs(drafts): spec the timeline track and compressed catalog in moq-hang - #2732
Conversation
The hang implementation publishes a per-rendition timeline (the Timeline
catalog section on VideoConfig/AudioConfig plus the companion track of
{group, pts} records) and a compressed catalog.json.z sibling, but the
draft documented neither. Add a Timeline section (catalog schema, wall
clock anchor, track format), a Compression section (the shared-window
sync-flushed DEFLATE framing both .z tracks use), the timeline common
rendition field, and the compressed catalog sibling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe draft adds a recommended compressed 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@drafts/draft-lcurley-moq-hang.md`:
- Around line 392-396: The timeline specification must reconcile throttled
record emission with its duration model: define how consumers determine or
interpolate omitted group boundaries when consecutive emitted records span
multiple groups, including lookup rules, and specify how the final record
without a following PTS represents the live edge. Update the surrounding
group-duration and sequence-number semantics accordingly.
- Around line 358-374: The Timeline contract must require timescale to be a
positive integer and define how legacy microsecond media timestamps are
converted to Timeline.pts using the declared timescale, including the required
rounding rule. Update the Timeline documentation near the timescale and pts
definitions, including the related section around the legacy container timestamp
handling, so publishers and consumers use identical conversion semantics.
- Around line 104-106: Update the catalog track description to explicitly define
that catalog.json.z contains the same group-ordered snapshot and RFC 7396 JSON
Merge Patch delta framing as catalog.json, with only compression differing.
Specify the consumer behavior when the compressed track is unavailable,
including fallback to catalog.json.
🪄 Autofix
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 Plus
Run ID: 7140fb4c-67b6-404a-b143-6c1b3ee3502f
📒 Files selected for processing (1)
drafts/draft-lcurley-moq-hang.md
![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: 3b44f17f9e
ℹ️ 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".
Fix the timeline track's placement (it follows the rendition's broadcast field, matching the moq-hls consumer), require a positive timescale (Timescale is NonZero and the consumer errors on 0), state the round-down pts conversion (Timestamp::as_scale truncates), bound timeline integers to 2^53-1 (the JS schema's u53), define record spans under throttling plus the live-edge semantics of the last record, and note the compressed catalog shares the plaintext track's group/frame rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a087bf9097
ℹ️ 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".
| Measuring from 2020 rather than 1970 keeps the values small. | ||
| A consumer derives the wall-clock time of any group as `wall + pts`. | ||
|
|
||
| Timeline integers (`timescale`, `wall`, and each record's `pts`) MUST NOT exceed 2^53 - 1, so they survive JSON consumers that parse numbers as IEEE 754 doubles. |
There was a problem hiding this comment.
When a publisher chooses timescale = 2^32, the value is permitted by the new 2^53 - 1 ceiling and accepted by the JS TimelineSchema, but rs/hang/src/catalog/timeline.rs::Timeline.timescale is a u32, so Rust rejects the entire catalog during deserialization. Cap this field at 2^32 - 1, or widen the Rust field before standardizing larger values. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
|
|
||
| The `wall` field anchors the timeline to the wall clock, if known: the wall-clock time of timestamp 0, in `timescale` units since the moq epoch, 2020-01-01T00:00:00Z (1577836800 Unix seconds). | ||
| Measuring from 2020 rather than 1970 keeps the values small. | ||
| A consumer derives the wall-clock time of any group as `wall + pts`. |
There was a problem hiding this comment.
When both operands approach the new ceiling, the required wall + pts calculation can leave JavaScript's safe-integer range even though each operand conforms; for example, (2^53 - 1) + 2 is rounded. The new individual-value bound is fresh evidence that the previously reported arithmetic issue remains, so constrain the sum rather than only its operands or consumers can derive an incorrect wall-clock time. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
|
|
||
| ~~~ | ||
| type Record = { | ||
| "group": number, |
There was a problem hiding this comment.
When a Rust publisher uses an explicit group sequence above 2^53 - 1, which moq_net::track::Producer::create_group accepts, this JSON number loses precision in double-based consumers because group is omitted from the safe-integer rule. A rounded value identifies a different group for subscription or fetch, so constrain group alongside pts or encode it losslessly. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
| ~~~ | ||
|
|
||
| The `group` field is the sequence number of the media track's group, as used by subscriptions and fetches. | ||
| The `pts` field is the group's start, its first frame's presentation timestamp, re-expressed in the timeline's `timescale` (rounding down when the media clock is finer). |
There was a problem hiding this comment.
When two recorded groups open within one tick of the chosen timeline timescale, rounding down gives both records the same pts. The span rule below then creates a zero-length boundary and cannot uniquely map a target time to a record. Require successive emitted records to have strictly increasing pts by choosing sufficient resolution or coalescing/skipping collisions. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
1 participant
Summary
The hang draft had drifted from the implementation. The catalog code (
rs/hang/src/catalog,js/hang/src/catalog) carries a per-renditiontimelinefield pointing at a companion timeline track, and publishes a compressedcatalog.json.zsibling, butdraft-lcurley-moq-hangdocumented neither. This reconciles the draft with the implementation as the source of truth:Timelinecatalog schema (track,timescaledefaulting to 1000,wallanchored to the 2020-01-01 moq epoch) and the timeline track format (a single never-rolled compressed group of{group, pts}JSON records, appended at group open, duration implicit, optional granularity throttling). Matchesrs/hang/src/catalog/timeline.rs,rs/hang/src/timeline.rs, and themoq-muxtimeline producer/consumer..ztracks, matchingmoq-flate.timelinealongsidebroadcast/container/jitter.catalog.json.zsibling a publisher SHOULD also serve.Notably not touched: the
textroot section, which was already cleanly reverted from both the draft and the implementation on main by #2571 (it lives on dev), so draft and code agree there.Docs-only; no wire change (the draft catches up to what already ships). No changelog appendix exists in this draft, so none updated.
Testing
nix develop --command just drafts check(kramdown-rfc parses all drafts)nix develop --command bun run --cwd doc check(VitePress renders the generated /draft/ pages; drafts.test.ts passes, only existing kramdown constructs used)(written by Fable 5)
🤖 Generated with Claude Code