[libcamera-devel] [PATCH v1 19/23] gst: libcamerasrc: Allocate and release buffers

[Nicolas Dufresne]

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 at 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 at 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,