[libcamera-devel] [PATCH v2 5/7] libcamera: camera: Add acquire() and release()

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jan 25, 2019 at 04:33:38PM +0100, Niklas Söderlund wrote:
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/camera.h |  5 +++++
>  src/libcamera/camera.cpp   | 35 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index a2ded62de94814c4..7e358f8c0aa093cf 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -29,6 +29,9 @@ public:
>  
>  	Signal<Camera *> disconnected;
>  
> +	int acquire();
> +	void release();
> +
>  private:
>  	Camera(PipelineHandler *pipe, const std::string &name);
>  	~Camera();
> @@ -38,6 +41,8 @@ private:
>  
>  	std::shared_ptr<PipelineHandler> pipe_;
>  	std::string name_;
> +
> +	bool acquired_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 9cec289282e4797b..fd19e8cf6694cc1b 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -102,7 +102,7 @@ const std::string &Camera::name() const
>   */
>  
>  Camera::Camera(PipelineHandler *pipe, const std::string &name)
> -	: pipe_(pipe->shared_from_this()), name_(name)
> +	: pipe_(pipe->shared_from_this()), name_(name), acquired_(false)
>  {
>  }
>  
> @@ -127,4 +127,37 @@ void Camera::disconnect()
>  	disconnected.emit(this);
>  }
>  
> +/**
> + * \brief Acquire the camera device for exclusive access
> + *
> + * After opening the device with open(), exclusive access must be obtained
> + * before performing operations that change the device state. This function is
> + * not blocking, if the device has already been acquired (by the same or another
> + * process) the -EBUSY error code is returned.
> + *
> + * Once exclusive access isn't needed anymore, the device should be released
> + * with a call to the release() function.

Let's add a

  \todo Implement exclusive access across processes.

> + *
> + * \return 0 on success or a negative error code on error.
> + */
> +int Camera::acquire()
> +{
> +	if (acquired_)
> +		return -EBUSY;
> +
> +	acquired_ = true;
> +	return 0;
> +}
> +
> +/**
> + * \brief Release exclusive access to the camera device
> + *
> + * Releasing the camera device allows other users to acquire exclusive access
> + * with the acquire() function.
> + */
> +void Camera::release()
> +{
> +	acquired_ = false;
> +}
> +
>  } /* namespace libcamera */

Should we add a warning in the destructor in case acquired_ is true,
similarly to what is done in DeviceEnumerator::~DeviceEnumerator() ?

-- 
Regards,

Laurent Pinchart