[{"id":26043,"web_url":"https://patchwork.libcamera.org/comment/26043/","msgid":"<20221212095058.x2olcq6xaixmewkm@uno.localdomain>","date":"2022-12-12T09:50:58","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush\n\nOn Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add a read-only flag to the ControlInfo flag to indicate whether a V4L2 control\n> is read-only or not. This flag only makes sense for V2L2 controls at preset, so\n> only update the ControlInfo constructor signature used by the V4L2Device class\n> to set the value of the flag.\n>\n\nIt feels a bit the three constructors are mis-aligned now :/\n\nI would have gone for a default parameter for all three of them to be\nhonest... What do others think ?\n\n> Add a ControlInfo::readOnly() member function to retrive this flag.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/controls.h | 5 ++++-\n> src/libcamera/controls.cpp | 17 +++++++++++++----\n> src/libcamera/v4l2_device.cpp | 12 ++++++++----\n> 3 files changed, 25 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index cf94205577a5..488663a7ba04 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -270,7 +270,8 @@ class ControlInfo\n> public:\n> \texplicit ControlInfo(const ControlValue &min = {},\n> \t\t\t const ControlValue &max = {},\n> -\t\t\t const ControlValue &def = {});\n> +\t\t\t const ControlValue &def = {},\n> +\t\t\t bool readOnly = false);\n> \texplicit ControlInfo(Span values,\n> \t\t\t const ControlValue &def = {});\n> \texplicit ControlInfo(std::set values, bool def);\n> @@ -279,6 +280,7 @@ public:\n> \tconst ControlValue &min() const { return min_; }\n> \tconst ControlValue &max() const { return max_; }\n> \tconst ControlValue &def() const { return def_; }\n> +\tbool readOnly() const { return readOnly_; }\n> \tconst std::vector &values() const { return values_; }\n>\n> \tstd::string toString() const;\n> @@ -297,6 +299,7 @@ private:\n> \tControlValue min_;\n> \tControlValue max_;\n> \tControlValue def_;\n> +\tbool readOnly_;\n> \tstd::vector values_;\n> };\n>\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 6dbf9b348709..fc66abad600d 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * \\param[in] min The control minimum value\n> * \\param[in] max The control maximum value\n> * \\param[in] def The control default value\n> + * \\param[in] readOnly Read-only status of a V4L2 control\n> */\n> ControlInfo::ControlInfo(const ControlValue &min,\n> \t\t\t const ControlValue &max,\n> -\t\t\t const ControlValue &def)\n> -\t: min_(min), max_(max), def_(def)\n> +\t\t\t const ControlValue &def,\n> +\t\t\t bool readOnly)\n> +\t: min_(min), max_(max), def_(def), readOnly_(readOnly)\n> {\n> }\n>\n> @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span values,\n> * default value is \\a def.\n> */\n> ControlInfo::ControlInfo(std::set values, bool def)\n> -\t: min_(false), max_(true), def_(def), values_({ false, true })\n> +\t: min_(false), max_(true), def_(def), readOnly_(false),\n> +\t values_({ false, true })\n> {\n> \tASSERT(values.count(def) && values.size() == 2);\n> }\n> @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set values, bool def)\n> * value. The minimum, maximum, and default values will all be \\a value.\n> */\n> ControlInfo::ControlInfo(bool value)\n> -\t: min_(value), max_(value), def_(value)\n> +\t: min_(value), max_(value), def_(value), readOnly_(false)\n> {\n> \tvalues_ = { value };\n> }\n> @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)\n> * \\return A ControlValue with the default value for the control\n> */\n>\n> +/**\n> + * \\fn ControlInfo::readOnly()\n> + * \\brief Identifies if a V4L2 control is flagged as read-only\n> + * \\return True if the V4L2 control is read-only, false otherwise\n\nI would not mention V4L2 here and keep the documentation generic\n\n> + */\n> +\n> /**\n> * \\fn ControlInfo::values()\n> * \\brief Retrieve the list of valid values\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 57a88d96b12c..9018f1b0b9e1 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -535,17 +535,20 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> \tcase V4L2_CTRL_TYPE_U8:\n> \t\treturn ControlInfo(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> +\t\t\t\t static_cast(ctrl.default_value),\n> +\t\t\t\t !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));\n>\n> \tcase V4L2_CTRL_TYPE_BOOLEAN:\n> \t\treturn ControlInfo(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> +\t\t\t\t static_cast(ctrl.default_value),\n> +\t\t\t\t !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));\n>\n> \tcase V4L2_CTRL_TYPE_INTEGER64:\n> \t\treturn ControlInfo(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> +\t\t\t\t static_cast(ctrl.default_value),\n> +\t\t\t\t !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));\n>\n> \tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> \tcase V4L2_CTRL_TYPE_MENU:\n> @@ -554,7 +557,8 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> \tdefault:\n> \t\treturn ControlInfo(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> +\t\t\t\t static_cast(ctrl.default_value),\n> +\t\t\t\t !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));\n> \t}\n> }\n>\n> --\n> 2.25.1\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 BE2E3BDE6B\n\tfor ;\n\tMon, 12 Dec 2022 09:51:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 491DB63354;\n\tMon, 12 Dec 2022 10:51:01 +0100 (CET)","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 A366C61F23\n\tfor ;\n\tMon, 12 Dec 2022 10:50:59 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 1747F20012;\n\tMon, 12 Dec 2022 09:50:58 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670838661;\n\tbh=5cToQ3WOlb5YS+6qTinS4W+OhcWqqMS6qnZM/aZjA3U=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hNv34R3FFciTN9vphWY+h7V2ofIj4lvoF7fF8aK2JsY2fA4DOI9IOoXQbqhNQ3Cr/\n\tTLEVI0JWnk0xLJKgxw4NDh7S/zj4buC5cuWEQoFP6KO+HM/jghiACsWtZ/9RraVqQF\n\trlFIrDRPGGXflUZ0/LWAP8AJOUYQxz6L3td+kZX2DnQv40mIuTMtFv8rLg7DbSJ1cT\n\tb0fWAjDOcN9VjnPmi43B7Y4L/ZXZp24kagjXGXnE8PU0PmDPtv/Ug+4xG1OUwgl42x\n\tq66vKps2RPyywiNprQt/vDdnJM/pGbBq+BXPm9lNpLlWon+0Q6kZ835lLEtwSsKXhI\n\tM39kaz7gi7aeg==","Date":"Mon, 12 Dec 2022 10:50:58 +0100","To":"Naushir Patuck ","Message-ID":"<20221212095058.x2olcq6xaixmewkm@uno.localdomain>","References":"<20221202124005.3643-1-naush@raspberrypi.com>\n\t<20221202124005.3643-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221202124005.3643-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26045,"web_url":"https://patchwork.libcamera.org/comment/26045/","msgid":"","date":"2022-12-12T10:16:35","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for your feedback!\n\nOn Mon, 12 Dec 2022 at 09:50, Jacopo Mondi wrote:\n\n> Hi Naush\n>\n> On Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> > Add a read-only flag to the ControlInfo flag to indicate whether a V4L2\n> control\n> > is read-only or not. This flag only makes sense for V2L2 controls at\n> preset, so\n> > only update the ControlInfo constructor signature used by the V4L2Device\n> class\n> > to set the value of the flag.\n> >\n>\n> It feels a bit the three constructors are mis-aligned now :/\n>\n\n> I would have gone for a default parameter for all three of them to be\n> honest... What do others think ?\n>\n\nI do kind of agree with this, the constructor signatures were the one bit\nwhere I was unsure what to do.\n\nHappy to change all the constructors to have a readOnly param if others\nagree!\n\nRegards,\nNaush\n\n\n\n>\n> > Add a ControlInfo::readOnly() member function to retrive this flag.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/controls.h | 5 ++++-\n> > src/libcamera/controls.cpp | 17 +++++++++++++----\n> > src/libcamera/v4l2_device.cpp | 12 ++++++++----\n> > 3 files changed, 25 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index cf94205577a5..488663a7ba04 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -270,7 +270,8 @@ class ControlInfo\n> > public:\n> > explicit ControlInfo(const ControlValue &min = {},\n> > const ControlValue &max = {},\n> > - const ControlValue &def = {});\n> > + const ControlValue &def = {},\n> > + bool readOnly = false);\n> > explicit ControlInfo(Span values,\n> > const ControlValue &def = {});\n> > explicit ControlInfo(std::set values, bool def);\n> > @@ -279,6 +280,7 @@ public:\n> > const ControlValue &min() const { return min_; }\n> > const ControlValue &max() const { return max_; }\n> > const ControlValue &def() const { return def_; }\n> > + bool readOnly() const { return readOnly_; }\n> > const std::vector &values() const { return values_; }\n> >\n> > std::string toString() const;\n> > @@ -297,6 +299,7 @@ private:\n> > ControlValue min_;\n> > ControlValue max_;\n> > ControlValue def_;\n> > + bool readOnly_;\n> > std::vector values_;\n> > };\n> >\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 6dbf9b348709..fc66abad600d 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool\n> isArray, std::size_t numElemen\n> > * \\param[in] min The control minimum value\n> > * \\param[in] max The control maximum value\n> > * \\param[in] def The control default value\n> > + * \\param[in] readOnly Read-only status of a V4L2 control\n> > */\n> > ControlInfo::ControlInfo(const ControlValue &min,\n> > const ControlValue &max,\n> > - const ControlValue &def)\n> > - : min_(min), max_(max), def_(def)\n> > + const ControlValue &def,\n> > + bool readOnly)\n> > + : min_(min), max_(max), def_(def), readOnly_(readOnly)\n> > {\n> > }\n> >\n> > @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span\n> values,\n> > * default value is \\a def.\n> > */\n> > ControlInfo::ControlInfo(std::set values, bool def)\n> > - : min_(false), max_(true), def_(def), values_({ false, true })\n> > + : min_(false), max_(true), def_(def), readOnly_(false),\n> > + values_({ false, true })\n> > {\n> > ASSERT(values.count(def) && values.size() == 2);\n> > }\n> > @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set values, bool\n> def)\n> > * value. The minimum, maximum, and default values will all be \\a value.\n> > */\n> > ControlInfo::ControlInfo(bool value)\n> > - : min_(value), max_(value), def_(value)\n> > + : min_(value), max_(value), def_(value), readOnly_(false)\n> > {\n> > values_ = { value };\n> > }\n> > @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)\n> > * \\return A ControlValue with the default value for the control\n> > */\n> >\n> > +/**\n> > + * \\fn ControlInfo::readOnly()\n> > + * \\brief Identifies if a V4L2 control is flagged as read-only\n> > + * \\return True if the V4L2 control is read-only, false otherwise\n>\n> I would not mention V4L2 here and keep the documentation generic\n>\n> > + */\n> > +\n> > /**\n> > * \\fn ControlInfo::values()\n> > * \\brief Retrieve the list of valid values\n> > diff --git a/src/libcamera/v4l2_device.cpp\n> b/src/libcamera/v4l2_device.cpp\n> > index 57a88d96b12c..9018f1b0b9e1 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -535,17 +535,20 @@ std::optional\n> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > case V4L2_CTRL_TYPE_U8:\n> > return ControlInfo(static_cast(ctrl.minimum),\n> > static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > +\n> static_cast(ctrl.default_value),\n> > + !!(ctrl.flags &\n> V4L2_CTRL_FLAG_READ_ONLY));\n> >\n> > case V4L2_CTRL_TYPE_BOOLEAN:\n> > return ControlInfo(static_cast(ctrl.minimum),\n> > static_cast(ctrl.maximum),\n> > - static_cast(ctrl.default_value));\n> > + static_cast(ctrl.default_value),\n> > + !!(ctrl.flags &\n> V4L2_CTRL_FLAG_READ_ONLY));\n> >\n> > case V4L2_CTRL_TYPE_INTEGER64:\n> > return ControlInfo(static_cast(ctrl.minimum),\n> > static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > +\n> static_cast(ctrl.default_value),\n> > + !!(ctrl.flags &\n> V4L2_CTRL_FLAG_READ_ONLY));\n> >\n> > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > case V4L2_CTRL_TYPE_MENU:\n> > @@ -554,7 +557,8 @@ std::optional\n> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > default:\n> > return ControlInfo(static_cast(ctrl.minimum),\n> > static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > +\n> static_cast(ctrl.default_value),\n> > + !!(ctrl.flags &\n> V4L2_CTRL_FLAG_READ_ONLY));\n> > }\n> > }\n> >\n> > --\n> > 2.25.1\n> >\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 730E7C31E9\n\tfor ;\n\tMon, 12 Dec 2022 10:16:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E69763354;\n\tMon, 12 Dec 2022 11:16:42 +0100 (CET)","from mail-vk1-xa2a.google.com (mail-vk1-xa2a.google.com\n\t[IPv6:2607:f8b0:4864:20::a2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE53461F23\n\tfor ;\n\tMon, 12 Dec 2022 11:16:40 +0100 (CET)","by mail-vk1-xa2a.google.com with SMTP id q7so866925vka.7\n\tfor ;\n\tMon, 12 Dec 2022 02:16:40 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670840202;\n\tbh=JS+P/I9lAo0ppecZnrl9/1YCK/XZ0TTLwCA7PgElOr0=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=f2NCdTGLsjRHVsd3zl5MJWRzZRI0lcDV9V9zzuGjjI0APrT3wOB5AMTvnxEawizu+\n\tjYW0rKvvbwsqppz3CN8mtMpKsc4RVwCpI7Pfxv/C/2G+FBOiPA8I2MRbOjiJ/lXKLS\n\tGJd3FeYMzj1yQ0JJqQOiBdWLL2AL7uERNwHKuu5mc6mHNjupQqDz+jzMcgbTVtSbrw\n\t6YRXJdGKPJcQwOXoqBgCw1ad/CdTzHthy1B01T7KQDxMwYgxccYLXxMa8SdTB8GgzN\n\tKvasDcKv/DiGPshiUwwpXU60dllJ51z1KtHeyL7qEe0UuicuORPtAh4xvQFvpjpMQ4\n\t6wHDEKKllsDEQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=tEHqFsr6tcOnQy4UpYPR9Wbd6v77iZQCUiG40oYZLJw=;\n\tb=aQZQIs7/+TCejit4sBVQ1z/PMS2ck9ip5TAkVWX3jx9e/t5FjN/ugTT1KahxL7qJtr\n\tQV9mmhM2cni3HOS6O24SKU929DpQjua1nEAuGQHJPMQG+rpgLnSZoQ/zTKjZfDZxIZYd\n\teJFAw+S/HVaNCulAlcBEZLLIDooTHqLdD1ui+370R3oCjf+M/ldzkQ/gNLeq4ZOQg9ta\n\tebPwBfavFN41LtGcz/oHrzb3mq26rKd+woYSFKvypL1Vh/xFIBWktGXy36SWveiCIxy7\n\tYVlXLzj6/sxMtaeSCJCDwRYLWLIvXm2iJDI0dT4NpbSOrRbeWHhabsXkki1z2cTvQhOZ\n\tp31Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"aQZQIs7/\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=tEHqFsr6tcOnQy4UpYPR9Wbd6v77iZQCUiG40oYZLJw=;\n\tb=Y42lH9bMqmeaPXwPqNxJb79GTHr6tLliXnRM+LjoRLzrgtop8qsN7CqWdIoOzZ1TYV\n\t3YM6KOfViJ6QOS3xlO/OwCT0z0dVbVszd53rApb3FGVOxcAZk1KfVkIt4Tb+orEOJPLL\n\t9Vph5o13rdeI3j3W/BGpyUwugFofSGgaWmTRDJKnD3aN82tCg/C4x3zsk1zgf0djvNe6\n\tL8cIbHXQXJw1X07SX2khURAFo49Fy2iTwB+/Fl2fwN2MjAsYydd27GmseY7zrW6c21ht\n\tGrY3lyEzis3QttNhbJd1bkn18ZxXIJq6T71j/BJUsfGJ3LEbetrYrJkx7sbLuMBH5fu2\n\t7DpQ==","X-Gm-Message-State":"ANoB5plAG86TbRfQWZf4CWa9xN0k1/AjlLr1GIbs+nS3+PqeeJbpKP6Y\n\t3peagBNONwSdVeimmz9XgmEwVn225hcE86JgSvsqfgnR2Ey1AAJZ","X-Google-Smtp-Source":"AA0mqf6ibAOs5LayvZ9WgYwczMJwlT7qmmEAZhOhGqjNrUN4BxAVPZoEDuje8VYJ4hVHJ8J0mvdhODKW33yibyDpZZk=","X-Received":"by 2002:a05:6122:d04:b0:3bd:6db7:262b with SMTP id\n\taz4-20020a0561220d0400b003bd6db7262bmr14018040vkb.27.1670840199783;\n\tMon, 12 Dec 2022 02:16:39 -0800 (PST)","MIME-Version":"1.0","References":"<20221202124005.3643-1-naush@raspberrypi.com>\n\t<20221202124005.3643-2-naush@raspberrypi.com>\n\t<20221212095058.x2olcq6xaixmewkm@uno.localdomain>","In-Reply-To":"<20221212095058.x2olcq6xaixmewkm@uno.localdomain>","Date":"Mon, 12 Dec 2022 10:16:35 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000095beac05ef9ecc28\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26375,"web_url":"https://patchwork.libcamera.org/comment/26375/","msgid":"<167508594722.42371.4190530498370217441@Monstersaurus>","date":"2023-01-30T13:39:07","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2022-12-12 10:16:35)\n> Hi Jacopo,\n> \n> Thank you for your feedback!\n> \n> On Mon, 12 Dec 2022 at 09:50, Jacopo Mondi wrote:\n> \n> > Hi Naush\n> >\n> > On Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via\n> > libcamera-devel wrote:\n> > > Add a read-only flag to the ControlInfo flag to indicate whether a V4L2\n> > control\n> > > is read-only or not. This flag only makes sense for V2L2 controls at\n> > preset, so\n> > > only update the ControlInfo constructor signature used by the V4L2Device\n> > class\n> > > to set the value of the flag.\n> > >\n> >\n> > It feels a bit the three constructors are mis-aligned now :/\n> >\n> \n> > I would have gone for a default parameter for all three of them to be\n> > honest... What do others think ?\n> >\n> \n> I do kind of agree with this, the constructor signatures were the one bit\n> where I was unsure what to do.\n> \n> Happy to change all the constructors to have a readOnly param if others\n> agree!\n\nIt's tough to know without knowing future use-cases, but if we have one\ntype of read-only control, it's probably fair to assume or expect that\nit's a 'feature' of controls, and should be possible to set on all\ntypes.\n\nSo I think this is worth having on all of them yes.\n\n\n\n> \n> Regards,\n> Naush\n> \n> \n> \n> >\n> > > Add a ControlInfo::readOnly() member function to retrive this flag.\n\ns/retrive/retrieve/\n\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/controls.h | 5 ++++-\n> > > src/libcamera/controls.cpp | 17 +++++++++++++----\n> > > src/libcamera/v4l2_device.cpp | 12 ++++++++----\n> > > 3 files changed, 25 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index cf94205577a5..488663a7ba04 100644\n> > > --- a/include/libcamera/controls.h\n> > > +++ b/include/libcamera/controls.h\n> > > @@ -270,7 +270,8 @@ class ControlInfo\n> > > public:\n> > > explicit ControlInfo(const ControlValue &min = {},\n> > > const ControlValue &max = {},\n> > > - const ControlValue &def = {});\n> > > + const ControlValue &def = {},\n> > > + bool readOnly = false);\n> > > explicit ControlInfo(Span values,\n> > > const ControlValue &def = {});\n> > > explicit ControlInfo(std::set values, bool def);\n> > > @@ -279,6 +280,7 @@ public:\n> > > const ControlValue &min() const { return min_; }\n> > > const ControlValue &max() const { return max_; }\n> > > const ControlValue &def() const { return def_; }\n> > > + bool readOnly() const { return readOnly_; }\n> > > const std::vector &values() const { return values_; }\n> > >\n> > > std::string toString() const;\n> > > @@ -297,6 +299,7 @@ private:\n> > > ControlValue min_;\n> > > ControlValue max_;\n> > > ControlValue def_;\n> > > + bool readOnly_;\n> > > std::vector values_;\n> > > };\n> > >\n> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > index 6dbf9b348709..fc66abad600d 100644\n> > > --- a/src/libcamera/controls.cpp\n> > > +++ b/src/libcamera/controls.cpp\n> > > @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool\n> > isArray, std::size_t numElemen\n> > > * \\param[in] min The control minimum value\n> > > * \\param[in] max The control maximum value\n> > > * \\param[in] def The control default value\n> > > + * \\param[in] readOnly Read-only status of a V4L2 control\n> > > */\n> > > ControlInfo::ControlInfo(const ControlValue &min,\n> > > const ControlValue &max,\n> > > - const ControlValue &def)\n> > > - : min_(min), max_(max), def_(def)\n> > > + const ControlValue &def,\n> > > + bool readOnly)\n> > > + : min_(min), max_(max), def_(def), readOnly_(readOnly)\n> > > {\n> > > }\n> > >\n> > > @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span\n> > values,\n> > > * default value is \\a def.\n> > > */\n> > > ControlInfo::ControlInfo(std::set values, bool def)\n> > > - : min_(false), max_(true), def_(def), values_({ false, true })\n> > > + : min_(false), max_(true), def_(def), readOnly_(false),\n> > > + values_({ false, true })\n> > > {\n> > > ASSERT(values.count(def) && values.size() == 2);\n> > > }\n> > > @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set values, bool\n> > def)\n> > > * value. The minimum, maximum, and default values will all be \\a value.\n> > > */\n> > > ControlInfo::ControlInfo(bool value)\n> > > - : min_(value), max_(value), def_(value)\n> > > + : min_(value), max_(value), def_(value), readOnly_(false)\n> > > {\n> > > values_ = { value };\n> > > }\n> > > @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)\n> > > * \\return A ControlValue with the default value for the control\n> > > */\n> > >\n> > > +/**\n> > > + * \\fn ControlInfo::readOnly()\n> > > + * \\brief Identifies if a V4L2 control is flagged as read-only\n> > > + * \\return True if the V4L2 control is read-only, false otherwise\n> >\n> > I would not mention V4L2 here and keep the documentation generic\n> >\n> > > + */\n> > > +\n> > > /**\n> > > * \\fn ControlInfo::values()\n> > > * \\brief Retrieve the list of valid values\n> > > diff --git a/src/libcamera/v4l2_device.cpp\n> > b/src/libcamera/v4l2_device.cpp\n> > > index 57a88d96b12c..9018f1b0b9e1 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -535,17 +535,20 @@ std::optional\n> > V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > case V4L2_CTRL_TYPE_U8:\n> > > return ControlInfo(static_cast(ctrl.minimum),\n> > > static_cast(ctrl.maximum),\n> > > -\n> > static_cast(ctrl.default_value));\n> > > +\n> > static_cast(ctrl.default_value),\n> > > + !!(ctrl.flags &\n> > V4L2_CTRL_FLAG_READ_ONLY));\n> > >\n> > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > return ControlInfo(static_cast(ctrl.minimum),\n> > > static_cast(ctrl.maximum),\n> > > - static_cast(ctrl.default_value));\n> > > + static_cast(ctrl.default_value),\n> > > + !!(ctrl.flags &\n> > V4L2_CTRL_FLAG_READ_ONLY));\n> > >\n> > > case V4L2_CTRL_TYPE_INTEGER64:\n> > > return ControlInfo(static_cast(ctrl.minimum),\n> > > static_cast(ctrl.maximum),\n> > > -\n> > static_cast(ctrl.default_value));\n> > > +\n> > static_cast(ctrl.default_value),\n> > > + !!(ctrl.flags &\n> > V4L2_CTRL_FLAG_READ_ONLY));\n> > >\n> > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > case V4L2_CTRL_TYPE_MENU:\n> > > @@ -554,7 +557,8 @@ std::optional\n> > V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > default:\n> > > return ControlInfo(static_cast(ctrl.minimum),\n> > > static_cast(ctrl.maximum),\n> > > -\n> > static_cast(ctrl.default_value));\n> > > +\n> > static_cast(ctrl.default_value),\n> > > + !!(ctrl.flags &\n> > V4L2_CTRL_FLAG_READ_ONLY));\n> > > }\n> > > }\n> > >\n> > > --\n> > > 2.25.1\n> > >\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 2FF68BEFBE\n\tfor ;\n\tMon, 30 Jan 2023 13:39:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ABFF5625E4;\n\tMon, 30 Jan 2023 14:39:11 +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 093AE60482\n\tfor ;\n\tMon, 30 Jan 2023 14:39:10 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D4698B8;\n\tMon, 30 Jan 2023 14:39:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675085951;\n\tbh=1kYo1QU3F1vsLexEtxGhX8WhtbKfm6TYqknoWXyzqt8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pq4/48cAN/VmTXuOb+espcvctNZeR0lj35exwXDnFrx7xjfGorB11LFu/TzGgEEJU\n\tNmNgbU+TdCo8+QkE+PPXTy4GsaM6Uh6hmZHYo60+zG+ALXYLvTAGK9S9STsWrz5HEo\n\toxbt2lnVlnV2Wg1kYySs8QQ4eFB1Zghzn3cw0LxX1/uq2/ewgLQFCqBp7XNReOXV8W\n\tDmsCf/Czia96M7O3Lwvt2vDLAXUuAIeJCdcr/pNasfynsagAf4T7nZJtXOgQh9w+UD\n\tj5lYdG2TRzZSRfVkFfcAVrh37pKTbYxhdWqa94BuYFgY9CNmKyEe7uugnAn3tiamfl\n\tDv9zAy2dgwAwg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675085949;\n\tbh=1kYo1QU3F1vsLexEtxGhX8WhtbKfm6TYqknoWXyzqt8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=P4o6p0SvSxTJ842HRfasWwE5u58RliP/2KHmWSLxQZkZ8UxrS7eezuRN0cYyrQHvZ\n\tTh9Y0enO59QD6l+e7qgWFmHzod1uWd5cq61AEfa5AZfrd1Yo2JvmHmpRyPJwGcmuig\n\tclDiEewd53CBLqWkkWMDAbRePCi25KUU+R3Uijpo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"P4o6p0Sv\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221202124005.3643-1-naush@raspberrypi.com>\n\t<20221202124005.3643-2-naush@raspberrypi.com>\n\t<20221212095058.x2olcq6xaixmewkm@uno.localdomain>\n\t","To":"Jacopo Mondi , Naushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Mon, 30 Jan 2023 13:39:07 +0000","Message-ID":"<167508594722.42371.4190530498370217441@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27734,"web_url":"https://patchwork.libcamera.org/comment/27734/","msgid":"<169386867612.3421751.14940035681609374606@ping.linuxembedded.co.uk>","date":"2023-09-04T23:04:36","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2023-01-30 13:39:07)\n> Quoting Naushir Patuck via libcamera-devel (2022-12-12 10:16:35)\n> > Hi Jacopo,\n> > \n> > Thank you for your feedback!\n> > \n> > On Mon, 12 Dec 2022 at 09:50, Jacopo Mondi wrote:\n> > \n> > > Hi Naush\n> > >\n> > > On Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via\n> > > libcamera-devel wrote:\n> > > > Add a read-only flag to the ControlInfo flag to indicate whether a V4L2\n> > > control\n> > > > is read-only or not. This flag only makes sense for V2L2 controls at\n> > > preset, so\n> > > > only update the ControlInfo constructor signature used by the V4L2Device\n> > > class\n> > > > to set the value of the flag.\n> > > >\n> > >\n> > > It feels a bit the three constructors are mis-aligned now :/\n> > >\n> > \n> > > I would have gone for a default parameter for all three of them to be\n> > > honest... What do others think ?\n> > >\n> > \n> > I do kind of agree with this, the constructor signatures were the one bit\n> > where I was unsure what to do.\n> > \n> > Happy to change all the constructors to have a readOnly param if others\n> > agree!\n> \n> It's tough to know without knowing future use-cases, but if we have one\n> type of read-only control, it's probably fair to assume or expect that\n> it's a 'feature' of controls, and should be possible to set on all\n> types.\n> \n> So I think this is worth having on all of them yes.\n\nSo - I've been hitting the issue resolved by this series with a driver\nthat has a read-only HBLANK control but the min/max are not == value.\n\nI don't know if that makes sense yet in the driver, and I 'fixed' this\nlocally by setting min==max==value in the driver - but it feels somewhat\nlike changing the kernel to fix a libcamera limitation...\n\nSo - resuming this series.\n\nIt seems quite difficult to introduce the extra argument to all the\nconstructors without facing issues though, where this affects which\nconstructor gets chosen ...\n\n../../src/libcamera/pipeline/uvcvideo/uvcvideo.cpp: In member function ‘void libcamera::UVCCameraData::addControl(uint32_t, const libcamera::ControlInfo&, libcamera::ControlInfoMap::Map*)’:\n../../src/libcamera/pipeline/uvcvideo/uvcvideo.cpp:629:57: error: narrowing conversion of ‘((float)(min - def) / scale)’ from ‘float’ to ‘bool’ [-Werror=narrowing]\n 629 | { static_cast(min - def) / scale },\n | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~\n../../src/libcamera/pipeline/uvcvideo/uvcvideo.cpp:629:57: error: narrowing conversion of ‘((float)(min - def) / scale)’ from ‘float’ to ‘bool’ [-Werror=narrowing]\n../../src/libcamera/pipeline/uvcvideo/uvcvideo.cpp:630:57: error: narrowing conversion of ‘((float)(max - def) / scale)’ from ‘float’ to ‘bool’ [-Werror=narrowing]\n 630 | { static_cast(max - def) / scale },\n | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~\n../../src/libcamera/pipeline/uvcvideo/uvcvideo.cpp:632:17: error: narrowing conversion of ‘0.0f’ from ‘float’ to ‘bool’ [-Wnarrowing]\n 632 | };\n | ^\n\nI presume this is because the compiler could implicitly map those types\nto bool when trying to construct the usual 3 parameter ControlInfo which\ncould match the now 3 parameter bool constructor.\n\nI wonder if maybe it's easier to just leave it as supported on the\nsingle min/max/def constructor indeed. It could be extended if it became\nrequired later...\n\n\n\nHere's my current diff/fixup on this patch:\n\n\n\n\n\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 802dd906937b..8af304bdeac9 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -484,7 +484,7 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n * \\param[in] min The control minimum value\n * \\param[in] max The control maximum value\n * \\param[in] def The control default value\n- * \\param[in] readOnly Read-only status of a V4L2 control\n+ * \\param[in] readOnly True if the control reports a dynamic property\n */\n ControlInfo::ControlInfo(const ControlValue &min,\n const ControlValue &max,\n@@ -510,6 +510,7 @@ ControlInfo::ControlInfo(Span values,\n min_ = values.front();\n max_ = values.back();\n def_ = !def.isNone() ? def : values.front();\n+ readOnly_ = false;\n\n values_.reserve(values.size());\n for (const ControlValue &value : values)\n@@ -541,7 +542,7 @@ ControlInfo::ControlInfo(std::set values, bool def)\n * value. The minimum, maximum, and default values will all be \\a value.\n */\n ControlInfo::ControlInfo(bool value)\n- : min_(value), max_(value), def_(value), readOnly_(false)\n+ : min_(value), max_(value), def_(value), readOnly_(true)\n {\n values_ = { value };\n }\n@@ -576,8 +577,12 @@ ControlInfo::ControlInfo(bool value)\n\n /**\n * \\fn ControlInfo::readOnly()\n- * \\brief Identifies if a V4L2 control is flagged as read-only\n- * \\return True if the V4L2 control is read-only, false otherwise\n+ * \\brief Identifies if a control is flagged as read-only\n+ * \\return True if the control is read-only, false otherwise\n+ *\n+ * Read-only controls may signify that the control is a volatile property and\n+ * can not be set. Adding a read-only control to a control list may cause the\n+ * list to fail when the list is submitted.\n */\n\n /**\n\n\n--\nKieran\n\n\n> \n> \n> \n> > \n> > Regards,\n> > Naush\n> > \n> > \n> > \n> > >\n> > > > Add a ControlInfo::readOnly() member function to retrive this flag.\n> \n> s/retrive/retrieve/\n> \n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > include/libcamera/controls.h | 5 ++++-\n> > > > src/libcamera/controls.cpp | 17 +++++++++++++----\n> > > > src/libcamera/v4l2_device.cpp | 12 ++++++++----\n> > > > 3 files changed, 25 insertions(+), 9 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > > index cf94205577a5..488663a7ba04 100644\n> > > > --- a/include/libcamera/controls.h\n> > > > +++ b/include/libcamera/controls.h\n> > > > @@ -270,7 +270,8 @@ class ControlInfo\n> > > > public:\n> > > > explicit ControlInfo(const ControlValue &min = {},\n> > > > const ControlValue &max = {},\n> > > > - const ControlValue &def = {});\n> > > > + const ControlValue &def = {},\n> > > > + bool readOnly = false);\n> > > > explicit ControlInfo(Span values,\n> > > > const ControlValue &def = {});\n> > > > explicit ControlInfo(std::set values, bool def);\n> > > > @@ -279,6 +280,7 @@ public:\n> > > > const ControlValue &min() const { return min_; }\n> > > > const ControlValue &max() const { return max_; }\n> > > > const ControlValue &def() const { return def_; }\n> > > > + bool readOnly() const { return readOnly_; }\n> > > > const std::vector &values() const { return values_; }\n> > > >\n> > > > std::string toString() const;\n> > > > @@ -297,6 +299,7 @@ private:\n> > > > ControlValue min_;\n> > > > ControlValue max_;\n> > > > ControlValue def_;\n> > > > + bool readOnly_;\n> > > > std::vector values_;\n> > > > };\n> > > >\n> > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > > index 6dbf9b348709..fc66abad600d 100644\n> > > > --- a/src/libcamera/controls.cpp\n> > > > +++ b/src/libcamera/controls.cpp\n> > > > @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool\n> > > isArray, std::size_t numElemen\n> > > > * \\param[in] min The control minimum value\n> > > > * \\param[in] max The control maximum value\n> > > > * \\param[in] def The control default value\n> > > > + * \\param[in] readOnly Read-only status of a V4L2 control\n> > > > */\n> > > > ControlInfo::ControlInfo(const ControlValue &min,\n> > > > const ControlValue &max,\n> > > > - const ControlValue &def)\n> > > > - : min_(min), max_(max), def_(def)\n> > > > + const ControlValue &def,\n> > > > + bool readOnly)\n> > > > + : min_(min), max_(max), def_(def), readOnly_(readOnly)\n> > > > {\n> > > > }\n> > > >\n> > > > @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span\n> > > values,\n> > > > * default value is \\a def.\n> > > > */\n> > > > ControlInfo::ControlInfo(std::set values, bool def)\n> > > > - : min_(false), max_(true), def_(def), values_({ false, true })\n> > > > + : min_(false), max_(true), def_(def), readOnly_(false),\n> > > > + values_({ false, true })\n> > > > {\n> > > > ASSERT(values.count(def) && values.size() == 2);\n> > > > }\n> > > > @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set values, bool\n> > > def)\n> > > > * value. The minimum, maximum, and default values will all be \\a value.\n> > > > */\n> > > > ControlInfo::ControlInfo(bool value)\n> > > > - : min_(value), max_(value), def_(value)\n> > > > + : min_(value), max_(value), def_(value), readOnly_(false)\n> > > > {\n> > > > values_ = { value };\n> > > > }\n> > > > @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)\n> > > > * \\return A ControlValue with the default value for the control\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\fn ControlInfo::readOnly()\n> > > > + * \\brief Identifies if a V4L2 control is flagged as read-only\n> > > > + * \\return True if the V4L2 control is read-only, false otherwise\n> > >\n> > > I would not mention V4L2 here and keep the documentation generic\n> > >\n> > > > + */\n> > > > +\n> > > > /**\n> > > > * \\fn ControlInfo::values()\n> > > > * \\brief Retrieve the list of valid values\n> > > > diff --git a/src/libcamera/v4l2_device.cpp\n> > > b/src/libcamera/v4l2_device.cpp\n> > > > index 57a88d96b12c..9018f1b0b9e1 100644\n> > > > --- a/src/libcamera/v4l2_device.cpp\n> > > > +++ b/src/libcamera/v4l2_device.cpp\n> > > > @@ -535,17 +535,20 @@ std::optional\n> > > V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > > case V4L2_CTRL_TYPE_U8:\n> > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > static_cast(ctrl.maximum),\n> > > > -\n> > > static_cast(ctrl.default_value));\n> > > > +\n> > > static_cast(ctrl.default_value),\n> > > > + !!(ctrl.flags &\n> > > V4L2_CTRL_FLAG_READ_ONLY));\n> > > >\n> > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > static_cast(ctrl.maximum),\n> > > > - static_cast(ctrl.default_value));\n> > > > + static_cast(ctrl.default_value),\n> > > > + !!(ctrl.flags &\n> > > V4L2_CTRL_FLAG_READ_ONLY));\n> > > >\n> > > > case V4L2_CTRL_TYPE_INTEGER64:\n> > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > static_cast(ctrl.maximum),\n> > > > -\n> > > static_cast(ctrl.default_value));\n> > > > +\n> > > static_cast(ctrl.default_value),\n> > > > + !!(ctrl.flags &\n> > > V4L2_CTRL_FLAG_READ_ONLY));\n> > > >\n> > > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > case V4L2_CTRL_TYPE_MENU:\n> > > > @@ -554,7 +557,8 @@ std::optional\n> > > V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > > default:\n> > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > static_cast(ctrl.maximum),\n> > > > -\n> > > static_cast(ctrl.default_value));\n> > > > +\n> > > static_cast(ctrl.default_value),\n> > > > + !!(ctrl.flags &\n> > > V4L2_CTRL_FLAG_READ_ONLY));\n> > > > }\n> > > > }\n> > > >\n> > > > --\n> > > > 2.25.1\n> > > >\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 6191ABE080\n\tfor ;\n\tMon, 4 Sep 2023 23:04:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A158628E9;\n\tTue, 5 Sep 2023 01:04:41 +0200 (CEST)","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 77FA2627E8\n\tfor ;\n\tTue, 5 Sep 2023 01:04:39 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1DB51497;\n\tTue, 5 Sep 2023 01:03:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693868681;\n\tbh=yfPLGrZcnBUeJiCV3Kx/zG1tEjl7cygJm8E8fXyYd4w=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=jLRNYo/TFU1gq2ZFb7YyAO0llX2zD0FZCwBpmH/+AG6tjkkffb3T7VxbAO8F6C/lw\n\tH0q+g9PZ43W+bVM2Xr1Ou6+cwXrpWtnLFwrojOuP3RpdCzlkPZphA3As4qs3atexPS\n\taQX7akLZzN1XI+S2PkE1Z6nlSXBKu0HjUvvOS3rAkdlW1ZbtbdQV3stL24qlaBI0/x\n\tznfKaQecxZdU5OvB7JmcnHZVMfaBnYcEOGebpkhESEXSLrwpjFah6hiI7N96mBP56t\n\tPK0r9ATWq1ENu4QwaZAvlIz+pX1fRSr40iyXHOgfe8rp5CcV9wmeS3Qi9qMlHIm0AD\n\tM4QyczbB43rvQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693868593;\n\tbh=yfPLGrZcnBUeJiCV3Kx/zG1tEjl7cygJm8E8fXyYd4w=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GMQdJ9yN4APGzFxvYuA+Qse/37dB81WpQpBYH5ms7JpeBcJpiAzwYNNVpIyGZFmbV\n\tkPRWAx0UigzqOE/IUtgvzrDM3kVcabZEU4n5ff4QqK4rP9NRYHjyyKHvlt8c0uvQFi\n\thNSNKlc8wAj6bYurWxcjCOI2lChBuGkmNZH+2B0M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GMQdJ9yN\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<167508594722.42371.4190530498370217441@Monstersaurus>","References":"<20221202124005.3643-1-naush@raspberrypi.com>\n\t<20221202124005.3643-2-naush@raspberrypi.com>\n\t<20221212095058.x2olcq6xaixmewkm@uno.localdomain>\n\t\n\t<167508594722.42371.4190530498370217441@Monstersaurus>","To":"Jacopo Mondi , Naushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Tue, 05 Sep 2023 00:04:36 +0100","Message-ID":"<169386867612.3421751.14940035681609374606@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add\n\tread-only flag to ControlInfo","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]