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

[Kieran Bingham]

Quoting Nicolas Dufresne (2024-03-05 15:30:56)
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> With the camera manager, it is not possible to cleanly delete the

Does this mean 'with' or 'without the camera manager' ?

> FrameBufferAllocator object. Keep the camera manager alive until all the
> memory object have been released.
> 
> Fixes Bugzilla issue 211
> 

Bug: https://bugs.libcamera.org/show_bug.cgi?id=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.

s/represent that/represents the/ ?
s/have/has/

But those are just nits could fixed while applying.

I recall Laurent saying he had some worries about this series, thinking
that there might be a better way to handle keeping the lifetime managed.

But - This fixes a bug, and is fully green on the CI, and even
introduces a test to validate it. So I'd be keen to see this series
merged. There's always the opportunity to refactor in the future if
someone desired it.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +        */
> +       std::shared_ptr<CameraManager> *cm_ptr;
>  };
>  
>  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;
> +
>         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));
> +       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)
> -- 
> 2.43.2
>