[libcamera-devel] [PATCH v2 08/11] android: camera_stream: Add thread safety annotation

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Mon, Nov 29, 2021 at 08:44:50PM +0900, Hirokazu Honda wrote:
> This applies clang thread safety annotation to CameraStream.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_stream.cpp |  2 +-
>  src/android/camera_stream.h   | 11 +++++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 2181d094..ae659808 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -275,7 +275,7 @@ void CameraStream::PostProcessorWorker::run()
>  	MutexLocker locker(mutex_);
>  
>  	while (1) {
> -		cv_.wait(locker, [&] {
> +		cv_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(mutex_) {
>  			return state_ != State::Running || !requests_.empty();
>  		});
>  
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index c127a0e4..6737bd2d 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -8,13 +8,14 @@
>  #pragma once
>  
>  #include <memory>
> -#include <mutex>
>  #include <queue>
>  #include <vector>
>  
>  #include <hardware/camera3.h>
>  
> +#include <libcamera/base/mutex.h>
>  #include <libcamera/base/thread.h>
> +#include <libcamera/base/thread_annotations.h>

I think this belongs to patch 02/11.

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

>  #include <libcamera/camera.h>
>  #include <libcamera/framebuffer.h>
> @@ -155,8 +156,10 @@ private:
>  		libcamera::Mutex mutex_;
>  		libcamera::ConditionVariable cv_;
>  
> -		std::queue<Camera3RequestDescriptor::StreamBuffer *> requests_;
> -		State state_ = State::Stopped;
> +		std::queue<Camera3RequestDescriptor::StreamBuffer *> requests_
> +			LIBCAMERA_TSA_GUARDED_BY(mutex_);
> +
> +		State state_ LIBCAMERA_TSA_GUARDED_BY(mutex_) = State::Stopped;
>  	};
>  
>  	int waitFence(int fence);
> @@ -168,7 +171,7 @@ private:
>  	const unsigned int index_;
>  
>  	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
> -	std::vector<libcamera::FrameBuffer *> buffers_;
> +	std::vector<libcamera::FrameBuffer *> buffers_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  	/*
>  	 * The class has to be MoveConstructible as instances are stored in
>  	 * an std::vector in CameraDevice.

-- 
Regards,

Laurent Pinchart