[{"id":13336,"web_url":"https://patchwork.libcamera.org/comment/13336/","msgid":"<0e7a172c-6496-0539-2639-c1cd358de7a8@ideasonboard.com>","date":"2020-10-21T12:50:12","subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\n\nOn 20/10/2020 19:05, Jacopo Mondi wrote:\n> Add a constructor to the ControlInfo class that allows creating\n> a class instance from the list of the control supported values.\n> \n\nI see.\n\nplease ignore the request in my previous patch ;-) hehe\n\n\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/controls.h |  1 +\n>  src/libcamera/controls.cpp   | 17 +++++++++++++++++\n>  2 files changed, 18 insertions(+)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index d1f6d4490c35..d4fdf5807f1c 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -269,6 +269,7 @@ public:\n>  \t\t\t     const ControlValue &max = 0,\n>  \t\t\t     const ControlValue &def = 0,\n>  \t\t\t     const std::vector<ControlValue> &values = {});\n> +\texplicit ControlInfo(const std::vector<ControlValue> &values);\n>  \n>  \tconst ControlValue &min() const { return min_; }\n>  \tconst ControlValue &max() const { return max_; }\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 61feee37a1b8..e80f6116a684 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -493,6 +493,23 @@ ControlInfo::ControlInfo(const ControlValue &min,\n>  {\n>  }\n>  \n> +/**\n> + * \\brief Construct a ControlInfo from the list of supported values\n> + * \\param[in] values The control supported values\n> + *\n> + * Construct a ControlInfo from a list of supported values. The ControlInfo\n> + * minimum and maximum values are assigned to the first and last members of\n> + * the values list respectively. The ControlInfo default value is set to be\n> + * equal to the minimum one.\n> + */\n> +ControlInfo::ControlInfo(const std::vector<ControlValue> &values)\n> +\t: values_(values)\n> +{\n> +\tmin_ = values_.front();\n> +\tmax_ = values_.back();\n\nThat feels like a bit of an assumption that the list of values is sorted.\n\nMaybe there should be at least some extra checks here?\n\nOr just start with a values.sort() ?\n\n\n> +\tdef_ = min_;\n\nAnd I wonder if this should be (optionally?) supplied in the constructor\narguments?\n\nAnyway, with or without those comments:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> +}\n> +\n>  /**\n>   * \\fn ControlInfo::min()\n>   * \\brief Retrieve the minimum value of the control\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CE82ABDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Oct 2020 12:50:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66B7460CE0;\n\tWed, 21 Oct 2020 14:50:18 +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 6D0C260CDD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Oct 2020 14:50:17 +0200 (CEST)","from [192.168.0.20]\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 DE82892;\n\tWed, 21 Oct 2020 14:50:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gNSIkEGJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603284617;\n\tbh=XePm9b6X7YfxN9ayFvzUJj7WTsxz/jPLuGLkDeQwGO4=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=gNSIkEGJghU0bmuM6I86jAE1/PUQ6rGVAJ1gp6TFNB/L3QrDOPMVV4kUzxIVUaIXs\n\thbu6gT9cHy28mdupVTIIWzqfeJMYO1Lr9gOyn8KDOP7UoLWx3mlfzHNLWF7JDZTgoE\n\t4f8Ov+wm0Ny+J/A4boa+tegD6yyujoXQ2xOxecu0=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20201020180534.36855-1-jacopo@jmondi.org>\n\t<20201020180534.36855-5-jacopo@jmondi.org>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<0e7a172c-6496-0539-2639-c1cd358de7a8@ideasonboard.com>","Date":"Wed, 21 Oct 2020 13:50:12 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201020180534.36855-5-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13349,"web_url":"https://patchwork.libcamera.org/comment/13349/","msgid":"<20201021133109.6ly3omznns6dcbio@uno.localdomain>","date":"2020-10-21T13:31:09","subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Wed, Oct 21, 2020 at 01:50:12PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n>\n> On 20/10/2020 19:05, Jacopo Mondi wrote:\n> > Add a constructor to the ControlInfo class that allows creating\n> > a class instance from the list of the control supported values.\n> >\n>\n> I see.\n>\n> please ignore the request in my previous patch ;-) hehe\n>\n>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  include/libcamera/controls.h |  1 +\n> >  src/libcamera/controls.cpp   | 17 +++++++++++++++++\n> >  2 files changed, 18 insertions(+)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index d1f6d4490c35..d4fdf5807f1c 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -269,6 +269,7 @@ public:\n> >  \t\t\t     const ControlValue &max = 0,\n> >  \t\t\t     const ControlValue &def = 0,\n> >  \t\t\t     const std::vector<ControlValue> &values = {});\n> > +\texplicit ControlInfo(const std::vector<ControlValue> &values);\n> >\n> >  \tconst ControlValue &min() const { return min_; }\n> >  \tconst ControlValue &max() const { return max_; }\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 61feee37a1b8..e80f6116a684 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -493,6 +493,23 @@ ControlInfo::ControlInfo(const ControlValue &min,\n> >  {\n> >  }\n> >\n> > +/**\n> > + * \\brief Construct a ControlInfo from the list of supported values\n> > + * \\param[in] values The control supported values\n> > + *\n> > + * Construct a ControlInfo from a list of supported values. The ControlInfo\n> > + * minimum and maximum values are assigned to the first and last members of\n> > + * the values list respectively. The ControlInfo default value is set to be\n> > + * equal to the minimum one.\n> > + */\n> > +ControlInfo::ControlInfo(const std::vector<ControlValue> &values)\n> > +\t: values_(values)\n> > +{\n> > +\tmin_ = values_.front();\n> > +\tmax_ = values_.back();\n>\n> That feels like a bit of an assumption that the list of values is sorted.\n>\n> Maybe there should be at least some extra checks here?\n>\n> Or just start with a values.sort() ?\n\nWell, enums are defined in the .yaml file, we're in control of them,\naren't we ?\n>\n>\n> > +\tdef_ = min_;\n>\n> And I wonder if this should be (optionally?) supplied in the constructor\n> arguments?\n\nThis might be worth it, I agree.\n\n>\n> Anyway, with or without those comments:\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> > +}\n> > +\n> >  /**\n> >   * \\fn ControlInfo::min()\n> >   * \\brief Retrieve the minimum value of the control\n> >\n>\n> --\n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 709FFBDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Oct 2020 13:31:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A99F61DDB;\n\tWed, 21 Oct 2020 15:31:12 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 857E261D7F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Oct 2020 15:31:11 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 10F721BF203;\n\tWed, 21 Oct 2020 13:31:10 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 21 Oct 2020 15:31:09 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201021133109.6ly3omznns6dcbio@uno.localdomain>","References":"<20201020180534.36855-1-jacopo@jmondi.org>\n\t<20201020180534.36855-5-jacopo@jmondi.org>\n\t<0e7a172c-6496-0539-2639-c1cd358de7a8@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<0e7a172c-6496-0539-2639-c1cd358de7a8@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13352,"web_url":"https://patchwork.libcamera.org/comment/13352/","msgid":"<0f8657a8-baf6-60dc-ae06-2015bfdb68fa@ideasonboard.com>","date":"2020-10-21T13:37:03","subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 21/10/2020 14:31, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Wed, Oct 21, 2020 at 01:50:12PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>>\n>> On 20/10/2020 19:05, Jacopo Mondi wrote:\n>>> Add a constructor to the ControlInfo class that allows creating\n>>> a class instance from the list of the control supported values.\n>>>\n>>\n>> I see.\n>>\n>> please ignore the request in my previous patch ;-) hehe\n>>\n>>\n>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>> ---\n>>>  include/libcamera/controls.h |  1 +\n>>>  src/libcamera/controls.cpp   | 17 +++++++++++++++++\n>>>  2 files changed, 18 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>> index d1f6d4490c35..d4fdf5807f1c 100644\n>>> --- a/include/libcamera/controls.h\n>>> +++ b/include/libcamera/controls.h\n>>> @@ -269,6 +269,7 @@ public:\n>>>  \t\t\t     const ControlValue &max = 0,\n>>>  \t\t\t     const ControlValue &def = 0,\n>>>  \t\t\t     const std::vector<ControlValue> &values = {});\n>>> +\texplicit ControlInfo(const std::vector<ControlValue> &values);\n>>>\n>>>  \tconst ControlValue &min() const { return min_; }\n>>>  \tconst ControlValue &max() const { return max_; }\n>>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>>> index 61feee37a1b8..e80f6116a684 100644\n>>> --- a/src/libcamera/controls.cpp\n>>> +++ b/src/libcamera/controls.cpp\n>>> @@ -493,6 +493,23 @@ ControlInfo::ControlInfo(const ControlValue &min,\n>>>  {\n>>>  }\n>>>\n>>> +/**\n>>> + * \\brief Construct a ControlInfo from the list of supported values\n>>> + * \\param[in] values The control supported values\n>>> + *\n>>> + * Construct a ControlInfo from a list of supported values. The ControlInfo\n>>> + * minimum and maximum values are assigned to the first and last members of\n>>> + * the values list respectively. The ControlInfo default value is set to be\n>>> + * equal to the minimum one.\n>>> + */\n>>> +ControlInfo::ControlInfo(const std::vector<ControlValue> &values)\n>>> +\t: values_(values)\n>>> +{\n>>> +\tmin_ = values_.front();\n>>> +\tmax_ = values_.back();\n>>\n>> That feels like a bit of an assumption that the list of values is sorted.\n>>\n>> Maybe there should be at least some extra checks here?\n>>\n>> Or just start with a values.sort() ?\n> \n> Well, enums are defined in the .yaml file, we're in control of them,\n> aren't we ?\n\nHrm ... we are indeed putting them in the yaml ... I'm still a bit weary\n- but indeed that allays my initial fear.\n\nI would have suggested we could go one step further and make the\ngenerator script give the enums their values ... but there might be\ncases were we want to give explicit values ... so that gets complicated.\n\nOk, so leave this as is for now. If we ever hit an unsorted/unordered\nlist I'll try really hard not to say I told you so ;-)\n\n(but you're right, I don't think that should happen) hehe\n\n>>\n>>\n>>> +\tdef_ = min_;\n>>\n>> And I wonder if this should be (optionally?) supplied in the constructor\n>> arguments?\n> \n> This might be worth it, I agree.\n> \n>>\n>> Anyway, with or without those comments:\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>> +}\n>>> +\n>>>  /**\n>>>   * \\fn ControlInfo::min()\n>>>   * \\brief Retrieve the minimum value of the control\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 2DE84BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Oct 2020 13:37:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC95161E01;\n\tWed, 21 Oct 2020 15:37:08 +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 B521461D7F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Oct 2020 15:37:07 +0200 (CEST)","from [192.168.0.20]\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 DBF2192;\n\tWed, 21 Oct 2020 15:37:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"H0BO2ni8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603287427;\n\tbh=HO6eV5sTHBq26MA8cUZgl/ul48NOWanm0H3b26jax8A=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=H0BO2ni8MoRz6ydT2pT2ogzeGQ3yAqpfJJL9OXCTRvCConWUs/wwpqi79sqyAawcK\n\tWkI5nv1AI4/tag2A093sR/2h/vGhuEYZODcAi+1PMeEFFSXekvkix6l+Ogm7TjcJIi\n\tXwug4jPAbjS9v7Su8l1L5ApmEbb6fkFiY5ZHfzBM=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20201020180534.36855-1-jacopo@jmondi.org>\n\t<20201020180534.36855-5-jacopo@jmondi.org>\n\t<0e7a172c-6496-0539-2639-c1cd358de7a8@ideasonboard.com>\n\t<20201021133109.6ly3omznns6dcbio@uno.localdomain>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<0f8657a8-baf6-60dc-ae06-2015bfdb68fa@ideasonboard.com>","Date":"Wed, 21 Oct 2020 14:37:03 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201021133109.6ly3omznns6dcbio@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 04/13] libcamera: controls:\n\tConstruct from values list","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]