[{"id":2895,"web_url":"https://patchwork.libcamera.org/comment/2895/","msgid":"<20191015000743.GA5976@bigcity.dyn.berto.se>","date":"2019-10-15T00:07:43","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-10-14 02:27:47 +0300, Laurent Pinchart wrote:\n> We don't support V4L2 compound controls, the size field is thus unused.\n> Remove it to ease merging of the libcamera and V4L2 control info\n> classes. Support for array controls can then be added later on top, and\n> would be useful for libcamera controls too.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/include/v4l2_controls.h | 4 ----\n> src/libcamera/v4l2_controls.cpp | 8 --------\n> src/libcamera/v4l2_device.cpp | 14 ++++++++++++--\n> 3 files changed, 12 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 89cc74485e6f..133f5262febf 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -31,14 +31,10 @@ public:\n> \tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> \n> \tconst ControlId &id() const { return id_; }\n> -\tsize_t size() const { return size_; }\n> -\n> \tconst ControlRange &range() const { return range_; }\n> \n> private:\n> \tV4L2ControlId id_;\n> -\tsize_t size_;\n> -\n> \tControlRange range_;\n> };\n> \n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index c45d3fda2e1f..dcf31b7a8f26 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -127,8 +127,6 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> \t: id_(ctrl)\n> {\n> -\tsize_ = ctrl.elem_size * ctrl.elems;\n> -\n> \tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> \t\trange_ = ControlRange(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum));\n> @@ -143,12 +141,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> * \\return The V4L2 control ID\n> */\n> \n> -/**\n> - * \\fn V4L2ControlInfo::size()\n> - * \\brief Retrieve the control value data size (in bytes)\n> - * \\return The V4L2 control value data size\n> - */\n> -\n> /**\n> * \\fn V4L2ControlInfo::range()\n> * \\brief Retrieve the control value range\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index b47ba448f354..54cc214ecce9 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -8,11 +8,13 @@\n> #include \"v4l2_device.h\"\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> \n> #include \"log.h\"\n> +#include \"utils.h\"\n> #include \"v4l2_controls.h\"\n> \n> /**\n> @@ -360,6 +362,13 @@ void V4L2Device::listControls()\n> \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> \t\t\tcontinue;\n> \n> +\t\tif (ctrl.elems != 1 || ctrl.nr_of_dims) {\n> +\t\t\tLOG(V4L2, Debug)\n> +\t\t\t\t<< \"Array control \" << utils::hex(ctrl.id)\n> +\t\t\t\t<< \" not supported\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tswitch (ctrl.type) {\n> \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> @@ -371,8 +380,9 @@ void V4L2Device::listControls()\n> \t\t\tbreak;\n> \t\t/* \\todo Support compound controls. */\n> \t\tdefault:\n> -\t\t\tLOG(V4L2, Debug) << \"Control type '\" << ctrl.type\n> -\t\t\t\t\t << \"' not supported\";\n> +\t\t\tLOG(V4L2, Debug)\n> +\t\t\t\t<< \"Control \" << utils::hex(ctrl.id)\n> +\t\t\t\t<< \" has unsupported type \" << ctrl.type;\n> \t\t\tcontinue;\n\nThis looks like something for a separate patch ;-)\n\nWith or without this fixed,\n\nReviewed-by: Niklas Söderlund \n\n> \t\t}\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\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 mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3E50600F9\n\tfor ;\n\tTue, 15 Oct 2019 02:07:45 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id 7so18306349ljw.7\n\tfor ;\n\tMon, 14 Oct 2019 17:07:45 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tw13sm237057lfe.84.2019.10.14.17.07.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 14 Oct 2019 17:07:44 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=xbKDgd8F3KbK0o7lc4CSq5+NnDwkAgAygiJ2MZZvEow=;\n\tb=x1o03OEOGs/ep8Uz+sZhtEryLPS70zeCEzUPDIfKqnsY5uKG5ONEGovOkd3MjrALjn\n\tz3mnnBcKfUxSf1PGg8s4r7+JQPAp8pa5n8Dm5HDnUykhvaDvdzsjx19FJT80bl5jyhzs\n\tOFHmjoN6W0GMAj81lSFOXd0UTOHsqwC6ivJGZAjsS701vuewP+/vDn7TuPMYuFsAjcEX\n\thFiicO/G2ffSv43Wfhppc6ZSPg+csegGVj48A4bArtMZRLat+JlJUTyHTIpziJ/H3SD9\n\tdwnLUN5oOhhwiNk78o43NLgWhCeA7nLZUhlKtauOqnAjqb4flC+TW02m5jYXAcBEEbbu\n\ttWFw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=xbKDgd8F3KbK0o7lc4CSq5+NnDwkAgAygiJ2MZZvEow=;\n\tb=pWQsprVJVIGfXC2A6VTNhZN/lglFWY/L4hBDYpe5EAcSKRQsSvS6FDIt8RYH1YkEWs\n\thZ7V8UQx5/4aEInAtbqiBrsSgSJdyahRl3grrgBO6KI3iMKKqh5GCxshUCrVsSgxjtON\n\tdLjM3dk0MQmi/s/Pz9UNAlHu0/BrmzDwmtZF7RIhmfugoT0kSGcZmXIUHyYyiskconQw\n\tCAs9EOcODKGvCGhPxWQECD5EyP6F9b1o7ziOfLvYU88NrUobU+7aJi3f49DOkat9/7Oz\n\tbLW+uiPYwvtmmCoaUmS5eFozcWMYzDtzhw8SZh0HVzgCcxZsN5pNyBl0NlRSIyhjuTT3\n\ta8Yg==","X-Gm-Message-State":"APjAAAUhMweloLueacB9ABh6Yi6y6cnRHDidpgDSSWck5ZXdvWgFjFRr\n\tvmdk1Uw8QNCSmxZhaXDNGR3SX528uvo=","X-Google-Smtp-Source":"APXvYqyQoYNrcM14I0FOB6X4uU4cX3ur8UEViMCjBrjY7UIHLT+CeQ0OiJ/8MPHxHBVZ1CFLMhswBw==","X-Received":"by 2002:a2e:82cd:: with SMTP id\n\tn13mr20394961ljh.156.1571098064859; \n\tMon, 14 Oct 2019 17:07:44 -0700 (PDT)","Date":"Tue, 15 Oct 2019 02:07:43 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015000743.GA5976@bigcity.dyn.berto.se>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","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":"Tue, 15 Oct 2019 00:07:46 -0000"}},{"id":2910,"web_url":"https://patchwork.libcamera.org/comment/2910/","msgid":"<20191015145054.izs2kx3hjcuuzdan@uno.localdomain>","date":"2019-10-15T14:50:54","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Oct 14, 2019 at 02:27:47AM +0300, Laurent Pinchart wrote:\n> We don't support V4L2 compound controls, the size field is thus unused.\n> Remove it to ease merging of the libcamera and V4L2 control info\n> classes. Support for array controls can then be added later on top, and\n> would be useful for libcamera controls too.\n\nI understand for now it's trivial to get the size of the data from the\ntype, but we'll need this soon so I'm not sure it is worth removing\nthis.\n\nI would rather keep it and at the end of this series rework move it to\nControlId. For libcamera controls the size would be the size of the\ntype (unless otherwise specified in the Control<> definition). For\nV4L2 controls the size comes from the kernel, and V4L2ControlId is\nexactly contructed from a v4l2_query_ext_ctrl.\n\nWhat do you think ?\n\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/include/v4l2_controls.h | 4 ----\n> src/libcamera/v4l2_controls.cpp | 8 --------\n> src/libcamera/v4l2_device.cpp | 14 ++++++++++++--\n> 3 files changed, 12 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 89cc74485e6f..133f5262febf 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -31,14 +31,10 @@ public:\n> \tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n>\n> \tconst ControlId &id() const { return id_; }\n> -\tsize_t size() const { return size_; }\n> -\n> \tconst ControlRange &range() const { return range_; }\n>\n> private:\n> \tV4L2ControlId id_;\n> -\tsize_t size_;\n> -\n> \tControlRange range_;\n> };\n>\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index c45d3fda2e1f..dcf31b7a8f26 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -127,8 +127,6 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> \t: id_(ctrl)\n> {\n> -\tsize_ = ctrl.elem_size * ctrl.elems;\n> -\n> \tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> \t\trange_ = ControlRange(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum));\n> @@ -143,12 +141,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> * \\return The V4L2 control ID\n> */\n>\n> -/**\n> - * \\fn V4L2ControlInfo::size()\n> - * \\brief Retrieve the control value data size (in bytes)\n> - * \\return The V4L2 control value data size\n> - */\n> -\n> /**\n> * \\fn V4L2ControlInfo::range()\n> * \\brief Retrieve the control value range\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index b47ba448f354..54cc214ecce9 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -8,11 +8,13 @@\n> #include \"v4l2_device.h\"\n>\n> #include \n> +#include \n> #include \n> #include \n> #include \n>\n> #include \"log.h\"\n> +#include \"utils.h\"\n> #include \"v4l2_controls.h\"\n>\n> /**\n> @@ -360,6 +362,13 @@ void V4L2Device::listControls()\n> \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> \t\t\tcontinue;\n>\n> +\t\tif (ctrl.elems != 1 || ctrl.nr_of_dims) {\n> +\t\t\tLOG(V4L2, Debug)\n> +\t\t\t\t<< \"Array control \" << utils::hex(ctrl.id)\n> +\t\t\t\t<< \" not supported\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tswitch (ctrl.type) {\n> \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> @@ -371,8 +380,9 @@ void V4L2Device::listControls()\n> \t\t\tbreak;\n> \t\t/* \\todo Support compound controls. */\n> \t\tdefault:\n> -\t\t\tLOG(V4L2, Debug) << \"Control type '\" << ctrl.type\n> -\t\t\t\t\t << \"' not supported\";\n> +\t\t\tLOG(V4L2, Debug)\n> +\t\t\t\t<< \"Control \" << utils::hex(ctrl.id)\n> +\t\t\t\t<< \" has unsupported type \" << ctrl.type;\n> \t\t\tcontinue;\n> \t\t}\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB7DD61562\n\tfor ;\n\tTue, 15 Oct 2019 16:49:06 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 79DF32000F;\n\tTue, 15 Oct 2019 14:49:06 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 15 Oct 2019 16:50:54 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015145054.izs2kx3hjcuuzdan@uno.localdomain>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"zlvwdbgfoeko5bwp\"","Content-Disposition":"inline","In-Reply-To":"<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","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":"Tue, 15 Oct 2019 14:49:07 -0000"}},{"id":2913,"web_url":"https://patchwork.libcamera.org/comment/2913/","msgid":"<20191015145456.GD4875@pendragon.ideasonboard.com>","date":"2019-10-15T14:54:56","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Oct 15, 2019 at 04:50:54PM +0200, Jacopo Mondi wrote:\n> On Mon, Oct 14, 2019 at 02:27:47AM +0300, Laurent Pinchart wrote:\n> > We don't support V4L2 compound controls, the size field is thus unused.\n> > Remove it to ease merging of the libcamera and V4L2 control info\n> > classes. Support for array controls can then be added later on top, and\n> > would be useful for libcamera controls too.\n> \n> I understand for now it's trivial to get the size of the data from the\n> type, but we'll need this soon so I'm not sure it is worth removing\n> this.\n> \n> I would rather keep it and at the end of this series rework move it to\n> ControlId. For libcamera controls the size would be the size of the\n> type (unless otherwise specified in the Control<> definition). For\n> V4L2 controls the size comes from the kernel, and V4L2ControlId is\n> exactly contructed from a v4l2_query_ext_ctrl.\n> \n> What do you think ?\n\nI would prefer adding it back on top :-) Beside the obvious reason that\nit would be easier to avoid a complete rework (but that's hardly an\nexcuse for not doing it), I think it would be best to add the size field\nback when needed, when we will know what we need it for. Currently it\nstores the size of a compound V4L2 control, but we don't support those,\nand generalising it to ControlId will require us to think about how to\nsupport compound (or at least array) controls for libcamera controls\ntoo. I would rather not go through that design phase as a dependency for\nthis series.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/include/v4l2_controls.h | 4 ----\n> > src/libcamera/v4l2_controls.cpp | 8 --------\n> > src/libcamera/v4l2_device.cpp | 14 ++++++++++++--\n> > 3 files changed, 12 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index 89cc74485e6f..133f5262febf 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -31,14 +31,10 @@ public:\n> > \tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> >\n> > \tconst ControlId &id() const { return id_; }\n> > -\tsize_t size() const { return size_; }\n> > -\n> > \tconst ControlRange &range() const { return range_; }\n> >\n> > private:\n> > \tV4L2ControlId id_;\n> > -\tsize_t size_;\n> > -\n> > \tControlRange range_;\n> > };\n> >\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index c45d3fda2e1f..dcf31b7a8f26 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -127,8 +127,6 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> > V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > \t: id_(ctrl)\n> > {\n> > -\tsize_ = ctrl.elem_size * ctrl.elems;\n> > -\n> > \tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> > \t\trange_ = ControlRange(static_cast(ctrl.minimum),\n> > \t\t\t\t static_cast(ctrl.maximum));\n> > @@ -143,12 +141,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > * \\return The V4L2 control ID\n> > */\n> >\n> > -/**\n> > - * \\fn V4L2ControlInfo::size()\n> > - * \\brief Retrieve the control value data size (in bytes)\n> > - * \\return The V4L2 control value data size\n> > - */\n> > -\n> > /**\n> > * \\fn V4L2ControlInfo::range()\n> > * \\brief Retrieve the control value range\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index b47ba448f354..54cc214ecce9 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -8,11 +8,13 @@\n> > #include \"v4l2_device.h\"\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> >\n> > #include \"log.h\"\n> > +#include \"utils.h\"\n> > #include \"v4l2_controls.h\"\n> >\n> > /**\n> > @@ -360,6 +362,13 @@ void V4L2Device::listControls()\n> > \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > \t\t\tcontinue;\n> >\n> > +\t\tif (ctrl.elems != 1 || ctrl.nr_of_dims) {\n> > +\t\t\tLOG(V4L2, Debug)\n> > +\t\t\t\t<< \"Array control \" << utils::hex(ctrl.id)\n> > +\t\t\t\t<< \" not supported\";\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > \t\tswitch (ctrl.type) {\n> > \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > @@ -371,8 +380,9 @@ void V4L2Device::listControls()\n> > \t\t\tbreak;\n> > \t\t/* \\todo Support compound controls. */\n> > \t\tdefault:\n> > -\t\t\tLOG(V4L2, Debug) << \"Control type '\" << ctrl.type\n> > -\t\t\t\t\t << \"' not supported\";\n> > +\t\t\tLOG(V4L2, Debug)\n> > +\t\t\t\t<< \"Control \" << utils::hex(ctrl.id)\n> > +\t\t\t\t<< \" has unsupported type \" << ctrl.type;\n> > \t\t\tcontinue;\n> > \t\t}\n> >","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 9525661562\n\tfor ;\n\tTue, 15 Oct 2019 16:54:59 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FB09324;\n\tTue, 15 Oct 2019 16:54:59 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1571151299;\n\tbh=Pr291OJxOuHymdk31iQ1BIueb0FwVJuL2XC210y1qQo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ltj6wTDoSDRQ0u69FaCayxVeOl+xi45Dj8Yxf4cI5BUoyBVQ8sFyDDGW2nfnarao1\n\tGt0Kj/FRo+fWug2/dil6pNDKnVKKxtFJTZpW9/exI3UDj/v69m7bVIEt9l9mPGowiL\n\t8+AYep7HcMTLSkiBb/NqlzYjYCGWdkAAF8LT1+jU=","Date":"Tue, 15 Oct 2019 17:54:56 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015145456.GD4875@pendragon.ideasonboard.com>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>\n\t<20191015145054.izs2kx3hjcuuzdan@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191015145054.izs2kx3hjcuuzdan@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","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":"Tue, 15 Oct 2019 14:54:59 -0000"}},{"id":2914,"web_url":"https://patchwork.libcamera.org/comment/2914/","msgid":"<20191015150215.k22hwmgq4wy3skua@uno.localdomain>","date":"2019-10-15T15:02:15","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Oct 15, 2019 at 05:54:56PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Tue, Oct 15, 2019 at 04:50:54PM +0200, Jacopo Mondi wrote:\n> > On Mon, Oct 14, 2019 at 02:27:47AM +0300, Laurent Pinchart wrote:\n> > > We don't support V4L2 compound controls, the size field is thus unused.\n> > > Remove it to ease merging of the libcamera and V4L2 control info\n> > > classes. Support for array controls can then be added later on top, and\n> > > would be useful for libcamera controls too.\n> >\n> > I understand for now it's trivial to get the size of the data from the\n> > type, but we'll need this soon so I'm not sure it is worth removing\n> > this.\n> >\n> > I would rather keep it and at the end of this series rework move it to\n> > ControlId. For libcamera controls the size would be the size of the\n> > type (unless otherwise specified in the Control<> definition). For\n> > V4L2 controls the size comes from the kernel, and V4L2ControlId is\n> > exactly contructed from a v4l2_query_ext_ctrl.\n> >\n> > What do you think ?\n>\n> I would prefer adding it back on top :-) Beside the obvious reason that\n> it would be easier to avoid a complete rework (but that's hardly an\n> excuse for not doing it), I think it would be best to add the size field\n> back when needed, when we will know what we need it for. Currently it\n> stores the size of a compound V4L2 control, but we don't support those,\n> and generalising it to ControlId will require us to think about how to\n> support compound (or at least array) controls for libcamera controls\n> too. I would rather not go through that design phase as a dependency for\n> this series.\n>\n\nMakes sense, so please have my tag on this one\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/include/v4l2_controls.h | 4 ----\n> > > src/libcamera/v4l2_controls.cpp | 8 --------\n> > > src/libcamera/v4l2_device.cpp | 14 ++++++++++++--\n> > > 3 files changed, 12 insertions(+), 14 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > > index 89cc74485e6f..133f5262febf 100644\n> > > --- a/src/libcamera/include/v4l2_controls.h\n> > > +++ b/src/libcamera/include/v4l2_controls.h\n> > > @@ -31,14 +31,10 @@ public:\n> > > \tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> > >\n> > > \tconst ControlId &id() const { return id_; }\n> > > -\tsize_t size() const { return size_; }\n> > > -\n> > > \tconst ControlRange &range() const { return range_; }\n> > >\n> > > private:\n> > > \tV4L2ControlId id_;\n> > > -\tsize_t size_;\n> > > -\n> > > \tControlRange range_;\n> > > };\n> > >\n> > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > > index c45d3fda2e1f..dcf31b7a8f26 100644\n> > > --- a/src/libcamera/v4l2_controls.cpp\n> > > +++ b/src/libcamera/v4l2_controls.cpp\n> > > @@ -127,8 +127,6 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> > > V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > > \t: id_(ctrl)\n> > > {\n> > > -\tsize_ = ctrl.elem_size * ctrl.elems;\n> > > -\n> > > \tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> > > \t\trange_ = ControlRange(static_cast(ctrl.minimum),\n> > > \t\t\t\t static_cast(ctrl.maximum));\n> > > @@ -143,12 +141,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > > * \\return The V4L2 control ID\n> > > */\n> > >\n> > > -/**\n> > > - * \\fn V4L2ControlInfo::size()\n> > > - * \\brief Retrieve the control value data size (in bytes)\n> > > - * \\return The V4L2 control value data size\n> > > - */\n> > > -\n> > > /**\n> > > * \\fn V4L2ControlInfo::range()\n> > > * \\brief Retrieve the control value range\n> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > index b47ba448f354..54cc214ecce9 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -8,11 +8,13 @@\n> > > #include \"v4l2_device.h\"\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > >\n> > > #include \"log.h\"\n> > > +#include \"utils.h\"\n> > > #include \"v4l2_controls.h\"\n> > >\n> > > /**\n> > > @@ -360,6 +362,13 @@ void V4L2Device::listControls()\n> > > \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > > \t\t\tcontinue;\n> > >\n> > > +\t\tif (ctrl.elems != 1 || ctrl.nr_of_dims) {\n> > > +\t\t\tLOG(V4L2, Debug)\n> > > +\t\t\t\t<< \"Array control \" << utils::hex(ctrl.id)\n> > > +\t\t\t\t<< \" not supported\";\n> > > +\t\t\tcontinue;\n> > > +\t\t}\n> > > +\n> > > \t\tswitch (ctrl.type) {\n> > > \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > > \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > > @@ -371,8 +380,9 @@ void V4L2Device::listControls()\n> > > \t\t\tbreak;\n> > > \t\t/* \\todo Support compound controls. */\n> > > \t\tdefault:\n> > > -\t\t\tLOG(V4L2, Debug) << \"Control type '\" << ctrl.type\n> > > -\t\t\t\t\t << \"' not supported\";\n> > > +\t\t\tLOG(V4L2, Debug)\n> > > +\t\t\t\t<< \"Control \" << utils::hex(ctrl.id)\n> > > +\t\t\t\t<< \" has unsupported type \" << ctrl.type;\n> > > \t\t\tcontinue;\n> > > \t\t}\n> > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 40E9961562\n\tfor ;\n\tTue, 15 Oct 2019 17:00:27 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id D1907240007;\n\tTue, 15 Oct 2019 15:00:26 +0000 (UTC)"],"Date":"Tue, 15 Oct 2019 17:02:15 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015150215.k22hwmgq4wy3skua@uno.localdomain>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-2-laurent.pinchart@ideasonboard.com>\n\t<20191015145054.izs2kx3hjcuuzdan@uno.localdomain>\n\t<20191015145456.GD4875@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"jxsp5wmh6u2kg7pi\"","Content-Disposition":"inline","In-Reply-To":"<20191015145456.GD4875@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: v4l2_controls:\n\tRemove V4L2ControlInfo::size()","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":"Tue, 15 Oct 2019 15:00:27 -0000"}}]