| Message ID | 20211129114453.3186042-7-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Nov 29, 2021 at 08:44:48PM +0900, Hirokazu Honda wrote: > This applies clang thread safety annotation to CameraHalManager. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/android/camera_hal_manager.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h > index c07684a1..5395d419 100644 > --- a/src/android/camera_hal_manager.h > +++ b/src/android/camera_hal_manager.h > @@ -8,7 +8,6 @@ > #pragma once > > #include <map> > -#include <mutex> I think this belongs to patch 02/11. > #include <stddef.h> > #include <tuple> > #include <vector> > @@ -18,7 +17,8 @@ > #include <system/camera_metadata.h> > > #include <libcamera/base/class.h> > -#include <libcamera/base/thread.h> > +#include <libcamera/base/mutex.h> > +#include <libcamera/base/thread_annotations.h> Given that mutex.h includes thread_annotations.h, and that it will always do so (as our mutex.h is there only for the purpose of supporting TSA), I wonder if we should include it explicitly here. Same comment for the other patches in this series. > > #include <libcamera/camera_manager.h> > > @@ -54,14 +54,14 @@ private: > void cameraAdded(std::shared_ptr<libcamera::Camera> cam); > void cameraRemoved(std::shared_ptr<libcamera::Camera> cam); > > - CameraDevice *cameraDeviceFromHalId(unsigned int id); > + CameraDevice *cameraDeviceFromHalId(unsigned int id) LIBCAMERA_TSA_REQUIRES(mutex_); > > std::unique_ptr<libcamera::CameraManager> cameraManager_; > CameraHalConfig halConfig_; > > const camera_module_callbacks_t *callbacks_; > - std::vector<std::unique_ptr<CameraDevice>> cameras_; > - std::map<std::string, unsigned int> cameraIdsMap_; > + std::vector<std::unique_ptr<CameraDevice>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_); > + std::map<std::string, unsigned int> cameraIdsMap_ LIBCAMERA_TSA_GUARDED_BY(mutex_); > libcamera::Mutex mutex_; > > unsigned int numInternalCameras_;
diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h index c07684a1..5395d419 100644 --- a/src/android/camera_hal_manager.h +++ b/src/android/camera_hal_manager.h @@ -8,7 +8,6 @@ #pragma once #include <map> -#include <mutex> #include <stddef.h> #include <tuple> #include <vector> @@ -18,7 +17,8 @@ #include <system/camera_metadata.h> #include <libcamera/base/class.h> -#include <libcamera/base/thread.h> +#include <libcamera/base/mutex.h> +#include <libcamera/base/thread_annotations.h> #include <libcamera/camera_manager.h> @@ -54,14 +54,14 @@ private: void cameraAdded(std::shared_ptr<libcamera::Camera> cam); void cameraRemoved(std::shared_ptr<libcamera::Camera> cam); - CameraDevice *cameraDeviceFromHalId(unsigned int id); + CameraDevice *cameraDeviceFromHalId(unsigned int id) LIBCAMERA_TSA_REQUIRES(mutex_); std::unique_ptr<libcamera::CameraManager> cameraManager_; CameraHalConfig halConfig_; const camera_module_callbacks_t *callbacks_; - std::vector<std::unique_ptr<CameraDevice>> cameras_; - std::map<std::string, unsigned int> cameraIdsMap_; + std::vector<std::unique_ptr<CameraDevice>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_); + std::map<std::string, unsigned int> cameraIdsMap_ LIBCAMERA_TSA_GUARDED_BY(mutex_); libcamera::Mutex mutex_; unsigned int numInternalCameras_;