[{"id":22188,"web_url":"https://patchwork.libcamera.org/comment/22188/","msgid":"","date":"2022-02-23T14:51:57","subject":"Re: [libcamera-devel] [PATCH v2 2/4] ipa: rkisp1: Introduce Black\n\tLevel Correction","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Wed, Feb 23, 2022 at 11:48:22AM +0100, Jean-Michel Hautbois wrote:\n> In order to have the proper pixel levels, apply a fixed black level\n> correction, based on the imx219 tuning file in RPi. The value is 4096 on\n> 16 bits, and the pipeline for RkISP1 is on 12 bits, scale it.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Tested-by: Peter Griffin \n> ---\n> src/ipa/rkisp1/algorithms/blc.cpp | 57 +++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/blc.h | 30 ++++++++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 2 +\n> 4 files changed, 90 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/blc.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/blc.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> new file mode 100644\n> index 00000000..788953e3\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -0,0 +1,57 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * blc.cpp - RkISP1 Black Level Correction control\n> + */\n> +\n> +#include \"blc.h\"\n> +\n> +/**\n> + * \\file blc.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class BlackLevelCorrection\n> + * \\brief RkISP1 Black Level Correction control\n> + *\n> + * The pixels output by the camera normally include a black level, because\n> + * sensors do not always report a signal level of '0' for black. Pixels at or\n> + * below this level should be considered black. To achieve that, the RkISP BLC\n> + * algorithm subtracts a configurable offset from all pixels.\n> + *\n> + * The black level can be measured at runtime from an optical dark region of the\n> + * camera sensor, or measured during the camera tuning process. The first option\n> + * isn't currently supported.\n> + */\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void BlackLevelCorrection::prepare(IPAContext &context,\n> +\t\t\t\t rkisp1_params_cfg *params)\n> +{\n\tif (context.frameContext.frameId != 0)\n\t\treturn;\n\nas there's no need to set params->others.bls_config.* either in that\ncase.\n\nWe may want to improve the Algorithm class interface to simplify\none-time settings, but that's OK for now.\n\nThere's a potential issue here for the future though. At the moment the\nframeId starts from 0 and is sequential, as the pipeline handler doesn't\ntie it to the sequence number provided by the driver. That may change\nlater, in order to inform the IPA about lost frames. If the first frame\nis dropped, we'll then never run this. It would be nice to record this\nissue somewhere.\n\nThere's another issue: this function is called when requests are queued,\nwhile the frameId is stored in the context later, when statistics are\nready. The first few prepare() calls will have frameId uninitialized.\n\n> +\t/*\n> +\t * Substract fixed values taken from imx219 tuning file.\n> +\t * \\todo Use a configuration file for it ?\n> +\t */\n> +\tparams->others.bls_config.enable_auto = 0;\n> +\tparams->others.bls_config.fixed_val.r = 256;\n> +\tparams->others.bls_config.fixed_val.gr = 256;\n> +\tparams->others.bls_config.fixed_val.gb = 256;\n> +\tparams->others.bls_config.fixed_val.b = 256;\n> +\n> +\tif (context.frameContext.frameId == 0) {\n> +\t\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> +\t\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_BLS;\n> +\t\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> +\t}\n> +}\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> new file mode 100644\n> index 00000000..331a2209\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/blc.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * blc.h - RkISP1 Black Level Correction control\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +struct IPACameraSensorInfo;\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class BlackLevelCorrection : public Algorithm\n> +{\n> +public:\n> +\tBlackLevelCorrection() = default;\n> +\t~BlackLevelCorrection() = default;\n> +\n> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index a19c1a4f..27c97731 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -2,4 +2,5 @@\n> \n> rkisp1_ipa_algorithms = files([\n> 'agc.cpp',\n> + 'blc.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 732ca2bb..f82b7cb3 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -27,6 +27,7 @@\n> \n> #include \"algorithms/agc.h\"\n> #include \"algorithms/algorithm.h\"\n> +#include \"algorithms/blc.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> #include \"ipa_context.h\"\n> @@ -125,6 +126,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> \n> \t/* Construct our Algorithms */\n> \talgorithms_.push_back(std::make_unique());\n> +\talgorithms_.push_back(std::make_unique());\n> \n> \treturn 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 1B3F4BE08A\n\tfor ;\n\tWed, 23 Feb 2022 14:52:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79BDE610B3;\n\tWed, 23 Feb 2022 15:52:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78E8A601FF\n\tfor ;\n\tWed, 23 Feb 2022 15:52:08 +0100 (CET)","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 E1100DD;\n\tWed, 23 Feb 2022 15:52:07 +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=\"WqrwXK/t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1645627928;\n\tbh=HS3S1mDv90MZYNGGw3TOAGjGSC59qI1z545eHBeQXik=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WqrwXK/tKWh4qESGYBeZk3l5344I3QrdZmpjhbGadI8nhbf1NKUDY0imz0ExWvAnL\n\tqwVts3Zjl7ERBqGJul0RRslkmSmXrb4K5lnapLR+TB9mwzJ2fdvb5Qck/nAJMaZUTg\n\tn3hrIUo0GgunTPUzkZG5CSi6viIDxkAphu6uIIGU=","Date":"Wed, 23 Feb 2022 16:51:57 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220223104824.25723-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220223104824.25723-3-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220223104824.25723-3-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] ipa: rkisp1: Introduce Black\n\tLevel Correction","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\" "}}]