Warn on MIT-SHM not working on Linux X11 (rustdesk#6856)

* Clarify video capture method * fix improper level of pointer usage of xcb_generic_error_t * add ffi of xcb_shm_query_version * throw a warn about MIT-SHM not working * add missing #[cfg] * checks SHM validity on the fly, rather than cache on creation --------- Co-authored-by: root <root@localhost> Co-authored-by: rustdesk-fork <[email protected]>
MrVinyla · Jan 31, 2024 · c97cc15 · c97cc15
1 parent 750f1a1
commit c97cc15
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 12 deletions.
diff --git a/libs/scrap/src/common/x11.rs b/libs/scrap/src/common/x11.rs
@@ -41,7 +41,7 @@ pub struct PixelBuffer<'a> {
 }
 
 impl<'a> PixelBuffer<'a> {
-    pub fn new(data: &'a [u8], pixfmt: Pixfmt, width:usize, height: usize) -> Self {
+    pub fn new(data: &'a [u8], pixfmt: Pixfmt, width: usize, height: usize) -> Self {
         let stride0 = data.len() / height;
         let mut stride = Vec::new();
         stride.push(stride0);
@@ -131,4 +131,8 @@ impl Display {
     pub fn name(&self) -> String {
         self.0.name()
     }
+
+    pub fn get_shm_status(&self) -> Result<(), x11::Error> {
+        self.0.server().get_shm_status()
+    }
 }
diff --git a/libs/scrap/src/x11/ffi.rs b/libs/scrap/src/x11/ffi.rs
@@ -80,6 +80,14 @@ extern "C" {
     pub fn xcb_get_atom_name_name(reply: *const xcb_get_atom_name_request_t) -> *const u8;
 
     pub fn xcb_get_atom_name_name_length(reply: *const xcb_get_atom_name_reply_t) -> i32;
+
+    pub fn xcb_shm_query_version(c: *mut xcb_connection_t) -> xcb_shm_query_version_cookie_t;
+
+    pub fn xcb_shm_query_version_reply(
+        c: *mut xcb_connection_t,
+        cookie: xcb_shm_query_version_cookie_t,
+        e: *mut *mut xcb_generic_error_t,
+    ) -> *const xcb_shm_query_version_reply_t;
 }
 
 pub const XCB_IMAGE_FORMAT_Z_PIXMAP: u8 = 2;
@@ -221,3 +229,22 @@ pub struct xcb_randr_get_monitors_reply_t {
     pub n_outputs: u32,
     pub pad1: [u8; 12],
 }
+
+#[repr(C)]
+pub struct xcb_shm_query_version_cookie_t {
+    pub sequence: u32,
+}
+
+#[repr(C)]
+pub struct xcb_shm_query_version_reply_t {
+    pub response_type: u8,
+    pub shared_pixmaps: u8,
+    pub sequence: u16,
+    pub length: u32,
+    pub major_version: u16,
+    pub minor_version: u16,
+    pub uid: u16,
+    pub gid: u16,
+    pub pixmap_format: u8,
+    pub pad0: [u8; 15],
+}
diff --git a/libs/scrap/src/x11/iter.rs b/libs/scrap/src/x11/iter.rs
@@ -101,11 +101,11 @@ fn get_atom_name(conn: *mut xcb_connection_t, atom: xcb_atom_t) -> String {
         return empty;
     }
     unsafe {
-        let mut e: xcb_generic_error_t = std::mem::zeroed();
+        let mut e: *mut xcb_generic_error_t = std::ptr::null_mut();
         let reply = xcb_get_atom_name_reply(
             conn,
             xcb_get_atom_name(conn, atom),
-            &mut ((&mut e) as *mut xcb_generic_error_t) as _,
+            &mut e as _,
         );
         if reply == std::ptr::null() {
             return empty;

diff --git a/libs/scrap/src/x11/server.rs b/libs/scrap/src/x11/server.rs
@@ -87,6 +87,24 @@ impl Server {
     pub fn setup(&self) -> *const xcb_setup_t {
         self.setup
     }
+    pub fn get_shm_status(&self) -> Result<(), Error> {
+        unsafe { check_x11_shm_available(self.raw) }
+    }
+}
+
+unsafe fn check_x11_shm_available(c: *mut xcb_connection_t) -> Result<(), Error> {
+    let cookie = xcb_shm_query_version(c);
+    let mut e: *mut xcb_generic_error_t = std::ptr::null_mut();
+    let reply = xcb_shm_query_version_reply(c, cookie, &mut e as _);
+    if reply.is_null() {
+        // TODO: Should seperate SHM disabled from SHM not supported?
+        return Err(Error::UnsupportedExtension);
+    } else if e.is_null() {
+        return Ok(());
+    } else {
+        // TODO: Does "This request does never generate any errors" in manual means `e` is never set, so we would never reach here?
+        return Err(Error::Generic);
+    }
 }
 
 impl Drop for Server {

diff --git a/src/server/video_service.rs b/src/server/video_service.rs
@@ -193,17 +193,22 @@ fn create_capturer(
     match c {
         Some(c1) => return Ok(c1),
         None => {
-            log::debug!("Create capturer dxgi|gdi");
             #[cfg(windows)]
-            return crate::portable_service::client::create_capturer(
-                _current,
-                display,
-                _portable_service_running,
-            );
+            {
+                log::debug!("Create capturer dxgi|gdi");
+                return crate::portable_service::client::create_capturer(
+                    _current,
+                    display,
+                    _portable_service_running,
+                );
+            }
             #[cfg(not(windows))]
-            return Ok(Box::new(
-                Capturer::new(display).with_context(|| "Failed to create capturer")?,
-            ));
+            {
+                log::debug!("Create capturer from scrap");
+                return Ok(Box::new(
+                    Capturer::new(display).with_context(|| "Failed to create capturer")?,
+                ));
+            }
         }
     };
 }
@@ -304,6 +309,17 @@ fn get_capturer(current: usize, portable_service_running: bool) -> ResultType<Ca
         );
     }
     let display = displays.remove(current);
+
+    #[cfg(target_os = "linux")]
+    if let Display::X11(inner) = &display {
+        if let Err(err) = inner.get_shm_status() {
+            log::warn!(
+                "MIT-SHM extension not working properly on select X11 server: {:?}",
+                err
+            );
+        }
+    }
+
     let (origin, width, height) = (display.origin(), display.width(), display.height());
     let name = display.name();
     log::debug!(