[{"id":5241,"web_url":"https://patchwork.libcamera.org/comment/5241/","msgid":"<20200617131535.iodbsstkejt2uabn@uno.localdomain>","date":"2020-06-17T13:15:35","subject":"Re: [libcamera-devel] [PATCH 05/15] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_ENUMINPUT, VIDIOC_G/S_INPUT","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Tue, Jun 16, 2020 at 10:12:34PM +0900, Paul Elder wrote:\n> Implement VIDIOC_ENUMINPUT, VIDIOC_G_INPUT, and VIDIOC_S_INPUT. Only the\n> zeroth input device is supported, and the info returned by enuminput is\n> hardcoded and basic. This is sufficient to pass v4l2-compliance.\n\nI don't think we'll have much other usages for this IOCTLs apart from\nthis one :)\n\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 54 ++++++++++++++++++++++++++++++++++\n> src/v4l2/v4l2_camera_proxy.h | 3 ++\n> 2 files changed, 57 insertions(+)\n>\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index f268f7a..6c0dacc 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -397,6 +397,51 @@ int V4L2CameraProxy::vidioc_s_priority(int fd, enum v4l2_priority *arg)\n> \treturn 0;\n> }\n>\n> +int V4L2CameraProxy::vidioc_enuminput(int fd, struct v4l2_input *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enuminput fd = \" << fd;\n> +\n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n\nThis repeated patter makes me think we should probably centralize this\ncheck as suggested by Laurent on a previous patch\n\n> +\n> +\tif (arg->index != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\tmemset(arg, 0, sizeof(*arg));\n> +\n> +\tutils::strlcpy(reinterpret_cast(arg->name),\n> +\t\t reinterpret_cast(capabilities_.card),\n> +\t\t sizeof(arg->name));\n> +\targ->type = V4L2_INPUT_TYPE_CAMERA;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int V4L2CameraProxy::vidioc_g_input(int fd, int *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_input fd = \" << fd;\n> +\n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n> +\t*arg = 0;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int V4L2CameraProxy::vidioc_s_input(int fd, int *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_input fd = \" << fd;\n> +\n> +\tif (arg == nullptr)\n> +\t\treturn -EFAULT;\n> +\n> +\tif (*arg != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\treturn 0;\n> +}\n> +\n> int V4L2CameraProxy::freeBuffers()\n> {\n> \tLOG(V4L2Compat, Debug) << \"Freeing libcamera bufs\";\n> @@ -649,6 +694,15 @@ int V4L2CameraProxy::ioctl(int fd, unsigned long request, void *arg)\n> \tcase VIDIOC_S_PRIORITY:\n> \t\tret = vidioc_s_priority(fd, static_cast(arg));\n> \t\tbreak;\n> +\tcase VIDIOC_ENUMINPUT:\n> +\t\tret = vidioc_enuminput(fd, static_cast(arg));\n> +\t\tbreak;\n> +\tcase VIDIOC_G_INPUT:\n> +\t\tret = vidioc_g_input(fd, static_cast(arg));\n> +\t\tbreak;\n> +\tcase VIDIOC_S_INPUT:\n> +\t\tret = vidioc_s_input(fd, static_cast(arg));\n> +\t\tbreak;\n> \tcase VIDIOC_REQBUFS:\n> \t\tret = vidioc_reqbufs(fd, static_cast(arg));\n> \t\tbreak;\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index a0c373d..2e90bfd 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -51,6 +51,9 @@ private:\n> \tint vidioc_try_fmt(int fd, struct v4l2_format *arg);\n> \tint vidioc_g_priority(int fd, enum v4l2_priority *arg);\n> \tint vidioc_s_priority(int fd, enum v4l2_priority *arg);\n> +\tint vidioc_enuminput(int fd, struct v4l2_input *arg);\n> +\tint vidioc_g_input(int fd, int *arg);\n> +\tint vidioc_s_input(int fd, int *arg);\n\nLooks good\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \tint vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg);\n> \tint vidioc_querybuf(int fd, struct v4l2_buffer *arg);\n> \tint vidioc_qbuf(int fd, struct v4l2_buffer *arg);\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 83539603BF\n\tfor ;\n\tWed, 17 Jun 2020 15:12:16 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 16C4B20000C;\n\tWed, 17 Jun 2020 13:12:15 +0000 (UTC)"],"Date":"Wed, 17 Jun 2020 15:15:35 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200617131535.iodbsstkejt2uabn@uno.localdomain>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200616131244.70308-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 05/15] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_ENUMINPUT, VIDIOC_G/S_INPUT","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","X-List-Received-Date":"Wed, 17 Jun 2020 13:12:16 -0000"}},{"id":5252,"web_url":"https://patchwork.libcamera.org/comment/5252/","msgid":"<20200617153619.GI5838@pendragon.ideasonboard.com>","date":"2020-06-17T15:36:19","subject":"Re: [libcamera-devel] [PATCH 05/15] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_ENUMINPUT, VIDIOC_G/S_INPUT","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Jun 17, 2020 at 03:15:35PM +0200, Jacopo Mondi wrote:\n> On Tue, Jun 16, 2020 at 10:12:34PM +0900, Paul Elder wrote:\n> > Implement VIDIOC_ENUMINPUT, VIDIOC_G_INPUT, and VIDIOC_S_INPUT. Only the\n> > zeroth input device is supported, and the info returned by enuminput is\n> > hardcoded and basic. This is sufficient to pass v4l2-compliance.\n> \n> I don't think we'll have much other usages for this IOCTLs apart from\n> this one :)\n\nWell... :-) In the future I could imagine S_INPUT being used to select\nbetween different cameras handled by the same pipeline handler instance,\nwhen they're mutually exclusive.\n\n> > Signed-off-by: Paul Elder \n> > ---\n> > src/v4l2/v4l2_camera_proxy.cpp | 54 ++++++++++++++++++++++++++++++++++\n> > src/v4l2/v4l2_camera_proxy.h | 3 ++\n> > 2 files changed, 57 insertions(+)\n> >\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index f268f7a..6c0dacc 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -397,6 +397,51 @@ int V4L2CameraProxy::vidioc_s_priority(int fd, enum v4l2_priority *arg)\n> > \treturn 0;\n> > }\n> >\n> > +int V4L2CameraProxy::vidioc_enuminput(int fd, struct v4l2_input *arg)\n> > +{\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_enuminput fd = \" << fd;\n> > +\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> \n> This repeated patter makes me think we should probably centralize this\n> check as suggested by Laurent on a previous patch\n> \n> > +\n> > +\tif (arg->index != 0)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tmemset(arg, 0, sizeof(*arg));\n> > +\n> > +\tutils::strlcpy(reinterpret_cast(arg->name),\n> > +\t\t reinterpret_cast(capabilities_.card),\n> > +\t\t sizeof(arg->name));\n> > +\targ->type = V4L2_INPUT_TYPE_CAMERA;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int V4L2CameraProxy::vidioc_g_input(int fd, int *arg)\n> > +{\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_g_input fd = \" << fd;\n> > +\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> > +\t*arg = 0;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int V4L2CameraProxy::vidioc_s_input(int fd, int *arg)\n> > +{\n> > +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_s_input fd = \" << fd;\n> > +\n> > +\tif (arg == nullptr)\n> > +\t\treturn -EFAULT;\n> > +\n> > +\tif (*arg != 0)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > int V4L2CameraProxy::freeBuffers()\n> > {\n> > \tLOG(V4L2Compat, Debug) << \"Freeing libcamera bufs\";\n> > @@ -649,6 +694,15 @@ int V4L2CameraProxy::ioctl(int fd, unsigned long request, void *arg)\n> > \tcase VIDIOC_S_PRIORITY:\n> > \t\tret = vidioc_s_priority(fd, static_cast(arg));\n> > \t\tbreak;\n> > +\tcase VIDIOC_ENUMINPUT:\n> > +\t\tret = vidioc_enuminput(fd, static_cast(arg));\n> > +\t\tbreak;\n> > +\tcase VIDIOC_G_INPUT:\n> > +\t\tret = vidioc_g_input(fd, static_cast(arg));\n> > +\t\tbreak;\n> > +\tcase VIDIOC_S_INPUT:\n> > +\t\tret = vidioc_s_input(fd, static_cast(arg));\n> > +\t\tbreak;\n> > \tcase VIDIOC_REQBUFS:\n> > \t\tret = vidioc_reqbufs(fd, static_cast(arg));\n> > \t\tbreak;\n> > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > index a0c373d..2e90bfd 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.h\n> > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > @@ -51,6 +51,9 @@ private:\n> > \tint vidioc_try_fmt(int fd, struct v4l2_format *arg);\n> > \tint vidioc_g_priority(int fd, enum v4l2_priority *arg);\n> > \tint vidioc_s_priority(int fd, enum v4l2_priority *arg);\n> > +\tint vidioc_enuminput(int fd, struct v4l2_input *arg);\n> > +\tint vidioc_g_input(int fd, int *arg);\n> > +\tint vidioc_s_input(int fd, int *arg);\n> \n> Looks good\n> Reviewed-by: Jacopo Mondi \n\nLikewise,\n\nReviewed-by: Laurent Pinchart \n\n> > \tint vidioc_reqbufs(int fd, struct v4l2_requestbuffers *arg);\n> > \tint vidioc_querybuf(int fd, struct v4l2_buffer *arg);\n> > \tint vidioc_qbuf(int fd, struct v4l2_buffer *arg);","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D858A603BF\n\tfor ;\n\tWed, 17 Jun 2020 17:36:42 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 50075F9;\n\tWed, 17 Jun 2020 17:36:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nMUEpRO1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592408202;\n\tbh=s9BgcLx8FC7STExTzki3FRnPgmwKZKHAclxlw09ri5A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nMUEpRO1LCr2CzwnaC0MyptyqPvpkB7MZjU6//aIfmRVx14ZLJ2TWrweTaQIM576e\n\tzPCNCq7EO3M8g11y0JPWlde6nmiRIycko43TVCpLXjIElrpmlA2SaYdDPmh2xicAGg\n\tV31Lc58IBA1ZpCPbOOSIuw54m9NvXmcp+dcH8Zsw=","Date":"Wed, 17 Jun 2020 18:36:19 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20200617153619.GI5838@pendragon.ideasonboard.com>","References":"<20200616131244.70308-1-paul.elder@ideasonboard.com>\n\t<20200616131244.70308-6-paul.elder@ideasonboard.com>\n\t<20200617131535.iodbsstkejt2uabn@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200617131535.iodbsstkejt2uabn@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 05/15] v4l2: v4l2_camera_proxy:\n\tImplement VIDIOC_ENUMINPUT, VIDIOC_G/S_INPUT","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","X-List-Received-Date":"Wed, 17 Jun 2020 15:36:43 -0000"}}]