[libcamera-devel] [PATCH 6/8] android: Guard access to the camera state

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2021-05-10 12:52:33 +0200, Jacopo Mondi wrote:
> Guard access to the camera state and the start/stop sequences
> with a mutex.
> 
> Currently only stop() and the first call to processCaptureRequest()
> start and stop the camera, and they're not meant to race with each
> other. With the introduction of flush() the camera can be stopped
> concurrently to a processCaptureRequest() call, hence access to the
> camera state will need to be protected.
> 
> Prepare for that by guarding the existing paths with a mutex.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/android/camera_device.cpp | 23 ++++++++++++++---------
>  src/android/camera_device.h   |  1 +
>  2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index ff965a1c8c86..14a3f79a7402 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -758,6 +758,7 @@ void CameraDevice::close()
>  
>  void CameraDevice::stop()
>  {
> +	MutexLocker cameraLock(cameraMutex_);
>  	if (state_ == CameraStopped)
>  		return;
>  
> @@ -1913,17 +1914,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	if (!isValidRequest(camera3Request))
>  		return -EINVAL;
>  
> -	/* Start the camera if that's the first request we handle. */
> -	if (state_ == CameraStopped) {
> -		worker_.start();
> +	{
> +		MutexLocker cameraLock(cameraMutex_);
>  
> -		int ret = camera_->start();
> -		if (ret) {
> -			LOG(HAL, Error) << "Failed to start camera";
> -			return ret;
> -		}
> +		/* Start the camera if that's the first request we handle. */
> +		if (state_ == CameraStopped) {
> +			worker_.start();
>  
> -		state_ = CameraRunning;
> +			int ret = camera_->start();
> +			if (ret) {
> +				LOG(HAL, Error) << "Failed to start camera";
> +				return ret;
> +			}
> +
> +			state_ = CameraRunning;
> +		}
>  	}
>  
>  	/*
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 30b364decaab..f263fdae472a 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -120,6 +120,7 @@ private:
>  
>  	CameraWorker worker_;
>  
> +	libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */
>  	State state_;
>  
>  	std::shared_ptr<libcamera::Camera> camera_;
> -- 
> 2.31.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund