[{"id":33061,"web_url":"https://patchwork.libcamera.org/comment/33061/","msgid":"<Z4WWeFFzK3HwEafN@pyrite.rasen.tech>","date":"2025-01-13T22:40:56","subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Jan 09, 2025 at 12:53:55PM +0100, Stefan Klug wrote:\n> The AWB modes are specified in the libcamera core controls. It is\n> therefore quite likely that every AWB algorithm will implement them. Add\n> helper functions for parsing and storing the configured modes and the\n> currently selected mode to the AwbAlgorithm base class.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>  src/ipa/libipa/awb.cpp | 100 +++++++++++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/awb.h   |  12 +++++\n>  2 files changed, 112 insertions(+)\n> \n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> index 74e88d513b27..07502da66f73 100644\n> --- a/src/ipa/libipa/awb.cpp\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -9,6 +9,8 @@\n>  \n>  #include <libcamera/base/log.h>\n>  \n> +#include <libcamera/control_ids.h>\n> +\n>  /**\n>   * \\file awb.h\n>   * \\brief Base classes for AWB algorithms\n> @@ -132,6 +134,104 @@ namespace ipa {\n>   * \\brief Controls info map for the controls provided by the algorithm\n>   */\n>  \n> +/**\n> + * \\var AwbAlgorithm::modes_\n> + * \\brief Map of all configured modes\n> + * \\sa AwbAlgorithm::parseModeConfigs\n> + */\n> +\n> +/**\n> + * \\class AwbAlgorithm::ModeConfig\n> + * \\brief Holds the configuration of a single AWB mode\n> + *\n> + * Awb modes limit the regulation of the AWB algorithm to a specific range of\n> + * colour temperatures.\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::ModeConfig::ctLo\n> + * \\brief The lowest valid colour temperature of that mode\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::ModeConfig::ctHi\n> + * \\brief The highest valid colour temperature of that mode\n> + */\n> +\n> +/**\n> + * \\brief Parse the mode configurations from the tuning data\n> + * \\param[in] tuningData the YamlObject representing the tuning data\n> + *\n> + * Utility function to parse the tuning data for a AwbMode entry and read all\n> + * provided modes. It populetes AwbAlgorithm::controls_, AwbAlgorithm::modes_\n\ns/populetes/populates/\n\n> + * and sets the current mode to AwbAuto.\n> + *\n> + * \\return Zero on success, negative error code otherwise\n> + */\n> +int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData)\n> +{\n> +\tstd::vector<ControlValue> availableModes;\n> +\n> +\tconst YamlObject &yamlModes = tuningData[controls::AwbMode.name()];\n> +\tif (!yamlModes.isDictionary()) {\n> +\t\tLOG(Awb, Error)\n> +\t\t\t<< \"AwbModes must be a dictionary.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tfor (const auto &[modeName, modeDict] : yamlModes.asDict()) {\n> +\t\tif (controls::AwbModeNameValueMap.find(modeName) ==\n> +\t\t    controls::AwbModeNameValueMap.end()) {\n> +\t\t\tLOG(Awb, Warning)\n> +\t\t\t\t<< \"Skipping unknown awb mode '\"\n> +\t\t\t\t<< modeName << \"'\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (!modeDict.isDictionary()) {\n> +\t\t\tLOG(Awb, Error)\n> +\t\t\t\t<< \"Invalid awb mode '\" << modeName << \"'\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tconst auto &modeValue = static_cast<controls::AwbModeEnum>(\n> +\t\t\tcontrols::AwbModeNameValueMap.at(modeName));\n> +\n> +\t\tauto &config = modes_[modeValue];\n> +\t\tauto hi = modeDict[\"hi\"].get<double>();\n\nWould this be better non-auto...?\n\n\nEither way,\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> +\t\tif (!hi) {\n> +\t\t\tLOG(Awb, Error) << \"Failed to read hi param of mode \"\n> +\t\t\t\t\t<< modeName;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tconfig.ctHi = *hi;\n> +\n> +\t\tauto lo = modeDict[\"lo\"].get<double>();\n> +\t\tif (!lo) {\n> +\t\t\tLOG(Awb, Error) << \"Failed to read low param of mode \"\n> +\t\t\t\t\t<< modeName;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tconfig.ctLo = *lo;\n> +\n> +\t\tavailableModes.push_back(modeValue);\n> +\t}\n> +\n> +\tif (modes_.empty()) {\n> +\t\tLOG(Awb, Error) << \"No AWB modes configured\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (modes_.find(controls::AwbAuto) == modes_.end()) {\n> +\t\tLOG(Awb, Error) << \"AwbAuto mode is missing in the configuration.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcontrols_[&controls::AwbMode] = ControlInfo(availableModes);\n> +\n> +\treturn 0;\n> +}\n> +\n>  } /* namespace ipa */\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> index 2dd471606ec4..426fdd6dae77 100644\n> --- a/src/ipa/libipa/awb.h\n> +++ b/src/ipa/libipa/awb.h\n> @@ -7,7 +7,11 @@\n>  \n>  #pragma once\n>  \n> +#include <map>\n> +\n> +#include <libcamera/control_ids.h>\n>  #include <libcamera/controls.h>\n> +\n>  #include \"libcamera/internal/yaml_parser.h\"\n>  \n>  #include \"vector.h\"\n> @@ -43,7 +47,15 @@ public:\n>  \tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n>  \n>  protected:\n> +\tint parseModeConfigs(const YamlObject &tuningData);\n> +\n> +\tstruct ModeConfig {\n> +\t\tdouble ctHi;\n> +\t\tdouble ctLo;\n> +\t};\n> +\n>  \tControlInfoMap::Map controls_;\n> +\tstd::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;\n>  };\n>  \n>  } /* namespace ipa */\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 65213BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 13 Jan 2025 22:41:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC2476851C;\n\tMon, 13 Jan 2025 23:41:34 +0100 (CET)","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 EF783684E7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Jan 2025 23:41:32 +0100 (CET)","from pyrite.rasen.tech (unknown [173.16.167.215])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C19274A;\n\tMon, 13 Jan 2025 23:40:35 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VVWZdMqG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736808036;\n\tbh=Xk1Vgaxc2bsYF4pAIwaDtVZXjuS5Cf72wSo0vBySlKk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VVWZdMqGkoxxwZB7WIEKMpNoZj4EiwB594+j5hgnWAKN6xAZqe2AuXjYj56UpKdXE\n\tWf5l7leqTkWTl2ONqUm5ID9K6NusfzNbWnqqtAW3TcNbw7J0+5xDsmlrAhsTZICstt\n\tC9jcYFSXA7cWkhAj+cbACTQB0h53GFFcW1jBK+eY=","Date":"Mon, 13 Jan 2025 16:40:56 -0600","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","Message-ID":"<Z4WWeFFzK3HwEafN@pyrite.rasen.tech>","References":"<20250109115412.356768-1-stefan.klug@ideasonboard.com>\n\t<20250109115412.356768-5-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250109115412.356768-5-stefan.klug@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33093,"web_url":"https://patchwork.libcamera.org/comment/33093/","msgid":"<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","date":"2025-01-17T14:41:02","subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\n\nOn 09/01/2025 11:53, Stefan Klug wrote:\n> The AWB modes are specified in the libcamera core controls. It is\n> therefore quite likely that every AWB algorithm will implement them. Add\n> helper functions for parsing and storing the configured modes and the\n> currently selected mode to the AwbAlgorithm base class.\n\n\nI think that this can just be squashed into the last patch.\n\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>   src/ipa/libipa/awb.cpp | 100 +++++++++++++++++++++++++++++++++++++++++\n>   src/ipa/libipa/awb.h   |  12 +++++\n>   2 files changed, 112 insertions(+)\n>\n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> index 74e88d513b27..07502da66f73 100644\n> --- a/src/ipa/libipa/awb.cpp\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -9,6 +9,8 @@\n>   \n>   #include <libcamera/base/log.h>\n>   \n> +#include <libcamera/control_ids.h>\n> +\n>   /**\n>    * \\file awb.h\n>    * \\brief Base classes for AWB algorithms\n> @@ -132,6 +134,104 @@ namespace ipa {\n>    * \\brief Controls info map for the controls provided by the algorithm\n>    */\n>   \n> +/**\n> + * \\var AwbAlgorithm::modes_\n> + * \\brief Map of all configured modes\n> + * \\sa AwbAlgorithm::parseModeConfigs\n> + */\n> +\n> +/**\n> + * \\class AwbAlgorithm::ModeConfig\n> + * \\brief Holds the configuration of a single AWB mode\n> + *\n> + * Awb modes limit the regulation of the AWB algorithm to a specific range of\n> + * colour temperatures.\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::ModeConfig::ctLo\n> + * \\brief The lowest valid colour temperature of that mode\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::ModeConfig::ctHi\n> + * \\brief The highest valid colour temperature of that mode\n> + */\n> +\n> +/**\n> + * \\brief Parse the mode configurations from the tuning data\n> + * \\param[in] tuningData the YamlObject representing the tuning data\n> + *\n> + * Utility function to parse the tuning data for a AwbMode entry and read all\n> + * provided modes. It populetes AwbAlgorithm::controls_, AwbAlgorithm::modes_\ns/populetes/populates\n> + * and sets the current mode to AwbAuto.\n\nWhat if AwbAuto isn't one of the modes defined? Ah, I see it fails. Perhaps that is worth a mention. \nI think it's also probably worth an example of the expected tuning file format like in \nAgcMeanLuminance::parseTuningData().\n\n\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\n> + *\n> + * \\return Zero on success, negative error code otherwise\n> + */\n> +int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData)\n> +{\n> +\tstd::vector<ControlValue> availableModes;\n> +\n> +\tconst YamlObject &yamlModes = tuningData[controls::AwbMode.name()];\n> +\tif (!yamlModes.isDictionary()) {\n> +\t\tLOG(Awb, Error)\n> +\t\t\t<< \"AwbModes must be a dictionary.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tfor (const auto &[modeName, modeDict] : yamlModes.asDict()) {\n> +\t\tif (controls::AwbModeNameValueMap.find(modeName) ==\n> +\t\t    controls::AwbModeNameValueMap.end()) {\n> +\t\t\tLOG(Awb, Warning)\n> +\t\t\t\t<< \"Skipping unknown awb mode '\"\n> +\t\t\t\t<< modeName << \"'\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (!modeDict.isDictionary()) {\n> +\t\t\tLOG(Awb, Error)\n> +\t\t\t\t<< \"Invalid awb mode '\" << modeName << \"'\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tconst auto &modeValue = static_cast<controls::AwbModeEnum>(\n> +\t\t\tcontrols::AwbModeNameValueMap.at(modeName));\n> +\n> +\t\tauto &config = modes_[modeValue];\n> +\t\tauto hi = modeDict[\"hi\"].get<double>();\n> +\t\tif (!hi) {\n> +\t\t\tLOG(Awb, Error) << \"Failed to read hi param of mode \"\n> +\t\t\t\t\t<< modeName;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tconfig.ctHi = *hi;\n> +\n> +\t\tauto lo = modeDict[\"lo\"].get<double>();\n> +\t\tif (!lo) {\n> +\t\t\tLOG(Awb, Error) << \"Failed to read low param of mode \"\n> +\t\t\t\t\t<< modeName;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tconfig.ctLo = *lo;\n> +\n> +\t\tavailableModes.push_back(modeValue);\n> +\t}\n> +\n> +\tif (modes_.empty()) {\n> +\t\tLOG(Awb, Error) << \"No AWB modes configured\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (modes_.find(controls::AwbAuto) == modes_.end()) {\n> +\t\tLOG(Awb, Error) << \"AwbAuto mode is missing in the configuration.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcontrols_[&controls::AwbMode] = ControlInfo(availableModes);\n> +\n> +\treturn 0;\n> +}\n> +\n>   } /* namespace ipa */\n>   \n>   } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> index 2dd471606ec4..426fdd6dae77 100644\n> --- a/src/ipa/libipa/awb.h\n> +++ b/src/ipa/libipa/awb.h\n> @@ -7,7 +7,11 @@\n>   \n>   #pragma once\n>   \n> +#include <map>\n> +\n> +#include <libcamera/control_ids.h>\n>   #include <libcamera/controls.h>\n> +\n>   #include \"libcamera/internal/yaml_parser.h\"\n>   \n>   #include \"vector.h\"\n> @@ -43,7 +47,15 @@ public:\n>   \tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n>   \n>   protected:\n> +\tint parseModeConfigs(const YamlObject &tuningData);\n> +\n> +\tstruct ModeConfig {\n> +\t\tdouble ctHi;\n> +\t\tdouble ctLo;\n> +\t};\n> +\n>   \tControlInfoMap::Map controls_;\n> +\tstd::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;\n>   };\n>   \n>   } /* namespace ipa */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 99363C3273\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 17 Jan 2025 14:41:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C48B068545;\n\tFri, 17 Jan 2025 15:41:07 +0100 (CET)","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 3D15268516\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2025 15:41:05 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD7836DE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2025 15:40:05 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NeCVozQG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737124805;\n\tbh=qtmO8jjiruspwZ3qEYJXd0UtsD7ZM1YbVMob+DJOynk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=NeCVozQGYLXzkIsvXyp3FItkQfCtfIlpRmfsVSWVBl5P6tCYrMDZ33eVZmGfcZ3Is\n\tWODLBdi5371RnhVlCDdZ8Gb/wxHR6UxLCouW0vpVua+hb1F4QClXaGXCGa3STY+n9b\n\t93nOFmE+jxFlQcLm3zXPs20ufnP9B1+tCGPRf1OM=","Message-ID":"<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","Date":"Fri, 17 Jan 2025 14:41:02 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","To":"libcamera-devel@lists.libcamera.org","References":"<20250109115412.356768-1-stefan.klug@ideasonboard.com>\n\t<20250109115412.356768-5-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20250109115412.356768-5-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33104,"web_url":"https://patchwork.libcamera.org/comment/33104/","msgid":"<1b5c849d-5520-4bb2-8bd5-5a6a48745b0c@ideasonboard.com>","date":"2025-01-20T15:35:09","subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 17/01/2025 14:41, Dan Scally wrote:\n> Hi Stefan\n>\n>\n> On 09/01/2025 11:53, Stefan Klug wrote:\n>> The AWB modes are specified in the libcamera core controls. It is\n>> therefore quite likely that every AWB algorithm will implement them. Add\n>> helper functions for parsing and storing the configured modes and the\n>> currently selected mode to the AwbAlgorithm base class.\n>\n>\n> I think that this can just be squashed into the last patch.\n>\n>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>> ---\n>>   src/ipa/libipa/awb.cpp | 100 +++++++++++++++++++++++++++++++++++++++++\n>>   src/ipa/libipa/awb.h   |  12 +++++\n>>   2 files changed, 112 insertions(+)\n>>\n>> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n>> index 74e88d513b27..07502da66f73 100644\n>> --- a/src/ipa/libipa/awb.cpp\n>> +++ b/src/ipa/libipa/awb.cpp\n>> @@ -9,6 +9,8 @@\n>>     #include <libcamera/base/log.h>\n>>   +#include <libcamera/control_ids.h>\n>> +\n>>   /**\n>>    * \\file awb.h\n>>    * \\brief Base classes for AWB algorithms\n>> @@ -132,6 +134,104 @@ namespace ipa {\n>>    * \\brief Controls info map for the controls provided by the algorithm\n>>    */\n>>   +/**\n>> + * \\var AwbAlgorithm::modes_\n>> + * \\brief Map of all configured modes\n>> + * \\sa AwbAlgorithm::parseModeConfigs\n>> + */\n>> +\n>> +/**\n>> + * \\class AwbAlgorithm::ModeConfig\n>> + * \\brief Holds the configuration of a single AWB mode\n>> + *\n>> + * Awb modes limit the regulation of the AWB algorithm to a specific range of\n>> + * colour temperatures.\n>> + */\n>> +\n>> +/**\n>> + * \\var AwbAlgorithm::ModeConfig::ctLo\n>> + * \\brief The lowest valid colour temperature of that mode\n>> + */\n>> +\n>> +/**\n>> + * \\var AwbAlgorithm::ModeConfig::ctHi\n>> + * \\brief The highest valid colour temperature of that mode\n>> + */\n>> +\n>> +/**\n>> + * \\brief Parse the mode configurations from the tuning data\n>> + * \\param[in] tuningData the YamlObject representing the tuning data\n>> + *\n>> + * Utility function to parse the tuning data for a AwbMode entry and read all\n>> + * provided modes. It populetes AwbAlgorithm::controls_, AwbAlgorithm::modes_\n> s/populetes/populates\n>> + * and sets the current mode to AwbAuto.\n>\n> What if AwbAuto isn't one of the modes defined? Ah, I see it fails. Perhaps that is worth a \n> mention. I think it's also probably worth an example of the expected tuning file format like in \n> AgcMeanLuminance::parseTuningData().\n>\nActually, this doesn't happen here. The currentMode_ is set to AwbAuto by AwbBayes::init() later in \nthe series...so either that (and the member that stores the current mode) needs moving to this class \nor the comment needs correcting.\n\n\nThanks\n\nDan\n\n>\n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n>\n>> + *\n>> + * \\return Zero on success, negative error code otherwise\n>> + */\n>> +int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData)\n>> +{\n>> +    std::vector<ControlValue> availableModes;\n>> +\n>> +    const YamlObject &yamlModes = tuningData[controls::AwbMode.name()];\n>> +    if (!yamlModes.isDictionary()) {\n>> +        LOG(Awb, Error)\n>> +            << \"AwbModes must be a dictionary.\";\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    for (const auto &[modeName, modeDict] : yamlModes.asDict()) {\n>> +        if (controls::AwbModeNameValueMap.find(modeName) ==\n>> +            controls::AwbModeNameValueMap.end()) {\n>> +            LOG(Awb, Warning)\n>> +                << \"Skipping unknown awb mode '\"\n>> +                << modeName << \"'\";\n>> +            continue;\n>> +        }\n>> +\n>> +        if (!modeDict.isDictionary()) {\n>> +            LOG(Awb, Error)\n>> +                << \"Invalid awb mode '\" << modeName << \"'\";\n>> +            return -EINVAL;\n>> +        }\n>> +\n>> +        const auto &modeValue = static_cast<controls::AwbModeEnum>(\n>> +            controls::AwbModeNameValueMap.at(modeName));\n>> +\n>> +        auto &config = modes_[modeValue];\n>> +        auto hi = modeDict[\"hi\"].get<double>();\n>> +        if (!hi) {\n>> +            LOG(Awb, Error) << \"Failed to read hi param of mode \"\n>> +                    << modeName;\n>> +            return -EINVAL;\n>> +        }\n>> +        config.ctHi = *hi;\n>> +\n>> +        auto lo = modeDict[\"lo\"].get<double>();\n>> +        if (!lo) {\n>> +            LOG(Awb, Error) << \"Failed to read low param of mode \"\n>> +                    << modeName;\n>> +            return -EINVAL;\n>> +        }\n>> +        config.ctLo = *lo;\n>> +\n>> +        availableModes.push_back(modeValue);\n>> +    }\n>> +\n>> +    if (modes_.empty()) {\n>> +        LOG(Awb, Error) << \"No AWB modes configured\";\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    if (modes_.find(controls::AwbAuto) == modes_.end()) {\n>> +        LOG(Awb, Error) << \"AwbAuto mode is missing in the configuration.\";\n>> +        return -EINVAL;\n>> +    }\n>> +\n>> +    controls_[&controls::AwbMode] = ControlInfo(availableModes);\n>> +\n>> +    return 0;\n>> +}\n>> +\n>>   } /* namespace ipa */\n>>     } /* namespace libcamera */\n>> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n>> index 2dd471606ec4..426fdd6dae77 100644\n>> --- a/src/ipa/libipa/awb.h\n>> +++ b/src/ipa/libipa/awb.h\n>> @@ -7,7 +7,11 @@\n>>     #pragma once\n>>   +#include <map>\n>> +\n>> +#include <libcamera/control_ids.h>\n>>   #include <libcamera/controls.h>\n>> +\n>>   #include \"libcamera/internal/yaml_parser.h\"\n>>     #include \"vector.h\"\n>> @@ -43,7 +47,15 @@ public:\n>>       virtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n>>     protected:\n>> +    int parseModeConfigs(const YamlObject &tuningData);\n>> +\n>> +    struct ModeConfig {\n>> +        double ctHi;\n>> +        double ctLo;\n>> +    };\n>> +\n>>       ControlInfoMap::Map controls_;\n>> +    std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;\n>>   };\n>>     } /* namespace ipa */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5F69DC3273\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jan 2025 15:35:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C7366854E;\n\tMon, 20 Jan 2025 16:35:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C87760354\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jan 2025 16:35:12 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F0458594\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jan 2025 16:34:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EeUv3AA+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737387251;\n\tbh=5OzY+sJ1z9Eygj5lloB9v15nZeTnOwswxvk8+S8dHlg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=EeUv3AA+4vorIkhPKJ1PJiASMJ1VadMnWMuy6SXuhrMCLVIsxsk41yuI/DTSbMsrS\n\t27B8FUwtZXQOUYAnaO+jL4ACA2227IF7WmSJn2kmwZGZI6uC7dr8AdKgN7Cd8gY+Pi\n\tktBotfw+bFX2EAcUtZsJXAlWXfdaRY63Yg4LF8Cw=","Message-ID":"<1b5c849d-5520-4bb2-8bd5-5a6a48745b0c@ideasonboard.com>","Date":"Mon, 20 Jan 2025 15:35:09 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","To":"libcamera-devel@lists.libcamera.org","References":"<20250109115412.356768-1-stefan.klug@ideasonboard.com>\n\t<20250109115412.356768-5-stefan.klug@ideasonboard.com>\n\t<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33140,"web_url":"https://patchwork.libcamera.org/comment/33140/","msgid":"<4f7vx2ypjpmpw4nrkblwfkd6obocp2ns46kv7hy5cxp4hv35ny@gawqoi3tgp4w>","date":"2025-01-22T16:41:38","subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Dan\n\nThank you for the review. \n\nOn Fri, Jan 17, 2025 at 02:41:02PM +0000, Dan Scally wrote:\n> Hi Stefan\n> \n> \n> On 09/01/2025 11:53, Stefan Klug wrote:\n> > The AWB modes are specified in the libcamera core controls. It is\n> > therefore quite likely that every AWB algorithm will implement them. Add\n> > helper functions for parsing and storing the configured modes and the\n> > currently selected mode to the AwbAlgorithm base class.\n> \n> \n> I think that this can just be squashed into the last patch.\n> \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > ---\n> >   src/ipa/libipa/awb.cpp | 100 +++++++++++++++++++++++++++++++++++++++++\n> >   src/ipa/libipa/awb.h   |  12 +++++\n> >   2 files changed, 112 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> > index 74e88d513b27..07502da66f73 100644\n> > --- a/src/ipa/libipa/awb.cpp\n> > +++ b/src/ipa/libipa/awb.cpp\n> > @@ -9,6 +9,8 @@\n> >   #include <libcamera/base/log.h>\n> > +#include <libcamera/control_ids.h>\n> > +\n> >   /**\n> >    * \\file awb.h\n> >    * \\brief Base classes for AWB algorithms\n> > @@ -132,6 +134,104 @@ namespace ipa {\n> >    * \\brief Controls info map for the controls provided by the algorithm\n> >    */\n> > +/**\n> > + * \\var AwbAlgorithm::modes_\n> > + * \\brief Map of all configured modes\n> > + * \\sa AwbAlgorithm::parseModeConfigs\n> > + */\n> > +\n> > +/**\n> > + * \\class AwbAlgorithm::ModeConfig\n> > + * \\brief Holds the configuration of a single AWB mode\n> > + *\n> > + * Awb modes limit the regulation of the AWB algorithm to a specific range of\n> > + * colour temperatures.\n> > + */\n> > +\n> > +/**\n> > + * \\var AwbAlgorithm::ModeConfig::ctLo\n> > + * \\brief The lowest valid colour temperature of that mode\n> > + */\n> > +\n> > +/**\n> > + * \\var AwbAlgorithm::ModeConfig::ctHi\n> > + * \\brief The highest valid colour temperature of that mode\n> > + */\n> > +\n> > +/**\n> > + * \\brief Parse the mode configurations from the tuning data\n> > + * \\param[in] tuningData the YamlObject representing the tuning data\n> > + *\n> > + * Utility function to parse the tuning data for a AwbMode entry and read all\n> > + * provided modes. It populetes AwbAlgorithm::controls_, AwbAlgorithm::modes_\n> s/populetes/populates\n> > + * and sets the current mode to AwbAuto.\n> \n> What if AwbAuto isn't one of the modes defined? Ah, I see it fails. Perhaps\n> that is worth a mention. I think it's also probably worth an example of the\n> expected tuning file format like in AgcMeanLuminance::parseTuningData().\n\nYes, you are right, I'll add documentation for that. In v2 the AutoMode\nhandling will be removed from that function and replaced by a generic\ndef parameter.\n\n> \n> \n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\nThanks,\nStefan\n\n> \n> > + *\n> > + * \\return Zero on success, negative error code otherwise\n> > + */\n> > +int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData)\n> > +{\n> > +\tstd::vector<ControlValue> availableModes;\n> > +\n> > +\tconst YamlObject &yamlModes = tuningData[controls::AwbMode.name()];\n> > +\tif (!yamlModes.isDictionary()) {\n> > +\t\tLOG(Awb, Error)\n> > +\t\t\t<< \"AwbModes must be a dictionary.\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tfor (const auto &[modeName, modeDict] : yamlModes.asDict()) {\n> > +\t\tif (controls::AwbModeNameValueMap.find(modeName) ==\n> > +\t\t    controls::AwbModeNameValueMap.end()) {\n> > +\t\t\tLOG(Awb, Warning)\n> > +\t\t\t\t<< \"Skipping unknown awb mode '\"\n> > +\t\t\t\t<< modeName << \"'\";\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > +\t\tif (!modeDict.isDictionary()) {\n> > +\t\t\tLOG(Awb, Error)\n> > +\t\t\t\t<< \"Invalid awb mode '\" << modeName << \"'\";\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\n> > +\t\tconst auto &modeValue = static_cast<controls::AwbModeEnum>(\n> > +\t\t\tcontrols::AwbModeNameValueMap.at(modeName));\n> > +\n> > +\t\tauto &config = modes_[modeValue];\n> > +\t\tauto hi = modeDict[\"hi\"].get<double>();\n> > +\t\tif (!hi) {\n> > +\t\t\tLOG(Awb, Error) << \"Failed to read hi param of mode \"\n> > +\t\t\t\t\t<< modeName;\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\t\tconfig.ctHi = *hi;\n> > +\n> > +\t\tauto lo = modeDict[\"lo\"].get<double>();\n> > +\t\tif (!lo) {\n> > +\t\t\tLOG(Awb, Error) << \"Failed to read low param of mode \"\n> > +\t\t\t\t\t<< modeName;\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\t\tconfig.ctLo = *lo;\n> > +\n> > +\t\tavailableModes.push_back(modeValue);\n> > +\t}\n> > +\n> > +\tif (modes_.empty()) {\n> > +\t\tLOG(Awb, Error) << \"No AWB modes configured\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tif (modes_.find(controls::AwbAuto) == modes_.end()) {\n> > +\t\tLOG(Awb, Error) << \"AwbAuto mode is missing in the configuration.\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tcontrols_[&controls::AwbMode] = ControlInfo(availableModes);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> >   } /* namespace ipa */\n> >   } /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> > index 2dd471606ec4..426fdd6dae77 100644\n> > --- a/src/ipa/libipa/awb.h\n> > +++ b/src/ipa/libipa/awb.h\n> > @@ -7,7 +7,11 @@\n> >   #pragma once\n> > +#include <map>\n> > +\n> > +#include <libcamera/control_ids.h>\n> >   #include <libcamera/controls.h>\n> > +\n> >   #include \"libcamera/internal/yaml_parser.h\"\n> >   #include \"vector.h\"\n> > @@ -43,7 +47,15 @@ public:\n> >   \tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n> >   protected:\n> > +\tint parseModeConfigs(const YamlObject &tuningData);\n> > +\n> > +\tstruct ModeConfig {\n> > +\t\tdouble ctHi;\n> > +\t\tdouble ctLo;\n> > +\t};\n> > +\n> >   \tControlInfoMap::Map controls_;\n> > +\tstd::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;\n> >   };\n> >   } /* namespace ipa */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 BC7EBC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Jan 2025 16:41:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AF0768558;\n\tWed, 22 Jan 2025 17:41:44 +0100 (CET)","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 B5EEF68549\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jan 2025 17:41:42 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:98fa:5e38:1a0b:e8d9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 881E5520;\n\tWed, 22 Jan 2025 17:40:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KwAjC0Qf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737564039;\n\tbh=HGaZh6BZw01CtfWUiagondLP8mqGZVPRG3cD6SQs9uM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KwAjC0Qf7pMmGY2pjonmTPyKcGGSTbXEGuk7rcXIndL4xzO34PrbTVyOc/VIGWWr3\n\tE410gBabQhTf1m6G6LtgnHaopVIxXr/SNmBqRJZNJLKMCTNL3qs5+3RXzemjtNXlnb\n\t2laBGwIfi0bhUI+SIu96LONFv3RQg02toqgaxTpw=","Date":"Wed, 22 Jan 2025 17:41:38 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode\n\tsupport","Message-ID":"<4f7vx2ypjpmpw4nrkblwfkd6obocp2ns46kv7hy5cxp4hv35ny@gawqoi3tgp4w>","References":"<20250109115412.356768-1-stefan.klug@ideasonboard.com>\n\t<20250109115412.356768-5-stefan.klug@ideasonboard.com>\n\t<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<7d0f9f6a-2650-4af5-8e50-79c6dbc4b3c7@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]