Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
25 changes: 17 additions & 8 deletions apps/desktop/src-tauri/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ pub async fn generate_export_preview(
settings: ExportPreviewSettings,
) -> Result<ExportPreviewResult, String> {
use base64::{Engine, engine::general_purpose::STANDARD};
use cap_editor::create_segments;
use cap_editor::create_all_segments;
use std::time::Instant;

let recording_meta = RecordingMeta::load_for_project(&project_path)
Expand All @@ -337,12 +337,16 @@ pub async fn generate_export_preview(
return Err("Cannot preview non-studio recordings".to_string());
};

let project_config =
export_project_config(recording_meta.project_config(), settings.cursor_only);
let source_project_config = recording_meta.project_config();
let project_config = export_project_config(source_project_config.clone(), settings.cursor_only);

let recordings = Arc::new(
ProjectRecordingsMeta::new(&recording_meta.project_path, studio_meta)
.map_err(|e| format!("Failed to load recordings: {e}"))?,
ProjectRecordingsMeta::new_with_external(
&recording_meta.project_path,
studio_meta,
&source_project_config.external_recordings,
)
.map_err(|e| format!("Failed to load recordings: {e}"))?,
);

let render_constants = Arc::new(
Expand All @@ -355,9 +359,14 @@ pub async fn generate_export_preview(
.map_err(|e| format!("Failed to create render constants: {e}"))?,
);

let segments = create_segments(&recording_meta, studio_meta, false)
.await
.map_err(|e| format!("Failed to create segments: {e}"))?;
let segments = create_all_segments(
&recording_meta,
studio_meta,
&source_project_config.external_recordings,
false,
)
.await
.map_err(|e| format!("Failed to create segments: {e}"))?;

let render_segments: Vec<RenderSegment> = segments
.iter()
Expand Down
133 changes: 133 additions & 0 deletions apps/desktop/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,6 +2078,137 @@ 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,
}

#[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().unwrap();
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 ext_meta = RecordingMeta::load_for_project(&recording_path)
.map_err(|e| format!("Failed to load recording meta: {e}"))?;
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}"))?;

if let (Some(primary_first), Some(ext_first)) = (
primary_recordings.segments.first(),
ext_recordings.segments.first(),
) {
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| std::path::Path::new(&r.path) == recording_path)
{
return Err("This recording has already been imported".to_string());
}

let clip_index_offset = (primary_recordings.segments.len()
+ project_config
.external_recordings
.iter()
.map(|r| {
let p = std::path::PathBuf::from(&r.path);
RecordingMeta::load_for_project(&p)
.ok()
.and_then(|m| {
m.studio_meta().map(|s| match s {
cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize,
cap_project::StudioRecordingMeta::MultipleSegments { inner } => {
inner.segments.len()
}
})
})
.unwrap_or(0)
})
.sum::<usize>()) as u32;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Silent unwrap_or(0) corrupts clip indices when a prior external recording is missing

clip_index_offset is computed by summing the segment count of every previously-imported external recording. If any of those paths is missing or has been moved, load_for_project fails and the closure returns 0 for that recording. As a result the new recording's recording_clip indices are computed too low, silently colliding with the indices of an existing external recording in ProjectRecordingsMeta::segments. The timeline would then map multiple TimelineSegments to the wrong clips.

Consider propagating the error instead of swallowing it:

let ext_segment_counts = project_config
    .external_recordings
    .iter()
    .enumerate()
    .map(|(i, r)| {
        let p = std::path::PathBuf::from(&r.path);
        let m = RecordingMeta::load_for_project(&p)
            .map_err(|e| format!("existing external recording {i}: {e}"))?;
        Ok(m.studio_meta().map(|s| match s {
            cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize,
            cap_project::StudioRecordingMeta::MultipleSegments { inner } => inner.segments.len(),
        }).unwrap_or(0))
    })
    .collect::<Result<Vec<_>, String>>()?;

let clip_index_offset = (primary_recordings.segments.len()
    + ext_segment_counts.iter().sum::<usize>()) as u32;
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/lib.rs
Line: 2155-2173

Comment:
**Silent `unwrap_or(0)` corrupts clip indices when a prior external recording is missing**

`clip_index_offset` is computed by summing the segment count of every previously-imported external recording. If any of those paths is missing or has been moved, `load_for_project` fails and the closure returns `0` for that recording. As a result the new recording's `recording_clip` indices are computed too low, silently colliding with the indices of an existing external recording in `ProjectRecordingsMeta::segments`. The timeline would then map multiple `TimelineSegment`s to the wrong clips.

