[{"id":3018,"web_url":"https://patchwork.libcamera.org/comment/3018/","msgid":"<20191114094008.GJ7353@bigcity.dyn.berto.se>","date":"2019-11-14T09:40:08","subject":"Re: [libcamera-devel] [PATCH v2 05/24] libcamera: controls: Index\n\tControlList by unsigned int","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 patch.\n\nOn 2019-11-08 22:53:50 +0200, Laurent Pinchart wrote:\n> In preparation for serialization, index the ControlList by unsigned int.\n> This will allow deserializing a ControlList without requiring external\n> information.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/controls.h              | 26 +++++++---\n>  src/libcamera/camera_controls.cpp         |  4 +-\n>  src/libcamera/controls.cpp                | 59 +++++++++--------------\n>  src/libcamera/include/camera_controls.h   |  2 +-\n>  src/libcamera/include/control_validator.h |  2 +-\n>  src/libcamera/pipeline/uvcvideo.cpp       |  4 +-\n>  src/libcamera/pipeline/vimc.cpp           |  4 +-\n>  src/libcamera/v4l2_device.cpp             | 23 ++++-----\n>  8 files changed, 60 insertions(+), 64 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 19075858fbba..f9979a466eaa 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -78,12 +78,22 @@ private:\n>  \tControlType type_;\n>  };\n>  \n> -static inline bool operator==(const ControlId &lhs, const ControlId &rhs)\n> +static inline bool operator==(unsigned int lhs, const ControlId &rhs)\n>  {\n> -\treturn lhs.id() == rhs.id();\n> +\treturn lhs == rhs.id();\n>  }\n>  \n> -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs)\n> +static inline bool operator!=(unsigned int lhs, const ControlId &rhs)\n> +{\n> +\treturn !(lhs == rhs);\n> +}\n> +\n> +static inline bool operator==(const ControlId &lhs, unsigned int rhs)\n> +{\n> +\treturn lhs.id() == rhs;\n> +}\n> +\n> +static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n>  {\n>  \treturn !(lhs == rhs);\n>  }\n> @@ -175,7 +185,7 @@ private:\n>  class ControlList\n>  {\n>  private:\n> -\tusing ControlListMap = std::unordered_map<const ControlId *, ControlValue>;\n> +\tusing ControlListMap = std::unordered_map<unsigned int, ControlValue>;\n>  \n>  public:\n>  \tControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr);\n> @@ -199,7 +209,7 @@ public:\n>  \ttemplate<typename T>\n>  \tconst T &get(const Control<T> &ctrl) const\n>  \t{\n> -\t\tconst ControlValue *val = find(ctrl);\n> +\t\tconst ControlValue *val = find(ctrl.id());\n>  \t\tif (!val) {\n>  \t\t\tstatic T t(0);\n>  \t\t\treturn t;\n> @@ -211,7 +221,7 @@ public:\n>  \ttemplate<typename T>\n>  \tvoid set(const Control<T> &ctrl, const T &value)\n>  \t{\n> -\t\tControlValue *val = find(ctrl);\n> +\t\tControlValue *val = find(ctrl.id());\n>  \t\tif (!val)\n>  \t\t\treturn;\n>  \n> @@ -222,8 +232,8 @@ public:\n>  \tvoid set(unsigned int id, const ControlValue &value);\n>  \n>  private:\n> -\tconst ControlValue *find(const ControlId &id) const;\n> -\tControlValue *find(const ControlId &id);\n> +\tconst ControlValue *find(unsigned int id) const;\n> +\tControlValue *find(unsigned int id);\n>  \n>  \tControlValidator *validator_;\n>  \tconst ControlIdMap *idmap_;\n> diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp\n> index 341da56019f7..59dcede2ca36 100644\n> --- a/src/libcamera/camera_controls.cpp\n> +++ b/src/libcamera/camera_controls.cpp\n> @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const\n>   * \\param[in] id The control ID\n>   * \\return True if the control is valid, false otherwise\n>   */\n> -bool CameraControlValidator::validate(const ControlId &id) const\n> +bool CameraControlValidator::validate(unsigned int id) const\n>  {\n>  \tconst ControlInfoMap &controls = camera_->controls();\n> -\treturn controls.find(&id) != controls.end();\n> +\treturn controls.find(id) != controls.end();\n>  }\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 2c5c98633585..021d5f0990e0 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const\n>   */\n>  \n>  /**\n> - * \\fn bool operator==(const ControlId &lhs, const ControlId &rhs)\n> - * \\brief Compare two ControlId instances for equality\n> - * \\param[in] lhs Left-hand side ControlId\n> + * \\fn bool operator==(unsigned int lhs, const ControlId &rhs)\n> + * \\brief Compare a ControlId with a control numerical ID\n> + * \\param[in] lhs Left-hand side numerical ID\n>   * \\param[in] rhs Right-hand side ControlId\n>   *\n> - * ControlId instances are compared based on the numerical ControlId::id()\n> - * only, as an object may not have two separate controls with the same\n> - * numerical ID.\n> + * \\return True if \\a lhs is equal to \\a rhs.id(), false otherwise\n> + */\n> +\n> +/**\n> + * \\fn bool operator==(const ControlId &lhs, unsigned int rhs)\n> + * \\brief Compare a ControlId with a control numerical ID\n> + * \\param[in] lhs Left-hand side ControlId\n> + * \\param[in] rhs Right-hand side numerical ID\n>   *\n> - * \\return True if \\a lhs and \\a rhs have equal control IDs, false otherwise\n> + * \\return True if \\a lhs.id() is equal to \\a rhs, false otherwise\n>   */\n>  \n>  /**\n> @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator\n>   */\n>  bool ControlList::contains(const ControlId &id) const\n>  {\n> -\treturn controls_.find(&id) != controls_.end();\n> +\treturn controls_.find(id.id()) != controls_.end();\n>  }\n>  \n>  /**\n> @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const\n>   */\n>  bool ControlList::contains(unsigned int id) const\n>  {\n> -\tconst auto iter = idmap_->find(id);\n> -\tif (iter == idmap_->end())\n> -\t\treturn false;\n> -\n> -\treturn contains(*iter->second);\n> +\treturn controls_.find(id) != controls_.end();\n>  }\n>  \n>  /**\n> @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const\n>  {\n>  \tstatic ControlValue zero;\n>  \n> -\tconst auto ctrl = idmap_->find(id);\n> -\tif (ctrl == idmap_->end()) {\n> -\t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << utils::hex(id)\n> -\t\t\t<< \" is not supported\";\n> -\t\treturn zero;\n> -\t}\n> -\n> -\tconst ControlValue *val = find(*ctrl->second);\n> +\tconst ControlValue *val = find(id);\n>  \tif (!val)\n>  \t\treturn zero;\n>  \n> @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const\n>   */\n>  void ControlList::set(unsigned int id, const ControlValue &value)\n>  {\n> -\tconst auto ctrl = idmap_->find(id);\n> -\tif (ctrl == idmap_->end()) {\n> -\t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control 0x\" << utils::hex(id)\n> -\t\t\t<< \" is not supported\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tControlValue *val = find(*ctrl->second);\n> +\tControlValue *val = find(id);\n>  \tif (!val)\n>  \t\treturn;\n>  \n>  \t*val = value;\n>  }\n>  \n> -const ControlValue *ControlList::find(const ControlId &id) const\n> +const ControlValue *ControlList::find(unsigned int id) const\n>  {\n> -\tconst auto iter = controls_.find(&id);\n> +\tconst auto iter = controls_.find(id);\n>  \tif (iter == controls_.end()) {\n>  \t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << id.name() << \" not found\";\n> +\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>  \n>  \t\treturn nullptr;\n>  \t}\n> @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const\n>  \treturn &iter->second;\n>  }\n>  \n> -ControlValue *ControlList::find(const ControlId &id)\n> +ControlValue *ControlList::find(unsigned int id)\n>  {\n>  \tif (validator_ && !validator_->validate(id)) {\n>  \t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << id.name()\n> +\t\t\t<< \"Control \" << utils::hex(id)\n>  \t\t\t<< \" is not valid for \" << validator_->name();\n>  \t\treturn nullptr;\n>  \t}\n>  \n> -\treturn &controls_[&id];\n> +\treturn &controls_[id];\n>  }\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h\n> index 998a2d155a44..265c1fe379db 100644\n> --- a/src/libcamera/include/camera_controls.h\n> +++ b/src/libcamera/include/camera_controls.h\n> @@ -19,7 +19,7 @@ public:\n>  \tCameraControlValidator(Camera *camera);\n>  \n>  \tconst std::string &name() const override;\n> -\tbool validate(const ControlId &id) const override;\n> +\tbool validate(unsigned int id) const override;\n>  \n>  private:\n>  \tCamera *camera_;\n> diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h\n> index 3598b18f2f26..f1c9110b158f 100644\n> --- a/src/libcamera/include/control_validator.h\n> +++ b/src/libcamera/include/control_validator.h\n> @@ -19,7 +19,7 @@ public:\n>  \tvirtual ~ControlValidator() {}\n>  \n>  \tvirtual const std::string &name() const = 0;\n> -\tvirtual bool validate(const ControlId &id) const = 0;\n> +\tvirtual bool validate(unsigned int id) const = 0;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 45448d6f8c05..522229241ffb 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>  \tControlList controls(data->video_->controls());\n>  \n>  \tfor (auto it : request->controls()) {\n> -\t\tconst ControlId &id = *it.first;\n> +\t\tunsigned int id = it.first;\n>  \t\tControlValue &value = it.second;\n>  \n>  \t\tif (id == controls::Brightness) {\n> @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>  \n>  \tfor (const auto &ctrl : controls)\n>  \t\tLOG(UVC, Debug)\n> -\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \"Setting control \" << utils::hex(ctrl.first)\n>  \t\t\t<< \" to \" << ctrl.second.toString();\n>  \n>  \tint ret = data->video_->setControls(&controls);\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index e6ab6a085824..06664fed42e7 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>  \tControlList controls(data->sensor_->controls());\n>  \n>  \tfor (auto it : request->controls()) {\n> -\t\tconst ControlId &id = *it.first;\n> +\t\tunsigned int id = it.first;\n>  \t\tControlValue &value = it.second;\n>  \n>  \t\tif (id == controls::Brightness)\n> @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>  \n>  \tfor (const auto &ctrl : controls)\n>  \t\tLOG(VIMC, Debug)\n> -\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \"Setting control \" << utils::hex(ctrl.first)\n>  \t\t\t<< \" to \" << ctrl.second.toString();\n>  \n>  \tint ret = data->sensor_->setControls(&controls);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index a2b0d891b12f..0452f801029c 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls)\n>  \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\tunsigned int id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id);\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n> +\t\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n> -\t\tv4l2Ctrls[i].id = id->id();\n> +\t\tv4l2Ctrls[i].id = id;\n>  \t\ti++;\n>  \t}\n>  \n> @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls)\n>  \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\tunsigned int id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id);\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n> +\t\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n> -\t\tv4l2Ctrls[i].id = id->id();\n> +\t\tv4l2Ctrls[i].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 (id->type()) {\n> +\t\tswitch (iter->first->type()) {\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tv4l2Ctrls[i].value64 = value.get<int64_t>();\n>  \t\t\tbreak;\n> @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls,\n>  \t\t\tbreak;\n>  \n>  \t\tconst struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i];\n> -\t\tconst ControlId *id = ctrl.first;\n> +\t\tunsigned int id = ctrl.first;\n>  \t\tControlValue &value = ctrl.second;\n>  \n> -\t\tswitch (id->type()) {\n> +\t\tconst auto iter = controls_.find(id);\n> +\t\tswitch (iter->first->type()) {\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tvalue.set<int64_t>(v4l2Ctrl->value64);\n>  \t\t\tbreak;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 618F76136F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Nov 2019 10:40:11 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id p18so5920105ljc.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Nov 2019 01:40:11 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tr207sm2281895lff.93.2019.11.14.01.40.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 14 Nov 2019 01:40:09 -0800 (PST)"],"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=FfzCutlSXfiWsj/xAdJKm7JtbADuRx+2GKsT/koZA04=;\n\tb=LgIryIzT47IHdNOXoaV0AAa94c/eoZExYxH3VYvPKyzbuOxPMZOG8chUk27WQddmI1\n\t1yMp35ASgkDVvixYZ5Ob/+UIkM7hZDlAwPR1G9KxtA+YbL5CdRTgmxr/YEdx4x39cbHd\n\t9rsL10ei5IiTf0E5JwK0U2FgiP+AMhrotBVkndutQiw7Si4yy2KtCTfc/eNt4NQNMVai\n\tbfXXmTu/Kj7d2fH72QXS79/e9cjZyn4Lt3DWposK3a2HGQIC4JfVaFw0C7zfxylivua4\n\t8jZSiSw4dfNSMBXFlrxAPj8HZihYMu+7N0qxjoQNWlp0i7sERtCD4zgekmWRf3JJUlxD\n\taJiQ==","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=FfzCutlSXfiWsj/xAdJKm7JtbADuRx+2GKsT/koZA04=;\n\tb=fJ/oNvxNvXRHzRJ3Hk6k0SlXYN/WgEEgUZyETme5vbTz910Bh9ZXrfLLTx29rC1o+9\n\tlmkY5b3J/dxWvcdspIs4VdxaYORSErhZr0KaKVaJZ4xg5SyByU/VK1hklT8PnXUi98nL\n\tSCT7oNObJ5DQvBnbcuGd+rgLnMYjNeQJV5KeFkpiFtpabRLDJha564DzLg4+RURVnmzc\n\t3mghr5zruQmlOJdKYqezF957d07bXqkYWyhLZ8z2zG21ijwxUFLoN7+W2TGZd4Z9RUpv\n\tYPJEl1uxBYplQSB16hU1tp995NPW7Rj14C8cNogNSuYmR3Og5gdQRxwLv+ATGJ6AIoGc\n\tkWdQ==","X-Gm-Message-State":"APjAAAU/e3EhKvz/GdZObj6ArvXxoUtP8zim4xNuuGFVl/gxsZZqtrGK\n\t1A8sSxiD6B1inYV+x9Hz+9xgEwrV1X8=","X-Google-Smtp-Source":"APXvYqyD8k9ETp0jJC+UnLGAa94qFyF1KKVJUsR9m1RLcQf+IyhBXP6/iG/Pyxh8vGAPaWkSjGf3Mw==","X-Received":"by 2002:a2e:4703:: with SMTP id u3mr5438238lja.126.1573724410625;\n\tThu, 14 Nov 2019 01:40:10 -0800 (PST)","Date":"Thu, 14 Nov 2019 10:40:08 +0100","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":"<20191114094008.GJ7353@bigcity.dyn.berto.se>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-6-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":"<20191108205409.18845-6-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 05/24] libcamera: controls: Index\n\tControlList by unsigned int","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":"Thu, 14 Nov 2019 09:40:11 -0000"}},{"id":3025,"web_url":"https://patchwork.libcamera.org/comment/3025/","msgid":"<20191115160801.n2rujtcttvrdnakd@uno.localdomain>","date":"2019-11-15T16:08:01","subject":"Re: [libcamera-devel] [PATCH v2 05/24] libcamera: controls: Index\n\tControlList by unsigned int","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Nov 08, 2019 at 10:53:50PM +0200, Laurent Pinchart wrote:\n> In preparation for serialization, index the ControlList by unsigned int.\n> This will allow deserializing a ControlList without requiring external\n> information.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> ---\n>  include/libcamera/controls.h              | 26 +++++++---\n>  src/libcamera/camera_controls.cpp         |  4 +-\n>  src/libcamera/controls.cpp                | 59 +++++++++--------------\n>  src/libcamera/include/camera_controls.h   |  2 +-\n>  src/libcamera/include/control_validator.h |  2 +-\n>  src/libcamera/pipeline/uvcvideo.cpp       |  4 +-\n>  src/libcamera/pipeline/vimc.cpp           |  4 +-\n>  src/libcamera/v4l2_device.cpp             | 23 ++++-----\n>  8 files changed, 60 insertions(+), 64 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 19075858fbba..f9979a466eaa 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -78,12 +78,22 @@ private:\n>  \tControlType type_;\n>  };\n>\n> -static inline bool operator==(const ControlId &lhs, const ControlId &rhs)\n> +static inline bool operator==(unsigned int lhs, const ControlId &rhs)\n>  {\n> -\treturn lhs.id() == rhs.id();\n> +\treturn lhs == rhs.id();\n>  }\n>\n> -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs)\n> +static inline bool operator!=(unsigned int lhs, const ControlId &rhs)\n> +{\n> +\treturn !(lhs == rhs);\n> +}\n> +\n> +static inline bool operator==(const ControlId &lhs, unsigned int rhs)\n> +{\n> +\treturn lhs.id() == rhs;\n> +}\n> +\n> +static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n>  {\n>  \treturn !(lhs == rhs);\n>  }\n> @@ -175,7 +185,7 @@ private:\n>  class ControlList\n>  {\n>  private:\n> -\tusing ControlListMap = std::unordered_map<const ControlId *, ControlValue>;\n> +\tusing ControlListMap = std::unordered_map<unsigned int, ControlValue>;\n>\n>  public:\n>  \tControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr);\n> @@ -199,7 +209,7 @@ public:\n>  \ttemplate<typename T>\n>  \tconst T &get(const Control<T> &ctrl) const\n>  \t{\n> -\t\tconst ControlValue *val = find(ctrl);\n> +\t\tconst ControlValue *val = find(ctrl.id());\n>  \t\tif (!val) {\n>  \t\t\tstatic T t(0);\n>  \t\t\treturn t;\n> @@ -211,7 +221,7 @@ public:\n>  \ttemplate<typename T>\n>  \tvoid set(const Control<T> &ctrl, const T &value)\n>  \t{\n> -\t\tControlValue *val = find(ctrl);\n> +\t\tControlValue *val = find(ctrl.id());\n>  \t\tif (!val)\n>  \t\t\treturn;\n>\n> @@ -222,8 +232,8 @@ public:\n>  \tvoid set(unsigned int id, const ControlValue &value);\n>\n>  private:\n> -\tconst ControlValue *find(const ControlId &id) const;\n> -\tControlValue *find(const ControlId &id);\n> +\tconst ControlValue *find(unsigned int id) const;\n> +\tControlValue *find(unsigned int id);\n>\n>  \tControlValidator *validator_;\n>  \tconst ControlIdMap *idmap_;\n> diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp\n> index 341da56019f7..59dcede2ca36 100644\n> --- a/src/libcamera/camera_controls.cpp\n> +++ b/src/libcamera/camera_controls.cpp\n> @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const\n>   * \\param[in] id The control ID\n>   * \\return True if the control is valid, false otherwise\n>   */\n> -bool CameraControlValidator::validate(const ControlId &id) const\n> +bool CameraControlValidator::validate(unsigned int id) const\n>  {\n>  \tconst ControlInfoMap &controls = camera_->controls();\n> -\treturn controls.find(&id) != controls.end();\n> +\treturn controls.find(id) != controls.end();\n>  }\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 2c5c98633585..021d5f0990e0 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const\n>   */\n>\n>  /**\n> - * \\fn bool operator==(const ControlId &lhs, const ControlId &rhs)\n> - * \\brief Compare two ControlId instances for equality\n> - * \\param[in] lhs Left-hand side ControlId\n> + * \\fn bool operator==(unsigned int lhs, const ControlId &rhs)\n> + * \\brief Compare a ControlId with a control numerical ID\n> + * \\param[in] lhs Left-hand side numerical ID\n>   * \\param[in] rhs Right-hand side ControlId\n>   *\n> - * ControlId instances are compared based on the numerical ControlId::id()\n> - * only, as an object may not have two separate controls with the same\n> - * numerical ID.\n> + * \\return True if \\a lhs is equal to \\a rhs.id(), false otherwise\n> + */\n> +\n> +/**\n> + * \\fn bool operator==(const ControlId &lhs, unsigned int rhs)\n> + * \\brief Compare a ControlId with a control numerical ID\n> + * \\param[in] lhs Left-hand side ControlId\n> + * \\param[in] rhs Right-hand side numerical ID\n>   *\n> - * \\return True if \\a lhs and \\a rhs have equal control IDs, false otherwise\n> + * \\return True if \\a lhs.id() is equal to \\a rhs, false otherwise\n>   */\n>\n>  /**\n> @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator\n>   */\n>  bool ControlList::contains(const ControlId &id) const\n>  {\n> -\treturn controls_.find(&id) != controls_.end();\n> +\treturn controls_.find(id.id()) != controls_.end();\n>  }\n>\n>  /**\n> @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const\n>   */\n>  bool ControlList::contains(unsigned int id) const\n>  {\n> -\tconst auto iter = idmap_->find(id);\n> -\tif (iter == idmap_->end())\n> -\t\treturn false;\n> -\n> -\treturn contains(*iter->second);\n> +\treturn controls_.find(id) != controls_.end();\n>  }\n>\n>  /**\n> @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const\n>  {\n>  \tstatic ControlValue zero;\n>\n> -\tconst auto ctrl = idmap_->find(id);\n> -\tif (ctrl == idmap_->end()) {\n> -\t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << utils::hex(id)\n> -\t\t\t<< \" is not supported\";\n> -\t\treturn zero;\n> -\t}\n> -\n> -\tconst ControlValue *val = find(*ctrl->second);\n> +\tconst ControlValue *val = find(id);\n>  \tif (!val)\n>  \t\treturn zero;\n>\n> @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const\n>   */\n>  void ControlList::set(unsigned int id, const ControlValue &value)\n>  {\n> -\tconst auto ctrl = idmap_->find(id);\n> -\tif (ctrl == idmap_->end()) {\n> -\t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control 0x\" << utils::hex(id)\n> -\t\t\t<< \" is not supported\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tControlValue *val = find(*ctrl->second);\n> +\tControlValue *val = find(id);\n>  \tif (!val)\n>  \t\treturn;\n>\n>  \t*val = value;\n>  }\n>\n> -const ControlValue *ControlList::find(const ControlId &id) const\n> +const ControlValue *ControlList::find(unsigned int id) const\n>  {\n> -\tconst auto iter = controls_.find(&id);\n> +\tconst auto iter = controls_.find(id);\n>  \tif (iter == controls_.end()) {\n>  \t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << id.name() << \" not found\";\n> +\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>\n>  \t\treturn nullptr;\n>  \t}\n> @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const\n>  \treturn &iter->second;\n>  }\n>\n> -ControlValue *ControlList::find(const ControlId &id)\n> +ControlValue *ControlList::find(unsigned int id)\n>  {\n>  \tif (validator_ && !validator_->validate(id)) {\n>  \t\tLOG(Controls, Error)\n> -\t\t\t<< \"Control \" << id.name()\n> +\t\t\t<< \"Control \" << utils::hex(id)\n>  \t\t\t<< \" is not valid for \" << validator_->name();\n>  \t\treturn nullptr;\n>  \t}\n>\n> -\treturn &controls_[&id];\n> +\treturn &controls_[id];\n>  }\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h\n> index 998a2d155a44..265c1fe379db 100644\n> --- a/src/libcamera/include/camera_controls.h\n> +++ b/src/libcamera/include/camera_controls.h\n> @@ -19,7 +19,7 @@ public:\n>  \tCameraControlValidator(Camera *camera);\n>\n>  \tconst std::string &name() const override;\n> -\tbool validate(const ControlId &id) const override;\n> +\tbool validate(unsigned int id) const override;\n>\n>  private:\n>  \tCamera *camera_;\n> diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h\n> index 3598b18f2f26..f1c9110b158f 100644\n> --- a/src/libcamera/include/control_validator.h\n> +++ b/src/libcamera/include/control_validator.h\n> @@ -19,7 +19,7 @@ public:\n>  \tvirtual ~ControlValidator() {}\n>\n>  \tvirtual const std::string &name() const = 0;\n> -\tvirtual bool validate(const ControlId &id) const = 0;\n> +\tvirtual bool validate(unsigned int id) const = 0;\n>  };\n>\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 45448d6f8c05..522229241ffb 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>  \tControlList controls(data->video_->controls());\n>\n>  \tfor (auto it : request->controls()) {\n> -\t\tconst ControlId &id = *it.first;\n> +\t\tunsigned int id = it.first;\n>  \t\tControlValue &value = it.second;\n>\n>  \t\tif (id == controls::Brightness) {\n> @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>\n>  \tfor (const auto &ctrl : controls)\n>  \t\tLOG(UVC, Debug)\n> -\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \"Setting control \" << utils::hex(ctrl.first)\n>  \t\t\t<< \" to \" << ctrl.second.toString();\n>\n>  \tint ret = data->video_->setControls(&controls);\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index e6ab6a085824..06664fed42e7 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>  \tControlList controls(data->sensor_->controls());\n>\n>  \tfor (auto it : request->controls()) {\n> -\t\tconst ControlId &id = *it.first;\n> +\t\tunsigned int id = it.first;\n>  \t\tControlValue &value = it.second;\n>\n>  \t\tif (id == controls::Brightness)\n> @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>\n>  \tfor (const auto &ctrl : controls)\n>  \t\tLOG(VIMC, Debug)\n> -\t\t\t<< \"Setting control \" << ctrl.first->name()\n> +\t\t\t<< \"Setting control \" << utils::hex(ctrl.first)\n>  \t\t\t<< \" to \" << ctrl.second.toString();\n>\n>  \tint ret = data->sensor_->setControls(&controls);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index a2b0d891b12f..0452f801029c 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls)\n>\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\tunsigned int id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id);\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n> +\t\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n> -\t\tv4l2Ctrls[i].id = id->id();\n> +\t\tv4l2Ctrls[i].id = id;\n>  \t\ti++;\n>  \t}\n>\n> @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls)\n>\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\tunsigned int id = ctrl.first;\n> +\t\tconst auto iter = controls_.find(id);\n>  \t\tif (iter == controls_.end()) {\n>  \t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Control '\" << id->name() << \"' not found\";\n> +\t\t\t\t<< \"Control \" << utils::hex(id) << \" not found\";\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>\n> -\t\tv4l2Ctrls[i].id = id->id();\n> +\t\tv4l2Ctrls[i].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 (id->type()) {\n> +\t\tswitch (iter->first->type()) {\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tv4l2Ctrls[i].value64 = value.get<int64_t>();\n>  \t\t\tbreak;\n> @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls,\n>  \t\t\tbreak;\n>\n>  \t\tconst struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i];\n> -\t\tconst ControlId *id = ctrl.first;\n> +\t\tunsigned int id = ctrl.first;\n>  \t\tControlValue &value = ctrl.second;\n>\n> -\t\tswitch (id->type()) {\n> +\t\tconst auto iter = controls_.find(id);\n> +\t\tswitch (iter->first->type()) {\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tvalue.set<int64_t>(v4l2Ctrl->value64);\n>  \t\t\tbreak;\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 relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9AED6136F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Nov 2019 17:06:05 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 3DCC41BF20A;\n\tFri, 15 Nov 2019 16:06:04 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 15 Nov 2019 17:08:01 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191115160801.n2rujtcttvrdnakd@uno.localdomain>","References":"<20191108205409.18845-1-laurent.pinchart@ideasonboard.com>\n\t<20191108205409.18845-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"rw5puaya2o2ujqw2\"","Content-Disposition":"inline","In-Reply-To":"<20191108205409.18845-6-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 05/24] libcamera: controls: Index\n\tControlList by unsigned int","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":"Fri, 15 Nov 2019 16:06:05 -0000"}}]