[{"id":31146,"web_url":"https://patchwork.libcamera.org/comment/31146/","msgid":"<172591833055.2319503.3984464320350410308@ping.linuxembedded.co.uk>","date":"2024-09-09T21:45:30","subject":"Re: [PATCH v6 18/18] libcamera: software_isp: Update black level\n\tonly on exposure changes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-09-06 13:09:27)\n> The black level is likely to get updated, if ever, only after exposure\n> or gain changes. Don't compute its possible updates if exposure and\n> gain are unchanged.\n> \n> It's probably not worth trying to implement something more\n> sophisticated. Better to spend the effort on supporting tuning files.\n> \n\nFine with me. Indeed I think we just go with static/constant black\nlevels when we know more information about the sensor.\n\nI'll be curious to see how close this implementation measures against\nthe 'known' black level values, or measured blacks if we do calibration\nof this.\n\nMy worry on this automatic detection of black levels is what impact\nthere would be if the scene is always bright ... but it hasn't been an\nissue yet - and I'm sure it will continue to develop so I won't impeded\nhere:\n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/blc.cpp | 9 ++++++++-\n> src/ipa/simple/algorithms/blc.h | 4 ++++\n> 2 files changed, 12 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n> index 1ceae85d..5e0a36d9 100644\n> --- a/src/ipa/simple/algorithms/blc.cpp\n> +++ b/src/ipa/simple/algorithms/blc.cpp\n> @@ -30,10 +30,15 @@ int BlackLevel::init(IPAContext &context,\n> \n> void BlackLevel::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> - [[maybe_unused]] IPAFrameContext &frameContext,\n> + IPAFrameContext &frameContext,\n> const SwIspStats *stats,\n> [[maybe_unused]] ControlList &metadata)\n> {\n> + if (frameContext.sensor.exposure == exposure_ &&\n> + frameContext.sensor.gain == gain_) {\n> + return;\n> + }\n> +\n> const SwIspStats::Histogram &histogram = stats->yHistogram;\n> \n> /*\n> @@ -54,6 +59,8 @@ void BlackLevel::process(IPAContext &context,\n> if (seen >= pixelThreshold) {\n> context.activeState.blc.level =\n> static_cast(i) / SwIspStats::kYHistogramSize;\n> + exposure_ = frameContext.sensor.exposure;\n> + gain_ = frameContext.sensor.gain;\n> LOG(IPASoftBL, Debug)\n> << \"Auto-set black level: \"\n> << i << \"/\" << SwIspStats::kYHistogramSize\n> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> index c2140b4b..fc38375e 100644\n> --- a/src/ipa/simple/algorithms/blc.h\n> +++ b/src/ipa/simple/algorithms/blc.h\n> @@ -25,6 +25,10 @@ public:\n> IPAFrameContext &frameContext,\n> const SwIspStats *stats,\n> ControlList &metadata) override;\n> +\n> +private:\n> + uint32_t exposure_;\n> + double gain_;\n> };\n> \n> } /* namespace ipa::soft::algorithms */\n> -- \n> 2.44.1\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 BD5A9BF415\n\tfor ;\n\tMon, 9 Sep 2024 21:45:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6EB40634F7;\n\tMon, 9 Sep 2024 23:45:36 +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 5E116634EB\n\tfor ;\n\tMon, 9 Sep 2024 23:45:34 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DD3AF827;\n\tMon, 9 Sep 2024 23:44:17 +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=\"d48uc1Qx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725918258;\n\tbh=JCYm9XE2P86iACfHLSwykAQsCx8qAcoQ4vHFdlw0J5c=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=d48uc1QxUaHa/j7mSqgh4KPVL4SFgBTup+ONY3+QkMBcSp+fakV04PRFpb/X7P+Xn\n\tkR3NosXtfn2o6EuWmaCtcDHqNRvqWglHeiXgU7cOwbq2sBMw/TyDPSQZErIM/4dnis\n\t3fRivHfvMtr9CCl+M/ifra5tMUdRqS/cuRIqwE4o=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240906120927.4071508-19-mzamazal@redhat.com>","References":"<20240906120927.4071508-1-mzamazal@redhat.com>\n\t<20240906120927.4071508-19-mzamazal@redhat.com>","Subject":"Re: [PATCH v6 18/18] libcamera: software_isp: Update black level\n\tonly on exposure changes","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tUmang Jain ,\n\tLaurent Pinchart ,\n\tDaniel Scally ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Mon, 09 Sep 2024 22:45:30 +0100","Message-ID":"<172591833055.2319503.3984464320350410308@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}},{"id":31280,"web_url":"https://patchwork.libcamera.org/comment/31280/","msgid":"<87ikurlbva.fsf@redhat.com>","date":"2024-09-19T18:47:05","subject":"Re: [PATCH v6 18/18] libcamera: software_isp: Update black level\n\tonly on exposure changes","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham writes:\n\n> Quoting Milan Zamazal (2024-09-06 13:09:27)\n>> The black level is likely to get updated, if ever, only after exposure\n>> or gain changes. Don't compute its possible updates if exposure and\n>\n>> gain are unchanged.\n>> \n>> It's probably not worth trying to implement something more\n>> sophisticated. Better to spend the effort on supporting tuning files.\n>> \n>\n> Fine with me. Indeed I think we just go with static/constant black\n> levels when we know more information about the sensor.\n>\n> I'll be curious to see how close this implementation measures against\n> the 'known' black level values, or measured blacks if we do calibration\n> of this.\n\nIn my environment, it's accurate, within the 256 levels resolution.\nSometimes it doesn't reach the right value immediately but adjusts to it\nvery soon. This isn't representative but I haven't heard complaints\nfrom anybody so far. And the aim is not to substitute datasheets or\ncalibration but to serve as a reasonable fallback.\n\n> My worry on this automatic detection of black levels is what impact\n> there would be if the scene is always bright ... \n\nSessions start with minimum exposure, so hardly anything can be too\nbright initially. If this wasn't the case then I guess the effect would\nbe similar to autoexposure adjustments -- bringing the bright scene\ntowards middle brightness but with more contrast.\n\n> but it hasn't been an issue yet - and I'm sure it will continue to\n> develop so I won't impeded here:\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/blc.cpp | 9 ++++++++-\n>> src/ipa/simple/algorithms/blc.h | 4 ++++\n>> 2 files changed, 12 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n>> index 1ceae85d..5e0a36d9 100644\n>> --- a/src/ipa/simple/algorithms/blc.cpp\n>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>> @@ -30,10 +30,15 @@ int BlackLevel::init(IPAContext &context,\n>> \n>> void BlackLevel::process(IPAContext &context,\n>> [[maybe_unused]] const uint32_t frame,\n>> - [[maybe_unused]] IPAFrameContext &frameContext,\n>> + IPAFrameContext &frameContext,\n>> const SwIspStats *stats,\n>> [[maybe_unused]] ControlList &metadata)\n>> {\n>> + if (frameContext.sensor.exposure == exposure_ &&\n>> + frameContext.sensor.gain == gain_) {\n>> + return;\n>> + }\n>> +\n>> const SwIspStats::Histogram &histogram = stats->yHistogram;\n>> \n>> /*\n>> @@ -54,6 +59,8 @@ void BlackLevel::process(IPAContext &context,\n>> if (seen >= pixelThreshold) {\n>> context.activeState.blc.level =\n>> static_cast(i) / SwIspStats::kYHistogramSize;\n>> + exposure_ = frameContext.sensor.exposure;\n>> + gain_ = frameContext.sensor.gain;\n>> LOG(IPASoftBL, Debug)\n>> << \"Auto-set black level: \"\n>> << i << \"/\" << SwIspStats::kYHistogramSize\n>> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n>> index c2140b4b..fc38375e 100644\n>> --- a/src/ipa/simple/algorithms/blc.h\n>> +++ b/src/ipa/simple/algorithms/blc.h\n>> @@ -25,6 +25,10 @@ public:\n>> IPAFrameContext &frameContext,\n>> const SwIspStats *stats,\n>> ControlList &metadata) override;\n>> +\n>> +private:\n>> + uint32_t exposure_;\n>> + double gain_;\n>> };\n>> \n>> } /* namespace ipa::soft::algorithms */\n>> -- \n>> 2.44.1\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 E1CAAC3257\n\tfor ;\n\tThu, 19 Sep 2024 18:47:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F945634FC;\n\tThu, 19 Sep 2024 20:47:14 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96BD1618E7\n\tfor ;\n\tThu, 19 Sep 2024 20:47:12 +0200 (CEST)","from mail-ej1-f72.google.com (mail-ej1-f72.google.com\n\t[209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-461-fU4mlEgYNYuN-TICHawPkA-1; Thu, 19 Sep 2024 14:47:09 -0400","by mail-ej1-f72.google.com with SMTP id\n\ta640c23a62f3a-a8a9094c96cso76302066b.1\n\tfor ;\n\tThu, 19 Sep 2024 11:47:09 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a90612e50easm761377266b.174.2024.09.19.11.47.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 19 Sep 2024 11:47:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"TEhOdCKk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1726771631;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=+TMOCJ8raN7fjTFHxEGMASJx1NQdrr/0fPnyRL7m5PM=;\n\tb=TEhOdCKkJr7BaHAyTtuZgKBX940XC739Ik0K4CZ0arVZY1pjaX1V+LbfbNkNi5GapU/Iw3\n\tozXjNH0cbnFvZ79zIKKxtmmyVEOYond3x+spEusYsjLn0zFoh/RHc6B17X2FDOWMC7xLm8\n\tlojf6j8bRzlv0BJ0f4vygNfaOIRmNvA=","X-MC-Unique":"fU4mlEgYNYuN-TICHawPkA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726771628; x=1727376428;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=+TMOCJ8raN7fjTFHxEGMASJx1NQdrr/0fPnyRL7m5PM=;\n\tb=CtxoGQowC8zuoQxkquaJ4+QEKC8ZDitGqCnWvyGElMGpgSMWtUAfCcT2/4t+YKkuVW\n\tJJBdnkyixmFXpAOp+eK9o/8UvowZxe0di1Wbr4wn3+cP23/eacf+IHafzxSUm9ESOIxe\n\tR74D//cOLobx4LZNR8RfaPclHS3exyxLXXAbrCyx01DHFbkZ+bICB3W4PHtFeu7yj3P3\n\tNV0U20r+XRGLqd+GsYbYLd0hcnffzsJu0CjvD6/W1FIZ0nCypPk7LS0yajIx+U3nSgdL\n\tDe4mgaAUG+rWyqTsdEgTDmSExAI8BwRBMaT7ZdSu/ybBwGqJUSm4/KUqrEfRMYjcmCVF\n\tkrvQ==","X-Gm-Message-State":"AOJu0YzRiOTTKNier+RjhiFAwtzEquITACyNU8Gesez2W25pThWXAGhi\n\tWmlIgY8Bck2r59Pfif7ODAGdKdUSNewwXn0HEdyDhfAimteuF3m/89P+FqCneSV5MYduX+NSH0e\n\tanxJ+K5085evVIosGzWbxLUZTTKhhl2VRJVexBcjNl0iXqPa1nXc/c9jU42Hlaq+ZaZaDrAE=","X-Received":["by 2002:a17:907:2d8f:b0:a86:a481:248c with SMTP id\n\ta640c23a62f3a-a90d56dc54emr7182666b.19.1726771628081; \n\tThu, 19 Sep 2024 11:47:08 -0700 (PDT)","by 2002:a17:907:2d8f:b0:a86:a481:248c with SMTP id\n\ta640c23a62f3a-a90d56dc54emr7181366b.19.1726771627689; \n\tThu, 19 Sep 2024 11:47:07 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG6ZHg8OF0L/WwG6WIX+OILL5C7qRtrQIggVfQ9/slwABWhwU7sTpRN1J57v44Iyjmquw9VDw==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain\n\t, Laurent Pinchart\n\t, Daniel Scally\n\t","Subject":"Re: [PATCH v6 18/18] libcamera: software_isp: Update black level\n\tonly on exposure changes","In-Reply-To":"<172591833055.2319503.3984464320350410308@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Mon, 09 Sep 2024 22:45:30 +0100\")","References":"<20240906120927.4071508-1-mzamazal@redhat.com>\n\t<20240906120927.4071508-19-mzamazal@redhat.com>\n\t<172591833055.2319503.3984464320350410308@ping.linuxembedded.co.uk>","Date":"Thu, 19 Sep 2024 20:47:05 +0200","Message-ID":"<87ikurlbva.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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\" "}}]