chore(quest): plan the GStreamer, libmoq, and OBS quests - #3349
Conversation
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 reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
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 |
![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: 240a7d4384
ℹ️ 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".
| - Wire the new binary into the `just obs test` recipe under the same TSan | ||
| build, and mention it in `doc/bin/obs.md` next to the output test. |
There was a problem hiding this comment.
Completing this step still leaves the new test solely in just obs test, which cpp/obs/justfile explicitly marks as manual, while .github/workflows/obs.yml invokes only just obs ci. A callback-ordering regression such as #2856 would therefore still leave PR CI green, contrary to the goal's promised red build. Run the test from an automated recipe as well, possibly without TSan, or narrow the goal to manual coverage. (Written by GPT-5.6 Sol)
AGENTS.md reference: quest/AGENTS.md:L39-L45
Useful? React with 👍 / 👎.
240a7d4 to
4999949
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49999495ca
ℹ️ 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".
| - moq-net: dropping or refusing an unaccepted `track::Request`, and closing a | ||
| producer that never published info, resolves every pending subscribe with | ||
| `NotFound`; verify and pin that. |
There was a problem hiding this comment.
If this step is implemented by making a dropped track::Request resolve as NotFound, a dynamic handler that disappears because of a publisher or transport failure will be misreported as an absent track. rs/moq-net/src/model/track.rs deliberately documents dropped requests as Error::Dropped and maps a closed state without an abort reason to that error, while callers can already use request.reject(Error::NotFound) when catalog finality proves the track is absent. Preserve the generic drop semantics and explicitly reject only the requests the hang publisher knows are unservable, which also keeps catalog meaning out of the generic moq-net layer.
AGENTS.md reference: AGENTS.md:L64-L66
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4999949 to
dae8c1b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dae8c1bf46
ℹ️ 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".
| - Remove `_exit` from the smoke client's success path. The failure paths keep | ||
| it for the separate `user_data` lifetime reason from #2675. |
There was a problem hiding this comment.
When the success path is changed to return, waiting only for moq_session_connect's terminal status does not make the stack-allocated ctx_t safe to destroy. The same pointer is independently retained by moq_origin_consume_announced, moq_consume_catalog, and moq_consume_video, and each registration can use it until its own terminal callback; closing the session terminates only its status registration. A late catalog or frame callback can therefore access the stack or destroyed pthread objects after main returns, making the regression test fail for a second lifetime bug instead of isolating atexit teardown. Extend the plan to close and drain every callback registration before removing _exit.
AGENTS.md reference: AGENTS.md:L148-L152
Useful? React with 👍 / 👎.
| - Pad lifecycles reset into the new generation on join, reusing | ||
| `lifecycle.reset()` from `start_session`. |
There was a problem hiding this comment.
lifecycle.reset() replaces the pad's media state with Pad::new(), clearing both the cached caps and track producer. With the proposed lazy join on the next buffer, a normal post-FLUSH_STOP sequence may resend a segment but not CAPS because CAPS remains sticky, so Pad::push_buffer sees no track and silently drops that buffer. Preserve or replay the pad's sticky caps and reconstruct its producer before accepting the first buffer into the new generation, or the second broadcast can contain no media despite the stated test passing through the join path.
Useful? React with 👍 / 👎.
1 participant
Replans the GStreamer, libmoq, and OBS quests in m0.
Completionis monotonic per session. Design settled with its author:Eosstays terminal for its generation, the firstFLUSH_STOPafter EOS opens one new generation globally and other pads join it on their next buffer. Resized to [L].NotFoundwhen there is no producer and noDynamic; the parking case is a reserved or info-less track behind a live handler. Direction: closing the catalog is a promise the announced set is final, so publishers refuse those tracks and the subscription resolves; moqsrc ends that one pump.LazyLock, openh264 always linked, no sanitizer recipe; done means the smoke client's success path returns frommain.just obs test.Part of a per-cluster replan of
quest/m0: every issue-imported quest had the boilerplate "implement what the issue says" goal with the issue body pasted as the plan. Each quest now states its observable outcome and the decisions taken, and drops the pasted body.🤖 Generated with Claude Code
(written by Claude Fable 5.1)