recording: move display+crop conversion out of capture sources (#1257)

Brendonovich · coderabbitai[bot] · web-flow · commit 5355d7206365 · 2025-10-21T15:14:58.000+08:00
* recording: move display+crop conversion out of capture sources

* more anyhow errors

* more error handling fixes

* fix

* Update crates/recording/src/capture_pipeline.rs

Co-authored-by: coderabbitai[bot] &lt;136622811+coderabbitai[bot]@users.noreply.github.com&gt;

* delay cursor crop calculation

---------

Co-authored-by: coderabbitai[bot] &lt;136622811+coderabbitai[bot]@users.noreply.github.com&gt;
diff --git a/crates/recording/src/capture_pipeline.rs b/crates/recording/src/capture_pipeline.rs
@@ -2,13 +2,12 @@ use crate::{
     feeds::microphone::MicrophoneFeedLock,
     output_pipeline::*,
     sources,
-    sources::screen_capture::{
-        self, ScreenCaptureConfig, ScreenCaptureFormat, ScreenCaptureTarget,
-    },
+    sources::screen_capture::{self, CropBounds, ScreenCaptureFormat, ScreenCaptureTarget},
 };
+use anyhow::anyhow;
 use cap_timestamp::Timestamps;
-use scap_targets::WindowId;
-use std::{path::PathBuf, sync::Arc, time::SystemTime};
+use scap_targets::bounds::LogicalBounds;
+use std::{path::PathBuf, sync::Arc};
 
 pub trait MakeCapturePipeline: ScreenCaptureFormat + std::fmt::Debug + 'static {
     async fn make_studio_mode_pipeline(
@@ -127,30 +126,96 @@ pub type ScreenCaptureMethod = screen_capture::CMSampleBufferCapture;
 #[cfg(windows)]
 pub type ScreenCaptureMethod = screen_capture::Direct3DCapture;
 
-pub async fn create_screen_capture(
-    capture_target: &ScreenCaptureTarget,
-    force_show_cursor: bool,
-    max_fps: u32,
-    start_time: SystemTime,
-    system_audio: bool,
-    #[cfg(windows)] d3d_device: ::windows::Win32::Graphics::Direct3D11::ID3D11Device,
-    #[cfg(target_os = "macos")] shareable_content: cidre::arc::R<cidre::sc::ShareableContent>,
-    #[cfg(target_os = "macos")] excluded_windows: Vec<WindowId>,
-) -> anyhow::Result<ScreenCaptureConfig<ScreenCaptureMethod>> {
-    Ok(ScreenCaptureConfig::<ScreenCaptureMethod>::init(
-        capture_target,
-        force_show_cursor,
-        max_fps,
-        start_time,
-        system_audio,
-        #[cfg(windows)]
-        d3d_device,
-        #[cfg(target_os = "macos")]
-        shareable_content,
-        #[cfg(target_os = "macos")]
-        excluded_windows,
-    )
-    .await?)
+pub fn target_to_display_and_crop(
+    target: &ScreenCaptureTarget,
+) -> anyhow::Result<(scap_targets::Display, Option<CropBounds>)> {
+    use scap_targets::{bounds::*, *};
+
+    let display = target
+        .display()
+        .ok_or_else(|| anyhow!("Display not found"))?;
+
+    let crop_bounds = match target {
+        ScreenCaptureTarget::Display { .. } => None,
+        ScreenCaptureTarget::Window { id } => {
+            let window = Window::from_id(id).ok_or_else(|| anyhow!("Window not found"))?;
+
+            #[cfg(target_os = "macos")]
+            {
+                let raw_display_bounds = display
+                    .raw_handle()
+                    .logical_bounds()
+                    .ok_or_else(|| anyhow!("No display bounds"))?;
+                let raw_window_bounds = window
+                    .raw_handle()
+                    .logical_bounds()
+                    .ok_or_else(|| anyhow!("No window bounds"))?;
+
+                Some(LogicalBounds::new(
+                    LogicalPosition::new(
+                        raw_window_bounds.position().x() - raw_display_bounds.position().x(),
+                        raw_window_bounds.position().y() - raw_display_bounds.position().y(),
+                    ),
+                    raw_window_bounds.size(),
+                ))
+            }
+
+            #[cfg(windows)]
+            {
+                let raw_display_position = display
+                    .raw_handle()
+                    .physical_position()
+                    .ok_or_else(|| anyhow!("No display bounds"))?;
+                let raw_window_bounds = window
+                    .raw_handle()
+                    .physical_bounds()
+                    .ok_or_else(|| anyhow!("No window bounds"))?;
+
+                Some(PhysicalBounds::new(
+                    PhysicalPosition::new(
+                        raw_window_bounds.position().x() - raw_display_position.x(),
+                        raw_window_bounds.position().y() - raw_display_position.y(),
+                    ),
+                    raw_window_bounds.size(),
+                ))
+            }
+        }
+        ScreenCaptureTarget::Area {
+            bounds: relative_bounds,
+            ..
+        } => {
+            #[cfg(target_os = "macos")]
+            {
+                Some(*relative_bounds)
+            }
+
+            #[cfg(windows)]
+            {
+                let raw_display_size = display
+                    .physical_size()
+                    .ok_or_else(|| anyhow!("No display bounds"))?;
+                let logical_display_size = display
+                    .logical_size()
+                    .ok_or_else(|| anyhow!("No display logical size"))?;
+                Some(PhysicalBounds::new(
+                    PhysicalPosition::new(
+                        (relative_bounds.position().x() / logical_display_size.width())
+                            * raw_display_size.width(),
+                        (relative_bounds.position().y() / logical_display_size.height())
+                            * raw_display_size.height(),
+                    ),
+                    PhysicalSize::new(
+                        (relative_bounds.size().width() / logical_display_size.width())
+                            * raw_display_size.width(),
+                        (relative_bounds.size().height() / logical_display_size.height())
+                            * raw_display_size.height(),
+                    ),
+                ))
+            }
+        }
+    };
+
+    Ok((display, crop_bounds))
 }
 
 #[cfg(windows)]
diff --git a/crates/recording/src/instant_recording.rs b/crates/recording/src/instant_recording.rs
@@ -1,10 +1,13 @@
 use crate::{
     RecordingBaseInputs,
-    capture_pipeline::{MakeCapturePipeline, ScreenCaptureMethod, Stop, create_screen_capture},
+    capture_pipeline::{
+        MakeCapturePipeline, ScreenCaptureMethod, Stop, target_to_display_and_crop,
+    },
     feeds::microphone::MicrophoneFeedLock,
     output_pipeline::{self, OutputPipeline},
     sources::screen_capture::{ScreenCaptureConfig, ScreenCaptureTarget},
 };
+use anyhow::Context as _;
 use cap_media_info::{AudioInfo, VideoInfo};
 use cap_project::InstantRecordingMeta;
 use cap_utils::ensure_dir;
@@ -293,8 +296,12 @@ pub async fn spawn_instant_recording_actor(
     #[cfg(windows)]
     let d3d_device = crate::capture_pipeline::create_d3d_device()?;
 
-    let screen_source = create_screen_capture(
-        &inputs.capture_target,
+    let (display, crop) =
+        target_to_display_and_crop(&inputs.capture_target).context("target_display_crop")?;
+
+    let screen_source = ScreenCaptureConfig::<ScreenCaptureMethod>::init(
+        display,
+        crop,
         true,
         30,
         start_time,
@@ -306,7 +313,8 @@ pub async fn spawn_instant_recording_actor(
         #[cfg(target_os = "macos")]
         inputs.excluded_windows,
     )
-    .await?;
+    .await
+    .context("screen capture init")?;
 
     debug!("screen capture: {screen_source:#?}");
 
diff --git a/crates/recording/src/sources/screen_capture/mod.rs b/crates/recording/src/sources/screen_capture/mod.rs
@@ -253,14 +253,17 @@ impl<TCaptureFormat: ScreenCaptureFormat> Clone for ScreenCaptureConfig<TCapture
 #[derive(Clone, Debug)]
 pub struct Config {
     display: DisplayId,
-    #[cfg(windows)]
-    crop_bounds: Option<PhysicalBounds>,
-    #[cfg(target_os = "macos")]
-    crop_bounds: Option<LogicalBounds>,
+    crop_bounds: Option<CropBounds>,
     fps: u32,
     show_cursor: bool,
 }
 
+#[cfg(target_os = "macos")]
+pub type CropBounds = LogicalBounds;
+
+#[cfg(windows)]
+pub type CropBounds = PhysicalBounds;
+
 impl Config {
     pub fn fps(&self) -> u32 {
         self.fps
@@ -280,7 +283,8 @@ pub enum ScreenCaptureInitError {
 impl<TCaptureFormat: ScreenCaptureFormat> ScreenCaptureConfig<TCaptureFormat> {
     #[allow(clippy::too_many_arguments)]
     pub async fn init(
-        target: &ScreenCaptureTarget,
+        display: scap_targets::Display,
+        crop_bounds: Option<CropBounds>,
         show_cursor: bool,
         max_fps: u32,
         start_time: SystemTime,
@@ -291,92 +295,10 @@ impl<TCaptureFormat: ScreenCaptureFormat> ScreenCaptureConfig<TCaptureFormat> {
     ) -> Result<Self, ScreenCaptureInitError> {
         cap_fail::fail!("ScreenCaptureSource::init");
 
-        let display = target.display().ok_or(ScreenCaptureInitError::NoDisplay)?;
-
         let fps = max_fps.min(display.refresh_rate() as u32);
 
-        let crop_bounds = match target {
-            ScreenCaptureTarget::Display { .. } => None,
-            ScreenCaptureTarget::Window { id } => {
-                let window = Window::from_id(id).ok_or(ScreenCaptureInitError::NoWindow)?;
-
-                #[cfg(target_os = "macos")]
-                {
-                    let raw_display_bounds = display
-                        .raw_handle()
-                        .logical_bounds()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-                    let raw_window_bounds = window
-                        .raw_handle()
-                        .logical_bounds()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-
-                    Some(LogicalBounds::new(
-                        LogicalPosition::new(
-                            raw_window_bounds.position().x() - raw_display_bounds.position().x(),
-                            raw_window_bounds.position().y() - raw_display_bounds.position().y(),
-                        ),
-                        raw_window_bounds.size(),
-                    ))
-                }
-
-                #[cfg(windows)]
-                {
-                    let raw_display_position = display
-                        .raw_handle()
-                        .physical_position()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-                    let raw_window_bounds = window
-                        .raw_handle()
-                        .physical_bounds()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-
-                    Some(PhysicalBounds::new(
-                        PhysicalPosition::new(
-                            raw_window_bounds.position().x() - raw_display_position.x(),
-                            raw_window_bounds.position().y() - raw_display_position.y(),
-                        ),
-                        raw_window_bounds.size(),
-                    ))
-                }
-            }
-            ScreenCaptureTarget::Area {
-                bounds: relative_bounds,
-                ..
-            } => {
-                #[cfg(target_os = "macos")]
-                {
-                    Some(*relative_bounds)
-                }
-
-                #[cfg(windows)]
-                {
-                    let raw_display_size = display
-                        .physical_size()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-                    let logical_display_size = display
-                        .logical_size()
-                        .ok_or(ScreenCaptureInitError::NoBounds)?;
-
-                    Some(PhysicalBounds::new(
-                        PhysicalPosition::new(
-                            (relative_bounds.position().x() / logical_display_size.width())
-                                * raw_display_size.width(),
-                            (relative_bounds.position().y() / logical_display_size.height())
-                                * raw_display_size.height(),
-                        ),
-                        PhysicalSize::new(
-                            (relative_bounds.size().width() / logical_display_size.width())
-                                * raw_display_size.width(),
-                            (relative_bounds.size().height() / logical_display_size.height())
-                                * raw_display_size.height(),
-                        ),
-                    ))
-                }
-            }
-        };
-
         let output_size = crop_bounds
+            .clone()
             .and_then(|b| {
                 #[cfg(target_os = "macos")]
                 {
diff --git a/crates/recording/src/studio_recording.rs b/crates/recording/src/studio_recording.rs
@@ -1,10 +1,13 @@
 use crate::{
     ActorError, MediaError, RecordingBaseInputs, RecordingError,
-    capture_pipeline::{MakeCapturePipeline, ScreenCaptureMethod, Stop, create_screen_capture},
+    capture_pipeline::{
+        MakeCapturePipeline, ScreenCaptureMethod, Stop, target_to_display_and_crop,
+    },
     cursor::{CursorActor, Cursors, spawn_cursor_recorder},
     feeds::{camera::CameraFeedLock, microphone::MicrophoneFeedLock},
     ffmpeg::{Mp4Muxer, OggMuxer},
     output_pipeline::{DoneFut, FinishedOutputPipeline, OutputPipeline, PipelineDoneError},
+    screen_capture::ScreenCaptureConfig,
     sources::{self, screen_capture},
 };
 use anyhow::{Context as _, anyhow};
@@ -680,20 +683,15 @@ async fn create_segment_pipeline(
     custom_cursor_capture: bool,
     start_time: Timestamps,
 ) -> anyhow::Result<Pipeline> {
-    let display = base_inputs
-        .capture_target
-        .display()
-        .ok_or(CreateSegmentPipelineError::NoDisplay)?;
-    let crop_bounds = base_inputs
-        .capture_target
-        .cursor_crop()
-        .ok_or(CreateSegmentPipelineError::NoBounds)?;
-
     #[cfg(windows)]
     let d3d_device = crate::capture_pipeline::create_d3d_device().unwrap();
 
-    let screen_config = create_screen_capture(
-        &base_inputs.capture_target,
+    let (display, crop) =
+        target_to_display_and_crop(&base_inputs.capture_target).context("target_display_crop")?;
+
+    let screen_config = ScreenCaptureConfig::<ScreenCaptureMethod>::init(
+        display,
+        crop,
         !custom_cursor_capture,
         120,
         start_time.system_time(),
@@ -706,7 +704,7 @@ async fn create_segment_pipeline(
         base_inputs.excluded_windows,
     )
     .await
-    .unwrap();
+    .context("screen capture init")?;
 
     let (capture_source, system_audio) = screen_config.to_sources().await?;
 
@@ -758,21 +756,28 @@ async fn create_segment_pipeline(
     .transpose()
     .context("microphone pipeline setup")?;
 
-    let cursor = custom_cursor_capture.then(move || {
-        let cursor = spawn_cursor_recorder(
-            crop_bounds,
-            display,
-            cursors_dir.to_path_buf(),
-            prev_cursors,
-            next_cursors_id,
-            start_time,
-        );
-
-        CursorPipeline {
-            output_path: dir.join("cursor.json"),
-            actor: cursor,
-        }
-    });
+    let cursor = custom_cursor_capture
+        .then(move || {
+            let cursor_crop_bounds = base_inputs
+                .capture_target
+                .cursor_crop()
+                .ok_or(CreateSegmentPipelineError::NoBounds)?;
+
+            let cursor = spawn_cursor_recorder(
+                cursor_crop_bounds,
+                display,
+                cursors_dir.to_path_buf(),
+                prev_cursors,
+                next_cursors_id,
+                start_time,
+            );
+
+            Ok::<_, CreateSegmentPipelineError>(CursorPipeline {
+                output_path: dir.join("cursor.json"),
+                actor: cursor,
+            })
+        })
+        .transpose()?;
 
     info!("pipeline playing");
 
