Fix video export resolution scaling and post-export UI hang
- Scale the document to the selected output resolution. It was rendered at document size regardless of the export dimensions, so picking a different resolution didn't scale the stage. - Run the audio+video mux on a background thread instead of the UI thread, keeping the app responsive (showing "Finalizing") during the re-mux pass. - Send desktop notifications fire-and-forget. notify_rust's show() is a synchronous D-Bus call that blocked the UI for the full service activation timeout (~25s) when no notification daemon is running.
This commit is contained in:
parent
318720f89d
commit
83609cc9dc
|
|
@ -113,10 +113,10 @@ struct ParallelExportState {
|
||||||
video_progress_rx: Receiver<ExportProgress>,
|
video_progress_rx: Receiver<ExportProgress>,
|
||||||
/// Audio progress channel
|
/// Audio progress channel
|
||||||
audio_progress_rx: Receiver<ExportProgress>,
|
audio_progress_rx: Receiver<ExportProgress>,
|
||||||
/// Video encoder thread handle
|
/// Video encoder thread handle (taken when the mux thread is spawned).
|
||||||
video_thread: std::thread::JoinHandle<()>,
|
video_thread: Option<std::thread::JoinHandle<()>>,
|
||||||
/// Audio export thread handle
|
/// Audio export thread handle (taken when the mux thread is spawned).
|
||||||
audio_thread: std::thread::JoinHandle<()>,
|
audio_thread: Option<std::thread::JoinHandle<()>>,
|
||||||
/// Temporary video file path
|
/// Temporary video file path
|
||||||
temp_video_path: PathBuf,
|
temp_video_path: PathBuf,
|
||||||
/// Temporary audio file path
|
/// Temporary audio file path
|
||||||
|
|
@ -127,6 +127,9 @@ struct ParallelExportState {
|
||||||
video_progress: Option<ExportProgress>,
|
video_progress: Option<ExportProgress>,
|
||||||
/// Latest audio progress
|
/// Latest audio progress
|
||||||
audio_progress: Option<ExportProgress>,
|
audio_progress: Option<ExportProgress>,
|
||||||
|
/// Result channel for the background mux. `Some` once muxing has started; the
|
||||||
|
/// mux runs off the UI thread so the app stays responsive during finalization.
|
||||||
|
mux_rx: Option<Receiver<Result<(), String>>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ExportOrchestrator {
|
impl ExportOrchestrator {
|
||||||
|
|
@ -220,47 +223,33 @@ impl ExportOrchestrator {
|
||||||
parallel.audio_progress = Some(progress);
|
parallel.audio_progress = Some(progress);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if both are complete
|
// If a background mux is already running, poll it without blocking the UI.
|
||||||
let video_complete = matches!(parallel.video_progress, Some(ExportProgress::Complete { .. }));
|
if parallel.mux_rx.is_some() {
|
||||||
let audio_complete = matches!(parallel.audio_progress, Some(ExportProgress::Complete { .. }));
|
match parallel.mux_rx.as_ref().unwrap().try_recv() {
|
||||||
|
Ok(Ok(())) => {
|
||||||
if video_complete && audio_complete {
|
|
||||||
println!("🎬🎵 [PARALLEL] Both video and audio complete, starting mux");
|
|
||||||
|
|
||||||
// Take parallel state to extract file paths
|
|
||||||
let parallel_state = self.parallel_export.take().unwrap();
|
|
||||||
|
|
||||||
// Wait for threads to finish
|
|
||||||
parallel_state.video_thread.join().ok();
|
|
||||||
parallel_state.audio_thread.join().ok();
|
|
||||||
|
|
||||||
// Start muxing
|
|
||||||
match Self::mux_video_and_audio(
|
|
||||||
¶llel_state.temp_video_path,
|
|
||||||
¶llel_state.temp_audio_path,
|
|
||||||
¶llel_state.final_output_path,
|
|
||||||
) {
|
|
||||||
Ok(()) => {
|
|
||||||
println!("✅ [MUX] Muxing complete, cleaning up temp files");
|
println!("✅ [MUX] Muxing complete, cleaning up temp files");
|
||||||
|
let state = self.parallel_export.take().unwrap();
|
||||||
// Clean up temp files
|
std::fs::remove_file(&state.temp_video_path).ok();
|
||||||
std::fs::remove_file(¶llel_state.temp_video_path).ok();
|
std::fs::remove_file(&state.temp_audio_path).ok();
|
||||||
std::fs::remove_file(¶llel_state.temp_audio_path).ok();
|
return Some(ExportProgress::Complete { output_path: state.final_output_path });
|
||||||
|
|
||||||
return Some(ExportProgress::Complete {
|
|
||||||
output_path: parallel_state.final_output_path,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
Err(err) => {
|
Ok(Err(err)) => {
|
||||||
println!("❌ [MUX] Muxing failed: {}", err);
|
println!("❌ [MUX] Muxing failed: {}", err);
|
||||||
return Some(ExportProgress::Error {
|
self.parallel_export = None;
|
||||||
message: format!("Muxing failed: {}", err),
|
return Some(ExportProgress::Error { message: format!("Muxing failed: {}", err) });
|
||||||
});
|
}
|
||||||
|
Err(std::sync::mpsc::TryRecvError::Empty) => {
|
||||||
|
// Still muxing — keep the UI responsive and show finalizing state.
|
||||||
|
return Some(ExportProgress::Finalizing);
|
||||||
|
}
|
||||||
|
Err(std::sync::mpsc::TryRecvError::Disconnected) => {
|
||||||
|
self.parallel_export = None;
|
||||||
|
return Some(ExportProgress::Error { message: "Mux thread terminated unexpectedly".to_string() });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for errors
|
// Check for errors before completion.
|
||||||
if let Some(ExportProgress::Error { ref message }) = parallel.video_progress {
|
if let Some(ExportProgress::Error { ref message }) = parallel.video_progress {
|
||||||
return Some(ExportProgress::Error { message: format!("Video: {}", message) });
|
return Some(ExportProgress::Error { message: format!("Video: {}", message) });
|
||||||
}
|
}
|
||||||
|
|
@ -268,6 +257,30 @@ impl ExportOrchestrator {
|
||||||
return Some(ExportProgress::Error { message: format!("Audio: {}", message) });
|
return Some(ExportProgress::Error { message: format!("Audio: {}", message) });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Both streams done → spawn the mux on a background thread (the previous
|
||||||
|
// implementation muxed synchronously here on the UI thread, which froze the
|
||||||
|
// app for the whole re-mux pass after progress already hit 100%).
|
||||||
|
let video_complete = matches!(parallel.video_progress, Some(ExportProgress::Complete { .. }));
|
||||||
|
let audio_complete = matches!(parallel.audio_progress, Some(ExportProgress::Complete { .. }));
|
||||||
|
if video_complete && audio_complete {
|
||||||
|
println!("🎬🎵 [PARALLEL] Both video and audio complete, starting background mux");
|
||||||
|
let video_thread = parallel.video_thread.take();
|
||||||
|
let audio_thread = parallel.audio_thread.take();
|
||||||
|
let video_path = parallel.temp_video_path.clone();
|
||||||
|
let audio_path = parallel.temp_audio_path.clone();
|
||||||
|
let output_path = parallel.final_output_path.clone();
|
||||||
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
std::thread::spawn(move || {
|
||||||
|
// The export threads have signalled Complete; join is near-instant.
|
||||||
|
if let Some(t) = video_thread { t.join().ok(); }
|
||||||
|
if let Some(t) = audio_thread { t.join().ok(); }
|
||||||
|
let result = Self::mux_video_and_audio(&video_path, &audio_path, &output_path);
|
||||||
|
tx.send(result).ok();
|
||||||
|
});
|
||||||
|
parallel.mux_rx = Some(rx);
|
||||||
|
return Some(ExportProgress::Finalizing);
|
||||||
|
}
|
||||||
|
|
||||||
// Return combined progress
|
// Return combined progress
|
||||||
match (¶llel.video_progress, ¶llel.audio_progress) {
|
match (¶llel.video_progress, ¶llel.audio_progress) {
|
||||||
(Some(ExportProgress::FrameRendered { frame, total }), _) => {
|
(Some(ExportProgress::FrameRendered { frame, total }), _) => {
|
||||||
|
|
@ -979,13 +992,14 @@ impl ExportOrchestrator {
|
||||||
self.parallel_export = Some(ParallelExportState {
|
self.parallel_export = Some(ParallelExportState {
|
||||||
video_progress_rx,
|
video_progress_rx,
|
||||||
audio_progress_rx,
|
audio_progress_rx,
|
||||||
video_thread,
|
video_thread: Some(video_thread),
|
||||||
audio_thread,
|
audio_thread: Some(audio_thread),
|
||||||
temp_video_path,
|
temp_video_path,
|
||||||
temp_audio_path,
|
temp_audio_path,
|
||||||
final_output_path: output_path,
|
final_output_path: output_path,
|
||||||
video_progress: None,
|
video_progress: None,
|
||||||
audio_progress: None,
|
audio_progress: None,
|
||||||
|
mux_rx: None,
|
||||||
});
|
});
|
||||||
|
|
||||||
println!("🎬🎵 [PARALLEL EXPORT] Both threads spawned, ready for frames");
|
println!("🎬🎵 [PARALLEL EXPORT] Both threads spawned, ready for frames");
|
||||||
|
|
|
||||||
|
|
@ -982,8 +982,18 @@ pub fn render_frame_to_rgba_hdr(
|
||||||
// Set document time to the frame timestamp
|
// Set document time to the frame timestamp
|
||||||
document.current_time = timestamp;
|
document.current_time = timestamp;
|
||||||
|
|
||||||
// Use identity transform for export (document coordinates = pixel coordinates)
|
// Scale the document to the export resolution. The core renderer bakes this
|
||||||
let base_transform = Affine::IDENTITY;
|
// base transform into every layer (vector scenes, raster and video layer
|
||||||
|
// transforms), so the whole stage scales up/down to fill the output. When the
|
||||||
|
// export size matches the document this is the identity.
|
||||||
|
let base_transform = if document.width > 0.0 && document.height > 0.0 {
|
||||||
|
Affine::scale_non_uniform(
|
||||||
|
width as f64 / document.width,
|
||||||
|
height as f64 / document.height,
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
Affine::IDENTITY
|
||||||
|
};
|
||||||
|
|
||||||
// Render document for compositing (returns per-layer scenes)
|
// Render document for compositing (returns per-layer scenes)
|
||||||
let composite_result = render_document_for_compositing(
|
let composite_result = render_document_for_compositing(
|
||||||
|
|
@ -1189,8 +1199,18 @@ pub fn render_frame_to_gpu_rgba(
|
||||||
// Set document time to the frame timestamp
|
// Set document time to the frame timestamp
|
||||||
document.current_time = timestamp;
|
document.current_time = timestamp;
|
||||||
|
|
||||||
// Use identity transform for export (document coordinates = pixel coordinates)
|
// Scale the document to the export resolution. The core renderer bakes this
|
||||||
let base_transform = Affine::IDENTITY;
|
// base transform into every layer (vector scenes, raster and video layer
|
||||||
|
// transforms), so the whole stage scales up/down to fill the output. When the
|
||||||
|
// export size matches the document this is the identity.
|
||||||
|
let base_transform = if document.width > 0.0 && document.height > 0.0 {
|
||||||
|
Affine::scale_non_uniform(
|
||||||
|
width as f64 / document.width,
|
||||||
|
height as f64 / document.height,
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
Affine::IDENTITY
|
||||||
|
};
|
||||||
|
|
||||||
// Render document for compositing (returns per-layer scenes)
|
// Render document for compositing (returns per-layer scenes)
|
||||||
let composite_result = render_document_for_compositing(
|
let composite_result = render_document_for_compositing(
|
||||||
|
|
|
||||||
|
|
@ -5616,11 +5616,8 @@ impl eframe::App for EditorApp {
|
||||||
// Close the progress dialog after a brief delay
|
// Close the progress dialog after a brief delay
|
||||||
self.export_progress_dialog.close();
|
self.export_progress_dialog.close();
|
||||||
|
|
||||||
// Send desktop notification
|
// Send desktop notification (fire-and-forget; never blocks the UI)
|
||||||
if let Err(e) = notifications::notify_export_complete(output_path) {
|
notifications::notify_export_complete(output_path);
|
||||||
// Log but don't fail - notifications are non-critical
|
|
||||||
eprintln!("⚠️ Could not send desktop notification: {}", e);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
lightningbeam_core::export::ExportProgress::Error { ref message } => {
|
lightningbeam_core::export::ExportProgress::Error { ref message } => {
|
||||||
eprintln!("❌ Export error: {}", message);
|
eprintln!("❌ Export error: {}", message);
|
||||||
|
|
@ -5630,11 +5627,8 @@ impl eframe::App for EditorApp {
|
||||||
);
|
);
|
||||||
// Keep the dialog open to show the error
|
// Keep the dialog open to show the error
|
||||||
|
|
||||||
// Send desktop notification for error
|
// Send desktop notification for error (fire-and-forget)
|
||||||
if let Err(e) = notifications::notify_export_error(message) {
|
notifications::notify_export_error(message);
|
||||||
// Log but don't fail - notifications are non-critical
|
|
||||||
eprintln!("⚠️ Could not send desktop notification: {}", e);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -3,52 +3,52 @@
|
||||||
use notify_rust::Notification;
|
use notify_rust::Notification;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
|
|
||||||
/// Send a desktop notification for successful export
|
// `notify_rust`'s `Notification::show()` is a SYNCHRONOUS D-Bus call. When no
|
||||||
///
|
// notification daemon is running it can block for the service-activation timeout
|
||||||
/// # Arguments
|
// (~25s) before failing, so these are fire-and-forget on a detached thread — the
|
||||||
/// * `output_path` - Path to the exported file
|
// UI must never wait on them (e.g. they're triggered from the export-complete
|
||||||
///
|
// handler on the UI thread).
|
||||||
/// # Returns
|
|
||||||
/// Ok(()) if notification sent successfully, Err with log message if failed
|
/// Show a desktop notification for a successful export (fire-and-forget).
|
||||||
pub fn notify_export_complete(output_path: &Path) -> Result<(), String> {
|
pub fn notify_export_complete(output_path: &Path) {
|
||||||
let filename = output_path
|
let filename = output_path
|
||||||
.file_name()
|
.file_name()
|
||||||
.and_then(|n| n.to_str())
|
.and_then(|n| n.to_str())
|
||||||
.unwrap_or("unknown");
|
.unwrap_or("unknown")
|
||||||
|
.to_string();
|
||||||
|
|
||||||
Notification::new()
|
std::thread::spawn(move || {
|
||||||
.summary("Export Complete")
|
if let Err(e) = Notification::new()
|
||||||
.body(&format!("Successfully exported: {}", filename))
|
.summary("Export Complete")
|
||||||
.icon("dialog-information") // Standard icon name (freedesktop.org)
|
.body(&format!("Successfully exported: {}", filename))
|
||||||
.timeout(5000) // 5 seconds
|
.icon("dialog-information") // Standard icon name (freedesktop.org)
|
||||||
.show()
|
.timeout(5000) // 5 seconds
|
||||||
.map_err(|e| format!("Failed to show notification: {}", e))?;
|
.show()
|
||||||
|
{
|
||||||
Ok(())
|
eprintln!("⚠️ Could not send desktop notification: {}", e);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Send a desktop notification for export error
|
/// Show a desktop notification for an export error (fire-and-forget).
|
||||||
///
|
pub fn notify_export_error(error_message: &str) {
|
||||||
/// # Arguments
|
// Truncate very long error messages (on a char boundary).
|
||||||
/// * `error_message` - The error message to display
|
let truncated = if error_message.chars().count() > 100 {
|
||||||
///
|
let prefix: String = error_message.chars().take(97).collect();
|
||||||
/// # Returns
|
format!("{}...", prefix)
|
||||||
/// Ok(()) if notification sent successfully, Err with log message if failed
|
|
||||||
pub fn notify_export_error(error_message: &str) -> Result<(), String> {
|
|
||||||
// Truncate very long error messages
|
|
||||||
let truncated = if error_message.len() > 100 {
|
|
||||||
format!("{}...", &error_message[..97])
|
|
||||||
} else {
|
} else {
|
||||||
error_message.to_string()
|
error_message.to_string()
|
||||||
};
|
};
|
||||||
|
|
||||||
Notification::new()
|
std::thread::spawn(move || {
|
||||||
.summary("Export Failed")
|
if let Err(e) = Notification::new()
|
||||||
.body(&truncated)
|
.summary("Export Failed")
|
||||||
.icon("dialog-error") // Standard error icon
|
.body(&truncated)
|
||||||
.timeout(10000) // 10 seconds for errors (longer to read)
|
.icon("dialog-error") // Standard error icon
|
||||||
.show()
|
.timeout(10000) // 10 seconds for errors (longer to read)
|
||||||
.map_err(|e| format!("Failed to show notification: {}", e))?;
|
.show()
|
||||||
|
{
|
||||||
Ok(())
|
eprintln!("⚠️ Could not send desktop notification: {}", e);
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue