[PATCH v2 1/3] gstreamer: allocator: Ensure camera manager stay alive
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue May 14 20:40:44 CEST 2024
Hi Nicolas,
On Tue, May 14, 2024 at 12:53:51PM -0400, Nicolas Dufresne wrote:
> Le mardi 14 mai 2024 à 19:08 +0300, Laurent Pinchart a écrit :
> > 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/ ?
>
> Without, was already mentioned before.
>
> > > 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
>
> No, it can't, that C structure is alloc/freed by glib, as a side effect will be
> missing a destructor call to drop the shared pointer reference. In the
> GstElement code, that is why we have a C++ class, the "state". It makes
> everything more convenient. But until we have more then once C++ data member its
> not worth it.
You're right. Could you record this in the commit message ?
> > 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
More information about the libcamera-devel
mailing list