[libcamera-devel] [PATCH] libcamera: camera: Optimize camera deletion

[Laurent Pinchart]

Hi Umang,

On Tue, Sep 08, 2020 at 07:28:04PM +0530, Umang Jain wrote:
> On 9/8/20 1:24 PM, Laurent Pinchart wrote:
> > In most cases the last reference to a Camera instance will be the one
> > held by the CameraManager. That reference gets released when the
> > CameraManager thread cleans up, just before it stops. There's no need to
> > delete the camera with deleteLater() in that case.
> >
> > To optimize this case, use deleteLater() only when the camera gets
> > deleted from a different thread, and delete is synchronously otherwise.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Umang, would you be able to test this patch with the Android HAL ?
>
> I agree for the optimization, but do you have any specific use case in 
> mind. I tested it briefly with cros_camera_service cli to test both `if` 
> branches on hot-unplug but as you know my android build/test setup is 
> fairly limited with UVC. deleteLater() was brought in, when a currently 
> streaming camera in an app, is hot-unplugged from the system. Although I 
> am confident that this patch will hold correctly in such a situation as 
> well.

I expect deleteLater() to be called in the hot-unplug case, and the
regular delete when the camera manager is stopped, without unplug.

> Reviewed-by: Umang Jain <email at uajain.com>
>
> >   src/libcamera/camera.cpp | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 4a9c19c33cfb..ae16a64a3b44 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -16,6 +16,7 @@
> >   
> >   #include "libcamera/internal/log.h"
> >   #include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/thread.h"
> >   #include "libcamera/internal/utils.h"
> >   
> >   /**
> > @@ -470,7 +471,10 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> >   	struct Deleter : std::default_delete<Camera> {
> >   		void operator()(Camera *camera)
> >   		{
> > -			camera->deleteLater();
> > +			if (Thread::current() == camera->thread())
> > +				delete camera;
> > +			else
> > +				camera->deleteLater();
> >   		}
> >   	};
> >   

-- 
Regards,

Laurent Pinchart