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

[Umang Jain]

Hi Laurent,

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.

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();
>   		}
>   	};
>