Repository navigation
feat(moq-audio): capture and encode while enabled, with an on-demand opt-in - #7159
Conversation
Quest-Claim: 4ccb541284362268
…opt-in An enabled microphone now captures and encodes whether or not anyone listens, so the recent past exists for a late subscriber to fetch. `Capture::on_demand` restores opening the device only while the track has a subscriber. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main change: always-on capture and encoding with an on-demand opt-in for moq-audio. Description check The description directly explains the always-on default, the Capture::on_demand option, documentation updates, quest completion, and tests. 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.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 4 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
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.
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. |
![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: 918d14757c
ℹ️ 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".
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 918d147
No actionable runtime bugs found. The small Wanted adapter is an appropriate way to change the capture default while preserving the existing stop/replace, retry and cancellation paths. Always-on capture still exits on track closure, and bandwidth reservation remains tied to actual demand. The late-subscriber test checks capture without creating subscriber demand first.
The existing test-clock comment already covers the real-time 50 ms wait; no duplicate inline comment added. This is test determinism/efficiency feedback, not evidence of a production capture failure.
Verification: static review of the changes and surrounding supervisor/control/track-closure paths. I did not run tests or exercise microphone hardware, permissions, AEC, or cross-platform capture.
…test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 52e7176
Reviewed the delta from 918d147. The previously referenced real-time-sleep concern is addressed: the on-demand test now synchronizes on Waiting and checks the input-open count without an arbitrary 50 ms delay. This is the simpler approach for a demand transition rather than elapsed-time behavior; the subscription/open assertions remain.
No new actionable bugs found. Production capture behavior is unchanged. Static review only; I did not execute the test.
|
Merge summary:
(Written by Claude Opus 5.5) |
Summary
Finishes quest/m1/eager-encode/audio.md, part of the always-on encoding epic (moq-dev/moq#7101).
moq-audio's capture publication now keeps an enabled microphone open and encodes whether or not anyone is subscribed, so a late subscriber can FETCH the recent past or reach it withmaxDelay. The newCapture::on_demandoption brings back the old behavior: the device opens only while the track has a subscriber and closes when the last one leaves.encode::Capturegetspub on_demand: bool(defaultfalse). The struct is#[non_exhaustive], so adding the field doesn't break anything.Wantedwrapper. Whenon_demandis off, the track counts as wanted until it ends.Status::Waitingis only reported in on-demand mode.lib.rs,capture.rs,aec.rs, theControl::new/publish_capture/Status::Waitingdoc comments, anddoc/lib/rs/moq-audio.md.cli.mdnow names the audio option it should select.Public API: the default behavior changes, and one option is added. Wire: no change.
Tests
captures_without_a_subscriber: with no subscriber, the publication goes Live and publishes a group (seen throughConsumer::largest, which doesn't register demand). A subscriber that arrives later reads that frame at its capture time.on_demand_waits_for_a_subscriber: with the opt-in, the device is never opened while nobody listens, and it opens when a subscriber arrives.a_restart_after_idle_keeps_the_gapandhls_records_keep_the_live_timestampsexercise idle restarts, so they now use the opt-in.just check origin/mainpasses (1544 tests).cargo test -p moq-audio --all-features --lib encode::capturepasses (36), and so doescargo clippy -p moq-audio --all-features --all-targets -D warnings.Choices made without asking
on_demand: boolonCapture, named to match the plannedmoq publish --on-demandflag. The parallel moq-video quest should use the same name on its capture config.Waitingflash: in always-on mode the supervisor skips announcingWaiting, so aControl::changedwatcher seesStarting → Liveand never a brief "waiting for a subscriber".moq publishmicrophone capture goes always-on as soon as this merges. The--on-demandflag comes later in quest/m1/eager-encode/cli.md. This follows the epic's "the default follows the libraries".Follow-ups
None new. The remaining epic children (video, js, cli, raw-audio) and the epic's own doc sweep are already planned.
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)