[{"id":19657,"web_url":"https://patchwork.libcamera.org/comment/19657/","msgid":"","date":"2021-09-14T03:55:49","subject":"Re: [libcamera-devel] [PATCH 10/11] ipa: ipu3: tonemapping:\n\tGenerate the LUT only on gamma change","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, Sep 13, 2021 at 04:58:09PM +0200, Jean-Michel Hautbois wrote:\n> The Tonemapping algorithm calculates the gamma curve for every frame,\n\ns/Tonemapping/tone mapping/\n\n> regardless of whether the gamma value has changed or not. This issue is\n> exasperated as we currently hardcode the gamma to a single value.\n> \n> Optimise the implementation to only recalculate the look up table when\n> the gamma setting is changed, and store the gamma setting of the LUT\n> curve as part of the IPA context.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +++++\n> src/ipa/ipu3/ipa_context.h | 1 +\n> src/ipa/ipu3/ipu3.cpp | 3 +++\n> 3 files changed, 9 insertions(+)\n> \n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index 3af96261..40337f9d 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -43,6 +43,9 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,\n> \t */\n> \tgamma_ = 1.1;\n> \n> +\tif (context.frameContext.toneMapping.gamma == gamma_)\n> +\t\treturn;\n\nWe currently clear the context in IPAIPU3::configure() with\n\n\tcontext_ = {};\n\nThis ensures that the check above will never be true for the first\nframe, but it feels like it would probably be best to let algorithms\nperform the initial configuration. Should we implement\nToneMapping::configure() and set context.frameContext.toneMapping.gamma\nto 0.0 there ?\n\n> +\n> \tstruct ipu3_uapi_gamma_corr_lut &lut =\n> \t\tcontext.frameContext.toneMapping.gammaCorrection;\n> \n> @@ -53,6 +56,8 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,\n> \t\t/* The output value is expressed on 13 bits. */\n> \t\tlut.lut[i] = gamma * 8191;\n> \t}\n> +\n> +\tcontext.frameContext.toneMapping.gamma = gamma_;\n> }\n> \n> } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 9d9444dc..bffa7634 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -38,6 +38,7 @@ struct IPAFrameContext {\n> \t} awb;\n> \n> \tstruct {\n> +\t\tdouble gamma;\n> \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n> \t} toneMapping;\n> };\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 5b487dcd..f76627f6 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -129,6 +129,9 @@\n> * \\struct IPAFrameContext::toneMapping\n> * \\brief Context for ToneMapping and Gamma control\n> *\n> + * \\var IPAFrameContext::toneMapping::gamma\n> + * \\brief Gamma value for the LUT\n> + *\n> * \\var IPAFrameContext::toneMapping::gammaCorrection\n> * \\brief Per-pixel tone mapping implemented as a LUT\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 5D9FDBDC71\n\tfor ;\n\tTue, 14 Sep 2021 03:56:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FC1369189;\n\tTue, 14 Sep 2021 05:56:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 655E06916F\n\tfor ;\n\tTue, 14 Sep 2021 05:56:14 +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 D0BB82A5;\n\tTue, 14 Sep 2021 05:56:13 +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=\"nW6vzgvP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631591774;\n\tbh=XSpCQwKkeXoLITPTzQxgICwSRM2ZdjIfj6GHuCRmaVc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nW6vzgvP/ful7AzknI+WEe3yfHszRxywnJYoHK+xwj8hZxHMP3yFcoR3oZ5+/5gLR\n\tFujs6o07RkoJYc1XRQbuaF6vsGbX9pzhtzjbfEejqrTi2ZuhkWnNsOt+xQDFCdYr4E\n\tb7sRLyWV6NHDH0Leqm/34d2weWjPWWdwhAUMpRvg=","Date":"Tue, 14 Sep 2021 06:55:49 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210913145810.66515-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210913145810.66515-11-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210913145810.66515-11-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 10/11] ipa: ipu3: tonemapping:\n\tGenerate the LUT only on gamma change","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\" "}}]