[libcamera-devel,v3,5/8] android: Replace scoped_lock<> with libcamera::MutexLocker

12356 diff mbox series

The CameraDevice class uses std::scoped_lock<> to guard access to the
class' descriptors_ member.

std::scoped_lock<> provides a set of features that guarantees safety
when locking multiple mutexes in a critical section, while for single
locks happening in a scoped block it does not provides benefits compared
to the simplest std::unique_lock<> which libcamera provides the
MutexLocker type for.

Replace usage of std::scoped_lock<> with libcamera::MutexLocker to make
the implementation consistent with the rest of the code base.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/camera_device.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Hi Jacopo,

Thank you for the patch.

On Fri, May 21, 2021 at 05:42:24PM +0200, Jacopo Mondi wrote:
> The CameraDevice class uses std::scoped_lock<> to guard access to the
> class' descriptors_ member.
> 
> std::scoped_lock<> provides a set of features that guarantees safety
> when locking multiple mutexes in a critical section, while for single
> locks happening in a scoped block it does not provides benefits compared
> to the simplest std::unique_lock<> which libcamera provides the
> MutexLocker type for.
> 
> Replace usage of std::scoped_lock<> with libcamera::MutexLocker to make
> the implementation consistent with the rest of the code base.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

For additional consistency, you may want to use Mutex instead of
std::mutex in camera_device.h (and then drop the #include <mutex>).
Another option would be to use std::unique_lock<> instead of MutexLocker
in this patch. Or you could keep it as-is :-) Up to you, in any case.

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

> ---
>  src/android/camera_device.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index cf0e5e459213..fceae96a8b7c 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -23,6 +23,7 @@
>  
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/log.h"
> +#include "libcamera/internal/thread.h"
>  #include "libcamera/internal/utils.h"
>  
>  #include "system/graphics.h"
> @@ -1947,7 +1948,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	worker_.queueRequest(descriptor.request_.get());
>  
>  	{
> -		std::scoped_lock<std::mutex> lock(mutex_);
> +		MutexLocker lock(mutex_);
>  		descriptors_[descriptor.request_->cookie()] = std::move(descriptor);
>  	}
>  
> @@ -1958,7 +1959,7 @@ void CameraDevice::requestComplete(Request *request)
>  {
>  	decltype(descriptors_)::node_type node;
>  	{
> -		std::scoped_lock<std::mutex> lock(mutex_);
> +		MutexLocker lock(mutex_);
>  		auto it = descriptors_.find(request->cookie());
>  		if (it == descriptors_.end()) {
>  			/*