[{"id":24069,"web_url":"https://patchwork.libcamera.org/comment/24069/","msgid":"<YtwhtlfxaMzPWl/v@pendragon.ideasonboard.com>","date":"2022-07-23T16:28:38","subject":"Re: [libcamera-devel] [PATCH v6 5/8] ipa: raspberrypi: Introduce\n\tversion 2.0 format for the camera tuning file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Jul 18, 2022 at 09:15:59AM +0100, Naushir Patuck via libcamera-devel wrote:\n> The existing tuning file format (version 1.0) requires the controller algorithms\n> to run in the same order as listed in the JSON structure. The JSON specification\n> does not mandate any such ordering, but the Boost JSON parser would maintain\n> this order.\n> \n> In order to remove this reliance on the parser to provide ordering, introduce a\n> new version 2.0 format for the camera tuning file. In this version, the\n> algorithms are specified in a top level list node (\"algorithms\"), which does\n> require strict ordering of the elements.\n> \n> A \"version\" node is added to distinguish between the version 1.0 and 2.0\n> formats. The absence of the \"version\" node implies version 1.0.\n> \n> A \"target\" node is also added to specify the target platform for this\n> configuration.\n> \n> Update the controller to support either version of the tuning file by looking\n> at the version node. CreateAlgorithm member function to now load and configure\n> each algorithm. Additionally, make CreateAlgorithm a private member, it does not\n> get called externally.\n> \n> If a version 1.0 format tuning file is used, throw a warning message indicating\n> it will be soon deprecated.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/ipa/raspberrypi/controller/controller.cpp | 43 +++++++++++++------\n>  src/ipa/raspberrypi/controller/controller.hpp |  4 +-\n>  2 files changed, 34 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp\n> index 67d650ef0c1b..b22a17f18f8e 100644\n> --- a/src/ipa/raspberrypi/controller/controller.cpp\n> +++ b/src/ipa/raspberrypi/controller/controller.cpp\n> @@ -42,22 +42,41 @@ void Controller::Read(char const *filename)\n>  \t}\n>  \n>  \tstd::unique_ptr<YamlObject> root = YamlParser::parse(file);\n> -\n> -\tfor (auto const &[key, value] : root->asDict()) {\n> -\t\tAlgorithm *algo = CreateAlgorithm(key.c_str());\n> -\t\tif (algo) {\n> -\t\t\talgo->Read(value);\n> -\t\t\talgorithms_.push_back(AlgorithmPtr(algo));\n> -\t\t} else\n> -\t\t\tLOG(RPiController, Warning)\n> -\t\t\t\t<< \"No algorithm found for \\\"\" << key << \"\\\"\";\n> +\tdouble version = (*root)[\"version\"].get<double>(1.0);\n> +\tif (version < 2.0) {\n> +\t\tLOG(RPiController, Warning)\n> +\t\t\t<< \"This format of the tuning file will be deprecated soon!\"\n> +\t\t\t<< \" Please use the convert_tuning.py utility to update to version 2.0.\";\n> +\t\tfor (auto const &[key, value] : root->asDict())\n> +\t\t\tCreateAlgorithm(key, value);\n> +\t} else if (version < 3.0) {\n> +\t\tif (!root->contains(\"algorithms\")) {\n> +\t\t\tLOG(RPiController, Fatal)\n> +\t\t\t\t<< \"Tuning file \" << filename\n> +\t\t\t\t<< \" does not have an \\\"algorithms\\\" list!\";\n> +\t\t\treturn;\n> +\t\t}\n> +\t\tfor (auto const &rootAlgo : (*root)[\"algorithms\"].asList())\n> +\t\t\tfor (auto const &[key, value] : rootAlgo.asDict())\n> +\t\t\t\tCreateAlgorithm(key, value);\n> +\t} else {\n> +\t\tLOG(RPiController, Fatal)\n> +\t\t\t<< \"Unrecognised version \" << version\n> +\t\t\t<< \" for the tuning file \" << filename;\n>  \t}\n>  }\n>  \n> -Algorithm *Controller::CreateAlgorithm(char const *name)\n> +void Controller::CreateAlgorithm(const std::string &name, const YamlObject &params)\n>  {\n> -\tauto it = GetAlgorithms().find(std::string(name));\n> -\treturn it != GetAlgorithms().end() ? (*it->second)(this) : nullptr;\n> +\tauto it = GetAlgorithms().find(name);\n> +\tif (it == GetAlgorithms().end()) {\n> +\t\tLOG(RPiController, Warning)\n> +\t\t\t<< \"No algorithm found for \\\"\" << name << \"\\\"\";\n> +\t\treturn;\n> +\t}\n> +\tAlgorithm *algo = (*it->second)(this);\n> +\talgo->Read(params);\n> +\talgorithms_.push_back(AlgorithmPtr(algo));\n>  }\n>  \n>  void Controller::Initialise()\n> diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp\n> index 3b50ae770d11..9091deac5feb 100644\n> --- a/src/ipa/raspberrypi/controller/controller.hpp\n> +++ b/src/ipa/raspberrypi/controller/controller.hpp\n> @@ -15,6 +15,8 @@\n>  \n>  #include <linux/bcm2835-isp.h>\n>  \n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n>  #include \"camera_mode.h\"\n>  #include \"device_status.h\"\n>  #include \"metadata.hpp\"\n> @@ -36,7 +38,6 @@ public:\n>  \tController();\n>  \tController(char const *json_filename);\n>  \t~Controller();\n> -\tAlgorithm *CreateAlgorithm(char const *name);\n>  \tvoid Read(char const *filename);\n>  \tvoid Initialise();\n>  \tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata);\n> @@ -46,6 +47,7 @@ public:\n>  \tAlgorithm *GetAlgorithm(std::string const &name) const;\n>  \n>  protected:\n> +\tvoid CreateAlgorithm(const std::string &name, const libcamera::YamlObject &params);\n>  \tMetadata global_metadata_;\n>  \tstd::vector<AlgorithmPtr> algorithms_;\n>  \tbool switch_mode_called_;","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 89F59C3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 23 Jul 2022 16:28:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F31FD63312;\n\tSat, 23 Jul 2022 18:28:43 +0200 (CEST)","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 8C206603F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 23 Jul 2022 18:28:42 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB9C59F7;\n\tSat, 23 Jul 2022 18:28:41 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658593724;\n\tbh=F20WCAIOCzR3sRLUArL920nDE3YknUTN8toi5FJAchs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=L7laP4z0LavqLcJdivmB3aBpPklALagrK3jYJ7u+0vR5XfE/WqsrxBeJmsLQtN/xv\n\teLEWUAq72iP+aBQpP3oWMX69R9jt7VL7bnYVrEAPwl32CcWh5soGTLlYOzBiYJFj1/\n\te+gfszkVG+ON+kuIl/meiWOtHFY0zRmXX/2mUFeVdTyt/EKKOehaFGHj9C/pjH2Amj\n\tDqDSeOVRqKj8mPw1SzYXtq5TvwherP9Hp8z3IX2w5lHxpbNerRJ7JDFSwqa01g56ih\n\t3Ildh5QzrZ6gyAXvoHYRXrnIQB+5BssI7otn4E0BGyGMnRyDJ5Xa+YCSTKsuW9LTVh\n\twzw7S5q4tdwVg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658593722;\n\tbh=F20WCAIOCzR3sRLUArL920nDE3YknUTN8toi5FJAchs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fqkxoAwgLwRm3nTii19kLNp+Yw9vo6x//9Q4/aUSB+RVfPh5ufh7medd+Z/HEu4y9\n\t4wtU1gHSuZObX+BXJX9l2E27XrX2Kr0Qdnz5Q8G274CeoRqyRvMeNeieBq0elTwhQf\n\tovioCclLbzkLjz5eh+Pn7bknI2EBkjb3JwzQvUl0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fqkxoAwg\"; dkim-atps=neutral","Date":"Sat, 23 Jul 2022 19:28:38 +0300","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YtwhtlfxaMzPWl/v@pendragon.ideasonboard.com>","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-6-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220718081602.32535-6-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v6 5/8] ipa: raspberrypi: Introduce\n\tversion 2.0 format for the camera tuning file","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]