[{"id":18269,"web_url":"https://patchwork.libcamera.org/comment/18269/","msgid":"<20210722143123.5ri62evv4ufsk5b5@uno.localdomain>","date":"2021-07-22T14:31:23","subject":"Re: [libcamera-devel] [PATCH v5 6/9] android: Add helpers for\n\tsetting android metadata from libcamera controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Tue, Jul 20, 2021 at 07:13:04PM +0900, Paul Elder wrote:\n> Add helpers for setting android metadata from libcamera controls.\n>\n> There are two versions, for scalars and collections, both of which take\n> a default value to fill in the android control if the libcamera control\n> is not found. A version for scalars exists for no default, to not set\n> the android control at all if it is not found in libcamera. The\n> functions take two template parameters, the first for the android type,\n> and the second for the libcamera type of the control. They can be\n> different, for example, if the former is an enum and the latter is a\n> boolean, or if the former is an enum (uint8_t) and the latter is an enum\n> (int32_t).\n>\n> The versions that take a default value return the value that was set in\n> the android metadata.\n>\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v4:\n> - remove vector copy from the vector setter\n>\n> Changes in v3:\n> - setMetadata for collection only works with vectors\n> - change enum to enum class\n> - add two template parameters for android type and libcamera type\n> - add docs\n>\n> New in v2\n>\n> TODO: make ControlList versions so that we can use them in result\n> metadata\n> ---\n> src/android/camera_capabilities.cpp | 137 ++++++++++++++++++++++++++++\n> 1 file changed, 137 insertions(+)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index c97a17e6..a8fa7ffe 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n>\n> @@ -125,6 +126,142 @@ hwLevelStrings = {\n> \t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, \"EXTERNAL\" },\n> };\n>\n> +enum class ControlRange {\n> +\tMin,\n> +\tDef,\n> +\tMax,\n> +};\n> +\n> +/**\n> + * \\brief Set android metadata from libcamera ControlInfo\n> + * \\tparam T Type of the control in android\n> + * \\tparam V Type of the control in libcamera\n> + * \\param[in] metadata Android metadata pack to add the control value to\n> + * \\param[in] tag Android metadata tag\n> + * \\param[in] controlsInfo libcamera ControlInfoMap from which to find the control info\n> + * \\param[in] control libcamera ControlId to find from \\a controlsInfo\n> + * \\param[in] controlRange Whether to use the min, def, or max value from the control info\n> + *\n> + * Set the android metadata entry in \\a metadata with tag \\a tag based on the\n> + * control info found for the libcamera control \\a control in the libcamera\n> + * ControlInfoMap \\a controlsInfo. If no libcamera ControlInfo is found, then\n> + * the function returns without modifying anything.\n> + *\n> + * This function is for scalar values.\n> + */\n> +template +\t std::enable_if_t> * = nullptr,\n> +\t typename V>\n> +void setMetadata(CameraMetadata *metadata, uint32_t tag,\n> +\t\t const ControlInfoMap &controlsInfo, const ControlId *control,\n> +\t\t enum ControlRange controlRange)\n> +{\n> +\tconst auto &info = controlsInfo.find(control);\n> +\tif (info == controlsInfo.end())\n> +\t\treturn;\n> +\n> +\tT ret;\n> +\tswitch (controlRange) {\n> +\tcase ControlRange::Min:\n> +\t\tret = info->second.min().get();\n> +\t\tbreak;\n> +\tcase ControlRange::Def:\n> +\t\tret = info->second.def().get();\n> +\t\tbreak;\n> +\tcase ControlRange::Max:\n> +\t\tret = info->second.max().get();\n> +\t\tbreak;\n> +\t}\n> +\n> +\tmetadata->addEntry(tag, ret);\n> +\treturn;\n> +}\n> +\n> +/**\n> + * \\brief Set android metadata from libcamera ControlInfo or a default value\n> + * \\tparam T Type of the control in android\n> + * \\tparam U Type of the control in libcamera\n> + * \\param[in] metadata Android metadata pack to add the control value to\n> + * \\param[in] tag Android metadata tag\n> + * \\param[in] controlsInfo libcamera ControlInfoMap from which to find the control info\n> + * \\param[in] control libcamera ControlId to find from \\a controlsInfo\n> + * \\param[in] controlRange Whether to use the min, def, or max value from the control info\n> + * \\param[in] defaultValue The value to set in \\a metadata if \\a control is not found\n> + *\n> + * Set the android metadata entry in \\a metadata with tag \\a tag based on the\n> + * control info found for the libcamera control \\a control in the libcamera\n> + * ControlInfoMap \\a controlsInfo. If no libcamera ControlInfo is found, then\n> + * the android metadata entry is set to \\a defaultValue.\n> + *\n> + * This function is for scalar values.\n> + */\n> +template +\t typename U,\n> +\t typename V,\n\nI still think U and V are the same type\n\n> +\t std::enable_if_t ||\n> +\t\t\t std::is_enum_v> * = nullptr>\n\nI still think you don't need enable_if\n\nCould you please go through mine and Laurent's comments on v3 ?\n(if the patch hasn't changed in way I have missed, sorry if that's the\ncase).\n\nI think this could be greatly simplified.\n\n> +T setMetadata(CameraMetadata *metadata, uint32_t tag,\n> +\t const ControlInfoMap &controlsInfo, const ControlId *control,\n> +\t enum ControlRange controlRange, const V defaultValue)\n> +{\n> +\tT ret = defaultValue;\n> +\n> +\tconst auto &info = controlsInfo.find(control);\n> +\tif (info != controlsInfo.end()) {\n> +\t\tswitch (controlRange) {\n> +\t\tcase ControlRange::Min:\n> +\t\t\tret = info->second.min().get();\n> +\t\t\tbreak;\n> +\t\tcase ControlRange::Def:\n> +\t\t\tret = info->second.def().get();\n> +\t\t\tbreak;\n> +\t\tcase ControlRange::Max:\n> +\t\t\tret = info->second.max().get();\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tmetadata->addEntry(tag, ret);\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\brief Set android metadata from libcamera ControlInfo or a default value\n> + * \\tparam T Type of the control in android\n> + * \\tparam V Type of the control in libcamera\n> + * \\param[in] metadata Android metadata pack to add the control value to\n> + * \\param[in] tag Android metadata tag\n> + * \\param[in] controlsInfo libcamera ControlInfoMap from which to find the control info\n> + * \\param[in] control libcamera ControlId to find from \\a controlsInfo\n> + * \\param[in] defaultVector The value to set in \\a metadata if \\a control is not found\n> + *\n> + * Set the android metadata entry in \\a metadata with tag \\a tag based on the\n> + * control info found for the libcamera control \\a control in the libcamera\n> + * ControlInfoMap \\a controlsInfo. If no libcamera ControlInfo is found, then\n> + * the android metadata entry is set to \\a defaultVector.\n> + *\n> + * This function is for vector values.\n> + */\n> +template\n> +std::vector setMetadata(CameraMetadata *metadata, uint32_t tag,\n> +\t\t\t const ControlInfoMap &controlsInfo,\n> +\t\t\t const ControlId *control,\n> +\t\t\t const std::vector &defaultVector)\n> +{\n> +\tconst auto &info = controlsInfo.find(control);\n> +\tif (info == controlsInfo.end()) {\n> +\t\tmetadata->addEntry(tag, defaultVector);\n> +\t\treturn defaultVector;\n> +\t}\n> +\n> +\tstd::vector ret(info->second.values().size());\n> +\tfor (const auto &value : info->second.values())\n> +\t\tret.push_back(value.get());\n> +\tmetadata->addEntry(tag, ret);\n> +\n> +\treturn ret;\n> +}\n> +\n> } /* namespace */\n>\n> bool CameraCapabilities::validateManualSensorCapability(const CameraMetadata *staticMetadata)\n> --\n> 2.27.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8E727C322B\n\tfor ;\n\tThu, 22 Jul 2021 14:30:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 140746877A;\n\tThu, 22 Jul 2021 16:30:38 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2545F68779\n\tfor ;\n\tThu, 22 Jul 2021 16:30:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 9AB166000B;\n\tThu, 22 Jul 2021 14:30:35 +0000 (UTC)"],"Date":"Thu, 22 Jul 2021 16:31:23 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210722143123.5ri62evv4ufsk5b5@uno.localdomain>","References":"<20210720101307.26010-1-paul.elder@ideasonboard.com>\n\t<20210720101307.26010-7-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210720101307.26010-7-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 6/9] android: Add helpers for\n\tsetting android metadata from libcamera controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]