[{"id":2067,"web_url":"https://patchwork.libcamera.org/comment/2067/","msgid":"<8d71dbc0-6312-a5af-72ce-09e5dffd0bc8@ideasonboard.com>","date":"2019-07-01T09:25:10","subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 01/07/2019 00:38, Laurent Pinchart wrote:\n> The ControlList class implements a map from control specifier to control\n> ID. To avoid constant lookups of ControlInfo when using the class in the\n> libcamera core or in pipeline handlers, the map uses ControlInfo\n> pointers instead of ControlId values. This is however not very\n> convenient.\n\nThis paragraph sort of 'stops'. Perhaps needs a finishing:\n\n\"\"\"\nFacilitate ease of use of ControlLists by implementing an internal\nlookup of the ControlInfo from the controls provided by the Camera.\n\"\"\"\n\n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Laurent Pinchart \n\n> ---\n> include/libcamera/controls.h | 2 ++\n> src/libcamera/controls.cpp | 57 ++++++++++++++++++++++++++++++++++++\n> 2 files changed, 59 insertions(+)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 9b37dfb16b89..d827318ee0fa 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -117,11 +117,13 @@ public:\n> \tconst_iterator begin() const { return controls_.begin(); }\n> \tconst_iterator end() const { return controls_.end(); }\n> \n> +\tbool contains(ControlId id) const;\n> \tbool contains(const ControlInfo *info) const { return controls_.count(info); };\n> \tbool empty() const { return controls_.empty(); }\n> \tstd::size_t size() const { return controls_.size(); }\n> \tvoid clear() { controls_.clear(); }\n> \n> +\tControlValue &operator[](ControlId id);\n> \tControlValue &operator[](const ControlInfo *info) { return controls_[info]; }\n> \n> \tvoid update(const ControlList &list);\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index e4c41b97a354..17e09fc7f153 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -10,6 +10,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \"log.h\"\n> #include \"utils.h\"\n> \n> @@ -372,6 +374,30 @@ ControlList::ControlList(Camera *camera)\n> * instance\n> */\n> \n> +/**\n> + * \\brief Check if the ist contains a control with the specified \\a id\n\ns/ist/List/\n\n> + * \\param[in] id The control ID\n> + *\n> + * The behaviour is undefined if the control \\a id is not supported by the\n> + * camera that the ControlList refers to.\n> + *\n> + * \\return True if the list contains a matching control, false otherwise\n> + */\n> +bool ControlList::contains(ControlId id) const\n> +{\n> +\tconst ControlInfoMap &controls = camera_->controls();\n> +\tconst auto iter = controls.find(id);\n> +\tif (iter == controls.end()) {\n> +\t\tLOG(Controls, Error)\n> +\t\t\t<< \"Camera \" << camera_->name()\n> +\t\t\t<< \" does not support control \" << id;\n> +\n> +\t\treturn false;\n> +\t}\n> +\n> +\treturn controls_.find(&iter->second) != controls_.end();\n> +}\n> +\n> /**\n> * \\fn ControlList::contains(const ControlInfo *info) const\n> * \\brief Check if the ist contains a control with the specified \\a info\n> @@ -396,6 +422,34 @@ ControlList::ControlList(Camera *camera)\n> * \\brief Removes all controls from the list\n> */\n> \n> +/**\n> + * \\brief Access or insert the control specified by \\a id\n> + * \\param[in] id The control ID\n> + *\n> + * This method returns a reference to the control identified by \\a id, inserting\n> + * it in the list if the ID is not already present.\n> + *\n> + * The behaviour is undefined if the control \\a id is not supported by the\n> + * camera that the ControlList refers to.\n> + *\n> + * \\return A reference to the value of the control identified by \\a id\n> + */\n> +ControlValue &ControlList::operator[](ControlId id)\n> +{\n> +\tconst ControlInfoMap &controls = camera_->controls();\n> +\tconst auto iter = controls.find(id);\n> +\tif (iter == controls.end()) {\n> +\t\tLOG(Controls, Error)\n> +\t\t\t<< \"Camera \" << camera_->name()\n> +\t\t\t<< \" does not support control \" << id;\n> +\n> +\t\tstatic ControlValue empty;\n> +\t\treturn empty;\n> +\t}\n> +\n> +\treturn controls_[&iter->second];\n> +}\n> +\n> /**\n> * \\fn ControlList::operator[](const ControlInfo *info)\n> * \\brief Access or insert the control specified by \\a info\n> @@ -413,6 +467,9 @@ ControlList::ControlList(Camera *camera)\n> *\n> * Update all controls in the ControlList, by the values given by \\a list\n> * If the list already contains a control of this ID then it will be overwritten\n> + *\n> + * The behaviour is undefined if the two lists refer to different Camera\n> + * instances.\n> */\n> void ControlList::update(const ControlList &list)\n> {\n>","headers":{"Return-Path":"","Received":["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 00A7961950\n\tfor ;\n\tMon, 1 Jul 2019 11:25:13 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B015524;\n\tMon, 1 Jul 2019 11:25:13 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561973113;\n\tbh=ypS6VRf151eSCQrExxt1GMv2CM6FvQ9ePasD/lq08SI=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ANd1iKaZ0Pa7dQ4zwHf0RkZMiFUike3KvxTuC65VHW4lqUJrSwMqiPAChaebvJQvB\n\textJScZ3cIgIMqU78AIFSUWK23FEA5zkcI8p5Z00TPaI0Qtp8loRNsnSU8VtOdQX8v\n\tZNOkVeKrXCjiphPao24h6oM8VqTyB+xVTnInMZKE=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-7-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<8d71dbc0-6312-a5af-72ce-09e5dffd0bc8@ideasonboard.com>","Date":"Mon, 1 Jul 2019 10:25:10 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190630233817.10130-7-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 01 Jul 2019 09:25:14 -0000"}},{"id":2072,"web_url":"https://patchwork.libcamera.org/comment/2072/","msgid":"<20190701111129.GD5018@pendragon.ideasonboard.com>","date":"2019-07-01T11:11:29","subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jul 01, 2019 at 10:25:10AM +0100, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 01/07/2019 00:38, Laurent Pinchart wrote:\n> > The ControlList class implements a map from control specifier to control\n> > ID. To avoid constant lookups of ControlInfo when using the class in the\n> > libcamera core or in pipeline handlers, the map uses ControlInfo\n> > pointers instead of ControlId values. This is however not very\n> > convenient.\n> \n> This paragraph sort of 'stops'. Perhaps needs a finishing:\n> \n> \"\"\"\n> Facilitate ease of use of ControlLists by implementing an internal\n> lookup of the ControlInfo from the controls provided by the Camera.\n> \"\"\"\n\nIndeed. I started writing the commit message, got interrupted, and then\nforgot about it.\n\n\"The ControlList class implements a map from control specifier to control\nID. To avoid constant lookups of ControlInfo when using the class in the\nlibcamera core or in pipeline handlers, the map uses ControlInfo\npointers instead of ControlId values. This is however not very\nconvenient for applications or pipeline handlers, as they would be \nforced to first look up the ControlInfo pointers for the controls they \nwant to access. Facilitate ease of use of ControlLists by implementing \nan internal lookup of the ControlInfo from the controls provided by the\nCamera.\"\n\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Laurent Pinchart \n> \n> > ---\n> > include/libcamera/controls.h | 2 ++\n> > src/libcamera/controls.cpp | 57 ++++++++++++++++++++++++++++++++++++\n> > 2 files changed, 59 insertions(+)\n> > \n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 9b37dfb16b89..d827318ee0fa 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -117,11 +117,13 @@ public:\n> > \tconst_iterator begin() const { return controls_.begin(); }\n> > \tconst_iterator end() const { return controls_.end(); }\n> > \n> > +\tbool contains(ControlId id) const;\n> > \tbool contains(const ControlInfo *info) const { return controls_.count(info); };\n> > \tbool empty() const { return controls_.empty(); }\n> > \tstd::size_t size() const { return controls_.size(); }\n> > \tvoid clear() { controls_.clear(); }\n> > \n> > +\tControlValue &operator[](ControlId id);\n> > \tControlValue &operator[](const ControlInfo *info) { return controls_[info]; }\n> > \n> > \tvoid update(const ControlList &list);\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index e4c41b97a354..17e09fc7f153 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -10,6 +10,8 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +\n> > #include \"log.h\"\n> > #include \"utils.h\"\n> > \n> > @@ -372,6 +374,30 @@ ControlList::ControlList(Camera *camera)\n> > * instance\n> > */\n> > \n> > +/**\n> > + * \\brief Check if the ist contains a control with the specified \\a id\n> \n> s/ist/List/\n> \n> > + * \\param[in] id The control ID\n> > + *\n> > + * The behaviour is undefined if the control \\a id is not supported by the\n> > + * camera that the ControlList refers to.\n> > + *\n> > + * \\return True if the list contains a matching control, false otherwise\n> > + */\n> > +bool ControlList::contains(ControlId id) const\n> > +{\n> > +\tconst ControlInfoMap &controls = camera_->controls();\n> > +\tconst auto iter = controls.find(id);\n> > +\tif (iter == controls.end()) {\n> > +\t\tLOG(Controls, Error)\n> > +\t\t\t<< \"Camera \" << camera_->name()\n> > +\t\t\t<< \" does not support control \" << id;\n> > +\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\treturn controls_.find(&iter->second) != controls_.end();\n> > +}\n> > +\n> > /**\n> > * \\fn ControlList::contains(const ControlInfo *info) const\n> > * \\brief Check if the ist contains a control with the specified \\a info\n> > @@ -396,6 +422,34 @@ ControlList::ControlList(Camera *camera)\n> > * \\brief Removes all controls from the list\n> > */\n> > \n> > +/**\n> > + * \\brief Access or insert the control specified by \\a id\n> > + * \\param[in] id The control ID\n> > + *\n> > + * This method returns a reference to the control identified by \\a id, inserting\n> > + * it in the list if the ID is not already present.\n> > + *\n> > + * The behaviour is undefined if the control \\a id is not supported by the\n> > + * camera that the ControlList refers to.\n> > + *\n> > + * \\return A reference to the value of the control identified by \\a id\n> > + */\n> > +ControlValue &ControlList::operator[](ControlId id)\n> > +{\n> > +\tconst ControlInfoMap &controls = camera_->controls();\n> > +\tconst auto iter = controls.find(id);\n> > +\tif (iter == controls.end()) {\n> > +\t\tLOG(Controls, Error)\n> > +\t\t\t<< \"Camera \" << camera_->name()\n> > +\t\t\t<< \" does not support control \" << id;\n> > +\n> > +\t\tstatic ControlValue empty;\n> > +\t\treturn empty;\n> > +\t}\n> > +\n> > +\treturn controls_[&iter->second];\n> > +}\n> > +\n> > /**\n> > * \\fn ControlList::operator[](const ControlInfo *info)\n> > * \\brief Access or insert the control specified by \\a info\n> > @@ -413,6 +467,9 @@ ControlList::ControlList(Camera *camera)\n> > *\n> > * Update all controls in the ControlList, by the values given by \\a list\n> > * If the list already contains a control of this ID then it will be overwritten\n> > + *\n> > + * The behaviour is undefined if the two lists refer to different Camera\n> > + * instances.\n> > */\n> > void ControlList::update(const ControlList &list)\n> > {\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","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 59D5C619E5\n\tfor ;\n\tMon, 1 Jul 2019 13:11:49 +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 D1B0D524;\n\tMon, 1 Jul 2019 13:11:48 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561979509;\n\tbh=XEc/e7gvCdy5cXIM51ocsRmwustrTFA35lZzQLCrNds=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ux8GkgaXjnxcrdvLOcEJZUxTWP/EQUqj6pqCSbZIjMEiKtpf7CIrHIsbtFt9Vb7eh\n\tbPyjmqOFNMGlzEEuv2Kh+hf81F4v6CuKNKtOBvu/nE6xpHYpAWDRkdK4Sbj4dj9Qjd\n\tuWxjm82hoVAlaPARFZtqlHkVIbf9e8vJYmje7TTo=","Date":"Mon, 1 Jul 2019 14:11:29 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701111129.GD5018@pendragon.ideasonboard.com>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-7-laurent.pinchart@ideasonboard.com>\n\t<8d71dbc0-6312-a5af-72ce-09e5dffd0bc8@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<8d71dbc0-6312-a5af-72ce-09e5dffd0bc8@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 01 Jul 2019 11:11:49 -0000"}},{"id":2081,"web_url":"https://patchwork.libcamera.org/comment/2081/","msgid":"<20190701165046.aarmqtxgkz4y4n5u@uno.localdomain>","date":"2019-07-01T16:50:46","subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Mon, Jul 01, 2019 at 02:38:09AM +0300, Laurent Pinchart wrote:\n> The ControlList class implements a map from control specifier to control\n> ID. To avoid constant lookups of ControlInfo when using the class in the\n> libcamera core or in pipeline handlers, the map uses ControlInfo\n> pointers instead of ControlId values. This is however not very\n> convenient.\n>\n> Signed-off-by: Laurent Pinchart \n\nWith Kieran's spelling comment addressed\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/controls.h | 2 ++\n> src/libcamera/controls.cpp | 57 ++++++++++++++++++++++++++++++++++++\n> 2 files changed, 59 insertions(+)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 9b37dfb16b89..d827318ee0fa 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -117,11 +117,13 @@ public:\n> \tconst_iterator begin() const { return controls_.begin(); }\n> \tconst_iterator end() const { return controls_.end(); }\n>\n> +\tbool contains(ControlId id) const;\n> \tbool contains(const ControlInfo *info) const { return controls_.count(info); };\n> \tbool empty() const { return controls_.empty(); }\n> \tstd::size_t size() const { return controls_.size(); }\n> \tvoid clear() { controls_.clear(); }\n>\n> +\tControlValue &operator[](ControlId id);\n> \tControlValue &operator[](const ControlInfo *info) { return controls_[info]; }\n>\n> \tvoid update(const ControlList &list);\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index e4c41b97a354..17e09fc7f153 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -10,6 +10,8 @@\n> #include \n> #include \n>\n> +#include \n> +\n> #include \"log.h\"\n> #include \"utils.h\"\n>\n> @@ -372,6 +374,30 @@ ControlList::ControlList(Camera *camera)\n> * instance\n> */\n>\n> +/**\n> + * \\brief Check if the ist contains a control with the specified \\a id\n> + * \\param[in] id The control ID\n> + *\n> + * The behaviour is undefined if the control \\a id is not supported by the\n> + * camera that the ControlList refers to.\n> + *\n> + * \\return True if the list contains a matching control, false otherwise\n> + */\n> +bool ControlList::contains(ControlId id) const\n> +{\n> +\tconst ControlInfoMap &controls = camera_->controls();\n> +\tconst auto iter = controls.find(id);\n> +\tif (iter == controls.end()) {\n> +\t\tLOG(Controls, Error)\n> +\t\t\t<< \"Camera \" << camera_->name()\n> +\t\t\t<< \" does not support control \" << id;\n> +\n> +\t\treturn false;\n> +\t}\n> +\n> +\treturn controls_.find(&iter->second) != controls_.end();\n> +}\n> +\n> /**\n> * \\fn ControlList::contains(const ControlInfo *info) const\n> * \\brief Check if the ist contains a control with the specified \\a info\n> @@ -396,6 +422,34 @@ ControlList::ControlList(Camera *camera)\n> * \\brief Removes all controls from the list\n> */\n>\n> +/**\n> + * \\brief Access or insert the control specified by \\a id\n> + * \\param[in] id The control ID\n> + *\n> + * This method returns a reference to the control identified by \\a id, inserting\n> + * it in the list if the ID is not already present.\n> + *\n> + * The behaviour is undefined if the control \\a id is not supported by the\n> + * camera that the ControlList refers to.\n> + *\n> + * \\return A reference to the value of the control identified by \\a id\n> + */\n> +ControlValue &ControlList::operator[](ControlId id)\n> +{\n> +\tconst ControlInfoMap &controls = camera_->controls();\n> +\tconst auto iter = controls.find(id);\n> +\tif (iter == controls.end()) {\n> +\t\tLOG(Controls, Error)\n> +\t\t\t<< \"Camera \" << camera_->name()\n> +\t\t\t<< \" does not support control \" << id;\n> +\n> +\t\tstatic ControlValue empty;\n> +\t\treturn empty;\n> +\t}\n> +\n> +\treturn controls_[&iter->second];\n> +}\n> +\n> /**\n> * \\fn ControlList::operator[](const ControlInfo *info)\n> * \\brief Access or insert the control specified by \\a info\n> @@ -413,6 +467,9 @@ ControlList::ControlList(Camera *camera)\n> *\n> * Update all controls in the ControlList, by the values given by \\a list\n> * If the list already contains a control of this ID then it will be overwritten\n> + *\n> + * The behaviour is undefined if the two lists refer to different Camera\n> + * instances.\n> */\n> void ControlList::update(const ControlList &list)\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 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 01E9560BF8\n\tfor ;\n\tMon, 1 Jul 2019 18:49:29 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 7E5F71BF20E;\n\tMon, 1 Jul 2019 16:49:29 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 1 Jul 2019 18:50:46 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701165046.aarmqtxgkz4y4n5u@uno.localdomain>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-7-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"riplpxh2hveohtpf\"","Content-Disposition":"inline","In-Reply-To":"<20190630233817.10130-7-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v3 06/14] libcamera: controls: Extend\n\tControlList to access controls by ID","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 01 Jul 2019 16:49:30 -0000"}}]