[{"id":2064,"web_url":"https://patchwork.libcamera.org/comment/2064/","msgid":"<813e54e9-0b64-15d6-2b23-a903dc40ceec@ideasonboard.com>","date":"2019-07-01T08:03:09","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","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> Add a new controls() method to the V4L2Device class to retrieve the map\n> of all supported controls. This is needed in order to dynamically query\n> the supported controls, for instance for drivers that support different\n> sets of controls depending on the device model.\n> \n> To make the API easier to use, create a type alias for the control ID to\n> ControlInfo and use it.\n> \n> Remove the getControlInfo() method that is not used externally, as it\n> can now be replaced by accessing the full list of controls.\n\nMaybe it's the C-dev in me but I kinda prefer the lookup, with a null\nreturn check over a lookup with open coding the iter != controls_.end();\n\nFunctionally the same I guess, I think it's just more ingrained in my\nhead, but the .end() check is more 'STL' so it's certainly not a problem\nreally.\n\n\n\n> Update the CameraSensor API accordingly.\n\nSmall comment on the map type chosen, but I think either std::map is\nprobably fine too.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/camera_sensor.cpp       | 10 ++++------\n>  src/libcamera/include/camera_sensor.h |  5 ++---\n>  src/libcamera/include/v4l2_controls.h |  6 ++++--\n>  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n>  src/libcamera/v4l2_controls.cpp       |  5 +++++\n>  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n>  6 files changed, 28 insertions(+), 32 deletions(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 471c3e2e6e47..14f631e8ecf7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n>  }\n>  \n>  /**\n> - * \\brief Retrieve information about a control\n> - * \\param[in] id The control ID\n> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> - * if the sensor doesn't have such a control\n> + * \\brief Retrieve the supported V4L2 controls\n> + * \\return A map of the V4L2 controls supported by the sensor\n>   */\n> -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n> +const V4L2ControlInfoMap &CameraSensor::controls() const\n>  {\n> -\treturn subdev_->getControlInfo(id);\n> +\treturn subdev_->controls();\n>  }\n>  \n>  /**\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index b42e7b8e1343..fe033fb374c1 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -13,12 +13,11 @@\n>  #include <libcamera/geometry.h>\n>  \n>  #include \"log.h\"\n> +#include \"v4l2_controls.h\"\n>  \n>  namespace libcamera {\n>  \n>  class MediaEntity;\n> -class V4L2ControlInfo;\n> -class V4L2ControlList;\n>  class V4L2Subdevice;\n>  \n>  struct V4L2SubdeviceFormat;\n> @@ -43,7 +42,7 @@ public:\n>  \t\t\t\t      const Size &size) const;\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>  \n> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> +\tconst V4L2ControlInfoMap &controls() const;\n>  \tint getControls(V4L2ControlList *ctrls);\n>  \tint setControls(V4L2ControlList *ctrls);\n>  \n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 0047efab11fa..10b726504951 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -8,11 +8,11 @@\n>  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n>  #define __LIBCAMERA_V4L2_CONTROLS_H__\n>  \n> +#include <map>\n> +#include <stdint.h>\n>  #include <string>\n>  #include <vector>\n>  \n> -#include <stdint.h>\n> -\n>  #include <linux/v4l2-controls.h>\n>  #include <linux/videodev2.h>\n>  \n> @@ -41,6 +41,8 @@ private:\n>  \tint64_t max_;\n>  };\n>  \n> +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n\nDoes V4L2ControlInfoMap need to be ordered?\n(I don't think we do at this part do we?)\n\nstd::unordered_map could be faster if we don't need to order the map.\nI think at the scale of controls we'll have it probably won't make much\ndifference...\n\nWe could do some measurements perhaps on a map of 1000 elements vs an\nunordered_map of 1000 elements, but I think that's the approx max scale\nthat we'll contain right?\n\nAs the key is an unsigned int, I think the hasher will function out of\nthe box?\n\n\n> +\n>  class V4L2Control\n>  {\n>  public:\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 24a0134a2cba..e7e9829cb05f 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -13,19 +13,18 @@\n>  #include <linux/videodev2.h>\n>  \n>  #include \"log.h\"\n> +#include \"v4l2_controls.h\"\n>  \n>  namespace libcamera {\n>  \n> -class V4L2ControlInfo;\n> -class V4L2ControlList;\n> -\n>  class V4L2Device : protected Loggable\n>  {\n>  public:\n>  \tvoid close();\n>  \tbool isOpen() const { return fd_ != -1; }\n>  \n> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> +\n>  \tint getControls(V4L2ControlList *ctrls);\n>  \tint setControls(V4L2ControlList *ctrls);\n>  \n> @@ -48,7 +47,7 @@ private:\n>  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t    unsigned int count);\n>  \n> -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n> +\tV4L2ControlInfoMap controls_;\n>  \tstd::string deviceNode_;\n>  \tint fd_;\n>  };\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index af017bce48ba..84258d9954d0 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n>   * \\return The V4L2 control maximum value\n>   */\n>  \n> +/**\n> + * \\typedef V4L2ControlInfoMap\n> + * \\brief A map of control ID to V4L2ControlInfo\n> + */\n> +\n>  /**\n>   * \\class V4L2Control\n>   * \\brief A V4L2 control value\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 06939dead705..59fc98cefd4e 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -111,19 +111,10 @@ void V4L2Device::close()\n>   */\n>  \n>  /**\n> - * \\brief Retrieve information about a control\n> - * \\param[in] id The control ID\n> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> - * if the device doesn't have such a control\n> + * \\fn V4L2Device::controls()\n> + * \\brief Retrieve the supported V4L2 controls\n> + * \\return A map of the V4L2 controls supported by the device\n>   */\n> -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n> -{\n> -\tauto it = controls_.find(id);\n> -\tif (it == controls_.end())\n> -\t\treturn nullptr;\n> -\n> -\treturn &it->second;\n> -}\n>  \n>  /**\n>   * \\brief Read controls from the device\n> @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>  \n>  \tfor (unsigned int i = 0; i < count; ++i) {\n>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> -\t\tif (!info) {\n> +\t\tconst auto iter = controls_.find(ctrl->id());\n> +\t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n> +\t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n>  \t\tv4l2Ctrls[i].id = info->id();\n>  \t}\n> @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n>  \n>  \tfor (unsigned int i = 0; i < count; ++i) {\n>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> -\t\tif (!info) {\n> +\t\tconst auto iter = controls_.find(ctrl->id());\n> +\t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n> +\t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n>  \t\tv4l2Ctrls[i].id = info->id();\n>  \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","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 16C6260C2B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 10:03: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 A9B45524;\n\tMon,  1 Jul 2019 10:03:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561968192;\n\tbh=6hcnygnbZGj03MKe/GIE+AOcNlyp3w2JFSMyP6GPvvc=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=HwlFCAIkf4zSBk2JLG/pWj/L9y4HT32n2anDnK0i8J9kFGGPKT0HHGGJ8kH50edSb\n\tyXG5wer+USUFlbXQ6GgpA7Q6ZBK6MeEoZe33CsLNUEolxgB0s3ILjmLtmIUCoPd3sB\n\tSh3qU+Ko6yHqvVu/tRq7VP6q+CQyhQ5yv/14hFn8=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<813e54e9-0b64-15d6-2b23-a903dc40ceec@ideasonboard.com>","Date":"Mon, 1 Jul 2019 09:03:09 +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-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Mon, 01 Jul 2019 08:03:13 -0000"}},{"id":2070,"web_url":"https://patchwork.libcamera.org/comment/2070/","msgid":"<20190701110047.GB5018@pendragon.ideasonboard.com>","date":"2019-07-01T11:00:47","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jul 01, 2019 at 09:03:09AM +0100, Kieran Bingham wrote:\n> On 01/07/2019 00:38, Laurent Pinchart wrote:\n> > Add a new controls() method to the V4L2Device class to retrieve the map\n> > of all supported controls. This is needed in order to dynamically query\n> > the supported controls, for instance for drivers that support different\n> > sets of controls depending on the device model.\n> > \n> > To make the API easier to use, create a type alias for the control ID to\n> > ControlInfo and use it.\n> > \n> > Remove the getControlInfo() method that is not used externally, as it\n> > can now be replaced by accessing the full list of controls.\n> \n> Maybe it's the C-dev in me but I kinda prefer the lookup, with a null\n> return check over a lookup with open coding the iter != controls_.end();\n> \n> Functionally the same I guess, I think it's just more ingrained in my\n> head, but the .end() check is more 'STL' so it's certainly not a problem\n> really.\n\nI agree, so if we find this is too much hasle to use, I'm fine adding\nback a method to look up a single control (or I wonder if we could add a\nhelper method to make look up easier on std::map).\n\n> > Update the CameraSensor API accordingly.\n> \n> Small comment on the map type chosen, but I think either std::map is\n> probably fine too.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/camera_sensor.cpp       | 10 ++++------\n> >  src/libcamera/include/camera_sensor.h |  5 ++---\n> >  src/libcamera/include/v4l2_controls.h |  6 ++++--\n> >  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n> >  src/libcamera/v4l2_controls.cpp       |  5 +++++\n> >  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n> >  6 files changed, 28 insertions(+), 32 deletions(-)\n> > \n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 471c3e2e6e47..14f631e8ecf7 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> >  }\n> >  \n> >  /**\n> > - * \\brief Retrieve information about a control\n> > - * \\param[in] id The control ID\n> > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > - * if the sensor doesn't have such a control\n> > + * \\brief Retrieve the supported V4L2 controls\n> > + * \\return A map of the V4L2 controls supported by the sensor\n> >   */\n> > -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n> > +const V4L2ControlInfoMap &CameraSensor::controls() const\n> >  {\n> > -\treturn subdev_->getControlInfo(id);\n> > +\treturn subdev_->controls();\n> >  }\n> >  \n> >  /**\n> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> > index b42e7b8e1343..fe033fb374c1 100644\n> > --- a/src/libcamera/include/camera_sensor.h\n> > +++ b/src/libcamera/include/camera_sensor.h\n> > @@ -13,12 +13,11 @@\n> >  #include <libcamera/geometry.h>\n> >  \n> >  #include \"log.h\"\n> > +#include \"v4l2_controls.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> >  class MediaEntity;\n> > -class V4L2ControlInfo;\n> > -class V4L2ControlList;\n> >  class V4L2Subdevice;\n> >  \n> >  struct V4L2SubdeviceFormat;\n> > @@ -43,7 +42,7 @@ public:\n> >  \t\t\t\t      const Size &size) const;\n> >  \tint setFormat(V4L2SubdeviceFormat *format);\n> >  \n> > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > +\tconst V4L2ControlInfoMap &controls() const;\n> >  \tint getControls(V4L2ControlList *ctrls);\n> >  \tint setControls(V4L2ControlList *ctrls);\n> >  \n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index 0047efab11fa..10b726504951 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -8,11 +8,11 @@\n> >  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n> >  #define __LIBCAMERA_V4L2_CONTROLS_H__\n> >  \n> > +#include <map>\n> > +#include <stdint.h>\n> >  #include <string>\n> >  #include <vector>\n> >  \n> > -#include <stdint.h>\n> > -\n> >  #include <linux/v4l2-controls.h>\n> >  #include <linux/videodev2.h>\n> >  \n> > @@ -41,6 +41,8 @@ private:\n> >  \tint64_t max_;\n> >  };\n> >  \n> > +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> \n> Does V4L2ControlInfoMap need to be ordered?\n> (I don't think we do at this part do we?)\n> \n> std::unordered_map could be faster if we don't need to order the map.\n> I think at the scale of controls we'll have it probably won't make much\n> difference...\n> \n> We could do some measurements perhaps on a map of 1000 elements vs an\n> unordered_map of 1000 elements, but I think that's the approx max scale\n> that we'll contain right?\n\nI think we'll be one, if not two orders of magnitude smaller. Would you\nlike to perform measurements ? :-)\n\n> As the key is an unsigned int, I think the hasher will function out of\n> the box?\n\nCorrect.\n\n> > +\n> >  class V4L2Control\n> >  {\n> >  public:\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index 24a0134a2cba..e7e9829cb05f 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -13,19 +13,18 @@\n> >  #include <linux/videodev2.h>\n> >  \n> >  #include \"log.h\"\n> > +#include \"v4l2_controls.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > -class V4L2ControlInfo;\n> > -class V4L2ControlList;\n> > -\n> >  class V4L2Device : protected Loggable\n> >  {\n> >  public:\n> >  \tvoid close();\n> >  \tbool isOpen() const { return fd_ != -1; }\n> >  \n> > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> > +\n> >  \tint getControls(V4L2ControlList *ctrls);\n> >  \tint setControls(V4L2ControlList *ctrls);\n> >  \n> > @@ -48,7 +47,7 @@ private:\n> >  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n> >  \t\t\t    unsigned int count);\n> >  \n> > -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n> > +\tV4L2ControlInfoMap controls_;\n> >  \tstd::string deviceNode_;\n> >  \tint fd_;\n> >  };\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index af017bce48ba..84258d9954d0 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> >   * \\return The V4L2 control maximum value\n> >   */\n> >  \n> > +/**\n> > + * \\typedef V4L2ControlInfoMap\n> > + * \\brief A map of control ID to V4L2ControlInfo\n> > + */\n> > +\n> >  /**\n> >   * \\class V4L2Control\n> >   * \\brief A V4L2 control value\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 06939dead705..59fc98cefd4e 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -111,19 +111,10 @@ void V4L2Device::close()\n> >   */\n> >  \n> >  /**\n> > - * \\brief Retrieve information about a control\n> > - * \\param[in] id The control ID\n> > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > - * if the device doesn't have such a control\n> > + * \\fn V4L2Device::controls()\n> > + * \\brief Retrieve the supported V4L2 controls\n> > + * \\return A map of the V4L2 controls supported by the device\n> >   */\n> > -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n> > -{\n> > -\tauto it = controls_.find(id);\n> > -\tif (it == controls_.end())\n> > -\t\treturn nullptr;\n> > -\n> > -\treturn &it->second;\n> > -}\n> >  \n> >  /**\n> >   * \\brief Read controls from the device\n> > @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n> >  \n> >  \tfor (unsigned int i = 0; i < count; ++i) {\n> >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > -\t\tif (!info) {\n> > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > +\t\tif (iter == controls_.end()) {\n> >  \t\t\tLOG(V4L2, Error)\n> >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> >  \t\t\treturn -EINVAL;\n> >  \t\t}\n> >  \n> > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> >  \t\tcontrolInfo[i] = info;\n> >  \t\tv4l2Ctrls[i].id = info->id();\n> >  \t}\n> > @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n> >  \n> >  \tfor (unsigned int i = 0; i < count; ++i) {\n> >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > -\t\tif (!info) {\n> > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > +\t\tif (iter == controls_.end()) {\n> >  \t\t\tLOG(V4L2, Error)\n> >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> >  \t\t\treturn -EINVAL;\n> >  \t\t}\n> >  \n> > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> >  \t\tcontrolInfo[i] = info;\n> >  \t\tv4l2Ctrls[i].id = info->id();\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 B8EB7619E5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 13:01:06 +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 3A97B524;\n\tMon,  1 Jul 2019 13:01:06 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561978866;\n\tbh=KsXvSOP0KiWlv0uCM7xA75aTyIaYm6AWfIiJmmHvQq0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GSIqyMPLsok3cok7Ln3bZDCzdfXYrbROvzriyUSUtHxhsxXOKY2SfLs26zNAwDS+G\n\tOiJiKm3jmlrz7x7lVe+WHZkiedt9mu6aUdKw5s7rq1svEf+KBA0+K+/CGT1ssqv+Ky\n\tHL972EO4tAtLCAGU5woNEFXgAsx94XiN1bCTsNVU=","Date":"Mon, 1 Jul 2019 14:00:47 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701110047.GB5018@pendragon.ideasonboard.com>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>\n\t<813e54e9-0b64-15d6-2b23-a903dc40ceec@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<813e54e9-0b64-15d6-2b23-a903dc40ceec@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Mon, 01 Jul 2019 11:01:06 -0000"}},{"id":2074,"web_url":"https://patchwork.libcamera.org/comment/2074/","msgid":"<20190701150544.x7qe76auu7xm2awo@uno.localdomain>","date":"2019-07-01T15:05:44","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","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:06AM +0300, Laurent Pinchart wrote:\n> Add a new controls() method to the V4L2Device class to retrieve the map\n> of all supported controls. This is needed in order to dynamically query\n> the supported controls, for instance for drivers that support different\n> sets of controls depending on the device model.\n>\n> To make the API easier to use, create a type alias for the control ID to\n> ControlInfo and use it.\n>\n> Remove the getControlInfo() method that is not used externally, as it\n> can now be replaced by accessing the full list of controls.\n>\n> Update the CameraSensor API accordingly.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/camera_sensor.cpp       | 10 ++++------\n>  src/libcamera/include/camera_sensor.h |  5 ++---\n>  src/libcamera/include/v4l2_controls.h |  6 ++++--\n>  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n>  src/libcamera/v4l2_controls.cpp       |  5 +++++\n>  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n>  6 files changed, 28 insertions(+), 32 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 471c3e2e6e47..14f631e8ecf7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n>  }\n>\n>  /**\n> - * \\brief Retrieve information about a control\n> - * \\param[in] id The control ID\n> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> - * if the sensor doesn't have such a control\n> + * \\brief Retrieve the supported V4L2 controls\n> + * \\return A map of the V4L2 controls supported by the sensor\n>   */\n> -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n> +const V4L2ControlInfoMap &CameraSensor::controls() const\n>  {\n> -\treturn subdev_->getControlInfo(id);\n> +\treturn subdev_->controls();\n>  }\n>\n>  /**\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index b42e7b8e1343..fe033fb374c1 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -13,12 +13,11 @@\n>  #include <libcamera/geometry.h>\n>\n>  #include \"log.h\"\n> +#include \"v4l2_controls.h\"\n>\n>  namespace libcamera {\n>\n>  class MediaEntity;\n> -class V4L2ControlInfo;\n> -class V4L2ControlList;\n>  class V4L2Subdevice;\n>\n>  struct V4L2SubdeviceFormat;\n> @@ -43,7 +42,7 @@ public:\n>  \t\t\t\t      const Size &size) const;\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>\n> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> +\tconst V4L2ControlInfoMap &controls() const;\n>  \tint getControls(V4L2ControlList *ctrls);\n>  \tint setControls(V4L2ControlList *ctrls);\n>\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 0047efab11fa..10b726504951 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -8,11 +8,11 @@\n>  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n>  #define __LIBCAMERA_V4L2_CONTROLS_H__\n>\n> +#include <map>\n> +#include <stdint.h>\n\nNit: C and C++ includes mixed?\n\n>  #include <string>\n>  #include <vector>\n>\n> -#include <stdint.h>\n> -\n>  #include <linux/v4l2-controls.h>\n>  #include <linux/videodev2.h>\n>\n> @@ -41,6 +41,8 @@ private:\n>  \tint64_t max_;\n>  };\n>\n> +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> +\n>  class V4L2Control\n>  {\n>  public:\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 24a0134a2cba..e7e9829cb05f 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -13,19 +13,18 @@\n>  #include <linux/videodev2.h>\n>\n>  #include \"log.h\"\n> +#include \"v4l2_controls.h\"\n>\n>  namespace libcamera {\n>\n> -class V4L2ControlInfo;\n> -class V4L2ControlList;\n> -\n>  class V4L2Device : protected Loggable\n>  {\n>  public:\n>  \tvoid close();\n>  \tbool isOpen() const { return fd_ != -1; }\n>\n> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> +\n>  \tint getControls(V4L2ControlList *ctrls);\n>  \tint setControls(V4L2ControlList *ctrls);\n>\n> @@ -48,7 +47,7 @@ private:\n>  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t    unsigned int count);\n>\n> -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n> +\tV4L2ControlInfoMap controls_;\n>  \tstd::string deviceNode_;\n>  \tint fd_;\n>  };\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index af017bce48ba..84258d9954d0 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n>   * \\return The V4L2 control maximum value\n>   */\n>\n> +/**\n> + * \\typedef V4L2ControlInfoMap\n> + * \\brief A map of control ID to V4L2ControlInfo\n\nNit: shouldn't these be plurals?\n\n> + */\n> +\n>  /**\n>   * \\class V4L2Control\n>   * \\brief A V4L2 control value\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 06939dead705..59fc98cefd4e 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -111,19 +111,10 @@ void V4L2Device::close()\n>   */\n>\n>  /**\n> - * \\brief Retrieve information about a control\n> - * \\param[in] id The control ID\n> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> - * if the device doesn't have such a control\n> + * \\fn V4L2Device::controls()\n> + * \\brief Retrieve the supported V4L2 controls\n\n\" and the associated control information\" ?\n> + * \\return A map of the V4L2 controls supported by the device\n>   */\n> -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n> -{\n> -\tauto it = controls_.find(id);\n> -\tif (it == controls_.end())\n> -\t\treturn nullptr;\n> -\n> -\treturn &it->second;\n> -}\n>\n>  /**\n>   * \\brief Read controls from the device\n> @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>\n>  \tfor (unsigned int i = 0; i < count; ++i) {\n>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> -\t\tif (!info) {\n> +\t\tconst auto iter = controls_.find(ctrl->id());\n> +\t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n> +\t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n>  \t\tv4l2Ctrls[i].id = info->id();\n>  \t}\n> @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n>\n>  \tfor (unsigned int i = 0; i < count; ++i) {\n>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> -\t\tif (!info) {\n> +\t\tconst auto iter = controls_.find(ctrl->id());\n> +\t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n> +\t\tconst V4L2ControlInfo *info = &iter->second;\n\nAn operation wrapping the explicit lookup was probably shorter to\nwrite and read, but the change is good.\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nThanks\n  j\n\n>  \t\tcontrolInfo[i] = info;\n>  \t\tv4l2Ctrls[i].id = info->id();\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":"<jacopo@jmondi.org>","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DDB160BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 17:04:30 +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 relay1-d.mail.gandi.net (Postfix) with ESMTPSA id D0F2D24000E;\n\tMon,  1 Jul 2019 15:04:29 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 1 Jul 2019 17:05:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701150544.x7qe76auu7xm2awo@uno.localdomain>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"4r2p4h6z7amt5lds\"","Content-Disposition":"inline","In-Reply-To":"<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Mon, 01 Jul 2019 15:04:30 -0000"}},{"id":2076,"web_url":"https://patchwork.libcamera.org/comment/2076/","msgid":"<20190701160504.GH5018@pendragon.ideasonboard.com>","date":"2019-07-01T16:05:04","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jul 01, 2019 at 05:05:44PM +0200, Jacopo Mondi wrote:\n> On Mon, Jul 01, 2019 at 02:38:06AM +0300, Laurent Pinchart wrote:\n> > Add a new controls() method to the V4L2Device class to retrieve the map\n> > of all supported controls. This is needed in order to dynamically query\n> > the supported controls, for instance for drivers that support different\n> > sets of controls depending on the device model.\n> >\n> > To make the API easier to use, create a type alias for the control ID to\n> > ControlInfo and use it.\n> >\n> > Remove the getControlInfo() method that is not used externally, as it\n> > can now be replaced by accessing the full list of controls.\n> >\n> > Update the CameraSensor API accordingly.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/camera_sensor.cpp       | 10 ++++------\n> >  src/libcamera/include/camera_sensor.h |  5 ++---\n> >  src/libcamera/include/v4l2_controls.h |  6 ++++--\n> >  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n> >  src/libcamera/v4l2_controls.cpp       |  5 +++++\n> >  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n> >  6 files changed, 28 insertions(+), 32 deletions(-)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 471c3e2e6e47..14f631e8ecf7 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> >  }\n> >\n> >  /**\n> > - * \\brief Retrieve information about a control\n> > - * \\param[in] id The control ID\n> > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > - * if the sensor doesn't have such a control\n> > + * \\brief Retrieve the supported V4L2 controls\n> > + * \\return A map of the V4L2 controls supported by the sensor\n> >   */\n> > -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n> > +const V4L2ControlInfoMap &CameraSensor::controls() const\n> >  {\n> > -\treturn subdev_->getControlInfo(id);\n> > +\treturn subdev_->controls();\n> >  }\n> >\n> >  /**\n> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> > index b42e7b8e1343..fe033fb374c1 100644\n> > --- a/src/libcamera/include/camera_sensor.h\n> > +++ b/src/libcamera/include/camera_sensor.h\n> > @@ -13,12 +13,11 @@\n> >  #include <libcamera/geometry.h>\n> >\n> >  #include \"log.h\"\n> > +#include \"v4l2_controls.h\"\n> >\n> >  namespace libcamera {\n> >\n> >  class MediaEntity;\n> > -class V4L2ControlInfo;\n> > -class V4L2ControlList;\n> >  class V4L2Subdevice;\n> >\n> >  struct V4L2SubdeviceFormat;\n> > @@ -43,7 +42,7 @@ public:\n> >  \t\t\t\t      const Size &size) const;\n> >  \tint setFormat(V4L2SubdeviceFormat *format);\n> >\n> > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > +\tconst V4L2ControlInfoMap &controls() const;\n> >  \tint getControls(V4L2ControlList *ctrls);\n> >  \tint setControls(V4L2ControlList *ctrls);\n> >\n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index 0047efab11fa..10b726504951 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -8,11 +8,11 @@\n> >  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n> >  #define __LIBCAMERA_V4L2_CONTROLS_H__\n> >\n> > +#include <map>\n> > +#include <stdint.h>\n> \n> Nit: C and C++ includes mixed?\n\nI wondered about that, as it seemed to me it was the general rules in\nlibcamera, and the fact that they were separated here made me hesitate.\nI checked through the code base, and we generally mix them.\n\nI also thought we had documented this in the coding style, but it seems\nwe haven't. I then remembered it was documented in\nhttps://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes.\n\n> >  #include <string>\n> >  #include <vector>\n> >\n> > -#include <stdint.h>\n> > -\n> >  #include <linux/v4l2-controls.h>\n> >  #include <linux/videodev2.h>\n> >\n> > @@ -41,6 +41,8 @@ private:\n> >  \tint64_t max_;\n> >  };\n> >\n> > +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> > +\n> >  class V4L2Control\n> >  {\n> >  public:\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index 24a0134a2cba..e7e9829cb05f 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -13,19 +13,18 @@\n> >  #include <linux/videodev2.h>\n> >\n> >  #include \"log.h\"\n> > +#include \"v4l2_controls.h\"\n> >\n> >  namespace libcamera {\n> >\n> > -class V4L2ControlInfo;\n> > -class V4L2ControlList;\n> > -\n> >  class V4L2Device : protected Loggable\n> >  {\n> >  public:\n> >  \tvoid close();\n> >  \tbool isOpen() const { return fd_ != -1; }\n> >\n> > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> > +\n> >  \tint getControls(V4L2ControlList *ctrls);\n> >  \tint setControls(V4L2ControlList *ctrls);\n> >\n> > @@ -48,7 +47,7 @@ private:\n> >  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n> >  \t\t\t    unsigned int count);\n> >\n> > -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n> > +\tV4L2ControlInfoMap controls_;\n> >  \tstd::string deviceNode_;\n> >  \tint fd_;\n> >  };\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index af017bce48ba..84258d9954d0 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> >   * \\return The V4L2 control maximum value\n> >   */\n> >\n> > +/**\n> > + * \\typedef V4L2ControlInfoMap\n> > + * \\brief A map of control ID to V4L2ControlInfo\n> \n> Nit: shouldn't these be plurals?\n\nI think both work (\"this container maps a control ID to a\nV4L2ControlInfo\" or \"this containter maps control IDs to V4L2ControlInfo\ninstances\"). I went for the former to avoid adding \"instances\". Now that\nI'm re-reading the code I will also replace control ID with ControlId.\n\n> > + */\n> > +\n> >  /**\n> >   * \\class V4L2Control\n> >   * \\brief A V4L2 control value\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 06939dead705..59fc98cefd4e 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -111,19 +111,10 @@ void V4L2Device::close()\n> >   */\n> >\n> >  /**\n> > - * \\brief Retrieve information about a control\n> > - * \\param[in] id The control ID\n> > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > - * if the device doesn't have such a control\n> > + * \\fn V4L2Device::controls()\n> > + * \\brief Retrieve the supported V4L2 controls\n> \n> \" and the associated control information\" ?\n\nI'm not sure that's needed, but I'll add it :-) How about \"and their\ninformation\" to make it shorter ?\n\n> > + * \\return A map of the V4L2 controls supported by the device\n> >   */\n> > -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n> > -{\n> > -\tauto it = controls_.find(id);\n> > -\tif (it == controls_.end())\n> > -\t\treturn nullptr;\n> > -\n> > -\treturn &it->second;\n> > -}\n> >\n> >  /**\n> >   * \\brief Read controls from the device\n> > @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n> >\n> >  \tfor (unsigned int i = 0; i < count; ++i) {\n> >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > -\t\tif (!info) {\n> > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > +\t\tif (iter == controls_.end()) {\n> >  \t\t\tLOG(V4L2, Error)\n> >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> >  \t\t\treturn -EINVAL;\n> >  \t\t}\n> >\n> > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> >  \t\tcontrolInfo[i] = info;\n> >  \t\tv4l2Ctrls[i].id = info->id();\n> >  \t}\n> > @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n> >\n> >  \tfor (unsigned int i = 0; i < count; ++i) {\n> >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > -\t\tif (!info) {\n> > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > +\t\tif (iter == controls_.end()) {\n> >  \t\t\tLOG(V4L2, Error)\n> >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> >  \t\t\treturn -EINVAL;\n> >  \t\t}\n> >\n> > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> \n> An operation wrapping the explicit lookup was probably shorter to\n> write and read, but the change is good.\n\nKieran made the same comment, and I agree. If the slightly longer error\ncheck is only needed here I think it's fine. If we end up finding that a\npointer-or-nullptr version would be useful in more places, then I would\nlike to look into how we could provide a utility to extend containers\ninstead of having to add custom functions every time. e.g.\n\n\tconst V4L2ControlInfo *info = utils::find(video_.controls(), ctrl->id());\n\nThat would still be slightly longer to write than\n\n\tconst V4L2ControlInfo *info = video_.getControlInfo(ctrl->id());\n\nbut in my opinion reasonably so, while providing the convenience helper\nfor all our containers, without needing to explicitly provide a separate\nmethod each time.\n\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> >  \t\tcontrolInfo[i] = info;\n> >  \t\tv4l2Ctrls[i].id = info->id();\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 1CD1D60BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 18:05:24 +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 947E9524;\n\tMon,  1 Jul 2019 18:05:23 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561997123;\n\tbh=hzmXp6kWdWRypt63C24JFnmPyzzkzQ/9RWF3IW76bP8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pi7ES37s4feByjtpUkauMI/Lt+5om+EkCNq3zRn024wIKLsaxmI3awNIyw3dJv9vM\n\tspszIYPOCh0lKKPo9SeyKcG3QRq6UzDC33Q6lJqwfLkeQ09H9tenksVhyA9d3XxA23\n\twaKpd18n0Ig0eqFrmXzi/A7Jsu6heN6zesJmYtzI=","Date":"Mon, 1 Jul 2019 19:05:04 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701160504.GH5018@pendragon.ideasonboard.com>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>\n\t<20190701150544.x7qe76auu7xm2awo@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190701150544.x7qe76auu7xm2awo@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Mon, 01 Jul 2019 16:05:24 -0000"}},{"id":2078,"web_url":"https://patchwork.libcamera.org/comment/2078/","msgid":"<20190701161253.fj7p2fj52ktxdfwv@uno.localdomain>","date":"2019-07-01T16:12:53","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","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 07:05:04PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Jul 01, 2019 at 05:05:44PM +0200, Jacopo Mondi wrote:\n> > On Mon, Jul 01, 2019 at 02:38:06AM +0300, Laurent Pinchart wrote:\n> > > Add a new controls() method to the V4L2Device class to retrieve the map\n> > > of all supported controls. This is needed in order to dynamically query\n> > > the supported controls, for instance for drivers that support different\n> > > sets of controls depending on the device model.\n> > >\n> > > To make the API easier to use, create a type alias for the control ID to\n> > > ControlInfo and use it.\n> > >\n> > > Remove the getControlInfo() method that is not used externally, as it\n> > > can now be replaced by accessing the full list of controls.\n> > >\n> > > Update the CameraSensor API accordingly.\n> > >\n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  src/libcamera/camera_sensor.cpp       | 10 ++++------\n> > >  src/libcamera/include/camera_sensor.h |  5 ++---\n> > >  src/libcamera/include/v4l2_controls.h |  6 ++++--\n> > >  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n> > >  src/libcamera/v4l2_controls.cpp       |  5 +++++\n> > >  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n> > >  6 files changed, 28 insertions(+), 32 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 471c3e2e6e47..14f631e8ecf7 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> > >  }\n> > >\n> > >  /**\n> > > - * \\brief Retrieve information about a control\n> > > - * \\param[in] id The control ID\n> > > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > > - * if the sensor doesn't have such a control\n> > > + * \\brief Retrieve the supported V4L2 controls\n> > > + * \\return A map of the V4L2 controls supported by the sensor\n> > >   */\n> > > -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n> > > +const V4L2ControlInfoMap &CameraSensor::controls() const\n> > >  {\n> > > -\treturn subdev_->getControlInfo(id);\n> > > +\treturn subdev_->controls();\n> > >  }\n> > >\n> > >  /**\n> > > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> > > index b42e7b8e1343..fe033fb374c1 100644\n> > > --- a/src/libcamera/include/camera_sensor.h\n> > > +++ b/src/libcamera/include/camera_sensor.h\n> > > @@ -13,12 +13,11 @@\n> > >  #include <libcamera/geometry.h>\n> > >\n> > >  #include \"log.h\"\n> > > +#include \"v4l2_controls.h\"\n> > >\n> > >  namespace libcamera {\n> > >\n> > >  class MediaEntity;\n> > > -class V4L2ControlInfo;\n> > > -class V4L2ControlList;\n> > >  class V4L2Subdevice;\n> > >\n> > >  struct V4L2SubdeviceFormat;\n> > > @@ -43,7 +42,7 @@ public:\n> > >  \t\t\t\t      const Size &size) const;\n> > >  \tint setFormat(V4L2SubdeviceFormat *format);\n> > >\n> > > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > > +\tconst V4L2ControlInfoMap &controls() const;\n> > >  \tint getControls(V4L2ControlList *ctrls);\n> > >  \tint setControls(V4L2ControlList *ctrls);\n> > >\n> > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > > index 0047efab11fa..10b726504951 100644\n> > > --- a/src/libcamera/include/v4l2_controls.h\n> > > +++ b/src/libcamera/include/v4l2_controls.h\n> > > @@ -8,11 +8,11 @@\n> > >  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n> > >  #define __LIBCAMERA_V4L2_CONTROLS_H__\n> > >\n> > > +#include <map>\n> > > +#include <stdint.h>\n> >\n> > Nit: C and C++ includes mixed?\n>\n> I wondered about that, as it seemed to me it was the general rules in\n> libcamera, and the fact that they were separated here made me hesitate.\n> I checked through the code base, and we generally mix them.\n>\n> I also thought we had documented this in the coding style, but it seems\n> we haven't. I then remembered it was documented in\n> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes.\n\nI see. I've been doing it wrong then\n\n>\n> > >  #include <string>\n> > >  #include <vector>\n> > >\n> > > -#include <stdint.h>\n> > > -\n> > >  #include <linux/v4l2-controls.h>\n> > >  #include <linux/videodev2.h>\n> > >\n> > > @@ -41,6 +41,8 @@ private:\n> > >  \tint64_t max_;\n> > >  };\n> > >\n> > > +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> > > +\n> > >  class V4L2Control\n> > >  {\n> > >  public:\n> > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > > index 24a0134a2cba..e7e9829cb05f 100644\n> > > --- a/src/libcamera/include/v4l2_device.h\n> > > +++ b/src/libcamera/include/v4l2_device.h\n> > > @@ -13,19 +13,18 @@\n> > >  #include <linux/videodev2.h>\n> > >\n> > >  #include \"log.h\"\n> > > +#include \"v4l2_controls.h\"\n> > >\n> > >  namespace libcamera {\n> > >\n> > > -class V4L2ControlInfo;\n> > > -class V4L2ControlList;\n> > > -\n> > >  class V4L2Device : protected Loggable\n> > >  {\n> > >  public:\n> > >  \tvoid close();\n> > >  \tbool isOpen() const { return fd_ != -1; }\n> > >\n> > > -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n> > > +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> > > +\n> > >  \tint getControls(V4L2ControlList *ctrls);\n> > >  \tint setControls(V4L2ControlList *ctrls);\n> > >\n> > > @@ -48,7 +47,7 @@ private:\n> > >  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n> > >  \t\t\t    unsigned int count);\n> > >\n> > > -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n> > > +\tV4L2ControlInfoMap controls_;\n> > >  \tstd::string deviceNode_;\n> > >  \tint fd_;\n> > >  };\n> > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > > index af017bce48ba..84258d9954d0 100644\n> > > --- a/src/libcamera/v4l2_controls.cpp\n> > > +++ b/src/libcamera/v4l2_controls.cpp\n> > > @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > >   * \\return The V4L2 control maximum value\n> > >   */\n> > >\n> > > +/**\n> > > + * \\typedef V4L2ControlInfoMap\n> > > + * \\brief A map of control ID to V4L2ControlInfo\n> >\n> > Nit: shouldn't these be plurals?\n>\n> I think both work (\"this container maps a control ID to a\n> V4L2ControlInfo\" or \"this containter maps control IDs to V4L2ControlInfo\n> instances\"). I went for the former to avoid adding \"instances\". Now that\n> I'm re-reading the code I will also replace control ID with ControlId.\n>\n\nNo worries, was a very minor comment.\n\n> > > + */\n> > > +\n> > >  /**\n> > >   * \\class V4L2Control\n> > >   * \\brief A V4L2 control value\n> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > index 06939dead705..59fc98cefd4e 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -111,19 +111,10 @@ void V4L2Device::close()\n> > >   */\n> > >\n> > >  /**\n> > > - * \\brief Retrieve information about a control\n> > > - * \\param[in] id The control ID\n> > > - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n> > > - * if the device doesn't have such a control\n> > > + * \\fn V4L2Device::controls()\n> > > + * \\brief Retrieve the supported V4L2 controls\n> >\n> > \" and the associated control information\" ?\n>\n> I'm not sure that's needed, but I'll add it :-) How about \"and their\n> information\" to make it shorter ?\ni\nthis was very minor as well. As you wish.\n\n>\n> > > + * \\return A map of the V4L2 controls supported by the device\n> > >   */\n> > > -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n> > > -{\n> > > -\tauto it = controls_.find(id);\n> > > -\tif (it == controls_.end())\n> > > -\t\treturn nullptr;\n> > > -\n> > > -\treturn &it->second;\n> > > -}\n> > >\n> > >  /**\n> > >   * \\brief Read controls from the device\n> > > @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n> > >\n> > >  \tfor (unsigned int i = 0; i < count; ++i) {\n> > >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > > -\t\tif (!info) {\n> > > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > > +\t\tif (iter == controls_.end()) {\n> > >  \t\t\tLOG(V4L2, Error)\n> > >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> > >  \t\t\treturn -EINVAL;\n> > >  \t\t}\n> > >\n> > > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> > >  \t\tcontrolInfo[i] = info;\n> > >  \t\tv4l2Ctrls[i].id = info->id();\n> > >  \t}\n> > > @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n> > >\n> > >  \tfor (unsigned int i = 0; i < count; ++i) {\n> > >  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> > > -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n> > > -\t\tif (!info) {\n> > > +\t\tconst auto iter = controls_.find(ctrl->id());\n> > > +\t\tif (iter == controls_.end()) {\n> > >  \t\t\tLOG(V4L2, Error)\n> > >  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> > >  \t\t\treturn -EINVAL;\n> > >  \t\t}\n> > >\n> > > +\t\tconst V4L2ControlInfo *info = &iter->second;\n> >\n> > An operation wrapping the explicit lookup was probably shorter to\n> > write and read, but the change is good.\n>\n> Kieran made the same comment, and I agree. If the slightly longer error\n> check is only needed here I think it's fine. If we end up finding that a\n> pointer-or-nullptr version would be useful in more places, then I would\n> like to look into how we could provide a utility to extend containers\n> instead of having to add custom functions every time. e.g.\n>\n> \tconst V4L2ControlInfo *info = utils::find(video_.controls(), ctrl->id());\n\nI would then prefer what we have here :)\n>\n> That would still be slightly longer to write than\n>\n> \tconst V4L2ControlInfo *info = video_.getControlInfo(ctrl->id());\n>\n> but in my opinion reasonably so, while providing the convenience helper\n> for all our containers, without needing to explicitly provide a separate\n> method each time.\n\nNo worries, we're talking about details. What you have here is fine!\n\nThanks\n   j\n\n\n>\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > >  \t\tcontrolInfo[i] = info;\n> > >  \t\tv4l2Ctrls[i].id = info->id();\n> > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEB8A60BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jul 2019 18:11:36 +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 relay12.mail.gandi.net (Postfix) with ESMTPSA id 642FE200002;\n\tMon,  1 Jul 2019 16:11:36 +0000 (UTC)"],"Date":"Mon, 1 Jul 2019 18:12:53 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701161253.fj7p2fj52ktxdfwv@uno.localdomain>","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>\n\t<20190701150544.x7qe76auu7xm2awo@uno.localdomain>\n\t<20190701160504.GH5018@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qvfzinq452z2sugh\"","Content-Disposition":"inline","In-Reply-To":"<20190701160504.GH5018@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Mon, 01 Jul 2019 16:11:37 -0000"}},{"id":2123,"web_url":"https://patchwork.libcamera.org/comment/2123/","msgid":"<add4d6cd-ebc5-65e8-19ec-17afdd9784a4@ideasonboard.com>","date":"2019-07-02T09:13:31","subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","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 12:00, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, Jul 01, 2019 at 09:03:09AM +0100, Kieran Bingham wrote:\n>> On 01/07/2019 00:38, Laurent Pinchart wrote:\n>>> Add a new controls() method to the V4L2Device class to retrieve the map\n>>> of all supported controls. This is needed in order to dynamically query\n>>> the supported controls, for instance for drivers that support different\n>>> sets of controls depending on the device model.\n>>>\n>>> To make the API easier to use, create a type alias for the control ID to\n>>> ControlInfo and use it.\n>>>\n>>> Remove the getControlInfo() method that is not used externally, as it\n>>> can now be replaced by accessing the full list of controls.\n>>\n>> Maybe it's the C-dev in me but I kinda prefer the lookup, with a null\n>> return check over a lookup with open coding the iter != controls_.end();\n>>\n>> Functionally the same I guess, I think it's just more ingrained in my\n>> head, but the .end() check is more 'STL' so it's certainly not a problem\n>> really.\n> \n> I agree, so if we find this is too much hasle to use, I'm fine adding\n> back a method to look up a single control (or I wonder if we could add a\n> helper method to make look up easier on std::map).\n> \n>>> Update the CameraSensor API accordingly.\n>>\n>> Small comment on the map type chosen, but I think either std::map is\n>> probably fine too.\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>\n>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> ---\n>>>  src/libcamera/camera_sensor.cpp       | 10 ++++------\n>>>  src/libcamera/include/camera_sensor.h |  5 ++---\n>>>  src/libcamera/include/v4l2_controls.h |  6 ++++--\n>>>  src/libcamera/include/v4l2_device.h   |  9 ++++-----\n>>>  src/libcamera/v4l2_controls.cpp       |  5 +++++\n>>>  src/libcamera/v4l2_device.cpp         | 25 +++++++++----------------\n>>>  6 files changed, 28 insertions(+), 32 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n>>> index 471c3e2e6e47..14f631e8ecf7 100644\n>>> --- a/src/libcamera/camera_sensor.cpp\n>>> +++ b/src/libcamera/camera_sensor.cpp\n>>> @@ -248,14 +248,12 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n>>>  }\n>>>  \n>>>  /**\n>>> - * \\brief Retrieve information about a control\n>>> - * \\param[in] id The control ID\n>>> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n>>> - * if the sensor doesn't have such a control\n>>> + * \\brief Retrieve the supported V4L2 controls\n>>> + * \\return A map of the V4L2 controls supported by the sensor\n>>>   */\n>>> -const V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id) const\n>>> +const V4L2ControlInfoMap &CameraSensor::controls() const\n>>>  {\n>>> -\treturn subdev_->getControlInfo(id);\n>>> +\treturn subdev_->controls();\n>>>  }\n>>>  \n>>>  /**\n>>> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n>>> index b42e7b8e1343..fe033fb374c1 100644\n>>> --- a/src/libcamera/include/camera_sensor.h\n>>> +++ b/src/libcamera/include/camera_sensor.h\n>>> @@ -13,12 +13,11 @@\n>>>  #include <libcamera/geometry.h>\n>>>  \n>>>  #include \"log.h\"\n>>> +#include \"v4l2_controls.h\"\n>>>  \n>>>  namespace libcamera {\n>>>  \n>>>  class MediaEntity;\n>>> -class V4L2ControlInfo;\n>>> -class V4L2ControlList;\n>>>  class V4L2Subdevice;\n>>>  \n>>>  struct V4L2SubdeviceFormat;\n>>> @@ -43,7 +42,7 @@ public:\n>>>  \t\t\t\t      const Size &size) const;\n>>>  \tint setFormat(V4L2SubdeviceFormat *format);\n>>>  \n>>> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n>>> +\tconst V4L2ControlInfoMap &controls() const;\n>>>  \tint getControls(V4L2ControlList *ctrls);\n>>>  \tint setControls(V4L2ControlList *ctrls);\n>>>  \n>>> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n>>> index 0047efab11fa..10b726504951 100644\n>>> --- a/src/libcamera/include/v4l2_controls.h\n>>> +++ b/src/libcamera/include/v4l2_controls.h\n>>> @@ -8,11 +8,11 @@\n>>>  #ifndef __LIBCAMERA_V4L2_CONTROLS_H__\n>>>  #define __LIBCAMERA_V4L2_CONTROLS_H__\n>>>  \n>>> +#include <map>\n>>> +#include <stdint.h>\n>>>  #include <string>\n>>>  #include <vector>\n>>>  \n>>> -#include <stdint.h>\n>>> -\n>>>  #include <linux/v4l2-controls.h>\n>>>  #include <linux/videodev2.h>\n>>>  \n>>> @@ -41,6 +41,8 @@ private:\n>>>  \tint64_t max_;\n>>>  };\n>>>  \n>>> +using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n>>\n>> Does V4L2ControlInfoMap need to be ordered?\n>> (I don't think we do at this part do we?)\n>>\n>> std::unordered_map could be faster if we don't need to order the map.\n>> I think at the scale of controls we'll have it probably won't make much\n>> difference...\n>>\n>> We could do some measurements perhaps on a map of 1000 elements vs an\n>> unordered_map of 1000 elements, but I think that's the approx max scale\n>> that we'll contain right?\n> \n> I think we'll be one, if not two orders of magnitude smaller. Would you\n> like to perform measurements ? :-)\n\nI've measured doing 1000 random lookups on maps of 1000 V4L2controlInfo\nstructures:\n\ntimes in nanoSeconds:\n\nstd::map<unsigned int, V4L2ControlInfo>; \t\t~479000 nS\nstd::unordered_map<unsigned int, V4L2ControlInfo>;\t~175000 nS\n\n\n\nWhen the number of entries is less than 50, the differences are very minor.\n\n\nWhen it comes to creating a map and inserting only a small number of\nentries, in fact the std::map starts to win.\n\nInserting 10 items, for 1000000 repeats\n2707779741 nS: std::map: Create and Insert\n3415575028 nS: std::unordered_map: Create and Insert\n\nYet, we're still talking the difference between 2.7 microSeconds and 3.4\nmicroseconds per list. So I think the optimisations here are minimal :)\n\n--\nKieran\n\n\n>> As the key is an unsigned int, I think the hasher will function out of\n>> the box?\n> \n> Correct.\n> \n>>> +\n>>>  class V4L2Control\n>>>  {\n>>>  public:\n>>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n>>> index 24a0134a2cba..e7e9829cb05f 100644\n>>> --- a/src/libcamera/include/v4l2_device.h\n>>> +++ b/src/libcamera/include/v4l2_device.h\n>>> @@ -13,19 +13,18 @@\n>>>  #include <linux/videodev2.h>\n>>>  \n>>>  #include \"log.h\"\n>>> +#include \"v4l2_controls.h\"\n>>>  \n>>>  namespace libcamera {\n>>>  \n>>> -class V4L2ControlInfo;\n>>> -class V4L2ControlList;\n>>> -\n>>>  class V4L2Device : protected Loggable\n>>>  {\n>>>  public:\n>>>  \tvoid close();\n>>>  \tbool isOpen() const { return fd_ != -1; }\n>>>  \n>>> -\tconst V4L2ControlInfo *getControlInfo(unsigned int id) const;\n>>> +\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n>>> +\n>>>  \tint getControls(V4L2ControlList *ctrls);\n>>>  \tint setControls(V4L2ControlList *ctrls);\n>>>  \n>>> @@ -48,7 +47,7 @@ private:\n>>>  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n>>>  \t\t\t    unsigned int count);\n>>>  \n>>> -\tstd::map<unsigned int, V4L2ControlInfo> controls_;\n>>> +\tV4L2ControlInfoMap controls_;\n>>>  \tstd::string deviceNode_;\n>>>  \tint fd_;\n>>>  };\n>>> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n>>> index af017bce48ba..84258d9954d0 100644\n>>> --- a/src/libcamera/v4l2_controls.cpp\n>>> +++ b/src/libcamera/v4l2_controls.cpp\n>>> @@ -114,6 +114,11 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n>>>   * \\return The V4L2 control maximum value\n>>>   */\n>>>  \n>>> +/**\n>>> + * \\typedef V4L2ControlInfoMap\n>>> + * \\brief A map of control ID to V4L2ControlInfo\n>>> + */\n>>> +\n>>>  /**\n>>>   * \\class V4L2Control\n>>>   * \\brief A V4L2 control value\n>>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n>>> index 06939dead705..59fc98cefd4e 100644\n>>> --- a/src/libcamera/v4l2_device.cpp\n>>> +++ b/src/libcamera/v4l2_device.cpp\n>>> @@ -111,19 +111,10 @@ void V4L2Device::close()\n>>>   */\n>>>  \n>>>  /**\n>>> - * \\brief Retrieve information about a control\n>>> - * \\param[in] id The control ID\n>>> - * \\return A pointer to the V4L2ControlInfo for control \\a id, or nullptr\n>>> - * if the device doesn't have such a control\n>>> + * \\fn V4L2Device::controls()\n>>> + * \\brief Retrieve the supported V4L2 controls\n>>> + * \\return A map of the V4L2 controls supported by the device\n>>>   */\n>>> -const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const\n>>> -{\n>>> -\tauto it = controls_.find(id);\n>>> -\tif (it == controls_.end())\n>>> -\t\treturn nullptr;\n>>> -\n>>> -\treturn &it->second;\n>>> -}\n>>>  \n>>>  /**\n>>>   * \\brief Read controls from the device\n>>> @@ -158,13 +149,14 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>>>  \n>>>  \tfor (unsigned int i = 0; i < count; ++i) {\n>>>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n>>> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n>>> -\t\tif (!info) {\n>>> +\t\tconst auto iter = controls_.find(ctrl->id());\n>>> +\t\tif (iter == controls_.end()) {\n>>>  \t\t\tLOG(V4L2, Error)\n>>>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>>>  \t\t\treturn -EINVAL;\n>>>  \t\t}\n>>>  \n>>> +\t\tconst V4L2ControlInfo *info = &iter->second;\n>>>  \t\tcontrolInfo[i] = info;\n>>>  \t\tv4l2Ctrls[i].id = info->id();\n>>>  \t}\n>>> @@ -232,13 +224,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n>>>  \n>>>  \tfor (unsigned int i = 0; i < count; ++i) {\n>>>  \t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n>>> -\t\tconst V4L2ControlInfo *info = getControlInfo(ctrl->id());\n>>> -\t\tif (!info) {\n>>> +\t\tconst auto iter = controls_.find(ctrl->id());\n>>> +\t\tif (iter == controls_.end()) {\n>>>  \t\t\tLOG(V4L2, Error)\n>>>  \t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n>>>  \t\t\treturn -EINVAL;\n>>>  \t\t}\n>>>  \n>>> +\t\tconst V4L2ControlInfo *info = &iter->second;\n>>>  \t\tcontrolInfo[i] = info;\n>>>  \t\tv4l2Ctrls[i].id = info->id();\n>>>  \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","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 ADFBE61F4A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jul 2019 11:13:34 +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 18C7B2C6;\n\tTue,  2 Jul 2019 11:13:34 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562058814;\n\tbh=qIyI2Q66Q5HXeGPbvvrN4XLljtryyW4AWpxavYIImJs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=JhrETQcfK+2nftzwXpr4419Dmy7wWcm+bzzPW9bHeQGP3vOGK1Q5n3gcgC08oopjt\n\tL10hmdvS+Ao5UFOd/bTRraML1gzoUbPK0V/cBb5mozrdpok7mxXd4nLbSWtPpgIoKQ\n\tQMX4aP+kycwYEF3h6+7ZjjU+Cb4M083FwpX3ZOsY=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190630233817.10130-1-laurent.pinchart@ideasonboard.com>\n\t<20190630233817.10130-4-laurent.pinchart@ideasonboard.com>\n\t<813e54e9-0b64-15d6-2b23-a903dc40ceec@ideasonboard.com>\n\t<20190701110047.GB5018@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<add4d6cd-ebc5-65e8-19ec-17afdd9784a4@ideasonboard.com>","Date":"Tue, 2 Jul 2019 10:13:31 +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":"<20190701110047.GB5018@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 03/14] libcamera: v4l2_device: Add\n\tmethod to retrieve all supported controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 02 Jul 2019 09:13:34 -0000"}}]