[{"id":3175,"web_url":"https://patchwork.libcamera.org/comment/3175/","msgid":"<20191204154832.GC7811@pendragon.ideasonboard.com>","date":"2019-12-04T15:48:32","subject":"Re: [libcamera-devel] [PATCH 04/10] libcamera: controls: Add\n\tdefault to ControlRange","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Dec 04, 2019 at 02:21:00PM +0100, Jacopo Mondi wrote:\n> Augment the the ControlRange class to store the control default value.\n> \n> This is particularly relevant for v4l2 controls used to create\n> Camera properties, which are constructed using immutable video device\n> properties, whose value won't change at runtime.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/controls.h | 7 ++++++-\n> src/libcamera/controls.cpp | 17 +++++++++++++++--\n> 2 files changed, 21 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b1b73367e874..1e2f284eafeb 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -114,10 +114,12 @@ class ControlRange\n> {\n> public:\n> \texplicit ControlRange(const ControlValue &min = 0,\n> -\t\t\t const ControlValue &max = 0);\n> +\t\t\t const ControlValue &max = 0,\n> +\t\t\t const ControlValue &defaultValue = 0);\n\nI wish we could call this default, but that's a reserved keyword :-/\nMaybe just 'def' to match min and max ? I'll leave it up to you.\n\n\n\n> \n> \tconst ControlValue &min() const { return min_; }\n> \tconst ControlValue &max() const { return max_; }\n> +\tconst ControlValue &defaultValue() const { return defaultValue_; }\n> \n> \tstd::string toString() const;\n> \n> @@ -131,6 +133,9 @@ public:\n> \t\treturn !(*this == other);\n> \t}\n> \n> +protected:\n> +\tControlValue defaultValue_;\n> +\n\nWhy is this protected while min_ and max_ are private ? I don't think\nthe default value should be special.\n\n> private:\n> \tControlValue min_;\n> \tControlValue max_;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 7d8a0e97ee3a..bacba0fbf68a 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -353,14 +353,21 @@ Control::Control(unsigned int id, const char *name)\n> * pipeline handlers to describe the controls they support.\n> */\n> \n> +/**\n> + * \\var ControlRange::defaultValue_\n> + * \\brief The control default value\n> + */\n> +\n> /**\n> * \\brief Construct a ControlRange with minimum and maximum range parameters\n> * \\param[in] min The control minimum value\n> * \\param[in] max The control maximum value\n> + * \\param[in] defaultValue The control default value\n> */\n> ControlRange::ControlRange(const ControlValue &min,\n> -\t\t\t const ControlValue &max)\n> -\t: min_(min), max_(max)\n> +\t\t\t const ControlValue &max,\n> +\t\t\t const ControlValue &defaultValue)\n> +\t: defaultValue_(defaultValue), min_(min), max_(max)\n> {\n> }\n> \n> @@ -376,6 +383,12 @@ ControlRange::ControlRange(const ControlValue &min,\n> * \\return A ControlValue with the maximum value for the control\n> */\n> \n> +/**\n> + * \\fn ControlRange::defaultValue()\n> + * \\brief Retrieve the default value of the control\n> + * \\return A ControlValue with the default value for the control\n> + */\n> +\n> /**\n> * \\brief Provide a string representation of the ControlRange\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 20AE460BFF\n\tfor ;\n\tWed, 4 Dec 2019 16:48:40 +0100 (CET)","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 7CCB92E5;\n\tWed, 4 Dec 2019 16:48:39 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1575474519;\n\tbh=JvyNgYLjrjiOHqJt+lYU7xL8tl2FzKCe4zHVyhZ50uA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Sodkg/AqbueU5Y1vqfv3SJ4AlljHN8kVc+FHsgX5TvDLnbhVYLboBcQoYLyd03FLq\n\tel6KUbgqIZKxt6GPxxNTzhJvA3okiH5HqQxr0ZEqz1WyswUp0Qs0xwwlqvZO3GI9R0\n\tW9ol/2n8lU7o9iFJn5u5w3ANIBtW/B/dCJRBvpY8=","Date":"Wed, 4 Dec 2019 17:48:32 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191204154832.GC7811@pendragon.ideasonboard.com>","References":"<20191204132106.21582-1-jacopo@jmondi.org>\n\t<20191204132106.21582-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191204132106.21582-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 04/10] libcamera: controls: Add\n\tdefault to ControlRange","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, 04 Dec 2019 15:48:40 -0000"}},{"id":3184,"web_url":"https://patchwork.libcamera.org/comment/3184/","msgid":"<20191205095313.er37rr7aamdfruxb@uno.localdomain>","date":"2019-12-05T09:53:13","subject":"Re: [libcamera-devel] [PATCH 04/10] libcamera: controls: Add\n\tdefault to ControlRange","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Dec 04, 2019 at 05:48:32PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 04, 2019 at 02:21:00PM +0100, Jacopo Mondi wrote:\n> > Augment the the ControlRange class to store the control default value.\n> >\n> > This is particularly relevant for v4l2 controls used to create\n> > Camera properties, which are constructed using immutable video device\n> > properties, whose value won't change at runtime.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > include/libcamera/controls.h | 7 ++++++-\n> > src/libcamera/controls.cpp | 17 +++++++++++++++--\n> > 2 files changed, 21 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index b1b73367e874..1e2f284eafeb 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -114,10 +114,12 @@ class ControlRange\n> > {\n> > public:\n> > \texplicit ControlRange(const ControlValue &min = 0,\n> > -\t\t\t const ControlValue &max = 0);\n> > +\t\t\t const ControlValue &max = 0,\n> > +\t\t\t const ControlValue &defaultValue = 0);\n>\n> I wish we could call this default, but that's a reserved keyword :-/\n> Maybe just 'def' to match min and max ? I'll leave it up to you.\n>\n>\n>\n> >\n> > \tconst ControlValue &min() const { return min_; }\n> > \tconst ControlValue &max() const { return max_; }\n> > +\tconst ControlValue &defaultValue() const { return defaultValue_; }\n> >\n> > \tstd::string toString() const;\n> >\n> > @@ -131,6 +133,9 @@ public:\n> > \t\treturn !(*this == other);\n> > \t}\n> >\n> > +protected:\n> > +\tControlValue defaultValue_;\n> > +\n>\n> Why is this protected while min_ and max_ are private ? I don't think\n> the default value should be special.\n>\n\nOh, that's a leftover from when I set the default value outside of the\nconstructor in V4L2ControlRange subclass.\n\nI'll fix\n\nThanks\n j\n\n> > private:\n> > \tControlValue min_;\n> > \tControlValue max_;\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 7d8a0e97ee3a..bacba0fbf68a 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -353,14 +353,21 @@ Control::Control(unsigned int id, const char *name)\n> > * pipeline handlers to describe the controls they support.\n> > */\n> >\n> > +/**\n> > + * \\var ControlRange::defaultValue_\n> > + * \\brief The control default value\n> > + */\n> > +\n> > /**\n> > * \\brief Construct a ControlRange with minimum and maximum range parameters\n> > * \\param[in] min The control minimum value\n> > * \\param[in] max The control maximum value\n> > + * \\param[in] defaultValue The control default value\n> > */\n> > ControlRange::ControlRange(const ControlValue &min,\n> > -\t\t\t const ControlValue &max)\n> > -\t: min_(min), max_(max)\n> > +\t\t\t const ControlValue &max,\n> > +\t\t\t const ControlValue &defaultValue)\n> > +\t: defaultValue_(defaultValue), min_(min), max_(max)\n> > {\n> > }\n> >\n> > @@ -376,6 +383,12 @@ ControlRange::ControlRange(const ControlValue &min,\n> > * \\return A ControlValue with the maximum value for the control\n> > */\n> >\n> > +/**\n> > + * \\fn ControlRange::defaultValue()\n> > + * \\brief Retrieve the default value of the control\n> > + * \\return A ControlValue with the default value for the control\n> > + */\n> > +\n> > /**\n> > * \\brief Provide a string representation of the ControlRange\n> > */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B41A60BBC\n\tfor ;\n\tThu, 5 Dec 2019 10:51:03 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id D0B8AE000E;\n\tThu, 5 Dec 2019 09:51:02 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 5 Dec 2019 10:53:13 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191205095313.er37rr7aamdfruxb@uno.localdomain>","References":"<20191204132106.21582-1-jacopo@jmondi.org>\n\t<20191204132106.21582-5-jacopo@jmondi.org>\n\t<20191204154832.GC7811@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"7skqcnpxnfvamrd5\"","Content-Disposition":"inline","In-Reply-To":"<20191204154832.GC7811@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 04/10] libcamera: controls: Add\n\tdefault to ControlRange","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":"Thu, 05 Dec 2019 09:51:03 -0000"}}]