Skip to content

Hold camera and mic locks while recording is active #1162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 9, 2025
Merged

Hold camera and mic locks while recording is active #1162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2d442a
hold camera and mic locks while recording is active
Brendonovich Oct 9, 2025
1f6b8cb
hold locks inside sources
Brendonovich Oct 9, 2025
7007c9c
hold camera feed on instant mode
Brendonovich Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions apps/desktop/src-tauri/src/recording.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use cap_project::{
cursor::CursorEvents,
};
use cap_recording::PipelineDoneError;
use cap_recording::feeds::camera::CameraFeedLock;
use cap_recording::feeds::microphone::MicrophoneFeedLock;
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify that the imports are actually needed.

The imports for CameraFeedLock and MicrophoneFeedLock are not explicitly referenced by name in the code shown (types are inferred at lines 436 and 442). These might be unused imports unless they're needed elsewhere in the file.

Run the following script to check if these types are explicitly used:

🏁 Script executed:

#!/bin/bash
# Check if CameraFeedLock and MicrophoneFeedLock are explicitly referenced in recording.rs

echo "Checking for explicit references to CameraFeedLock..."
rg -n 'CameraFeedLock' apps/desktop/src-tauri/src/recording.rs | grep -v '^11:' || echo "No references found outside import"

echo ""
echo "Checking for explicit references to MicrophoneFeedLock..."
rg -n 'MicrophoneFeedLock' apps/desktop/src-tauri/src/recording.rs | grep -v '^12:' || echo "No references found outside import"

Length of output: 182

Remove unused imports of CameraFeedLock and MicrophoneFeedLock
Neither type is referenced anywhere in apps/desktop/src-tauri/src/recording.rs; these imports can be deleted.

🤖 Prompt for AI Agents 
In apps/desktop/src-tauri/src/recording.rs around lines 11 to 12, the file
imports CameraFeedLock and MicrophoneFeedLock from cap_recording but never uses
them; remove the two unused use lines (use
cap_recording::feeds::camera::CameraFeedLock; and use
cap_recording::feeds::microphone::MicrophoneFeedLock;) to clean up unused
imports and avoid compiler warnings.

