[libcamera-devel] [PATCH 3/5] v4l2: v4l2_camera_proxy: Acquire only one buffer semaphore on VIDIOC_DQBUF

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Wed, Jun 03, 2020 at 11:16:07PM +0900, Paul Elder wrote:
> We use a semaphore to atomically keep track of how many buffers are
> available for dequeueing. The check for how to acquire the semaphore was
> incorrect, leading to a double acquire upon a successful nonblocking
> acquire. Fix this.

Good catch.

> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index ec6d265d..d2419b96 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -428,7 +428,7 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>  
>  	if (nonBlocking_ && !vcam_->bufferSema_.tryAcquire())
>  		return -EAGAIN;
> -	else
> +	else if (!nonBlocking_)
>  		vcam_->bufferSema_.acquire();

This looks a bit confusing. How about the following ?

	if (nonBlocking_) {
		if (!vcam_->bufferSema_.tryAcquire())
			return -EAGAIN;
	} else {
		vcam_->bufferSema_.acquire();
	}

or

	if (!nonBlocking_)
		vcam_->bufferSema_.acquire();
	else if (!vcam_->bufferSema_.tryAcquire())
		return -EAGAIN;

Pick the one you like best.

>  
>  	updateBuffers();

-- 
Regards,

Laurent Pinchart