[{"id":30292,"web_url":"https://patchwork.libcamera.org/comment/30292/","msgid":"","date":"2024-07-04T08:52:02","subject":"Re: [PATCH v3 3/6] ipa: rkisp1: blc: Query black levels from camera\n\tsensor helper","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Jul 03, 2024 at 03:49:51PM +0200, Stefan Klug wrote:\n> As the camera sensor helper now has the ability to provide the black\n> level, use it. Black levels can still be overwritten by the tuning\n> file, but the direction is to remove them from the tuning files and move\n> them into the sensor helpers.\n> \n> Additionally interpret all values based on 16bits. The conversion to the\n> scale required by the hardware is done in process(). It ensures all the\n> values inside libcamera are the same scale and is in preparation for the\n> i.MX8MP where black levels are based on a 20bit scale. Note that this\n> breaks existing tuning files. The tuning files distributed with\n> libcamera will be fixed in a later patch.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/rkisp1/algorithms/blc.cpp | 54 ++++++++++++++++++++++++++-----\n> 1 file changed, 46 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index d2e743541c99..b17b549c55df 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -46,10 +46,47 @@ BlackLevelCorrection::BlackLevelCorrection()\n> int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,\n> \t\t\t const YamlObject &tuningData)\n> {\n> -\tblackLevelRed_ = tuningData[\"R\"].get(256);\n> -\tblackLevelGreenR_ = tuningData[\"Gr\"].get(256);\n> -\tblackLevelGreenB_ = tuningData[\"Gb\"].get(256);\n> -\tblackLevelBlue_ = tuningData[\"B\"].get(256);\n> +\tstd::optional levelRed = tuningData[\"R\"].get();\n> +\tstd::optional levelGreenR = tuningData[\"Gr\"].get();\n> +\tstd::optional levelGreenB = tuningData[\"Gb\"].get();\n> +\tstd::optional levelBlue = tuningData[\"B\"].get();\n> +\tbool tuningHasLevels = levelRed && levelGreenR && levelGreenB && levelBlue;\n> +\n> +\tauto blackLevel = context.camHelper->blackLevel();\n> +\tif (!blackLevel) {\n> +\t\t/*\n> +\t\t * Not all camera sensor helpers have been updated with black\n> +\t\t * levels. Print a warning and fall back to the levels from the\n> +\t\t * tuning data to preserve backward compatibility. This should\n> +\t\t * be removed once all helpers provide the data.\n> +\t\t */\n> +\t\tLOG(RkISP1Blc, Warning)\n> +\t\t\t<< \"No black levels provided by camera sensor helper\"\n> +\t\t\t<< \", please fix\";\n> +\n> +\t\tblackLevelRed_ = levelRed.value_or(4096);\n> +\t\tblackLevelGreenR_ = levelGreenR.value_or(4096);\n> +\t\tblackLevelGreenB_ = levelGreenB.value_or(4096);\n> +\t\tblackLevelBlue_ = levelBlue.value_or(4096);\n> +\t} else if (tuningHasLevels) {\n> +\t\t/*\n> +\t\t * If black levels are provided in the tuning file, use them to\n> +\t\t * avoid breaking existing camera tuning. This is deprecated and\n> +\t\t * will be removed.\n> +\t\t */\n> +\t\tLOG(RkISP1Blc, Warning)\n> +\t\t\t<< \"Deprecated: black levels overwritten by tuning file\";\n> +\n> +\t\tblackLevelRed_ = *levelRed;\n> +\t\tblackLevelGreenR_ = *levelGreenR;\n> +\t\tblackLevelGreenB_ = *levelGreenB;\n> +\t\tblackLevelBlue_ = *levelBlue;\n> +\t} else {\n> +\t\tblackLevelRed_ = *blackLevel;\n> +\t\tblackLevelGreenR_ = *blackLevel;\n> +\t\tblackLevelGreenB_ = *blackLevel;\n> +\t\tblackLevelBlue_ = *blackLevel;\n> +\t}\n> \n> \ttuningParameters_ = true;\n> \n> @@ -77,10 +114,11 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> \t\treturn;\n> \n> \tparams->others.bls_config.enable_auto = 0;\n> -\tparams->others.bls_config.fixed_val.r = blackLevelRed_;\n> -\tparams->others.bls_config.fixed_val.gr = blackLevelGreenR_;\n> -\tparams->others.bls_config.fixed_val.gb = blackLevelGreenB_;\n> -\tparams->others.bls_config.fixed_val.b = blackLevelBlue_;\n> +\t/* The rkisp1 uses 12bit based black levels. Scale down accordingly. */\n> +\tparams->others.bls_config.fixed_val.r = blackLevelRed_ >> 4;\n> +\tparams->others.bls_config.fixed_val.gr = blackLevelGreenR_ >> 4;\n> +\tparams->others.bls_config.fixed_val.gb = blackLevelGreenB_ >> 4;\n> +\tparams->others.bls_config.fixed_val.b = blackLevelBlue_ >> 4;\n> \n> \tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_BLS;\n> -- \n> 2.43.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 18DE5BD87C\n\tfor ;\n\tThu, 4 Jul 2024 08:52:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9638362E22;\n\tThu, 4 Jul 2024 10:52:12 +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 1ADEB619C4\n\tfor ;\n\tThu, 4 Jul 2024 10:52:11 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CB6A0541;\n\tThu, 4 Jul 2024 10:51:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WCecTnte\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720083102;\n\tbh=urpsMjFDFJx9P4L8GEvIK5H83RCNav1wW0+ejIK+ZFM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WCecTnteOgitaXmgKzacX2I+5Ts/kRoXDpQYR6P2Y7fYmSTlNKdZd8BrGYGYXN3r8\n\tts0EIJo3baFlB1o8o4Xq76ZIYTxyAy/7Wa2LguBNyzC/a0IluHC2uNYFLNpscqKNf0\n\t63gFfbbebrEmcGjPALLxRVPr817lWC4hXpRtXg00=","Date":"Thu, 4 Jul 2024 17:52:02 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tLaurent Pinchart ","Subject":"Re: [PATCH v3 3/6] ipa: rkisp1: blc: Query black levels from camera\n\tsensor helper","Message-ID":"","References":"<20240703135000.242227-1-stefan.klug@ideasonboard.com>\n\t<20240703135000.242227-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240703135000.242227-4-stefan.klug@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]