[{"id":2903,"web_url":"https://patchwork.libcamera.org/comment/2903/","msgid":"<20191015004049.GI5976@bigcity.dyn.berto.se>","date":"2019-10-15T00:40:49","subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:\n> The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with\n> the latter adding convenience accessors based on numerical IDs for the\n> former, as well as a cached idmap. Both features can be useful for\n> ControlInfoMap in the context of serialisation, and merging the two\n> classes will further simplify the IPA API.\n> \n> Import all the features of V4L2ControlInfoMap into ControlInfoMap,\n> turning the latter into a real class. A few new constructors and\n> assignment operators are added for completeness.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nThe change to pipeline handler implementations to shorten the line \nlength could be done in a separate patch ;-) With out without that \naddressed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/ipa/ipa_interface.h              |   2 +-\n>  include/libcamera/controls.h             |  45 +++++++-\n>  src/ipa/ipa_vimc.cpp                     |   2 +-\n>  src/ipa/rkisp1/rkisp1.cpp                |   6 +-\n>  src/libcamera/camera_sensor.cpp          |   2 +-\n>  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-\n>  src/libcamera/include/camera_sensor.h    |   4 +-\n>  src/libcamera/include/v4l2_controls.h    |  36 +-----\n>  src/libcamera/include/v4l2_device.h      |   4 +-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-\n>  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-\n>  src/libcamera/pipeline/vimc.cpp          |  11 +-\n>  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-\n>  src/libcamera/v4l2_controls.cpp          |  94 +--------------\n>  src/libcamera/v4l2_device.cpp            |   2 +-\n>  test/v4l2_videodevice/controls.cpp       |   2 +-\n>  16 files changed, 220 insertions(+), 154 deletions(-)\n> \n> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> index dfb1bcbee516..8fd658af5fdb 100644\n> --- a/include/ipa/ipa_interface.h\n> +++ b/include/ipa/ipa_interface.h\n> @@ -43,7 +43,7 @@ public:\n>  \tvirtual int init() = 0;\n>  \n>  \tvirtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;\n> +\t\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;\n>  \n>  \tvirtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;\n>  \tvirtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 5534a2edb567..80414c6f0748 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -118,7 +118,50 @@ private:\n>  };\n>  \n>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;\n> -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;\n> +\n> +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>\n> +{\n> +public:\n> +\tusing Map = std::unordered_map<const ControlId *, ControlRange>;\n> +\n> +\tControlInfoMap() = default;\n> +\tControlInfoMap(const ControlInfoMap &other) = default;\n> +\tControlInfoMap(std::initializer_list<Map::value_type> init);\n> +\n> +\tControlInfoMap &operator=(const ControlInfoMap &other) = default;\n> +\tControlInfoMap &operator=(std::initializer_list<Map::value_type> init);\n> +\tControlInfoMap &operator=(Map &&info);\n> +\n> +\tusing Map::key_type;\n> +\tusing Map::mapped_type;\n> +\tusing Map::value_type;\n> +\tusing Map::size_type;\n> +\tusing Map::iterator;\n> +\tusing Map::const_iterator;\n> +\n> +\tusing Map::begin;\n> +\tusing Map::cbegin;\n> +\tusing Map::end;\n> +\tusing Map::cend;\n> +\tusing Map::at;\n> +\tusing Map::empty;\n> +\tusing Map::size;\n> +\tusing Map::count;\n> +\tusing Map::find;\n> +\n> +\tmapped_type &at(unsigned int key);\n> +\tconst mapped_type &at(unsigned int key) const;\n> +\tsize_type count(unsigned int key) const;\n> +\titerator find(unsigned int key);\n> +\tconst_iterator find(unsigned int key) const;\n> +\n> +\tconst ControlIdMap &idmap() const { return idmap_; }\n> +\n> +private:\n> +\tvoid generateIdmap();\n> +\n> +\tControlIdMap idmap_;\n> +};\n>  \n>  class ControlList\n>  {\n> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp\n> index 620dc25f456e..63d578b4e2aa 100644\n> --- a/src/ipa/ipa_vimc.cpp\n> +++ b/src/ipa/ipa_vimc.cpp\n> @@ -31,7 +31,7 @@ public:\n>  \n>  \tint init() override;\n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n>  \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index d64c334cbf0c..bd703898c523 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -33,7 +33,7 @@ public:\n>  \tint init() override { return 0; }\n>  \n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override;\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n>  \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -49,7 +49,7 @@ private:\n>  \n>  \tstd::map<unsigned int, BufferMemory> bufferInfo_;\n>  \n> -\tV4L2ControlInfoMap ctrls_;\n> +\tControlInfoMap ctrls_;\n>  \n>  \t/* Camera sensor controls. */\n>  \tbool autoExposure_;\n> @@ -62,7 +62,7 @@ private:\n>  };\n>  \n>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t\t  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)\n> +\t\t\t  const std::map<unsigned int, ControlInfoMap> &entityControls)\n>  {\n>  \tif (entityControls.empty())\n>  \t\treturn;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 1b8e8c0e07da..86f0c772861b 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n>   * \\brief Retrieve the supported V4L2 controls and their information\n>   * \\return A map of the V4L2 controls supported by the sensor\n>   */\n> -const V4L2ControlInfoMap &CameraSensor::controls() const\n> +const ControlInfoMap &CameraSensor::controls() const\n>  {\n>  \treturn subdev_->controls();\n>  }\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 6a0301f3a2ae..bf7634aab470 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -393,10 +393,147 @@ std::string ControlRange::toString() const\n>   */\n>  \n>  /**\n> - * \\typedef ControlInfoMap\n> + * \\class ControlInfoMap\n>   * \\brief A map of ControlId to ControlRange\n> + *\n> + * The ControlInfoMap class describes controls supported by an object as an\n> + * unsorted map of ControlId pointers to ControlRange instances. Unlike the\n> + * standard std::unsorted_map<> class, it is designed the be immutable once\n> + * constructed, and thus only exposes the read accessors of the\n> + * std::unsorted_map<> base class.\n> + *\n> + * In addition to the features of the standard unsorted map, this class also\n> + * provides access to the mapped elements using numerical ID keys. It maintains\n> + * an internal map of numerical ID to ControlId for this purpose, and exposes it\n> + * through the idmap() method to help construction of ControlList instances.\n>   */\n>  \n> +/**\n> + * \\typedef ControlInfoMap::Map\n> + * \\brief The base std::unsorted_map<> container\n> + */\n> +\n> +/**\n> + * \\fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)\n> + * \\brief Copy constructor, construct a ControlInfoMap from a copy of \\a other\n> + * \\param[in] other The other ControlInfoMap\n> + */\n> +\n> +/**\n> + * \\brief Construct a ControlInfoMap from an initializer list\n> + * \\param[in] init The initializer list\n> + */\n> +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)\n> +\t: Map(init)\n> +{\n> +\tgenerateIdmap();\n> +}\n> +\n> +/**\n> + * \\fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)\n> + * \\brief Copy assignment operator, replace the contents with a copy of \\a other\n> + * \\param[in] other The other ControlInfoMap\n> + * \\return A reference to the ControlInfoMap\n> + */\n> +\n> +/**\n> + * \\brief Replace the contents with those from the initializer list\n> + * \\param[in] init The initializer list\n> + * \\return A reference to the ControlInfoMap\n> + */\n> +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)\n> +{\n> +\tMap::operator=(init);\n> +\tgenerateIdmap();\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\brief Move assignment operator from a plain map\n> + * \\param[in] info The control info plain map\n> + *\n> + * Populate the map by replacing its contents with those of \\a info using move\n> + * semantics. Upon return the \\a info map will be empty.\n> + *\n> + * \\return A reference to the populated ControlInfoMap\n> + */\n> +ControlInfoMap &ControlInfoMap::operator=(Map &&info)\n> +{\n> +\tMap::operator=(std::move(info));\n> +\tgenerateIdmap();\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\brief Access specified element by numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A reference to the element whose ID is equal to \\a id\n> + */\n> +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)\n> +{\n> +\treturn at(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Access specified element by numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A const reference to the element whose ID is equal to \\a id\n> + */\n> +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> +{\n> +\treturn at(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Count the number of elements matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return The number of elements matching the numerical \\a id\n> + */\n> +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> +{\n> +\treturn count(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Find the element matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return An iterator pointing to the element matching the numerical \\a id, or\n> + * end() if no such element exists\n> + */\n> +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> +{\n> +\treturn find(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Find the element matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A const iterator pointing to the element matching the numerical\n> + * \\a id, or end() if no such element exists\n> + */\n> +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> +{\n> +\treturn find(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\fn const ControlIdMap &ControlInfoMap::idmap() const\n> + * \\brief Retrieve the ControlId map\n> + *\n> + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> + * for the V4L2 device that the control list targets. This helper method\n> + * returns a suitable idmap for that purpose.\n> + *\n> + * \\return The ControlId map\n> + */\n> +\n> +void ControlInfoMap::generateIdmap()\n> +{\n> +\tidmap_.clear();\n> +\tfor (const auto &ctrl : *this)\n> +\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> +}\n> +\n>  /**\n>   * \\class ControlList\n>   * \\brief Associate a list of ControlId with their values for an object\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index f426e29b84bf..1fb36a4f4951 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -16,9 +16,9 @@\n>  \n>  namespace libcamera {\n>  \n> +class ControlInfoMap;\n>  class ControlList;\n>  class MediaEntity;\n> -class V4L2ControlInfoMap;\n>  class V4L2Subdevice;\n>  \n>  struct V4L2SubdeviceFormat;\n> @@ -43,7 +43,7 @@ public:\n>  \t\t\t\t      const Size &size) const;\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>  \n> -\tconst V4L2ControlInfoMap &controls() const;\n> +\tconst ControlInfoMap &controls() const;\n>  \tint getControls(ControlList *ctrls);\n>  \tint setControls(ControlList *ctrls);\n>  \n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index c427b845d8b4..e16c4957f9d1 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -31,44 +31,10 @@ public:\n>  \tV4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);\n>  };\n>  \n> -class V4L2ControlInfoMap : private ControlInfoMap\n> -{\n> -public:\n> -\tV4L2ControlInfoMap &operator=(ControlInfoMap &&info);\n> -\n> -\tusing ControlInfoMap::key_type;\n> -\tusing ControlInfoMap::mapped_type;\n> -\tusing ControlInfoMap::value_type;\n> -\tusing ControlInfoMap::size_type;\n> -\tusing ControlInfoMap::iterator;\n> -\tusing ControlInfoMap::const_iterator;\n> -\n> -\tusing ControlInfoMap::begin;\n> -\tusing ControlInfoMap::cbegin;\n> -\tusing ControlInfoMap::end;\n> -\tusing ControlInfoMap::cend;\n> -\tusing ControlInfoMap::at;\n> -\tusing ControlInfoMap::empty;\n> -\tusing ControlInfoMap::size;\n> -\tusing ControlInfoMap::count;\n> -\tusing ControlInfoMap::find;\n> -\n> -\tmapped_type &at(unsigned int key);\n> -\tconst mapped_type &at(unsigned int key) const;\n> -\tsize_type count(unsigned int key) const;\n> -\titerator find(unsigned int key);\n> -\tconst_iterator find(unsigned int key) const;\n> -\n> -\tconst ControlIdMap &idmap() const { return idmap_; }\n> -\n> -private:\n> -\tControlIdMap idmap_;\n> -};\n> -\n>  class V4L2ControlList : public ControlList\n>  {\n>  public:\n> -\tV4L2ControlList(const V4L2ControlInfoMap &info)\n> +\tV4L2ControlList(const ControlInfoMap &info)\n>  \t\t: ControlList(info.idmap())\n>  \t{\n>  \t}\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index f30b1c2cde34..6bfddefe336c 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -24,7 +24,7 @@ public:\n>  \tvoid close();\n>  \tbool isOpen() const { return fd_ != -1; }\n>  \n> -\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> +\tconst ControlInfoMap &controls() const { return controls_; }\n>  \n>  \tint getControls(ControlList *ctrls);\n>  \tint setControls(ControlList *ctrls);\n> @@ -49,7 +49,7 @@ private:\n>  \t\t\t    unsigned int count);\n>  \n>  \tstd::vector<std::unique_ptr<V4L2ControlId>> controlIds_;\n> -\tV4L2ControlInfoMap controls_;\n> +\tControlInfoMap controls_;\n>  \tstd::string deviceNode_;\n>  \tint fd_;\n>  };\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 9b19bde8a274..7a28b03b8d38 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n>  \t\t.size = data->stream_.configuration().size,\n>  \t};\n>  \n> -\tstd::map<unsigned int, V4L2ControlInfoMap> entityControls;\n> +\tstd::map<unsigned int, ControlInfoMap> entityControls;\n>  \tentityControls.emplace(0, data->sensor_->controls());\n>  \n>  \tdata->ipa_->configure(streamConfig, entityControls);\n> @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>  \tstd::unique_ptr<RkISP1CameraData> data =\n>  \t\tutils::make_unique<RkISP1CameraData>(this);\n>  \n> -\tdata->controlInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t   std::forward_as_tuple(&controls::AeEnable),\n> -\t\t\t\t   std::forward_as_tuple(false, true));\n> +\tControlInfoMap::Map ctrls;\n> +\tctrls.emplace(std::piecewise_construct,\n> +\t\t      std::forward_as_tuple(&controls::AeEnable),\n> +\t\t      std::forward_as_tuple(false, true));\n> +\n> +\tdata->controlInfo_ = std::move(ctrls);\n>  \n>  \tdata->sensor_ = new CameraSensor(sensor);\n>  \tret = data->sensor_->init();\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 018c7691d263..a64e1af4c550 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \tvideo_->bufferReady.connect(this, &UVCCameraData::bufferReady);\n>  \n>  \t/* Initialise the supported controls. */\n> -\tconst V4L2ControlInfoMap &controls = video_->controls();\n> +\tconst ControlInfoMap &controls = video_->controls();\n> +\tControlInfoMap::Map ctrls;\n> +\n>  \tfor (const auto &ctrl : controls) {\n>  \t\tconst ControlRange &range = ctrl.second;\n>  \t\tconst ControlId *id;\n> @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n> -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(range));\n> +\t\tctrls.emplace(std::piecewise_construct,\n> +\t\t\t      std::forward_as_tuple(id),\n> +\t\t\t      std::forward_as_tuple(range));\n>  \t}\n>  \n> +\tcontrolInfo_ = std::move(ctrls);\n> +\n>  \treturn 0;\n>  }\n>  \n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f7f82edc6530..6a4244119dc5 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t\treturn -ENODEV;\n>  \n>  \t/* Initialise the supported controls. */\n> -\tconst V4L2ControlInfoMap &controls = sensor_->controls();\n> +\tconst ControlInfoMap &controls = sensor_->controls();\n> +\tControlInfoMap::Map ctrls;\n> +\n>  \tfor (const auto &ctrl : controls) {\n>  \t\tconst ControlRange &range = ctrl.second;\n>  \t\tconst ControlId *id;\n> @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n> -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(range));\n> +\t\tctrls.emplace(std::piecewise_construct,\n> +\t\t\t      std::forward_as_tuple(id),\n> +\t\t\t      std::forward_as_tuple(range));\n>  \t}\n>  \n> +\tcontrolInfo_ = std::move(ctrls);\n>  \treturn 0;\n>  }\n>  \n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 41bd965f0284..4e6fa6899e07 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -28,7 +28,7 @@ public:\n>  \n>  \tint init() override { return 0; }\n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n>  \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index e783457caf94..bd5e18590b76 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)\n>  \t\t\t\t\t\t     static_cast<int32_t>(ctrl.maximum)));\n>  }\n>  \n> -/**\n> - * \\class V4L2ControlInfoMap\n> - * \\brief A map of controlID to ControlRange for V4L2 controls\n> - */\n> -\n> -/**\n> - * \\brief Move assignment operator from a ControlInfoMap\n> - * \\param[in] info The control info map\n> - *\n> - * Populate the map by replacing its contents with those of \\a info using move\n> - * semantics. Upon return the \\a info map will be empty.\n> - *\n> - * This is the only supported way to populate a V4L2ControlInfoMap.\n> - *\n> - * \\return The populated V4L2ControlInfoMap\n> - */\n> -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)\n> -{\n> -\tControlInfoMap::operator=(std::move(info));\n> -\n> -\tidmap_.clear();\n> -\tfor (const auto &ctrl : *this)\n> -\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> -\n> -\treturn *this;\n> -}\n> -\n> -/**\n> - * \\brief Access specified element by numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A reference to the element whose ID is equal to \\a id\n> - */\n> -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)\n> -{\n> -\treturn at(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Access specified element by numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A const reference to the element whose ID is equal to \\a id\n> - */\n> -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const\n> -{\n> -\treturn at(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Count the number of elements matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return The number of elements matching the numerical \\a id\n> - */\n> -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const\n> -{\n> -\treturn count(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Find the element matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return An iterator pointing to the element matching the numerical \\a id, or\n> - * end() if no such element exists\n> - */\n> -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)\n> -{\n> -\treturn find(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Find the element matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A const iterator pointing to the element matching the numerical\n> - * \\a id, or end() if no such element exists\n> - */\n> -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const\n> -{\n> -\treturn find(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\fn const ControlIdMap &V4L2ControlInfoMap::idmap() const\n> - * \\brief Retrieve the ControlId map\n> - *\n> - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> - * for the V4L2 device that the control list targets. This helper method\n> - * returns a suitable idmap for that purpose.\n> - *\n> - * \\return The ControlId map\n> - */\n> -\n>  /**\n>   * \\class V4L2ControlList\n>   * \\brief A list of controls for a V4L2 device\n>   *\n>   * This class specialises the ControList class for use with V4L2 devices. It\n> - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.\n> + * offers a convenience API to create a ControlList from a ControlInfoMap.\n>   *\n>   * V4L2ControlList allows easy construction of a ControlList containing V4L2\n>   * controls for a device. It can be used to construct the list of controls\n> @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con\n>   */\n>  \n>  /**\n> - * \\fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)\n> + * \\fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)\n>   * \\brief Construct a V4L2ControlList associated with a V4L2 device\n>   * \\param[in] info The V4L2 device control info map\n>   */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 06155eb51c03..a2b0d891b12f 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n>   */\n>  void V4L2Device::listControls()\n>  {\n> -\tControlInfoMap ctrls;\n> +\tControlInfoMap::Map ctrls;\n>  \tstruct v4l2_query_ext_ctrl ctrl = {};\n>  \n>  \t/* \\todo Add support for menu and compound controls. */\n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> index 182228f3a5b1..31879b794ed0 100644\n> --- a/test/v4l2_videodevice/controls.cpp\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -26,7 +26,7 @@ public:\n>  protected:\n>  \tint run()\n>  \t{\n> -\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> +\t\tconst ControlInfoMap &info = capture_->controls();\n>  \n>  \t\t/* Test control enumeration. */\n>  \t\tif (info.empty()) {\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BBEB600F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 02:40:51 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id v24so18367712ljj.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Oct 2019 17:40:51 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\t196sm4606615ljj.76.2019.10.14.17.40.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 14 Oct 2019 17:40:49 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=MjrkxWt0mQaFF9B5gt7La6LOSfUkCCJuinxUr/k4Jes=;\n\tb=KlwCK4tU0MyianH5f5OSNcOYJdVvvtaBZY1JHKURbPNI94PDXlo63xElYa/vglj4wR\n\tMYTM56O7uJXsBUaxDFAUIM/3t8bn2paPMwdC8bUqjM3ATT2GAB/lpyBUgdag4y10iRzV\n\tFK+wuj5iQXZfmxnyCUn3e3q9DbjtHKKM15Y9NNCztKBMQd9NNB9Rb3o2qOh7z+lbhFtR\n\t904YkHXvz3KDLI7WV05yPG9320S93Dwg38Z0t9JHOy5MzC7YJr+3zgvm0g6xcRUjkRG5\n\t9Vm+5ehDR05dYYKTIZP3ckCq1DtvLig+U1g2mCSRBMjSiDLa6ITPtdvzei3CjukiB+Cm\n\tI3dA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=MjrkxWt0mQaFF9B5gt7La6LOSfUkCCJuinxUr/k4Jes=;\n\tb=l+0qseMWpiMLCmgrVaNEIEZMyNSwJrSWnmipqWMpEg8fEHSQ7T/yooO4nZ1jCthbc4\n\t5U3Z6zdzZfND16BngABDXAlEC7hvNgceM7O+2w6bsQHFMNK8Ho9vXzDQmn1Tc75vPwCf\n\t50u8oJ7Yx8Rrq8rIft9ABJIZKFEu7Ihi30nc2ya5jN1G21V4LId4s8tn8bnWqHKbtaqU\n\t7bbg+GxM4yb891wAhCIVp8Kyo1gLSc5gkUqktvA2Wi2E/VVt1KUv9APMQVGJIEEZYhaO\n\t8gTiy+MPCgValH3sjWD1kY2D0ngDSbWnvEYsSLf6tHDtJI6Rxy63CU9LnTWbH441rm9I\n\tbJLg==","X-Gm-Message-State":"APjAAAW4Typ/Bgdz1dguSoktznBaCzyjbeee9t/9NDa6F0Az6u1PRXI8\n\t8vJLZSEjvzbX/SRaF7T0dwku3Zma15w=","X-Google-Smtp-Source":"APXvYqxt5lXuiyb87wCkq/4QZUppntyDoyy3I2geX4MmVNb78EuWPfT+hkGSyHeB9RCKbV+NGen73g==","X-Received":"by 2002:a05:651c:1042:: with SMTP id\n\tx2mr21108013ljm.127.1571100050488; \n\tMon, 14 Oct 2019 17:40:50 -0700 (PDT)","Date":"Tue, 15 Oct 2019 02:40:49 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015004049.GI5976@bigcity.dyn.berto.se>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Oct 2019 00:40:51 -0000"}},{"id":2907,"web_url":"https://patchwork.libcamera.org/comment/2907/","msgid":"<20191015135455.GB4875@pendragon.ideasonboard.com>","date":"2019-10-15T13:54:55","subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Tue, Oct 15, 2019 at 02:40:49AM +0200, Niklas Söderlund wrote:\n> On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:\n> > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with\n> > the latter adding convenience accessors based on numerical IDs for the\n> > former, as well as a cached idmap. Both features can be useful for\n> > ControlInfoMap in the context of serialisation, and merging the two\n> > classes will further simplify the IPA API.\n> > \n> > Import all the features of V4L2ControlInfoMap into ControlInfoMap,\n> > turning the latter into a real class. A few new constructors and\n> > assignment operators are added for completeness.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> The change to pipeline handler implementations to shorten the line \n> length could be done in a separate patch ;-) With out without that \n> addressed,\n\nThey're not there to shorten the line length, they actually change the\nbehaviour.\n\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> > ---\n> >  include/ipa/ipa_interface.h              |   2 +-\n> >  include/libcamera/controls.h             |  45 +++++++-\n> >  src/ipa/ipa_vimc.cpp                     |   2 +-\n> >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-\n> >  src/libcamera/camera_sensor.cpp          |   2 +-\n> >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-\n> >  src/libcamera/include/camera_sensor.h    |   4 +-\n> >  src/libcamera/include/v4l2_controls.h    |  36 +-----\n> >  src/libcamera/include/v4l2_device.h      |   4 +-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-\n> >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-\n> >  src/libcamera/pipeline/vimc.cpp          |  11 +-\n> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-\n> >  src/libcamera/v4l2_controls.cpp          |  94 +--------------\n> >  src/libcamera/v4l2_device.cpp            |   2 +-\n> >  test/v4l2_videodevice/controls.cpp       |   2 +-\n> >  16 files changed, 220 insertions(+), 154 deletions(-)\n\n[snip]\n\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 5534a2edb567..80414c6f0748 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -118,7 +118,50 @@ private:\n> >  };\n> >  \n> >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;\n> > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;\n> > +\n> > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>\n> > +{\n> > +public:\n> > +\tusing Map = std::unordered_map<const ControlId *, ControlRange>;\n> > +\n> > +\tControlInfoMap() = default;\n> > +\tControlInfoMap(const ControlInfoMap &other) = default;\n> > +\tControlInfoMap(std::initializer_list<Map::value_type> init);\n> > +\n> > +\tControlInfoMap &operator=(const ControlInfoMap &other) = default;\n> > +\tControlInfoMap &operator=(std::initializer_list<Map::value_type> init);\n> > +\tControlInfoMap &operator=(Map &&info);\n> > +\n> > +\tusing Map::key_type;\n> > +\tusing Map::mapped_type;\n> > +\tusing Map::value_type;\n> > +\tusing Map::size_type;\n> > +\tusing Map::iterator;\n> > +\tusing Map::const_iterator;\n> > +\n> > +\tusing Map::begin;\n> > +\tusing Map::cbegin;\n> > +\tusing Map::end;\n> > +\tusing Map::cend;\n> > +\tusing Map::at;\n> > +\tusing Map::empty;\n> > +\tusing Map::size;\n> > +\tusing Map::count;\n> > +\tusing Map::find;\n> > +\n> > +\tmapped_type &at(unsigned int key);\n> > +\tconst mapped_type &at(unsigned int key) const;\n> > +\tsize_type count(unsigned int key) const;\n> > +\titerator find(unsigned int key);\n> > +\tconst_iterator find(unsigned int key) const;\n> > +\n> > +\tconst ControlIdMap &idmap() const { return idmap_; }\n> > +\n> > +private:\n> > +\tvoid generateIdmap();\n> > +\n> > +\tControlIdMap idmap_;\n> > +};\n\n[snip]\n\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 9b19bde8a274..7a28b03b8d38 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n\n[snip]\n\n> > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >  \tstd::unique_ptr<RkISP1CameraData> data =\n> >  \t\tutils::make_unique<RkISP1CameraData>(this);\n> >  \n> > -\tdata->controlInfo_.emplace(std::piecewise_construct,\n> > -\t\t\t\t   std::forward_as_tuple(&controls::AeEnable),\n> > -\t\t\t\t   std::forward_as_tuple(false, true));\n> > +\tControlInfoMap::Map ctrls;\n> > +\tctrls.emplace(std::piecewise_construct,\n> > +\t\t      std::forward_as_tuple(&controls::AeEnable),\n> > +\t\t      std::forward_as_tuple(false, true));\n> > +\n> > +\tdata->controlInfo_ = std::move(ctrls);\n\nctrls is a ControlInfoMap::Map, and data->controlInfo_ is a\nControlInfoMap. The latter is constructed from the former using the\nControlInfoMap::operator=(Map &&info) assignment operator. This is\nneeded as ControlInfoMap doesn't provide accessors that allow modifying\nits contents.\n\n> >  \n> >  \tdata->sensor_ = new CameraSensor(sensor);\n> >  \tret = data->sensor_->init();","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 BCEEB61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 15:54:58 +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 38216324;\n\tTue, 15 Oct 2019 15:54:58 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1571147698;\n\tbh=Q1XDNqoALhoWobNiN2dwmAk0bKihopZ01d+ncR6GhEU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=adSBFa6/K0vTq0Xe8JYsBKk8e17+cq0fvqRMumQoyZqc3Ce5+VOPvgPCWh0Tuhpun\n\tIfRbQGvNGUIkaJZwkymqpqg/lH3+z0tV5qXiMLp7T/5MDXs9VczI0Jk9M5CjMrvmL3\n\tDcLq7ANg90XsHWseoDGsI2h5jMyuWIWCMp5F1HJA=","Date":"Tue, 15 Oct 2019 16:54:55 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015135455.GB4875@pendragon.ideasonboard.com>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>\n\t<20191015004049.GI5976@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191015004049.GI5976@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Oct 2019 13:54:58 -0000"}},{"id":2908,"web_url":"https://patchwork.libcamera.org/comment/2908/","msgid":"<20191015143906.GA3511@bigcity.dyn.berto.se>","date":"2019-10-15T14:39:06","subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2019-10-15 16:54:55 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Tue, Oct 15, 2019 at 02:40:49AM +0200, Niklas Söderlund wrote:\n> > On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:\n> > > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with\n> > > the latter adding convenience accessors based on numerical IDs for the\n> > > former, as well as a cached idmap. Both features can be useful for\n> > > ControlInfoMap in the context of serialisation, and merging the two\n> > > classes will further simplify the IPA API.\n> > > \n> > > Import all the features of V4L2ControlInfoMap into ControlInfoMap,\n> > > turning the latter into a real class. A few new constructors and\n> > > assignment operators are added for completeness.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > The change to pipeline handler implementations to shorten the line \n> > length could be done in a separate patch ;-) With out without that \n> > addressed,\n> \n> They're not there to shorten the line length, they actually change the\n> behaviour.\n> \n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > \n> > > ---\n> > >  include/ipa/ipa_interface.h              |   2 +-\n> > >  include/libcamera/controls.h             |  45 +++++++-\n> > >  src/ipa/ipa_vimc.cpp                     |   2 +-\n> > >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-\n> > >  src/libcamera/camera_sensor.cpp          |   2 +-\n> > >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-\n> > >  src/libcamera/include/camera_sensor.h    |   4 +-\n> > >  src/libcamera/include/v4l2_controls.h    |  36 +-----\n> > >  src/libcamera/include/v4l2_device.h      |   4 +-\n> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-\n> > >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-\n> > >  src/libcamera/pipeline/vimc.cpp          |  11 +-\n> > >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-\n> > >  src/libcamera/v4l2_controls.cpp          |  94 +--------------\n> > >  src/libcamera/v4l2_device.cpp            |   2 +-\n> > >  test/v4l2_videodevice/controls.cpp       |   2 +-\n> > >  16 files changed, 220 insertions(+), 154 deletions(-)\n> \n> [snip]\n> \n> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index 5534a2edb567..80414c6f0748 100644\n> > > --- a/include/libcamera/controls.h\n> > > +++ b/include/libcamera/controls.h\n> > > @@ -118,7 +118,50 @@ private:\n> > >  };\n> > >  \n> > >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;\n> > > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;\n> > > +\n> > > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>\n> > > +{\n> > > +public:\n> > > +\tusing Map = std::unordered_map<const ControlId *, ControlRange>;\n> > > +\n> > > +\tControlInfoMap() = default;\n> > > +\tControlInfoMap(const ControlInfoMap &other) = default;\n> > > +\tControlInfoMap(std::initializer_list<Map::value_type> init);\n> > > +\n> > > +\tControlInfoMap &operator=(const ControlInfoMap &other) = default;\n> > > +\tControlInfoMap &operator=(std::initializer_list<Map::value_type> init);\n> > > +\tControlInfoMap &operator=(Map &&info);\n> > > +\n> > > +\tusing Map::key_type;\n> > > +\tusing Map::mapped_type;\n> > > +\tusing Map::value_type;\n> > > +\tusing Map::size_type;\n> > > +\tusing Map::iterator;\n> > > +\tusing Map::const_iterator;\n> > > +\n> > > +\tusing Map::begin;\n> > > +\tusing Map::cbegin;\n> > > +\tusing Map::end;\n> > > +\tusing Map::cend;\n> > > +\tusing Map::at;\n> > > +\tusing Map::empty;\n> > > +\tusing Map::size;\n> > > +\tusing Map::count;\n> > > +\tusing Map::find;\n> > > +\n> > > +\tmapped_type &at(unsigned int key);\n> > > +\tconst mapped_type &at(unsigned int key) const;\n> > > +\tsize_type count(unsigned int key) const;\n> > > +\titerator find(unsigned int key);\n> > > +\tconst_iterator find(unsigned int key) const;\n> > > +\n> > > +\tconst ControlIdMap &idmap() const { return idmap_; }\n> > > +\n> > > +private:\n> > > +\tvoid generateIdmap();\n> > > +\n> > > +\tControlIdMap idmap_;\n> > > +};\n> \n> [snip]\n> \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 9b19bde8a274..7a28b03b8d38 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> \n> [snip]\n> \n> > > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > >  \tstd::unique_ptr<RkISP1CameraData> data =\n> > >  \t\tutils::make_unique<RkISP1CameraData>(this);\n> > >  \n> > > -\tdata->controlInfo_.emplace(std::piecewise_construct,\n> > > -\t\t\t\t   std::forward_as_tuple(&controls::AeEnable),\n> > > -\t\t\t\t   std::forward_as_tuple(false, true));\n> > > +\tControlInfoMap::Map ctrls;\n> > > +\tctrls.emplace(std::piecewise_construct,\n> > > +\t\t      std::forward_as_tuple(&controls::AeEnable),\n> > > +\t\t      std::forward_as_tuple(false, true));\n> > > +\n> > > +\tdata->controlInfo_ = std::move(ctrls);\n> \n> ctrls is a ControlInfoMap::Map, and data->controlInfo_ is a\n> ControlInfoMap. The latter is constructed from the former using the\n> ControlInfoMap::operator=(Map &&info) assignment operator. This is\n> needed as ControlInfoMap doesn't provide accessors that allow modifying\n> its contents.\n\nMy bad I missed the ::Map part, thanks for clarifying.\n\n> \n> > >  \n> > >  \tdata->sensor_ = new CameraSensor(sensor);\n> > >  \tret = data->sensor_->init();\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27B0B61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 16:39:08 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id v24so20535273ljj.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 07:39:08 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tj26sm4845858lja.25.2019.10.15.07.39.06\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 15 Oct 2019 07:39:06 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=Wf/Gp8GoDSA2sTk3NZO6xhuW0e11zPyyDgZx/uAnwOM=;\n\tb=N4zcQTATm83fZM6i73D7rg209gc+1jQrCf/TfM48q7k/8lmppzQmtTIO6nwI7CAo9r\n\tos47U81W3Vv+/vqsnCj5I8KVo5E6S3j7w5jE5hc7Q/MydZT2iI+owDmfQcA70ej5IhPA\n\tllqTsyRSH8XcSTopx75FokyGaoSyTSMNuffghH+OX7Bx3IL7+Uw4E7N2YyrxedPzLaOX\n\t+XOlTA+wh8J/qhaROJkyCs0dPyEYDy0+OxWPzjA+D8PG6MsWQX1f4mUYRm1EPl9ts3uM\n\trhctJPm5kS2u24JHelXxbxuqzu6R3OVVpKTyxvkcrwcp9FTU3D8HZJzI7jgF/Pjl3jUN\n\tjz0Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=Wf/Gp8GoDSA2sTk3NZO6xhuW0e11zPyyDgZx/uAnwOM=;\n\tb=tH9abPK859WICYR+If67t/UieR5Yt5C5SA5w0aubXh93UqpdUg4MvigWKNpIjDwKNk\n\tJfaUo06xNmZOGPiGuM/beMqnpRWyXMo5MsOMZK20DH/HPSqrGP4i9kfbwZrDIa3zgA4n\n\ttZkPehtrF8xWQT9ptjv1VAODJRt1bV9sUCfHZtmziC41uCM7NRgALbnwRv3+y4WsEeh8\n\t+mPsO8qnUBDbdP2kYRSl/r8SaBYADKhK8C19Bjw2BjzjIh75X3fsALYPeg5HV8oK9N+M\n\tESepypdfZvzvG0Wt7YyYSkqk6LBpCbBDDfUUzy3mMcHWtsy98sRJyjHibiz8EFoNZKG2\n\tV8oQ==","X-Gm-Message-State":"APjAAAXqKj/N59qDuec0iSocPWYxBOvMPIEdsybZJO1U+XrSXCj6NjBY\n\to09clEAXM6mWLBAwGrGd7uaWeuSyLO4=","X-Google-Smtp-Source":"APXvYqzo53M9YE6NFNs4Ncz+t7rLkUzWrQsQ2ym0FcywAvtSo4qTEGhAeDWKlxUsSPoeWmTT+9oGcg==","X-Received":"by 2002:a2e:9604:: with SMTP id v4mr3830967ljh.101.1571150347321;\n\tTue, 15 Oct 2019 07:39:07 -0700 (PDT)","Date":"Tue, 15 Oct 2019 16:39:06 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015143906.GA3511@bigcity.dyn.berto.se>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>\n\t<20191015004049.GI5976@bigcity.dyn.berto.se>\n\t<20191015135455.GB4875@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191015135455.GB4875@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Oct 2019 14:39:08 -0000"}},{"id":2923,"web_url":"https://patchwork.libcamera.org/comment/2923/","msgid":"<20191015184450.v4ygtimldxfkomie@uno.localdomain>","date":"2019-10-15T18:44:50","subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Oct 14, 2019 at 02:27:55AM +0300, Laurent Pinchart wrote:\n> The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with\n> the latter adding convenience accessors based on numerical IDs for the\n> former, as well as a cached idmap. Both features can be useful for\n> ControlInfoMap in the context of serialisation, and merging the two\n> classes will further simplify the IPA API.\n>\n> Import all the features of V4L2ControlInfoMap into ControlInfoMap,\n> turning the latter into a real class. A few new constructors and\n> assignment operators are added for completeness.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/ipa/ipa_interface.h              |   2 +-\n>  include/libcamera/controls.h             |  45 +++++++-\n>  src/ipa/ipa_vimc.cpp                     |   2 +-\n>  src/ipa/rkisp1/rkisp1.cpp                |   6 +-\n>  src/libcamera/camera_sensor.cpp          |   2 +-\n>  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-\n>  src/libcamera/include/camera_sensor.h    |   4 +-\n>  src/libcamera/include/v4l2_controls.h    |  36 +-----\n>  src/libcamera/include/v4l2_device.h      |   4 +-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-\n>  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-\n>  src/libcamera/pipeline/vimc.cpp          |  11 +-\n>  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-\n>  src/libcamera/v4l2_controls.cpp          |  94 +--------------\n>  src/libcamera/v4l2_device.cpp            |   2 +-\n>  test/v4l2_videodevice/controls.cpp       |   2 +-\n>  16 files changed, 220 insertions(+), 154 deletions(-)\n>\n> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> index dfb1bcbee516..8fd658af5fdb 100644\n> --- a/include/ipa/ipa_interface.h\n> +++ b/include/ipa/ipa_interface.h\n> @@ -43,7 +43,7 @@ public:\n>  \tvirtual int init() = 0;\n>\n>  \tvirtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;\n> +\t\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;\n>\n>  \tvirtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;\n>  \tvirtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 5534a2edb567..80414c6f0748 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -118,7 +118,50 @@ private:\n>  };\n>\n>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;\n> -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;\n> +\n> +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>\n> +{\n> +public:\n> +\tusing Map = std::unordered_map<const ControlId *, ControlRange>;\n> +\n> +\tControlInfoMap() = default;\n> +\tControlInfoMap(const ControlInfoMap &other) = default;\n> +\tControlInfoMap(std::initializer_list<Map::value_type> init);\n> +\n> +\tControlInfoMap &operator=(const ControlInfoMap &other) = default;\n> +\tControlInfoMap &operator=(std::initializer_list<Map::value_type> init);\n> +\tControlInfoMap &operator=(Map &&info);\n> +\n> +\tusing Map::key_type;\n> +\tusing Map::mapped_type;\n> +\tusing Map::value_type;\n> +\tusing Map::size_type;\n> +\tusing Map::iterator;\n> +\tusing Map::const_iterator;\n> +\n> +\tusing Map::begin;\n> +\tusing Map::cbegin;\n> +\tusing Map::end;\n> +\tusing Map::cend;\n> +\tusing Map::at;\n> +\tusing Map::empty;\n> +\tusing Map::size;\n> +\tusing Map::count;\n> +\tusing Map::find;\n> +\n> +\tmapped_type &at(unsigned int key);\n> +\tconst mapped_type &at(unsigned int key) const;\n> +\tsize_type count(unsigned int key) const;\n> +\titerator find(unsigned int key);\n> +\tconst_iterator find(unsigned int key) const;\n> +\n> +\tconst ControlIdMap &idmap() const { return idmap_; }\n> +\n> +private:\n> +\tvoid generateIdmap();\n> +\n> +\tControlIdMap idmap_;\n> +};\n>\n>  class ControlList\n>  {\n> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp\n> index 620dc25f456e..63d578b4e2aa 100644\n> --- a/src/ipa/ipa_vimc.cpp\n> +++ b/src/ipa/ipa_vimc.cpp\n> @@ -31,7 +31,7 @@ public:\n>\n>  \tint init() override;\n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n>  \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index d64c334cbf0c..bd703898c523 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -33,7 +33,7 @@ public:\n>  \tint init() override { return 0; }\n>\n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override;\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n>  \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -49,7 +49,7 @@ private:\n>\n>  \tstd::map<unsigned int, BufferMemory> bufferInfo_;\n>\n> -\tV4L2ControlInfoMap ctrls_;\n> +\tControlInfoMap ctrls_;\n>\n>  \t/* Camera sensor controls. */\n>  \tbool autoExposure_;\n> @@ -62,7 +62,7 @@ private:\n>  };\n>\n>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t\t  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)\n> +\t\t\t  const std::map<unsigned int, ControlInfoMap> &entityControls)\n>  {\n>  \tif (entityControls.empty())\n>  \t\treturn;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 1b8e8c0e07da..86f0c772861b 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n>   * \\brief Retrieve the supported V4L2 controls and their information\n>   * \\return A map of the V4L2 controls supported by the sensor\n>   */\n> -const V4L2ControlInfoMap &CameraSensor::controls() const\n> +const ControlInfoMap &CameraSensor::controls() const\n>  {\n>  \treturn subdev_->controls();\n>  }\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 6a0301f3a2ae..bf7634aab470 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -393,10 +393,147 @@ std::string ControlRange::toString() const\n>   */\n>\n>  /**\n> - * \\typedef ControlInfoMap\n> + * \\class ControlInfoMap\n>   * \\brief A map of ControlId to ControlRange\n> + *\n> + * The ControlInfoMap class describes controls supported by an object as an\n> + * unsorted map of ControlId pointers to ControlRange instances. Unlike the\n> + * standard std::unsorted_map<> class, it is designed the be immutable once\n> + * constructed, and thus only exposes the read accessors of the\n> + * std::unsorted_map<> base class.\n> + *\n> + * In addition to the features of the standard unsorted map, this class also\n> + * provides access to the mapped elements using numerical ID keys. It maintains\n> + * an internal map of numerical ID to ControlId for this purpose, and exposes it\n> + * through the idmap() method to help construction of ControlList instances.\n\nJust a late thoughts on this... the ControlInfoMap generated idmap is\nonly used for constructing ControlList of v4l2 controls, while for\nlibcamera controls we have a global id map. This creates a little\nasymmetry which is not nice.\n\nTo enforce this concept I wonder if we should not ask users of\nControlList of v4l2 controls to explicitly pass the idmap generated\nfrom the ControlInfoMap, by deleting\n\tControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr);\n\nOr either, make this one the only available constructor, so also\nControlList of libcamera controls cannot be created with the global\ncontrols:controls map, but only from the ControlInfoMap of a Camera.\n\n>   */\n>\n> +/**\n> + * \\typedef ControlInfoMap::Map\n> + * \\brief The base std::unsorted_map<> container\n> + */\n> +\n> +/**\n> + * \\fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)\n> + * \\brief Copy constructor, construct a ControlInfoMap from a copy of \\a other\n> + * \\param[in] other The other ControlInfoMap\n> + */\n> +\n> +/**\n> + * \\brief Construct a ControlInfoMap from an initializer list\n> + * \\param[in] init The initializer list\n> + */\n> +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)\n> +\t: Map(init)\n> +{\n> +\tgenerateIdmap();\n> +}\n> +\n> +/**\n> + * \\fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)\n> + * \\brief Copy assignment operator, replace the contents with a copy of \\a other\n\ncontent or contents ?\n\n> + * \\param[in] other The other ControlInfoMap\n> + * \\return A reference to the ControlInfoMap\n> + */\n> +\n> +/**\n> + * \\brief Replace the contents with those from the initializer list\n> + * \\param[in] init The initializer list\n> + * \\return A reference to the ControlInfoMap\n> + */\n> +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)\n> +{\n> +\tMap::operator=(init);\n> +\tgenerateIdmap();\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\brief Move assignment operator from a plain map\n> + * \\param[in] info The control info plain map\n> + *\n> + * Populate the map by replacing its contents with those of \\a info using move\n\nThis second controls makes me think it was intentional in first place\n\n> + * semantics. Upon return the \\a info map will be empty.\n\nbut this one 'semantics' should probably be singular\n\n> + *\n> + * \\return A reference to the populated ControlInfoMap\n> + */\n> +ControlInfoMap &ControlInfoMap::operator=(Map &&info)\n> +{\n> +\tMap::operator=(std::move(info));\n> +\tgenerateIdmap();\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\brief Access specified element by numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A reference to the element whose ID is equal to \\a id\n> + */\n> +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)\n> +{\n> +\treturn at(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Access specified element by numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A const reference to the element whose ID is equal to \\a id\n> + */\n> +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> +{\n> +\treturn at(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Count the number of elements matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return The number of elements matching the numerical \\a id\n> + */\n> +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> +{\n> +\treturn count(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Find the element matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return An iterator pointing to the element matching the numerical \\a id, or\n> + * end() if no such element exists\n> + */\n> +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> +{\n> +\treturn find(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\brief Find the element matching a numerical ID\n> + * \\param[in] id The numerical ID\n> + * \\return A const iterator pointing to the element matching the numerical\n> + * \\a id, or end() if no such element exists\n> + */\n> +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> +{\n> +\treturn find(idmap_.at(id));\n> +}\n> +\n> +/**\n> + * \\fn const ControlIdMap &ControlInfoMap::idmap() const\n> + * \\brief Retrieve the ControlId map\n> + *\n> + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> + * for the V4L2 device that the control list targets. This helper method\n> + * returns a suitable idmap for that purpose.\n> + *\n> + * \\return The ControlId map\n> + */\n> +\n> +void ControlInfoMap::generateIdmap()\n> +{\n> +\tidmap_.clear();\n> +\tfor (const auto &ctrl : *this)\n> +\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> +}\n> +\n>  /**\n>   * \\class ControlList\n>   * \\brief Associate a list of ControlId with their values for an object\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index f426e29b84bf..1fb36a4f4951 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -16,9 +16,9 @@\n>\n>  namespace libcamera {\n>\n> +class ControlInfoMap;\n>  class ControlList;\n>  class MediaEntity;\n> -class V4L2ControlInfoMap;\n>  class V4L2Subdevice;\n>\n>  struct V4L2SubdeviceFormat;\n> @@ -43,7 +43,7 @@ public:\n>  \t\t\t\t      const Size &size) const;\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>\n> -\tconst V4L2ControlInfoMap &controls() const;\n> +\tconst ControlInfoMap &controls() const;\n>  \tint getControls(ControlList *ctrls);\n>  \tint setControls(ControlList *ctrls);\n>\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index c427b845d8b4..e16c4957f9d1 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -31,44 +31,10 @@ public:\n>  \tV4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);\n>  };\n>\n> -class V4L2ControlInfoMap : private ControlInfoMap\n> -{\n> -public:\n> -\tV4L2ControlInfoMap &operator=(ControlInfoMap &&info);\n> -\n> -\tusing ControlInfoMap::key_type;\n> -\tusing ControlInfoMap::mapped_type;\n> -\tusing ControlInfoMap::value_type;\n> -\tusing ControlInfoMap::size_type;\n> -\tusing ControlInfoMap::iterator;\n> -\tusing ControlInfoMap::const_iterator;\n> -\n> -\tusing ControlInfoMap::begin;\n> -\tusing ControlInfoMap::cbegin;\n> -\tusing ControlInfoMap::end;\n> -\tusing ControlInfoMap::cend;\n> -\tusing ControlInfoMap::at;\n> -\tusing ControlInfoMap::empty;\n> -\tusing ControlInfoMap::size;\n> -\tusing ControlInfoMap::count;\n> -\tusing ControlInfoMap::find;\n> -\n> -\tmapped_type &at(unsigned int key);\n> -\tconst mapped_type &at(unsigned int key) const;\n> -\tsize_type count(unsigned int key) const;\n> -\titerator find(unsigned int key);\n> -\tconst_iterator find(unsigned int key) const;\n> -\n> -\tconst ControlIdMap &idmap() const { return idmap_; }\n> -\n> -private:\n> -\tControlIdMap idmap_;\n> -};\n> -\n>  class V4L2ControlList : public ControlList\n>  {\n>  public:\n> -\tV4L2ControlList(const V4L2ControlInfoMap &info)\n> +\tV4L2ControlList(const ControlInfoMap &info)\n>  \t\t: ControlList(info.idmap())\n>  \t{\n>  \t}\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index f30b1c2cde34..6bfddefe336c 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -24,7 +24,7 @@ public:\n>  \tvoid close();\n>  \tbool isOpen() const { return fd_ != -1; }\n>\n> -\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> +\tconst ControlInfoMap &controls() const { return controls_; }\n>\n>  \tint getControls(ControlList *ctrls);\n>  \tint setControls(ControlList *ctrls);\n> @@ -49,7 +49,7 @@ private:\n>  \t\t\t    unsigned int count);\n>\n>  \tstd::vector<std::unique_ptr<V4L2ControlId>> controlIds_;\n> -\tV4L2ControlInfoMap controls_;\n> +\tControlInfoMap controls_;\n>  \tstd::string deviceNode_;\n>  \tint fd_;\n>  };\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 9b19bde8a274..7a28b03b8d38 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n>  \t\t.size = data->stream_.configuration().size,\n>  \t};\n>\n> -\tstd::map<unsigned int, V4L2ControlInfoMap> entityControls;\n> +\tstd::map<unsigned int, ControlInfoMap> entityControls;\n>  \tentityControls.emplace(0, data->sensor_->controls());\n>\n>  \tdata->ipa_->configure(streamConfig, entityControls);\n> @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>  \tstd::unique_ptr<RkISP1CameraData> data =\n>  \t\tutils::make_unique<RkISP1CameraData>(this);\n>\n> -\tdata->controlInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t   std::forward_as_tuple(&controls::AeEnable),\n> -\t\t\t\t   std::forward_as_tuple(false, true));\n> +\tControlInfoMap::Map ctrls;\n> +\tctrls.emplace(std::piecewise_construct,\n> +\t\t      std::forward_as_tuple(&controls::AeEnable),\n> +\t\t      std::forward_as_tuple(false, true));\n> +\n> +\tdata->controlInfo_ = std::move(ctrls);\n>\n>  \tdata->sensor_ = new CameraSensor(sensor);\n>  \tret = data->sensor_->init();\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 018c7691d263..a64e1af4c550 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \tvideo_->bufferReady.connect(this, &UVCCameraData::bufferReady);\n>\n>  \t/* Initialise the supported controls. */\n> -\tconst V4L2ControlInfoMap &controls = video_->controls();\n> +\tconst ControlInfoMap &controls = video_->controls();\n> +\tControlInfoMap::Map ctrls;\n> +\n>  \tfor (const auto &ctrl : controls) {\n>  \t\tconst ControlRange &range = ctrl.second;\n>  \t\tconst ControlId *id;\n> @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \t\t\tcontinue;\n>  \t\t}\n>\n> -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(range));\n> +\t\tctrls.emplace(std::piecewise_construct,\n> +\t\t\t      std::forward_as_tuple(id),\n> +\t\t\t      std::forward_as_tuple(range));\n>  \t}\n>\n> +\tcontrolInfo_ = std::move(ctrls);\n> +\n>  \treturn 0;\n>  }\n>\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f7f82edc6530..6a4244119dc5 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t\treturn -ENODEV;\n>\n>  \t/* Initialise the supported controls. */\n> -\tconst V4L2ControlInfoMap &controls = sensor_->controls();\n> +\tconst ControlInfoMap &controls = sensor_->controls();\n> +\tControlInfoMap::Map ctrls;\n> +\n>  \tfor (const auto &ctrl : controls) {\n>  \t\tconst ControlRange &range = ctrl.second;\n>  \t\tconst ControlId *id;\n> @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t\t\tcontinue;\n>  \t\t}\n>\n> -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> -\t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(range));\n> +\t\tctrls.emplace(std::piecewise_construct,\n> +\t\t\t      std::forward_as_tuple(id),\n> +\t\t\t      std::forward_as_tuple(range));\n>  \t}\n>\n> +\tcontrolInfo_ = std::move(ctrls);\n>  \treturn 0;\n>  }\n>\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 41bd965f0284..4e6fa6899e07 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -28,7 +28,7 @@ public:\n>\n>  \tint init() override { return 0; }\n>  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n>  \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index e783457caf94..bd5e18590b76 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)\n>  \t\t\t\t\t\t     static_cast<int32_t>(ctrl.maximum)));\n>  }\n>\n> -/**\n> - * \\class V4L2ControlInfoMap\n> - * \\brief A map of controlID to ControlRange for V4L2 controls\n> - */\n> -\n> -/**\n> - * \\brief Move assignment operator from a ControlInfoMap\n> - * \\param[in] info The control info map\n> - *\n> - * Populate the map by replacing its contents with those of \\a info using move\n> - * semantics. Upon return the \\a info map will be empty.\n> - *\n> - * This is the only supported way to populate a V4L2ControlInfoMap.\n> - *\n> - * \\return The populated V4L2ControlInfoMap\n> - */\n> -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)\n> -{\n> -\tControlInfoMap::operator=(std::move(info));\n> -\n> -\tidmap_.clear();\n> -\tfor (const auto &ctrl : *this)\n> -\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> -\n> -\treturn *this;\n> -}\n> -\n> -/**\n> - * \\brief Access specified element by numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A reference to the element whose ID is equal to \\a id\n> - */\n> -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)\n> -{\n> -\treturn at(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Access specified element by numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A const reference to the element whose ID is equal to \\a id\n> - */\n> -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const\n> -{\n> -\treturn at(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Count the number of elements matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return The number of elements matching the numerical \\a id\n> - */\n> -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const\n> -{\n> -\treturn count(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Find the element matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return An iterator pointing to the element matching the numerical \\a id, or\n> - * end() if no such element exists\n> - */\n> -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)\n> -{\n> -\treturn find(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\brief Find the element matching a numerical ID\n> - * \\param[in] id The numerical ID\n> - * \\return A const iterator pointing to the element matching the numerical\n> - * \\a id, or end() if no such element exists\n> - */\n> -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const\n> -{\n> -\treturn find(idmap_.at(id));\n> -}\n> -\n> -/**\n> - * \\fn const ControlIdMap &V4L2ControlInfoMap::idmap() const\n> - * \\brief Retrieve the ControlId map\n> - *\n> - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> - * for the V4L2 device that the control list targets. This helper method\n> - * returns a suitable idmap for that purpose.\n> - *\n> - * \\return The ControlId map\n> - */\n> -\n>  /**\n>   * \\class V4L2ControlList\n>   * \\brief A list of controls for a V4L2 device\n>   *\n>   * This class specialises the ControList class for use with V4L2 devices. It\n> - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.\n> + * offers a convenience API to create a ControlList from a ControlInfoMap.\n>   *\n>   * V4L2ControlList allows easy construction of a ControlList containing V4L2\n>   * controls for a device. It can be used to construct the list of controls\n> @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con\n>   */\n>\n>  /**\n> - * \\fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)\n> + * \\fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)\n>   * \\brief Construct a V4L2ControlList associated with a V4L2 device\n>   * \\param[in] info The V4L2 device control info map\n>   */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 06155eb51c03..a2b0d891b12f 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n>   */\n>  void V4L2Device::listControls()\n>  {\n> -\tControlInfoMap ctrls;\n> +\tControlInfoMap::Map ctrls;\n>  \tstruct v4l2_query_ext_ctrl ctrl = {};\n>\n>  \t/* \\todo Add support for menu and compound controls. */\n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> index 182228f3a5b1..31879b794ed0 100644\n> --- a/test/v4l2_videodevice/controls.cpp\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -26,7 +26,7 @@ public:\n>  protected:\n>  \tint run()\n>  \t{\n> -\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> +\t\tconst ControlInfoMap &info = capture_->controls();\n\nLooks good, thanks\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n>\n>  \t\t/* Test control enumeration. */\n>  \t\tif (info.empty()) {\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 relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E70EB61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 20:43:02 +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 relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 66AA160004;\n\tTue, 15 Oct 2019 18:43:02 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 15 Oct 2019 20:44:50 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015184450.v4ygtimldxfkomie@uno.localdomain>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qjmpe6vismia3fi7\"","Content-Disposition":"inline","In-Reply-To":"<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Oct 2019 18:43:03 -0000"}},{"id":2927,"web_url":"https://patchwork.libcamera.org/comment/2927/","msgid":"<20191015192904.GA19403@pendragon.ideasonboard.com>","date":"2019-10-15T19:29:04","subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Oct 15, 2019 at 08:44:50PM +0200, Jacopo Mondi wrote:\n> On Mon, Oct 14, 2019 at 02:27:55AM +0300, Laurent Pinchart wrote:\n> > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with\n> > the latter adding convenience accessors based on numerical IDs for the\n> > former, as well as a cached idmap. Both features can be useful for\n> > ControlInfoMap in the context of serialisation, and merging the two\n> > classes will further simplify the IPA API.\n> >\n> > Import all the features of V4L2ControlInfoMap into ControlInfoMap,\n> > turning the latter into a real class. A few new constructors and\n> > assignment operators are added for completeness.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/ipa/ipa_interface.h              |   2 +-\n> >  include/libcamera/controls.h             |  45 +++++++-\n> >  src/ipa/ipa_vimc.cpp                     |   2 +-\n> >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-\n> >  src/libcamera/camera_sensor.cpp          |   2 +-\n> >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-\n> >  src/libcamera/include/camera_sensor.h    |   4 +-\n> >  src/libcamera/include/v4l2_controls.h    |  36 +-----\n> >  src/libcamera/include/v4l2_device.h      |   4 +-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-\n> >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-\n> >  src/libcamera/pipeline/vimc.cpp          |  11 +-\n> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-\n> >  src/libcamera/v4l2_controls.cpp          |  94 +--------------\n> >  src/libcamera/v4l2_device.cpp            |   2 +-\n> >  test/v4l2_videodevice/controls.cpp       |   2 +-\n> >  16 files changed, 220 insertions(+), 154 deletions(-)\n> >\n> > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> > index dfb1bcbee516..8fd658af5fdb 100644\n> > --- a/include/ipa/ipa_interface.h\n> > +++ b/include/ipa/ipa_interface.h\n> > @@ -43,7 +43,7 @@ public:\n> >  \tvirtual int init() = 0;\n> >\n> >  \tvirtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;\n> > +\t\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;\n> >\n> >  \tvirtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;\n> >  \tvirtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 5534a2edb567..80414c6f0748 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -118,7 +118,50 @@ private:\n> >  };\n> >\n> >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;\n> > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;\n> > +\n> > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>\n> > +{\n> > +public:\n> > +\tusing Map = std::unordered_map<const ControlId *, ControlRange>;\n> > +\n> > +\tControlInfoMap() = default;\n> > +\tControlInfoMap(const ControlInfoMap &other) = default;\n> > +\tControlInfoMap(std::initializer_list<Map::value_type> init);\n> > +\n> > +\tControlInfoMap &operator=(const ControlInfoMap &other) = default;\n> > +\tControlInfoMap &operator=(std::initializer_list<Map::value_type> init);\n> > +\tControlInfoMap &operator=(Map &&info);\n> > +\n> > +\tusing Map::key_type;\n> > +\tusing Map::mapped_type;\n> > +\tusing Map::value_type;\n> > +\tusing Map::size_type;\n> > +\tusing Map::iterator;\n> > +\tusing Map::const_iterator;\n> > +\n> > +\tusing Map::begin;\n> > +\tusing Map::cbegin;\n> > +\tusing Map::end;\n> > +\tusing Map::cend;\n> > +\tusing Map::at;\n> > +\tusing Map::empty;\n> > +\tusing Map::size;\n> > +\tusing Map::count;\n> > +\tusing Map::find;\n> > +\n> > +\tmapped_type &at(unsigned int key);\n> > +\tconst mapped_type &at(unsigned int key) const;\n> > +\tsize_type count(unsigned int key) const;\n> > +\titerator find(unsigned int key);\n> > +\tconst_iterator find(unsigned int key) const;\n> > +\n> > +\tconst ControlIdMap &idmap() const { return idmap_; }\n> > +\n> > +private:\n> > +\tvoid generateIdmap();\n> > +\n> > +\tControlIdMap idmap_;\n> > +};\n> >\n> >  class ControlList\n> >  {\n> > diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp\n> > index 620dc25f456e..63d578b4e2aa 100644\n> > --- a/src/ipa/ipa_vimc.cpp\n> > +++ b/src/ipa/ipa_vimc.cpp\n> > @@ -31,7 +31,7 @@ public:\n> >\n> >  \tint init() override;\n> >  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> > +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n> >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n> >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n> >  \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index d64c334cbf0c..bd703898c523 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -33,7 +33,7 @@ public:\n> >  \tint init() override { return 0; }\n> >\n> >  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;\n> > +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override;\n> >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> >  \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -49,7 +49,7 @@ private:\n> >\n> >  \tstd::map<unsigned int, BufferMemory> bufferInfo_;\n> >\n> > -\tV4L2ControlInfoMap ctrls_;\n> > +\tControlInfoMap ctrls_;\n> >\n> >  \t/* Camera sensor controls. */\n> >  \tbool autoExposure_;\n> > @@ -62,7 +62,7 @@ private:\n> >  };\n> >\n> >  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t\t  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)\n> > +\t\t\t  const std::map<unsigned int, ControlInfoMap> &entityControls)\n> >  {\n> >  \tif (entityControls.empty())\n> >  \t\treturn;\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 1b8e8c0e07da..86f0c772861b 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> >   * \\brief Retrieve the supported V4L2 controls and their information\n> >   * \\return A map of the V4L2 controls supported by the sensor\n> >   */\n> > -const V4L2ControlInfoMap &CameraSensor::controls() const\n> > +const ControlInfoMap &CameraSensor::controls() const\n> >  {\n> >  \treturn subdev_->controls();\n> >  }\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 6a0301f3a2ae..bf7634aab470 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -393,10 +393,147 @@ std::string ControlRange::toString() const\n> >   */\n> >\n> >  /**\n> > - * \\typedef ControlInfoMap\n> > + * \\class ControlInfoMap\n> >   * \\brief A map of ControlId to ControlRange\n> > + *\n> > + * The ControlInfoMap class describes controls supported by an object as an\n> > + * unsorted map of ControlId pointers to ControlRange instances. Unlike the\n> > + * standard std::unsorted_map<> class, it is designed the be immutable once\n> > + * constructed, and thus only exposes the read accessors of the\n> > + * std::unsorted_map<> base class.\n> > + *\n> > + * In addition to the features of the standard unsorted map, this class also\n> > + * provides access to the mapped elements using numerical ID keys. It maintains\n> > + * an internal map of numerical ID to ControlId for this purpose, and exposes it\n> > + * through the idmap() method to help construction of ControlList instances.\n> \n> Just a late thoughts on this... the ControlInfoMap generated idmap is\n> only used for constructing ControlList of v4l2 controls, while for\n> libcamera controls we have a global id map. This creates a little\n> asymmetry which is not nice.\n> \n> To enforce this concept I wonder if we should not ask users of\n> ControlList of v4l2 controls to explicitly pass the idmap generated\n> from the ControlInfoMap, by deleting\n> \tControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr);\n> \n> Or either, make this one the only available constructor, so also\n> ControlList of libcamera controls cannot be created with the global\n> controls:controls map, but only from the ControlInfoMap of a Camera.\n\nI've thought about this too, and I prefer the latter. I wasn't sure\nabout the practical implications though, so decided to not include that\nchange in this series. Would you like to give it a try ? :-)\n\n> >   */\n> >\n> > +/**\n> > + * \\typedef ControlInfoMap::Map\n> > + * \\brief The base std::unsorted_map<> container\n> > + */\n> > +\n> > +/**\n> > + * \\fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)\n> > + * \\brief Copy constructor, construct a ControlInfoMap from a copy of \\a other\n> > + * \\param[in] other The other ControlInfoMap\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a ControlInfoMap from an initializer list\n> > + * \\param[in] init The initializer list\n> > + */\n> > +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)\n> > +\t: Map(init)\n> > +{\n> > +\tgenerateIdmap();\n> > +}\n> > +\n> > +/**\n> > + * \\fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)\n> > + * \\brief Copy assignment operator, replace the contents with a copy of \\a other\n> \n> content or contents ?\n\nIt's usually used in plural form according to\nhttps://en.wiktionary.org/wiki/contents.\n\n> > + * \\param[in] other The other ControlInfoMap\n> > + * \\return A reference to the ControlInfoMap\n> > + */\n> > +\n> > +/**\n> > + * \\brief Replace the contents with those from the initializer list\n> > + * \\param[in] init The initializer list\n> > + * \\return A reference to the ControlInfoMap\n> > + */\n> > +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)\n> > +{\n> > +\tMap::operator=(init);\n> > +\tgenerateIdmap();\n> > +\treturn *this;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Move assignment operator from a plain map\n> > + * \\param[in] info The control info plain map\n> > + *\n> > + * Populate the map by replacing its contents with those of \\a info using move\n> \n> This second controls makes me think it was intentional in first place\n\nYes :-)\n\n> > + * semantics. Upon return the \\a info map will be empty.\n> \n> but this one 'semantics' should probably be singular\n\nhttps://en.wiktionary.org/wiki/semantics\n\n> > + *\n> > + * \\return A reference to the populated ControlInfoMap\n> > + */\n> > +ControlInfoMap &ControlInfoMap::operator=(Map &&info)\n> > +{\n> > +\tMap::operator=(std::move(info));\n> > +\tgenerateIdmap();\n> > +\treturn *this;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Access specified element by numerical ID\n> > + * \\param[in] id The numerical ID\n> > + * \\return A reference to the element whose ID is equal to \\a id\n> > + */\n> > +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)\n> > +{\n> > +\treturn at(idmap_.at(id));\n> > +}\n> > +\n> > +/**\n> > + * \\brief Access specified element by numerical ID\n> > + * \\param[in] id The numerical ID\n> > + * \\return A const reference to the element whose ID is equal to \\a id\n> > + */\n> > +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const\n> > +{\n> > +\treturn at(idmap_.at(id));\n> > +}\n> > +\n> > +/**\n> > + * \\brief Count the number of elements matching a numerical ID\n> > + * \\param[in] id The numerical ID\n> > + * \\return The number of elements matching the numerical \\a id\n> > + */\n> > +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const\n> > +{\n> > +\treturn count(idmap_.at(id));\n> > +}\n> > +\n> > +/**\n> > + * \\brief Find the element matching a numerical ID\n> > + * \\param[in] id The numerical ID\n> > + * \\return An iterator pointing to the element matching the numerical \\a id, or\n> > + * end() if no such element exists\n> > + */\n> > +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)\n> > +{\n> > +\treturn find(idmap_.at(id));\n> > +}\n> > +\n> > +/**\n> > + * \\brief Find the element matching a numerical ID\n> > + * \\param[in] id The numerical ID\n> > + * \\return A const iterator pointing to the element matching the numerical\n> > + * \\a id, or end() if no such element exists\n> > + */\n> > +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const\n> > +{\n> > +\treturn find(idmap_.at(id));\n> > +}\n> > +\n> > +/**\n> > + * \\fn const ControlIdMap &ControlInfoMap::idmap() const\n> > + * \\brief Retrieve the ControlId map\n> > + *\n> > + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> > + * for the V4L2 device that the control list targets. This helper method\n> > + * returns a suitable idmap for that purpose.\n> > + *\n> > + * \\return The ControlId map\n> > + */\n> > +\n> > +void ControlInfoMap::generateIdmap()\n> > +{\n> > +\tidmap_.clear();\n> > +\tfor (const auto &ctrl : *this)\n> > +\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> > +}\n> > +\n> >  /**\n> >   * \\class ControlList\n> >   * \\brief Associate a list of ControlId with their values for an object\n> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> > index f426e29b84bf..1fb36a4f4951 100644\n> > --- a/src/libcamera/include/camera_sensor.h\n> > +++ b/src/libcamera/include/camera_sensor.h\n> > @@ -16,9 +16,9 @@\n> >\n> >  namespace libcamera {\n> >\n> > +class ControlInfoMap;\n> >  class ControlList;\n> >  class MediaEntity;\n> > -class V4L2ControlInfoMap;\n> >  class V4L2Subdevice;\n> >\n> >  struct V4L2SubdeviceFormat;\n> > @@ -43,7 +43,7 @@ public:\n> >  \t\t\t\t      const Size &size) const;\n> >  \tint setFormat(V4L2SubdeviceFormat *format);\n> >\n> > -\tconst V4L2ControlInfoMap &controls() const;\n> > +\tconst ControlInfoMap &controls() const;\n> >  \tint getControls(ControlList *ctrls);\n> >  \tint setControls(ControlList *ctrls);\n> >\n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index c427b845d8b4..e16c4957f9d1 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -31,44 +31,10 @@ public:\n> >  \tV4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);\n> >  };\n> >\n> > -class V4L2ControlInfoMap : private ControlInfoMap\n> > -{\n> > -public:\n> > -\tV4L2ControlInfoMap &operator=(ControlInfoMap &&info);\n> > -\n> > -\tusing ControlInfoMap::key_type;\n> > -\tusing ControlInfoMap::mapped_type;\n> > -\tusing ControlInfoMap::value_type;\n> > -\tusing ControlInfoMap::size_type;\n> > -\tusing ControlInfoMap::iterator;\n> > -\tusing ControlInfoMap::const_iterator;\n> > -\n> > -\tusing ControlInfoMap::begin;\n> > -\tusing ControlInfoMap::cbegin;\n> > -\tusing ControlInfoMap::end;\n> > -\tusing ControlInfoMap::cend;\n> > -\tusing ControlInfoMap::at;\n> > -\tusing ControlInfoMap::empty;\n> > -\tusing ControlInfoMap::size;\n> > -\tusing ControlInfoMap::count;\n> > -\tusing ControlInfoMap::find;\n> > -\n> > -\tmapped_type &at(unsigned int key);\n> > -\tconst mapped_type &at(unsigned int key) const;\n> > -\tsize_type count(unsigned int key) const;\n> > -\titerator find(unsigned int key);\n> > -\tconst_iterator find(unsigned int key) const;\n> > -\n> > -\tconst ControlIdMap &idmap() const { return idmap_; }\n> > -\n> > -private:\n> > -\tControlIdMap idmap_;\n> > -};\n> > -\n> >  class V4L2ControlList : public ControlList\n> >  {\n> >  public:\n> > -\tV4L2ControlList(const V4L2ControlInfoMap &info)\n> > +\tV4L2ControlList(const ControlInfoMap &info)\n> >  \t\t: ControlList(info.idmap())\n> >  \t{\n> >  \t}\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index f30b1c2cde34..6bfddefe336c 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -24,7 +24,7 @@ public:\n> >  \tvoid close();\n> >  \tbool isOpen() const { return fd_ != -1; }\n> >\n> > -\tconst V4L2ControlInfoMap &controls() const { return controls_; }\n> > +\tconst ControlInfoMap &controls() const { return controls_; }\n> >\n> >  \tint getControls(ControlList *ctrls);\n> >  \tint setControls(ControlList *ctrls);\n> > @@ -49,7 +49,7 @@ private:\n> >  \t\t\t    unsigned int count);\n> >\n> >  \tstd::vector<std::unique_ptr<V4L2ControlId>> controlIds_;\n> > -\tV4L2ControlInfoMap controls_;\n> > +\tControlInfoMap controls_;\n> >  \tstd::string deviceNode_;\n> >  \tint fd_;\n> >  };\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 9b19bde8a274..7a28b03b8d38 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> >  \t\t.size = data->stream_.configuration().size,\n> >  \t};\n> >\n> > -\tstd::map<unsigned int, V4L2ControlInfoMap> entityControls;\n> > +\tstd::map<unsigned int, ControlInfoMap> entityControls;\n> >  \tentityControls.emplace(0, data->sensor_->controls());\n> >\n> >  \tdata->ipa_->configure(streamConfig, entityControls);\n> > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >  \tstd::unique_ptr<RkISP1CameraData> data =\n> >  \t\tutils::make_unique<RkISP1CameraData>(this);\n> >\n> > -\tdata->controlInfo_.emplace(std::piecewise_construct,\n> > -\t\t\t\t   std::forward_as_tuple(&controls::AeEnable),\n> > -\t\t\t\t   std::forward_as_tuple(false, true));\n> > +\tControlInfoMap::Map ctrls;\n> > +\tctrls.emplace(std::piecewise_construct,\n> > +\t\t      std::forward_as_tuple(&controls::AeEnable),\n> > +\t\t      std::forward_as_tuple(false, true));\n> > +\n> > +\tdata->controlInfo_ = std::move(ctrls);\n> >\n> >  \tdata->sensor_ = new CameraSensor(sensor);\n> >  \tret = data->sensor_->init();\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 018c7691d263..a64e1af4c550 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)\n> >  \tvideo_->bufferReady.connect(this, &UVCCameraData::bufferReady);\n> >\n> >  \t/* Initialise the supported controls. */\n> > -\tconst V4L2ControlInfoMap &controls = video_->controls();\n> > +\tconst ControlInfoMap &controls = video_->controls();\n> > +\tControlInfoMap::Map ctrls;\n> > +\n> >  \tfor (const auto &ctrl : controls) {\n> >  \t\tconst ControlRange &range = ctrl.second;\n> >  \t\tconst ControlId *id;\n> > @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)\n> >  \t\t\tcontinue;\n> >  \t\t}\n> >\n> > -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> > -\t\t\t\t     std::forward_as_tuple(id),\n> > -\t\t\t\t     std::forward_as_tuple(range));\n> > +\t\tctrls.emplace(std::piecewise_construct,\n> > +\t\t\t      std::forward_as_tuple(id),\n> > +\t\t\t      std::forward_as_tuple(range));\n> >  \t}\n> >\n> > +\tcontrolInfo_ = std::move(ctrls);\n> > +\n> >  \treturn 0;\n> >  }\n> >\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index f7f82edc6530..6a4244119dc5 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)\n> >  \t\treturn -ENODEV;\n> >\n> >  \t/* Initialise the supported controls. */\n> > -\tconst V4L2ControlInfoMap &controls = sensor_->controls();\n> > +\tconst ControlInfoMap &controls = sensor_->controls();\n> > +\tControlInfoMap::Map ctrls;\n> > +\n> >  \tfor (const auto &ctrl : controls) {\n> >  \t\tconst ControlRange &range = ctrl.second;\n> >  \t\tconst ControlId *id;\n> > @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)\n> >  \t\t\tcontinue;\n> >  \t\t}\n> >\n> > -\t\tcontrolInfo_.emplace(std::piecewise_construct,\n> > -\t\t\t\t     std::forward_as_tuple(id),\n> > -\t\t\t\t     std::forward_as_tuple(range));\n> > +\t\tctrls.emplace(std::piecewise_construct,\n> > +\t\t\t      std::forward_as_tuple(id),\n> > +\t\t\t      std::forward_as_tuple(range));\n> >  \t}\n> >\n> > +\tcontrolInfo_ = std::move(ctrls);\n> >  \treturn 0;\n> >  }\n> >\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index 41bd965f0284..4e6fa6899e07 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -28,7 +28,7 @@ public:\n> >\n> >  \tint init() override { return 0; }\n> >  \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}\n> > +\t\t       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}\n> >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n> >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override {}\n> >  \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index e783457caf94..bd5e18590b76 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)\n> >  \t\t\t\t\t\t     static_cast<int32_t>(ctrl.maximum)));\n> >  }\n> >\n> > -/**\n> > - * \\class V4L2ControlInfoMap\n> > - * \\brief A map of controlID to ControlRange for V4L2 controls\n> > - */\n> > -\n> > -/**\n> > - * \\brief Move assignment operator from a ControlInfoMap\n> > - * \\param[in] info The control info map\n> > - *\n> > - * Populate the map by replacing its contents with those of \\a info using move\n> > - * semantics. Upon return the \\a info map will be empty.\n> > - *\n> > - * This is the only supported way to populate a V4L2ControlInfoMap.\n> > - *\n> > - * \\return The populated V4L2ControlInfoMap\n> > - */\n> > -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)\n> > -{\n> > -\tControlInfoMap::operator=(std::move(info));\n> > -\n> > -\tidmap_.clear();\n> > -\tfor (const auto &ctrl : *this)\n> > -\t\tidmap_[ctrl.first->id()] = ctrl.first;\n> > -\n> > -\treturn *this;\n> > -}\n> > -\n> > -/**\n> > - * \\brief Access specified element by numerical ID\n> > - * \\param[in] id The numerical ID\n> > - * \\return A reference to the element whose ID is equal to \\a id\n> > - */\n> > -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)\n> > -{\n> > -\treturn at(idmap_.at(id));\n> > -}\n> > -\n> > -/**\n> > - * \\brief Access specified element by numerical ID\n> > - * \\param[in] id The numerical ID\n> > - * \\return A const reference to the element whose ID is equal to \\a id\n> > - */\n> > -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const\n> > -{\n> > -\treturn at(idmap_.at(id));\n> > -}\n> > -\n> > -/**\n> > - * \\brief Count the number of elements matching a numerical ID\n> > - * \\param[in] id The numerical ID\n> > - * \\return The number of elements matching the numerical \\a id\n> > - */\n> > -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const\n> > -{\n> > -\treturn count(idmap_.at(id));\n> > -}\n> > -\n> > -/**\n> > - * \\brief Find the element matching a numerical ID\n> > - * \\param[in] id The numerical ID\n> > - * \\return An iterator pointing to the element matching the numerical \\a id, or\n> > - * end() if no such element exists\n> > - */\n> > -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)\n> > -{\n> > -\treturn find(idmap_.at(id));\n> > -}\n> > -\n> > -/**\n> > - * \\brief Find the element matching a numerical ID\n> > - * \\param[in] id The numerical ID\n> > - * \\return A const iterator pointing to the element matching the numerical\n> > - * \\a id, or end() if no such element exists\n> > - */\n> > -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const\n> > -{\n> > -\treturn find(idmap_.at(id));\n> > -}\n> > -\n> > -/**\n> > - * \\fn const ControlIdMap &V4L2ControlInfoMap::idmap() const\n> > - * \\brief Retrieve the ControlId map\n> > - *\n> > - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap\n> > - * for the V4L2 device that the control list targets. This helper method\n> > - * returns a suitable idmap for that purpose.\n> > - *\n> > - * \\return The ControlId map\n> > - */\n> > -\n> >  /**\n> >   * \\class V4L2ControlList\n> >   * \\brief A list of controls for a V4L2 device\n> >   *\n> >   * This class specialises the ControList class for use with V4L2 devices. It\n> > - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.\n> > + * offers a convenience API to create a ControlList from a ControlInfoMap.\n> >   *\n> >   * V4L2ControlList allows easy construction of a ControlList containing V4L2\n> >   * controls for a device. It can be used to construct the list of controls\n> > @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con\n> >   */\n> >\n> >  /**\n> > - * \\fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)\n> > + * \\fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)\n> >   * \\brief Construct a V4L2ControlList associated with a V4L2 device\n> >   * \\param[in] info The V4L2 device control info map\n> >   */\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 06155eb51c03..a2b0d891b12f 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> >   */\n> >  void V4L2Device::listControls()\n> >  {\n> > -\tControlInfoMap ctrls;\n> > +\tControlInfoMap::Map ctrls;\n> >  \tstruct v4l2_query_ext_ctrl ctrl = {};\n> >\n> >  \t/* \\todo Add support for menu and compound controls. */\n> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> > index 182228f3a5b1..31879b794ed0 100644\n> > --- a/test/v4l2_videodevice/controls.cpp\n> > +++ b/test/v4l2_videodevice/controls.cpp\n> > @@ -26,7 +26,7 @@ public:\n> >  protected:\n> >  \tint run()\n> >  \t{\n> > -\t\tconst V4L2ControlInfoMap &info = capture_->controls();\n> > +\t\tconst ControlInfoMap &info = capture_->controls();\n> \n> Looks good, thanks\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> >\n> >  \t\t/* Test control enumeration. */\n> >  \t\tif (info.empty()) {","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 E75BF61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Oct 2019 21:29:07 +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 57430324;\n\tTue, 15 Oct 2019 21:29:07 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1571167747;\n\tbh=0GiSNDEzjDYYvkGL0R2jPD6CRUAmLySwShGjE3jDQmQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HuPkTGNKPmYqrB2bZ8l2Ce5oVc0000yp+HuXhujwG/b7zPxiaoEAuNkycHhoQ2xc1\n\t0mfTrg93bW1UMWnUGNylHfjO1ZijFQv6PTetvANSWshr8MNXXDVx62ksXNbeT1aKBx\n\tGaSK3DMEENanV7fTW1DG/PwQC9SLEmdBF9aFwr8s=","Date":"Tue, 15 Oct 2019 22:29:04 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191015192904.GA19403@pendragon.ideasonboard.com>","References":"<20191013232755.3292-1-laurent.pinchart@ideasonboard.com>\n\t<20191013232755.3292-10-laurent.pinchart@ideasonboard.com>\n\t<20191015184450.v4ygtimldxfkomie@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191015184450.v4ygtimldxfkomie@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 09/10] libcamera: controls: Merge\n\tControlInfoMap and V4L2ControlInfoMap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Oct 2019 19:29:08 -0000"}}]