[libcamera-devel,v1,19/23] gst: libcamerasrc: Allocate and release buffers
diff mbox series

Message ID 20200129033210.278800-20-nicolas@ndufresne.ca
State Superseded
Headers show
Series
  • GStreamer Element for libcamera
Related show

Commit Message

Nicolas Dufresne Jan. 29, 2020, 3:32 a.m. UTC 
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Setup the allocation and the release of buffers in the
element. We have one pooling GstAllocator that wraps the
FrameBufferAllocator and tracks the live time of FrameBuffer
object. Then for each pads we have a GstBufferPool object
which is only used to avoid re-allocation the GstBuffer
shell everything.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

Comments

Laurent Pinchart Feb. 12, 2020, 12:27 a.m. UTC | #1 
Hi Nicolas,

Thank you for the patch.

On Tue, Jan 28, 2020 at 10:32:06PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Setup the allocation and the release of buffers in the
> element. We have one pooling GstAllocator that wraps the
> FrameBufferAllocator and tracks the live time of FrameBuffer

s/live time/lifetime/

> object. Then for each pads we have a GstBufferPool object

s/pads/pad/

> which is only used to avoid re-allocation the GstBuffer

s/allocation/allocating/ ?

> shell everything.

I'm not sure to get that.

> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 0df71c6..5fc4393 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -8,6 +8,8 @@
>  
>  #include "gstlibcamerasrc.h"
>  #include "gstlibcamerapad.h"
> +#include "gstlibcameraallocator.h"
> +#include "gstlibcamerapool.h"
>  #include "gstlibcamera-utils.h"
>  
>  #include <libcamera/camera.h>
> @@ -37,6 +39,7 @@ struct _GstLibcameraSrc {
>  	gchar *camera_name;
>  
>  	GstLibcameraSrcState *state;
> +	GstLibcameraAllocator *allocator;
>  };
>  
>  enum {
> @@ -207,6 +210,23 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
>  		return;
>  	}
>  
> +	self->allocator = gst_libcamera_allocator_new(state->cam);
> +	if (!self->allocator) {
> +		GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,
> +				  ("Failed to allocate memory"),
> +				  ("gst_libcamera_allocator_new() failed."));
> +		gst_task_stop(task);
> +		return;
> +	}
> +
> +	for (gsize i = 0; i < state->srcpads.size(); i++) {
> +		GstPad *srcpad = state->srcpads[i];
> +		const StreamConfiguration &stream_cfg = state->config->at(i);
> +		GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
> +								stream_cfg.stream());
> +		gst_libcamera_pad_set_pool(srcpad, pool);
> +	}
> +
>  done:
>  	switch (flow_ret) {
>  	case GST_FLOW_NOT_NEGOTIATED:
> @@ -225,8 +245,15 @@ static void
>  gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)
>  {
>  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> +	GstLibcameraSrcState *state = self->state;
>  
>  	GST_DEBUG_OBJECT(self, "Streaming thread is about to stop");
> +	state->cam->stop();

As this patch doesn't add cam->start(), doesn't this belong to another
patch ?

> +
> +	for (GstPad *srcpad : state->srcpads)
> +		gst_libcamera_pad_set_pool(srcpad, NULL);
> +
> +	g_clear_object(&self->allocator);
>  }
>  
>  static void
> @@ -235,6 +262,8 @@ gst_libcamera_src_close(GstLibcameraSrc *self)
>  	GstLibcameraSrcState *state = self->state;
>  	gint ret;
>  
> +	GST_DEBUG_OBJECT(self, "Releasing resources");
> +
>  	ret = state->cam->release();
>  	if (ret) {
>  		GST_ELEMENT_WARNING(self, RESOURCE, BUSY,
Nicolas Dufresne Feb. 12, 2020, 5:38 p.m. UTC | #2 
Le mercredi 12 février 2020 à 02:27 +0200, Laurent Pinchart a écrit :
> Hi Nicolas,
> 
> Thank you for the patch.
> 
> On Tue, Jan 28, 2020 at 10:32:06PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > Setup the allocation and the release of buffers in the
> > element. We have one pooling GstAllocator that wraps the
> > FrameBufferAllocator and tracks the live time of FrameBuffer
> 
> s/live time/lifetime/
> 
> > object. Then for each pads we have a GstBufferPool object
> 
> s/pads/pad/
> 
> > which is only used to avoid re-allocation the GstBuffer
> 
> s/allocation/allocating/ ?
> 
> > shell everything.
> 
> I'm not sure to get that.

Same, I'll try and figure out.

> 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 0df71c6..5fc4393 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -8,6 +8,8 @@
> >  
> >  #include "gstlibcamerasrc.h"
> >  #include "gstlibcamerapad.h"
> > +#include "gstlibcameraallocator.h"
> > +#include "gstlibcamerapool.h"
> >  #include "gstlibcamera-utils.h"
> >  
> >  #include <libcamera/camera.h>
> > @@ -37,6 +39,7 @@ struct _GstLibcameraSrc {
> >  	gchar *camera_name;
> >  
> >  	GstLibcameraSrcState *state;
> > +	GstLibcameraAllocator *allocator;
> >  };
> >  
> >  enum {
> > @@ -207,6 +210,23 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
> >  		return;
> >  	}
> >  
> > +	self->allocator = gst_libcamera_allocator_new(state->cam);
> > +	if (!self->allocator) {
> > +		GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,
> > +				  ("Failed to allocate memory"),
> > +				  ("gst_libcamera_allocator_new() failed."));
> > +		gst_task_stop(task);
> > +		return;
> > +	}
> > +
> > +	for (gsize i = 0; i < state->srcpads.size(); i++) {
> > +		GstPad *srcpad = state->srcpads[i];
> > +		const StreamConfiguration &stream_cfg = state->config->at(i);
> > +		GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
> > +								stream_cfg.stream());
> > +		gst_libcamera_pad_set_pool(srcpad, pool);
> > +	}
> > +
> >  done:
> >  	switch (flow_ret) {
> >  	case GST_FLOW_NOT_NEGOTIATED:
> > @@ -225,8 +245,15 @@ static void
> >  gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)
> >  {
> >  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> > +	GstLibcameraSrcState *state = self->state;
> >  
> >  	GST_DEBUG_OBJECT(self, "Streaming thread is about to stop");
> > +	state->cam->stop();
> 
> As this patch doesn't add cam->start(), doesn't this belong to another
> patch ?
> 
> > +
> > +	for (GstPad *srcpad : state->srcpads)
> > +		gst_libcamera_pad_set_pool(srcpad, NULL);
> > +
> > +	g_clear_object(&self->allocator);
> >  }
> >  
> >  static void
> > @@ -235,6 +262,8 @@ gst_libcamera_src_close(GstLibcameraSrc *self)
> >  	GstLibcameraSrcState *state = self->state;
> >  	gint ret;
> >  
> > +	GST_DEBUG_OBJECT(self, "Releasing resources");
> > +
> >  	ret = state->cam->release();
> >  	if (ret) {
> >  		GST_ELEMENT_WARNING(self, RESOURCE, BUSY,

Patch
diff mbox series 

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 0df71c6..5fc4393 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -8,6 +8,8 @@ 
 
 #include "gstlibcamerasrc.h"
 #include "gstlibcamerapad.h"
+#include "gstlibcameraallocator.h"
+#include "gstlibcamerapool.h"
 #include "gstlibcamera-utils.h"
 
 #include <libcamera/camera.h>
@@ -37,6 +39,7 @@  struct _GstLibcameraSrc {
 	gchar *camera_name;
 
 	GstLibcameraSrcState *state;
+	GstLibcameraAllocator *allocator;
 };
 
 enum {
@@ -207,6 +210,23 @@  gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
 		return;
 	}
 
+	self->allocator = gst_libcamera_allocator_new(state->cam);
+	if (!self->allocator) {
+		GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,
+				  ("Failed to allocate memory"),
+				  ("gst_libcamera_allocator_new() failed."));
+		gst_task_stop(task);
+		return;
+	}
+
+	for (gsize i = 0; i < state->srcpads.size(); i++) {
+		GstPad *srcpad = state->srcpads[i];
+		const StreamConfiguration &stream_cfg = state->config->at(i);
+		GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
+								stream_cfg.stream());
+		gst_libcamera_pad_set_pool(srcpad, pool);
+	}
+
 done:
 	switch (flow_ret) {
 	case GST_FLOW_NOT_NEGOTIATED:
@@ -225,8 +245,15 @@  static void
 gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)
 {
 	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
+	GstLibcameraSrcState *state = self->state;
 
 	GST_DEBUG_OBJECT(self, "Streaming thread is about to stop");
+	state->cam->stop();
+
+	for (GstPad *srcpad : state->srcpads)
+		gst_libcamera_pad_set_pool(srcpad, NULL);
+
+	g_clear_object(&self->allocator);
 }
 
 static void
@@ -235,6 +262,8 @@  gst_libcamera_src_close(GstLibcameraSrc *self)
 	GstLibcameraSrcState *state = self->state;
 	gint ret;
 
+	GST_DEBUG_OBJECT(self, "Releasing resources");
+
 	ret = state->cam->release();
 	if (ret) {
 		GST_ELEMENT_WARNING(self, RESOURCE, BUSY,