Some refactoring for linux/display.cpp

sunshine/platform/linux/display.cpp

@@ -37,12 +37,15 @@ void freeX(XFixesCursorImage*);
 using ifaddr_t = util::safe_ptr<ifaddrs, freeifaddrs>;
 using xcb_connect_t = util::safe_ptr<xcb_connection_t, xcb_disconnect>;
 using xcb_img_t = util::c_ptr<xcb_shm_get_image_reply_t>;
-using xcb_cursor_img = util::c_ptr<xcb_xfixes_get_cursor_image_reply_t>;
 
 using xdisplay_t = util::safe_ptr_v2<Display, int, XCloseDisplay>;
 using ximg_t = util::safe_ptr<XImage, freeImage>;
 using xcursor_t = util::safe_ptr<XFixesCursorImage, freeX>;
 
+using crtc_info_t = util::safe_ptr<_XRRCrtcInfo, XRRFreeCrtcInfo>;
+using output_info_t = util::safe_ptr<_XRROutputInfo, XRRFreeOutputInfo>;
+using screen_res_t = util::safe_ptr<_XRRScreenResources, XRRFreeScreenResources>;
+
 class shm_id_t {
 public:
   shm_id_t() : id { -1 } {}
@@ -62,24 +65,16 @@ public:
 
 class shm_data_t {
 public:
-  shm_data_t() : data { (void*) -1 }
+  shm_data_t() : data { (void*) -1 } {}
-  {
+  shm_data_t(void *data) : data { data } {}
-  }
-  shm_data_t(void *data) : data { data }
-  {
-  }
 
-  shm_data_t(shm_data_t &&other) noexcept : data(other.data)
+  shm_data_t(shm_data_t &&other) noexcept : data(other.data) {
-  {
     other.data = (void*)-1;
   }
 
-  ~shm_data_t()
+  ~shm_data_t() {
-  {
+    if((std::uintptr_t) data != -1) {
-    if ((std::uintptr_t) data != -1)
-    {
       shmdt(data);
-      data = (void*) -1;
     }
   }
 
@@ -91,8 +86,7 @@ struct x11_img_t: public img_t {
 };
 
 struct shm_img_t: public img_t {
-  ~shm_img_t() override
+  ~shm_img_t() override {
-  {
     delete[] data;
     data = nullptr;
   }
@@ -102,8 +96,7 @@ void blend_cursor(Display *display, img_t &img, int offsetX, int offsetY) {
   xcursor_t overlay { XFixesGetCursorImage(display) };
 
   if (!overlay) {
-    BOOST_LOG(error)
+    BOOST_LOG(error) << "Couldn't get cursor from XFixesGetCursorImage"sv;
-    << "Couldn't get cursor from XFixesGetCursorImage"sv;
     return;
   }
 
@@ -154,10 +147,9 @@ struct x11_attr_t: public display_t
   Window xwindow;
   XWindowAttributes xattr;
 
-  Display* displayDisplay;
-
   /*
-   * Remember last X (NOT the streamed monitor!) size. This way we can trigger reinitialization if the dimensions changed while streaming
+   * Last X (NOT the streamed monitor!) size.
+   * This way we can trigger reinitialization if the dimensions changed while streaming
    */
   int lastWidth, lastHeight;
 
@@ -166,13 +158,14 @@ struct x11_attr_t: public display_t
    */
   int displayOffsetX, displayOffsetY;
 
-  x11_attr_t() : xdisplay { displayDisplay = XOpenDisplay(nullptr) }, xwindow { }, xattr { }
+  x11_attr_t() : xdisplay { XOpenDisplay(nullptr) }, xwindow { }, xattr { }, displayOffsetX { 0 }, displayOffsetY { 0 } {
-  {
     XInitThreads();
+  }
+
+  int init() {
     if(!xdisplay) {
-      BOOST_LOG(fatal) << "Could not open x11 display"sv;
+      BOOST_LOG(error) << "Could not open X11 display"sv;
-      log_flush();
+      return -1;
-      std::abort();
     }
 
     xwindow = DefaultRootWindow(xdisplay.get());
@@ -184,38 +177,30 @@ struct x11_attr_t: public display_t
       streamedMonitor = (int)util::from_view(config::video.output_name);
     }
 
-    //If the value has been set at all
     if(streamedMonitor != -1) {
-
+      BOOST_LOG(info) << "Configuring selected monitor ("sv << streamedMonitor << ") to stream"sv;
-      BOOST_LOG(info) << "Configuring selected monitor ("<< streamedMonitor<<") to stream. If it fails here, you may need Xrandr >= 1.5"sv;
+      screen_res_t screenr { XRRGetScreenResources(xdisplay.get(), xwindow) };
-      XRRScreenResources *screenr = XRRGetScreenResources(displayDisplay, xwindow);
-      // This is the key right here. Use XRRScreenResources::noutput
       int output = screenr->noutput;
 
       if(streamedMonitor >= output) {
-        BOOST_LOG(error)<< "Could not stream selected display number because there aren't so many."sv;
+        BOOST_LOG(error) << "Could not stream display number ["sv << streamedMonitor << "], there are only ["sv << output << "] displays."sv;
+        return -1;
       }
-      else {
+
-        XRROutputInfo* out_info = XRRGetOutputInfo(displayDisplay, screenr, screenr->outputs[streamedMonitor]);
+      output_info_t out_info { XRRGetOutputInfo(xdisplay.get(), screenr.get(), screenr->outputs[streamedMonitor]) };
-        if (NULL == out_info || out_info->connection != RR_Connected)
+      if(!out_info || out_info->connection != RR_Connected) {
-        {
         BOOST_LOG(error) << "Could not stream selected display because it doesn't seem to be connected"sv;
+        return -1;
       }
-        else
+
-        {
+      crtc_info_t crt_info { XRRGetCrtcInfo(xdisplay.get(), screenr.get(), out_info->crtc) };
-          XRRCrtcInfo* crt_info = XRRGetCrtcInfo(displayDisplay, screenr, out_info->crtc);
+      BOOST_LOG(info)
-          BOOST_LOG(info)<<"Streaming display: "<<out_info->name<<" with res "<<crt_info->width<<" x "<<crt_info->height<<+" offset by "<<crt_info->x<<" x "<<crt_info->y<<"."sv;
+        << "Streaming display: "sv << out_info->name << " with res "sv << crt_info->width << 'x' << crt_info->height << " offset by "sv << crt_info->x << 'x' << crt_info->y;
 
       width = crt_info->width;
       height = crt_info->height;
       displayOffsetX = crt_info->x;
       displayOffsetY = crt_info->y;
-
-          XRRFreeCrtcInfo(crt_info);
-        }
-        XRRFreeOutputInfo(out_info);
-      }
-      XRRFreeScreenResources(screenr);
     }
     else {
       width = xattr.width;
@@ -224,13 +209,14 @@ struct x11_attr_t: public display_t
 
     lastWidth = xattr.width;
     lastHeight = xattr.height;
+
+    return 0;
   }
 
   /**
    * Called when the display attributes should change.
    */
-  void refresh()
+  void refresh() {
-  {
     XGetWindowAttributes(xdisplay.get(), xwindow, &xattr); //Update xattr's
   }
 
@@ -337,17 +323,18 @@ struct shm_attr_t: public x11_attr_t {
   }
 
   int init() {
+    if(x11_attr_t::init()) {
+      return 1;
+    }
+
     shm_xdisplay.reset(XOpenDisplay(nullptr));
     xcb.reset(xcb_connect(nullptr, nullptr));
-    if (xcb_connection_has_error(xcb.get()))
+    if(xcb_connection_has_error(xcb.get())) {
-    {
       return -1;
     }
 
-    if (!xcb_get_extension_data(xcb.get(), &xcb_shm_id)->present)
+    if(!xcb_get_extension_data(xcb.get(), &xcb_shm_id)->present) {
-    {
+      BOOST_LOG(error) << "Missing SHM extension"sv;
-      BOOST_LOG(error)
-      << "Missing SHM extension"sv;
 
       return -1;
     }
@@ -357,30 +344,20 @@ struct shm_attr_t: public x11_attr_t {
     seg = xcb_generate_id(xcb.get());
 
     shm_id.id = shmget(IPC_PRIVATE, frame_size(), IPC_CREAT | 0777);
-    if (shm_id.id == -1)
+    if(shm_id.id == -1) {
-    {
+      BOOST_LOG(error) << "shmget failed"sv;
-      BOOST_LOG(error)
-      << "shmget failed"sv;
       return -1;
     }
 
     xcb_shm_attach(xcb.get(), seg, shm_id.id, false);
     data.data = shmat(shm_id.id, nullptr, 0);
 
-    if ((uintptr_t) data.data == -1)
+    if((uintptr_t)data.data == -1) {
-    {
+      BOOST_LOG(error) << "shmat failed"sv;
-      BOOST_LOG(error)
-      << "shmat failed"sv;
 
       return -1;
     }
 
-    /*
-     * Commented out resetting of the sizes when intializing X in SHM mode. It might be wrong. Expect issues. This is the default mode, so poisoning those variables is not desired
-     */
-//    width = display->width_in_pixels;
-//    height = display->height_in_pixels;
-
     return 0;
   }
 
@@ -401,10 +378,8 @@ struct mic_attr_t: public mic_t {
 
     auto buf = sample_buf.data();
     int status;
-    if (pa_simple_read(mic.get(), buf, sample_size * 2, &status))
+    if(pa_simple_read(mic.get(), buf, sample_size *2, &status)) {
-    {
+      BOOST_LOG(error) << "pa_simple_read() failed: "sv << pa_strerror(status);
-      BOOST_LOG(error)
-      << "pa_simple_read() failed: "sv << pa_strerror(status);
 
       return capture_e::error;
     }
@@ -413,31 +388,34 @@ struct mic_attr_t: public mic_t {
   }
 };
 
-std::shared_ptr<display_t> shm_display() {
-  auto shm = std::make_shared<shm_attr_t>();
-
-  if (shm->init()) {
-    return nullptr;
-  }
-
-  return shm;
-}
-
 std::shared_ptr<display_t> display(platf::dev_type_e hwdevice_type) {
   if(hwdevice_type != platf::dev_type_e::none) {
     BOOST_LOG(error)<< "Could not initialize display with the given hw device type."sv;
     return nullptr;
   }
 
-  auto shm_disp = shm_display(); //Attempt to use shared memory X11 to avoid copying the frame
+  // Attempt to use shared memory X11 to avoid copying the frame
+  auto shm_disp = std::make_shared<shm_attr_t>();
 
-  if (!shm_disp) {
+  auto status = shm_disp->init();
-    return std::make_shared<x11_attr_t>(); //Fallback to standard way if else
+  if(status > 0) {
+    // x11_attr_t::init() failed, don't bother trying again.
+    return nullptr;
   }
 
+  if(status == 0) {
     return shm_disp;
   }
 
+  // Fallback
+  auto x11_disp = std::make_shared<x11_attr_t>();
+  if(x11_disp->init()) {
+    return nullptr;
+  }
+
+  return x11_disp;
+}
+
 std::unique_ptr<mic_t> microphone(std::uint32_t sample_rate) {
   auto mic = std::make_unique<mic_attr_t>(PA_SAMPLE_S16LE, sample_rate, 2);
 
@@ -506,8 +484,7 @@ std::pair<std::uint16_t, std::string> from_sockaddr_ex(const sockaddr *const ip_
     port = ((sockaddr_in*) ip_addr)->sin_port;
   }
 
-  return
+  return { port, std::string {data} };
-  { port, std::string {data}};
 }
 
 std::string get_mac_address(const std::string_view &address) {
@@ -522,8 +499,8 @@ std::string get_mac_address(const std::string_view &address) {
       }
     }
   }
-  BOOST_LOG(warning)
+
-  << "Unable to find MAC address for "sv << address;
+  BOOST_LOG(warning) << "Unable to find MAC address for "sv << address;
   return "00:00:00:00:00:00"s;
 }