Consider propagating the error instead of swallowing it:
```rust
let ext_segment_counts = project_config
    .external_recordings
    .iter()
    .enumerate()
    .map(|(i, r)| {
        let p = std::path::PathBuf::from(&r.path);
        let m = RecordingMeta::load_for_project(&p)
            .map_err(|e| format!("existing external recording {i}: {e}"))?;
        Ok(m.studio_meta().map(|s| match s {
            cap_project::StudioRecordingMeta::SingleSegment { .. } => 1usize,
            cap_project::StudioRecordingMeta::MultipleSegments { inner } => inner.segments.len(),
        }).unwrap_or(0))
    })
    .collect::<Result<Vec<_>, String>>()?;

let clip_index_offset = (primary_recordings.segments.len()
    + ext_segment_counts.iter().sum::<usize>()) as u32;
```

How can I resolve this? If you propose a fix, please make it concise.


let label = ext_meta.pretty_name.clone();

project_config
.external_recordings
.push(cap_project::ExternalRecordingReference {
path: recording_path.to_string_lossy().to_string(),
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,
});
}

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_string_lossy().to_string(),
}
.emit(&window)
.map_err(|e| format!("Failed to emit event: {e}"))?;

Ok(())
}

#[tauri::command]
#[specta::specta]
#[instrument(skip(editor))]
Expand Down Expand Up @@ -3355,6 +3486,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
export::generate_export_preview_fast,
import::start_video_import,
import::check_import_ready,
import_cap_recording,
copy_file_to_path,
copy_video_to_clipboard,
copy_screenshot_to_clipboard,
Expand Down Expand Up @@ -3461,6 +3593,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) {
hotkeys::OnEscapePress,
upload::UploadProgressEvent,
import::VideoImportProgress,
CapRecordingImported,
SetCaptureAreaPending,
DevicesUpdated,
])
Expand Down
5 changes: 2 additions & 3 deletions apps/desktop/src-tauri/tauri.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
},
"bundle": {
"active": true,
"createUpdaterArtifacts": true,
"createUpdaterArtifacts": false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Likely unintentional: createUpdaterArtifacts set to false

This change disables generation of auto-update artifacts (.sig / .tar.gz / .nsis.zip) at bundle time. If the Cap desktop app ships auto-update, this will silently break it for any release built from this commit — users on older versions won't receive the update. The previous value was true. Please verify this was intentional; if not, revert.

Suggested change
"createUpdaterArtifacts": false,
"createUpdaterArtifacts": true,
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/tauri.conf.json
Line: 39

Comment:
**Likely unintentional: `createUpdaterArtifacts` set to `false`**

This change disables generation of auto-update artifacts (`.sig` / `.tar.gz` / `.nsis.zip`) at bundle time. If the Cap desktop app ships auto-update, this will silently break it for any release built from this commit — users on older versions won't receive the update. The previous value was `true`. Please verify this was intentional; if not, revert.

```suggestion
		"createUpdaterArtifacts": true,
```

How can I resolve this? If you propose a fix, please make it concise.

"targets": "all",
"icon": [
"icons/32x32.png",
Expand Down Expand Up @@ -64,8 +64,7 @@
"x": 480,
"y": 140
}
},
"frameworks": ["../../../target/native-deps/Spacedrive.framework"]
}
},
"windows": {
"nsis": {
Expand Down
83 changes: 81 additions & 2 deletions apps/desktop/src/routes/editor/Timeline/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createElementBounds } from "@solid-primitives/bounds";
import { createEventListener } from "@solid-primitives/event-listener";
import { LogicalPosition } from "@tauri-apps/api/dpi";
import { Menu, MenuItem } from "@tauri-apps/api/menu";
import { open as openDialog } from "@tauri-apps/plugin-dialog";
import { platform } from "@tauri-apps/plugin-os";
import { cx } from "cva";
import {
Expand All @@ -25,7 +26,7 @@ import "./styles.css";
import Tooltip from "~/components/Tooltip";
import { defaultCaptionSettings } from "~/store/captions";
import { defaultKeyboardSettings } from "~/store/keyboard";
import { commands } from "~/utils/tauri";
import { commands, events } from "~/utils/tauri";
import {
applyCaptionResultToProject,
getCaptionGenerationErrorMessage,
Expand Down Expand Up @@ -722,6 +723,35 @@ export function Timeline(props: {
}
};

const [isImporting, setIsImporting] = createSignal(false);

const handleImportCapRecording = async () => {
const selected = await openDialog({
directory: true,
title: "Select a Cap Recording to Import",
filters: [{ name: "Cap Recording", extensions: ["cap"] }],
});
Comment on lines +728 to +732
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 filters has no effect for directory pickers on most platforms

openDialog is called with directory: true, which opens a folder-selection dialog. The filters: [{ name: "Cap Recording", extensions: ["cap"] }] option is designed for file pickers and is ignored by OS-level folder dialogs on macOS and Windows. The .endsWith(".cap") check below correctly guards against bad input, but the filter gives a false impression of UI-side enforcement.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Timeline/index.tsx
Line: 728-733

Comment:
**`filters` has no effect for directory pickers on most platforms**

`openDialog` is called with `directory: true`, which opens a folder-selection dialog. The `filters: [{ name: "Cap Recording", extensions: ["cap"] }]` option is designed for file pickers and is ignored by OS-level folder dialogs on macOS and Windows. The `.endsWith(".cap")` check below correctly guards against bad input, but the filter gives a false impression of UI-side enforcement.

How can I resolve this? If you propose a fix, please make it concise.

if (!selected || typeof selected !== "string") return;
if (!selected.endsWith(".cap")) {
toast.error("Please select a .cap recording folder");
return;
}
setIsImporting(true);
try {
await commands.importCapRecording(selected);
} catch (e) {
toast.error(String(e));
setIsImporting(false);
}
};

const importedListenerPromise = events.capRecordingImported.listen(() => {
window.location.reload();
});
onCleanup(() => {
importedListenerPromise.then((unlisten) => unlisten());
});

const split = () => editorState.timeline.interactMode === "split";

const maskImage = () => {
Expand Down Expand Up @@ -840,14 +870,29 @@ export function Timeline(props: {
<div class="absolute inset-0 flex items-end">
<TimelineMarkings />
</div>
<div class="absolute bottom-0 z-30">
<div class="absolute bottom-0 z-30 flex items-center gap-2">
<Tooltip content="Add track">
<TrackManager
options={trackOptions()}
onToggle={handleToggleTrack}
onAdd={handleAddTrack}
/>
</Tooltip>
<Tooltip content="Import Cap recording">
<button
class="flex items-center gap-1.5 px-3 py-1.5 rounded-xl bg-blue-500 text-white text-xs font-medium disabled:opacity-50"
onClick={handleImportCapRecording}
disabled={isImporting()}
>
<Show
when={isImporting()}
fallback={<IconLucidePlus class="size-3" />}
>
<IconLucideLoader2 class="size-3 animate-spin" />
</Show>
Import recording
</button>
</Tooltip>
</div>
</div>
<Show when={!editorState.playing && editorState.previewTime}>
Expand Down Expand Up @@ -998,6 +1043,19 @@ export function Timeline(props: {
/>
</TrackRow>
</Show>
<button
class="flex items-center gap-1.5 px-3 py-2 mt-1 rounded-xl bg-blue-500 text-white text-xs font-medium disabled:opacity-50 self-start"
onClick={handleImportCapRecording}
disabled={isImporting()}
>
<Show
when={isImporting()}
fallback={<IconLucidePlus class="size-3" />}
>
<IconLucideLoader2 class="size-3 animate-spin" />
</Show>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Duplicate "Import recording" button

There are now two separate "Import recording" buttons that call the same handler: one added to the timeline toolbar (line ~878) and this one inside the track-rows scroll area. If this second one is intended as an empty-state CTA, it should be wrapped in a <Show> guard; otherwise having two identical call-to-action buttons in the same view is likely to confuse users.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/Timeline/index.tsx
Line: 1045-1056

Comment:
**Duplicate "Import recording" button**

There are now two separate "Import recording" buttons that call the same handler: one added to the timeline toolbar (line ~878) and this one inside the track-rows scroll area. If this second one is intended as an empty-state CTA, it should be wrapped in a `<Show>` guard; otherwise having two identical call-to-action buttons in the same view is likely to confuse users.

How can I resolve this? If you propose a fix, please make it concise.

Import recording
</button>
</div>
</div>
</div>
Expand All @@ -1011,6 +1069,8 @@ function TrackRow(props: {
children: JSX.Element;
onDelete?: () => void;
onContextMenu?: (e: MouseEvent) => void;
onImport?: () => void;
importing?: boolean;
}) {
return (
<div
Expand Down Expand Up @@ -1039,6 +1099,25 @@ function TrackRow(props: {
<IconCapTrash class="size-4" />
</button>
</Show>
<Show when={props.onImport}>
<button
class="absolute inset-0 z-20 flex items-center justify-center rounded-xl border border-blue-400/70 bg-blue-500/90 text-white disabled:opacity-50"
onClick={(e) => {
e.stopPropagation();
props.onImport?.();
}}
onMouseDown={(e) => e.stopPropagation()}
disabled={props.importing}
title="Import Cap recording"
>
<Show
when={props.importing}
fallback={<IconLucidePlus class="size-4" />}
>
<IconLucideLoader2 class="size-4 animate-spin" />
</Show>
</button>
</Show>
</div>
<div class="flex-1 relative overflow-hidden min-w-0">
{props.children}
Expand Down
Loading
Loading