[libcamera-devel] [PATCH v2 5/5] v4l2: v4l2_camera: Apply clang thread safety annotation

[Laurent Pinchart]

Hi Umang and Hiro,

Thank you for the patch.

On Mon, Jun 20, 2022 at 10:20:27PM +0530, Umang Jain via libcamera-devel wrote:
> From: Hirokazu Honda <hiroh at chromium.org>
> 
> This annotates member functions and variables of V4L2Camera by
> clang thread safety annotations.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp |  5 ++---
>  src/v4l2/v4l2_camera.h   | 14 ++++++++------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index e922b9e6..7b97c2d5 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -71,11 +71,10 @@ std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers()
>  {
>  	std::vector<Buffer> v;
>  
> -	bufferLock_.lock();
> +	MutexLocker lock(bufferLock_);

Not strictly within the scope of the commit message, but this is a fine
change.

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

>  	for (std::unique_ptr<Buffer> &metadata : completedBuffers_)
>  		v.push_back(*metadata.get());
>  	completedBuffers_.clear();
> -	bufferLock_.unlock();
>  
>  	return v;
>  }
> @@ -278,7 +277,7 @@ int V4L2Camera::qbuf(unsigned int index)
>  void V4L2Camera::waitForBufferAvailable()
>  {
>  	MutexLocker locker(bufferMutex_);
> -	bufferCV_.wait(locker, [&] {
> +	bufferCV_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(bufferMutex_) {
>  			       return bufferAvailableCount_ >= 1 || !isRunning_;
>  		       });
>  	if (isRunning_)
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 03e74118..d3483444 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -39,7 +39,7 @@ public:
>  	void bind(int efd);
>  	void unbind();
>  
> -	std::vector<Buffer> completedBuffers();
> +	std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_);
>  
>  	int configure(libcamera::StreamConfiguration *streamConfigOut,
>  		      const libcamera::Size &size,
> @@ -58,13 +58,14 @@ public:
>  
>  	int qbuf(unsigned int index);
>  
> -	void waitForBufferAvailable();
> -	bool isBufferAvailable();
> +	void waitForBufferAvailable() LIBCAMERA_TSA_EXCLUDES(bufferMutex_);
> +	bool isBufferAvailable() LIBCAMERA_TSA_EXCLUDES(bufferMutex_);
>  
>  	bool isRunning();
>  
>  private:
> -	void requestComplete(libcamera::Request *request);
> +	void requestComplete(libcamera::Request *request)
> +		LIBCAMERA_TSA_EXCLUDES(bufferLock_);
>  
>  	std::shared_ptr<libcamera::Camera> camera_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;
> @@ -77,11 +78,12 @@ private:
>  	std::vector<std::unique_ptr<libcamera::Request>> requestPool_;
>  
>  	std::deque<libcamera::Request *> pendingRequests_;
> -	std::deque<std::unique_ptr<Buffer>> completedBuffers_;
> +	std::deque<std::unique_ptr<Buffer>> completedBuffers_
> +		LIBCAMERA_TSA_GUARDED_BY(bufferLock_);
>  
>  	int efd_;
>  
>  	libcamera::Mutex bufferMutex_;
>  	libcamera::ConditionVariable bufferCV_;
> -	unsigned int bufferAvailableCount_;
> +	unsigned int bufferAvailableCount_ LIBCAMERA_TSA_GUARDED_BY(bufferMutex_);
>  };

-- 
Regards,

Laurent Pinchart