Patch Detail
Show a patch.
GET /api/patches/2174/?format=api
{ "id": 2174, "url": "https://patchwork.libcamera.org/api/patches/2174/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2174/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20191012184407.31684-14-laurent.pinchart@ideasonboard.com>", "date": "2019-10-12T18:44:06", "name": "[libcamera-devel,v2,13/14] libcamera: v4l2_device: Replace V4L2ControlList with ControlList", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "0c8cf899d39aa6e860c4340525b51dcd05da364c", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2174/mbox/", "series": [ { "id": 531, "url": "https://patchwork.libcamera.org/api/series/531/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=531", "date": "2019-10-12T18:43:53", "name": "Use ControlList for both libcamera and V4L2 controls", "version": 2, "mbox": "https://patchwork.libcamera.org/series/531/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2174/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2174/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D21761992\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Oct 2019 20:44:23 +0200 (CEST)", "from pendragon.bb.dnainternet.fi\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98E7F9C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Oct 2019 20:44:22 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570905862;\n\tbh=oEl0RtMWRwCO1T25+14EclV/mIo5AIWgKBa4j/ltJPo=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=ViFT4Rpgp5VEgiBPQzoDHK7+QUzudP3sd1jjVw8WUjRV+/36opqmITV3izHonuTAv\n\tFJ2g0naPG33HcJpzzD8fKlrkYth6IpFK+Mvka4fLPUkTyN/Kkizo4GuUpf3387Xa68\n\tnVpkuNzIcYIR4qFn/oBhKrjA6epPsGTiUX2y7BUs=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 12 Oct 2019 21:44:06 +0300", "Message-Id": "<20191012184407.31684-14-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>", "References": "<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device: Replace\n\tV4L2ControlList 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": "Sat, 12 Oct 2019 18:44:24 -0000" }, "content": "The V4L2Device class uses V4L2ControlList as a controls container for\nthe getControls() and setControls() operations. Having a distinct\ncontainer from ControlList will makes the IPA API more complex, as it\nneeds to explicitly transport both types of lists. This will become even\nmore painful when implementing serialisation and deserialisation.\n\nTo simplify the IPA API and ease the implementation of serialisation and\ndeserialisation, replace usage of V4L2ControlList with ControlList in\nthe V4L2Device (and thus CameraSensor) API. The V4L2ControlList class\nbecomes a thin wrapper around ControlList that slightly simplifies the\ncreation of control lists for V4L2 controls, and may be removed in the\nfuture.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges 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(-)", "diff": "diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 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);\ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 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 }\ndiff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\nindex 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;\ndiff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\nindex 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 */\ndiff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\nindex 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);\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 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\";\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 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;\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 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;\ndiff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\nindex 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 */\ndiff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\nindex 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 \ndiff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp\nindex 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", "prefixes": [ "libcamera-devel", "v2", "13/14" ] }