[{"id":19006,"web_url":"https://patchwork.libcamera.org/comment/19006/","msgid":"","date":"2021-08-23T17:01:55","subject":"Re: [libcamera-devel] [PATCH v1 7/7] ipa: ipu3: 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 Mon, Aug 23, 2021 at 02:49:37PM +0200, Jean-Michel Hautbois wrote:\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\n> or below this level should be considered black.\n\nThat's a bit misleading, I read it as a need to clamp the value of\npixels below a certain threshold to 0, while what we need it subtracting\nan offset from all pixels.\n\n> This can be taken into\n> account by reading the lowest value of a special region on sensors which\n> is not exposed to the lens. This provides a scaling factor to be able to\n\nScaling is a multiplication.\n\n> adjust the expected black levels in the resultant images.\n> \n> For a camera outputting 10-bit pixel values (in the range 0 to 1023) a\n> typical black level might be 64, but the correct value would be found in\n> the sensor manaufacturer’s datasheet.\n\ns/manaufacturer/manufacturer/\n\nexcept that it's not found in datasheets, especially given that it\nvaries based on the temperature (among other things).\n\n64 isn't a \"typical\" value. It's a fixed value (how did you obtain it\nbtw ?) that... we consider good enough as a very first approximation,\nuntil we measure it during a tuning process and include it in a\nconfiguration file ?\n\n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/algorithms/awb.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 294871b1..ad3784d3 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -366,9 +366,17 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> \t/* The CCM matrix may change when color temperature will be used */\n> \tparams->acc_param.ccm = imguCssCcmDefault;\n> \n> +\t/* The Optical Black Level correction values */\n\nThis deserves a todo comment to explain this is an initial value that\nneeds to be moved to a configuration file.\n\n> +\tparams->obgrid_param.gr = 64;\n> +\tparams->obgrid_param.r = 63;\n> +\tparams->obgrid_param.b = 63;\n> +\tparams->obgrid_param.gb = 64;\n\nWhy 64/63 ?\n\nDoes the unit of the value depend on the bpp of the sensor (that is, the\nvalue will be subtracted from the pixel value without being scaled\nfirst), or is it expressed in a fixed unit (for instance 10, 12 or\n16bpp, in which case it will be scaled by the ImgU before subtraction) ?\n\n> +\n> \tparams->use.acc_awb = 1;\n> \tparams->use.acc_bnr = 1;\n> \tparams->use.acc_ccm = 1;\n> +\tparams->use.obgrid = 1;\n> +\tparams->use.obgrid_param = 1;\n> }\n> \n> } /* namespace ipa::ipu3::algorithms */","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 BB7E1BD87C\n\tfor ;\n\tMon, 23 Aug 2021 17:02:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D0F9688A6;\n\tMon, 23 Aug 2021 19:02:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E59536025B\n\tfor ;\n\tMon, 23 Aug 2021 19:02:05 +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 6F6092A5;\n\tMon, 23 Aug 2021 19:02:05 +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=\"BFL2KWPy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629738125;\n\tbh=sha8HwQrlmX04GXZr5Ru+XTzfEJNISMgyVzeKMKtwig=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BFL2KWPyy3T4DraVMs4tt72hn7rCDkdUxfBCVU5NGFI/ClVtQmCLErRAUUg651dsA\n\tsk92xJI+/Tf9TgoQ4MwkMeJvdwtaje2khD7IlA1B5Njzaba/idemB5jI0YTalkGdyi\n\trf0hcgixrfRBiNuIvdzKlGiFHLU3y3Uz2+m1sRsA=","Date":"Mon, 23 Aug 2021 20:01:55 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210823124937.253539-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210823124937.253539-8-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210823124937.253539-8-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 7/7] ipa: ipu3: 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\" "}}]