[RFC,spice-streaminh-agent,2/3] mjpeg-fallback: Use Xlib capture helper

Submitted by Snir Sheriber on Aug. 26, 2019, 8:39 a.m.

Details

Message ID 20190826083948.29309-2-ssheribe@redhat.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Snir Sheriber Aug. 26, 2019, 8:39 a.m.
---
 src/Makefile.am        |  4 ++++
 src/mjpeg-fallback.cpp | 38 +++++++++++++-------------------------
 2 files changed, 17 insertions(+), 25 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/Makefile.am b/src/Makefile.am
index 0133bf5..31b8af1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -21,6 +21,7 @@  AM_CPPFLAGS = \
 	$(X11_CFLAGS) \
 	$(XFIXES_CFLAGS) \
 	$(XRANDR_CFLAGS) \
+	$(XEXT_CFLAGS) \
 	$(NULL)
 
 AM_CFLAGS = \
@@ -56,6 +57,7 @@  spice_streaming_agent_LDADD = \
 	$(X11_LIBS) \
 	$(XFIXES_LIBS) \
 	$(XRANDR_LIBS) \
+	$(XEXT_LIBS) \
 	$(JPEG_LIBS) \
 	$(NULL)
 
@@ -77,6 +79,8 @@  spice_streaming_agent_SOURCES = \
 	utils.cpp \
 	utils.hpp \
 	x11-display-info.cpp \
+	xlib-capture.cpp \
+	xlib-capture.hpp \
 	$(NULL)
 
 if HAVE_GST
diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
index 03247a1..a43825b 100644
--- a/src/mjpeg-fallback.cpp
+++ b/src/mjpeg-fallback.cpp
@@ -16,6 +16,7 @@ 
 #include <sstream>
 #include <memory>
 #include <syslog.h>
+#include "xlib-capture.hpp"
 
 using namespace spice::streaming_agent;
 
@@ -44,11 +45,10 @@  public:
 private:
     MjpegSettings settings;
     Display *const dpy;
+    XlibCapture *xc;
 
     std::vector<uint8_t> frame;
 
-    // last frame sizes
-    int last_width = -1, last_height = -1;
     // last time before capture
     uint64_t last_time = 0;
 };
@@ -58,19 +58,21 @@  private:
 MjpegFrameCapture::MjpegFrameCapture(const MjpegSettings& settings):
     settings(settings),dpy(XOpenDisplay(nullptr))
 {
-    if (!dpy)
+    if (!dpy) {
         throw std::runtime_error("Unable to initialize X11");
+    }
+    xc = new XlibCapture(dpy);
 }
 
 MjpegFrameCapture::~MjpegFrameCapture()
 {
+    delete xc;
     XCloseDisplay(dpy);
 }
 
 void MjpegFrameCapture::Reset()
 {
     frame.clear();
-    last_width = last_height = -1;
 }
 
 FrameInfo MjpegFrameCapture::CaptureFrame()
@@ -100,34 +102,20 @@  FrameInfo MjpegFrameCapture::CaptureFrame()
         }
     }
 
-    int screen = XDefaultScreen(dpy);
-
-    Window win = RootWindow(dpy, screen);
+    XImg *image = xc->capture();
 
-    XWindowAttributes win_info;
-    XGetWindowAttributes(dpy, win, &win_info);
+    bool is_first = image->new_resolution();
 
-    bool is_first = false;
-    if (win_info.width != last_width || win_info.height != last_height) {
-        last_width = win_info.width;
-        last_height = win_info.height;
-        is_first = true;
-    }
+    info.size.width = image->width();
+    info.size.height = image->height();
 
-    info.size.width = win_info.width;
-    info.size.height = win_info.height;
-
-    int format = ZPixmap;
-    // TODO handle errors
-    XImage *image = XGetImage(dpy, win, win_info.x, win_info.y,
-                              win_info.width, win_info.height, AllPlanes, format);
 
     // TODO handle errors
     // TODO multiple formats (only 32 bit)
-    write_JPEG_file(frame, settings.quality, (uint8_t*) image->data,
-                    image->width, image->height);
+    write_JPEG_file(frame, settings.quality, (uint8_t*) image->get_data(),
+                    image->width(), image->height());
 
-    image->f.destroy_image(image);
+    delete image;
 
     info.buffer = &frame[0];
     info.buffer_size = frame.size();

Comments

