[libcamera-devel,v2,4/7] v4l2: v4l2_camera_proxy: Acquire only one buffer semaphore on VIDIOC_DQBUF

Message ID 20200605090106.15424-5-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • Support qv4l2 with v4l2-compat
Related show

Commit Message

Paul Elder June 5, 2020, 9:01 a.m. UTC
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.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2: restructured the if block so it's easier to read
---
 src/v4l2/v4l2_camera_proxy.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart June 5, 2020, 10:16 a.m. UTC | #1
Hi Paul,

Thank you for the patch.

On Fri, Jun 05, 2020 at 06:01:03PM +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.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

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

> 
> ---
> Changes in v2: restructured the if block so it's easier to read
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index ec6d265d..a0c6deea 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -426,10 +426,10 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>  	    !validateMemoryType(arg->memory))
>  		return -EINVAL;
>  
> -	if (nonBlocking_ && !vcam_->bufferSema_.tryAcquire())
> -		return -EAGAIN;
> -	else
> +	if (!nonBlocking_)
>  		vcam_->bufferSema_.acquire();
> +	else if (!vcam_->bufferSema_.tryAcquire())
> +		return -EAGAIN;
>  
>  	updateBuffers();
>
Niklas Söderlund June 5, 2020, 6:18 p.m. UTC | #2
Hi Paul,

Thanks for your patch.

On 2020-06-05 18:01:03 +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.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> 
> ---
> Changes in v2: restructured the if block so it's easier to read
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index ec6d265d..a0c6deea 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -426,10 +426,10 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>  	    !validateMemoryType(arg->memory))
>  		return -EINVAL;
>  
> -	if (nonBlocking_ && !vcam_->bufferSema_.tryAcquire())
> -		return -EAGAIN;
> -	else
> +	if (!nonBlocking_)
>  		vcam_->bufferSema_.acquire();
> +	else if (!vcam_->bufferSema_.tryAcquire())
> +		return -EAGAIN;
>  
>  	updateBuffers();
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index ec6d265d..a0c6deea 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -426,10 +426,10 @@  int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
 	    !validateMemoryType(arg->memory))
 		return -EINVAL;
 
-	if (nonBlocking_ && !vcam_->bufferSema_.tryAcquire())
-		return -EAGAIN;
-	else
+	if (!nonBlocking_)
 		vcam_->bufferSema_.acquire();
+	else if (!vcam_->bufferSema_.tryAcquire())
+		return -EAGAIN;
 
 	updateBuffers();