[libcamera-devel,09/10] libcamera: controls: Merge ControlInfoMap and V4L2ControlInfoMap

2191 diff mbox series

The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
the latter adding convenience accessors based on numerical IDs for the
former, as well as a cached idmap. Both features can be useful for
ControlInfoMap in the context of serialisation, and merging the two
classes will further simplify the IPA API.

Import all the features of V4L2ControlInfoMap into ControlInfoMap,
turning the latter into a real class. A few new constructors and
assignment operators are added for completeness.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/ipa/ipa_interface.h              |   2 +-
 include/libcamera/controls.h             |  45 +++++++-
 src/ipa/ipa_vimc.cpp                     |   2 +-
 src/ipa/rkisp1/rkisp1.cpp                |   6 +-
 src/libcamera/camera_sensor.cpp          |   2 +-
 src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
 src/libcamera/include/camera_sensor.h    |   4 +-
 src/libcamera/include/v4l2_controls.h    |  36 +-----
 src/libcamera/include/v4l2_device.h      |   4 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
 src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
 src/libcamera/pipeline/vimc.cpp          |  11 +-
 src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
 src/libcamera/v4l2_controls.cpp          |  94 +--------------
 src/libcamera/v4l2_device.cpp            |   2 +-
 test/v4l2_videodevice/controls.cpp       |   2 +-
 16 files changed, 220 insertions(+), 154 deletions(-)

Hi Laurent,

Thanks for your work.

