Repository navigation
fix(desktop-gpui): hold the start cue until capture is ready (Instant mode lost openings) - #3036
heidar-droid wants to merge 1 commit into
Conversation
In Instant mode, start_internal awaits prepare_instant_upload (a network round trip to create the shareable link) before the capture pipeline is built, but the start cue plays on the countdown regardless. On a slow or busy uplink the cue played 2-10 s (once 102 s) before capture began, so the user started talking into nothing and the opening was lost. The cue now waits on an engine_ready watch that flips when the current start resolves Ok. Failed starts still abort the cue via abort_start_cue, and Studio mode (no network on start) is unaffected. Fixes CapSoftware#3035
|
@heidar-droid is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
| // The cue tells the user recording has started, so hold it | ||
| // until capture can actually begin. A failed start aborts | ||
| // this future through `abort_start_cue`. | ||
| let _ = engine_ready.wait_for(|ready| *ready).await; |
There was a problem hiding this comment.
This wait can outlast the gate’s 20-second failsafe. The gate is created before prepare_instant_upload, so a slow request such as the reported 102-second case leaves it expired when capture starts. The first media check then opens the gate before the delayed cue finishes. The cue’s later arm() cannot replace that start time, so the saved recording can include the cue and unwanted lead-in.
Keep the gate closed through the readiness wait, and start its failsafe clock when capture is ready.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/session.rs
Line: 769
Comment:
**Capture can precede the cue**
This wait can outlast the gate’s 20-second failsafe. The gate is created before `prepare_instant_upload`, so a slow request such as the reported 102-second case leaves it expired when capture starts. The first media check then opens the gate before the delayed cue finishes. The cue’s later `arm()` cannot replace that start time, so the saved recording can include the cue and unwanted lead-in.
Keep the gate closed through the readiness wait, and start its failsafe clock when capture is ready.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // The cue tells the user recording has started, so hold it | ||
| // until capture can actually begin. A failed start aborts | ||
| // this future through `abort_start_cue`. | ||
| let _ = engine_ready.wait_for(|ready| *ready).await; |
There was a problem hiding this comment.
The new wait has no test that exercises it. Existing countdown tests check countdown_advance, and the stop test installs an abort handle without starting play_start_cue. The timing behavior this fix depends on could regress without those tests catching it.
Add tests that hold readiness false, release it, and cancel or fail while waiting. Check that neither the cue nor the gate fires early and that an old attempt cannot release a new cue.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/session.rs
Line: 769
Comment:
**Readiness wait has no tests**
The new wait has no test that exercises it. Existing countdown tests check `countdown_advance`, and the stop test installs an abort handle without starting `play_start_cue`. The timing behavior this fix depends on could regress without those tests catching it.
Add tests that hold readiness false, release it, and cancel or fail while waiting. Check that neither the cue nor the gate fires early and that an old attempt cannot release a new cue.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #3035.
Problem
In Instant mode,
start_internal(apps/desktop-gpui/src/recording.rs) awaitsprepare_instant_uploadbefore the capture pipeline is built, butRecordingSession::startplays the start cue on the countdown regardless. When the create-video request is slow (a busy uplink while earlier takes are still uploading), the cue plays seconds before capture begins and the user's opening is lost.From
cap-gpui.logon 0.6.1 (cue →starting recording): 0.01–0.17 s with nothing uploading, 2.3–9.9 s once uploads were queued, once 102 s.Fix
RecordingSessiongets anengine_ready: watch::Sender<bool>:falseinstart()truewhen the current generation's start resolvesOk(before it's stored asprimedor activated)play_start_cueawaitswait_for(|ready| *ready)before playing the cue and arming the gateSo the cue, and the gate it arms, only fire once capture can actually begin. The countdown UI holds at its pre-start state meanwhile.
abort_start_cue, which aborts the waiting future.send_replace(true), so an old start can't release a new cue.No new dependencies (
tokio::sync::watchis already used in the crate).🤖 Generated with Claude Code
This appears safe to merge, with non-blocking concerns about long-start lead-in and missing tests.
Findings
Fix with agent prompt
Summary
This change delays the start cue until capture startup succeeds.
Reviews (1) · Last reviewed commit: "fix(desktop-gpui): hold the start cue un..." · Reviewed by Greptile