[libcamera-devel,15/15] v4l2: v4l2_camera_proxy: Fix v4l2-compliance streaming tests

Message ID 20200616131244.70308-16-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Support v4l2-compliance
Related show

Commit Message

Paul Elder June 16, 2020, 1:12 p.m. UTC
Fix v4l2-compliance streaming tests, for reqbufs, qbuf, dqbuf, and
streamon.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/v4l2/v4l2_camera_proxy.cpp | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi June 17, 2020, 3:43 p.m. UTC | #1
Hi Paul,

On Tue, Jun 16, 2020 at 10:12:44PM +0900, Paul Elder wrote:
> Fix v4l2-compliance streaming tests, for reqbufs, qbuf, dqbuf, and
> streamon.

This seems a bit of a collection of random fixes. I'm not opposed to
that in principle, but I think the single issues should at least be
listed in the commit message.

>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index e4d534a..fca728a 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -232,7 +232,6 @@ void V4L2CameraProxy::updateBuffers()
>  			buf.timestamp.tv_usec = fmd.timestamp % 1000000;
>  			buf.sequence = fmd.sequence;
>
> -			buf.flags |= V4L2_BUF_FLAG_DONE;
>  			break;
>  		case FrameMetadata::FrameError:
>  			buf.flags |= V4L2_BUF_FLAG_ERROR;
> @@ -517,6 +516,9 @@ int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)
>  				return ret;
>  		}
>
> +		if (!mmaps_.empty())
> +			return -EBUSY;
> +
>  		unlock(fd);
>  		return freeBuffers();
>  	}
> @@ -616,6 +618,9 @@ int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)
>  	if (arg->index >= bufferCount_)
>  		return -EINVAL;
>
> +	if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED)
> +		return -EINVAL;
> +
>  	int ret = lock(fd);
>  	if (ret < 0)
>  		return ret;
> @@ -629,9 +634,12 @@ int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)
>  	if (ret < 0)
>  		return ret;
>
> -	arg->flags |= V4L2_BUF_FLAG_QUEUED;
> +	arg->flags |= V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_MAPPED;
>  	arg->flags &= ~V4L2_BUF_FLAG_DONE;
>
> +	buffers_[arg->index].flags |= V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_MAPPED;
> +	buffers_[arg->index].flags &= ~V4L2_BUF_FLAG_DONE;
> +
>  	return ret;
>  }
>
> @@ -694,6 +702,9 @@ int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)
>  	if (arg == nullptr)
>  		return -EFAULT;
>
> +	if (bufferCount_ == 0)
> +		return -EINVAL;
> +

It's a bit hard to review them without much context :/

>  	if (streaming_)
>  		return 0;
>
> --
> 2.27.0
>
> _______________________________________________
> 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 e4d534a..fca728a 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -232,7 +232,6 @@  void V4L2CameraProxy::updateBuffers()
 			buf.timestamp.tv_usec = fmd.timestamp % 1000000;
 			buf.sequence = fmd.sequence;
 
-			buf.flags |= V4L2_BUF_FLAG_DONE;
 			break;
 		case FrameMetadata::FrameError:
 			buf.flags |= V4L2_BUF_FLAG_ERROR;
@@ -517,6 +516,9 @@  int V4L2CameraProxy::vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg)
 				return ret;
 		}
 
+		if (!mmaps_.empty())
+			return -EBUSY;
+
 		unlock(fd);
 		return freeBuffers();
 	}
@@ -616,6 +618,9 @@  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)
 	if (arg->index >= bufferCount_)
 		return -EINVAL;
 
+	if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED)
+		return -EINVAL;
+
 	int ret = lock(fd);
 	if (ret < 0)
 		return ret;
@@ -629,9 +634,12 @@  int V4L2CameraProxy::vidioc_qbuf(int fd, struct v4l2_buffer *arg)
 	if (ret < 0)
 		return ret;
 
-	arg->flags |= V4L2_BUF_FLAG_QUEUED;
+	arg->flags |= V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_MAPPED;
 	arg->flags &= ~V4L2_BUF_FLAG_DONE;
 
+	buffers_[arg->index].flags |= V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_MAPPED;
+	buffers_[arg->index].flags &= ~V4L2_BUF_FLAG_DONE;
+
 	return ret;
 }
 
@@ -694,6 +702,9 @@  int V4L2CameraProxy::vidioc_streamon(int fd, int *arg)
 	if (arg == nullptr)
 		return -EFAULT;
 
+	if (bufferCount_ == 0)
+		return -EINVAL;
+
 	if (streaming_)
 		return 0;