[{"id":23422,"web_url":"https://patchwork.libcamera.org/comment/23422/","msgid":"<Yqr4P5NcA87i8sGB@pendragon.ideasonboard.com>","date":"2022-06-16T09:30:39","subject":"Re: [libcamera-devel] [PATCH v2 3/5] ipa: rkisp1: Add Yaml tuning\n\tfile support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Florian,\n\nThank you for the patch.\n\nOn Thu, Jun 16, 2022 at 10:07:42AM +0200, Florian Sylvestre via libcamera-devel wrote:\n> Retrieve root node in Yaml tuning file and provide to\n> each algorithm this YamlObject to allow them to grab their default\n> parameters by calling init() function.\n> \n> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>\n> ---\n>  src/ipa/rkisp1/algorithms/algorithm.h |  4 ++-\n>  src/ipa/rkisp1/rkisp1.cpp             | 35 +++++++++++++++++++++++++++\n>  2 files changed, 38 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h\n> index d46c3188..5b95fd30 100644\n> --- a/src/ipa/rkisp1/algorithms/algorithm.h\n> +++ b/src/ipa/rkisp1/algorithms/algorithm.h\n> @@ -17,9 +17,11 @@\n>  \n>  namespace libcamera {\n>  \n> +class YamlObject;\n> +\n>  namespace ipa::rkisp1 {\n>  \n> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>;\n> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, YamlObject, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>;\n\nI think this needs to be part of patch 2/5, otherwise patch 2/5 will\nfail to compile.\n\nBtw, could you rebase on top of the latest master branch ? There's a\nconflict here.\n\n>  \n>  } /* namespace ipa::rkisp1 */\n>  \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 23cb95b5..6cc501a9 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -14,6 +14,7 @@\n>  #include <linux/rkisp1-config.h>\n>  #include <linux/v4l2-controls.h>\n>  \n> +#include <libcamera/base/file.h>\n>  #include <libcamera/base/log.h>\n>  \n>  #include <libcamera/control_ids.h>\n> @@ -24,6 +25,7 @@\n>  #include <libcamera/request.h>\n>  \n>  #include <libcamera/internal/mapped_framebuffer.h>\n> +#include <libcamera/internal/yaml_parser.h>\n>  \n>  #include \"algorithms/agc.h\"\n>  #include \"algorithms/algorithm.h\"\n> @@ -61,6 +63,7 @@ public:\n>  private:\n>  \tvoid setControls(unsigned int frame);\n>  \tvoid prepareMetadata(unsigned int frame, unsigned int aeState);\n> +\tint parseConfigurationFile(File &file);\n>  \n>  \tstd::map<unsigned int, FrameBuffer> buffers_;\n>  \tstd::map<unsigned int, MappedFrameBuffer> mappedBuffers_;\n> @@ -126,6 +129,38 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n>  \talgorithms_.push_back(std::make_unique<algorithms::Awb>());\n>  \talgorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());\n>  \n> +\t/* Load the tuning file for this sensor. */\n> +\tFile file(settings.configurationFile.c_str());\n> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n> +\t\tint ret = file.error();\n> +\t\tLOG(IPARkISP1, Error) << \"Failed to open configuration file \"\n> +\t\t\t\t      << settings.configurationFile << \": \" << strerror(-ret);\n\nThe usual style to shorten lines is\n\n\t\tLOG(IPARkISP1, Error)\n\t\t\t<< \"Failed to open configuration file \"\n\t\t\t<< settings.configurationFile << \": \" << strerror(-ret);\n\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tint ret = parseConfigurationFile(file);\n> +\tif (ret)\n> +\t\treturn -EINVAL;\n\n\t\treturn ret;\n\n> +\n> +\treturn 0;\n\nYou could replace all this with\n\n\treturn parseConfigurationFile(file);\n\nUp to you.\n\n> +}\n> +\n> +int IPARkISP1::parseConfigurationFile(File &file)\n> +{\n> +\tstd::unique_ptr<libcamera::YamlObject> root = YamlParser::parse(file);\n> +\tif (!root)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (!root->isDictionary())\n> +\t\treturn -EINVAL;\n> +\n> +\t/* Allow each algo to get parameters from configuration file. */\n> +\tfor (auto const &algo : algorithms_) {\n> +\t\tint ret = algo->init(context_, root.get());\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n>  \treturn 0;\n>  }\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 8370EBD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 16 Jun 2022 09:30:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBB1565631;\n\tThu, 16 Jun 2022 11:30:52 +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 A773B601F1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Jun 2022 11:30:50 +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 164C4415;\n\tThu, 16 Jun 2022 11:30:50 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655371852;\n\tbh=xVrAxYFLPue236HC9TFx3ggAxwWQhiIQCIwS4jlowGM=;\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=NY8kzvaGvmKJimJyFNJZrwwM9+w6wX4CaABMb7dCqPRF4a62TgowtF6SNjtrNvjey\n\tJIs81grdhQaSK7n8A5OoACG1uqNiK9Z/v4MIpgDYERhj+WQ8O74KGcGsA63E7LelcG\n\tVVGOf21StDNp2ZhwHtzzbtiY3mfO7Iktp5dOi/NaqhtUYQTP9G7ARoYYUbnD4NmsFI\n\tikD6fX5mByDiDG2n7IxydXK/Y/up7+9ZMt2KVMyuuR88XVy07d3rKLtEnUqDVdFC5M\n\t8+aRT6hiHCh3GjLwf+QZvXfHQz+A/z+lffgOdQoVEM5+40EVOgg9yLfr8kUpWnhKEy\n\tVdwwby9pNoc/g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655371850;\n\tbh=xVrAxYFLPue236HC9TFx3ggAxwWQhiIQCIwS4jlowGM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MYm0JaAyQrohwZq6xyWW39FOuLwjejzkhCe7+WXN+l0RppF5O87Dnf6y2dwFm1OB9\n\tTrGjv/2wood7RpGRC3kAWd/YVSnwnTbVqwEEYwvK5h6Ba92tp5ZXwJf1sl8CVswaI8\n\tDFbfPzUVEi9W+nOHaJmUuiCZroX+DLxcBdYABecg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MYm0JaAy\"; dkim-atps=neutral","Date":"Thu, 16 Jun 2022 12:30:39 +0300","To":"Florian Sylvestre <fsylvestre@baylibre.com>","Message-ID":"<Yqr4P5NcA87i8sGB@pendragon.ideasonboard.com>","References":"<20220616080744.548995-1-fsylvestre@baylibre.com>\n\t<20220616080744.548995-4-fsylvestre@baylibre.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220616080744.548995-4-fsylvestre@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] ipa: rkisp1: Add Yaml tuning\n\tfile support","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>"}}]