| Message ID | 20200623190836.53446-22-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Jun 24, 2020 at 04:08:35AM +0900, Paul Elder wrote: > Fix buffer flags related to queueing and dequeueing: > - don't allow a buffer with the same index that is already in the queue > to be enqueued again > - don't set the done flag on dequeueing in > V4L2CameraProxy::updateBuffers() > - set the flags in V4L2CameraProxy's internal buffers, and not just in > the buffers returned from qbuf > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v3: > - remove "set the mapped flag on enqueueing" > - clean up some logic > - expand commit message very slightly > > New in v2 > - split from "Fix v4l2-compliance streaming tests" > --- > src/v4l2/v4l2_camera_proxy.cpp | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index ce0d755..ed3bcbc 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -219,7 +219,6 @@ void V4L2CameraProxy::updateBuffers() > buf.timestamp.tv_usec = fmd.timestamp % 1000000; > buf.sequence = fmd.sequence; > > - buf.flags |= V4L2_BUF_FLAG_DONE; I think this hunk is incorrect, V4L2_BUF_FLAG_DONE isn't set anywhere anymore. The rest looks good, with this line restored, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > break; > case FrameMetadata::FrameError: > buf.flags |= V4L2_BUF_FLAG_ERROR; > @@ -570,6 +569,9 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > if (arg->index >= bufferCount_) > return -EINVAL; > > + if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED) > + return -EINVAL; > + > if (!hasOwnership(file)) > return -EBUSY; > > @@ -582,8 +584,10 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > if (ret < 0) > return ret; > > - arg->flags |= V4L2_BUF_FLAG_QUEUED; > - arg->flags &= ~V4L2_BUF_FLAG_DONE; > + buffers_[arg->index].flags |= V4L2_BUF_FLAG_QUEUED; > + buffers_[arg->index].flags &= ~V4L2_BUF_FLAG_DONE; > + > + arg->flags = buffers_[arg->index].flags; > > return ret; > }
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index ce0d755..ed3bcbc 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -219,7 +219,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; @@ -570,6 +569,9 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) if (arg->index >= bufferCount_) return -EINVAL; + if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED) + return -EINVAL; + if (!hasOwnership(file)) return -EBUSY; @@ -582,8 +584,10 @@ int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) if (ret < 0) return ret; - arg->flags |= V4L2_BUF_FLAG_QUEUED; - arg->flags &= ~V4L2_BUF_FLAG_DONE; + buffers_[arg->index].flags |= V4L2_BUF_FLAG_QUEUED; + buffers_[arg->index].flags &= ~V4L2_BUF_FLAG_DONE; + + arg->flags = buffers_[arg->index].flags; return ret; }
Fix buffer flags related to queueing and dequeueing: - don't allow a buffer with the same index that is already in the queue to be enqueued again - don't set the done flag on dequeueing in V4L2CameraProxy::updateBuffers() - set the flags in V4L2CameraProxy's internal buffers, and not just in the buffers returned from qbuf Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v3: - remove "set the mapped flag on enqueueing" - clean up some logic - expand commit message very slightly New in v2 - split from "Fix v4l2-compliance streaming tests" --- src/v4l2/v4l2_camera_proxy.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)