Repository navigation
fix(transcode): finish the open live group when demand ends - #5938
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A fetch is demand for the rung's live loop, but the group consumer it resolves to is not. When the fetch ended mid-group the loop aborted the group it was writing, under a reader that had already picked it up from the cache, and a later fetch re-transcoded the sequence under the same name. The loop now rides the open group out, as it already does on retirement, and leaves its source at the group boundary; demand returning before then resumes the session. Drops the held query the mirroring test needed to keep the live path from aborting between fetches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome of the unattended quest run:
(Written by Claude Opus 5.5) |
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. |
|
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
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe live transcode loop now starts on any demand, including fetches. If demand ends during an open group, the loop completes the group unless demand returns first. A new test checks that the source remains in use until the group ends and that the output contains both frames. The documentation and m1 requirement list reflect the changed behavior. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the primary behavior change: finishing the open live transcode group when demand ends. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The pull request description directly explains the transcode demand bug, the approach to finish an open live group, the affected tests, and the expected behavior changes. It matches the changeset. |
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![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: 8fdd4167cd
ℹ️ 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".
A select that picked a feed item over the `used` branch, or demand that returned during the encoder flush at `Item::End`, left `leaving` stale and ended the session anyway, so a returning viewer could miss the next group while a new session attached. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95ce660752
ℹ️ 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".
The post-item demand recheck only cleared `leaving`, so demand that ended during the encoder flush or lost the select to a queued Group let the live path open that group and ride it out with nobody asking. Snapshot demand both ways, and leave on a stray frame with no group open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64b557d421
ℹ️ 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".
Demand that left while the encoder flushed or cut at a group boundary still opened the next output group, which the live path then rode out with nobody asking. Recheck right before create_group. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Problem
Since #5820 a fetch is demand on a rung track, so it starts the rung's live loop. The group consumer a fetch resolves to is not demand, though. When the fetch ended mid-group, the loop saw the track unused and aborted the output group it was writing with
Cancel:Cancelpartway through;two_instances_mirror_source_groupsonly passed by holding a query across its fetches.Approach
Per the decision recorded in the quest (finish the group, then leave):
unusedwith a group open,livesetsleavingand rides the group out, the same way retirement already does. It finishes the group at the source's group end and then drops its listener and encoder, which releases the source.Groupis not opened, and a frame arriving with no group open does not hold the source. A final check after the boundary's flush and cut, right beforecreate_group, covers demand that leaves while the codec works.Tests:
ended_demand_finishes_the_open_live_group(paused time). A fetch for a group no one serves is the only demand, and the live path claims open source group 0. A reader takes group 0 from the cache, then the fetch is dropped. Without the fix the live path leaves its source mid-group and the reader getsCancel. With it, the reader reads both frames and the source is released only after the group ends.ended_demand_opens_no_new_live_group(paused time). Source group 1 is queued just before the last fetch ends, so the live path sees both at once. Without the fix it opened output group 1 and rode it out with nobody asking, about half the rounds; the test runs eight rounds and fails every run without the fix.two_instances_mirror_source_groupsno longer holds a query. Before the fix it failed withCancelwithout one.Impact
Alternatives
Both were rejected in the quest:
Follow-ups
moq-transcode. Whichever of the two lands second has to rebase overlive'sused/unusedselect.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)