Merge pull request #18 from filleduchaos/ft/macos-stride-fix

Prevent frame data corruption on certain Mac configurations

apps/desktop/src-tauri/src/capture/src/common/dxgi.rs

@@ -38,6 +38,13 @@ impl Capturer {
 
 pub struct Frame<'a>(&'a [u8]);
 
+impl Frame <'_> {
+    pub fn stride_override(&self) -> Option<usize> {
+        // No need for an override on DirectX Graphics
+        None
+    }
+}
+
 impl<'a> ops::Deref for Frame<'a> {
     type Target = [u8];
     fn deref(&self) -> &[u8] {

apps/desktop/src-tauri/src/capture/src/common/quartz.rs

@@ -53,6 +53,24 @@ pub struct Frame<'a>(
     PhantomData<&'a [u8]>
 );
 
+impl Frame <'_> {
+    pub fn stride_override(&self) -> Option<usize> {
+        // On Macs, CoreGraphics strives to ensure that pixel buffers (such as this framedata) are
+        // aligned to squeeze the best performance out of the underlying hardware; in other words,
+        // each row/scanline has to be cleanly divisible by a hardware-specific byte length so that
+        // the buffer can be read in chunks without running into overlapping rows in a single chunk.
+        // This behaviour is only referred to fairly obliquely in documentation - for instance on
+        // [this page](https://developer.apple.com/library/archive/qa/qa1829/_index.html).
+        //
+        // This means that certain Mac configurations can end up with pixel buffers that contain
+        // more bytes per row than would be expected from just the row width and the image format.
+        // Thankfully, the Core Graphics API exposes methods for obtaining what the stride in use
+        // actually is, so we can retrieve and use it here.
+
+        Some(unsafe { self.0.bytes_per_row() })
+    }
+}
+
 impl<'a> ops::Deref for Frame<'a> {
     type Target = [u8];
     fn deref(&self) -> &[u8] {

apps/desktop/src-tauri/src/capture/src/common/x11.rs

@@ -26,6 +26,13 @@ impl Capturer {
 
 pub struct Frame<'a>(&'a [u8]);
 
+impl Frame <'_> {
+    pub fn stride_override(&self) -> Option<usize> {
+        // No need to for an override on X11
+        None
+    }
+}
+
 impl<'a> ops::Deref for Frame<'a> {
     type Target = [u8];
     fn deref(&self) -> &[u8] {

apps/desktop/src-tauri/src/capture/src/quartz/ffi.rs

@@ -203,8 +203,7 @@ extern {
         options: u32,
         seed: *mut u32
     ) -> i32;
-    pub fn IOSurfaceGetBaseAddressOfPlane(buffer: IOSurfaceRef, index: usize) -> *mut c_void;
-    pub fn IOSurfaceGetBytesPerRowOfPlane(buffer: IOSurfaceRef, index: usize) -> usize;
+    pub fn IOSurfaceGetBytesPerRow(buffer: IOSurfaceRef) -> usize;
 
     // Dispatch
 

apps/desktop/src-tauri/src/capture/src/quartz/frame.rs

@@ -24,6 +24,10 @@ impl Frame {
 
         Frame { surface, inner }
     }
+
+    pub unsafe fn bytes_per_row(&self) -> usize {
+        IOSurfaceGetBytesPerRow(self.surface)
+    }
 }
 
 impl ops::Deref for Frame {

apps/desktop/src-tauri/src/media.rs

@@ -61,17 +61,16 @@ impl MediaRecorder {
 
         let host = cpal::default_host();
         let devices = host.devices().expect("Failed to get devices");
-        let display = Display::primary().expect("Failed to find primary display");
-        let mut w = max_screen_width;
-        let mut h = max_screen_height;
+        let _display = Display::primary().expect("Failed to find primary display");
+        let w = max_screen_width;
+        let h = max_screen_height;
 
         let adjusted_width = w & !2;
         let adjusted_height = h & !2;
         let capture_size = adjusted_width * adjusted_height * 4;
         let (audio_tx, audio_rx) = tokio::sync::mpsc::channel::<Vec<u8>>(2048);
         let (video_tx, video_rx) = tokio::sync::mpsc::channel::<Vec<u8>>(2048);
-        let bytes_per_pixel = display.bits_per_pixel() / 8;
-        let calculated_stride = (bytes_per_pixel * adjusted_width) as usize;
+        let calculated_stride = (adjusted_width * 4) as usize;
 
         println!("Display width: {}", w);
         println!("Display height: {}", h);
@@ -297,78 +296,68 @@ impl MediaRecorder {
             let mut screenshot_captured: bool = false;
 
             while !should_stop.load(Ordering::SeqCst) {
-              let options_clone = options.clone();
-              let now = Instant::now();
+                let options_clone = options.clone();
+                let now = Instant::now();
 
                 if now >= time_next {
+                    match capturer.frame() {
+                        Ok(frame) => {
+                            let mut frame_data = Vec::with_capacity(capture_size.try_into().unwrap());
 
-                    if !is_local_mode {
-                        if now - start_time >= capture_frame_at && !screenshot_captured {
-                            if let Ok(frame) = capturer.frame() {
+                            for row in 0..adjusted_height {
+                                let padded_stride = frame.stride_override().unwrap_or(calculated_stride);
+                                assert!(padded_stride >= calculated_stride, "Image stride with padding should not be smaller than calculated bytes per row");
+                                // Each row should skip the padding of the previous row
+                                let start = row * padded_stride;
+                                // Each row should stop before/trim off its padding, for compatibility with software that doesn't follow arbitrary padding.
+                                let end = start + calculated_stride;
+                                frame_data.extend_from_slice(&frame[start..end]);
+                            }
+
+                            if now - start_time >= capture_frame_at && !screenshot_captured {
                                 screenshot_captured = true;
-                                let screenshot_file_path_owned_cloned = screenshot_file_path_owned.clone(); 
-                                let w_cloned = adjusted_width.clone();
-                                let h_cloned = adjusted_height.clone();
-
-                                let frame_clone = frame.to_vec();
+                                let screenshot_file_path_owned_cloned = screenshot_file_path_owned.clone();
+                                let mut frame_data_clone = frame_data.clone();
+
                                 std::thread::spawn(move || {
-                                    let mut frame_data = Vec::with_capacity(capture_size.try_into().unwrap());
-                                    println!("Frame length: {}", frame_clone.len());
-                                    let rt = tokio::runtime::Runtime::new().unwrap();
-
-                                    for row in 0..adjusted_height { 
-                                        let start: usize = row as usize * calculated_stride as usize;
-                                        let end: usize = start + calculated_stride as usize;
-                                        let mut row_data = frame_clone[start..end].to_vec();
-                                        for chunk in row_data.chunks_mut(4) {
-                                            chunk.swap(0, 2);
-                                        }
-                                        frame_data.extend_from_slice(&row_data);
+                                    for chunk in frame_data_clone.chunks_mut(4) {
+                                        chunk.swap(0, 2);
                                     }
-    
+
                                     let path = Path::new(&screenshot_file_path_owned_cloned);
                                     let image: ImageBuffer<Rgba<u8>, Vec<u8>> = ImageBuffer::from_raw(
-                                        w_cloned.try_into().unwrap(),
-                                        h_cloned.try_into().unwrap(),
-                                        frame_data
+                                        adjusted_width.try_into().unwrap(),
+                                        adjusted_height.try_into().unwrap(),
+                                        frame_data_clone
                                     ).expect("Failed to create image buffer");
-    
+
                                     let mut output_file = std::fs::File::create(&path).expect("Failed to create output file");
                                     let mut encoder = JpegEncoder::new_with_quality(&mut output_file, 20);
-    
+
                                     if let Err(e) = encoder.encode_image(&image) {
                                         eprintln!("Failed to save screenshot: {}", e);
                                     } else {
-                                        let screenshot_file_path_owned_cloned_copy = screenshot_file_path_owned_cloned.clone();
-                                        rt.block_on(async {
-                                            let upload_task = tokio::spawn(upload_file(Some(options_clone), screenshot_file_path_owned_cloned_copy.clone(), "screenshot".to_string()));
-                                            match upload_task.await {
-                                                Ok(result) => {
-                                                    match result {
-                                                        Ok(_) => println!("Screenshot captured and saved to {:?}", path),
-                                                        Err(e) => eprintln!("Failed to upload file: {}", e),
-                                                    }
-                                                },
-                                                Err(e) => eprintln!("Failed to join task: {}", e),
-                                            }
-                                        });
+                                        if !is_local_mode {
+                                            let rt = tokio::runtime::Runtime::new().unwrap();
+                                            let screenshot_file_path_owned_cloned_copy = screenshot_file_path_owned_cloned.clone();
+                                            rt.block_on(async {
+                                                let upload_task = tokio::spawn(upload_file(Some(options_clone), screenshot_file_path_owned_cloned_copy.clone(), "screenshot".to_string()));
+                                                match upload_task.await {
+                                                    Ok(result) => {
+                                                        match result {
+                                                            Ok(_) => println!("Screenshot captured and saved to {:?}", path),
+                                                            Err(e) => eprintln!("Failed to upload file: {}", e),
+                                                        }
+                                                    },
+                                                    Err(e) => eprintln!("Failed to join task: {}", e),
+                                                }
+                                            });
+                                        }
                                         println!("Screenshot captured and saved to {:?}", path);
                                     }
                                 });
                             }
-                        }
-                    }
-
-
-                    let mut frame_data = Vec::with_capacity(capture_size.try_into().unwrap());
 
-                    match capturer.frame() {
-                        Ok(frame) => {
-                            for row in 0..adjusted_height {
-                                let start: usize = row as usize * calculated_stride as usize;
-                                let end: usize = start + calculated_stride as usize;
-                                frame_data.extend_from_slice(&frame[start..end]);
-                            }
                             if let Some(sender) = &video_channel_sender {
                                 if sender.try_send(frame_data).is_err() {
                                     eprintln!("Channel send error. Dropping data.");