On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:
> The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> the latter adding convenience accessors based on numerical IDs for the
> former, as well as a cached idmap. Both features can be useful for
> ControlInfoMap in the context of serialisation, and merging the two
> classes will further simplify the IPA API.
> 
> Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> turning the latter into a real class. A few new constructors and
> assignment operators are added for completeness.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The change to pipeline handler implementations to shorten the line 
length could be done in a separate patch ;-) With out without that 
addressed,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/ipa/ipa_interface.h              |   2 +-
>  include/libcamera/controls.h             |  45 +++++++-
>  src/ipa/ipa_vimc.cpp                     |   2 +-
>  src/ipa/rkisp1/rkisp1.cpp                |   6 +-
>  src/libcamera/camera_sensor.cpp          |   2 +-
>  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
>  src/libcamera/include/camera_sensor.h    |   4 +-
>  src/libcamera/include/v4l2_controls.h    |  36 +-----
>  src/libcamera/include/v4l2_device.h      |   4 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
>  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
>  src/libcamera/pipeline/vimc.cpp          |  11 +-
>  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
>  src/libcamera/v4l2_controls.cpp          |  94 +--------------
>  src/libcamera/v4l2_device.cpp            |   2 +-
>  test/v4l2_videodevice/controls.cpp       |   2 +-
>  16 files changed, 220 insertions(+), 154 deletions(-)
> 
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index dfb1bcbee516..8fd658af5fdb 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -43,7 +43,7 @@ public:
>  	virtual int init() = 0;
>  
>  	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;
> +			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
>  
>  	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
>  	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 5534a2edb567..80414c6f0748 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -118,7 +118,50 @@ private:
>  };
>  
>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> +
> +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> +{
> +public:
> +	using Map = std::unordered_map<const ControlId *, ControlRange>;
> +
> +	ControlInfoMap() = default;
> +	ControlInfoMap(const ControlInfoMap &other) = default;
> +	ControlInfoMap(std::initializer_list<Map::value_type> init);
> +
> +	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> +	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> +	ControlInfoMap &operator=(Map &&info);
> +
> +	using Map::key_type;
> +	using Map::mapped_type;
> +	using Map::value_type;
> +	using Map::size_type;
> +	using Map::iterator;
> +	using Map::const_iterator;
> +
> +	using Map::begin;
> +	using Map::cbegin;
> +	using Map::end;
> +	using Map::cend;
> +	using Map::at;
> +	using Map::empty;
> +	using Map::size;
> +	using Map::count;
> +	using Map::find;
> +
> +	mapped_type &at(unsigned int key);
> +	const mapped_type &at(unsigned int key) const;
> +	size_type count(unsigned int key) const;
> +	iterator find(unsigned int key);
> +	const_iterator find(unsigned int key) const;
> +
> +	const ControlIdMap &idmap() const { return idmap_; }
> +
> +private:
> +	void generateIdmap();
> +
> +	ControlIdMap idmap_;
> +};
>  
>  class ControlList
>  {
> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> index 620dc25f456e..63d578b4e2aa 100644
> --- a/src/ipa/ipa_vimc.cpp
> +++ b/src/ipa/ipa_vimc.cpp
> @@ -31,7 +31,7 @@ public:
>  
>  	int init() override;
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d64c334cbf0c..bd703898c523 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -33,7 +33,7 @@ public:
>  	int init() override { return 0; }
>  
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  	void processEvent(const IPAOperationData &event) override;
> @@ -49,7 +49,7 @@ private:
>  
>  	std::map<unsigned int, BufferMemory> bufferInfo_;
>  
> -	V4L2ControlInfoMap ctrls_;
> +	ControlInfoMap ctrls_;
>  
>  	/* Camera sensor controls. */
>  	bool autoExposure_;
> @@ -62,7 +62,7 @@ private:
>  };
>  
>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)
> +			  const std::map<unsigned int, ControlInfoMap> &entityControls)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 1b8e8c0e07da..86f0c772861b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
>   * \brief Retrieve the supported V4L2 controls and their information
>   * \return A map of the V4L2 controls supported by the sensor
>   */
> -const V4L2ControlInfoMap &CameraSensor::controls() const
> +const ControlInfoMap &CameraSensor::controls() const
>  {
>  	return subdev_->controls();
>  }
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 6a0301f3a2ae..bf7634aab470 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -393,10 +393,147 @@ std::string ControlRange::toString() const
>   */
>  
>  /**
> - * \typedef ControlInfoMap
> + * \class ControlInfoMap
>   * \brief A map of ControlId to ControlRange
> + *
> + * The ControlInfoMap class describes controls supported by an object as an
> + * unsorted map of ControlId pointers to ControlRange instances. Unlike the
> + * standard std::unsorted_map<> class, it is designed the be immutable once
> + * constructed, and thus only exposes the read accessors of the
> + * std::unsorted_map<> base class.
> + *
> + * In addition to the features of the standard unsorted map, this class also
> + * provides access to the mapped elements using numerical ID keys. It maintains
> + * an internal map of numerical ID to ControlId for this purpose, and exposes it
> + * through the idmap() method to help construction of ControlList instances.
>   */
>  
> +/**
> + * \typedef ControlInfoMap::Map
> + * \brief The base std::unsorted_map<> container
> + */
> +
> +/**
> + * \fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)
> + * \brief Copy constructor, construct a ControlInfoMap from a copy of \a other
> + * \param[in] other The other ControlInfoMap
> + */
> +
> +/**
> + * \brief Construct a ControlInfoMap from an initializer list
> + * \param[in] init The initializer list
> + */
> +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
> +	: Map(init)
> +{
> +	generateIdmap();
> +}
> +
> +/**
> + * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
> + * \brief Copy assignment operator, replace the contents with a copy of \a other
> + * \param[in] other The other ControlInfoMap
> + * \return A reference to the ControlInfoMap
> + */
> +
> +/**
> + * \brief Replace the contents with those from the initializer list
> + * \param[in] init The initializer list
> + * \return A reference to the ControlInfoMap
> + */
> +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)
> +{
> +	Map::operator=(init);
> +	generateIdmap();
> +	return *this;
> +}
> +
> +/**
> + * \brief Move assignment operator from a plain map
> + * \param[in] info The control info plain map
> + *
> + * Populate the map by replacing its contents with those of \a info using move
> + * semantics. Upon return the \a info map will be empty.
> + *
> + * \return A reference to the populated ControlInfoMap
> + */
> +ControlInfoMap &ControlInfoMap::operator=(Map &&info)
> +{
> +	Map::operator=(std::move(info));
> +	generateIdmap();
> +	return *this;
> +}
> +
> +/**
> + * \brief Access specified element by numerical ID
> + * \param[in] id The numerical ID
> + * \return A reference to the element whose ID is equal to \a id
> + */
> +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> +{
> +	return at(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Access specified element by numerical ID
> + * \param[in] id The numerical ID
> + * \return A const reference to the element whose ID is equal to \a id
> + */
> +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> +{
> +	return at(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Count the number of elements matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return The number of elements matching the numerical \a id
> + */
> +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> +{
> +	return count(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Find the element matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return An iterator pointing to the element matching the numerical \a id, or
> + * end() if no such element exists
> + */
> +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> +{
> +	return find(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Find the element matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return A const iterator pointing to the element matching the numerical
> + * \a id, or end() if no such element exists
> + */
> +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> +{
> +	return find(idmap_.at(id));
> +}
> +
> +/**
> + * \fn const ControlIdMap &ControlInfoMap::idmap() const
> + * \brief Retrieve the ControlId map
> + *
> + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> + * for the V4L2 device that the control list targets. This helper method
> + * returns a suitable idmap for that purpose.
> + *
> + * \return The ControlId map
> + */
> +
> +void ControlInfoMap::generateIdmap()
> +{
> +	idmap_.clear();
> +	for (const auto &ctrl : *this)
> +		idmap_[ctrl.first->id()] = ctrl.first;
> +}
> +
>  /**
>   * \class ControlList
>   * \brief Associate a list of ControlId with their values for an object
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index f426e29b84bf..1fb36a4f4951 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -16,9 +16,9 @@
>  
>  namespace libcamera {
>  
> +class ControlInfoMap;
>  class ControlList;
>  class MediaEntity;
> -class V4L2ControlInfoMap;
>  class V4L2Subdevice;
>  
>  struct V4L2SubdeviceFormat;
> @@ -43,7 +43,7 @@ public:
>  				      const Size &size) const;
>  	int setFormat(V4L2SubdeviceFormat *format);
>  
> -	const V4L2ControlInfoMap &controls() const;
> +	const ControlInfoMap &controls() const;
>  	int getControls(ControlList *ctrls);
>  	int setControls(ControlList *ctrls);
>  
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index c427b845d8b4..e16c4957f9d1 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -31,44 +31,10 @@ public:
>  	V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);
>  };
>  
> -class V4L2ControlInfoMap : private ControlInfoMap
> -{
> -public:
> -	V4L2ControlInfoMap &operator=(ControlInfoMap &&info);
> -
> -	using ControlInfoMap::key_type;
> -	using ControlInfoMap::mapped_type;
> -	using ControlInfoMap::value_type;
> -	using ControlInfoMap::size_type;
> -	using ControlInfoMap::iterator;
> -	using ControlInfoMap::const_iterator;
> -
> -	using ControlInfoMap::begin;
> -	using ControlInfoMap::cbegin;
> -	using ControlInfoMap::end;
> -	using ControlInfoMap::cend;
> -	using ControlInfoMap::at;
> -	using ControlInfoMap::empty;
> -	using ControlInfoMap::size;
> -	using ControlInfoMap::count;
> -	using ControlInfoMap::find;
> -
> -	mapped_type &at(unsigned int key);
> -	const mapped_type &at(unsigned int key) const;
> -	size_type count(unsigned int key) const;
> -	iterator find(unsigned int key);
> -	const_iterator find(unsigned int key) const;
> -
> -	const ControlIdMap &idmap() const { return idmap_; }
> -
> -private:
> -	ControlIdMap idmap_;
> -};
> -
>  class V4L2ControlList : public ControlList
>  {
>  public:
> -	V4L2ControlList(const V4L2ControlInfoMap &info)
> +	V4L2ControlList(const ControlInfoMap &info)
>  		: ControlList(info.idmap())
>  	{
>  	}
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index f30b1c2cde34..6bfddefe336c 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -24,7 +24,7 @@ public:
>  	void close();
>  	bool isOpen() const { return fd_ != -1; }
>  
> -	const V4L2ControlInfoMap &controls() const { return controls_; }
> +	const ControlInfoMap &controls() const { return controls_; }
>  
>  	int getControls(ControlList *ctrls);
>  	int setControls(ControlList *ctrls);
> @@ -49,7 +49,7 @@ private:
>  			    unsigned int count);
>  
>  	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
> -	V4L2ControlInfoMap controls_;
> +	ControlInfoMap controls_;
>  	std::string deviceNode_;
>  	int fd_;
>  };
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 9b19bde8a274..7a28b03b8d38 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		.size = data->stream_.configuration().size,
>  	};
>  
> -	std::map<unsigned int, V4L2ControlInfoMap> entityControls;
> +	std::map<unsigned int, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>  
>  	data->ipa_->configure(streamConfig, entityControls);
> @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	std::unique_ptr<RkISP1CameraData> data =
>  		utils::make_unique<RkISP1CameraData>(this);
>  
> -	data->controlInfo_.emplace(std::piecewise_construct,
> -				   std::forward_as_tuple(&controls::AeEnable),
> -				   std::forward_as_tuple(false, true));
> +	ControlInfoMap::Map ctrls;
> +	ctrls.emplace(std::piecewise_construct,
> +		      std::forward_as_tuple(&controls::AeEnable),
> +		      std::forward_as_tuple(false, true));
> +
> +	data->controlInfo_ = std::move(ctrls);
>  
>  	data->sensor_ = new CameraSensor(sensor);
>  	ret = data->sensor_->init();
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 018c7691d263..a64e1af4c550 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)
>  	video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
>  
>  	/* Initialise the supported controls. */
> -	const V4L2ControlInfoMap &controls = video_->controls();
> +	const ControlInfoMap &controls = video_->controls();
> +	ControlInfoMap::Map ctrls;
> +
>  	for (const auto &ctrl : controls) {
>  		const ControlRange &range = ctrl.second;
>  		const ControlId *id;
> @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)
>  			continue;
>  		}
>  
> -		controlInfo_.emplace(std::piecewise_construct,
> -				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(range));
> +		ctrls.emplace(std::piecewise_construct,
> +			      std::forward_as_tuple(id),
> +			      std::forward_as_tuple(range));
>  	}
>  
> +	controlInfo_ = std::move(ctrls);
> +
>  	return 0;
>  }
>  
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index f7f82edc6530..6a4244119dc5 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)
>  		return -ENODEV;
>  
>  	/* Initialise the supported controls. */
> -	const V4L2ControlInfoMap &controls = sensor_->controls();
> +	const ControlInfoMap &controls = sensor_->controls();
> +	ControlInfoMap::Map ctrls;
> +
>  	for (const auto &ctrl : controls) {
>  		const ControlRange &range = ctrl.second;
>  		const ControlId *id;
> @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)
>  			continue;
>  		}
>  
> -		controlInfo_.emplace(std::piecewise_construct,
> -				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(range));
> +		ctrls.emplace(std::piecewise_construct,
> +			      std::forward_as_tuple(id),
> +			      std::forward_as_tuple(range));
>  	}
>  
> +	controlInfo_ = std::move(ctrls);
>  	return 0;
>  }
>  
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index 41bd965f0284..4e6fa6899e07 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -28,7 +28,7 @@ public:
>  
>  	int init() override { return 0; }
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index e783457caf94..bd5e18590b76 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)
>  						     static_cast<int32_t>(ctrl.maximum)));
>  }
>  
> -/**
> - * \class V4L2ControlInfoMap
> - * \brief A map of controlID to ControlRange for V4L2 controls
> - */
> -
> -/**
> - * \brief Move assignment operator from a ControlInfoMap
> - * \param[in] info The control info map
> - *
> - * Populate the map by replacing its contents with those of \a info using move
> - * semantics. Upon return the \a info map will be empty.
> - *
> - * This is the only supported way to populate a V4L2ControlInfoMap.
> - *
> - * \return The populated V4L2ControlInfoMap
> - */
> -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)
> -{
> -	ControlInfoMap::operator=(std::move(info));
> -
> -	idmap_.clear();
> -	for (const auto &ctrl : *this)
> -		idmap_[ctrl.first->id()] = ctrl.first;
> -
> -	return *this;
> -}
> -
> -/**
> - * \brief Access specified element by numerical ID
> - * \param[in] id The numerical ID
> - * \return A reference to the element whose ID is equal to \a id
> - */
> -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)
> -{
> -	return at(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Access specified element by numerical ID
> - * \param[in] id The numerical ID
> - * \return A const reference to the element whose ID is equal to \a id
> - */
> -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const
> -{
> -	return at(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Count the number of elements matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return The number of elements matching the numerical \a id
> - */
> -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const
> -{
> -	return count(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Find the element matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return An iterator pointing to the element matching the numerical \a id, or
> - * end() if no such element exists
> - */
> -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)
> -{
> -	return find(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Find the element matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return A const iterator pointing to the element matching the numerical
> - * \a id, or end() if no such element exists
> - */
> -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const
> -{
> -	return find(idmap_.at(id));
> -}
> -
> -/**
> - * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const
> - * \brief Retrieve the ControlId map
> - *
> - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> - * for the V4L2 device that the control list targets. This helper method
> - * returns a suitable idmap for that purpose.
> - *
> - * \return The ControlId map
> - */
> -
>  /**
>   * \class V4L2ControlList
>   * \brief A list of controls for a V4L2 device
>   *
>   * This class specialises the ControList class for use with V4L2 devices. It
> - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.
> + * offers a convenience API to create a ControlList from a ControlInfoMap.
>   *
>   * V4L2ControlList allows easy construction of a ControlList containing V4L2
>   * controls for a device. It can be used to construct the list of controls
> @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con
>   */
>  
>  /**
> - * \fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)
> + * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)
>   * \brief Construct a V4L2ControlList associated with a V4L2 device
>   * \param[in] info The V4L2 device control info map
>   */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 06155eb51c03..a2b0d891b12f 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   */
>  void V4L2Device::listControls()
>  {
> -	ControlInfoMap ctrls;
> +	ControlInfoMap::Map ctrls;
>  	struct v4l2_query_ext_ctrl ctrl = {};
>  
>  	/* \todo Add support for menu and compound controls. */
> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> index 182228f3a5b1..31879b794ed0 100644
> --- a/test/v4l2_videodevice/controls.cpp
> +++ b/test/v4l2_videodevice/controls.cpp
> @@ -26,7 +26,7 @@ public:
>  protected:
>  	int run()
>  	{
> -		const V4L2ControlInfoMap &info = capture_->controls();
> +		const ControlInfoMap &info = capture_->controls();
>  
>  		/* Test control enumeration. */
>  		if (info.empty()) {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Niklas,

On Tue, Oct 15, 2019 at 02:40:49AM +0200, Niklas Söderlund wrote:
> On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:
> > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> > the latter adding convenience accessors based on numerical IDs for the
> > former, as well as a cached idmap. Both features can be useful for
> > ControlInfoMap in the context of serialisation, and merging the two
> > classes will further simplify the IPA API.
> > 
> > Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> > turning the latter into a real class. A few new constructors and
> > assignment operators are added for completeness.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> The change to pipeline handler implementations to shorten the line 
> length could be done in a separate patch ;-) With out without that 
> addressed,

They're not there to shorten the line length, they actually change the
behaviour.

> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> > ---
> >  include/ipa/ipa_interface.h              |   2 +-
> >  include/libcamera/controls.h             |  45 +++++++-
> >  src/ipa/ipa_vimc.cpp                     |   2 +-
> >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-
> >  src/libcamera/camera_sensor.cpp          |   2 +-
> >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
> >  src/libcamera/include/camera_sensor.h    |   4 +-
> >  src/libcamera/include/v4l2_controls.h    |  36 +-----
> >  src/libcamera/include/v4l2_device.h      |   4 +-
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
> >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
> >  src/libcamera/pipeline/vimc.cpp          |  11 +-
> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
> >  src/libcamera/v4l2_controls.cpp          |  94 +--------------
> >  src/libcamera/v4l2_device.cpp            |   2 +-
> >  test/v4l2_videodevice/controls.cpp       |   2 +-
> >  16 files changed, 220 insertions(+), 154 deletions(-)

[snip]

> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 5534a2edb567..80414c6f0748 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -118,7 +118,50 @@ private:
> >  };
> >  
> >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> > +
> > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> > +{
> > +public:
> > +	using Map = std::unordered_map<const ControlId *, ControlRange>;
> > +
> > +	ControlInfoMap() = default;
> > +	ControlInfoMap(const ControlInfoMap &other) = default;
> > +	ControlInfoMap(std::initializer_list<Map::value_type> init);
> > +
> > +	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> > +	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> > +	ControlInfoMap &operator=(Map &&info);
> > +
> > +	using Map::key_type;
> > +	using Map::mapped_type;
> > +	using Map::value_type;
> > +	using Map::size_type;
> > +	using Map::iterator;
> > +	using Map::const_iterator;
> > +
> > +	using Map::begin;
> > +	using Map::cbegin;
> > +	using Map::end;
> > +	using Map::cend;
> > +	using Map::at;
> > +	using Map::empty;
> > +	using Map::size;
> > +	using Map::count;
> > +	using Map::find;
> > +
> > +	mapped_type &at(unsigned int key);
> > +	const mapped_type &at(unsigned int key) const;
> > +	size_type count(unsigned int key) const;
> > +	iterator find(unsigned int key);
> > +	const_iterator find(unsigned int key) const;
> > +
> > +	const ControlIdMap &idmap() const { return idmap_; }
> > +
> > +private:
> > +	void generateIdmap();
> > +
> > +	ControlIdMap idmap_;
> > +};

[snip]

> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 9b19bde8a274..7a28b03b8d38 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp

[snip]

> > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	std::unique_ptr<RkISP1CameraData> data =
> >  		utils::make_unique<RkISP1CameraData>(this);
> >  
> > -	data->controlInfo_.emplace(std::piecewise_construct,
> > -				   std::forward_as_tuple(&controls::AeEnable),
> > -				   std::forward_as_tuple(false, true));
> > +	ControlInfoMap::Map ctrls;
> > +	ctrls.emplace(std::piecewise_construct,
> > +		      std::forward_as_tuple(&controls::AeEnable),
> > +		      std::forward_as_tuple(false, true));
> > +
> > +	data->controlInfo_ = std::move(ctrls);

ctrls is a ControlInfoMap::Map, and data->controlInfo_ is a
ControlInfoMap. The latter is constructed from the former using the
ControlInfoMap::operator=(Map &&info) assignment operator. This is
needed as ControlInfoMap doesn't provide accessors that allow modifying
its contents.

> >  
> >  	data->sensor_ = new CameraSensor(sensor);
> >  	ret = data->sensor_->init();

Hi Laurent,

On 2019-10-15 16:54:55 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> On Tue, Oct 15, 2019 at 02:40:49AM +0200, Niklas Söderlund wrote:
> > On 2019-10-14 02:27:55 +0300, Laurent Pinchart wrote:
> > > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> > > the latter adding convenience accessors based on numerical IDs for the
> > > former, as well as a cached idmap. Both features can be useful for
> > > ControlInfoMap in the context of serialisation, and merging the two
> > > classes will further simplify the IPA API.
> > > 
> > > Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> > > turning the latter into a real class. A few new constructors and
> > > assignment operators are added for completeness.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > The change to pipeline handler implementations to shorten the line 
> > length could be done in a separate patch ;-) With out without that 
> > addressed,
> 
> They're not there to shorten the line length, they actually change the
> behaviour.
> 
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > 
> > > ---
> > >  include/ipa/ipa_interface.h              |   2 +-
> > >  include/libcamera/controls.h             |  45 +++++++-
> > >  src/ipa/ipa_vimc.cpp                     |   2 +-
> > >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-
> > >  src/libcamera/camera_sensor.cpp          |   2 +-
> > >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
> > >  src/libcamera/include/camera_sensor.h    |   4 +-
> > >  src/libcamera/include/v4l2_controls.h    |  36 +-----
> > >  src/libcamera/include/v4l2_device.h      |   4 +-
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
> > >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
> > >  src/libcamera/pipeline/vimc.cpp          |  11 +-
> > >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
> > >  src/libcamera/v4l2_controls.cpp          |  94 +--------------
> > >  src/libcamera/v4l2_device.cpp            |   2 +-
> > >  test/v4l2_videodevice/controls.cpp       |   2 +-
> > >  16 files changed, 220 insertions(+), 154 deletions(-)
> 
> [snip]
> 
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index 5534a2edb567..80414c6f0748 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -118,7 +118,50 @@ private:
> > >  };
> > >  
> > >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> > > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> > > +
> > > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> > > +{
> > > +public:
> > > +	using Map = std::unordered_map<const ControlId *, ControlRange>;
> > > +
> > > +	ControlInfoMap() = default;
> > > +	ControlInfoMap(const ControlInfoMap &other) = default;
> > > +	ControlInfoMap(std::initializer_list<Map::value_type> init);
> > > +
> > > +	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> > > +	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> > > +	ControlInfoMap &operator=(Map &&info);
> > > +
> > > +	using Map::key_type;
> > > +	using Map::mapped_type;
> > > +	using Map::value_type;
> > > +	using Map::size_type;
> > > +	using Map::iterator;
> > > +	using Map::const_iterator;
> > > +
> > > +	using Map::begin;
> > > +	using Map::cbegin;
> > > +	using Map::end;
> > > +	using Map::cend;
> > > +	using Map::at;
> > > +	using Map::empty;
> > > +	using Map::size;
> > > +	using Map::count;
> > > +	using Map::find;
> > > +
> > > +	mapped_type &at(unsigned int key);
> > > +	const mapped_type &at(unsigned int key) const;
> > > +	size_type count(unsigned int key) const;
> > > +	iterator find(unsigned int key);
> > > +	const_iterator find(unsigned int key) const;
> > > +
> > > +	const ControlIdMap &idmap() const { return idmap_; }
> > > +
> > > +private:
> > > +	void generateIdmap();
> > > +
> > > +	ControlIdMap idmap_;
> > > +};
> 
> [snip]
> 
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 9b19bde8a274..7a28b03b8d38 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> 
> [snip]
> 
> > > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> > >  	std::unique_ptr<RkISP1CameraData> data =
> > >  		utils::make_unique<RkISP1CameraData>(this);
> > >  
> > > -	data->controlInfo_.emplace(std::piecewise_construct,
> > > -				   std::forward_as_tuple(&controls::AeEnable),
> > > -				   std::forward_as_tuple(false, true));
> > > +	ControlInfoMap::Map ctrls;
> > > +	ctrls.emplace(std::piecewise_construct,
> > > +		      std::forward_as_tuple(&controls::AeEnable),
> > > +		      std::forward_as_tuple(false, true));
> > > +
> > > +	data->controlInfo_ = std::move(ctrls);
> 
> ctrls is a ControlInfoMap::Map, and data->controlInfo_ is a
> ControlInfoMap. The latter is constructed from the former using the
> ControlInfoMap::operator=(Map &&info) assignment operator. This is
> needed as ControlInfoMap doesn't provide accessors that allow modifying
> its contents.

My bad I missed the ::Map part, thanks for clarifying.

> 
> > >  
> > >  	data->sensor_ = new CameraSensor(sensor);
> > >  	ret = data->sensor_->init();
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Hi Laurent,

On Mon, Oct 14, 2019 at 02:27:55AM +0300, Laurent Pinchart wrote:
> The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> the latter adding convenience accessors based on numerical IDs for the
> former, as well as a cached idmap. Both features can be useful for
> ControlInfoMap in the context of serialisation, and merging the two
> classes will further simplify the IPA API.
>
> Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> turning the latter into a real class. A few new constructors and
> assignment operators are added for completeness.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/ipa/ipa_interface.h              |   2 +-
>  include/libcamera/controls.h             |  45 +++++++-
>  src/ipa/ipa_vimc.cpp                     |   2 +-
>  src/ipa/rkisp1/rkisp1.cpp                |   6 +-
>  src/libcamera/camera_sensor.cpp          |   2 +-
>  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
>  src/libcamera/include/camera_sensor.h    |   4 +-
>  src/libcamera/include/v4l2_controls.h    |  36 +-----
>  src/libcamera/include/v4l2_device.h      |   4 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
>  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
>  src/libcamera/pipeline/vimc.cpp          |  11 +-
>  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
>  src/libcamera/v4l2_controls.cpp          |  94 +--------------
>  src/libcamera/v4l2_device.cpp            |   2 +-
>  test/v4l2_videodevice/controls.cpp       |   2 +-
>  16 files changed, 220 insertions(+), 154 deletions(-)
>
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index dfb1bcbee516..8fd658af5fdb 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -43,7 +43,7 @@ public:
>  	virtual int init() = 0;
>
>  	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;
> +			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
>
>  	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
>  	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 5534a2edb567..80414c6f0748 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -118,7 +118,50 @@ private:
>  };
>
>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> +
> +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> +{
> +public:
> +	using Map = std::unordered_map<const ControlId *, ControlRange>;
> +
> +	ControlInfoMap() = default;
> +	ControlInfoMap(const ControlInfoMap &other) = default;
> +	ControlInfoMap(std::initializer_list<Map::value_type> init);
> +
> +	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> +	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> +	ControlInfoMap &operator=(Map &&info);
> +
> +	using Map::key_type;
> +	using Map::mapped_type;
> +	using Map::value_type;
> +	using Map::size_type;
> +	using Map::iterator;
> +	using Map::const_iterator;
> +
> +	using Map::begin;
> +	using Map::cbegin;
> +	using Map::end;
> +	using Map::cend;
> +	using Map::at;
> +	using Map::empty;
> +	using Map::size;
> +	using Map::count;
> +	using Map::find;
> +
> +	mapped_type &at(unsigned int key);
> +	const mapped_type &at(unsigned int key) const;
> +	size_type count(unsigned int key) const;
> +	iterator find(unsigned int key);
> +	const_iterator find(unsigned int key) const;
> +
> +	const ControlIdMap &idmap() const { return idmap_; }
> +
> +private:
> +	void generateIdmap();
> +
> +	ControlIdMap idmap_;
> +};
>
>  class ControlList
>  {
> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> index 620dc25f456e..63d578b4e2aa 100644
> --- a/src/ipa/ipa_vimc.cpp
> +++ b/src/ipa/ipa_vimc.cpp
> @@ -31,7 +31,7 @@ public:
>
>  	int init() override;
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d64c334cbf0c..bd703898c523 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -33,7 +33,7 @@ public:
>  	int init() override { return 0; }
>
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  	void processEvent(const IPAOperationData &event) override;
> @@ -49,7 +49,7 @@ private:
>
>  	std::map<unsigned int, BufferMemory> bufferInfo_;
>
> -	V4L2ControlInfoMap ctrls_;
> +	ControlInfoMap ctrls_;
>
>  	/* Camera sensor controls. */
>  	bool autoExposure_;
> @@ -62,7 +62,7 @@ private:
>  };
>
>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)
> +			  const std::map<unsigned int, ControlInfoMap> &entityControls)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 1b8e8c0e07da..86f0c772861b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
>   * \brief Retrieve the supported V4L2 controls and their information
>   * \return A map of the V4L2 controls supported by the sensor
>   */
> -const V4L2ControlInfoMap &CameraSensor::controls() const
> +const ControlInfoMap &CameraSensor::controls() const
>  {
>  	return subdev_->controls();
>  }
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 6a0301f3a2ae..bf7634aab470 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -393,10 +393,147 @@ std::string ControlRange::toString() const
>   */
>
>  /**
> - * \typedef ControlInfoMap
> + * \class ControlInfoMap
>   * \brief A map of ControlId to ControlRange
> + *
> + * The ControlInfoMap class describes controls supported by an object as an
> + * unsorted map of ControlId pointers to ControlRange instances. Unlike the
> + * standard std::unsorted_map<> class, it is designed the be immutable once
> + * constructed, and thus only exposes the read accessors of the
> + * std::unsorted_map<> base class.
> + *
> + * In addition to the features of the standard unsorted map, this class also
> + * provides access to the mapped elements using numerical ID keys. It maintains
> + * an internal map of numerical ID to ControlId for this purpose, and exposes it
> + * through the idmap() method to help construction of ControlList instances.

Just a late thoughts on this... the ControlInfoMap generated idmap is
only used for constructing ControlList of v4l2 controls, while for
libcamera controls we have a global id map. This creates a little
asymmetry which is not nice.

To enforce this concept I wonder if we should not ask users of
ControlList of v4l2 controls to explicitly pass the idmap generated
from the ControlInfoMap, by deleting
	ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr);

Or either, make this one the only available constructor, so also
ControlList of libcamera controls cannot be created with the global
controls:controls map, but only from the ControlInfoMap of a Camera.

>   */
>
> +/**
> + * \typedef ControlInfoMap::Map
> + * \brief The base std::unsorted_map<> container
> + */
> +
> +/**
> + * \fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)
> + * \brief Copy constructor, construct a ControlInfoMap from a copy of \a other
> + * \param[in] other The other ControlInfoMap
> + */
> +
> +/**
> + * \brief Construct a ControlInfoMap from an initializer list
> + * \param[in] init The initializer list
> + */
> +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
> +	: Map(init)
> +{
> +	generateIdmap();
> +}
> +
> +/**
> + * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
> + * \brief Copy assignment operator, replace the contents with a copy of \a other

content or contents ?

> + * \param[in] other The other ControlInfoMap
> + * \return A reference to the ControlInfoMap
> + */
> +
> +/**
> + * \brief Replace the contents with those from the initializer list
> + * \param[in] init The initializer list
> + * \return A reference to the ControlInfoMap
> + */
> +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)
> +{
> +	Map::operator=(init);
> +	generateIdmap();
> +	return *this;
> +}
> +
> +/**
> + * \brief Move assignment operator from a plain map
> + * \param[in] info The control info plain map
> + *
> + * Populate the map by replacing its contents with those of \a info using move

This second controls makes me think it was intentional in first place

> + * semantics. Upon return the \a info map will be empty.

but this one 'semantics' should probably be singular

> + *
> + * \return A reference to the populated ControlInfoMap
> + */
> +ControlInfoMap &ControlInfoMap::operator=(Map &&info)
> +{
> +	Map::operator=(std::move(info));
> +	generateIdmap();
> +	return *this;
> +}
> +
> +/**
> + * \brief Access specified element by numerical ID
> + * \param[in] id The numerical ID
> + * \return A reference to the element whose ID is equal to \a id
> + */
> +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> +{
> +	return at(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Access specified element by numerical ID
> + * \param[in] id The numerical ID
> + * \return A const reference to the element whose ID is equal to \a id
> + */
> +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> +{
> +	return at(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Count the number of elements matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return The number of elements matching the numerical \a id
> + */
> +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> +{
> +	return count(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Find the element matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return An iterator pointing to the element matching the numerical \a id, or
> + * end() if no such element exists
> + */
> +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> +{
> +	return find(idmap_.at(id));
> +}
> +
> +/**
> + * \brief Find the element matching a numerical ID
> + * \param[in] id The numerical ID
> + * \return A const iterator pointing to the element matching the numerical
> + * \a id, or end() if no such element exists
> + */
> +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> +{
> +	return find(idmap_.at(id));
> +}
> +
> +/**
> + * \fn const ControlIdMap &ControlInfoMap::idmap() const
> + * \brief Retrieve the ControlId map
> + *
> + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> + * for the V4L2 device that the control list targets. This helper method
> + * returns a suitable idmap for that purpose.
> + *
> + * \return The ControlId map
> + */
> +
> +void ControlInfoMap::generateIdmap()
> +{
> +	idmap_.clear();
> +	for (const auto &ctrl : *this)
> +		idmap_[ctrl.first->id()] = ctrl.first;
> +}
> +
>  /**
>   * \class ControlList
>   * \brief Associate a list of ControlId with their values for an object
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index f426e29b84bf..1fb36a4f4951 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -16,9 +16,9 @@
>
>  namespace libcamera {
>
> +class ControlInfoMap;
>  class ControlList;
>  class MediaEntity;
> -class V4L2ControlInfoMap;
>  class V4L2Subdevice;
>
>  struct V4L2SubdeviceFormat;
> @@ -43,7 +43,7 @@ public:
>  				      const Size &size) const;
>  	int setFormat(V4L2SubdeviceFormat *format);
>
> -	const V4L2ControlInfoMap &controls() const;
> +	const ControlInfoMap &controls() const;
>  	int getControls(ControlList *ctrls);
>  	int setControls(ControlList *ctrls);
>
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index c427b845d8b4..e16c4957f9d1 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -31,44 +31,10 @@ public:
>  	V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);
>  };
>
> -class V4L2ControlInfoMap : private ControlInfoMap
> -{
> -public:
> -	V4L2ControlInfoMap &operator=(ControlInfoMap &&info);
> -
> -	using ControlInfoMap::key_type;
> -	using ControlInfoMap::mapped_type;
> -	using ControlInfoMap::value_type;
> -	using ControlInfoMap::size_type;
> -	using ControlInfoMap::iterator;
> -	using ControlInfoMap::const_iterator;
> -
> -	using ControlInfoMap::begin;
> -	using ControlInfoMap::cbegin;
> -	using ControlInfoMap::end;
> -	using ControlInfoMap::cend;
> -	using ControlInfoMap::at;
> -	using ControlInfoMap::empty;
> -	using ControlInfoMap::size;
> -	using ControlInfoMap::count;
> -	using ControlInfoMap::find;
> -
> -	mapped_type &at(unsigned int key);
> -	const mapped_type &at(unsigned int key) const;
> -	size_type count(unsigned int key) const;
> -	iterator find(unsigned int key);
> -	const_iterator find(unsigned int key) const;
> -
> -	const ControlIdMap &idmap() const { return idmap_; }
> -
> -private:
> -	ControlIdMap idmap_;
> -};
> -
>  class V4L2ControlList : public ControlList
>  {
>  public:
> -	V4L2ControlList(const V4L2ControlInfoMap &info)
> +	V4L2ControlList(const ControlInfoMap &info)
>  		: ControlList(info.idmap())
>  	{
>  	}
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index f30b1c2cde34..6bfddefe336c 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -24,7 +24,7 @@ public:
>  	void close();
>  	bool isOpen() const { return fd_ != -1; }
>
> -	const V4L2ControlInfoMap &controls() const { return controls_; }
> +	const ControlInfoMap &controls() const { return controls_; }
>
>  	int getControls(ControlList *ctrls);
>  	int setControls(ControlList *ctrls);
> @@ -49,7 +49,7 @@ private:
>  			    unsigned int count);
>
>  	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
> -	V4L2ControlInfoMap controls_;
> +	ControlInfoMap controls_;
>  	std::string deviceNode_;
>  	int fd_;
>  };
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 9b19bde8a274..7a28b03b8d38 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		.size = data->stream_.configuration().size,
>  	};
>
> -	std::map<unsigned int, V4L2ControlInfoMap> entityControls;
> +	std::map<unsigned int, ControlInfoMap> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>
>  	data->ipa_->configure(streamConfig, entityControls);
> @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	std::unique_ptr<RkISP1CameraData> data =
>  		utils::make_unique<RkISP1CameraData>(this);
>
> -	data->controlInfo_.emplace(std::piecewise_construct,
> -				   std::forward_as_tuple(&controls::AeEnable),
> -				   std::forward_as_tuple(false, true));
> +	ControlInfoMap::Map ctrls;
> +	ctrls.emplace(std::piecewise_construct,
> +		      std::forward_as_tuple(&controls::AeEnable),
> +		      std::forward_as_tuple(false, true));
> +
> +	data->controlInfo_ = std::move(ctrls);
>
>  	data->sensor_ = new CameraSensor(sensor);
>  	ret = data->sensor_->init();
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 018c7691d263..a64e1af4c550 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)
>  	video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
>
>  	/* Initialise the supported controls. */
> -	const V4L2ControlInfoMap &controls = video_->controls();
> +	const ControlInfoMap &controls = video_->controls();
> +	ControlInfoMap::Map ctrls;
> +
>  	for (const auto &ctrl : controls) {
>  		const ControlRange &range = ctrl.second;
>  		const ControlId *id;
> @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)
>  			continue;
>  		}
>
> -		controlInfo_.emplace(std::piecewise_construct,
> -				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(range));
> +		ctrls.emplace(std::piecewise_construct,
> +			      std::forward_as_tuple(id),
> +			      std::forward_as_tuple(range));
>  	}
>
> +	controlInfo_ = std::move(ctrls);
> +
>  	return 0;
>  }
>
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index f7f82edc6530..6a4244119dc5 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)
>  		return -ENODEV;
>
>  	/* Initialise the supported controls. */
> -	const V4L2ControlInfoMap &controls = sensor_->controls();
> +	const ControlInfoMap &controls = sensor_->controls();
> +	ControlInfoMap::Map ctrls;
> +
>  	for (const auto &ctrl : controls) {
>  		const ControlRange &range = ctrl.second;
>  		const ControlId *id;
> @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)
>  			continue;
>  		}
>
> -		controlInfo_.emplace(std::piecewise_construct,
> -				     std::forward_as_tuple(id),
> -				     std::forward_as_tuple(range));
> +		ctrls.emplace(std::piecewise_construct,
> +			      std::forward_as_tuple(id),
> +			      std::forward_as_tuple(range));
>  	}
>
> +	controlInfo_ = std::move(ctrls);
>  	return 0;
>  }
>
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index 41bd965f0284..4e6fa6899e07 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -28,7 +28,7 @@ public:
>
>  	int init() override { return 0; }
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index e783457caf94..bd5e18590b76 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)
>  						     static_cast<int32_t>(ctrl.maximum)));
>  }
>
> -/**
> - * \class V4L2ControlInfoMap
> - * \brief A map of controlID to ControlRange for V4L2 controls
> - */
> -
> -/**
> - * \brief Move assignment operator from a ControlInfoMap
> - * \param[in] info The control info map
> - *
> - * Populate the map by replacing its contents with those of \a info using move
> - * semantics. Upon return the \a info map will be empty.
> - *
> - * This is the only supported way to populate a V4L2ControlInfoMap.
> - *
> - * \return The populated V4L2ControlInfoMap
> - */
> -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)
> -{
> -	ControlInfoMap::operator=(std::move(info));
> -
> -	idmap_.clear();
> -	for (const auto &ctrl : *this)
> -		idmap_[ctrl.first->id()] = ctrl.first;
> -
> -	return *this;
> -}
> -
> -/**
> - * \brief Access specified element by numerical ID
> - * \param[in] id The numerical ID
> - * \return A reference to the element whose ID is equal to \a id
> - */
> -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)
> -{
> -	return at(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Access specified element by numerical ID
> - * \param[in] id The numerical ID
> - * \return A const reference to the element whose ID is equal to \a id
> - */
> -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const
> -{
> -	return at(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Count the number of elements matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return The number of elements matching the numerical \a id
> - */
> -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const
> -{
> -	return count(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Find the element matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return An iterator pointing to the element matching the numerical \a id, or
> - * end() if no such element exists
> - */
> -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)
> -{
> -	return find(idmap_.at(id));
> -}
> -
> -/**
> - * \brief Find the element matching a numerical ID
> - * \param[in] id The numerical ID
> - * \return A const iterator pointing to the element matching the numerical
> - * \a id, or end() if no such element exists
> - */
> -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const
> -{
> -	return find(idmap_.at(id));
> -}
> -
> -/**
> - * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const
> - * \brief Retrieve the ControlId map
> - *
> - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> - * for the V4L2 device that the control list targets. This helper method
> - * returns a suitable idmap for that purpose.
> - *
> - * \return The ControlId map
> - */
> -
>  /**
>   * \class V4L2ControlList
>   * \brief A list of controls for a V4L2 device
>   *
>   * This class specialises the ControList class for use with V4L2 devices. It
> - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.
> + * offers a convenience API to create a ControlList from a ControlInfoMap.
>   *
>   * V4L2ControlList allows easy construction of a ControlList containing V4L2
>   * controls for a device. It can be used to construct the list of controls
> @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con
>   */
>
>  /**
> - * \fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)
> + * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)
>   * \brief Construct a V4L2ControlList associated with a V4L2 device
>   * \param[in] info The V4L2 device control info map
>   */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 06155eb51c03..a2b0d891b12f 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   */
>  void V4L2Device::listControls()
>  {
> -	ControlInfoMap ctrls;
> +	ControlInfoMap::Map ctrls;
>  	struct v4l2_query_ext_ctrl ctrl = {};
>
>  	/* \todo Add support for menu and compound controls. */
> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> index 182228f3a5b1..31879b794ed0 100644
> --- a/test/v4l2_videodevice/controls.cpp
> +++ b/test/v4l2_videodevice/controls.cpp
> @@ -26,7 +26,7 @@ public:
>  protected:
>  	int run()
>  	{
> -		const V4L2ControlInfoMap &info = capture_->controls();
> +		const ControlInfoMap &info = capture_->controls();

Looks good, thanks
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j
>
>  		/* Test control enumeration. */
>  		if (info.empty()) {
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

On Tue, Oct 15, 2019 at 08:44:50PM +0200, Jacopo Mondi wrote:
> On Mon, Oct 14, 2019 at 02:27:55AM +0300, Laurent Pinchart wrote:
> > The ControlInfoMap and V4L2ControlInfoMap classes are very similar, with
> > the latter adding convenience accessors based on numerical IDs for the
> > former, as well as a cached idmap. Both features can be useful for
> > ControlInfoMap in the context of serialisation, and merging the two
> > classes will further simplify the IPA API.
> >
> > Import all the features of V4L2ControlInfoMap into ControlInfoMap,
> > turning the latter into a real class. A few new constructors and
> > assignment operators are added for completeness.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/ipa/ipa_interface.h              |   2 +-
> >  include/libcamera/controls.h             |  45 +++++++-
> >  src/ipa/ipa_vimc.cpp                     |   2 +-
> >  src/ipa/rkisp1/rkisp1.cpp                |   6 +-
> >  src/libcamera/camera_sensor.cpp          |   2 +-
> >  src/libcamera/controls.cpp               | 139 ++++++++++++++++++++++-
> >  src/libcamera/include/camera_sensor.h    |   4 +-
> >  src/libcamera/include/v4l2_controls.h    |  36 +-----
> >  src/libcamera/include/v4l2_device.h      |   4 +-
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  11 +-
> >  src/libcamera/pipeline/uvcvideo.cpp      |  12 +-
> >  src/libcamera/pipeline/vimc.cpp          |  11 +-
> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |   2 +-
> >  src/libcamera/v4l2_controls.cpp          |  94 +--------------
> >  src/libcamera/v4l2_device.cpp            |   2 +-
> >  test/v4l2_videodevice/controls.cpp       |   2 +-
> >  16 files changed, 220 insertions(+), 154 deletions(-)
> >
> > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> > index dfb1bcbee516..8fd658af5fdb 100644
> > --- a/include/ipa/ipa_interface.h
> > +++ b/include/ipa/ipa_interface.h
> > @@ -43,7 +43,7 @@ public:
> >  	virtual int init() = 0;
> >
> >  	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > -			       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) = 0;
> > +			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
> >
> >  	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
> >  	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 5534a2edb567..80414c6f0748 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -118,7 +118,50 @@ private:
> >  };
> >
> >  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> > -using ControlInfoMap = std::unordered_map<const ControlId *, ControlRange>;
> > +
> > +class ControlInfoMap : private std::unordered_map<const ControlId *, ControlRange>
> > +{
> > +public:
> > +	using Map = std::unordered_map<const ControlId *, ControlRange>;
> > +
> > +	ControlInfoMap() = default;
> > +	ControlInfoMap(const ControlInfoMap &other) = default;
> > +	ControlInfoMap(std::initializer_list<Map::value_type> init);
> > +
> > +	ControlInfoMap &operator=(const ControlInfoMap &other) = default;
> > +	ControlInfoMap &operator=(std::initializer_list<Map::value_type> init);
> > +	ControlInfoMap &operator=(Map &&info);
> > +
> > +	using Map::key_type;
> > +	using Map::mapped_type;
> > +	using Map::value_type;
> > +	using Map::size_type;
> > +	using Map::iterator;
> > +	using Map::const_iterator;
> > +
> > +	using Map::begin;
> > +	using Map::cbegin;
> > +	using Map::end;
> > +	using Map::cend;
> > +	using Map::at;
> > +	using Map::empty;
> > +	using Map::size;
> > +	using Map::count;
> > +	using Map::find;
> > +
> > +	mapped_type &at(unsigned int key);
> > +	const mapped_type &at(unsigned int key) const;
> > +	size_type count(unsigned int key) const;
> > +	iterator find(unsigned int key);
> > +	const_iterator find(unsigned int key) const;
> > +
> > +	const ControlIdMap &idmap() const { return idmap_; }
> > +
> > +private:
> > +	void generateIdmap();
> > +
> > +	ControlIdMap idmap_;
> > +};
> >
> >  class ControlList
> >  {
> > diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> > index 620dc25f456e..63d578b4e2aa 100644
> > --- a/src/ipa/ipa_vimc.cpp
> > +++ b/src/ipa/ipa_vimc.cpp
> > @@ -31,7 +31,7 @@ public:
> >
> >  	int init() override;
> >  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> > +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> >  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> >  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> >  	void processEvent(const IPAOperationData &event) override {}
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index d64c334cbf0c..bd703898c523 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -33,7 +33,7 @@ public:
> >  	int init() override { return 0; }
> >
> >  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override;
> > +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
> >  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> >  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
> >  	void processEvent(const IPAOperationData &event) override;
> > @@ -49,7 +49,7 @@ private:
> >
> >  	std::map<unsigned int, BufferMemory> bufferInfo_;
> >
> > -	V4L2ControlInfoMap ctrls_;
> > +	ControlInfoMap ctrls_;
> >
> >  	/* Camera sensor controls. */
> >  	bool autoExposure_;
> > @@ -62,7 +62,7 @@ private:
> >  };
> >
> >  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > -			  const std::map<unsigned int, V4L2ControlInfoMap> &entityControls)
> > +			  const std::map<unsigned int, ControlInfoMap> &entityControls)
> >  {
> >  	if (entityControls.empty())
> >  		return;
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 1b8e8c0e07da..86f0c772861b 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -251,7 +251,7 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
> >   * \brief Retrieve the supported V4L2 controls and their information
> >   * \return A map of the V4L2 controls supported by the sensor
> >   */
> > -const V4L2ControlInfoMap &CameraSensor::controls() const
> > +const ControlInfoMap &CameraSensor::controls() const
> >  {
> >  	return subdev_->controls();
> >  }
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 6a0301f3a2ae..bf7634aab470 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -393,10 +393,147 @@ std::string ControlRange::toString() const
> >   */
> >
> >  /**
> > - * \typedef ControlInfoMap
> > + * \class ControlInfoMap
> >   * \brief A map of ControlId to ControlRange
> > + *
> > + * The ControlInfoMap class describes controls supported by an object as an
> > + * unsorted map of ControlId pointers to ControlRange instances. Unlike the
> > + * standard std::unsorted_map<> class, it is designed the be immutable once
> > + * constructed, and thus only exposes the read accessors of the
> > + * std::unsorted_map<> base class.
> > + *
> > + * In addition to the features of the standard unsorted map, this class also
> > + * provides access to the mapped elements using numerical ID keys. It maintains
> > + * an internal map of numerical ID to ControlId for this purpose, and exposes it
> > + * through the idmap() method to help construction of ControlList instances.
> 
> Just a late thoughts on this... the ControlInfoMap generated idmap is
> only used for constructing ControlList of v4l2 controls, while for
> libcamera controls we have a global id map. This creates a little
> asymmetry which is not nice.
> 
> To enforce this concept I wonder if we should not ask users of
> ControlList of v4l2 controls to explicitly pass the idmap generated
> from the ControlInfoMap, by deleting
> 	ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr);
> 
> Or either, make this one the only available constructor, so also
> ControlList of libcamera controls cannot be created with the global
> controls:controls map, but only from the ControlInfoMap of a Camera.

I've thought about this too, and I prefer the latter. I wasn't sure
about the practical implications though, so decided to not include that
change in this series. Would you like to give it a try ? :-)

> >   */
> >
> > +/**
> > + * \typedef ControlInfoMap::Map
> > + * \brief The base std::unsorted_map<> container
> > + */
> > +
> > +/**
> > + * \fn ControlInfoMap::ControlInfoMap(const ControlInfoMap &other)
> > + * \brief Copy constructor, construct a ControlInfoMap from a copy of \a other
> > + * \param[in] other The other ControlInfoMap
> > + */
> > +
> > +/**
> > + * \brief Construct a ControlInfoMap from an initializer list
> > + * \param[in] init The initializer list
> > + */
> > +ControlInfoMap::ControlInfoMap(std::initializer_list<Map::value_type> init)
> > +	: Map(init)
> > +{
> > +	generateIdmap();
> > +}
> > +
> > +/**
> > + * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other)
> > + * \brief Copy assignment operator, replace the contents with a copy of \a other
> 
> content or contents ?

It's usually used in plural form according to
https://en.wiktionary.org/wiki/contents.

> > + * \param[in] other The other ControlInfoMap
> > + * \return A reference to the ControlInfoMap
> > + */
> > +
> > +/**
> > + * \brief Replace the contents with those from the initializer list
> > + * \param[in] init The initializer list
> > + * \return A reference to the ControlInfoMap
> > + */
> > +ControlInfoMap &ControlInfoMap::operator=(std::initializer_list<Map::value_type> init)
> > +{
> > +	Map::operator=(init);
> > +	generateIdmap();
> > +	return *this;
> > +}
> > +
> > +/**
> > + * \brief Move assignment operator from a plain map
> > + * \param[in] info The control info plain map
> > + *
> > + * Populate the map by replacing its contents with those of \a info using move
> 
> This second controls makes me think it was intentional in first place

Yes :-)

> > + * semantics. Upon return the \a info map will be empty.
> 
> but this one 'semantics' should probably be singular

https://en.wiktionary.org/wiki/semantics

> > + *
> > + * \return A reference to the populated ControlInfoMap
> > + */
> > +ControlInfoMap &ControlInfoMap::operator=(Map &&info)
> > +{
> > +	Map::operator=(std::move(info));
> > +	generateIdmap();
> > +	return *this;
> > +}
> > +
> > +/**
> > + * \brief Access specified element by numerical ID
> > + * \param[in] id The numerical ID
> > + * \return A reference to the element whose ID is equal to \a id
> > + */
> > +ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id)
> > +{
> > +	return at(idmap_.at(id));
> > +}
> > +
> > +/**
> > + * \brief Access specified element by numerical ID
> > + * \param[in] id The numerical ID
> > + * \return A const reference to the element whose ID is equal to \a id
> > + */
> > +const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const
> > +{
> > +	return at(idmap_.at(id));
> > +}
> > +
> > +/**
> > + * \brief Count the number of elements matching a numerical ID
> > + * \param[in] id The numerical ID
> > + * \return The number of elements matching the numerical \a id
> > + */
> > +ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const
> > +{
> > +	return count(idmap_.at(id));
> > +}
> > +
> > +/**
> > + * \brief Find the element matching a numerical ID
> > + * \param[in] id The numerical ID
> > + * \return An iterator pointing to the element matching the numerical \a id, or
> > + * end() if no such element exists
> > + */
> > +ControlInfoMap::iterator ControlInfoMap::find(unsigned int id)
> > +{
> > +	return find(idmap_.at(id));
> > +}
> > +
> > +/**
> > + * \brief Find the element matching a numerical ID
> > + * \param[in] id The numerical ID
> > + * \return A const iterator pointing to the element matching the numerical
> > + * \a id, or end() if no such element exists
> > + */
> > +ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const
> > +{
> > +	return find(idmap_.at(id));
> > +}
> > +
> > +/**
> > + * \fn const ControlIdMap &ControlInfoMap::idmap() const
> > + * \brief Retrieve the ControlId map
> > + *
> > + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> > + * for the V4L2 device that the control list targets. This helper method
> > + * returns a suitable idmap for that purpose.
> > + *
> > + * \return The ControlId map
> > + */
> > +
> > +void ControlInfoMap::generateIdmap()
> > +{
> > +	idmap_.clear();
> > +	for (const auto &ctrl : *this)
> > +		idmap_[ctrl.first->id()] = ctrl.first;
> > +}
> > +
> >  /**
> >   * \class ControlList
> >   * \brief Associate a list of ControlId with their values for an object
> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> > index f426e29b84bf..1fb36a4f4951 100644
> > --- a/src/libcamera/include/camera_sensor.h
> > +++ b/src/libcamera/include/camera_sensor.h
> > @@ -16,9 +16,9 @@
> >
> >  namespace libcamera {
> >
> > +class ControlInfoMap;
> >  class ControlList;
> >  class MediaEntity;
> > -class V4L2ControlInfoMap;
> >  class V4L2Subdevice;
> >
> >  struct V4L2SubdeviceFormat;
> > @@ -43,7 +43,7 @@ public:
> >  				      const Size &size) const;
> >  	int setFormat(V4L2SubdeviceFormat *format);
> >
> > -	const V4L2ControlInfoMap &controls() const;
> > +	const ControlInfoMap &controls() const;
> >  	int getControls(ControlList *ctrls);
> >  	int setControls(ControlList *ctrls);
> >
> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> > index c427b845d8b4..e16c4957f9d1 100644
> > --- a/src/libcamera/include/v4l2_controls.h
> > +++ b/src/libcamera/include/v4l2_controls.h
> > @@ -31,44 +31,10 @@ public:
> >  	V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl);
> >  };
> >
> > -class V4L2ControlInfoMap : private ControlInfoMap
> > -{
> > -public:
> > -	V4L2ControlInfoMap &operator=(ControlInfoMap &&info);
> > -
> > -	using ControlInfoMap::key_type;
> > -	using ControlInfoMap::mapped_type;
> > -	using ControlInfoMap::value_type;
> > -	using ControlInfoMap::size_type;
> > -	using ControlInfoMap::iterator;
> > -	using ControlInfoMap::const_iterator;
> > -
> > -	using ControlInfoMap::begin;
> > -	using ControlInfoMap::cbegin;
> > -	using ControlInfoMap::end;
> > -	using ControlInfoMap::cend;
> > -	using ControlInfoMap::at;
> > -	using ControlInfoMap::empty;
> > -	using ControlInfoMap::size;
> > -	using ControlInfoMap::count;
> > -	using ControlInfoMap::find;
> > -
> > -	mapped_type &at(unsigned int key);
> > -	const mapped_type &at(unsigned int key) const;
> > -	size_type count(unsigned int key) const;
> > -	iterator find(unsigned int key);
> > -	const_iterator find(unsigned int key) const;
> > -
> > -	const ControlIdMap &idmap() const { return idmap_; }
> > -
> > -private:
> > -	ControlIdMap idmap_;
> > -};
> > -
> >  class V4L2ControlList : public ControlList
> >  {
> >  public:
> > -	V4L2ControlList(const V4L2ControlInfoMap &info)
> > +	V4L2ControlList(const ControlInfoMap &info)
> >  		: ControlList(info.idmap())
> >  	{
> >  	}
> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> > index f30b1c2cde34..6bfddefe336c 100644
> > --- a/src/libcamera/include/v4l2_device.h
> > +++ b/src/libcamera/include/v4l2_device.h
> > @@ -24,7 +24,7 @@ public:
> >  	void close();
> >  	bool isOpen() const { return fd_ != -1; }
> >
> > -	const V4L2ControlInfoMap &controls() const { return controls_; }
> > +	const ControlInfoMap &controls() const { return controls_; }
> >
> >  	int getControls(ControlList *ctrls);
> >  	int setControls(ControlList *ctrls);
> > @@ -49,7 +49,7 @@ private:
> >  			    unsigned int count);
> >
> >  	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
> > -	V4L2ControlInfoMap controls_;
> > +	ControlInfoMap controls_;
> >  	std::string deviceNode_;
> >  	int fd_;
> >  };
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 9b19bde8a274..7a28b03b8d38 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -776,7 +776,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >  		.size = data->stream_.configuration().size,
> >  	};
> >
> > -	std::map<unsigned int, V4L2ControlInfoMap> entityControls;
> > +	std::map<unsigned int, ControlInfoMap> entityControls;
> >  	entityControls.emplace(0, data->sensor_->controls());
> >
> >  	data->ipa_->configure(streamConfig, entityControls);
> > @@ -875,9 +875,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	std::unique_ptr<RkISP1CameraData> data =
> >  		utils::make_unique<RkISP1CameraData>(this);
> >
> > -	data->controlInfo_.emplace(std::piecewise_construct,
> > -				   std::forward_as_tuple(&controls::AeEnable),
> > -				   std::forward_as_tuple(false, true));
> > +	ControlInfoMap::Map ctrls;
> > +	ctrls.emplace(std::piecewise_construct,
> > +		      std::forward_as_tuple(&controls::AeEnable),
> > +		      std::forward_as_tuple(false, true));
> > +
> > +	data->controlInfo_ = std::move(ctrls);
> >
> >  	data->sensor_ = new CameraSensor(sensor);
> >  	ret = data->sensor_->init();
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 018c7691d263..a64e1af4c550 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -335,7 +335,9 @@ int UVCCameraData::init(MediaEntity *entity)
> >  	video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
> >
> >  	/* Initialise the supported controls. */
> > -	const V4L2ControlInfoMap &controls = video_->controls();
> > +	const ControlInfoMap &controls = video_->controls();
> > +	ControlInfoMap::Map ctrls;
> > +
> >  	for (const auto &ctrl : controls) {
> >  		const ControlRange &range = ctrl.second;
> >  		const ControlId *id;
> > @@ -360,11 +362,13 @@ int UVCCameraData::init(MediaEntity *entity)
> >  			continue;
> >  		}
> >
> > -		controlInfo_.emplace(std::piecewise_construct,
> > -				     std::forward_as_tuple(id),
> > -				     std::forward_as_tuple(range));
> > +		ctrls.emplace(std::piecewise_construct,
> > +			      std::forward_as_tuple(id),
> > +			      std::forward_as_tuple(range));
> >  	}
> >
> > +	controlInfo_ = std::move(ctrls);
> > +
> >  	return 0;
> >  }
> >
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index f7f82edc6530..6a4244119dc5 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -411,7 +411,9 @@ int VimcCameraData::init(MediaDevice *media)
> >  		return -ENODEV;
> >
> >  	/* Initialise the supported controls. */
> > -	const V4L2ControlInfoMap &controls = sensor_->controls();
> > +	const ControlInfoMap &controls = sensor_->controls();
> > +	ControlInfoMap::Map ctrls;
> > +
> >  	for (const auto &ctrl : controls) {
> >  		const ControlRange &range = ctrl.second;
> >  		const ControlId *id;
> > @@ -430,11 +432,12 @@ int VimcCameraData::init(MediaDevice *media)
> >  			continue;
> >  		}
> >
> > -		controlInfo_.emplace(std::piecewise_construct,
> > -				     std::forward_as_tuple(id),
> > -				     std::forward_as_tuple(range));
> > +		ctrls.emplace(std::piecewise_construct,
> > +			      std::forward_as_tuple(id),
> > +			      std::forward_as_tuple(range));
> >  	}
> >
> > +	controlInfo_ = std::move(ctrls);
> >  	return 0;
> >  }
> >
> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > index 41bd965f0284..4e6fa6899e07 100644
> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > @@ -28,7 +28,7 @@ public:
> >
> >  	int init() override { return 0; }
> >  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > -		       const std::map<unsigned int, V4L2ControlInfoMap> &entityControls) override {}
> > +		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> >  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> >  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> >  	void processEvent(const IPAOperationData &event) override {}
> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > index e783457caf94..bd5e18590b76 100644
> > --- a/src/libcamera/v4l2_controls.cpp
> > +++ b/src/libcamera/v4l2_controls.cpp
> > @@ -134,102 +134,12 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl)
> >  						     static_cast<int32_t>(ctrl.maximum)));
> >  }
> >
> > -/**
> > - * \class V4L2ControlInfoMap
> > - * \brief A map of controlID to ControlRange for V4L2 controls
> > - */
> > -
> > -/**
> > - * \brief Move assignment operator from a ControlInfoMap
> > - * \param[in] info The control info map
> > - *
> > - * Populate the map by replacing its contents with those of \a info using move
> > - * semantics. Upon return the \a info map will be empty.
> > - *
> > - * This is the only supported way to populate a V4L2ControlInfoMap.
> > - *
> > - * \return The populated V4L2ControlInfoMap
> > - */
> > -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(ControlInfoMap &&info)
> > -{
> > -	ControlInfoMap::operator=(std::move(info));
> > -
> > -	idmap_.clear();
> > -	for (const auto &ctrl : *this)
> > -		idmap_[ctrl.first->id()] = ctrl.first;
> > -
> > -	return *this;
> > -}
> > -
> > -/**
> > - * \brief Access specified element by numerical ID
> > - * \param[in] id The numerical ID
> > - * \return A reference to the element whose ID is equal to \a id
> > - */
> > -V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id)
> > -{
> > -	return at(idmap_.at(id));
> > -}
> > -
> > -/**
> > - * \brief Access specified element by numerical ID
> > - * \param[in] id The numerical ID
> > - * \return A const reference to the element whose ID is equal to \a id
> > - */
> > -const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const
> > -{
> > -	return at(idmap_.at(id));
> > -}
> > -
> > -/**
> > - * \brief Count the number of elements matching a numerical ID
> > - * \param[in] id The numerical ID
> > - * \return The number of elements matching the numerical \a id
> > - */
> > -V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const
> > -{
> > -	return count(idmap_.at(id));
> > -}
> > -
> > -/**
> > - * \brief Find the element matching a numerical ID
> > - * \param[in] id The numerical ID
> > - * \return An iterator pointing to the element matching the numerical \a id, or
> > - * end() if no such element exists
> > - */
> > -V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id)
> > -{
> > -	return find(idmap_.at(id));
> > -}
> > -
> > -/**
> > - * \brief Find the element matching a numerical ID
> > - * \param[in] id The numerical ID
> > - * \return A const iterator pointing to the element matching the numerical
> > - * \a id, or end() if no such element exists
> > - */
> > -V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const
> > -{
> > -	return find(idmap_.at(id));
> > -}
> > -
> > -/**
> > - * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const
> > - * \brief Retrieve the ControlId map
> > - *
> > - * Constructing ControlList instances for V4L2 controls requires a ControlIdMap
> > - * for the V4L2 device that the control list targets. This helper method
> > - * returns a suitable idmap for that purpose.
> > - *
> > - * \return The ControlId map
> > - */
> > -
> >  /**
> >   * \class V4L2ControlList
> >   * \brief A list of controls for a V4L2 device
> >   *
> >   * This class specialises the ControList class for use with V4L2 devices. It
> > - * offers a convenience API to create a ControlList from a V4L2ControlInfoMap.
> > + * offers a convenience API to create a ControlList from a ControlInfoMap.
> >   *
> >   * V4L2ControlList allows easy construction of a ControlList containing V4L2
> >   * controls for a device. It can be used to construct the list of controls
> > @@ -239,7 +149,7 @@ V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) con
> >   */
> >
> >  /**
> > - * \fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)
> > + * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info)
> >   * \brief Construct a V4L2ControlList associated with a V4L2 device
> >   * \param[in] info The V4L2 device control info map
> >   */
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 06155eb51c03..a2b0d891b12f 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -342,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
> >   */
> >  void V4L2Device::listControls()
> >  {
> > -	ControlInfoMap ctrls;
> > +	ControlInfoMap::Map ctrls;
> >  	struct v4l2_query_ext_ctrl ctrl = {};
> >
> >  	/* \todo Add support for menu and compound controls. */
> > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
> > index 182228f3a5b1..31879b794ed0 100644
> > --- a/test/v4l2_videodevice/controls.cpp
> > +++ b/test/v4l2_videodevice/controls.cpp
> > @@ -26,7 +26,7 @@ public:
> >  protected:
> >  	int run()
> >  	{
> > -		const V4L2ControlInfoMap &info = capture_->controls();
> > +		const ControlInfoMap &info = capture_->controls();
> 
> Looks good, thanks
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> >
> >  		/* Test control enumeration. */
> >  		if (info.empty()) {