[Spice-devel] [PATCH spice-streaming-agent 3/4] gst-plugin: Free input buffer and XImage as soon as possible

[Uri Lublin]

On 8/1/19 6:01 PM, Snir Sheriber wrote:
> ---
>   src/gst-plugin.cpp | 31 ++++++++++++++++++++-----------
>   1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 0a1d041..c7412c5 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -67,6 +67,15 @@ struct GstSampleDeleter {
>   
>   using GstSampleUPtr = std::unique_ptr<GstSample, GstSampleDeleter>;
>   
> +struct GstBufferDeleter {
> +    void operator()(GstBuffer* p)
> +    {
> +        gst_buffer_unref(p);
> +    }
> +};
> +
> +using GstBufferUPtr = std::unique_ptr<GstBuffer, GstBufferDeleter>;
> +
>   class GstreamerFrameCapture final : public FrameCapture
>   {
>   public:
> @@ -86,7 +95,6 @@ private:
>       Display *const dpy;
>   #if XLIB_CAPTURE
>       void xlib_capture();
> -    XImage *image = nullptr;
>   #endif
>       GstObjectUPtr<GstElement> pipeline, capture, sink;
>       GstSampleUPtr sample;
> @@ -306,12 +314,6 @@ void GstreamerFrameCapture::free_sample()
>           gst_buffer_unmap(gst_sample_get_buffer(sample.get()), &map);
>           sample.reset();
>       }
> -#if XLIB_CAPTURE
> -    if(image) {
> -        image->f.destroy_image(image);
> -        image = nullptr;
> -    }
> -#endif
>   }
>   
>   GstreamerFrameCapture::~GstreamerFrameCapture()
> @@ -327,9 +329,14 @@ void GstreamerFrameCapture::Reset()
>   }
>   
>   #if XLIB_CAPTURE
> +void free_ximage(gpointer data) {
> +    ((XImage*)data)->f.destroy_image((XImage*)data);

Hi,

It would be nicer to split into two lines
      XImage *image = data;
      image->f.destroy_image(image)

Uri.

> +}
> +
>   void GstreamerFrameCapture::xlib_capture()
>   {
>       int screen = XDefaultScreen(dpy);
> +    XImage *image = nullptr;
>   
>       Window win = RootWindow(dpy, screen);
>       XWindowAttributes win_info;
> @@ -355,9 +362,11 @@ void GstreamerFrameCapture::xlib_capture()
>           throw std::runtime_error("Cannot capture from X");
>       }
>   
> -    GstBuffer *buf;
> -    buf = gst_buffer_new_wrapped(image->data, image->height * image->bytes_per_line);
> -    if (!buf) {
> +    GstBufferUPtr buf(gst_buffer_new_wrapped_full((GstMemoryFlags)0, image->data,
> +                                                  image->height * image->bytes_per_line, 0,
> +                                                  image->height * image->bytes_per_line, image,
> +                                                  free_ximage));
> +    if (!buf.get()) {
>           throw std::runtime_error("Failed to wrap image in gstreamer buffer");
>       }
>   
> @@ -369,7 +378,7 @@ void GstreamerFrameCapture::xlib_capture()
>                                            nullptr));
>   
>       // Push sample
> -    GstSampleUPtr appsrc_sample(gst_sample_new(buf, caps.get(), nullptr, nullptr));
> +    GstSampleUPtr appsrc_sample(gst_sample_new(buf.get(), caps.get(), nullptr, nullptr));
>       if (gst_app_src_push_sample(GST_APP_SRC(capture.get()), appsrc_sample.get()) != GST_FLOW_OK) {
>           throw std::runtime_error("gstramer appsrc element cannot push sample");
>       }
>