[{"id":22035,"web_url":"https://patchwork.libcamera.org/comment/22035/","msgid":"","date":"2022-01-15T16:27:29","subject":"Re: [libcamera-devel] [PATCH v2] 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 Wed, Jan 12, 2022 at 01:41:02AM +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 most likely don't depend on these conditions being\n> handled correctly.\n> \n> Signed-off-by: Vedant Paranjape \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++++++++++++++++++\n> src/v4l2/v4l2_camera_proxy.h | 1 +\n> 2 files changed, 33 insertions(+)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 4d529bc29a4d..695af5be5c69 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -544,6 +544,37 @@ 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\nWe can replace this with\n\n\tLOG(V4L2Compat, Debug)\n\t\t<< \"[\" << file->description() << \"] \" << __func__\n\t\t<< \"(index=\" << arg->index << \")\";\n\nnow that the V4L2 compat debugging improvements have been merged.\n\n> +\n> +\tif (!hasOwnership(file))\n> +\t\treturn -EBUSY;\n> +\n> +\tif (arg->index >= bufferCount_)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (!validateBufferType(arg->type) ||\n> +\t !validateMemoryType(arg->memory))\n> +\t\treturn -EINVAL;\n> +\n> +\tstruct v4l2_buffer &buffer = buffers_[arg->index];\n> +\n> +\tif (buffer.flags & V4L2_BUF_FLAG_QUEUED ||\n> +\t buffer.flags & V4L2_BUF_FLAG_PREPARED)\n> +\t\treturn -EINVAL;\n> +\n> +\tbuffer.flags |= V4L2_BUF_FLAG_PREPARED;\n\nThis looks good, but there's one missing thing:\nV4L2CameraProxy::vidioc_dqbuf() should now clear the\nV4L2_BUF_FLAG_PREPARED flag. Can you send a v3 that addresses those two\nsmall issues ?\n\n> +\n> +\targ->flags = buffer.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 +740,7 @@ const std::set 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":"","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 B0406BE086\n\tfor ;\n\tSat, 15 Jan 2022 16:27:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA8B1604F5;\n\tSat, 15 Jan 2022 17:27:42 +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 E8A566017B\n\tfor ;\n\tSat, 15 Jan 2022 17:27:41 +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 4CDBDA24;\n\tSat, 15 Jan 2022 17:27:41 +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=\"qfX55siy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1642264061;\n\tbh=/QT/1W+l7EhjIB99JTFheIazBiuib2ItLNVKVH63yGo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qfX55siybVkSTDiLkpEiyp4q18A30BQt0E3OcHWCL2YtMeGZXHOdYBwXy4SwcFvoi\n\tncHTQAWPUnGOvlcjrvJKP2cKoBBx1zvJiMvOQP3lzLe/oix31mKXLcR9eO1oq8139B\n\tmAvEb3BNIuvWhXzjBTVAUb+3iNaBPqRL5Jp2QGLM=","Date":"Sat, 15 Jan 2022 18:27:29 +0200","From":"Laurent Pinchart ","To":"Vedant Paranjape ","Message-ID":"","References":"<20220111201102.58256-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220111201102.58256-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22036,"web_url":"https://patchwork.libcamera.org/comment/22036/","msgid":"","date":"2022-01-15T16:35:44","subject":"Re: [libcamera-devel] [PATCH v2] 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":"Sure, will do that.\n\nOn Sat, 15 Jan, 2022, 21:57 Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Vedant,\n>\n> Thank you for the patch.\n>\n> On Wed, Jan 12, 2022 at 01:41:02AM +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 most likely don't depend on these conditions being\n> > handled correctly.\n> >\n> > Signed-off-by: Vedant Paranjape \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++++++++++++++++++\n> > src/v4l2/v4l2_camera_proxy.h | 1 +\n> > 2 files changed, 33 insertions(+)\n> >\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp\n> b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 4d529bc29a4d..695af5be5c69 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -544,6 +544,37 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile\n> *file, struct v4l2_buffer *a\n> > return 0;\n> > }\n> >\n> > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct\n> v4l2_buffer *arg)\n> > +{\n> > + LOG(V4L2Compat, Debug) << \"Servicing vidioc_prepare_buf, index = \"\n> > + << arg->index << \" fd = \" << file->efd();\n>\n> We can replace this with\n>\n> LOG(V4L2Compat, Debug)\n> << \"[\" << file->description() << \"] \" << __func__\n> << \"(index=\" << arg->index << \")\";\n>\n> now that the V4L2 compat debugging improvements have been merged.\n>\n> > +\n> > + if (!hasOwnership(file))\n> > + return -EBUSY;\n> > +\n> > + if (arg->index >= bufferCount_)\n> > + return -EINVAL;\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_QUEUED ||\n> > + buffer.flags & V4L2_BUF_FLAG_PREPARED)\n> > + return -EINVAL;\n> > +\n> > + buffer.flags |= V4L2_BUF_FLAG_PREPARED;\n>\n> This looks good, but there's one missing thing:\n> V4L2CameraProxy::vidioc_dqbuf() should now clear the\n> V4L2_BUF_FLAG_PREPARED flag. Can you send a v3 that addresses those two\n> small issues ?\n>\n> > +\n> > + arg->flags = buffer.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 +740,7 @@ const std::set\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 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 *arg);\n> > + int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer\n> *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":"","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 A693EBF415\n\tfor ;\n\tSat, 15 Jan 2022 16:35:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 09F04605A8;\n\tSat, 15 Jan 2022 17:35:59 +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 CD2496017B\n\tfor ;\n\tSat, 15 Jan 2022 17:35:56 +0100 (CET)","by mail-yb1-xb2e.google.com with SMTP id m6so32771070ybc.9\n\tfor ;\n\tSat, 15 Jan 2022 08:35:56 -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=\"k3Yce8mX\"; 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=bBQ4JIcQGZ8UxibVnl6lN+FQVigZkUcwmln5urvGs5w=;\n\tb=k3Yce8mXNinvGfA9q7NUGJZi+X2A+okNg344ESaewCKnIBO8P14oII/b0qRYUa9mCF\n\tLByYWK7EAQ3qTgsP00zamqTt004nyqMbmneA5KAQNt+auKf6lXXjHhZ6SAmztft9Yoi+\n\tpr11qtJkDPpXci1ke/97fvklWpujNJDUQ+pMdxElqUFKVvWgXkLR/l1Dr4EqWcb7+Kvc\n\t7CGqxVyo7OTl5E9oMdo9rDAVeGVaY3YVsIi1t/HY8YNzZ3yD8wu3xxjqPlbt1AdkRPr4\n\tPCA8An1MOa9Oo/5P2qPxutRDCLkkUZPa4UPI2uGfUeQVCLsE4l9L4VpsFCiKkkQbtCXK\n\te6Jw==","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=bBQ4JIcQGZ8UxibVnl6lN+FQVigZkUcwmln5urvGs5w=;\n\tb=ag3TFZahrTn4cqyZFsnleA0nWJ/AUsx+M8NDqtH9VSZyv3rgCbIgRpYd8YUYIaLFmC\n\t6Khq7XnQUBfABHD/xUERbMgioiAlSCDcvSRzcOQmfq8Syi/UpwdHGHlnQMZzem/S4WZc\n\tCrLvZUCcTBBDIfXugmf4aP3ZWqvQEW8k3n6nf/lYBoz99EF4HPSHSMbHTf+U5yCTUtqM\n\tcYjUxpSY4w1DJofGftNDL/bN5wEGngpc8yo2gQnLgMlGb/jsXmDRziMRcoxyuKawDw95\n\t3jKCwZyNXqzAne2il01khBjSxlu+nyj6625nYmE/dncgjAx+1HUreSkg0HD5FZGRJ+uY\n\t1OMQ==","X-Gm-Message-State":"AOAM532Un4GDgMGa0aDt94Nr9oXfblqF4+O692jEaVohxC7kOdMvCypP\n\tdtenIsZ+Ulv9JQx9LsTv5AOlDU87pwnWFaH7K5J6xke0","X-Google-Smtp-Source":"ABdhPJz4TAV7wXmQTbUC80i+FUVwitCDlZQ5YWBWdmP3JuMsz1FWGqCa8/w0PuUiRnNdSIud0A6KQQENXaaM+BIRxeE=","X-Received":"by 2002:a5b:14a:: with SMTP id\n\tc10mr19019297ybp.586.1642264555520; \n\tSat, 15 Jan 2022 08:35:55 -0800 (PST)","MIME-Version":"1.0","References":"<20220111201102.58256-1-vedantparanjape160201@gmail.com>\n\t","In-Reply-To":"","From":"Vedant Paranjape ","Date":"Sat, 15 Jan 2022 22:05:44 +0530","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000075850705d5a1835b\"","Subject":"Re: [libcamera-devel] [PATCH v2] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"LibCamera Devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]