> 
> ---
>  src/Makefile.am        |  4 ++++
>  src/mjpeg-fallback.cpp | 38 +++++++++++++-------------------------
>  2 files changed, 17 insertions(+), 25 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 0133bf5..31b8af1 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -21,6 +21,7 @@ AM_CPPFLAGS = \
>  	$(X11_CFLAGS) \
>  	$(XFIXES_CFLAGS) \
>  	$(XRANDR_CFLAGS) \
> +	$(XEXT_CFLAGS) \
>  	$(NULL)
>  
>  AM_CFLAGS = \
> @@ -56,6 +57,7 @@ spice_streaming_agent_LDADD = \
>  	$(X11_LIBS) \
>  	$(XFIXES_LIBS) \
>  	$(XRANDR_LIBS) \
> +	$(XEXT_LIBS) \
>  	$(JPEG_LIBS) \
>  	$(NULL)
>  
> @@ -77,6 +79,8 @@ spice_streaming_agent_SOURCES = \
>  	utils.cpp \
>  	utils.hpp \
>  	x11-display-info.cpp \
> +	xlib-capture.cpp \
> +	xlib-capture.hpp \
>  	$(NULL)
>  
>  if HAVE_GST
> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 03247a1..a43825b 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -16,6 +16,7 @@
>  #include <sstream>
>  #include <memory>
>  #include <syslog.h>
> +#include "xlib-capture.hpp"
>  
>  using namespace spice::streaming_agent;
>  
> @@ -44,11 +45,10 @@ public:
>  private:
>      MjpegSettings settings;
>      Display *const dpy;
> +    XlibCapture *xc;

unique_ptr ?

>  
>      std::vector<uint8_t> frame;
>  
> -    // last frame sizes
> -    int last_width = -1, last_height = -1;
>      // last time before capture
>      uint64_t last_time = 0;
>  };
> @@ -58,19 +58,21 @@ private:
>  MjpegFrameCapture::MjpegFrameCapture(const MjpegSettings& settings):
>      settings(settings),dpy(XOpenDisplay(nullptr))
>  {
> -    if (!dpy)
> +    if (!dpy) {
>          throw std::runtime_error("Unable to initialize X11");
> +    }
> +    xc = new XlibCapture(dpy);
>  }
>  
>  MjpegFrameCapture::~MjpegFrameCapture()
>  {
> +    delete xc;
>      XCloseDisplay(dpy);
>  }
>  
>  void MjpegFrameCapture::Reset()
>  {
>      frame.clear();
> -    last_width = last_height = -1;
>  }
>  
>  FrameInfo MjpegFrameCapture::CaptureFrame()
> @@ -100,34 +102,20 @@ FrameInfo MjpegFrameCapture::CaptureFrame()
>          }
>      }
>  
> -    int screen = XDefaultScreen(dpy);
> -
> -    Window win = RootWindow(dpy, screen);
> +    XImg *image = xc->capture();
>  

The capture returns a NULL value on failure, better to check it
(was "// TODO handle errors").

> -    XWindowAttributes win_info;
> -    XGetWindowAttributes(dpy, win, &win_info);
> +    bool is_first = image->new_resolution();
>  
> -    bool is_first = false;
> -    if (win_info.width != last_width || win_info.height != last_height) {
> -        last_width = win_info.width;
> -        last_height = win_info.height;
> -        is_first = true;
> -    }
> +    info.size.width = image->width();
> +    info.size.height = image->height();
>  
> -    info.size.width = win_info.width;
> -    info.size.height = win_info.height;
> -
> -    int format = ZPixmap;
> -    // TODO handle errors
> -    XImage *image = XGetImage(dpy, win, win_info.x, win_info.y,
> -                              win_info.width, win_info.height, AllPlanes,
> format);
>  
>      // TODO handle errors
>      // TODO multiple formats (only 32 bit)
> -    write_JPEG_file(frame, settings.quality, (uint8_t*) image->data,
> -                    image->width, image->height);
> +    write_JPEG_file(frame, settings.quality, (uint8_t*) image->get_data(),
> +                    image->width(), image->height());
>  
> -    image->f.destroy_image(image);
> +    delete image;
>  
>      info.buffer = &frame[0];
>      info.buffer_size = frame.size();

