Skip to content

feat(hang)!: name the container producer's timeline break discontinuity() - #4141

Merged
kixelated merged 3 commits into
devfrom
quest/m2/js-discontinuity
Sep 25, 2026
Merged

kixelated merged 3 commits into
devfrom
quest/m2/js-discontinuity

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

@moq/hang's Container.Legacy.Producer.cut() closes the group and writes the empty marker group that tells subscribers to re-anchor. Rust moq_mux::container::Producer splits that into cut(end) for a plain group close and discontinuity() for the break. The same name meant different things in the two languages.

Approach

Rename the JS method to discontinuity(end?), matching Rust. It behaves the same as before. No JS caller needs a plain group close yet, so none is made public. The publish video encoder and the tests now call the new name. Deletes quest/m2/js-discontinuity.md.

Impact

  • @moq/hang: Container.Legacy.Producer.cut(end?) is renamed to discontinuity(end?). This breaks the published API, so the PR targets dev.
  • Wire: none.

Alternatives

  • Keep a public cut() for the plain group close next to discontinuity(). Skipped until a caller needs it.

Follow-ups

None.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

@kixelated
kixelated marked this pull request as ready for review September 25, 2026 12:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T14:19:05.666784Z 740c41a New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06c85c4032

ℹ️ 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".

* is written until a frame follows the last one. Throws if `end` precedes the last video frame.
*/
cut(end?: Time.Micro) {
discontinuity(end?: Time.Micro) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reserve a skipped sequence for data discontinuities

When this newly named method is used with a data format, the branch at line 146 returns after closing the current group, so the next group receives the immediately adjacent sequence, as the changed test explicitly expects. In contrast, moq_mux::container::Producer::discontinuity() skips a sequence for data because an empty payload is valid data. Without either a marker or a sequence gap, JS consumers cannot distinguish the two epochs and treat the resumed data as continuous.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving the data-track path unchanged. discontinuity() still only closes the group, and the next sequence stays adjacent; the existing test locks that in. Rust skips a sequence because an empty payload is data, and the quest this PR finishes deferred that skip until a JS data-track caller exists. None does. Skipping a sequence here would change what gets published, which this rename does not.

(Written by Grok 4.7)

Comment thread js/publish/src/video/encoder.ts
kixelated and others added 2 commits September 25, 2026 07:14
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ty()

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Verdict: MERGE

Clean naming alignment: JS Container.Legacy.Producer.cut() wrote the empty marker group (timeline break), while Rust cut is a plain group close and discontinuity is the break. Renaming JS to discontinuity(end?) removes that cross-language trap.

Change is a pure rename plus call-site/test updates, quest doc retired, and it correctly targets dev as a published @moq/hang break. Keeping a public plain cut private until a caller needs it is the right restraint—no extra API surface for curiosity.

Worth doing; no better alternative that preserves clarity.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated
kixelated force-pushed the quest/m2/js-discontinuity branch from 06c85c4 to 740c41a Compare September 25, 2026 14:17

Copy link
Copy Markdown
Collaborator Author

Rebased onto current dev after #4144 merged main. The audio encoder comment now points at discontinuity() instead of cut(). Data tracks still close the group without skipping a sequence; that behavior is unchanged.

(Written by Grok 4.7)

@kixelated
kixelated enabled auto-merge (squash) September 25, 2026 14:17
@kixelated
kixelated merged commit d4c27de into dev Sep 25, 2026
4 checks passed
@kixelated
kixelated deleted the quest/m2/js-discontinuity branch September 25, 2026 15:10
@kixelated kixelated mentioned this pull request Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant