[{"id":2874,"web_url":"https://patchwork.libcamera.org/comment/2874/","msgid":"<20191013145920.udm5pnvl7fvftl5f@uno.localdomain>","date":"2019-10-13T14:59:20","subject":"Re: [libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device:\n\tReplace V4L2ControlList with ControlList","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Oct 12, 2019 at 09:44:06PM +0300, Laurent Pinchart wrote:\n> The V4L2Device class uses V4L2ControlList as a controls container for\n> the getControls() and setControls() operations. Having a distinct\n> container from ControlList will makes the IPA API more complex, as it\n> needs to explicitly transport both types of lists. This will become even\n> more painful when implementing serialisation and deserialisation.\n>\n> To simplify the IPA API and ease the implementation of serialisation and\n> deserialisation, replace usage of V4L2ControlList with ControlList in\n> the V4L2Device (and thus CameraSensor) API. The V4L2ControlList class\n> becomes a thin wrapper around ControlList that slightly simplifies the\n> creation of control lists for V4L2 controls, and may be removed in the\n> future.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Changes since v1:\n>\n> - Fix typo in comment\n> - Use the v4l2_ext_control::value field instead of value64 where\n>   appropriate\n> - Break loop in updateControls() when reaching count\n> - Rename controls_ to info_ (and related variables)\n> - Add reference to V4L2Device::[gs]etControls() to V4L2ControlList class\n>   documentation\n> - Update the rkisp1 IPA\n> - Add V4L2ControlInfoMap class\n> - Remove ID-based accessors from V4L2ControlList as they are now present\n>   in the base ControlList class\n> - Don't include v4l2_controls.h in camera_sensor.h\n\nWith the v1 comments fixed, this seems all good now\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> ---\n>  src/ipa/rkisp1/rkisp1.cpp             |  18 +--\n>  src/libcamera/camera_sensor.cpp       |   4 +-\n>  src/libcamera/include/camera_sensor.h |   7 +-\n>  src/libcamera/include/v4l2_controls.h |  44 ++----\n>  src/libcamera/include/v4l2_device.h   |   6 +-\n>  src/libcamera/pipeline/ipu3/ipu3.cpp  |   9 +-\n>  src/libcamera/pipeline/uvcvideo.cpp   |  24 ++--\n>  src/libcamera/pipeline/vimc.cpp       |  25 ++--\n>  src/libcamera/v4l2_controls.cpp       | 196 +++-----------------------\n>  src/libcamera/v4l2_device.cpp         |  51 ++++---\n>  test/v4l2_videodevice/controls.cpp    |  32 ++---\n>  11 files changed, 124 insertions(+), 292 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index b0d23dd154be..69ced840585f 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -49,6 +49,8 @@ private:\n>\n>  \tstd::map<unsigned int, BufferMemory> bufferInfo_;\n>\n> +\tV4L2ControlInfoMap ctrls_;\n> +\n>  \t/* Camera sensor controls. */\n>  \tbool autoExposure_;\n>  \tuint32_t exposure_;\n> @@ -65,16 +67,16 @@ void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n>  \tif (entityControls.empty())\n>  \t\treturn;\n>\n> -\tconst V4L2ControlInfoMap &ctrls = entityControls.at(0);\n> +\tctrls_ = entityControls.at(0);\n>\n> -\tconst auto itExp = ctrls.find(V4L2_CID_EXPOSURE);\n> -\tif (itExp == ctrls.end()) {\n> +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> +\tif (itExp == ctrls_.end()) {\n>  \t\tLOG(IPARkISP1, Error) << \"Can't find exposure control\";\n>  \t\treturn;\n>  \t}\n>\n> -\tconst auto itGain = ctrls.find(V4L2_CID_ANALOGUE_GAIN);\n> -\tif (itGain == ctrls.end()) {\n> +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> +\tif (itGain == ctrls_.end()) {\n>  \t\tLOG(IPARkISP1, Error) << \"Can't find gain control\";\n>  \t\treturn;\n>  \t}\n> @@ -210,9 +212,9 @@ void IPARkISP1::setControls(unsigned int frame)\n>  \tIPAOperationData op;\n>  \top.operation = RKISP1_IPA_ACTION_V4L2_SET;\n>\n> -\tV4L2ControlList ctrls;\n> -\tctrls.add(V4L2_CID_EXPOSURE, exposure_);\n> -\tctrls.add(V4L2_CID_ANALOGUE_GAIN, gain_);\n> +\tV4L2ControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n>  \top.v4l2controls.push_back(ctrls);\n>\n>  \tqueueFrameAction.emit(frame, op);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a7670b449b31..9e8b44a23850 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -279,7 +279,7 @@ const V4L2ControlInfoMap &CameraSensor::controls() const\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int CameraSensor::getControls(V4L2ControlList *ctrls)\n> +int CameraSensor::getControls(ControlList *ctrls)\n>  {\n>  \treturn subdev_->getControls(ctrls);\n>  }\n> @@ -309,7 +309,7 @@ int CameraSensor::getControls(V4L2ControlList *ctrls)\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int CameraSensor::setControls(V4L2ControlList *ctrls)\n> +int CameraSensor::setControls(ControlList *ctrls)\n>  {\n>  \treturn subdev_->setControls(ctrls);\n>  }\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index fe033fb374c1..f426e29b84bf 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -13,11 +13,12 @@\n>  #include <libcamera/geometry.h>\n>\n>  #include \"log.h\"\n> -#include \"v4l2_controls.h\"\n>\n>  namespace libcamera {\n>\n> +class ControlList;\n>  class MediaEntity;\n> +class V4L2ControlInfoMap;\n>  class V4L2Subdevice;\n>\n>  struct V4L2SubdeviceFormat;\n> @@ -43,8 +44,8 @@ public:\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>\n>  \tconst V4L2ControlInfoMap &controls() const;\n> -\tint getControls(V4L2ControlList *ctrls);\n> -\tint setControls(V4L2ControlList *ctrls);\n> +\tint getControls(ControlList *ctrls);\n> +\tint setControls(ControlList *ctrls);\n>\n>  protected:\n>  \tstd::string logPrefix() const;\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 8990e755a385..89cc74485e6f 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -13,7 +13,6 @@\n>  #include <string>\n>  #include <vector>\n>\n> -#include <linux/v4l2-controls.h>\n>  #include <linux/videodev2.h>\n>\n>  #include <libcamera/controls.h>\n> @@ -57,47 +56,20 @@ public:\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::size;\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::count;\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::find;\n> +\n> +\tconst ControlIdMap &idmap() const { return idmap_; }\n> +\n> +private:\n> +\tControlIdMap idmap_;\n>  };\n>\n> -class V4L2Control\n> +class V4L2ControlList : public ControlList\n>  {\n>  public:\n> -\tV4L2Control(unsigned int id, const ControlValue &value = ControlValue())\n> -\t\t: id_(id), value_(value)\n> +\tV4L2ControlList(const V4L2ControlInfoMap &info)\n> +\t\t: ControlList(info.idmap())\n>  \t{\n>  \t}\n> -\n> -\tunsigned int id() const { return id_; }\n> -\tconst ControlValue &value() const { return value_; }\n> -\tControlValue &value() { return value_; }\n> -\n> -private:\n> -\tunsigned int id_;\n> -\tControlValue value_;\n> -};\n> -\n> -class V4L2ControlList\n> -{\n> -public:\n> -\tusing iterator = std::vector<V4L2Control>::iterator;\n> -\tusing const_iterator = std::vector<V4L2Control>::const_iterator;\n> -\n> -\titerator begin() { return controls_.begin(); }\n> -\tconst_iterator begin() const { return controls_.begin(); }\n> -\titerator end() { return controls_.end(); }\n> -\tconst_iterator end() const { return controls_.end(); }\n> -\n> -\tbool empty() const { return controls_.empty(); }\n> -\tstd::size_t size() const { return controls_.size(); }\n> -\n> -\tvoid clear() { controls_.clear(); }\n> -\tvoid add(unsigned int id, int64_t value = 0);\n> -\n> -\tV4L2Control *getByIndex(unsigned int index);\n> -\tV4L2Control *operator[](unsigned int id);\n> -\n> -private:\n> -\tstd::vector<V4L2Control> controls_;\n>  };\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 75a52c33d821..daa762d58d2b 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -25,8 +25,8 @@ public:\n>\n>  \tconst V4L2ControlInfoMap &controls() const { return controls_; }\n>\n> -\tint getControls(V4L2ControlList *ctrls);\n> -\tint setControls(V4L2ControlList *ctrls);\n> +\tint getControls(ControlList *ctrls);\n> +\tint setControls(ControlList *ctrls);\n>\n>  \tconst std::string &deviceNode() const { return deviceNode_; }\n>\n> @@ -43,7 +43,7 @@ protected:\n>\n>  private:\n>  \tvoid listControls();\n> -\tvoid updateControls(V4L2ControlList *ctrls,\n> +\tvoid updateControls(ControlList *ctrls,\n>  \t\t\t    const V4L2ControlInfo **controlInfo,\n>  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t    unsigned int count);\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 827906d5cd2e..9776b36b88cd 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -199,6 +199,7 @@ class PipelineHandlerIPU3 : public PipelineHandler\n>  {\n>  public:\n>  \tstatic constexpr unsigned int V4L2_CID_IPU3_PIPE_MODE = 0x009819c1;\n> +\n>  \tenum IPU3PipeModes {\n>  \t\tIPU3PipeModeVideo = 0,\n>  \t\tIPU3PipeModeStillCapture = 1,\n> @@ -603,10 +604,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \t\treturn ret;\n>\n>  \t/* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> -\tV4L2ControlList ctrls;\n> -\tctrls.add(V4L2_CID_IPU3_PIPE_MODE, vfStream->active_ ?\n> -\t\t\t\t\t   IPU3PipeModeVideo :\n> -\t\t\t\t\t   IPU3PipeModeStillCapture);\n> +\tV4L2ControlList ctrls(imgu->imgu_->controls());\n> +\tctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n> +\t\t  static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo :\n> +\t\t\t\t       IPU3PipeModeStillCapture));\n>  \tret = imgu->imgu_->setControls(&ctrls);\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 88f7fb9bc568..547ad5ca4e55 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -228,31 +228,30 @@ void PipelineHandlerUVC::stop(Camera *camera)\n>\n>  int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>  {\n> -\tV4L2ControlList controls;\n> +\tV4L2ControlList controls(data->video_->controls());\n>\n>  \tfor (auto it : request->controls()) {\n>  \t\tconst ControlId &id = *it.first;\n>  \t\tControlValue &value = it.second;\n>\n>  \t\tif (id == controls::Brightness) {\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_BRIGHTNESS, value);\n>  \t\t} else if (id == controls::Contrast) {\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_CONTRAST, value);\n>  \t\t} else if (id == controls::Saturation) {\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_SATURATION, value);\n>  \t\t} else if (id == controls::ManualExposure) {\n> -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_AUTO, 1);\n> -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(1));\n> +\t\t\tcontrols.set(V4L2_CID_EXPOSURE_ABSOLUTE, value);\n>  \t\t} else if (id == controls::ManualGain) {\n> -\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_GAIN, value);\n>  \t\t}\n>  \t}\n>\n> -\tfor (const V4L2Control &ctrl : controls)\n> +\tfor (const auto &ctrl : controls)\n>  \t\tLOG(UVC, Debug)\n> -\t\t\t<< \"Setting control 0x\"\n> -\t\t\t<< std::hex << std::setw(8) << ctrl.id() << std::dec\n> -\t\t\t<< \" to \" << ctrl.value().toString();\n> +\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \" to \" << ctrl.second.toString();\n>\n>  \tint ret = data->video_->setControls(&controls);\n>  \tif (ret) {\n> @@ -338,11 +337,10 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \t/* Initialise the supported controls. */\n>  \tconst V4L2ControlInfoMap &controls = video_->controls();\n>  \tfor (const auto &ctrl : controls) {\n> -\t\tunsigned int v4l2Id = ctrl.first;\n>  \t\tconst V4L2ControlInfo &info = ctrl.second;\n>  \t\tconst ControlId *id;\n>\n> -\t\tswitch (v4l2Id) {\n> +\t\tswitch (info.id().id()) {\n>  \t\tcase V4L2_CID_BRIGHTNESS:\n>  \t\t\tid = &controls::Brightness;\n>  \t\t\tbreak;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f1cfd0ed35cf..53d00360eb6e 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -279,26 +279,24 @@ void PipelineHandlerVimc::stop(Camera *camera)\n>\n>  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>  {\n> -\tV4L2ControlList controls;\n> +\tV4L2ControlList controls(data->sensor_->controls());\n>\n>  \tfor (auto it : request->controls()) {\n>  \t\tconst ControlId &id = *it.first;\n>  \t\tControlValue &value = it.second;\n>\n> -\t\tif (id == controls::Brightness) {\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> -\t\t} else if (id == controls::Contrast) {\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> -\t\t} else if (id == controls::Saturation) {\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> -\t\t}\n> +\t\tif (id == controls::Brightness)\n> +\t\t\tcontrols.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\telse if (id == controls::Contrast)\n> +\t\t\tcontrols.set(V4L2_CID_CONTRAST, value);\n> +\t\telse if (id == controls::Saturation)\n> +\t\t\tcontrols.set(V4L2_CID_SATURATION, value);\n>  \t}\n>\n> -\tfor (const V4L2Control &ctrl : controls)\n> +\tfor (const auto &ctrl : controls)\n>  \t\tLOG(VIMC, Debug)\n> -\t\t\t<< \"Setting control 0x\"\n> -\t\t\t<< std::hex << std::setw(8) << ctrl.id() << std::dec\n> -\t\t\t<< \" to \" << ctrl.value().toString();\n> +\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \" to \" << ctrl.second.toString();\n>\n>  \tint ret = data->sensor_->setControls(&controls);\n>  \tif (ret) {\n> @@ -415,11 +413,10 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t/* Initialise the supported controls. */\n>  \tconst V4L2ControlInfoMap &controls = sensor_->controls();\n>  \tfor (const auto &ctrl : controls) {\n> -\t\tunsigned int v4l2Id = ctrl.first;\n>  \t\tconst V4L2ControlInfo &info = ctrl.second;\n>  \t\tconst ControlId *id;\n>\n> -\t\tswitch (v4l2Id) {\n> +\t\tswitch (info.id().id()) {\n>  \t\tcase V4L2_CID_BRIGHTNESS:\n>  \t\t\tid = &controls::Brightness;\n>  \t\t\tbreak;\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index 438e8d8bdb99..b4bb7e791491 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -40,7 +40,7 @@\n>   * sizes.\n>   *\n>   * The libcamera V4L2 Controls framework operates on lists of controls, wrapped\n> - * by the V4L2ControlList class, to match the V4L2 extended controls API. The\n> + * by the ControlList class, to match the V4L2 extended controls API. The\n>   * interface to set and get control is implemented by the V4L2Device class, and\n>   * this file only provides the data type definitions.\n>   *\n> @@ -172,194 +172,42 @@ V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2Con\n>  {\n>  \tstd::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info));\n>\n> +\tidmap_.clear();\n> +\tfor (const auto &ctrl : *this)\n> +\t\tidmap_[ctrl.first] = &ctrl.second.id();\n> +\n>  \treturn *this;\n>  }\n>\n>  /**\n> - * \\class V4L2Control\n> - * \\brief A V4L2 control value\n> + * \\fn const ControlIdMap &V4L2ControlInfoMap::idmap() const\n> + * \\brief Retrieve the ControlId map\n>   *\n> - * The V4L2Control class represent the value of a V4L2 control. The class\n> - * stores values that have been read from or will be applied to a V4L2 device.\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> - * The value stored in the class instances does not reflect what is actually\n> - * applied to the hardware but is a pure software cache optionally initialized\n> - * at control creation time and modified by a control read or write operation.\n> - *\n> - * The write and read controls the V4L2Control class instances are not meant\n> - * to be directly used but are instead intended to be grouped in\n> - * V4L2ControlList instances, which are then passed as parameters to\n> - * V4L2Device::setControls() and V4L2Device::getControls() operations.\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::V4L2Control\n> - * \\brief Construct a V4L2 control with \\a id and \\a value\n> - * \\param id The V4L2 control ID\n> - * \\param value The control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::value() const\n> - * \\brief Retrieve the value of the control\n> - *\n> - * This method is a const version of V4L2Control::value(), returning a const\n> - * reference to the value.\n> - *\n> - * \\return The V4L2 control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::value()\n> - * \\brief Retrieve the value of the control\n> - *\n> - * This method returns the cached control value, initially set by\n> - * V4L2ControlList::add() and then updated when the controls are read or\n> - * written with V4L2Device::getControls() and V4L2Device::setControls().\n> - *\n> - * \\return The V4L2 control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::id()\n> - * \\brief Retrieve the control ID this instance refers to\n> - * \\return The V4L2Control ID\n> + * \\return The ControlId map\n>   */\n>\n>  /**\n>   * \\class V4L2ControlList\n> - * \\brief Container of V4L2Control instances\n> + * \\brief A list of controls for a V4L2 device\n>   *\n> - * The V4L2ControlList class works as a container for a list of V4L2Control\n> - * instances. The class provides operations to add a new control to the list,\n> - * get back a control value, and reset the list of controls it contains.\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>   *\n> - * In order to set and get controls, user of the libcamera V4L2 control\n> - * framework should operate on instances of the V4L2ControlList class, and use\n> - * them as argument for the V4L2Device::setControls() and\n> - * V4L2Device::getControls() operations, which write and read a list of\n> - * controls to or from a V4L2 device (a video device or a subdevice).\n> - *\n> - * Controls are added to a V4L2ControlList instance with the add() method, with\n> - * or without a value.\n> - *\n> - * To write controls to a device, the controls of interest shall be added with\n> - * an initial value by calling V4L2ControlList::add(unsigned int id, int64_t\n> - * value) to prepare for a write operation. Once the values of all controls of\n> - * interest have been added, the V4L2ControlList instance is passed to the\n> - * V4L2Device::setControls(), which sets the controls on the device.\n> - *\n> - * To read controls from a device, the desired controls are added with\n> - * V4L2ControlList::add(unsigned int id) to prepare for a read operation. The\n> - * V4L2ControlList instance is then passed to V4L2Device::getControls(), which\n> - * reads the controls from the device and updates the values stored in\n> - * V4L2ControlList.\n> - *\n> - * V4L2ControlList instances can be reset to remove all controls they contain\n> - * and prepare to be re-used for a new control write/read sequence.\n> - */\n> -\n> -/**\n> - * \\typedef V4L2ControlList::iterator\n> - * \\brief Iterator on the V4L2 controls contained in the instance\n> - */\n> -\n> -/**\n> - * \\typedef V4L2ControlList::const_iterator\n> - * \\brief Const iterator on the V4L2 controls contained in the instance\n> - */\n> -\n> -/**\n> - * \\fn iterator V4L2ControlList::begin()\n> - * \\brief Retrieve an iterator to the first V4L2Control in the instance\n> - * \\return An iterator to the first V4L2 control\n> - */\n> -\n> -/**\n> - * \\fn const_iterator V4L2ControlList::begin() const\n> - * \\brief Retrieve a constant iterator to the first V4L2Control in the instance\n> - * \\return A constant iterator to the first V4L2 control\n> - */\n> -\n> -/**\n> - * \\fn iterator V4L2ControlList::end()\n> - * \\brief Retrieve an iterator pointing to the past-the-end V4L2Control in the\n> - * instance\n> - * \\return An iterator to the element following the last V4L2 control in the\n> - * instance\n> - */\n> -\n> -/**\n> - * \\fn const_iterator V4L2ControlList::end() const\n> - * \\brief Retrieve a constant iterator pointing to the past-the-end V4L2Control\n> - * in the instance\n> - * \\return A constant iterator to the element following the last V4L2 control\n> - * in the instance\n> - */\n> -\n> -/**\n> - * \\fn V4L2ControlList::empty()\n> - * \\brief Verify if the instance does not contain any control\n> - * \\return True if the instance does not contain any control, false otherwise\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> + * passed to the V4L2Device::getControls() and V4L2Device::setControls()\n> + * methods. The class should however not be used in place of ControlList in\n> + * APIs.\n>   */\n>\n>  /**\n> - * \\fn V4L2ControlList::size()\n> - * \\brief Retrieve the number on controls in the instance\n> - * \\return The number of V4L2Control stored in the instance\n> - */\n> -\n> -/**\n> - * \\fn V4L2ControlList::clear()\n> - * \\brief Remove all controls in the instance\n> - */\n> -\n> -/**\n> - * \\brief Add control with \\a id and optional \\a value to the instance\n> - * \\param id The V4L2 control ID (V4L2_CID_*)\n> - * \\param value The V4L2 control value\n> - *\n> - * This method adds a new V4L2 control to the V4L2ControlList. The newly\n> - * inserted control shall not already be present in the control lists, otherwise\n> - * this method, and further use of the control list lead to undefined behaviour.\n> + * \\fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)\n> + * \\brief Construct a V4L2ControlList associated with a V4L2 device\n> + * \\param[in] info The V4L2 device control info map\n>   */\n> -void V4L2ControlList::add(unsigned int id, int64_t value)\n> -{\n> -\tcontrols_.emplace_back(id, value);\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the control at \\a index\n> - * \\param[in] index The control index\n> - *\n> - * Controls are stored in a V4L2ControlList in order of insertion and this\n> - * method retrieves the control at \\a index.\n> - *\n> - * \\return A pointer to the V4L2Control at \\a index or nullptr if the\n> - * index is larger than the number of controls\n> - */\n> -V4L2Control *V4L2ControlList::getByIndex(unsigned int index)\n> -{\n> -\tif (index >= controls_.size())\n> -\t\treturn nullptr;\n> -\n> -\treturn &controls_[index];\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the control with \\a id\n> - * \\param[in] id The V4L2 control ID (V4L2_CID_xx)\n> - * \\return A pointer to the V4L2Control with \\a id or nullptr if the\n> - * control ID is not part of this instance.\n> - */\n> -V4L2Control *V4L2ControlList::operator[](unsigned int id)\n> -{\n> -\tfor (V4L2Control &ctrl : controls_) {\n> -\t\tif (ctrl.id() == id)\n> -\t\t\treturn &ctrl;\n> -\t}\n> -\n> -\treturn nullptr;\n> -}\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 1f755f0f3ef6..b47ba448f354 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -163,7 +163,7 @@ void V4L2Device::close()\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::getControls(V4L2ControlList *ctrls)\n> +int V4L2Device::getControls(ControlList *ctrls)\n>  {\n>  \tunsigned int count = ctrls->size();\n>  \tif (count == 0)\n> @@ -173,18 +173,21 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>  \tstruct v4l2_ext_control v4l2Ctrls[count];\n>  \tmemset(v4l2Ctrls, 0, sizeof(v4l2Ctrls));\n>\n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> -\t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst auto iter = controls_.find(ctrl->id());\n> +\tunsigned int i = 0;\n> +\tfor (const auto &ctrl : *ctrls) {\n> +\t\tconst ControlId *id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id->id());\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> +\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n>  \t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n> -\t\tv4l2Ctrls[i].id = ctrl->id();\n> +\t\tv4l2Ctrls[i].id = id->id();\n> +\n> +\t\ti++;\n>  \t}\n>\n>  \tstruct v4l2_ext_controls v4l2ExtCtrls = {};\n> @@ -238,7 +241,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::setControls(V4L2ControlList *ctrls)\n> +int V4L2Device::setControls(ControlList *ctrls)\n>  {\n>  \tunsigned int count = ctrls->size();\n>  \tif (count == 0)\n> @@ -248,32 +251,36 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n>  \tstruct v4l2_ext_control v4l2Ctrls[count];\n>  \tmemset(v4l2Ctrls, 0, sizeof(v4l2Ctrls));\n>\n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> -\t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst auto iter = controls_.find(ctrl->id());\n> +\tunsigned int i = 0;\n> +\tfor (const auto &ctrl : *ctrls) {\n> +\t\tconst ControlId *id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id->id());\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> +\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n>  \t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n> -\t\tv4l2Ctrls[i].id = ctrl->id();\n> +\t\tv4l2Ctrls[i].id = id->id();\n>\n>  \t\t/* Set the v4l2_ext_control value for the write operation. */\n> +\t\tconst ControlValue &value = ctrl.second;\n>  \t\tswitch (info->id().type()) {\n>  \t\tcase ControlTypeInteger64:\n> -\t\t\tv4l2Ctrls[i].value64 = ctrl->value().get<int64_t>();\n> +\t\t\tv4l2Ctrls[i].value64 = value.get<int64_t>();\n>  \t\t\tbreak;\n>  \t\tdefault:\n>  \t\t\t/*\n>  \t\t\t * \\todo To be changed when support for string and\n>  \t\t\t * compound controls will be added.\n>  \t\t\t */\n> -\t\t\tv4l2Ctrls[i].value = ctrl->value().get<int32_t>();\n> +\t\t\tv4l2Ctrls[i].value = value.get<int32_t>();\n>  \t\t\tbreak;\n>  \t\t}\n> +\n> +\t\ti++;\n>  \t}\n>\n>  \tstruct v4l2_ext_controls v4l2ExtCtrls = {};\n> @@ -385,28 +392,34 @@ void V4L2Device::listControls()\n>   * \\param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver\n>   * \\param[in] count The number of controls to update\n>   */\n> -void V4L2Device::updateControls(V4L2ControlList *ctrls,\n> +void V4L2Device::updateControls(ControlList *ctrls,\n>  \t\t\t\tconst V4L2ControlInfo **controlInfo,\n>  \t\t\t\tconst struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t\tunsigned int count)\n>  {\n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> +\tunsigned int i = 0;\n> +\tfor (auto &ctrl : *ctrls) {\n> +\t\tif (i == count)\n> +\t\t\tbreak;\n> +\n>  \t\tconst struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i];\n>  \t\tconst V4L2ControlInfo *info = controlInfo[i];\n> -\t\tV4L2Control *ctrl = ctrls->getByIndex(i);\n> +\t\tControlValue &value = ctrl.second;\n>\n>  \t\tswitch (info->id().type()) {\n>  \t\tcase ControlTypeInteger64:\n> -\t\t\tctrl->value().set<int64_t>(v4l2Ctrl->value64);\n> +\t\t\tvalue.set<int64_t>(v4l2Ctrl->value64);\n>  \t\t\tbreak;\n>  \t\tdefault:\n>  \t\t\t/*\n>  \t\t\t * \\todo To be changed when support for string and\n>  \t\t\t * compound controls will be added.\n>  \t\t\t */\n> -\t\t\tctrl->value().set<int32_t>(v4l2Ctrl->value);\n> +\t\t\tvalue.set<int32_t>(v4l2Ctrl->value);\n>  \t\t\tbreak;\n>  \t\t}\n> +\n> +\t\ti++;\n>  \t}\n>  }\n>\n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> index 4a7535245c00..783b695b8cbf 100644\n> --- a/test/v4l2_videodevice/controls.cpp\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -44,10 +44,10 @@ protected:\n>  \t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n>\n>  \t\t/* Test getting controls. */\n> -\t\tV4L2ControlList ctrls;\n> -\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> -\t\tctrls.add(V4L2_CID_CONTRAST);\n> -\t\tctrls.add(V4L2_CID_SATURATION);\n> +\t\tV4L2ControlList ctrls(info);\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, -1);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, -1);\n> +\t\tctrls.set(V4L2_CID_SATURATION, -1);\n>\n>  \t\tint ret = capture_->getControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -55,17 +55,17 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> -\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> -\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n> +\t\tif (ctrls.get(V4L2_CID_BRIGHTNESS).get<int32_t>() == -1 ||\n> +\t\t    ctrls.get(V4L2_CID_CONTRAST).get<int32_t>() == -1 ||\n> +\t\t    ctrls.get(V4L2_CID_SATURATION).get<int32_t>() == -1) {\n>  \t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n>  \t\t/* Test setting controls. */\n> -\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> -\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> -\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min());\n> +\t\tctrls.set(V4L2_CID_CONTRAST, contrast.range().max());\n> +\t\tctrls.set(V4L2_CID_SATURATION, saturation.range().min());\n>\n>  \t\tret = capture_->setControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -74,9 +74,9 @@ protected:\n>  \t\t}\n>\n>  \t\t/* Test setting controls outside of range. */\n> -\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> -\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> -\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min().get<int32_t>() - 1);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, contrast.range().max().get<int32_t>() + 1);\n> +\t\tctrls.set(V4L2_CID_SATURATION, saturation.range().min().get<int32_t>() + 1);\n>\n>  \t\tret = capture_->setControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -84,9 +84,9 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> -\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> -\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> +\t\tif (ctrls.get(V4L2_CID_BRIGHTNESS) != brightness.range().min() ||\n> +\t\t    ctrls.get(V4L2_CID_CONTRAST) != brightness.range().max() ||\n> +\t\t    ctrls.get(V4L2_CID_SATURATION) != saturation.range().min().get<int32_t>() + 1) {\n>  \t\t\tcerr << \"Controls not updated when set\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\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 relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C15961562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 16:57:33 +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 relay4-d.mail.gandi.net (Postfix) with ESMTPSA id DFE21E0003;\n\tSun, 13 Oct 2019 14:57:32 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sun, 13 Oct 2019 16:59:20 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191013145920.udm5pnvl7fvftl5f@uno.localdomain>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-14-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"xpdalbcmpxtbe3hi\"","Content-Disposition":"inline","In-Reply-To":"<20191012184407.31684-14-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device:\n\tReplace V4L2ControlList with ControlList","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":"Sun, 13 Oct 2019 14:57:33 -0000"}},{"id":2891,"web_url":"https://patchwork.libcamera.org/comment/2891/","msgid":"<20191013162812.GV23166@bigcity.dyn.berto.se>","date":"2019-10-13T16:28:12","subject":"Re: [libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device:\n\tReplace V4L2ControlList with ControlList","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-12 21:44:06 +0300, Laurent Pinchart wrote:\n> The V4L2Device class uses V4L2ControlList as a controls container for\n> the getControls() and setControls() operations. Having a distinct\n> container from ControlList will makes the IPA API more complex, as it\n> needs to explicitly transport both types of lists. This will become even\n> more painful when implementing serialisation and deserialisation.\n> \n> To simplify the IPA API and ease the implementation of serialisation and\n> deserialisation, replace usage of V4L2ControlList with ControlList in\n> the V4L2Device (and thus CameraSensor) API. The V4L2ControlList class\n> becomes a thin wrapper around ControlList that slightly simplifies the\n> creation of control lists for V4L2 controls, and may be removed in the\n> future.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n> Changes since v1:\n> \n> - Fix typo in comment\n> - Use the v4l2_ext_control::value field instead of value64 where\n>   appropriate\n> - Break loop in updateControls() when reaching count\n> - Rename controls_ to info_ (and related variables)\n> - Add reference to V4L2Device::[gs]etControls() to V4L2ControlList class\n>   documentation\n> - Update the rkisp1 IPA\n> - Add V4L2ControlInfoMap class\n> - Remove ID-based accessors from V4L2ControlList as they are now present\n>   in the base ControlList class\n> - Don't include v4l2_controls.h in camera_sensor.h\n> ---\n>  src/ipa/rkisp1/rkisp1.cpp             |  18 +--\n>  src/libcamera/camera_sensor.cpp       |   4 +-\n>  src/libcamera/include/camera_sensor.h |   7 +-\n>  src/libcamera/include/v4l2_controls.h |  44 ++----\n>  src/libcamera/include/v4l2_device.h   |   6 +-\n>  src/libcamera/pipeline/ipu3/ipu3.cpp  |   9 +-\n>  src/libcamera/pipeline/uvcvideo.cpp   |  24 ++--\n>  src/libcamera/pipeline/vimc.cpp       |  25 ++--\n>  src/libcamera/v4l2_controls.cpp       | 196 +++-----------------------\n>  src/libcamera/v4l2_device.cpp         |  51 ++++---\n>  test/v4l2_videodevice/controls.cpp    |  32 ++---\n>  11 files changed, 124 insertions(+), 292 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index b0d23dd154be..69ced840585f 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -49,6 +49,8 @@ private:\n>  \n>  \tstd::map<unsigned int, BufferMemory> bufferInfo_;\n>  \n> +\tV4L2ControlInfoMap ctrls_;\n> +\n>  \t/* Camera sensor controls. */\n>  \tbool autoExposure_;\n>  \tuint32_t exposure_;\n> @@ -65,16 +67,16 @@ void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n>  \tif (entityControls.empty())\n>  \t\treturn;\n>  \n> -\tconst V4L2ControlInfoMap &ctrls = entityControls.at(0);\n> +\tctrls_ = entityControls.at(0);\n>  \n> -\tconst auto itExp = ctrls.find(V4L2_CID_EXPOSURE);\n> -\tif (itExp == ctrls.end()) {\n> +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> +\tif (itExp == ctrls_.end()) {\n>  \t\tLOG(IPARkISP1, Error) << \"Can't find exposure control\";\n>  \t\treturn;\n>  \t}\n>  \n> -\tconst auto itGain = ctrls.find(V4L2_CID_ANALOGUE_GAIN);\n> -\tif (itGain == ctrls.end()) {\n> +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> +\tif (itGain == ctrls_.end()) {\n>  \t\tLOG(IPARkISP1, Error) << \"Can't find gain control\";\n>  \t\treturn;\n>  \t}\n> @@ -210,9 +212,9 @@ void IPARkISP1::setControls(unsigned int frame)\n>  \tIPAOperationData op;\n>  \top.operation = RKISP1_IPA_ACTION_V4L2_SET;\n>  \n> -\tV4L2ControlList ctrls;\n> -\tctrls.add(V4L2_CID_EXPOSURE, exposure_);\n> -\tctrls.add(V4L2_CID_ANALOGUE_GAIN, gain_);\n> +\tV4L2ControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n>  \top.v4l2controls.push_back(ctrls);\n>  \n>  \tqueueFrameAction.emit(frame, op);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a7670b449b31..9e8b44a23850 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -279,7 +279,7 @@ const V4L2ControlInfoMap &CameraSensor::controls() const\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int CameraSensor::getControls(V4L2ControlList *ctrls)\n> +int CameraSensor::getControls(ControlList *ctrls)\n>  {\n>  \treturn subdev_->getControls(ctrls);\n>  }\n> @@ -309,7 +309,7 @@ int CameraSensor::getControls(V4L2ControlList *ctrls)\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int CameraSensor::setControls(V4L2ControlList *ctrls)\n> +int CameraSensor::setControls(ControlList *ctrls)\n>  {\n>  \treturn subdev_->setControls(ctrls);\n>  }\n> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\n> index fe033fb374c1..f426e29b84bf 100644\n> --- a/src/libcamera/include/camera_sensor.h\n> +++ b/src/libcamera/include/camera_sensor.h\n> @@ -13,11 +13,12 @@\n>  #include <libcamera/geometry.h>\n>  \n>  #include \"log.h\"\n> -#include \"v4l2_controls.h\"\n>  \n>  namespace libcamera {\n>  \n> +class ControlList;\n>  class MediaEntity;\n> +class V4L2ControlInfoMap;\n>  class V4L2Subdevice;\n>  \n>  struct V4L2SubdeviceFormat;\n> @@ -43,8 +44,8 @@ public:\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n>  \n>  \tconst V4L2ControlInfoMap &controls() const;\n> -\tint getControls(V4L2ControlList *ctrls);\n> -\tint setControls(V4L2ControlList *ctrls);\n> +\tint getControls(ControlList *ctrls);\n> +\tint setControls(ControlList *ctrls);\n>  \n>  protected:\n>  \tstd::string logPrefix() const;\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index 8990e755a385..89cc74485e6f 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -13,7 +13,6 @@\n>  #include <string>\n>  #include <vector>\n>  \n> -#include <linux/v4l2-controls.h>\n>  #include <linux/videodev2.h>\n>  \n>  #include <libcamera/controls.h>\n> @@ -57,47 +56,20 @@ public:\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::size;\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::count;\n>  \tusing std::map<unsigned int, V4L2ControlInfo>::find;\n> +\n> +\tconst ControlIdMap &idmap() const { return idmap_; }\n> +\n> +private:\n> +\tControlIdMap idmap_;\n>  };\n>  \n> -class V4L2Control\n> +class V4L2ControlList : public ControlList\n>  {\n>  public:\n> -\tV4L2Control(unsigned int id, const ControlValue &value = ControlValue())\n> -\t\t: id_(id), value_(value)\n> +\tV4L2ControlList(const V4L2ControlInfoMap &info)\n> +\t\t: ControlList(info.idmap())\n>  \t{\n>  \t}\n> -\n> -\tunsigned int id() const { return id_; }\n> -\tconst ControlValue &value() const { return value_; }\n> -\tControlValue &value() { return value_; }\n> -\n> -private:\n> -\tunsigned int id_;\n> -\tControlValue value_;\n> -};\n> -\n> -class V4L2ControlList\n> -{\n> -public:\n> -\tusing iterator = std::vector<V4L2Control>::iterator;\n> -\tusing const_iterator = std::vector<V4L2Control>::const_iterator;\n> -\n> -\titerator begin() { return controls_.begin(); }\n> -\tconst_iterator begin() const { return controls_.begin(); }\n> -\titerator end() { return controls_.end(); }\n> -\tconst_iterator end() const { return controls_.end(); }\n> -\n> -\tbool empty() const { return controls_.empty(); }\n> -\tstd::size_t size() const { return controls_.size(); }\n> -\n> -\tvoid clear() { controls_.clear(); }\n> -\tvoid add(unsigned int id, int64_t value = 0);\n> -\n> -\tV4L2Control *getByIndex(unsigned int index);\n> -\tV4L2Control *operator[](unsigned int id);\n> -\n> -private:\n> -\tstd::vector<V4L2Control> controls_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 75a52c33d821..daa762d58d2b 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -25,8 +25,8 @@ public:\n>  \n>  \tconst V4L2ControlInfoMap &controls() const { return controls_; }\n>  \n> -\tint getControls(V4L2ControlList *ctrls);\n> -\tint setControls(V4L2ControlList *ctrls);\n> +\tint getControls(ControlList *ctrls);\n> +\tint setControls(ControlList *ctrls);\n>  \n>  \tconst std::string &deviceNode() const { return deviceNode_; }\n>  \n> @@ -43,7 +43,7 @@ protected:\n>  \n>  private:\n>  \tvoid listControls();\n> -\tvoid updateControls(V4L2ControlList *ctrls,\n> +\tvoid updateControls(ControlList *ctrls,\n>  \t\t\t    const V4L2ControlInfo **controlInfo,\n>  \t\t\t    const struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t    unsigned int count);\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 827906d5cd2e..9776b36b88cd 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -199,6 +199,7 @@ class PipelineHandlerIPU3 : public PipelineHandler\n>  {\n>  public:\n>  \tstatic constexpr unsigned int V4L2_CID_IPU3_PIPE_MODE = 0x009819c1;\n> +\n>  \tenum IPU3PipeModes {\n>  \t\tIPU3PipeModeVideo = 0,\n>  \t\tIPU3PipeModeStillCapture = 1,\n> @@ -603,10 +604,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \t\treturn ret;\n>  \n>  \t/* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> -\tV4L2ControlList ctrls;\n> -\tctrls.add(V4L2_CID_IPU3_PIPE_MODE, vfStream->active_ ?\n> -\t\t\t\t\t   IPU3PipeModeVideo :\n> -\t\t\t\t\t   IPU3PipeModeStillCapture);\n> +\tV4L2ControlList ctrls(imgu->imgu_->controls());\n> +\tctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n> +\t\t  static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo :\n> +\t\t\t\t       IPU3PipeModeStillCapture));\n>  \tret = imgu->imgu_->setControls(&ctrls);\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 88f7fb9bc568..547ad5ca4e55 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -228,31 +228,30 @@ void PipelineHandlerUVC::stop(Camera *camera)\n>  \n>  int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>  {\n> -\tV4L2ControlList controls;\n> +\tV4L2ControlList controls(data->video_->controls());\n>  \n>  \tfor (auto it : request->controls()) {\n>  \t\tconst ControlId &id = *it.first;\n>  \t\tControlValue &value = it.second;\n>  \n>  \t\tif (id == controls::Brightness) {\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_BRIGHTNESS, value);\n>  \t\t} else if (id == controls::Contrast) {\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_CONTRAST, value);\n>  \t\t} else if (id == controls::Saturation) {\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_SATURATION, value);\n>  \t\t} else if (id == controls::ManualExposure) {\n> -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_AUTO, 1);\n> -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(1));\n> +\t\t\tcontrols.set(V4L2_CID_EXPOSURE_ABSOLUTE, value);\n>  \t\t} else if (id == controls::ManualGain) {\n> -\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int32_t>());\n> +\t\t\tcontrols.set(V4L2_CID_GAIN, value);\n>  \t\t}\n>  \t}\n>  \n> -\tfor (const V4L2Control &ctrl : controls)\n> +\tfor (const auto &ctrl : controls)\n>  \t\tLOG(UVC, Debug)\n> -\t\t\t<< \"Setting control 0x\"\n> -\t\t\t<< std::hex << std::setw(8) << ctrl.id() << std::dec\n> -\t\t\t<< \" to \" << ctrl.value().toString();\n> +\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \" to \" << ctrl.second.toString();\n>  \n>  \tint ret = data->video_->setControls(&controls);\n>  \tif (ret) {\n> @@ -338,11 +337,10 @@ int UVCCameraData::init(MediaEntity *entity)\n>  \t/* Initialise the supported controls. */\n>  \tconst V4L2ControlInfoMap &controls = video_->controls();\n>  \tfor (const auto &ctrl : controls) {\n> -\t\tunsigned int v4l2Id = ctrl.first;\n>  \t\tconst V4L2ControlInfo &info = ctrl.second;\n>  \t\tconst ControlId *id;\n>  \n> -\t\tswitch (v4l2Id) {\n> +\t\tswitch (info.id().id()) {\n>  \t\tcase V4L2_CID_BRIGHTNESS:\n>  \t\t\tid = &controls::Brightness;\n>  \t\t\tbreak;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f1cfd0ed35cf..53d00360eb6e 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -279,26 +279,24 @@ void PipelineHandlerVimc::stop(Camera *camera)\n>  \n>  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>  {\n> -\tV4L2ControlList controls;\n> +\tV4L2ControlList controls(data->sensor_->controls());\n>  \n>  \tfor (auto it : request->controls()) {\n>  \t\tconst ControlId &id = *it.first;\n>  \t\tControlValue &value = it.second;\n>  \n> -\t\tif (id == controls::Brightness) {\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> -\t\t} else if (id == controls::Contrast) {\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> -\t\t} else if (id == controls::Saturation) {\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> -\t\t}\n> +\t\tif (id == controls::Brightness)\n> +\t\t\tcontrols.set(V4L2_CID_BRIGHTNESS, value);\n> +\t\telse if (id == controls::Contrast)\n> +\t\t\tcontrols.set(V4L2_CID_CONTRAST, value);\n> +\t\telse if (id == controls::Saturation)\n> +\t\t\tcontrols.set(V4L2_CID_SATURATION, value);\n>  \t}\n>  \n> -\tfor (const V4L2Control &ctrl : controls)\n> +\tfor (const auto &ctrl : controls)\n>  \t\tLOG(VIMC, Debug)\n> -\t\t\t<< \"Setting control 0x\"\n> -\t\t\t<< std::hex << std::setw(8) << ctrl.id() << std::dec\n> -\t\t\t<< \" to \" << ctrl.value().toString();\n> +\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \" to \" << ctrl.second.toString();\n>  \n>  \tint ret = data->sensor_->setControls(&controls);\n>  \tif (ret) {\n> @@ -415,11 +413,10 @@ int VimcCameraData::init(MediaDevice *media)\n>  \t/* Initialise the supported controls. */\n>  \tconst V4L2ControlInfoMap &controls = sensor_->controls();\n>  \tfor (const auto &ctrl : controls) {\n> -\t\tunsigned int v4l2Id = ctrl.first;\n>  \t\tconst V4L2ControlInfo &info = ctrl.second;\n>  \t\tconst ControlId *id;\n>  \n> -\t\tswitch (v4l2Id) {\n> +\t\tswitch (info.id().id()) {\n>  \t\tcase V4L2_CID_BRIGHTNESS:\n>  \t\t\tid = &controls::Brightness;\n>  \t\t\tbreak;\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index 438e8d8bdb99..b4bb7e791491 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -40,7 +40,7 @@\n>   * sizes.\n>   *\n>   * The libcamera V4L2 Controls framework operates on lists of controls, wrapped\n> - * by the V4L2ControlList class, to match the V4L2 extended controls API. The\n> + * by the ControlList class, to match the V4L2 extended controls API. The\n>   * interface to set and get control is implemented by the V4L2Device class, and\n>   * this file only provides the data type definitions.\n>   *\n> @@ -172,194 +172,42 @@ V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2Con\n>  {\n>  \tstd::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info));\n>  \n> +\tidmap_.clear();\n> +\tfor (const auto &ctrl : *this)\n> +\t\tidmap_[ctrl.first] = &ctrl.second.id();\n> +\n>  \treturn *this;\n>  }\n>  \n>  /**\n> - * \\class V4L2Control\n> - * \\brief A V4L2 control value\n> + * \\fn const ControlIdMap &V4L2ControlInfoMap::idmap() const\n> + * \\brief Retrieve the ControlId map\n>   *\n> - * The V4L2Control class represent the value of a V4L2 control. The class\n> - * stores values that have been read from or will be applied to a V4L2 device.\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> - * The value stored in the class instances does not reflect what is actually\n> - * applied to the hardware but is a pure software cache optionally initialized\n> - * at control creation time and modified by a control read or write operation.\n> - *\n> - * The write and read controls the V4L2Control class instances are not meant\n> - * to be directly used but are instead intended to be grouped in\n> - * V4L2ControlList instances, which are then passed as parameters to\n> - * V4L2Device::setControls() and V4L2Device::getControls() operations.\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::V4L2Control\n> - * \\brief Construct a V4L2 control with \\a id and \\a value\n> - * \\param id The V4L2 control ID\n> - * \\param value The control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::value() const\n> - * \\brief Retrieve the value of the control\n> - *\n> - * This method is a const version of V4L2Control::value(), returning a const\n> - * reference to the value.\n> - *\n> - * \\return The V4L2 control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::value()\n> - * \\brief Retrieve the value of the control\n> - *\n> - * This method returns the cached control value, initially set by\n> - * V4L2ControlList::add() and then updated when the controls are read or\n> - * written with V4L2Device::getControls() and V4L2Device::setControls().\n> - *\n> - * \\return The V4L2 control value\n> - */\n> -\n> -/**\n> - * \\fn V4L2Control::id()\n> - * \\brief Retrieve the control ID this instance refers to\n> - * \\return The V4L2Control ID\n> + * \\return The ControlId map\n>   */\n>  \n>  /**\n>   * \\class V4L2ControlList\n> - * \\brief Container of V4L2Control instances\n> + * \\brief A list of controls for a V4L2 device\n>   *\n> - * The V4L2ControlList class works as a container for a list of V4L2Control\n> - * instances. The class provides operations to add a new control to the list,\n> - * get back a control value, and reset the list of controls it contains.\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>   *\n> - * In order to set and get controls, user of the libcamera V4L2 control\n> - * framework should operate on instances of the V4L2ControlList class, and use\n> - * them as argument for the V4L2Device::setControls() and\n> - * V4L2Device::getControls() operations, which write and read a list of\n> - * controls to or from a V4L2 device (a video device or a subdevice).\n> - *\n> - * Controls are added to a V4L2ControlList instance with the add() method, with\n> - * or without a value.\n> - *\n> - * To write controls to a device, the controls of interest shall be added with\n> - * an initial value by calling V4L2ControlList::add(unsigned int id, int64_t\n> - * value) to prepare for a write operation. Once the values of all controls of\n> - * interest have been added, the V4L2ControlList instance is passed to the\n> - * V4L2Device::setControls(), which sets the controls on the device.\n> - *\n> - * To read controls from a device, the desired controls are added with\n> - * V4L2ControlList::add(unsigned int id) to prepare for a read operation. The\n> - * V4L2ControlList instance is then passed to V4L2Device::getControls(), which\n> - * reads the controls from the device and updates the values stored in\n> - * V4L2ControlList.\n> - *\n> - * V4L2ControlList instances can be reset to remove all controls they contain\n> - * and prepare to be re-used for a new control write/read sequence.\n> - */\n> -\n> -/**\n> - * \\typedef V4L2ControlList::iterator\n> - * \\brief Iterator on the V4L2 controls contained in the instance\n> - */\n> -\n> -/**\n> - * \\typedef V4L2ControlList::const_iterator\n> - * \\brief Const iterator on the V4L2 controls contained in the instance\n> - */\n> -\n> -/**\n> - * \\fn iterator V4L2ControlList::begin()\n> - * \\brief Retrieve an iterator to the first V4L2Control in the instance\n> - * \\return An iterator to the first V4L2 control\n> - */\n> -\n> -/**\n> - * \\fn const_iterator V4L2ControlList::begin() const\n> - * \\brief Retrieve a constant iterator to the first V4L2Control in the instance\n> - * \\return A constant iterator to the first V4L2 control\n> - */\n> -\n> -/**\n> - * \\fn iterator V4L2ControlList::end()\n> - * \\brief Retrieve an iterator pointing to the past-the-end V4L2Control in the\n> - * instance\n> - * \\return An iterator to the element following the last V4L2 control in the\n> - * instance\n> - */\n> -\n> -/**\n> - * \\fn const_iterator V4L2ControlList::end() const\n> - * \\brief Retrieve a constant iterator pointing to the past-the-end V4L2Control\n> - * in the instance\n> - * \\return A constant iterator to the element following the last V4L2 control\n> - * in the instance\n> - */\n> -\n> -/**\n> - * \\fn V4L2ControlList::empty()\n> - * \\brief Verify if the instance does not contain any control\n> - * \\return True if the instance does not contain any control, false otherwise\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> + * passed to the V4L2Device::getControls() and V4L2Device::setControls()\n> + * methods. The class should however not be used in place of ControlList in\n> + * APIs.\n>   */\n>  \n>  /**\n> - * \\fn V4L2ControlList::size()\n> - * \\brief Retrieve the number on controls in the instance\n> - * \\return The number of V4L2Control stored in the instance\n> - */\n> -\n> -/**\n> - * \\fn V4L2ControlList::clear()\n> - * \\brief Remove all controls in the instance\n> - */\n> -\n> -/**\n> - * \\brief Add control with \\a id and optional \\a value to the instance\n> - * \\param id The V4L2 control ID (V4L2_CID_*)\n> - * \\param value The V4L2 control value\n> - *\n> - * This method adds a new V4L2 control to the V4L2ControlList. The newly\n> - * inserted control shall not already be present in the control lists, otherwise\n> - * this method, and further use of the control list lead to undefined behaviour.\n> + * \\fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info)\n> + * \\brief Construct a V4L2ControlList associated with a V4L2 device\n> + * \\param[in] info The V4L2 device control info map\n>   */\n> -void V4L2ControlList::add(unsigned int id, int64_t value)\n> -{\n> -\tcontrols_.emplace_back(id, value);\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the control at \\a index\n> - * \\param[in] index The control index\n> - *\n> - * Controls are stored in a V4L2ControlList in order of insertion and this\n> - * method retrieves the control at \\a index.\n> - *\n> - * \\return A pointer to the V4L2Control at \\a index or nullptr if the\n> - * index is larger than the number of controls\n> - */\n> -V4L2Control *V4L2ControlList::getByIndex(unsigned int index)\n> -{\n> -\tif (index >= controls_.size())\n> -\t\treturn nullptr;\n> -\n> -\treturn &controls_[index];\n> -}\n> -\n> -/**\n> - * \\brief Retrieve the control with \\a id\n> - * \\param[in] id The V4L2 control ID (V4L2_CID_xx)\n> - * \\return A pointer to the V4L2Control with \\a id or nullptr if the\n> - * control ID is not part of this instance.\n> - */\n> -V4L2Control *V4L2ControlList::operator[](unsigned int id)\n> -{\n> -\tfor (V4L2Control &ctrl : controls_) {\n> -\t\tif (ctrl.id() == id)\n> -\t\t\treturn &ctrl;\n> -\t}\n> -\n> -\treturn nullptr;\n> -}\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 1f755f0f3ef6..b47ba448f354 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -163,7 +163,7 @@ void V4L2Device::close()\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::getControls(V4L2ControlList *ctrls)\n> +int V4L2Device::getControls(ControlList *ctrls)\n>  {\n>  \tunsigned int count = ctrls->size();\n>  \tif (count == 0)\n> @@ -173,18 +173,21 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>  \tstruct v4l2_ext_control v4l2Ctrls[count];\n>  \tmemset(v4l2Ctrls, 0, sizeof(v4l2Ctrls));\n>  \n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> -\t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst auto iter = controls_.find(ctrl->id());\n> +\tunsigned int i = 0;\n> +\tfor (const auto &ctrl : *ctrls) {\n> +\t\tconst ControlId *id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id->id());\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> +\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n>  \t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n> -\t\tv4l2Ctrls[i].id = ctrl->id();\n> +\t\tv4l2Ctrls[i].id = id->id();\n> +\n> +\t\ti++;\n>  \t}\n>  \n>  \tstruct v4l2_ext_controls v4l2ExtCtrls = {};\n> @@ -238,7 +241,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls)\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::setControls(V4L2ControlList *ctrls)\n> +int V4L2Device::setControls(ControlList *ctrls)\n>  {\n>  \tunsigned int count = ctrls->size();\n>  \tif (count == 0)\n> @@ -248,32 +251,36 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)\n>  \tstruct v4l2_ext_control v4l2Ctrls[count];\n>  \tmemset(v4l2Ctrls, 0, sizeof(v4l2Ctrls));\n>  \n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> -\t\tconst V4L2Control *ctrl = ctrls->getByIndex(i);\n> -\t\tconst auto iter = controls_.find(ctrl->id());\n> +\tunsigned int i = 0;\n> +\tfor (const auto &ctrl : *ctrls) {\n> +\t\tconst ControlId *id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id->id());\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << ctrl->id() << \"' not found\";\n> +\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n>  \t\tconst V4L2ControlInfo *info = &iter->second;\n>  \t\tcontrolInfo[i] = info;\n> -\t\tv4l2Ctrls[i].id = ctrl->id();\n> +\t\tv4l2Ctrls[i].id = id->id();\n>  \n>  \t\t/* Set the v4l2_ext_control value for the write operation. */\n> +\t\tconst ControlValue &value = ctrl.second;\n>  \t\tswitch (info->id().type()) {\n>  \t\tcase ControlTypeInteger64:\n> -\t\t\tv4l2Ctrls[i].value64 = ctrl->value().get<int64_t>();\n> +\t\t\tv4l2Ctrls[i].value64 = value.get<int64_t>();\n>  \t\t\tbreak;\n>  \t\tdefault:\n>  \t\t\t/*\n>  \t\t\t * \\todo To be changed when support for string and\n>  \t\t\t * compound controls will be added.\n>  \t\t\t */\n> -\t\t\tv4l2Ctrls[i].value = ctrl->value().get<int32_t>();\n> +\t\t\tv4l2Ctrls[i].value = value.get<int32_t>();\n>  \t\t\tbreak;\n>  \t\t}\n> +\n> +\t\ti++;\n>  \t}\n>  \n>  \tstruct v4l2_ext_controls v4l2ExtCtrls = {};\n> @@ -385,28 +392,34 @@ void V4L2Device::listControls()\n>   * \\param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver\n>   * \\param[in] count The number of controls to update\n>   */\n> -void V4L2Device::updateControls(V4L2ControlList *ctrls,\n> +void V4L2Device::updateControls(ControlList *ctrls,\n>  \t\t\t\tconst V4L2ControlInfo **controlInfo,\n>  \t\t\t\tconst struct v4l2_ext_control *v4l2Ctrls,\n>  \t\t\t\tunsigned int count)\n>  {\n> -\tfor (unsigned int i = 0; i < count; ++i) {\n> +\tunsigned int i = 0;\n> +\tfor (auto &ctrl : *ctrls) {\n> +\t\tif (i == count)\n> +\t\t\tbreak;\n> +\n>  \t\tconst struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i];\n>  \t\tconst V4L2ControlInfo *info = controlInfo[i];\n> -\t\tV4L2Control *ctrl = ctrls->getByIndex(i);\n> +\t\tControlValue &value = ctrl.second;\n>  \n>  \t\tswitch (info->id().type()) {\n>  \t\tcase ControlTypeInteger64:\n> -\t\t\tctrl->value().set<int64_t>(v4l2Ctrl->value64);\n> +\t\t\tvalue.set<int64_t>(v4l2Ctrl->value64);\n>  \t\t\tbreak;\n>  \t\tdefault:\n>  \t\t\t/*\n>  \t\t\t * \\todo To be changed when support for string and\n>  \t\t\t * compound controls will be added.\n>  \t\t\t */\n> -\t\t\tctrl->value().set<int32_t>(v4l2Ctrl->value);\n> +\t\t\tvalue.set<int32_t>(v4l2Ctrl->value);\n>  \t\t\tbreak;\n>  \t\t}\n> +\n> +\t\ti++;\n>  \t}\n>  }\n>  \n> diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\n> index 4a7535245c00..783b695b8cbf 100644\n> --- a/test/v4l2_videodevice/controls.cpp\n> +++ b/test/v4l2_videodevice/controls.cpp\n> @@ -44,10 +44,10 @@ protected:\n>  \t\tconst V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second;\n>  \n>  \t\t/* Test getting controls. */\n> -\t\tV4L2ControlList ctrls;\n> -\t\tctrls.add(V4L2_CID_BRIGHTNESS);\n> -\t\tctrls.add(V4L2_CID_CONTRAST);\n> -\t\tctrls.add(V4L2_CID_SATURATION);\n> +\t\tV4L2ControlList ctrls(info);\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, -1);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, -1);\n> +\t\tctrls.set(V4L2_CID_SATURATION, -1);\n>  \n>  \t\tint ret = capture_->getControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -55,17 +55,17 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value().get<int32_t>() == -1 ||\n> -\t\t    ctrls[V4L2_CID_CONTRAST]->value().get<int32_t>() == -1 ||\n> -\t\t    ctrls[V4L2_CID_SATURATION]->value().get<int32_t>() == -1) {\n> +\t\tif (ctrls.get(V4L2_CID_BRIGHTNESS).get<int32_t>() == -1 ||\n> +\t\t    ctrls.get(V4L2_CID_CONTRAST).get<int32_t>() == -1 ||\n> +\t\t    ctrls.get(V4L2_CID_SATURATION).get<int32_t>() == -1) {\n>  \t\t\tcerr << \"Incorrect value for retrieved controls\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n>  \t\t/* Test setting controls. */\n> -\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min();\n> -\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max();\n> -\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min();\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min());\n> +\t\tctrls.set(V4L2_CID_CONTRAST, contrast.range().max());\n> +\t\tctrls.set(V4L2_CID_SATURATION, saturation.range().min());\n>  \n>  \t\tret = capture_->setControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -74,9 +74,9 @@ protected:\n>  \t\t}\n>  \n>  \t\t/* Test setting controls outside of range. */\n> -\t\tctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get<int32_t>() - 1;\n> -\t\tctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get<int32_t>() + 1;\n> -\t\tctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get<int32_t>() + 1;\n> +\t\tctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min().get<int32_t>() - 1);\n> +\t\tctrls.set(V4L2_CID_CONTRAST, contrast.range().max().get<int32_t>() + 1);\n> +\t\tctrls.set(V4L2_CID_SATURATION, saturation.range().min().get<int32_t>() + 1);\n>  \n>  \t\tret = capture_->setControls(&ctrls);\n>  \t\tif (ret != 0) {\n> @@ -84,9 +84,9 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\tif (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() ||\n> -\t\t    ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() ||\n> -\t\t    ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get<int32_t>() + 1) {\n> +\t\tif (ctrls.get(V4L2_CID_BRIGHTNESS) != brightness.range().min() ||\n> +\t\t    ctrls.get(V4L2_CID_CONTRAST) != brightness.range().max() ||\n> +\t\t    ctrls.get(V4L2_CID_SATURATION) != saturation.range().min().get<int32_t>() + 1) {\n>  \t\t\tcerr << \"Controls not updated when set\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\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-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 CD5CC61562\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 18:28:14 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id a22so14293869ljd.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 13 Oct 2019 09:28:14 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tx30sm3566065ljd.39.2019.10.13.09.28.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 13 Oct 2019 09:28:12 -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=MMfF5mb8H/7gH5K3TV7evpu4gsYH4cxelVuA97qfza0=;\n\tb=UnXZKNkZx153ViM8YHUvrejJb+nKUY40HcVJF3rzkMT5hyqjFsngMHBBylQrdx9ggs\n\tVsVd+k1GZaNjtcYxQBZ6PmnjYTRWIlKTaagXcyu7/aeFG9oMiI4iQThL16czLCQhFu0+\n\tu+tSF6F8nO5gUMh+J8KlpM0sf0ogL7Xy3O1gvOmMdwdQhRAQZZx5+R1FC5mpGFvZMLKt\n\tJhMAf6yzTYxww6Fg871JNxykeYCFFimIdzJTUbEyIFCSWyebIiS0zamn61bI4XMTnGY3\n\tiVmEIA2sYm4uIdHsQ6fvmjZI0Qe+maB7b4iPIuOaZxhFUIejekrDPv/gkpTpGmhixnIP\n\tzBfg==","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=MMfF5mb8H/7gH5K3TV7evpu4gsYH4cxelVuA97qfza0=;\n\tb=bxk6ONuMkoAXpom1hEGJfVoKsoPpw8NcYaxr9uOfw6okgJBuCg53JzhhdNbkiD5I7q\n\tw3TXZ/lAKYXBcCRzI6ogvriceD3wJN11y5rCeaLQV/gbWmxMee/ruyYoVF/ghBe3i7br\n\t/CXT1QrTpS3cANYenozdzxlCJALdvaNjjGDI54Nas2Hwa+qSsZThfviSI6LVqXFD+Luc\n\tstw5ORyngi7e1oh8fOqmaRdEv/gMK04bdNx4LFAZ0UELh/zCKYbUGtvptwzOKGznmn7c\n\tEf7CnEZ0mAYpi54e9Y8NxEroAoxVAbCaI80BJRW5e4VHVBHKIIvFi/aFHb1oJIR/IBvs\n\tik9A==","X-Gm-Message-State":"APjAAAUe7EPnnzsJIju5BuC4Ce43Izhd4/Jr7JgbhHvGHSeGwpGlFgCf\n\t8GqQg3zyLiknOV7SV8hFGDuR4HJ52xY=","X-Google-Smtp-Source":"APXvYqxGOpG6TLqyqqVnCS+b1/WQK7yR0G/QjtJUgla7O84lae+BBR+Y2wgXKarHlmA9ZMMti4gzUw==","X-Received":"by 2002:a2e:1b52:: with SMTP id\n\tb79mr16068975ljb.225.1570984093748; \n\tSun, 13 Oct 2019 09:28:13 -0700 (PDT)","Date":"Sun, 13 Oct 2019 18:28:12 +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":"<20191013162812.GV23166@bigcity.dyn.berto.se>","References":"<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>\n\t<20191012184407.31684-14-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":"<20191012184407.31684-14-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device:\n\tReplace V4L2ControlList with ControlList","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":"Sun, 13 Oct 2019 16:28:15 -0000"}}]