Skip to content
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

fix(export): prevent export progress from freezing at 102% #232

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
44 changes: 27 additions & 17 deletions apps/desktop/src-tauri/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,24 @@ pub async fn export_video(
force: bool,
use_custom_muxer: bool,
) -> Result<PathBuf, String> {
let VideoRecordingMetadata { duration, .. } =
get_video_metadata(app.clone(), video_id.clone(), Some(VideoType::Screen))
.await
.unwrap();
let metadata = match get_video_metadata(app.clone(), video_id.clone(), Some(VideoType::Screen)).await {
Ok(meta) => meta,
Err(e) => {
sentry::capture_message(&format!("Failed to get video metadata: {}", e), sentry::Level::Error);
return Err("Failed to read video metadata. The recording may be from an incompatible version.".to_string());
}
};

// 30 FPS (calculated for output video)
let total_frames = (duration * 30.0).round() as u32;
let VideoRecordingMetadata { duration, .. } = metadata;

// Calculate total frames with ceiling to ensure we don't exceed 100%
let total_frames = ((duration * 30.0).ceil() as u32).max(1);

let editor_instance = upsert_editor_instance(&app, video_id.clone()).await;

let output_path = editor_instance.meta().output_path();

// If the file exists, return it immediately
// If the file exists and we're not forcing a re-render, return it
if output_path.exists() && !force {
return Ok(output_path);
}
Expand All @@ -41,9 +46,11 @@ pub async fn export_video(
project,
output_path.clone(),
move |frame_index| {
// Ensure progress never exceeds total frames
let current_frame = (frame_index + 1).min(total_frames);
progress
.send(RenderProgress::FrameRendered {
current_frame: frame_index + 1,
current_frame,
})
.ok();
},
Expand All @@ -57,17 +64,20 @@ pub async fn export_video(
e.to_string()
})?;

if use_custom_muxer {
let result = if use_custom_muxer {
exporter.export_with_custom_muxer().await
} else {
exporter.export_with_ffmpeg_cli().await
}
.map_err(|e| {
sentry::capture_message(&e.to_string(), sentry::Level::Error);
e.to_string()
})?;
};

ShowCapWindow::PrevRecordings.show(&app).ok();

Ok(output_path)
match result {
Ok(_) => {
ShowCapWindow::PrevRecordings.show(&app).ok();
Ok(output_path)
}
Err(e) => {
sentry::capture_message(&e.to_string(), sentry::Level::Error);
Err(e.to_string())
}
}
}
64 changes: 42 additions & 22 deletions apps/desktop/src/routes/editor/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ import { save } from "@tauri-apps/plugin-dialog";
import { DEFAULT_PROJECT_CONFIG } from "./projectConfig";
import { createMutation } from "@tanstack/solid-query";
import { getRequestEvent } from "solid-js/web";
import { checkIsUpgradedAndUpdate } from "~/utils/plans";

function ExportButton() {
const { videoId, project, prettyName } = useEditorContext();
Expand All @@ -260,19 +261,26 @@ function ExportButton() {
const progress = new Channel<RenderProgress>();
progress.onmessage = (p) => {
if (p.type === "FrameRendered" && progressState.type === "saving") {
const percentComplete = Math.round(
(p.current_frame / (progressState.totalFrames || 1)) * 100
const percentComplete = Math.min(
Math.round((p.current_frame / (progressState.totalFrames || 1)) * 100),
100

Choose a reason for hiding this comment

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

Why do we use 'Math.min' here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so it will not exceed 100%

);

setProgressState({
...progressState,
renderProgress: p.current_frame,
message: `Rendering video - ${percentComplete}%`,
});

// If rendering is complete, update to finalizing state
if (percentComplete === 100) {
setProgressState({
...progressState,
message: "Finalizing export...",
});
}
}
if (
p.type === "EstimatedTotalFrames" &&
progressState.type === "saving"
) {
if (p.type === "EstimatedTotalFrames" && progressState.type === "saving") {
setProgressState({
...progressState,
totalFrames: p.total_frames,
Expand All @@ -281,25 +289,30 @@ function ExportButton() {
}
};

const videoPath = await commands.exportVideo(
videoId,
project,
progress,
true,
useCustomMuxer
);
await commands.copyFileToPath(videoPath, path);
try {
const videoPath = await commands.exportVideo(
videoId,
project,
progress,
true,
useCustomMuxer
);
await commands.copyFileToPath(videoPath, path);

setProgressState({
type: "saving",
progress: 100,
message: "Saved successfully!",
mediaPath: path,
});
setProgressState({
type: "saving",
progress: 100,
message: "Saved successfully!",
mediaPath: path,
});

setTimeout(() => {
setTimeout(() => {
setProgressState({ type: "idle" });
}, 1500);
} catch (error) {
setProgressState({ type: "idle" });
}, 1500);
throw error;
}
},
}));

Expand Down Expand Up @@ -330,6 +343,13 @@ function ShareButton() {
throw new Error("Recording metadata not available");
}

// Check for pro access first before starting the export
const isUpgraded = await checkIsUpgradedAndUpdate();
if (!isUpgraded) {
await commands.showWindow("Upgrade");
throw new Error("Upgrade required to share recordings");
}

let unlisten: (() => void) | undefined;

try {
Expand Down
16 changes: 14 additions & 2 deletions apps/desktop/src/routes/recordings-overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,26 @@ export default function () {
undefined &&
progressState.totalFrames
) {
return `${Math.min(
const progress = Math.min(
Math.round(
(progressState.renderProgress /
progressState.totalFrames) *
100
),
100
)}%`;
);

// If we hit 100%, transition to the next stage
if (progress === 100 && progressState.type === "uploading") {
setProgressState({
...progressState,
stage: "uploading",
message: "Starting upload...",
uploadProgress: 0
});
}

return `${progress}%`;
}

return progressState.message;
Expand Down
Loading