[libcamera-devel] [PATCH v4 11/11] libcamera: camera: Lock the pipeline handler in acquire()

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, May 17, 2019 at 02:54:47AM +0200, Niklas Söderlund wrote:
> To allow more than one application using libcamera simultaneously there
> can be no overlap between which cameras are in use by which user. As a
> camera is part of a pipeline handler and there might be shared resources
> between all cameras exposed by that pipeline handler it's not enough to
> to only lock access to a single camera, all cameras from that pipeline
> need to be tied to the same process.
> 
> Allow for this by locking the whole pipeline when one of its cameras
> is acquired by the user. Other processes can still enumerate and list
> all cameras in the system but can't acquire a camera from a locked
> pipeline handler.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/camera.cpp | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index cb45bafe3fb1ff85..a83768fcde126c32 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -470,13 +470,17 @@ void Camera::disconnect()
>   * not blocking, if the device has already been acquired (by the same or another
>   * process) the -EBUSY error code is returned.
>   *
> + * Acquiring a camera will limit usage of any other camera(s) provided by the
> + * same pipeline hander to the same instance of libcamera. The limit is in

s/hander/handler/

> + * effect until all cameras from the pipeline handler are released(). Other

"released" isn't a function, you should write "released" or "released
with release()". As the release() function is mentioned elsewhere in
this documentation block, I would leave it out here.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + * instances of libcamera can still list and examine the cameras but will fail
> + * if they attempt to acquire() any of them.
> + *
>   * Once exclusive access isn't needed anymore, the device should be released
>   * with a call to the release() function.
>   *
>   * This function affects the state of the camera, see \ref camera_operation.
>   *
> - * \todo Implement exclusive access across processes.
> - *
>   * \return 0 on success or a negative error code otherwise
>   * \retval -ENODEV The camera has been disconnected from the system
>   * \retval -EBUSY The camera is not free and can't be acquired by the caller
> @@ -489,6 +493,12 @@ int Camera::acquire()
>  	if (!stateIs(CameraAvailable))
>  		return -EBUSY;
>  
> +	if (!pipe_->lock()) {
> +		LOG(Camera, Info)
> +			<< "Pipeline handler in use by another process";
> +		return -EBUSY;
> +	}
> +
>  	state_ = CameraAcquired;
>  
>  	return 0;
> @@ -510,6 +520,8 @@ int Camera::release()
>  	if (!stateBetween(CameraAvailable, CameraConfigured))
>  		return -EBUSY;
>  
> +	pipe_->unlock();
> +
>  	state_ = CameraAvailable;
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart