Skip to content

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Dec 7, 2025

Improvements to camera initialization and error handling in the desktop application. The main focus is on making the camera startup process more robust by adding a timeout for receiving the first frame, improving error handling during camera setup, and ensuring the application state is updated correctly when camera initialization fails. Additionally, the logic for releasing microphone and camera resources has been refined.

Summary by CodeRabbit

Bug Fixes

  • Enhanced camera initialization reliability with startup timeout protection to ensure proper first-frame detection
  • Improved camera state tracking to prevent unintended resource cleanup during active camera operations
  • Refined window closure behavior to properly respect and maintain camera session state before input cleanup

✏️ Tip: You can customize this high-level summary in your review settings.

@cursor
Copy link

cursor bot commented Dec 7, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 8.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

Walkthrough

The changes add first-frame startup timeout logic (5-second maximum) to camera initialization and refine state management in camera setup. Application state is pre-set when a camera ID is provided, with improved error handling and cleanup on initialization failure. Input removal logic on window close is expanded to consider active camera usage.

Changes

Cohort / File(s) Summary
Camera startup timeout
apps/desktop/src-tauri/src/camera.rs
Implements first-frame timeout mechanism using Instant and Duration. Tracks first frame reception; waits up to 5 seconds before timeout, then logs warning and closes. After first frame received, timeout is lifted enabling indefinite wait.
Camera initialization state management
apps/desktop/src-tauri/src/lib.rs
Pre-sets application state (selected_camera_id, camera_in_use flag) when camera ID provided. Refactors initialization error path to break loop and clean up state before returning error. Expands input removal logic on window close to check camera window status and camera_in_use flag, preventing premature input removal while camera is active.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Timeout logic in camera.rs: Verify Instant/Duration calculations and select branch modifications don't introduce logic errors or race conditions
  • State pre-setting in lib.rs: Review initialization state synchronization and potential race conditions between pre-setting and cleanup paths
  • Error handling flow: Confirm cleanup logic properly resets state (selected_camera_id, camera_in_use) on all failure paths

Possibly related PRs

Suggested reviewers

  • Brendonovich

Poem

📹 First frames now wait with patience true,
Five seconds—then we say "it's you!"
State synced clean, no tangled mess,
Timeouts tamed, the camera blessed! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: adding a camera preview timeout and improving camera cleanup logic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch camera-fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@richiemcilroy richiemcilroy merged commit ae8bd29 into main Dec 7, 2025
15 of 17 checks passed
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/camera.rs (1)

500-503: First-frame timeout logic is sound; consider a small readability tweak

The startup timeout behaves correctly: you cap initial wait at 5 seconds using saturating_sub, bail out cleanly on expiry, and lift the timeout after the first frame via received_first_frame. This integrates well with the existing select loop and avoids underflow.

If you want to make intent even clearer and avoid scheduling a practically unbounded sleep, you could conditionally include the timeout branch only while !received_first_frame instead of using Duration::MAX, but the current behavior is functionally fine.

Also applies to: 506-515, 526-529, 534-535

apps/desktop/src-tauri/src/lib.rs (1)

459-464: Camera init retry/state handling looks correct; minor redundancy only

The new flow in set_camera_input for Some(id) is solid:

  • You pre-set selected_camera_id, camera_in_use, and camera_cleanup_done before attempting initialization so other code can observe that the camera is being used.
  • The init_result loop retries up to three times with a clear error message and backoff, then rolls state back to None/false on failure and returns the error, avoiding a stuck “camera in use” flag on failed init.

The only minor nit is redundancy: the trailing block later in the function reassigns selected_camera_id and camera_in_use again on success. You could limit that block to handling disconnected_inputs/events (and perhaps just assert consistency) to reduce double writes, but it’s not functionally problematic.

Also applies to: 467-496, 498-502

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fabd186 and fdad15c.

📒 Files selected for processing (2)
  • apps/desktop/src-tauri/src/camera.rs (3 hunks)
  • apps/desktop/src-tauri/src/lib.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Never use dbg!() macro; use proper logging (tracing::debug!, etc.) instead
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them
Use duration.saturating_sub(other) instead of duration - other to avoid panics on underflow
Merge nested if statements: write 'if a && b { }' instead of 'if a { if b { } }'
Don't call .clone() on Copy types (integers, bools, etc.); copy them directly
Use function references directly: iter.map(foo) instead of iter.map(|x| foo(x))
Accept &[T] or &str instead of &Vec or &String in function parameters for flexibility
Use .is_empty() instead of .len() == 0 or .len() > 0 / .len() != 0
Don't assign () to a variable: write foo(); instead of let _ = foo(); or let x = foo(); when return is unit
Use .unwrap_or(val) instead of .unwrap_or_else(|| val) when the default is a simple/cheap value
Use 'for item in &collection' or 'for (i, item) in collection.iter().enumerate()' instead of 'for i in 0..collection.len()'
Use value.clamp(min, max) instead of manual if chains or .min(max).max(min) patterns
Always handle Result/Option or types marked #[must_use]; never ignore them

**/*.rs: Use rustfmt and workspace clippy lints for Rust code formatting and linting
Use snake_case for Rust module names and kebab-case for crate names
Never use dbg!() macro in Rust code; use proper logging instead (Clippy: dbg_macro = deny)
Always handle Result/Option or types marked #[must_use]; never ignore them (Rust compiler lint: unused_must_use = deny)
Never write let _ = async_fn() which silently drops futures; await or explicitly handle them (Clippy: let_underscore_future = deny)
Use saturating_sub instead of - for Duration to avoid panics (Clippy: unchecked_duration_subtraction = deny)
Merge nested if statements: use if a && b { } instead of if a { if b { } } (Clippy: collapsible_if = deny)
Don't call .clone() on Copy types; just copy them directly (Clippy: clone_on_copy = deny)
U...

Files:

  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/camera.rs
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (AGENTS.md)

Never add comments to code (//, /* */, ///, //!, #, etc.); code must be self-explanatory through naming, types, and structure

Files:

  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/camera.rs
🧠 Learnings (1)
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.

Applied to files:

  • apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (1)
apps/desktop/src-tauri/src/windows.rs (9)
  • app (233-233)
  • app (252-252)
  • app (409-409)
  • app (510-510)
  • app (846-846)
  • app (866-866)
  • app (1073-1073)
  • app (1085-1085)
  • id (840-872)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src-tauri/src/camera.rs (1)

20-22: New imports are aligned with the timeout and logging changes

Bringing Duration/Instant into scope and narrowing the tracing import to the used macros is appropriate and keeps the module tidy.

apps/desktop/src-tauri/src/lib.rs (1)

2773-2779: Main-window destroy cleanup now respects camera usage and preview window

The added camera_window_open check and the !app_state.camera_in_use guard ensure that mic/camera inputs and selections are only torn down when:

  • No recording is active or pending,
  • No camera window is open, and
  • The app state does not think the camera is in use.

This aligns with the new camera lifecycle behavior and avoids prematurely dropping camera input when the preview or recording is still active.

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.

2 participants