Message ID | 20220620165027.549085-6-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
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@chromium.org> > > This annotates member functions and variables of V4L2Camera by > clang thread safety annotations. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > Signed-off-by: Umang Jain <umang.jain@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@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_); > };
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_); 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_); };