feat(js/net): enforce the subscriber latency budget - #2926
Conversation
![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: cdde14f60e
ℹ️ 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".
0a4b614 to
a73c5d7
Compare
cdde14f to
3b4eee0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b4eee0f9d
ℹ️ 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".
3b4eee0 to
4bc781c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bc781c3f4
ℹ️ 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".
4bc781c to
79cf74a
Compare
The JS mirror of the moq-net work in the parent PR: js/net applies the same drift budget as it reads, and js/watch requests the playback latency ceiling from its first subscription. Drift is measured against both a presentation-time and a wall-clock live edge, from the reader's own position rather than the group's first frame, so a reader keeping pace is not convicted by how long ago its group opened. A group with frames in hand always drains; only a read that would wait is judged, which is also what keeps the policy off the read path. A drained group the budget gives up on ends rather than fails, since nothing was lost; the latency error is reserved for a caller still holding content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Matches the moq-mux rename in the parent PR: `subscribeAll` said nothing about what "all" meant and read like a subscribe-to-every-track operation. It is a replay window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirrors the moq-net fix in the parent PR. Drafts 01 and 02 have no latency field in SUBSCRIBE, so it decodes as `0`, which this branch would otherwise act on: the serving side discards every group but the live edge for a subscriber that never declined the backlog. Those two are the most preferred negotiated versions, so it is the ordinary wire. `carriesLatency` is the one place that answers whether a decoded zero means "real time" or "not stated". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: GPT-5.6 Sol <noreply@openai.com>
79cf74a to
ff72194
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7219415e
ℹ️ 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".
Co-Authored-By: GPT-5.6 Sol <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07f6a0365e
ℹ️ 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".
| const frames = this.#state.frames.peek(); | ||
| const frame = frames.shift(); | ||
| if (!frame) return undefined; | ||
| if (frames.length > 0) { |
There was a problem hiding this comment.
When the wire publisher has removed the last frame with readGroupFrame() and its guarded write is still blocked, frames.length is zero even though #pendingFrames is nonzero. A retention prune therefore skips the terminal verdict here; Track.Producer.#evict subsequently removes the group from the timeline before closing it, so the guard can no longer determine that the frame expired and may continue transmitting evicted content indefinitely. Treat pending frames as unread so eviction rejects the guarded operation. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| groups.splice(i, 1); | ||
| group.close(); |
There was a problem hiding this comment.
When readFrameSequence() finds an old buffered group outside the current maxAge but no newer frame is readable yet, this splice permanently discards the old group before the pending read returns anything. A subsequent Subscriber.update() that widens maxAge cannot recover that frame, and the pending wait does not observe the update signal either; the paired Rust implementation deliberately leaves the cursor stationary for a pending scan so widening the budget can admit the frame. Retain skipped candidates until a successful read advances the cursor and wake the scan on subscription changes. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L185-L192
Useful? React with 👍 / 👎.
1 participant
Summary
maxAgebudget while groups and frames are buffered or mirroredRoot cause
The JS model stored age limits on subscriptions but did not apply them consistently after a group or frame had been handed out. Buffered writes could also guard against the next frame's cursor instead of the frame being written, clean source closure could mask a later in-flight expiry, and the aggregate producer subscription could request history older than the track retained.
API and wire impact
No public API or wire-format changes. This ports the existing work to the current
maxAgeAPI and keeps the new helpers internal.Validation
just fixMOQ_STRICT=1 nix develop --command just checkMOQ_STRICT=1 nix develop --command just testbbb.hangdemo, including 1280x720 video, AAC audio, and live-latency advancementThe branch-scoped suite passed, including 572
@moq/nettests and the affected watch, hang, and JSON suites.(Written by GPT-5.6 Sol)