Hi,
On 8/1/19 6:39 PM, Frediano Ziglio 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) {
bracket on next line
+ ((XImage*)data)->f.destroy_image((XImage*)data);
+}
+
void GstreamerFrameCapture::xlib_capture()
{
int screen = XDefaultScreen(dpy);
+ XImage *image = nullptr;
I would declare + initialize on the same line
Sure
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));
I'm a bit confused on reference counting here.
The change seems to indicate that now "buf" is freed automatically
(as the unique_ptr).
So, is this patch also fixing a memory leak ?
Indeed, forgot to mention, this was unnoticeable in common usage.
I noticed that leak only when i worked on this patch .
The issue is with the push_sample function which in oppose to push_buffer,
does not take ownership.
Snir.
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");
}
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel