[{"id":22005,"web_url":"https://patchwork.libcamera.org/comment/22005/","msgid":"<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>","date":"2022-01-11T19:10:51","subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nThank you for the patch.\n\nOn Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:\n> Add support for PREPARE_BUF as one of the ioctl. Since this is a compat\n> layer, there doesn't seem to be an equivalent to the \"transfer ownership\n> of the buffer to kernel driver\" in V4L2Camera class.\n> \n> To match the error checks done by kernel implementation, we'd have to\n> check if dmabuf fd is valid and that the buffer size is large enough.\n> Doing so will not add any particular value to the program as\n> applications don't depend on these conditions being handled correctly.\n\ns/don't depend/most likely don't depend/ (there can always be corner\ncases, developers sometimes get inventive)\n\n> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> ---\n> Source builds and tests fine, also passes the v4l2-compliance\n> \n> <snip>\n> vedant@pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py ./build/src/v4l2/v4l2-compat.so\n> Testing /dev/video0 with uvcvideo driver... success\n> </snip>\n> \n> Still figuring out how to test this with v4l2-ctl, if you have hints\n> please let me know :)\n\nv4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough as\na test.\n\n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++\n>  src/v4l2/v4l2_camera_proxy.h   |  1 +\n>  2 files changed, 29 insertions(+)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 4d529bc29a4d..8c28e8738a06 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -544,6 +544,33 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a\n>  \treturn 0;\n>  }\n>  \n> +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index = \"\n> +\t\t\t       << arg->index << \" fd = \" << file->efd();\n> +\n> +\tif (!hasOwnership(file))\n> +\t\treturn -EBUSY;\n> +\n> +\tif (arg->index >= bufferCount_)\n> +\t\treturn -EINVAL;\n> +\n\n\tstruct v4l2_buffer &buffer = buffers_[arg->index];\n\nand use buffer below.\n\nWith this change, and v4l2-compliance passing,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\tif (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||\n> +\t    buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||\n> +\t    !validateBufferType(arg->type) ||\n> +\t    !validateMemoryType(arg->memory))\n> +\t\treturn -EINVAL;\n> +\n> +\tbuffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;\n> +\n> +\targ->flags = buffers_[arg->index].flags;\n> +\n> +\treturn 0;\n> +}\n> +\n>  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> @@ -709,6 +736,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n>  \tVIDIOC_S_INPUT,\n>  \tVIDIOC_REQBUFS,\n>  \tVIDIOC_QUERYBUF,\n> +\tVIDIOC_PREPARE_BUF,\n>  \tVIDIOC_QBUF,\n>  \tVIDIOC_DQBUF,\n>  \tVIDIOC_EXPBUF,\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index 14e027c3e7d1..6baba94262a9 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -57,6 +57,7 @@ private:\n>  \tint vidioc_s_input(V4L2CameraFile *file, int *arg);\n>  \tint vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);\n>  \tint vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> +\tint vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n>  \tint vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n>  \tint vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n>  \t\t\t libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);\n> -- \n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 66778BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jan 2022 19:11:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A582560921;\n\tTue, 11 Jan 2022 20:11:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0FF8360217\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 20:11:02 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 63C47340;\n\tTue, 11 Jan 2022 20:11:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sOx6wOKo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641928261;\n\tbh=EWF1Oq8A1lYvzRtytQ8YkwWC5ydedk78aTMorCxm0Hc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sOx6wOKolVoupFlCN8guaUrD1d4dmdqRoWWCUNyX9UhP89a3Y4zWfvL6BQUGP2Fi5\n\t34vHqz1kKWXnVtpz2287pG6ObifH89kvDYakayiwU5w7BUmAnoUxfsP04UsNULSow3\n\tQouo1JsNpkpk06afRArmY1r+nzvVdJXaS/YVlrSY=","Date":"Tue, 11 Jan 2022 21:10:51 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>","References":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22006,"web_url":"https://patchwork.libcamera.org/comment/22006/","msgid":"<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>","date":"2022-01-11T19:21:49","subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 11, 2022 at 09:10:53PM +0200, Laurent Pinchart wrote:\n> Hi Vedant,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:\n> > Add support for PREPARE_BUF as one of the ioctl. Since this is a compat\n> > layer, there doesn't seem to be an equivalent to the \"transfer ownership\n> > of the buffer to kernel driver\" in V4L2Camera class.\n> > \n> > To match the error checks done by kernel implementation, we'd have to\n> > check if dmabuf fd is valid and that the buffer size is large enough.\n> > Doing so will not add any particular value to the program as\n> > applications don't depend on these conditions being handled correctly.\n> \n> s/don't depend/most likely don't depend/ (there can always be corner\n> cases, developers sometimes get inventive)\n> \n> > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > ---\n> > Source builds and tests fine, also passes the v4l2-compliance\n> > \n> > <snip>\n> > vedant@pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py ./build/src/v4l2/v4l2-compat.so\n> > Testing /dev/video0 with uvcvideo driver... success\n> > </snip>\n> > \n> > Still figuring out how to test this with v4l2-ctl, if you have hints\n> > please let me know :)\n> \n> v4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough as\n> a test.\n> \n> > ---\n> > src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++\n> >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> >  2 files changed, 29 insertions(+)\n> > \n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 4d529bc29a4d..8c28e8738a06 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -544,6 +544,33 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a\n> >  \treturn 0;\n> >  }\n> >  \n> > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> > +{\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index = \"\n> > +\t\t\t       << arg->index << \" fd = \" << file->efd();\n> > +\n> > +\tif (!hasOwnership(file))\n> > +\t\treturn -EBUSY;\n> > +\n> > +\tif (arg->index >= bufferCount_)\n> > +\t\treturn -EINVAL;\n> > +\n> \n> \tstruct v4l2_buffer &buffer = buffers_[arg->index];\n> \n> and use buffer below.\n> \n> With this change, and v4l2-compliance passing,\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> > +\tif (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||\n\nActually, this isn't correct, you need to check arg->flags here.\n\n> > +\t    buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tif (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||\n> > +\t    !validateBufferType(arg->type) ||\n> > +\t    !validateMemoryType(arg->memory))\n> > +\t\treturn -EINVAL;\n\nAnd let's group all the flags check:\n\n\tif (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)\n\t\treturn -EINVAL;\n\n\tif (!validateBufferType(arg->type) || !validateMemoryType(arg->memory))\n\t\treturn -EINVAL;\n\n \tstruct v4l2_buffer &buffer = buffers_[arg->index];\n\n\tif (buffer.flags & V4L2_BUF_FLAG_PREPARED ||\n\t    buffer.flags & V4L2_BUF_FLAG_QUEUED ||\n\t\treturn -EINVAL;\n\n> > +\n> > +\tbuffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;\n> > +\n> > +\targ->flags = buffers_[arg->index].flags;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> >  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> >  {\n> >  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > @@ -709,6 +736,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> >  \tVIDIOC_S_INPUT,\n> >  \tVIDIOC_REQBUFS,\n> >  \tVIDIOC_QUERYBUF,\n> > +\tVIDIOC_PREPARE_BUF,\n> >  \tVIDIOC_QBUF,\n> >  \tVIDIOC_DQBUF,\n> >  \tVIDIOC_EXPBUF,\n> > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > index 14e027c3e7d1..6baba94262a9 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.h\n> > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > @@ -57,6 +57,7 @@ private:\n> >  \tint vidioc_s_input(V4L2CameraFile *file, int *arg);\n> >  \tint vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);\n> >  \tint vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > +\tint vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> >  \tint vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> >  \tint vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> >  \t\t\t libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B35A7BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jan 2022 19:22:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E355B604F4;\n\tTue, 11 Jan 2022 20:22:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6586C60217\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 20:22:00 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C5EAC340;\n\tTue, 11 Jan 2022 20:21:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XJA0Y4/F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641928920;\n\tbh=KMrgFdNCOeDwumf+J4dTt1R7fbWKFDn46/Lp0Wiu9do=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XJA0Y4/FSm1EGnkOHIHe5+V3Pgr7PUg/7YCpZiICH4vjUFRekHzr08BO1iY5+HYWk\n\t26x4J4IDrPI/TWq6cbx9ECvHgh97R1Q5JHOCHgYTnJCBhxdUaiiznPPtuw5l7A2mdV\n\te62YEZq2GbzMoFKMi+uOXhiSvbaM5o0fNb0RFMac=","Date":"Tue, 11 Jan 2022 21:21:49 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>","References":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>\n\t<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22007,"web_url":"https://patchwork.libcamera.org/comment/22007/","msgid":"<CACGrz-M=XJ7nPsN3j4oA2z09Q9RXpL50cn93cGsga-BdN5sYEw@mail.gmail.com>","date":"2022-01-11T19:33:23","subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hello Laurent,\n\n\nOn Wed, Jan 12, 2022 at 12:52 AM Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> On Tue, Jan 11, 2022 at 09:10:53PM +0200, Laurent Pinchart wrote:\n> > Hi Vedant,\n> >\n> > Thank you for the patch.\n> >\n> > On Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:\n> > > Add support for PREPARE_BUF as one of the ioctl. Since this is a compat\n> > > layer, there doesn't seem to be an equivalent to the \"transfer ownership\n> > > of the buffer to kernel driver\" in V4L2Camera class.\n> > >\n> > > To match the error checks done by kernel implementation, we'd have to\n> > > check if dmabuf fd is valid and that the buffer size is large enough.\n> > > Doing so will not add any particular value to the program as\n> > > applications don't depend on these conditions being handled correctly.\n> >\n> > s/don't depend/most likely don't depend/ (there can always be corner\n> > cases, developers sometimes get inventive)\n> >\n> > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > ---\n> > > Source builds and tests fine, also passes the v4l2-compliance\n> > >\n> > > <snip>\n> > > vedant@pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py ./build/src/v4l2/v4l2-compat.so\n> > > Testing /dev/video0 with uvcvideo driver... success\n> > > </snip>\n> > >\n> > > Still figuring out how to test this with v4l2-ctl, if you have hints\n> > > please let me know :)\n> >\n> > v4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough as\n> > a test.\n> >\n> > > ---\n> > > src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++\n> > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > >  2 files changed, 29 insertions(+)\n> > >\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index 4d529bc29a4d..8c28e8738a06 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -544,6 +544,33 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a\n> > >     return 0;\n> > >  }\n> > >\n> > > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> > > +{\n> > > +   LOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index = \"\n> > > +                          << arg->index << \" fd = \" << file->efd();\n> > > +\n> > > +   if (!hasOwnership(file))\n> > > +           return -EBUSY;\n> > > +\n> > > +   if (arg->index >= bufferCount_)\n> > > +           return -EINVAL;\n> > > +\n> >\n> >       struct v4l2_buffer &buffer = buffers_[arg->index];\n> >\n> > and use buffer below.\n> >\n> > With this change, and v4l2-compliance passing,\n> >\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >\n> > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||\n>\n> Actually, this isn't correct, you need to check arg->flags here.\n\nI missed this, but I can't figure out the reason why this is incorrect ?\n\n> > > +       buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)\n> > > +           return -EINVAL;\n> > > +\n> > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||\n> > > +       !validateBufferType(arg->type) ||\n> > > +       !validateMemoryType(arg->memory))\n> > > +           return -EINVAL;\n>\n> And let's group all the flags check:\n>\n>         if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)\n>                 return -EINVAL;\n>\n>         if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory))\n>                 return -EINVAL;\n>\n>         struct v4l2_buffer &buffer = buffers_[arg->index];\n>\n>         if (buffer.flags & V4L2_BUF_FLAG_PREPARED ||\n>             buffer.flags & V4L2_BUF_FLAG_QUEUED ||\n>                 return -EINVAL;\n>\n> > > +\n> > > +   buffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;\n> > > +\n> > > +   arg->flags = buffers_[arg->index].flags;\n> > > +\n> > > +   return 0;\n> > > +}\n> > > +\n> > >  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> > >  {\n> > >     LOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > > @@ -709,6 +736,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> > >     VIDIOC_S_INPUT,\n> > >     VIDIOC_REQBUFS,\n> > >     VIDIOC_QUERYBUF,\n> > > +   VIDIOC_PREPARE_BUF,\n> > >     VIDIOC_QBUF,\n> > >     VIDIOC_DQBUF,\n> > >     VIDIOC_EXPBUF,\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > > index 14e027c3e7d1..6baba94262a9 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > @@ -57,6 +57,7 @@ private:\n> > >     int vidioc_s_input(V4L2CameraFile *file, int *arg);\n> > >     int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);\n> > >     int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > +   int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > >     int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > >     int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > >                      libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nRegards,\nVedant Paranjape","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1020EBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jan 2022 19:33:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C64660935;\n\tTue, 11 Jan 2022 20:33:38 +0100 (CET)","from mail-yb1-xb2f.google.com (mail-yb1-xb2f.google.com\n\t[IPv6:2607:f8b0:4864:20::b2f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4021760217\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 20:33:37 +0100 (CET)","by mail-yb1-xb2f.google.com with SMTP id v186so45258ybg.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 11:33:37 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"cioe2NsF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=6IjqC4i8zk8VwYBBsgpkMGVlbyG6rTWQwgjlGtQCpBU=;\n\tb=cioe2NsFxVVWxZb8sNKLy+WXSomOTgivk1tz8uS5nLbK78MkBE8dmPrhvkCpu7zPVv\n\tUd4yXY8eaIqhF0kJ4Jf3bEg21FUZMyVfl66En9AT7rdOeC9KthenBrs4IGyUVC37W5yA\n\tUPB0IhBNyomytBkNgQphHGbvYStlmdE1E7m3yb8lNWooAhWR+qDFrgAmoBHbUobaHVvW\n\tcLFAUr/8pDI60ZtJSdhxl8DTB4JG5hA8k+pYM/ApfBixL4fU+4w5BV4lOgCkBhGM8+9B\n\t/ad7LyIeIwthPQvnHPpxUJqW0Vpab4dAB4sqEUaeIP8Zg4F+dVuC8GYz+kaMubttrxCq\n\t2NrA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=6IjqC4i8zk8VwYBBsgpkMGVlbyG6rTWQwgjlGtQCpBU=;\n\tb=j0AuVtwCA61CLnA5nt4hlPZhN3FK3OgqHUguGcWBnl5AGn/KRyFrykUGSgNERCdwaZ\n\tE7gtoBFsd64aH1HKeEVmDmdkeCM0YXc3Gw2nkBwqhu9R81T+/1RZ/5lgb+w4uRAq9Bvt\n\tWP2Hs4b9uFF5sn2ZL01P+MLmXhKmvC+2s+y88d+wkBTsdJRIPbwJNILEawOMWIqPnQ4l\n\tPOLD/oKi07BQjCgrlhs/hjBY976uZbqSkCiHUcbafN8GcuXEuMUt8ll8fcHWtd7FSrLX\n\teIYjsmRZWvsfur8DtrnshPhCVCnDsogfj/nrNjKlov/9lgDHy5+OgQEj1XaRvCJ+xSO7\n\tXO5g==","X-Gm-Message-State":"AOAM532+NwP8E6Loa2VRNDcyt3IOvb5OMvWF0RRP64Z6aBWt9VnTYg4H\n\t8eizEBK7OkiGFL1/N+rph1GuG+jrUeWnUjQGR34=","X-Google-Smtp-Source":"ABdhPJydnt5/lbGIBoieU4BrNPvERzBz+ABK4okHAEW2glfx3yWPPyN9GtdglDWzVxW3hYstgc+TRX121UMT9MzW+bk=","X-Received":"by 2002:a25:e60a:: with SMTP id d10mr8499115ybh.71.1641929615637;\n\tTue, 11 Jan 2022 11:33:35 -0800 (PST)","MIME-Version":"1.0","References":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>\n\t<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>\n\t<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>","In-Reply-To":"<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>","From":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Date":"Wed, 12 Jan 2022 01:03:23 +0530","Message-ID":"<CACGrz-M=XJ7nPsN3j4oA2z09Q9RXpL50cn93cGsga-BdN5sYEw@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22008,"web_url":"https://patchwork.libcamera.org/comment/22008/","msgid":"<Yd3b+/5IdXwHqh00@pendragon.ideasonboard.com>","date":"2022-01-11T19:35:23","subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Wed, Jan 12, 2022 at 01:03:23AM +0530, Vedant Paranjape wrote:\n> On Wed, Jan 12, 2022 at 12:52 AM Laurent Pinchart wrote:\n> > On Tue, Jan 11, 2022 at 09:10:53PM +0200, Laurent Pinchart wrote:\n> > > On Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:\n> > > > Add support for PREPARE_BUF as one of the ioctl. Since this is a compat\n> > > > layer, there doesn't seem to be an equivalent to the \"transfer ownership\n> > > > of the buffer to kernel driver\" in V4L2Camera class.\n> > > >\n> > > > To match the error checks done by kernel implementation, we'd have to\n> > > > check if dmabuf fd is valid and that the buffer size is large enough.\n> > > > Doing so will not add any particular value to the program as\n> > > > applications don't depend on these conditions being handled correctly.\n> > >\n> > > s/don't depend/most likely don't depend/ (there can always be corner\n> > > cases, developers sometimes get inventive)\n> > >\n> > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > > ---\n> > > > Source builds and tests fine, also passes the v4l2-compliance\n> > > >\n> > > > <snip>\n> > > > vedant@pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py ./build/src/v4l2/v4l2-compat.so\n> > > > Testing /dev/video0 with uvcvideo driver... success\n> > > > </snip>\n> > > >\n> > > > Still figuring out how to test this with v4l2-ctl, if you have hints\n> > > > please let me know :)\n> > >\n> > > v4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough as\n> > > a test.\n> > >\n> > > > ---\n> > > > src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++\n> > > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > > >  2 files changed, 29 insertions(+)\n> > > >\n> > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > index 4d529bc29a4d..8c28e8738a06 100644\n> > > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > @@ -544,6 +544,33 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a\n> > > >     return 0;\n> > > >  }\n> > > >\n> > > > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> > > > +{\n> > > > +   LOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index = \"\n> > > > +                          << arg->index << \" fd = \" << file->efd();\n> > > > +\n> > > > +   if (!hasOwnership(file))\n> > > > +           return -EBUSY;\n> > > > +\n> > > > +   if (arg->index >= bufferCount_)\n> > > > +           return -EINVAL;\n> > > > +\n> > >\n> > >       struct v4l2_buffer &buffer = buffers_[arg->index];\n> > >\n> > > and use buffer below.\n> > >\n> > > With this change, and v4l2-compliance passing,\n> > >\n> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > >\n> > > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||\n> >\n> > Actually, this isn't correct, you need to check arg->flags here.\n> \n> I missed this, but I can't figure out the reason why this is incorrect ?\n\nBecause V4L2_BUF_FLAG_REQUEST_FD is a flag that is passed to\nVIDIOC_PREPARE_BUF, not a flag we set internally.\n\n> > > > +       buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)\n> > > > +           return -EINVAL;\n> > > > +\n> > > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||\n> > > > +       !validateBufferType(arg->type) ||\n> > > > +       !validateMemoryType(arg->memory))\n> > > > +           return -EINVAL;\n> >\n> > And let's group all the flags check:\n> >\n> >         if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)\n> >                 return -EINVAL;\n> >\n> >         if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory))\n> >                 return -EINVAL;\n> >\n> >         struct v4l2_buffer &buffer = buffers_[arg->index];\n> >\n> >         if (buffer.flags & V4L2_BUF_FLAG_PREPARED ||\n> >             buffer.flags & V4L2_BUF_FLAG_QUEUED ||\n> >                 return -EINVAL;\n> >\n> > > > +\n> > > > +   buffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;\n> > > > +\n> > > > +   arg->flags = buffers_[arg->index].flags;\n> > > > +\n> > > > +   return 0;\n> > > > +}\n> > > > +\n> > > >  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)\n> > > >  {\n> > > >     LOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > > > @@ -709,6 +736,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> > > >     VIDIOC_S_INPUT,\n> > > >     VIDIOC_REQBUFS,\n> > > >     VIDIOC_QUERYBUF,\n> > > > +   VIDIOC_PREPARE_BUF,\n> > > >     VIDIOC_QBUF,\n> > > >     VIDIOC_DQBUF,\n> > > >     VIDIOC_EXPBUF,\n> > > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > > > index 14e027c3e7d1..6baba94262a9 100644\n> > > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > > @@ -57,6 +57,7 @@ private:\n> > > >     int vidioc_s_input(V4L2CameraFile *file, int *arg);\n> > > >     int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);\n> > > >     int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > > +   int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > >     int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > >     int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > >                      libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EE597BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jan 2022 19:35:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 41EEC60921;\n\tTue, 11 Jan 2022 20:35:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 80A8660217\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 20:35:33 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D9F68340;\n\tTue, 11 Jan 2022 20:35:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OvR7Ceue\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641929733;\n\tbh=Dlol/ugSBK7YE4WlTg3D0rUqH7qVlLihCkFIUI/33Us=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OvR7Ceue6WB8hEf88kPE8PntSascOKq/fjgFlFJShQ3exQvJid5ddVLUH95eGhBxu\n\tK9KI9K3S0eNER7ohM4MzmD2WkIFG1BY/NzYZRsBpjbsFsmP92mO4YJLdXAvR+DaYHA\n\trb13CXWjBgK+6qf+fkmGG46dWZOn84J4MyoE/jcE=","Date":"Tue, 11 Jan 2022 21:35:23 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<Yd3b+/5IdXwHqh00@pendragon.ideasonboard.com>","References":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>\n\t<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>\n\t<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>\n\t<CACGrz-M=XJ7nPsN3j4oA2z09Q9RXpL50cn93cGsga-BdN5sYEw@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CACGrz-M=XJ7nPsN3j4oA2z09Q9RXpL50cn93cGsga-BdN5sYEw@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22009,"web_url":"https://patchwork.libcamera.org/comment/22009/","msgid":"<CACGrz-OvA_v+MZ40rZmqmZLp9zVanLW6PYwGyN1u6ogrRHWroQ@mail.gmail.com>","date":"2022-01-11T19:42:27","subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Ah, I see.\n\nOn Wed, 12 Jan, 2022, 01:05 Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Vedant,\n>\n> On Wed, Jan 12, 2022 at 01:03:23AM +0530, Vedant Paranjape wrote:\n> > On Wed, Jan 12, 2022 at 12:52 AM Laurent Pinchart wrote:\n> > > On Tue, Jan 11, 2022 at 09:10:53PM +0200, Laurent Pinchart wrote:\n> > > > On Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:\n> > > > > Add support for PREPARE_BUF as one of the ioctl. Since this is a\n> compat\n> > > > > layer, there doesn't seem to be an equivalent to the \"transfer\n> ownership\n> > > > > of the buffer to kernel driver\" in V4L2Camera class.\n> > > > >\n> > > > > To match the error checks done by kernel implementation, we'd have\n> to\n> > > > > check if dmabuf fd is valid and that the buffer size is large\n> enough.\n> > > > > Doing so will not add any particular value to the program as\n> > > > > applications don't depend on these conditions being handled\n> correctly.\n> > > >\n> > > > s/don't depend/most likely don't depend/ (there can always be corner\n> > > > cases, developers sometimes get inventive)\n> > > >\n> > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > > > ---\n> > > > > Source builds and tests fine, also passes the v4l2-compliance\n> > > > >\n> > > > > <snip>\n> > > > > vedant@pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py\n> ./build/src/v4l2/v4l2-compat.so\n> > > > > Testing /dev/video0 with uvcvideo driver... success\n> > > > > </snip>\n> > > > >\n> > > > > Still figuring out how to test this with v4l2-ctl, if you have\n> hints\n> > > > > please let me know :)\n> > > >\n> > > > v4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough\n> as\n> > > > a test.\n> > > >\n> > > > > ---\n> > > > > src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++\n> > > > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > > > >  2 files changed, 29 insertions(+)\n> > > > >\n> > > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > index 4d529bc29a4d..8c28e8738a06 100644\n> > > > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > @@ -544,6 +544,33 @@ int\n> V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a\n> > > > >     return 0;\n> > > > >  }\n> > > > >\n> > > > > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file,\n> struct v4l2_buffer *arg)\n> > > > > +{\n> > > > > +   LOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index\n> = \"\n> > > > > +                          << arg->index << \" fd = \" <<\n> file->efd();\n> > > > > +\n> > > > > +   if (!hasOwnership(file))\n> > > > > +           return -EBUSY;\n> > > > > +\n> > > > > +   if (arg->index >= bufferCount_)\n> > > > > +           return -EINVAL;\n> > > > > +\n> > > >\n> > > >       struct v4l2_buffer &buffer = buffers_[arg->index];\n> > > >\n> > > > and use buffer below.\n> > > >\n> > > > With this change, and v4l2-compliance passing,\n> > > >\n> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > >\n> > > > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||\n> > >\n> > > Actually, this isn't correct, you need to check arg->flags here.\n> >\n> > I missed this, but I can't figure out the reason why this is incorrect ?\n>\n> Because V4L2_BUF_FLAG_REQUEST_FD is a flag that is passed to\n> VIDIOC_PREPARE_BUF, not a flag we set internally.\n>\n> > > > > +       buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)\n> > > > > +           return -EINVAL;\n> > > > > +\n> > > > > +   if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||\n> > > > > +       !validateBufferType(arg->type) ||\n> > > > > +       !validateMemoryType(arg->memory))\n> > > > > +           return -EINVAL;\n> > >\n> > > And let's group all the flags check:\n> > >\n> > >         if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)\n> > >                 return -EINVAL;\n> > >\n> > >         if (!validateBufferType(arg->type) ||\n> !validateMemoryType(arg->memory))\n> > >                 return -EINVAL;\n> > >\n> > >         struct v4l2_buffer &buffer = buffers_[arg->index];\n> > >\n> > >         if (buffer.flags & V4L2_BUF_FLAG_PREPARED ||\n> > >             buffer.flags & V4L2_BUF_FLAG_QUEUED ||\n> > >                 return -EINVAL;\n> > >\n> > > > > +\n> > > > > +   buffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;\n> > > > > +\n> > > > > +   arg->flags = buffers_[arg->index].flags;\n> > > > > +\n> > > > > +   return 0;\n> > > > > +}\n> > > > > +\n> > > > >  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct\n> v4l2_buffer *arg)\n> > > > >  {\n> > > > >     LOG(V4L2Compat, Debug) << \"Servicing vidioc_qbuf, index = \"\n> > > > > @@ -709,6 +736,7 @@ const std::set<unsigned long>\n> V4L2CameraProxy::supportedIoctls_ = {\n> > > > >     VIDIOC_S_INPUT,\n> > > > >     VIDIOC_REQBUFS,\n> > > > >     VIDIOC_QUERYBUF,\n> > > > > +   VIDIOC_PREPARE_BUF,\n> > > > >     VIDIOC_QBUF,\n> > > > >     VIDIOC_DQBUF,\n> > > > >     VIDIOC_EXPBUF,\n> > > > > diff --git a/src/v4l2/v4l2_camera_proxy.h\n> b/src/v4l2/v4l2_camera_proxy.h\n> > > > > index 14e027c3e7d1..6baba94262a9 100644\n> > > > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > > > @@ -57,6 +57,7 @@ private:\n> > > > >     int vidioc_s_input(V4L2CameraFile *file, int *arg);\n> > > > >     int vidioc_reqbufs(V4L2CameraFile *file, struct\n> v4l2_requestbuffers *arg);\n> > > > >     int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer\n> *arg);\n> > > > > +   int vidioc_prepare_buf(V4L2CameraFile *file, struct\n> v4l2_buffer *arg);\n> > > > >     int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > > >     int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > > >                      libcamera::Mutex *lock)\n> LIBCAMERA_TSA_REQUIRES(*lock);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F3C41BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jan 2022 19:42:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3591F604F4;\n\tTue, 11 Jan 2022 20:42:42 +0100 (CET)","from mail-yb1-xb2e.google.com (mail-yb1-xb2e.google.com\n\t[IPv6:2607:f8b0:4864:20::b2e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D9DA60217\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 20:42:40 +0100 (CET)","by mail-yb1-xb2e.google.com with SMTP id m6so40085919ybc.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 11 Jan 2022 11:42:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"dZZdlrl6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Z60Ls+dE7g5mRPV2bnhh6mUDr2dw9mueN01zLtxuAYo=;\n\tb=dZZdlrl6JbgwoiDcNNaww5KDCk/UEt7LCEZZpHHXHFxS0kSTHRfSk3ODtd0HtOnTot\n\t9EyzfTdA1g7n8Msnvqkndyf9QwFWSnLRcH36+3dqHYYHZgrkO8mIFOF4JaxPFyA2BZP/\n\tuNtyqtHZjb0zC6VV9K6DdB77TsNfBrCMr7PUmhiKHoS71lOtex0HMTfyDy3+awqw4O8P\n\tclY+QW71U67xUDgW2ZoVYd/516FkHjpIonNThRB/q5upuT2n4kC7eln4BEUOpv3iQ9b+\n\tpX8OXEXmcYrjY6xZ7sVRT/q/eITIjm+0jUxeQISLqFVd/Aonwtq0AG5XWwSsuinFvlbX\n\ttjnQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Z60Ls+dE7g5mRPV2bnhh6mUDr2dw9mueN01zLtxuAYo=;\n\tb=AuRAYM/J+QzdhWVdpgi7ljMG18sGr1TkKyM8KWGB66ziQUMUc7FKMbw3omBtRHbN+x\n\tkX18Tt5G6cRoFzPO7pYaEsH1H4s9s4SXi/4S8EEpRLMp0V/70ZHmv8MCU6OgUyEMUcnB\n\tiAWKrSfLV7XKDD+UhI3Imi2TjNIZHegLucSR2ecykvMZ+F/IJj5rDXmHJLiXRXUPzq0o\n\tV3oDm9goCIjfPozzwvIJDtUpmo7FOYr3FtCk3Gwl4yZxP79uU3eZP2yEiiwj0+ZhwC8D\n\tVSX4nsvabbaHeYTeSnDniEgeMAvyD2tBYMVpdsiJl2uD8sm4EWg4A5l8l+Wnk2Mnpvp0\n\tsTgQ==","X-Gm-Message-State":"AOAM532H5axRiUwVsE3l240OCWf/+/dmxl7JVtNX8u3bhSIudJ2WAdwW\n\tFSsFY7IoGqHez1V8taZLkurdijymGD0IPp0e5RqwclXE","X-Google-Smtp-Source":"ABdhPJyDKWSDemwauWQ5mMrRIVLRBEDaWWwL+vkjZukNrzysA4oliqYjTHWt48zRQ7KCcTv0nqhG234ZI6utcbk6hN4=","X-Received":"by 2002:a25:15c7:: with SMTP id 190mr7996531ybv.56.1641930158827;\n\tTue, 11 Jan 2022 11:42:38 -0800 (PST)","MIME-Version":"1.0","References":"<20220111163329.363418-1-vedantparanjape160201@gmail.com>\n\t<Yd3WO0UKtbVh5PVl@pendragon.ideasonboard.com>\n\t<Yd3YzT7E6PARjNjf@pendragon.ideasonboard.com>\n\t<CACGrz-M=XJ7nPsN3j4oA2z09Q9RXpL50cn93cGsga-BdN5sYEw@mail.gmail.com>\n\t<Yd3b+/5IdXwHqh00@pendragon.ideasonboard.com>","In-Reply-To":"<Yd3b+/5IdXwHqh00@pendragon.ideasonboard.com>","From":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Date":"Wed, 12 Jan 2022 01:12:27 +0530","Message-ID":"<CACGrz-OvA_v+MZ40rZmqmZLp9zVanLW6PYwGyN1u6ogrRHWroQ@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000dcebd605d553a78c\"","Subject":"Re: [libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support\n\tfor PREPARE_BUF as one of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]