use cap_recording::{
RecordingError, RecordingMode,
feeds::{camera, microphone},
Expand Down Expand Up @@ -53,20 +55,24 @@ use crate::{
windows::{CapWindowId, ShowCapWindow},
};

pub struct InProgressRecordingCommon {
pub target_name: String,
pub inputs: StartRecordingInputs,
pub recording_dir: PathBuf,
}

pub enum InProgressRecording {
Instant {
target_name: String,
handle: instant_recording::ActorHandle,
progressive_upload: InstantMultipartUpload,
video_upload_info: VideoUploadInfo,
inputs: StartRecordingInputs,
recording_dir: PathBuf,
common: InProgressRecordingCommon,
// camera isn't used as part of recording pipeline so we hold lock here
camera_feed: Option<Arc<CameraFeedLock>>,
Comment on lines +69 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove the comment to comply with coding guidelines.

The inline comment violates the guideline: "Do not add inline, block, or docstring comments in any language; code must be self-explanatory."

Apply this diff:

     common: InProgressRecordingCommon,
-    // camera isn't used as part of recording pipeline so we hold lock here
     camera_feed: Option<Arc<CameraFeedLock>>,

As per coding guidelines

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
common: InProgressRecordingCommon,
// camera isn't used as part of recording pipeline so we hold lock here
camera_feed: Option<Arc<CameraFeedLock>>,
common: InProgressRecordingCommon,
camera_feed: Option<Arc<CameraFeedLock>>,
🤖 Prompt for AI Agents 
In apps/desktop/src-tauri/src/recording.rs around lines 69 to 71, remove the
inline comment "// camera isn't used as part of recording pipeline so we hold
lock here" so the code complies with the guideline forbidding
inline/block/docstring comments; leave the field definition and surrounding
formatting unchanged.

},
Studio {
target_name: String,
handle: studio_recording::ActorHandle,
inputs: StartRecordingInputs,
recording_dir: PathBuf,
common: InProgressRecordingCommon,
},
}

Expand All @@ -80,8 +86,8 @@ impl InProgressRecording {

pub fn inputs(&self) -> &StartRecordingInputs {
match self {
Self::Instant { inputs, .. } => inputs,
Self::Studio { inputs, .. } => inputs,
Self::Instant { common, .. } => &common.inputs,
Self::Studio { common, .. } => &common.inputs,
}
}

Expand All @@ -101,8 +107,8 @@ impl InProgressRecording {

pub fn recording_dir(&self) -> &PathBuf {
match self {
Self::Instant { recording_dir, .. } => recording_dir,
Self::Studio { recording_dir, .. } => recording_dir,
Self::Instant { common, .. } => &common.recording_dir,
Self::Studio { common, .. } => &common.recording_dir,
}
}

Expand All @@ -112,21 +118,17 @@ impl InProgressRecording {
handle,
progressive_upload,
video_upload_info,
target_name,
common,
..
} => CompletedRecording::Instant {
recording: handle.stop().await?,
progressive_upload,
video_upload_info,
target_name,
target_name: common.target_name,
},
Self::Studio {
handle,
target_name,
..
} => CompletedRecording::Studio {
Self::Studio { handle, common, .. } => CompletedRecording::Studio {
recording: handle.stop().await?,
target_name,
target_name: common.target_name,
},
})
}
Expand Down Expand Up @@ -449,6 +451,12 @@ pub async fn start_recording(
.map_err(|e| format!("GetShareableContent: {e}"))?
.ok_or_else(|| format!("GetShareableContent/NotAvailable"))?;

let common = InProgressRecordingCommon {
target_name,
inputs: inputs.clone(),
recording_dir: recording_dir.clone(),
};

let actor = match inputs.mode {
RecordingMode::Studio => {
let mut builder = studio_recording::Actor::builder(
Expand Down Expand Up @@ -481,12 +489,7 @@ pub async fn start_recording(
e.to_string()
})?;

InProgressRecording::Studio {
handle,
target_name,
inputs,
recording_dir: recording_dir.clone(),
}
InProgressRecording::Studio { handle, common }
}
RecordingMode::Instant => {
let Some(video_upload_info) = video_upload_info.clone() else {
Expand Down Expand Up @@ -526,9 +529,8 @@ pub async fn start_recording(
handle,
progressive_upload,
video_upload_info,
target_name,
inputs,
recording_dir: recording_dir.clone(),
common,
camera_feed,
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/recording/src/instant_recording.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
RecordingBaseInputs,
capture_pipeline::{MakeCapturePipeline, ScreenCaptureMethod, Stop, create_screen_capture},
feeds::microphone::MicrophoneFeedLock,
feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock},

Check warning on line 4 in crates/recording/src/instant_recording.rs

View workflow job for this annotation

GitHub Actions / Clippy

unused import: `camera::CameraFeedLock` 

warning: unused import: `camera::CameraFeedLock`
 --> crates/recording/src/instant_recording.rs:4:13
  |
4 |     feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock},
  |             ^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unused import: CameraFeedLock is not referenced in this file.

CameraFeedLock is imported but never used. Instant recording doesn't currently support camera feeds (camera_feed is hardcoded to None at line 255 in RecordingBaseInputs), so this import is unnecessary.

Apply this diff to remove the unused import:

-    feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock},
+    feeds::microphone::MicrophoneFeedLock,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock},
feeds::microphone::MicrophoneFeedLock,
🤖 Prompt for AI Agents 
In crates/recording/src/instant_recording.rs around line 4, the import
feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock} includes
CameraFeedLock which is unused because instant recording hardcodes camera_feed
to None; remove CameraFeedLock from the use/import list so only
MicrophoneFeedLock is imported to eliminate the unused-import warning.

output_pipeline::{self, OutputPipeline},
sources::screen_capture::{ScreenCaptureConfig, ScreenCaptureTarget},
};
Expand Down
6 changes: 3 additions & 3 deletions crates/recording/src/sources/camera.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ impl VideoSource for Camera {
type Frame = FFmpegVideoFrame;

async fn setup(
config: Self::Config,
feed_lock: Self::Config,
mut video_tx: mpsc::Sender<Self::Frame>,
_: &mut SetupCtx,
) -> anyhow::Result<Self>
Expand All @@ -24,7 +24,7 @@ impl VideoSource for Camera {
{
let (tx, rx) = flume::bounded(8);

config
feed_lock
.ask(camera::AddSender(tx))
.await
.map_err(|e| anyhow!("Failed to add camera sender: {e}"))?;
Expand All @@ -35,7 +35,7 @@ impl VideoSource for Camera {
}
});

Ok(Self(config))
Ok(Self(feed_lock))
}

fn video_info(&self) -> VideoInfo {
Expand Down
10 changes: 5 additions & 5 deletions crates/recording/src/sources/microphone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@ use cap_media_info::AudioInfo;
use futures::{SinkExt, channel::mpsc};
use std::sync::Arc;

pub struct Microphone(AudioInfo);
pub struct Microphone(AudioInfo, Arc<MicrophoneFeedLock>);

impl AudioSource for Microphone {
type Config = Arc<MicrophoneFeedLock>;

fn setup(
config: Self::Config,
feed_lock: Self::Config,
mut audio_tx: mpsc::Sender<AudioFrame>,
_: &mut crate::SetupCtx,
) -> impl Future<Output = anyhow::Result<Self>> + 'static
where
Self: Sized,
{
async move {
let audio_info = config.audio_info();
let audio_info = feed_lock.audio_info();
let (tx, rx) = flume::bounded(8);

config
feed_lock
.ask(microphone::AddSender(tx))
.await
.map_err(|e| anyhow!("Failed to add camera sender: {e}"))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix error message to reference microphone instead of camera.

The error message mentions "camera sender" but this code is for the microphone source.

Apply this diff to fix the message:

-            .map_err(|e| anyhow!("Failed to add camera sender: {e}"))?;
+            .map_err(|e| anyhow!("Failed to add microphone sender: {e}"))?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.map_err(|e| anyhow!("Failed to add camera sender: {e}"))?;
.map_err(|e| anyhow!("Failed to add microphone sender: {e}"))?;
🤖 Prompt for AI Agents 
In crates/recording/src/sources/microphone.rs around line 30, the error message
incorrectly says "Failed to add camera sender"; update the string to reference
the microphone (e.g., "Failed to add microphone sender: {e}") so the error
context matches this source, keeping the same map_err usage and error
interpolation.

Expand All @@ -40,7 +40,7 @@ impl AudioSource for Microphone {
}
});

Ok(Self(audio_info))
Ok(Self(audio_info, feed_lock))
}
}

Expand Down
Loading