Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ jobs:
uses: dtolnay/rust-toolchain@stable
with:
targets: ${{ matrix.settings.target }}
components: clippy

- name: Rust cache
uses: swatinem/rust-cache@v2
Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions apps/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,8 @@ tracing.workspace = true
tracing-subscriber = "0.3.19"
flume.workspace = true

[target.'cfg(target_os = "macos")'.dependencies]
cidre = { workspace = true }

[lints]
workspace = true
5 changes: 4 additions & 1 deletion apps/cli/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ impl RecordStart {
let actor = studio_recording::Actor::builder(path, target_info)
.with_system_audio(self.system_audio)
.with_custom_cursor(false)
.build()
.build(
#[cfg(target_os = "macos")]
cidre::sc::ShareableContent::current().await.unwrap(),
)
.await
.map_err(|e| e.to_string())?;
Comment on lines +64 to 69
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 | 🟠 Major

Propagate ShareableContent errors instead of panicking.

The unwrap() at line 66 will panic if ShareableContent::current() fails, crashing the CLI without user-friendly context. Since the outer await and map_err (lines 68-69) already handle build() errors, propagate the ShareableContent error through the same path.

Apply this diff:

             .build(
                 #[cfg(target_os = "macos")]
-                cidre::sc::ShareableContent::current().await.unwrap(),
+                cidre::sc::ShareableContent::current()
+                    .await
+                    .map_err(|e| format!("Failed to get shareable content: {}", e))?,
             )
             .await
             .map_err(|e| e.to_string())?;
📝 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
.build(
#[cfg(target_os = "macos")]
cidre::sc::ShareableContent::current().await.unwrap(),
)
.await
.map_err(|e| e.to_string())?;
.build(
#[cfg(target_os = "macos")]
cidre::sc::ShareableContent::current()
.await
.map_err(|e| format!("Failed to get shareable content: {}", e))?,
)
.await
.map_err(|e| e.to_string())?;
🤖 Prompt for AI Agents
In apps/cli/src/record.rs around lines 64 to 69, the code uses unwrap() on
ShareableContent::current() which can panic; replace the unwrap with error
propagation so the ShareableContent error flows through the existing map_err
path. Concretely, change the macOS branch to call
cidre::sc::ShareableContent::current().await.map_err(|e| e.to_string())? (i.e.
await, map the error to a string and use ?), so the failing ShareableContent
creation is returned instead of panicking.


Expand Down
18 changes: 18 additions & 0 deletions apps/desktop/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod presets;
mod recording;
mod recording_settings;
mod target_select_overlay;
mod thumbnails;
mod tray;
mod upload;
mod web_api;
Expand Down Expand Up @@ -79,6 +80,8 @@ use tauri_plugin_notification::{NotificationExt, PermissionState};
use tauri_plugin_opener::OpenerExt;
use tauri_plugin_shell::ShellExt;
use tauri_specta::Event;
#[cfg(target_os = "macos")]
use tokio::sync::Mutex;
use tokio::sync::{RwLock, oneshot};
use tracing::{error, trace};
use upload::{S3UploadMeta, create_or_get_video, upload_image, upload_video};
Expand Down Expand Up @@ -320,6 +323,12 @@ pub struct RequestOpenSettings {
page: String,
}

#[derive(Deserialize, specta::Type, Serialize, tauri_specta::Event, Debug, Clone)]
pub struct RequestScreenCapturePrewarm {
#[serde(default)]
pub force: bool,
}

#[derive(Deserialize, specta::Type, Serialize, tauri_specta::Event, Debug, Clone)]
pub struct NewNotification {
title: String,
Expand Down Expand Up @@ -1940,6 +1949,7 @@ pub async fn run(recording_logging_handle: LoggingHandle) {
RequestOpenRecordingPicker,
RequestNewScreenshot,
RequestOpenSettings,
RequestScreenCapturePrewarm,
NewNotification,
AuthenticationInvalid,
audio_meter::AudioInputLevelChange,
Expand Down Expand Up @@ -2078,6 +2088,8 @@ pub async fn run(recording_logging_handle: LoggingHandle) {
fake_window::init(&app);
app.manage(target_select_overlay::WindowFocusManager::default());
app.manage(EditorWindowIds::default());
#[cfg(target_os = "macos")]
app.manage(crate::platform::ScreenCapturePrewarmer::default());

tokio::spawn({
let camera_feed = camera_feed.clone();
Expand Down Expand Up @@ -2207,6 +2219,12 @@ pub async fn run(recording_logging_handle: LoggingHandle) {
.await;
});

#[cfg(target_os = "macos")]
RequestScreenCapturePrewarm::listen_any_spawn(&app, async |event, app| {
let prewarmer = app.state::<crate::platform::ScreenCapturePrewarmer>();
prewarmer.request(event.force).await;
});

let app_handle = app.clone();
app.deep_link().on_open_url(move |event| {
deeplink_actions::handle(&app_handle, event.urls());
Expand Down
3 changes: 3 additions & 0 deletions apps/desktop/src-tauri/src/platform/macos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
// use objc::{class, msg_send, sel, sel_impl};

pub mod delegates;
mod sc_shareable_content;

pub use sc_shareable_content::*;

pub fn set_window_level(window: tauri::Window, level: objc2_app_kit::NSWindowLevel) {
let c_window = window.clone();
Expand Down
238 changes: 238 additions & 0 deletions apps/desktop/src-tauri/src/platform/macos/sc_shareable_content.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
use cidre::{arc, ns, sc};
use core_graphics::{display::CGDirectDisplayID, window::CGWindowID};
use std::sync::Arc;
use std::{
collections::HashMap,
sync::{OnceLock, RwLock},
time::Instant,
};
use tokio::sync::{Mutex, Notify};
use tracing::{debug, info, trace};

#[derive(Default)]
struct CacheState {
cache: RwLock<Option<ShareableContentCache>>,
warmup: Mutex<Option<WarmupTask>>,
}

type WarmupResult = Result<(), arc::R<ns::Error>>;

#[derive(Clone)]
struct WarmupTask {
notify: Arc<Notify>,
result: Arc<Mutex<Option<WarmupResult>>>,
}

static STATE: OnceLock<CacheState> = OnceLock::new();

fn state() -> &'static CacheState {
STATE.get_or_init(CacheState::default)
}

pub async fn prewarm_shareable_content() -> Result<(), arc::R<ns::Error>> {
if state().cache.read().unwrap().is_some() {
trace!("ScreenCaptureKit shareable content already warmed");
return Ok(());
}

let warmup = {
let mut guard = state().warmup.lock().await;
if let Some(task) = guard.clone() {
trace!("Awaiting in-flight ScreenCaptureKit warmup");
task
} else {
let task = WarmupTask {
notify: Arc::new(Notify::new()),
result: Arc::new(Mutex::new(None)),
};
*guard = Some(task.clone());
tokio::spawn(run_warmup(task.clone()));
task
}
};

warmup.notify.notified().await;
warmup
.result
.lock()
.await
.clone()
.expect("ScreenCaptureKit warmup task missing result")
}

pub async fn get_shareable_content()
-> Result<Option<arc::R<sc::ShareableContent>>, arc::R<ns::Error>> {
let lookup_start = Instant::now();

if let Some(content) = state()
.cache
.read()
.unwrap()
.as_ref()
.map(|v| v.content.retained())
{
trace!(
elapsed_ms = lookup_start.elapsed().as_micros() as f64 / 1000.0,
"Resolved ScreenCaptureKit from warmed cache"
);
return Ok(Some(content));
}

prewarm_shareable_content().await?;

let content = state().cache.read().unwrap();
trace!(
elapsed_ms = lookup_start.elapsed().as_micros() as f64 / 1000.0,
cache_hit = content.is_some(),
"Resolved ScreenCaptureKit after cache populate"
);
Ok(content.as_ref().map(|v| v.content.retained()))
}

async fn run_warmup(task: WarmupTask) {
let result = async {
let warm_start = Instant::now();
debug!("Populating ScreenCaptureKit shareable content cache");

let content = sc::ShareableContent::current().await?;
let cache = ShareableContentCache::new(content);
let elapsed_ms = warm_start.elapsed().as_micros() as f64 / 1000.0;

let mut guard = state().cache.write().unwrap();
let replaced = guard.is_some();
*guard = Some(cache);

info!(
elapsed_ms,
replaced, "ScreenCaptureKit shareable content cache populated"
);
Ok::<(), arc::R<ns::Error>>(())
}
.await;

{
let mut res_guard = task.result.lock().await;
*res_guard = Some(result);
}

task.notify.notify_waiters();

let mut guard = state().warmup.lock().await;
if let Some(current) = guard.as_ref()
&& Arc::ptr_eq(&current.notify, &task.notify)
{
*guard = None;
}
}

#[derive(Debug)]
struct ShareableContentCache {
#[allow(dead_code)]
content: arc::R<sc::ShareableContent>,
displays: HashMap<CGDirectDisplayID, arc::R<sc::Display>>,
windows: HashMap<CGWindowID, arc::R<sc::Window>>,
}
Comment on lines +128 to +134
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

Remove incorrect #[allow(dead_code)] on content field.

The content field is used on line 72 (v.content.retained()), so the #[allow(dead_code)] attribute is incorrect.

Apply this diff:

 #[derive(Debug)]
 struct ShareableContentCache {
-    #[allow(dead_code)]
     content: arc::R<sc::ShareableContent>,
     displays: HashMap<CGDirectDisplayID, arc::R<sc::Display>>,
     windows: HashMap<CGWindowID, arc::R<sc::Window>>,
 }
📝 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
#[derive(Debug)]
struct ShareableContentCache {
#[allow(dead_code)]
content: arc::R<sc::ShareableContent>,
displays: HashMap<CGDirectDisplayID, arc::R<sc::Display>>,
windows: HashMap<CGWindowID, arc::R<sc::Window>>,
}
#[derive(Debug)]
struct ShareableContentCache {
content: arc::R<sc::ShareableContent>,
displays: HashMap<CGDirectDisplayID, arc::R<sc::Display>>,
windows: HashMap<CGWindowID, arc::R<sc::Window>>,
}
🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/platform/macos/sc_shareable_content.rs around
lines 128 to 134, the ShareableContentCache.content field is incorrectly
annotated with #[allow(dead_code)] despite being used elsewhere
(v.content.retained() at line 72); remove the #[allow(dead_code)] attribute from
the content field so the field's usage is correctly recognized by the compiler
and linter.


unsafe impl Send for ShareableContentCache {}
unsafe impl Sync for ShareableContentCache {}
Comment on lines +136 to +137
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 | 🔴 Critical

🧩 Analysis chain

Verify arc::R is thread-safe before manual Send/Sync impls.

Manual unsafe impl Send and Sync for ShareableContentCache assume that arc::R<sc::ShareableContent>, arc::R<sc::Display>, and arc::R<sc::Window> are thread-safe. Verify that cidre::arc::R behaves like std::sync::Arc and is safe to send/share across threads.


🌐 Web query:

Is cidre::arc::R thread-safe and similar to std::sync::Arc in Rust?

💡 Result:

Short answer: No.

Details:

  • In the cidre crate arc module, R is a type alias for Retained, a thin wrapper around a &'static mut T that calls Objective‑C retain/release (the module defines Retained/Allocated and pub type R = Retained). This is an Obj‑C ownership helper, not an atomically reference‑counted, thread‑safe pointer. [1]
  • std::sync::Arc is an atomically reference‑counted, thread‑safe shared pointer (atomic refcounting; implements Send/Sync when T does). R in cidre does not provide that behavior. [2]

If you want thread‑safe shared ownership in Rust use std::sync::Arc (or wrap cidre types in synchronization primitives yourself).

Sources:
[1] cidre crate arc.rs (defines Retained/Allocated and pub type R = Retained). (docs.rs)
[2] std::sync::Arc documentation (thread‑safe, atomically reference counted). (doc.rust-lang.org)

Citations:


Remove manual unsafe impl Send/Sync for ShareableContentCache (apps/desktop/src-tauri/src/platform/macos/sc_shareable_content.rs lines 136–137): cidre::arc::R is not thread-safe. Use std::sync::Arc or wrap the contents in proper synchronization primitives for cross-thread sharing.

🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/platform/macos/sc_shareable_content.rs around
lines 136–137, remove the manual unsafe impl Send/Sync for ShareableContentCache
because cidre::arc::R is not thread-safe; instead make the cache use thread-safe
primitives (for example replace cidre::arc::R with std::sync::Arc if the inner
type is Send + Sync, or wrap the inner data in std::sync::Mutex/RwLock/Arc to
ensure proper synchronization), delete the unsafe impls, and adjust types/usages
so ShareableContentCache is only shared across threads via these safe, standard
synchronization primitives.


impl ShareableContentCache {
fn new(content: arc::R<sc::ShareableContent>) -> Self {
let displays = content
.displays()
.iter()
.map(|display| (display.display_id().0, display.retained()))
.collect();

let windows = content
.windows()
.iter()
.map(|window| (window.id(), window.retained()))
.collect();

Self {
content,
displays,
windows,
}
}

fn display(&self, id: CGDirectDisplayID) -> Option<arc::R<sc::Display>> {
self.displays.get(&id).cloned()
}

fn window(&self, id: CGWindowID) -> Option<arc::R<sc::Window>> {
self.windows.get(&id).cloned()
}
}

pub(crate) struct ScreenCapturePrewarmer {
state: Mutex<PrewarmState>,
}

impl Default for ScreenCapturePrewarmer {
fn default() -> Self {
Self {
state: Mutex::new(PrewarmState::Idle),
}
}
}

impl ScreenCapturePrewarmer {
pub async fn request(&self, force: bool) {
let should_start = {
let mut state = self.state.lock().await;

if force {
*state = PrewarmState::Idle;
}

match *state {
PrewarmState::Idle => {
*state = PrewarmState::Warming;
true
}
PrewarmState::Warming => {
trace!("ScreenCaptureKit prewarm already in progress");
false
}
PrewarmState::Warmed => {
if force {
*state = PrewarmState::Warming;
true
} else {
trace!("ScreenCaptureKit cache already warmed");
false
}
}
}
};

if !should_start {
return;
}

let warm_start = std::time::Instant::now();
let result = crate::platform::prewarm_shareable_content().await;

let mut state = self.state.lock().await;
match result {
Ok(()) => {
let elapsed_ms = warm_start.elapsed().as_micros() as f64 / 1000.0;
*state = PrewarmState::Warmed;
trace!(elapsed_ms, "ScreenCaptureKit cache warmed");
}
Err(error) => {
*state = PrewarmState::Idle;
tracing::warn!(error = %error, "ScreenCaptureKit prewarm failed");
}
}
}
}

#[derive(Clone, Copy, PartialEq, Eq)]
pub enum PrewarmState {
Idle,
Warming,
Warmed,
}
Loading
Loading