-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: implement actor-based hardware backend with resilience loops for recording stability #1816
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.
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
Changes from all commits
8b1ee43
5f29703
e4d1b4c
bec1ab2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,14 @@ use std::path::{Path, PathBuf}; | |||||||||||||||||||||||||||||||||
| use tauri::{AppHandle, Manager, Url}; | ||||||||||||||||||||||||||||||||||
| use tracing::trace; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| use crate::{App, ArcLock, recording::StartRecordingInputs, windows::ShowCapWindow}; | ||||||||||||||||||||||||||||||||||
| use crate::{ | ||||||||||||||||||||||||||||||||||
| App, ArcLock, | ||||||||||||||||||||||||||||||||||
| recording::{ | ||||||||||||||||||||||||||||||||||
| StartRecordingInputs, pause_recording, resume_recording, | ||||||||||||||||||||||||||||||||||
| toggle_pause, take_screenshot | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 11-14
Comment:
**Unused imports**
`pause_recording`, `resume_recording`, `toggle_pause`, and `take_screenshot` are imported here but every call site uses the fully-qualified `crate::recording::` path instead. These imports are dead — the compiler will emit `unused import` warnings, and `toggle_pause` will additionally cause an unresolved-import error as noted above.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||
| windows::ShowCapWindow | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #[derive(Debug, Serialize, Deserialize)] | ||||||||||||||||||||||||||||||||||
| #[serde(rename_all = "snake_case")] | ||||||||||||||||||||||||||||||||||
|
|
@@ -26,6 +33,16 @@ pub enum DeepLinkAction { | |||||||||||||||||||||||||||||||||
| mode: RecordingMode, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| StopRecording, | ||||||||||||||||||||||||||||||||||
| PauseRecording, | ||||||||||||||||||||||||||||||||||
| ResumeRecording, | ||||||||||||||||||||||||||||||||||
| TogglePause, | ||||||||||||||||||||||||||||||||||
| TakeScreenshot, | ||||||||||||||||||||||||||||||||||
| SwitchMicrophone { | ||||||||||||||||||||||||||||||||||
| mic_label: Option<String>, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| SwitchCamera { | ||||||||||||||||||||||||||||||||||
| camera: Option<DeviceOrModelID>, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| OpenEditor { | ||||||||||||||||||||||||||||||||||
| project_path: PathBuf, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
|
@@ -49,7 +66,6 @@ pub fn handle(app_handle: &AppHandle, urls: Vec<Url>) { | |||||||||||||||||||||||||||||||||
| ActionParseFromUrlError::Invalid => { | ||||||||||||||||||||||||||||||||||
| eprintln!("Invalid deep link format \"{}\"", &url) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| // Likely login action, not handled here. | ||||||||||||||||||||||||||||||||||
| ActionParseFromUrlError::NotAction => {} | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||||||||||||
|
|
@@ -147,6 +163,28 @@ impl DeepLinkAction { | |||||||||||||||||||||||||||||||||
| DeepLinkAction::StopRecording => { | ||||||||||||||||||||||||||||||||||
| crate::recording::stop_recording(app.clone(), app.state()).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::PauseRecording => { | ||||||||||||||||||||||||||||||||||
| crate::recording::pause_recording(app.clone(), app.state()).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::ResumeRecording => { | ||||||||||||||||||||||||||||||||||
| crate::recording::resume_recording(app.clone(), app.state()).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::TogglePause => { | ||||||||||||||||||||||||||||||||||
| crate::recording::toggle_pause_recording(app.clone(), app.state()).await | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Deep links expose screenshot and hardware controls without authentication Deep links can trigger screenshots and camera/mic switching with only a focus check. Fix: Remove these deep-link actions or gate them with user confirmation and a per-session nonce. |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::TakeScreenshot => { | ||||||||||||||||||||||||||||||||||
| let window = app.get_window("main").unwrap(); | ||||||||||||||||||||||||||||||||||
| if !window.is_focused().unwrap_or(false) { return; } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+176
to
+177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor thing in the focus gate:
Suggested change
|
||||||||||||||||||||||||||||||||||
| crate::recording::take_screenshot(app.clone(), app.state()).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+175
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/desktop/src-tauri/src/deeplink_actions.rs
Line: 175-177
Comment:
**Wrong argument types passed to `take_screenshot`**
`recording::take_screenshot` is declared as `async fn take_screenshot(app: AppHandle, target: ScreenCaptureTarget) -> Result<PathBuf, String>`, but it is called here with `(app.clone(), app.state())` — passing a `MutableState<'_, App>` where a `ScreenCaptureTarget` is required. This is a type mismatch that will prevent compilation, and even if it compiled, no capture target would be supplied.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+175
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As written this won’t compile: One low-friction option is to default to the first display and ignore the returned path:
Suggested change
|
||||||||||||||||||||||||||||||||||
| DeepLinkAction::SwitchMicrophone { mic_label } => { | ||||||||||||||||||||||||||||||||||
| let state = app.state::<ArcLock<App>>(); | ||||||||||||||||||||||||||||||||||
| crate::set_mic_input(state, mic_label).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::SwitchCamera { camera } => { | ||||||||||||||||||||||||||||||||||
| let state = app.state::<ArcLock<App>>(); | ||||||||||||||||||||||||||||||||||
| crate::set_camera_input(app.clone(), state, camera, None).await | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| DeepLinkAction::OpenEditor { project_path } => { | ||||||||||||||||||||||||||||||||||
| crate::open_project_from_path(Path::new(&project_path), app.clone()) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ mod flags; | |
| mod frame_ws; | ||
| mod general_settings; | ||
| mod hotkeys; | ||
| mod hardware_handlers; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/desktop/src-tauri/src/lib.rs
Line: 19
Comment:
**Missing module file — build will not compile**
`mod hardware_handlers;` declares a module but neither `src/hardware_handlers.rs` nor `src/hardware_handlers/mod.rs` exists in the repository. Rust will fail to compile the crate with `error[E0583]: file not found for module 'hardware_handlers'`. The PR description references this module as containing actor-based camera/microphone logic, but the actual file was never committed.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail to compile unless |
||
| mod http_client; | ||
| mod import; | ||
| mod logging; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toggle_pausedoes not exist incrate::recording. The actual function is namedtoggle_pause_recording(seerecording.rs:1539). This causes two compile errors: the import on this line, and the callcrate::recording::toggle_pause(...)at theTogglePausematch arm. Any deep-linkTogglePauseaction is therefore impossible to dispatch.Prompt To Fix With AI