[PATCH v2 1/3] gstreamer: allocator: Ensure camera manager stay alive

[Laurent Pinchart]

Hi Nicolas,

Thank you for the patch.

On Tue, Mar 05, 2024 at 10:30:56AM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> With the camera manager, it is not possible to cleanly delete the

Did you mean s/With/Without/ ?

> FrameBufferAllocator object. Keep the camera manager alive until all the
> memory object have been released.
> 
> Fixes Bugzilla issue 211
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> ---
>  src/gstreamer/gstlibcameraallocator.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
> index c740b8fc..844bdb17 100644
> --- a/src/gstreamer/gstlibcameraallocator.cpp
> +++ b/src/gstreamer/gstlibcameraallocator.cpp
> @@ -100,6 +100,11 @@ struct _GstLibcameraAllocator {
>  	 * FrameWrap.
>  	 */
>  	GHashTable *pools;
> +	/*
> +	 * The camera manager represent that library, which needs to be kept alive
> +	 * until all the memory have been released.
> +	 */
> +	std::shared_ptr<CameraManager> *cm_ptr;

This doesn't have to be a pointer, it can be

	std::shared_ptr<CameraManager> cm;

>  };
>  
>  G_DEFINE_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,
> @@ -173,6 +178,9 @@ gst_libcamera_allocator_finalize(GObject *object)
>  
>  	delete self->fb_allocator;
>  
> +	/* keep last */
> +	delete self->cm_ptr;
> +

This would become

	/* Keep last. */
	self->cm.reset();

>  	G_OBJECT_CLASS(gst_libcamera_allocator_parent_class)->finalize(object);
>  }
>  
> @@ -193,11 +201,17 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera,
>  {
>  	auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
>  							  nullptr));
> +	gint ret;
> +
> +	self->cm_ptr = new std::shared_ptr<CameraManager>(gst_libcamera_get_camera_manager(ret));

And here,

	self->cm = gst_libcamera_get_camera_manager(ret);

> +	if (ret) {
> +		g_object_unref(self);
> +		return nullptr;
> +	}
>  
>  	self->fb_allocator = new FrameBufferAllocator(camera);
>  	for (StreamConfiguration &streamCfg : *config_) {
>  		Stream *stream = streamCfg.stream();
> -		gint ret;
>  
>  		ret = self->fb_allocator->allocate(stream);
>  		if (ret == 0)

-- 
Regards,

Laurent Pinchart