Frediano
On 8/28/19 5:48 PM, Frediano Ziglio wrote:
>> ---
>>   src/Makefile.am        |  4 ++++
>>   src/mjpeg-fallback.cpp | 38 +++++++++++++-------------------------
>>   2 files changed, 17 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 0133bf5..31b8af1 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -21,6 +21,7 @@ AM_CPPFLAGS = \
>>   	$(X11_CFLAGS) \
>>   	$(XFIXES_CFLAGS) \
>>   	$(XRANDR_CFLAGS) \
>> +	$(XEXT_CFLAGS) \
>>   	$(NULL)
>>   
>>   AM_CFLAGS = \
>> @@ -56,6 +57,7 @@ spice_streaming_agent_LDADD = \
>>   	$(X11_LIBS) \
>>   	$(XFIXES_LIBS) \
>>   	$(XRANDR_LIBS) \
>> +	$(XEXT_LIBS) \
>>   	$(JPEG_LIBS) \
>>   	$(NULL)
>>   
>> @@ -77,6 +79,8 @@ spice_streaming_agent_SOURCES = \
>>   	utils.cpp \
>>   	utils.hpp \
>>   	x11-display-info.cpp \
>> +	xlib-capture.cpp \
>> +	xlib-capture.hpp \
>>   	$(NULL)
>>   
>>   if HAVE_GST
>> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
>> index 03247a1..a43825b 100644
>> --- a/src/mjpeg-fallback.cpp
>> +++ b/src/mjpeg-fallback.cpp
>> @@ -16,6 +16,7 @@
>>   #include <sstream>
>>   #include <memory>
>>   #include <syslog.h>
>> +#include "xlib-capture.hpp"
>>   
>>   using namespace spice::streaming_agent;
>>   
>> @@ -44,11 +45,10 @@ public:
>>   private:
>>       MjpegSettings settings;
>>       Display *const dpy;
>> +    XlibCapture *xc;
> unique_ptr ?

Hi,

Just noticed, if it's a unique_ptr connection with X may be destroyed before
that (by ~MjpegFrameCapture)

Snir.

>
>>   
>>       std::vector<uint8_t> frame;
>>   
>> -    // last frame sizes
>> -    int last_width = -1, last_height = -1;
>>       // last time before capture
>>       uint64_t last_time = 0;
>>   };
>> @@ -58,19 +58,21 @@ private:
>>   MjpegFrameCapture::MjpegFrameCapture(const MjpegSettings& settings):
>>       settings(settings),dpy(XOpenDisplay(nullptr))
>>   {
>> -    if (!dpy)
>> +    if (!dpy) {
>>           throw std::runtime_error("Unable to initialize X11");
>> +    }
>> +    xc = new XlibCapture(dpy);
>>   }
>>   
>>   MjpegFrameCapture::~MjpegFrameCapture()
>>   {
>> +    delete xc;
>>       XCloseDisplay(dpy);
>>   }
>>   
>>   void MjpegFrameCapture::Reset()
>>   {
>>       frame.clear();
>> -    last_width = last_height = -1;
>>   }
>>   
>>   FrameInfo MjpegFrameCapture::CaptureFrame()
>> @@ -100,34 +102,20 @@ FrameInfo MjpegFrameCapture::CaptureFrame()
>>           }
>>       }
>>   
>> -    int screen = XDefaultScreen(dpy);
>> -
>> -    Window win = RootWindow(dpy, screen);
>> +    XImg *image = xc->capture();
>>   
> The capture returns a NULL value on failure, better to check it
> (was "// TODO handle errors").
>
>> -    XWindowAttributes win_info;
>> -    XGetWindowAttributes(dpy, win, &win_info);
>> +    bool is_first = image->new_resolution();
>>   
>> -    bool is_first = false;
>> -    if (win_info.width != last_width || win_info.height != last_height) {
>> -        last_width = win_info.width;
>> -        last_height = win_info.height;
>> -        is_first = true;
>> -    }
>> +    info.size.width = image->width();
>> +    info.size.height = image->height();
>>   
>> -    info.size.width = win_info.width;
>> -    info.size.height = win_info.height;
>> -
>> -    int format = ZPixmap;
>> -    // TODO handle errors
>> -    XImage *image = XGetImage(dpy, win, win_info.x, win_info.y,
>> -                              win_info.width, win_info.height, AllPlanes,
>> format);
>>   
>>       // TODO handle errors
>>       // TODO multiple formats (only 32 bit)
>> -    write_JPEG_file(frame, settings.quality, (uint8_t*) image->data,
>> -                    image->width, image->height);
>> +    write_JPEG_file(frame, settings.quality, (uint8_t*) image->get_data(),
>> +                    image->width(), image->height());
>>   
>> -    image->f.destroy_image(image);
>> +    delete image;
>>   
>>       info.buffer = &frame[0];
>>       info.buffer_size = frame.size();
> Frediano