-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(editor): Cap recording import for timeline stitching #1719
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
base: main
Are you sure you want to change the base?
Changes from all commits
5207fbb
34cd3f4
c2ecf24
386ed05
092c0cc
c462553
05b46dd
37c7059
6fdc529
bafd548
cdf2607
dde84ab
4cb97b3
38cc5fe
a69f066
c23e33c
f5fcd8c
cd872f9
ffc59ea
0aeb48a
1f4b33f
52e87a3
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2799,7 +2799,10 @@ async fn create_editor_instance(window: Window) -> Result<SerializedEditorInstan | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let path = { | ||||||||||||||||||||||||
| let window_ids = EditorWindowIds::get(window.app_handle()); | ||||||||||||||||||||||||
| let window_ids = window_ids.ids.lock().unwrap(); | ||||||||||||||||||||||||
| let window_ids = window_ids | ||||||||||||||||||||||||
| .ids | ||||||||||||||||||||||||
| .lock() | ||||||||||||||||||||||||
| .map_err(|_| "Failed to lock editor window ids".to_string())?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let Some((path, _)) = window_ids.iter().find(|(_, _id)| *_id == id) else { | ||||||||||||||||||||||||
| return Err("Editor instance not found".to_string()); | ||||||||||||||||||||||||
|
|
@@ -2836,7 +2839,10 @@ async fn get_editor_project_path(window: Window) -> Result<PathBuf, String> { | |||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let window_ids = EditorWindowIds::get(window.app_handle()); | ||||||||||||||||||||||||
| let window_ids = window_ids.ids.lock().unwrap(); | ||||||||||||||||||||||||
| let window_ids = window_ids | ||||||||||||||||||||||||
| .ids | ||||||||||||||||||||||||
| .lock() | ||||||||||||||||||||||||
| .map_err(|_| "Failed to lock editor window ids".to_string())?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let Some((path, _)) = window_ids.iter().find(|(_, _id)| *_id == id) else { | ||||||||||||||||||||||||
| return Err("Editor instance not found".to_string()); | ||||||||||||||||||||||||
|
|
@@ -2845,6 +2851,198 @@ async fn get_editor_project_path(window: Window) -> Result<PathBuf, String> { | |||||||||||||||||||||||
| Ok(path.clone()) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[derive(Serialize, Type, tauri_specta::Event, Clone, Debug)] | ||||||||||||||||||||||||
| pub struct CapRecordingImported { | ||||||||||||||||||||||||
| pub project_path: String, | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| fn relative_path_from(base: &std::path::Path, target: &std::path::Path) -> PathBuf { | ||||||||||||||||||||||||
| #[cfg(windows)] | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| use std::path::Component; | ||||||||||||||||||||||||
| if let (Some(Component::Prefix(a)), Some(Component::Prefix(b))) = | ||||||||||||||||||||||||
| (base.components().next(), target.components().next()) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if a != b { | ||||||||||||||||||||||||
| return target.to_path_buf(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| let base: Vec<_> = base.components().collect(); | ||||||||||||||||||||||||
| let target: Vec<_> = target.components().collect(); | ||||||||||||||||||||||||
| let common = base.iter().zip(&target).take_while(|(a, b)| a == b).count(); | ||||||||||||||||||||||||
| let mut rel = PathBuf::new(); | ||||||||||||||||||||||||
| for _ in 0..(base.len() - common) { | ||||||||||||||||||||||||
| rel.push(".."); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| for c in &target[common..] { | ||||||||||||||||||||||||
| rel.push(c); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| rel | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| fn resolve_recording_path(stored: &str, project_path: &std::path::Path) -> PathBuf { | ||||||||||||||||||||||||
| let p = std::path::Path::new(stored); | ||||||||||||||||||||||||
| if p.is_absolute() { | ||||||||||||||||||||||||
| p.to_path_buf() | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| project_path.join(p) | ||||||||||||||||||||||||
|
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. P2: Stored external recording paths are not sanitized, allowing path traversal Unsanitized external recording path strings from project config are joined against the project directory, allowing traversal outside the project scope. Validate that resolved external recording paths are contained within an allowed base directory before loading. AI prompt |
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[tauri::command] | ||||||||||||||||||||||||
| #[specta::specta] | ||||||||||||||||||||||||
| async fn import_cap_recording(window: Window, recording_path: PathBuf) -> Result<(), String> { | ||||||||||||||||||||||||
| let CapWindowId::Editor { id } = | ||||||||||||||||||||||||
| CapWindowId::from_str(window.label()).map_err(|e| e.to_string())? | ||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||
| return Err("Invalid window".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let project_path = { | ||||||||||||||||||||||||
| let window_ids = EditorWindowIds::get(window.app_handle()); | ||||||||||||||||||||||||
| let window_ids = window_ids | ||||||||||||||||||||||||
| .ids | ||||||||||||||||||||||||
| .lock() | ||||||||||||||||||||||||
| .map_err(|_| "Failed to lock editor window ids".to_string())?; | ||||||||||||||||||||||||
| let Some((path, _)) = window_ids.iter().find(|(_, _id)| *_id == id) else { | ||||||||||||||||||||||||
| return Err("Editor instance not found".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| path.clone() | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if !recording_path.exists() || !recording_path.join("recording-meta.json").exists() { | ||||||||||||||||||||||||
| return Err("Not a valid Cap recording".to_string()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let recording_path = recording_path | ||||||||||||||||||||||||
| .canonicalize() | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to resolve recording path: {e}"))?; | ||||||||||||||||||||||||
|
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. Worth guarding against importing the project into itself (user selects the current
Suggested change
|
||||||||||||||||||||||||
| let project_path = project_path | ||||||||||||||||||||||||
| .canonicalize() | ||||||||||||||||||||||||
|
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 edge case: since |
||||||||||||||||||||||||
| .map_err(|e| format!("Failed to resolve project path: {e}"))?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if recording_path == project_path { | ||||||||||||||||||||||||
| return Err("Cannot import a recording into itself".to_string()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let ext_meta = RecordingMeta::load_for_project(&recording_path) | ||||||||||||||||||||||||
|
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 but helpful for dedupe/stability: canonicalize
Suggested change
|
||||||||||||||||||||||||
| .map_err(|e| format!("Failed to load recording meta: {e}"))?; | ||||||||||||||||||||||||
|
Comment on lines
+2929
to
+2930
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. Worth guarding against importing the project into itself (or via a symlink). Right now selecting the current project’s
Suggested change
|
||||||||||||||||||||||||
| let RecordingMetaInner::Studio(ext_studio_meta) = &ext_meta.inner else { | ||||||||||||||||||||||||
| return Err("External recording is not a studio recording".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let primary_meta = RecordingMeta::load_for_project(&project_path) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to load project meta: {e}"))?; | ||||||||||||||||||||||||
| let RecordingMetaInner::Studio(primary_studio_meta) = &primary_meta.inner else { | ||||||||||||||||||||||||
| return Err("Project is not a studio recording".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let primary_recordings = | ||||||||||||||||||||||||
| cap_rendering::ProjectRecordingsMeta::new(&primary_meta.project_path, primary_studio_meta) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to load primary recordings: {e}"))?; | ||||||||||||||||||||||||
| let ext_recordings = | ||||||||||||||||||||||||
| cap_rendering::ProjectRecordingsMeta::new(&recording_path, ext_studio_meta) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to load external recordings: {e}"))?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let Some(primary_first) = primary_recordings.segments.first() else { | ||||||||||||||||||||||||
| return Err("Project has no segments".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| let Some(ext_first) = ext_recordings.segments.first() else { | ||||||||||||||||||||||||
| return Err("External recording has no segments".to_string()); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| if ext_first.display.width != primary_first.display.width | ||||||||||||||||||||||||
| || ext_first.display.height != primary_first.display.height | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| return Err(format!( | ||||||||||||||||||||||||
| "Recording resolution {}x{} does not match project resolution {}x{}", | ||||||||||||||||||||||||
| ext_first.display.width, | ||||||||||||||||||||||||
| ext_first.display.height, | ||||||||||||||||||||||||
| primary_first.display.width, | ||||||||||||||||||||||||
| primary_first.display.height, | ||||||||||||||||||||||||
| )); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let mut project_config = ProjectConfiguration::load(&project_path) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to load project config: {e}"))?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if project_config.external_recordings.iter().any(|r| { | ||||||||||||||||||||||||
| resolve_recording_path(&r.path, &project_path) | ||||||||||||||||||||||||
| .canonicalize() | ||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||
| .as_deref() | ||||||||||||||||||||||||
| == Some(recording_path.as_path()) | ||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||
| return Err("This recording has already been imported".to_string()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let ext_segment_counts = project_config | ||||||||||||||||||||||||
| .external_recordings | ||||||||||||||||||||||||
| .iter() | ||||||||||||||||||||||||
| .enumerate() | ||||||||||||||||||||||||
| .map(|(i, r)| { | ||||||||||||||||||||||||
| let p = resolve_recording_path(&r.path, &project_path); | ||||||||||||||||||||||||
| let m = RecordingMeta::load_for_project(&p) | ||||||||||||||||||||||||
| .map_err(|e| format!("existing external recording {i}: {e}"))?; | ||||||||||||||||||||||||
| let studio = m.studio_meta().ok_or_else(|| { | ||||||||||||||||||||||||
| format!("existing external recording {i}: not a studio recording") | ||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||
| Ok(match studio { | ||||||||||||||||||||||||
| cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize, | ||||||||||||||||||||||||
| cap_project::StudioRecordingMeta::MultipleSegments { inner } => { | ||||||||||||||||||||||||
| inner.segments.len() | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
| .collect::<Result<Vec<_>, String>>()?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let clip_index_offset = | ||||||||||||||||||||||||
| (primary_recordings.segments.len() + ext_segment_counts.iter().sum::<usize>()) as u32; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let label = ext_meta.pretty_name.clone(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
Comment on lines
+3001
to
+3003
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.
Suggested change
|
||||||||||||||||||||||||
| project_config | ||||||||||||||||||||||||
| .external_recordings | ||||||||||||||||||||||||
| .push(cap_project::ExternalRecordingReference { | ||||||||||||||||||||||||
| path: relative_path_from(&project_path, &recording_path) | ||||||||||||||||||||||||
| .to_str() | ||||||||||||||||||||||||
| .ok_or("Recording path contains non-UTF-8 characters")? | ||||||||||||||||||||||||
| .to_string(), | ||||||||||||||||||||||||
|
Comment on lines
+3007
to
+3010
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, but
Suggested change
|
||||||||||||||||||||||||
| label: Some(label), | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let timeline = project_config.timeline.get_or_insert_with(Default::default); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| let ext_segment_count = ext_recordings.segments.len(); | ||||||||||||||||||||||||
| for i in 0..ext_segment_count { | ||||||||||||||||||||||||
| let duration = ext_recordings.segments[i].duration(); | ||||||||||||||||||||||||
| timeline.segments.push(cap_project::TimelineSegment { | ||||||||||||||||||||||||
| recording_clip: clip_index_offset + i as u32, | ||||||||||||||||||||||||
| start: 0.0, | ||||||||||||||||||||||||
| end: duration, | ||||||||||||||||||||||||
| timescale: 1.0, | ||||||||||||||||||||||||
| name: None, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| project_config | ||||||||||||||||||||||||
| .write(&project_path) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to save project config: {e}"))?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| EditorInstances::remove(window.clone()).await; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| CapRecordingImported { | ||||||||||||||||||||||||
| project_path: project_path | ||||||||||||||||||||||||
| .to_str() | ||||||||||||||||||||||||
| .ok_or("Project path contains non-UTF-8 characters")? | ||||||||||||||||||||||||
| .to_string(), | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| .emit(&window) | ||||||||||||||||||||||||
| .map_err(|e| format!("Failed to emit event: {e}"))?; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #[tauri::command] | ||||||||||||||||||||||||
| #[specta::specta] | ||||||||||||||||||||||||
| #[instrument(skip(editor))] | ||||||||||||||||||||||||
|
|
@@ -4320,6 +4518,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||
| import::add_existing_recording_to_editor, | ||||||||||||||||||||||||
| import::start_image_import, | ||||||||||||||||||||||||
| import::check_import_ready, | ||||||||||||||||||||||||
| import_cap_recording, | ||||||||||||||||||||||||
| copy_file_to_path, | ||||||||||||||||||||||||
| copy_video_to_clipboard, | ||||||||||||||||||||||||
| copy_screenshot_to_clipboard, | ||||||||||||||||||||||||
|
|
@@ -4441,6 +4640,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||
| hotkeys::OnEscapePress, | ||||||||||||||||||||||||
| upload::UploadProgressEvent, | ||||||||||||||||||||||||
| import::VideoImportProgress, | ||||||||||||||||||||||||
| CapRecordingImported, | ||||||||||||||||||||||||
| SetCaptureAreaPending, | ||||||||||||||||||||||||
| DevicesUpdated, | ||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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.
relative_path_fromassumesbase/targetshare a common prefix. On Windows, importing from a different drive (or UNC vs disk) will produce a nonsense path like..\..\C:\...and then break subsequent loads.