[{"id":32475,"web_url":"https://patchwork.libcamera.org/comment/32475/","msgid":"<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>","date":"2024-11-29T23:07:01","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Quickly retested on top of latest master, including \"libcamera: \nsoftware_isp: Initialize exposure+gain before agc calculations\"\n\nTested-by: Robert Mader <robert.mader@collabora.com>\n\nOn 08.11.24 10:43, Milan Zamazal wrote:\n> The black level obtained from the tuning file in software ISP is\n> retrieved in init (because this is the standard algorithm method with\n> access to tuning data) and stored into context.  But the context gets\n> reset in configure and the black level is lost and never applied.\n>\n> Let's store the black level from the tuning file into an algorithm\n> instance variable and put it into the context only later in configure.\n> This is similar to what rkisp1 IPA does with the values obtained from\n> the tuning file.\n>\n> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera: software_isp: Clear IPA context on configure and stop\")\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>   src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>   src/ipa/simple/algorithms/blc.h   | 4 ++++\n>   src/ipa/simple/soft_simple.cpp    | 3 +--\n>   3 files changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n> index b4e32fe1c..1d7d370b5 100644\n> --- a/src/ipa/simple/algorithms/blc.cpp\n> +++ b/src/ipa/simple/algorithms/blc.cpp\n> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>   {\n>   }\n>   \n> -int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)\n> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n> +\t\t     const YamlObject &tuningData)\n>   {\n>   \tauto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>   \tif (blackLevel.has_value()) {\n> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)\n>   \t\t * Convert 16 bit values from the tuning file to 8 bit black\n>   \t\t * level for the SoftISP.\n>   \t\t */\n> -\t\tcontext.configuration.black.level = blackLevel.value() >> 8;\n> +\t\tdefinedLevel_ = blackLevel.value() >> 8;\n>   \t}\n>   \treturn 0;\n>   }\n> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)\n>   int BlackLevel::configure(IPAContext &context,\n>   \t\t\t  [[maybe_unused]] const IPAConfigInfo &configInfo)\n>   {\n> +\tif (definedLevel_.has_value())\n> +\t\tcontext.configuration.black.level = definedLevel_;\n>   \tcontext.activeState.blc.level =\n>   \t\tcontext.configuration.black.level.value_or(255);\n>   \treturn 0;\n> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> index 2cf2a8774..453123d27 100644\n> --- a/src/ipa/simple/algorithms/blc.h\n> +++ b/src/ipa/simple/algorithms/blc.h\n> @@ -7,6 +7,9 @@\n>   \n>   #pragma once\n>   \n> +#include <optional>\n> +#include <stdint.h>\n> +\n>   #include \"algorithm.h\"\n>   \n>   namespace libcamera {\n> @@ -29,6 +32,7 @@ public:\n>   private:\n>   \tuint32_t exposure_;\n>   \tdouble gain_;\n> +\tstd::optional<uint8_t> definedLevel_;\n>   };\n>   \n>   } /* namespace ipa::soft::algorithms */\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index c8ad55a21..825c06757 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>   \t\t\t(context_.configuration.agc.againMax -\n>   \t\t\t context_.configuration.agc.againMin) /\n>   \t\t\t100.0;\n> -\t\tif (!context_.configuration.black.level.has_value() &&\n> -\t\t    camHelper_->blackLevel().has_value()) {\n> +\t\tif (camHelper_->blackLevel().has_value()) {\n>   \t\t\t/*\n>   \t\t\t * The black level from camHelper_ is a 16 bit value, software ISP\n>   \t\t\t * works with 8 bit pixel values, both regardless of the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D8C5ABE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 29 Nov 2024 23:07:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD92966048;\n\tSat, 30 Nov 2024 00:07:09 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3346265898\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Nov 2024 00:07:07 +0100 (CET)","by mx.zohomail.com with SMTPS id 1732921623733343.4579387790967;\n\tFri, 29 Nov 2024 15:07:03 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"JwVufFeJ\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1732921624; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=YP/767IMbaUFzxvQ3HH6hX7IO31R6OhRbvd8cWuJz1WFemls5E6y4p6/2Cb3gb7o1yGJZ1L2CIIUtCHFCMn1mVzWGO+EcByDkGWgpaTFITmqhj+eKxTc8e1MtArLkxWVRB9S4WCbpM1vJZ2B+ei9o8wtXGPHqtcGtdxJ85lbF5A=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1732921624;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=YrPVn9+QwWgZRc4daBZmI/lFGE+kl9nbHSTYriJydOA=; \n\tb=UkWOdrBgRfL3vi4vbfrLq/SVicx3IRJnM7rGQj0uRNH3dx781mAsutMUvtoUt46TPKVwwAWN0MilYnGiDyHql8e/91j4lu10dm7r6NurtOckP8q5aLIcVQx4QPPrEageGb0DumE8ZJ5ZUsSyq5xdQHsSokEeJc9q7lPY/o2wAO4=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1732921624;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=YrPVn9+QwWgZRc4daBZmI/lFGE+kl9nbHSTYriJydOA=;\n\tb=JwVufFeJkUO1R6/BRwfwvfHoLNwo1rfzUxzGhLkT1S3lZ8XyAhAfzlqHb3E0hmQs\n\t+qu5J/l/eZ/veLqCQt9U/3q5q8713ai6BIqQrdMnuXiKsVT56F1shvln2eAolfLycte\n\tkiR100G+gOh7Xej27UIrC0XqxULKFHkbO3MyJXsE=","Message-ID":"<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>","Date":"Sat, 30 Nov 2024 00:07:01 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>","Content-Language":"en-US","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<20241108094333.332643-2-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32476,"web_url":"https://patchwork.libcamera.org/comment/32476/","msgid":"<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>","date":"2024-11-29T23:17:35","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Wait, sorry, take that back. This commit is *not* needed any more. \nBlacklevels from tuning files now get applied on master.\n\nOn 30.11.24 00:07, Robert Mader wrote:\n> Quickly retested on top of latest master, including \"libcamera: \n> software_isp: Initialize exposure+gain before agc calculations\"\n>\n> Tested-by: Robert Mader <robert.mader@collabora.com>\n>\n> On 08.11.24 10:43, Milan Zamazal wrote:\n>> The black level obtained from the tuning file in software ISP is\n>> retrieved in init (because this is the standard algorithm method with\n>> access to tuning data) and stored into context.  But the context gets\n>> reset in configure and the black level is lost and never applied.\n>>\n>> Let's store the black level from the tuning file into an algorithm\n>> instance variable and put it into the context only later in configure.\n>> This is similar to what rkisp1 IPA does with the values obtained from\n>> the tuning file.\n>>\n>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera: \n>> software_isp: Clear IPA context on configure and stop\")\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>   src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>>   src/ipa/simple/algorithms/blc.h   | 4 ++++\n>>   src/ipa/simple/soft_simple.cpp    | 3 +--\n>>   3 files changed, 10 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/ipa/simple/algorithms/blc.cpp \n>> b/src/ipa/simple/algorithms/blc.cpp\n>> index b4e32fe1c..1d7d370b5 100644\n>> --- a/src/ipa/simple/algorithms/blc.cpp\n>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>>   {\n>>   }\n>>   -int BlackLevel::init(IPAContext &context, const YamlObject \n>> &tuningData)\n>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n>> +             const YamlObject &tuningData)\n>>   {\n>>       auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>>       if (blackLevel.has_value()) {\n>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const \n>> YamlObject &tuningData)\n>>            * Convert 16 bit values from the tuning file to 8 bit black\n>>            * level for the SoftISP.\n>>            */\n>> -        context.configuration.black.level = blackLevel.value() >> 8;\n>> +        definedLevel_ = blackLevel.value() >> 8;\n>>       }\n>>       return 0;\n>>   }\n>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const \n>> YamlObject &tuningData)\n>>   int BlackLevel::configure(IPAContext &context,\n>>                 [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>   {\n>> +    if (definedLevel_.has_value())\n>> +        context.configuration.black.level = definedLevel_;\n>>       context.activeState.blc.level =\n>>           context.configuration.black.level.value_or(255);\n>>       return 0;\n>> diff --git a/src/ipa/simple/algorithms/blc.h \n>> b/src/ipa/simple/algorithms/blc.h\n>> index 2cf2a8774..453123d27 100644\n>> --- a/src/ipa/simple/algorithms/blc.h\n>> +++ b/src/ipa/simple/algorithms/blc.h\n>> @@ -7,6 +7,9 @@\n>>     #pragma once\n>>   +#include <optional>\n>> +#include <stdint.h>\n>> +\n>>   #include \"algorithm.h\"\n>>     namespace libcamera {\n>> @@ -29,6 +32,7 @@ public:\n>>   private:\n>>       uint32_t exposure_;\n>>       double gain_;\n>> +    std::optional<uint8_t> definedLevel_;\n>>   };\n>>     } /* namespace ipa::soft::algorithms */\n>> diff --git a/src/ipa/simple/soft_simple.cpp \n>> b/src/ipa/simple/soft_simple.cpp\n>> index c8ad55a21..825c06757 100644\n>> --- a/src/ipa/simple/soft_simple.cpp\n>> +++ b/src/ipa/simple/soft_simple.cpp\n>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo \n>> &configInfo)\n>>               (context_.configuration.agc.againMax -\n>>                context_.configuration.agc.againMin) /\n>>               100.0;\n>> -        if (!context_.configuration.black.level.has_value() &&\n>> -            camHelper_->blackLevel().has_value()) {\n>> +        if (camHelper_->blackLevel().has_value()) {\n>>               /*\n>>                * The black level from camHelper_ is a 16 bit value, \n>> software ISP\n>>                * works with 8 bit pixel values, both regardless of \n>> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4E140C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 29 Nov 2024 23:17:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 30EE066048;\n\tSat, 30 Nov 2024 00:17:43 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE5E765898\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Nov 2024 00:17:40 +0100 (CET)","by mx.zohomail.com with SMTPS id 17329222577710.7902395163170013; \n\tFri, 29 Nov 2024 15:17:37 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"a8kaRJX5\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1732922258; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=Q7EraT3/ed7dkFXxOpX0v+o3cFdN/+mHnfMH0/wpEBrlFQLI/i1tJ6qKgnAl2QaOcC5QVILGsKwFdN8JLpk3dtW/y/OngQ1I0yLlFkdtovQ0vaZdmgj8o3W2uXVkw/HIA+1Ev6onXi1/xrGQzr1MA0olb1hnNyBjVYF/oXiyIqg=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1732922258;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=XgmmTLa/jesBm1sUUCL4YGqbq5cZi1kh2VzYC2NP4+E=; \n\tb=BYCq4y6J3r3oo+XlTLj7iiNG/EhRaJdg7pjLEyWpl8R4+mWeQByqAtHdP/QcSr7mZJwzs7FfinBeNqGRmlOShYDZAXEMM34gixlP/ex+c4d3Cj/enoj40LYHaPhzebsMJXE9MERVKP5m3I7+tse4OEEifQnAlsZemnWYWTlLKmU=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1732922258;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=XgmmTLa/jesBm1sUUCL4YGqbq5cZi1kh2VzYC2NP4+E=;\n\tb=a8kaRJX59hESQcgqVNmqroJtdFj6RoAuDUmFseLgyxIWrBKYygjyh4uM+TutkEqc\n\tTAtXrER4RNux9lP00G6HK32PzogzUAzUrYAws2SFk2BNbqPilUIbP5zk63rBjcOyK1d\n\tqAqt1YbyQI9q1BASEwDIG1NCH/pVi3hGLvTemHjU=","Message-ID":"<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>","Date":"Sat, 30 Nov 2024 00:17:35 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","To":"libcamera-devel@lists.libcamera.org","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>","Content-Language":"en-US","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32478,"web_url":"https://patchwork.libcamera.org/comment/32478/","msgid":"<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>","date":"2024-11-30T10:01:57","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Robert Mader (2024-11-29 23:17:35)\n> Wait, sorry, take that back. This commit is *not* needed any more. \n> Blacklevels from tuning files now get applied on master.\n\nOh - ok - Milan, do you think this patch is still needed for any other\nuse case?\n\nLet me know if you think this should still be applied or dropped.\n\n--\nKieran\n\n> \n> On 30.11.24 00:07, Robert Mader wrote:\n> > Quickly retested on top of latest master, including \"libcamera: \n> > software_isp: Initialize exposure+gain before agc calculations\"\n> >\n> > Tested-by: Robert Mader <robert.mader@collabora.com>\n> >\n> > On 08.11.24 10:43, Milan Zamazal wrote:\n> >> The black level obtained from the tuning file in software ISP is\n> >> retrieved in init (because this is the standard algorithm method with\n> >> access to tuning data) and stored into context.  But the context gets\n> >> reset in configure and the black level is lost and never applied.\n> >>\n> >> Let's store the black level from the tuning file into an algorithm\n> >> instance variable and put it into the context only later in configure.\n> >> This is similar to what rkisp1 IPA does with the values obtained from\n> >> the tuning file.\n> >>\n> >> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera: \n> >> software_isp: Clear IPA context on configure and stop\")\n> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> ---\n> >>   src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n> >>   src/ipa/simple/algorithms/blc.h   | 4 ++++\n> >>   src/ipa/simple/soft_simple.cpp    | 3 +--\n> >>   3 files changed, 10 insertions(+), 4 deletions(-)\n> >>\n> >> diff --git a/src/ipa/simple/algorithms/blc.cpp \n> >> b/src/ipa/simple/algorithms/blc.cpp\n> >> index b4e32fe1c..1d7d370b5 100644\n> >> --- a/src/ipa/simple/algorithms/blc.cpp\n> >> +++ b/src/ipa/simple/algorithms/blc.cpp\n> >> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n> >>   {\n> >>   }\n> >>   -int BlackLevel::init(IPAContext &context, const YamlObject \n> >> &tuningData)\n> >> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n> >> +             const YamlObject &tuningData)\n> >>   {\n> >>       auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n> >>       if (blackLevel.has_value()) {\n> >> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const \n> >> YamlObject &tuningData)\n> >>            * Convert 16 bit values from the tuning file to 8 bit black\n> >>            * level for the SoftISP.\n> >>            */\n> >> -        context.configuration.black.level = blackLevel.value() >> 8;\n> >> +        definedLevel_ = blackLevel.value() >> 8;\n> >>       }\n> >>       return 0;\n> >>   }\n> >> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const \n> >> YamlObject &tuningData)\n> >>   int BlackLevel::configure(IPAContext &context,\n> >>                 [[maybe_unused]] const IPAConfigInfo &configInfo)\n> >>   {\n> >> +    if (definedLevel_.has_value())\n> >> +        context.configuration.black.level = definedLevel_;\n> >>       context.activeState.blc.level =\n> >>           context.configuration.black.level.value_or(255);\n> >>       return 0;\n> >> diff --git a/src/ipa/simple/algorithms/blc.h \n> >> b/src/ipa/simple/algorithms/blc.h\n> >> index 2cf2a8774..453123d27 100644\n> >> --- a/src/ipa/simple/algorithms/blc.h\n> >> +++ b/src/ipa/simple/algorithms/blc.h\n> >> @@ -7,6 +7,9 @@\n> >>     #pragma once\n> >>   +#include <optional>\n> >> +#include <stdint.h>\n> >> +\n> >>   #include \"algorithm.h\"\n> >>     namespace libcamera {\n> >> @@ -29,6 +32,7 @@ public:\n> >>   private:\n> >>       uint32_t exposure_;\n> >>       double gain_;\n> >> +    std::optional<uint8_t> definedLevel_;\n> >>   };\n> >>     } /* namespace ipa::soft::algorithms */\n> >> diff --git a/src/ipa/simple/soft_simple.cpp \n> >> b/src/ipa/simple/soft_simple.cpp\n> >> index c8ad55a21..825c06757 100644\n> >> --- a/src/ipa/simple/soft_simple.cpp\n> >> +++ b/src/ipa/simple/soft_simple.cpp\n> >> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo \n> >> &configInfo)\n> >>               (context_.configuration.agc.againMax -\n> >>                context_.configuration.agc.againMin) /\n> >>               100.0;\n> >> -        if (!context_.configuration.black.level.has_value() &&\n> >> -            camHelper_->blackLevel().has_value()) {\n> >> +        if (camHelper_->blackLevel().has_value()) {\n> >>               /*\n> >>                * The black level from camHelper_ is a 16 bit value, \n> >> software ISP\n> >>                * works with 8 bit pixel values, both regardless of \n> >> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E74F5BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 30 Nov 2024 10:02:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAC986604D;\n\tSat, 30 Nov 2024 11:02:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B7A665F8F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Nov 2024 11:02:00 +0100 (CET)","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 C3E3A5B3;\n\tSat, 30 Nov 2024 11:01:34 +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=\"eB++IOkL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732960894;\n\tbh=ABQnAGig3gaTfYl/SljCVjL3nGd204QQ3/KSRdcu2GY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=eB++IOkLpNP2XzINpxa4mp2+rQQ0YmHzEem3+YWL9/bfeVsuscRefyLRnREgQ300O\n\t9igv+2U/1PpI0cqJzJVJoIJPUu7EXZ+pFv4t9o6175xTuQLRa4TZuInNibZlYU8zyS\n\tC0IWJCvMvBf7X8Z8gCZZWRc+sZjkyJOOAnQZsugM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>\n\t<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Robert Mader <robert.mader@collabora.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tMilan Zamazal <mzamazal@redhat.com>, ","Date":"Sat, 30 Nov 2024 10:01:57 +0000","Message-ID":"<173296091728.1810449.825331126323016294@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32480,"web_url":"https://patchwork.libcamera.org/comment/32480/","msgid":"<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com>","date":"2024-12-01T17:08:30","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Sorry, turned out that I tested with \nhttps://patchwork.libcamera.org/patch/21703/ applied, which applies \ncleanly on master (unlike the patch here). And without that, i.e. on \nmaster, tuning file values don't get applied yet. So either of these \npatches is still needed.\n\nOn 30.11.24 11:01, Kieran Bingham wrote:\n> Quoting Robert Mader (2024-11-29 23:17:35)\n>> Wait, sorry, take that back. This commit is *not* needed any more.\n>> Blacklevels from tuning files now get applied on master.\n> Oh - ok - Milan, do you think this patch is still needed for any other\n> use case?\n>\n> Let me know if you think this should still be applied or dropped.\n>\n> --\n> Kieran\n>\n>> On 30.11.24 00:07, Robert Mader wrote:\n>>> Quickly retested on top of latest master, including \"libcamera:\n>>> software_isp: Initialize exposure+gain before agc calculations\"\n>>>\n>>> Tested-by: Robert Mader <robert.mader@collabora.com>\n>>>\n>>> On 08.11.24 10:43, Milan Zamazal wrote:\n>>>> The black level obtained from the tuning file in software ISP is\n>>>> retrieved in init (because this is the standard algorithm method with\n>>>> access to tuning data) and stored into context.  But the context gets\n>>>> reset in configure and the black level is lost and never applied.\n>>>>\n>>>> Let's store the black level from the tuning file into an algorithm\n>>>> instance variable and put it into the context only later in configure.\n>>>> This is similar to what rkisp1 IPA does with the values obtained from\n>>>> the tuning file.\n>>>>\n>>>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera:\n>>>> software_isp: Clear IPA context on configure and stop\")\n>>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>>> ---\n>>>>    src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>>>>    src/ipa/simple/algorithms/blc.h   | 4 ++++\n>>>>    src/ipa/simple/soft_simple.cpp    | 3 +--\n>>>>    3 files changed, 10 insertions(+), 4 deletions(-)\n>>>>\n>>>> diff --git a/src/ipa/simple/algorithms/blc.cpp\n>>>> b/src/ipa/simple/algorithms/blc.cpp\n>>>> index b4e32fe1c..1d7d370b5 100644\n>>>> --- a/src/ipa/simple/algorithms/blc.cpp\n>>>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>>>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>>>>    {\n>>>>    }\n>>>>    -int BlackLevel::init(IPAContext &context, const YamlObject\n>>>> &tuningData)\n>>>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n>>>> +             const YamlObject &tuningData)\n>>>>    {\n>>>>        auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>>>>        if (blackLevel.has_value()) {\n>>>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const\n>>>> YamlObject &tuningData)\n>>>>             * Convert 16 bit values from the tuning file to 8 bit black\n>>>>             * level for the SoftISP.\n>>>>             */\n>>>> -        context.configuration.black.level = blackLevel.value() >> 8;\n>>>> +        definedLevel_ = blackLevel.value() >> 8;\n>>>>        }\n>>>>        return 0;\n>>>>    }\n>>>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const\n>>>> YamlObject &tuningData)\n>>>>    int BlackLevel::configure(IPAContext &context,\n>>>>                  [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>>>    {\n>>>> +    if (definedLevel_.has_value())\n>>>> +        context.configuration.black.level = definedLevel_;\n>>>>        context.activeState.blc.level =\n>>>>            context.configuration.black.level.value_or(255);\n>>>>        return 0;\n>>>> diff --git a/src/ipa/simple/algorithms/blc.h\n>>>> b/src/ipa/simple/algorithms/blc.h\n>>>> index 2cf2a8774..453123d27 100644\n>>>> --- a/src/ipa/simple/algorithms/blc.h\n>>>> +++ b/src/ipa/simple/algorithms/blc.h\n>>>> @@ -7,6 +7,9 @@\n>>>>      #pragma once\n>>>>    +#include <optional>\n>>>> +#include <stdint.h>\n>>>> +\n>>>>    #include \"algorithm.h\"\n>>>>      namespace libcamera {\n>>>> @@ -29,6 +32,7 @@ public:\n>>>>    private:\n>>>>        uint32_t exposure_;\n>>>>        double gain_;\n>>>> +    std::optional<uint8_t> definedLevel_;\n>>>>    };\n>>>>      } /* namespace ipa::soft::algorithms */\n>>>> diff --git a/src/ipa/simple/soft_simple.cpp\n>>>> b/src/ipa/simple/soft_simple.cpp\n>>>> index c8ad55a21..825c06757 100644\n>>>> --- a/src/ipa/simple/soft_simple.cpp\n>>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo\n>>>> &configInfo)\n>>>>                (context_.configuration.agc.againMax -\n>>>>                 context_.configuration.agc.againMin) /\n>>>>                100.0;\n>>>> -        if (!context_.configuration.black.level.has_value() &&\n>>>> -            camHelper_->blackLevel().has_value()) {\n>>>> +        if (camHelper_->blackLevel().has_value()) {\n>>>>                /*\n>>>>                 * The black level from camHelper_ is a 16 bit value,\n>>>> software ISP\n>>>>                 * works with 8 bit pixel values, both regardless of\n>>>> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4C02BBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  1 Dec 2024 17:08:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 198EB6605B;\n\tSun,  1 Dec 2024 18:08:42 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02E386600E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  1 Dec 2024 18:08:39 +0100 (CET)","by mx.zohomail.com with SMTPS id 1733072913554554.2734916719144;\n\tSun, 1 Dec 2024 09:08:33 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"Wj4gHgwv\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1733072916; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=RD0ikK4Xm/k7ZEJscdzrYq7SaNqYzBVj1goJB962CmTE+AncXqasTI0JZLatxkyzTkl929hvim3XDWIHsLEE5HnuaUr8tqJEE4no7HltFNEiEUfJDNS+0TKvZMdCe6xIx2/WTWEI5VJ1M1nD7pIpXX59Uo6UXxt7Os24NBcXgX4=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1733072916;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=TzlQEqBlFE9770HxqpYPBbhBtdjfQD4NwuZHQOKiKdE=; \n\tb=Br/phfdkIlKcwJSu///1uHvhkA9zIqos9mTFTsDJK8ruStwBWJDT8gA5+6Kjl0MUVrUM8BNv1NSrQaCQCo07r4Tid5bUnj1H/ttyDiwaGDp7oA0c3d62Rz23FqJ2Y+JmIQiqoUiTKKgjJbXntgF98M73tBR/cpU3qgd7bC6gtMM=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1733072916;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=TzlQEqBlFE9770HxqpYPBbhBtdjfQD4NwuZHQOKiKdE=;\n\tb=Wj4gHgwvhgLI3sRJ9+HoBw2VDDjooQoIGyJS5Z9HbFdE/ClUKTTGPCfXldHmX/86\n\twynCwMV62Aq8aXtor+ADmWSuhsIO0a/oLkLWz4M3EhfZWDMRxk5A1wWaw42+ii6u/K0\n\thmsYFsYaqMlm9fpqQ1DiBiWzoTyhJIqjjoppVwUY=","Message-ID":"<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com>","Date":"Sun, 1 Dec 2024 18:08:30 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, Milan Zamazal <mzamazal@redhat.com>","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>\n\t<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>\n\t<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>","Content-Language":"en-US","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32483,"web_url":"https://patchwork.libcamera.org/comment/32483/","msgid":"<87o71ujqoe.fsf@redhat.com>","date":"2024-12-02T11:00:01","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Robert,\n\nRobert Mader <robert.mader@collabora.com> writes:\n\n> Sorry, turned out that I tested with https://patchwork.libcamera.org/patch/21703/ applied, which applies cleanly on master\n> (unlike the patch here). And without that, i.e. on master, tuning file values don't get applied yet. So either of these\n> patches is still needed.\n\nYes.  I think my patch is the right fix for the given bit.  It applies\ncleanly on master.  Have you already tested it or would you like to test\nit?\n\n> On 30.11.24 11:01, Kieran Bingham wrote:\n>> Quoting Robert Mader (2024-11-29 23:17:35)\n>>> Wait, sorry, take that back. This commit is *not* needed any more.\n>>> Blacklevels from tuning files now get applied on master.\n>> Oh - ok - Milan, do you think this patch is still needed for any other\n>> use case?\n>>\n>> Let me know if you think this should still be applied or dropped.\n>>\n>> --\n>> Kieran\n>>\n>>> On 30.11.24 00:07, Robert Mader wrote:\n>>>> Quickly retested on top of latest master, including \"libcamera:\n>>>> software_isp: Initialize exposure+gain before agc calculations\"\n>>>>\n>>>> Tested-by: Robert Mader <robert.mader@collabora.com>\n>>>>\n>>>> On 08.11.24 10:43, Milan Zamazal wrote:\n>>>>> The black level obtained from the tuning file in software ISP is\n>>>>> retrieved in init (because this is the standard algorithm method with\n>>>>> access to tuning data) and stored into context.  But the context gets\n>>>>> reset in configure and the black level is lost and never applied.\n>>>>>\n>>>>> Let's store the black level from the tuning file into an algorithm\n>>>>> instance variable and put it into the context only later in configure.\n>>>>> This is similar to what rkisp1 IPA does with the values obtained from\n>>>>> the tuning file.\n>>>>>\n>>>>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera:\n>>>>> software_isp: Clear IPA context on configure and stop\")\n>>>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>>>> ---\n>>>>>    src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>>>>>    src/ipa/simple/algorithms/blc.h   | 4 ++++\n>>>>>    src/ipa/simple/soft_simple.cpp    | 3 +--\n>>>>>    3 files changed, 10 insertions(+), 4 deletions(-)\n>>>>>\n>>>>> diff --git a/src/ipa/simple/algorithms/blc.cpp\n>>>>> b/src/ipa/simple/algorithms/blc.cpp\n>>>>> index b4e32fe1c..1d7d370b5 100644\n>>>>> --- a/src/ipa/simple/algorithms/blc.cpp\n>>>>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>>>>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>>>>>    {\n>>>>>    }\n>>>>>    -int BlackLevel::init(IPAContext &context, const YamlObject\n>>>>> &tuningData)\n>>>>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n>>>>> +             const YamlObject &tuningData)\n>>>>>    {\n>>>>>        auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>>>>>        if (blackLevel.has_value()) {\n>>>>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const\n>>>>> YamlObject &tuningData)\n>>>>>             * Convert 16 bit values from the tuning file to 8 bit black\n>>>>>             * level for the SoftISP.\n>>>>>             */\n>>>>> -        context.configuration.black.level = blackLevel.value() >> 8;\n>>>>> +        definedLevel_ = blackLevel.value() >> 8;\n>>>>>        }\n>>>>>        return 0;\n>>>>>    }\n>>>>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const\n>>>>> YamlObject &tuningData)\n>>>>>    int BlackLevel::configure(IPAContext &context,\n>>>>>                  [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>>>>    {\n>>>>> +    if (definedLevel_.has_value())\n>>>>> +        context.configuration.black.level = definedLevel_;\n>>>>>        context.activeState.blc.level =\n>>>>>            context.configuration.black.level.value_or(255);\n>>>>>        return 0;\n>>>>> diff --git a/src/ipa/simple/algorithms/blc.h\n>>>>> b/src/ipa/simple/algorithms/blc.h\n>>>>> index 2cf2a8774..453123d27 100644\n>>>>> --- a/src/ipa/simple/algorithms/blc.h\n>>>>> +++ b/src/ipa/simple/algorithms/blc.h\n>>>>> @@ -7,6 +7,9 @@\n>>>>>      #pragma once\n>>>>>    +#include <optional>\n>>>>> +#include <stdint.h>\n>>>>> +\n>>>>>    #include \"algorithm.h\"\n>>>>>      namespace libcamera {\n>>>>> @@ -29,6 +32,7 @@ public:\n>>>>>    private:\n>>>>>        uint32_t exposure_;\n>>>>>        double gain_;\n>>>>> +    std::optional<uint8_t> definedLevel_;\n>>>>>    };\n>>>>>      } /* namespace ipa::soft::algorithms */\n>>>>> diff --git a/src/ipa/simple/soft_simple.cpp\n>>>>> b/src/ipa/simple/soft_simple.cpp\n>>>>> index c8ad55a21..825c06757 100644\n>>>>> --- a/src/ipa/simple/soft_simple.cpp\n>>>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>>>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo\n>>>>> &configInfo)\n>>>>>                (context_.configuration.agc.againMax -\n>>>>>                 context_.configuration.agc.againMin) /\n>>>>>                100.0;\n>>>>> -        if (!context_.configuration.black.level.has_value() &&\n>>>>> -            camHelper_->blackLevel().has_value()) {\n>>>>> +        if (camHelper_->blackLevel().has_value()) {\n>>>>>                /*\n>>>>>                 * The black level from camHelper_ is a 16 bit value,\n>>>>> software ISP\n>>>>>                 * works with 8 bit pixel values, both regardless of\n>>>>> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 10C6ABDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  2 Dec 2024 11:00:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 992F966061;\n\tMon,  2 Dec 2024 12:00:09 +0100 (CET)","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 E953365F8F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Dec 2024 12:00:07 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-137-aktdaqfzNKe32FBXZaII9w-1; Mon, 02 Dec 2024 06:00:05 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-434a467e970so28689435e9.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 02 Dec 2024 03:00:05 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-385ee60549dsm3890183f8f.34.2024.12.02.03.00.02\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 02 Dec 2024 03:00:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"GH+1Cqmy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1733137206;\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\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=udRKDEtrau2RP+iIw0AkkUwDb4Ree3K50ZhJJdY2wB8=;\n\tb=GH+1CqmynCv/+eyrT4VTsqX1FZjXuXbsHKkqxRA+EgdqhA61Nkil2SLt5LY/5YHU1KgO/B\n\tYmf7gEN6OM6pXQlDnVVXZADi7MsCvesx3lrmnuyyOejNgjNT1nZ4zrv6sRExdu6tg6Fyst\n\to2aXiIro3LsqLt7Al7zUkAGjSNa1tC8=","X-MC-Unique":"aktdaqfzNKe32FBXZaII9w-1","X-Mimecast-MFC-AGG-ID":"aktdaqfzNKe32FBXZaII9w","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733137204; x=1733742004;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=U7oF/t1eu5sBbj4SpDk3I8Mqx2tBkPgPNQPNvkrAmo0=;\n\tb=Lx+4cRegHMGWsQARJZTFvY+N23iuY+S7WQ7y3AajoKsJNt8RcBcq0XJSNIaZqAI8Nu\n\tbcvSPmy614cSVbneX6DBX/IsCjy/ztXzbSubqRQAoLMMkLYK319hg1mbI5qhC+tYGn7U\n\t2u+kgD6S8q55Zb7t84gjJKBMwdLCznzDZtKwXtli5haieu59sM4sPhwacW8seK2wgxoD\n\tjBHINaBYQjjyoV/SwLX+SAG4fLf9MsGZVskwtzx5vr/U/O04fFwgadaoeeVup2QSYzD0\n\ttvQvM+JPvt0pFtgPDY06JSHJ8VH1Ahhcw5XrSGM6CTeEG9YjUxdpDTLx+tpCnZeNzGtz\n\tzhqQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU0U20rSAyAvTU7at5Ti9leYtWGCFpyv0P63ntlBXl7lU+sET4E2CFdTBJdarg9a6IxZALczV4upuCsavhNLkU=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yw/szVQ49pnLZibSXY+sYaxie2Y1y07GSFJbkUB9XpyXs+vAM3P\n\t9+uxmx+VYS0em3y4iYItEguASgIS6mOAP6Xty58a3AjHB1Gy9irVhRjZgKRNOz+amiihyX+sgeN\n\ttZXZLvisFp4tB/UX1HnKWe64vpTNLX4OQn2WW3a1yk0Hwu8NB80JZihw4YkVaoU8d1OyvFRbg8f\n\tVbZYKIDu5902pJtxkcS5O3OXPRw+fgTyQUcfiXNzsjUpbrwNvwR9ZAZ90=","X-Gm-Gg":"ASbGnctgcnguf5UkpMQBGFsgkjLEf14Y5gv/yjb9WMIbFe6COPi7rTn6T8N9Bb2O8Qq\n\tuIl6zUA+cfVD1DYy75R84HoqvXgUb7KfJGkU/fJwlPZ7DWqMALajqAF1u6TJjqNdHAfyqq1Fzjo\n\tEv+NpNz2Ae/Vkyfi/9cQ8+fCLGSoBENdCrVxO+ODRn5VLh2JhqpllLVO1z9hpD0g7IDmKOGW1d9\n\tiZQSZDqSfmR01JO4V6ZeRQZgLvA91D2vR33YI8a8r8XmuiTv3PLb+q/gj/FFzRE4rgnCiQ=","X-Received":["by 2002:a05:600c:458b:b0:434:a924:44e9 with SMTP id\n\t5b1f17b1804b1-434a9dcfedfmr212901345e9.15.1733137203824; \n\tMon, 02 Dec 2024 03:00:03 -0800 (PST)","by 2002:a05:600c:458b:b0:434:a924:44e9 with SMTP id\n\t5b1f17b1804b1-434a9dcfedfmr212901035e9.15.1733137203385; \n\tMon, 02 Dec 2024 03:00:03 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IH/Al7NH85BcP3IXxaylaC1zLMSjh3+ydaUA81lHUiozS3kYVQpN+8ZuQBbjy3csg4L67tGZw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Robert Mader <robert.mader@collabora.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","In-Reply-To":"<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com> (Robert\n\tMader's message of \"Sun, 1 Dec 2024 18:08:30 +0100\")","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>\n\t<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>\n\t<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>\n\t<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com>","Date":"Mon, 02 Dec 2024 12:00:01 +0100","Message-ID":"<87o71ujqoe.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"suEt1dTXV4GO98XBpngu6-Z4sxvFLeD58FfExwthfp8_1733137204","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32488,"web_url":"https://patchwork.libcamera.org/comment/32488/","msgid":"<6e5b4caf-5d75-40c1-bc86-8580c3eacbb4@collabora.com>","date":"2024-12-02T17:49:20","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Hm, we are talking about https://patchwork.libcamera.org/patch/21865/, \ncorrect? It does not apply on master (a43ea7ff70e) here.\n\nOn 02.12.24 12:00, Milan Zamazal wrote:\n> Hi Robert,\n>\n> Robert Mader <robert.mader@collabora.com> writes:\n>\n>> Sorry, turned out that I tested with https://patchwork.libcamera.org/patch/21703/ applied, which applies cleanly on master\n>> (unlike the patch here). And without that, i.e. on master, tuning file values don't get applied yet. So either of these\n>> patches is still needed.\n> Yes.  I think my patch is the right fix for the given bit.  It applies\n> cleanly on master.  Have you already tested it or would you like to test\n> it?\n>\n>> On 30.11.24 11:01, Kieran Bingham wrote:\n>>> Quoting Robert Mader (2024-11-29 23:17:35)\n>>>> Wait, sorry, take that back. This commit is *not* needed any more.\n>>>> Blacklevels from tuning files now get applied on master.\n>>> Oh - ok - Milan, do you think this patch is still needed for any other\n>>> use case?\n>>>\n>>> Let me know if you think this should still be applied or dropped.\n>>>\n>>> --\n>>> Kieran\n>>>\n>>>> On 30.11.24 00:07, Robert Mader wrote:\n>>>>> Quickly retested on top of latest master, including \"libcamera:\n>>>>> software_isp: Initialize exposure+gain before agc calculations\"\n>>>>>\n>>>>> Tested-by: Robert Mader <robert.mader@collabora.com>\n>>>>>\n>>>>> On 08.11.24 10:43, Milan Zamazal wrote:\n>>>>>> The black level obtained from the tuning file in software ISP is\n>>>>>> retrieved in init (because this is the standard algorithm method with\n>>>>>> access to tuning data) and stored into context.  But the context gets\n>>>>>> reset in configure and the black level is lost and never applied.\n>>>>>>\n>>>>>> Let's store the black level from the tuning file into an algorithm\n>>>>>> instance variable and put it into the context only later in configure.\n>>>>>> This is similar to what rkisp1 IPA does with the values obtained from\n>>>>>> the tuning file.\n>>>>>>\n>>>>>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera:\n>>>>>> software_isp: Clear IPA context on configure and stop\")\n>>>>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>>>>> ---\n>>>>>>     src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>>>>>>     src/ipa/simple/algorithms/blc.h   | 4 ++++\n>>>>>>     src/ipa/simple/soft_simple.cpp    | 3 +--\n>>>>>>     3 files changed, 10 insertions(+), 4 deletions(-)\n>>>>>>\n>>>>>> diff --git a/src/ipa/simple/algorithms/blc.cpp\n>>>>>> b/src/ipa/simple/algorithms/blc.cpp\n>>>>>> index b4e32fe1c..1d7d370b5 100644\n>>>>>> --- a/src/ipa/simple/algorithms/blc.cpp\n>>>>>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>>>>>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>>>>>>     {\n>>>>>>     }\n>>>>>>     -int BlackLevel::init(IPAContext &context, const YamlObject\n>>>>>> &tuningData)\n>>>>>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n>>>>>> +             const YamlObject &tuningData)\n>>>>>>     {\n>>>>>>         auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>>>>>>         if (blackLevel.has_value()) {\n>>>>>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const\n>>>>>> YamlObject &tuningData)\n>>>>>>              * Convert 16 bit values from the tuning file to 8 bit black\n>>>>>>              * level for the SoftISP.\n>>>>>>              */\n>>>>>> -        context.configuration.black.level = blackLevel.value() >> 8;\n>>>>>> +        definedLevel_ = blackLevel.value() >> 8;\n>>>>>>         }\n>>>>>>         return 0;\n>>>>>>     }\n>>>>>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const\n>>>>>> YamlObject &tuningData)\n>>>>>>     int BlackLevel::configure(IPAContext &context,\n>>>>>>                   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>>>>>     {\n>>>>>> +    if (definedLevel_.has_value())\n>>>>>> +        context.configuration.black.level = definedLevel_;\n>>>>>>         context.activeState.blc.level =\n>>>>>>             context.configuration.black.level.value_or(255);\n>>>>>>         return 0;\n>>>>>> diff --git a/src/ipa/simple/algorithms/blc.h\n>>>>>> b/src/ipa/simple/algorithms/blc.h\n>>>>>> index 2cf2a8774..453123d27 100644\n>>>>>> --- a/src/ipa/simple/algorithms/blc.h\n>>>>>> +++ b/src/ipa/simple/algorithms/blc.h\n>>>>>> @@ -7,6 +7,9 @@\n>>>>>>       #pragma once\n>>>>>>     +#include <optional>\n>>>>>> +#include <stdint.h>\n>>>>>> +\n>>>>>>     #include \"algorithm.h\"\n>>>>>>       namespace libcamera {\n>>>>>> @@ -29,6 +32,7 @@ public:\n>>>>>>     private:\n>>>>>>         uint32_t exposure_;\n>>>>>>         double gain_;\n>>>>>> +    std::optional<uint8_t> definedLevel_;\n>>>>>>     };\n>>>>>>       } /* namespace ipa::soft::algorithms */\n>>>>>> diff --git a/src/ipa/simple/soft_simple.cpp\n>>>>>> b/src/ipa/simple/soft_simple.cpp\n>>>>>> index c8ad55a21..825c06757 100644\n>>>>>> --- a/src/ipa/simple/soft_simple.cpp\n>>>>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>>>>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo\n>>>>>> &configInfo)\n>>>>>>                 (context_.configuration.agc.againMax -\n>>>>>>                  context_.configuration.agc.againMin) /\n>>>>>>                 100.0;\n>>>>>> -        if (!context_.configuration.black.level.has_value() &&\n>>>>>> -            camHelper_->blackLevel().has_value()) {\n>>>>>> +        if (camHelper_->blackLevel().has_value()) {\n>>>>>>                 /*\n>>>>>>                  * The black level from camHelper_ is a 16 bit value,\n>>>>>> software ISP\n>>>>>>                  * works with 8 bit pixel values, both regardless of\n>>>>>> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 23B58BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  2 Dec 2024 17:49:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 168DB66077;\n\tMon,  2 Dec 2024 18:49:32 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D88876605F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Dec 2024 18:49:29 +0100 (CET)","by mx.zohomail.com with SMTPS id 173316176542664.31943103502681;\n\tMon, 2 Dec 2024 09:49:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"bdAQH3bZ\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1733161766; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=Sh4m4KWI+kX6TPgB+QIvgxAE/jjz2K1CCZBGZZmjxmrbhz1YBRjPv4r82saLJoZCDpxF5awKXHRJzvN6oCdg95r1oQ+vmRZhmj63mPSHBhtkOsCWNN0ona4TW0X9aJQ09Gte7/Wnzmf/ax1mafr9cAfWJOUSG2VY5OHeA74TaqY=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1733161766;\n\th=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To;\n\tbh=kWcI6/+COR6uK+FhbzYqjbnUnHXzqrCyIG9MM55TsHM=; \n\tb=IfQe7CxKJlH40z16eJSp18drhUzcfxio6tOGdlkaccIajL3hC9GYzfNTnhcLCXCqFnHq1UAqX/e2Ji9mXhMbX4kIrW6wHRebKIp9LuDpxgUdskQiEW4pqOgILpgIjxNnvNTISrWcWWYglfPb/nBTwWuhUXwNg3TfsJ2uRDg2FS4=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1733161766;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To;\n\tbh=kWcI6/+COR6uK+FhbzYqjbnUnHXzqrCyIG9MM55TsHM=;\n\tb=bdAQH3bZwhGxH5ZSzZjheF+mUmiKgRpi3MjQksgo2pasHwb4xUl5LCYIQDpL4Paa\n\tcBAkNXpUoE7UbCOMdjBjEL8+luyHR8Zvv1xQx/0k4UFyjbEuEfBzI3ow94/byqiU1wl\n\tM8Bd9+940UMtBWvxMeV2LclXuACB89rlL/dXZyWg=","Message-ID":"<6e5b4caf-5d75-40c1-bc86-8580c3eacbb4@collabora.com>","Date":"Mon, 2 Dec 2024 18:49:20 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>\n\t<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>\n\t<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>\n\t<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com>\n\t<87o71ujqoe.fsf@redhat.com>","Content-Language":"en-US","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<87o71ujqoe.fsf@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32490,"web_url":"https://patchwork.libcamera.org/comment/32490/","msgid":"<87r06pnly6.fsf@redhat.com>","date":"2024-12-03T09:40:49","subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Robert,\n\nRobert Mader <robert.mader@collabora.com> writes:\n\n> Hm, we are talking about https://patchwork.libcamera.org/patch/21865/,\n> correct? \n\nYes.\n\n> It does not apply on master (a43ea7ff70e) here.\n\nOh, sorry, either `git rebase' is smarter or I resolved it in the\nmeantime.\n\nPosted v2 rebased on master.\n\n> On 02.12.24 12:00, Milan Zamazal wrote:\n>> Hi Robert,\n>>\n>> Robert Mader <robert.mader@collabora.com> writes:\n>>\n>>> Sorry, turned out that I tested with https://patchwork.libcamera.org/patch/21703/ applied, which applies cleanly on master\n>>> (unlike the patch here). And without that, i.e. on master, tuning file values don't get applied yet. So either of these\n>>> patches is still needed.\n>> Yes.  I think my patch is the right fix for the given bit.  It applies\n>> cleanly on master.  Have you already tested it or would you like to test\n>> it?\n>>\n>>> On 30.11.24 11:01, Kieran Bingham wrote:\n>>>> Quoting Robert Mader (2024-11-29 23:17:35)\n>>>>> Wait, sorry, take that back. This commit is *not* needed any more.\n>>>>> Blacklevels from tuning files now get applied on master.\n>>>> Oh - ok - Milan, do you think this patch is still needed for any other\n>>>> use case?\n>>>>\n>>>> Let me know if you think this should still be applied or dropped.\n>>>>\n>>>> --\n>>>> Kieran\n>>>>\n>>>>> On 30.11.24 00:07, Robert Mader wrote:\n>>>>>> Quickly retested on top of latest master, including \"libcamera:\n>>>>>> software_isp: Initialize exposure+gain before agc calculations\"\n>>>>>>\n>>>>>> Tested-by: Robert Mader <robert.mader@collabora.com>\n>>>>>>\n>>>>>> On 08.11.24 10:43, Milan Zamazal wrote:\n>>>>>>> The black level obtained from the tuning file in software ISP is\n>>>>>>> retrieved in init (because this is the standard algorithm method with\n>>>>>>> access to tuning data) and stored into context.  But the context gets\n>>>>>>> reset in configure and the black level is lost and never applied.\n>>>>>>>\n>>>>>>> Let's store the black level from the tuning file into an algorithm\n>>>>>>> instance variable and put it into the context only later in configure.\n>>>>>>> This is similar to what rkisp1 IPA does with the values obtained from\n>>>>>>> the tuning file.\n>>>>>>>\n>>>>>>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 (\"libcamera:\n>>>>>>> software_isp: Clear IPA context on configure and stop\")\n>>>>>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>>>>>> ---\n>>>>>>>     src/ipa/simple/algorithms/blc.cpp | 7 +++++--\n>>>>>>>     src/ipa/simple/algorithms/blc.h   | 4 ++++\n>>>>>>>     src/ipa/simple/soft_simple.cpp    | 3 +--\n>>>>>>>     3 files changed, 10 insertions(+), 4 deletions(-)\n>>>>>>>\n>>>>>>> diff --git a/src/ipa/simple/algorithms/blc.cpp\n>>>>>>> b/src/ipa/simple/algorithms/blc.cpp\n>>>>>>> index b4e32fe1c..1d7d370b5 100644\n>>>>>>> --- a/src/ipa/simple/algorithms/blc.cpp\n>>>>>>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>>>>>>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()\n>>>>>>>     {\n>>>>>>>     }\n>>>>>>>     -int BlackLevel::init(IPAContext &context, const YamlObject\n>>>>>>> &tuningData)\n>>>>>>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,\n>>>>>>> +             const YamlObject &tuningData)\n>>>>>>>     {\n>>>>>>>         auto blackLevel = tuningData[\"blackLevel\"].get<int16_t>();\n>>>>>>>         if (blackLevel.has_value()) {\n>>>>>>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const\n>>>>>>> YamlObject &tuningData)\n>>>>>>>              * Convert 16 bit values from the tuning file to 8 bit black\n>>>>>>>              * level for the SoftISP.\n>>>>>>>              */\n>>>>>>> -        context.configuration.black.level = blackLevel.value() >> 8;\n>>>>>>> +        definedLevel_ = blackLevel.value() >> 8;\n>>>>>>>         }\n>>>>>>>         return 0;\n>>>>>>>     }\n>>>>>>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const\n>>>>>>> YamlObject &tuningData)\n>>>>>>>     int BlackLevel::configure(IPAContext &context,\n>>>>>>>                   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>>>>>>>     {\n>>>>>>> +    if (definedLevel_.has_value())\n>>>>>>> +        context.configuration.black.level = definedLevel_;\n>>>>>>>         context.activeState.blc.level =\n>>>>>>>             context.configuration.black.level.value_or(255);\n>>>>>>>         return 0;\n>>>>>>> diff --git a/src/ipa/simple/algorithms/blc.h\n>>>>>>> b/src/ipa/simple/algorithms/blc.h\n>>>>>>> index 2cf2a8774..453123d27 100644\n>>>>>>> --- a/src/ipa/simple/algorithms/blc.h\n>>>>>>> +++ b/src/ipa/simple/algorithms/blc.h\n>>>>>>> @@ -7,6 +7,9 @@\n>>>>>>>       #pragma once\n>>>>>>>     +#include <optional>\n>>>>>>> +#include <stdint.h>\n>>>>>>> +\n>>>>>>>     #include \"algorithm.h\"\n>>>>>>>       namespace libcamera {\n>>>>>>> @@ -29,6 +32,7 @@ public:\n>>>>>>>     private:\n>>>>>>>         uint32_t exposure_;\n>>>>>>>         double gain_;\n>>>>>>> +    std::optional<uint8_t> definedLevel_;\n>>>>>>>     };\n>>>>>>>       } /* namespace ipa::soft::algorithms */\n>>>>>>> diff --git a/src/ipa/simple/soft_simple.cpp\n>>>>>>> b/src/ipa/simple/soft_simple.cpp\n>>>>>>> index c8ad55a21..825c06757 100644\n>>>>>>> --- a/src/ipa/simple/soft_simple.cpp\n>>>>>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>>>>>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo\n>>>>>>> &configInfo)\n>>>>>>>                 (context_.configuration.agc.againMax -\n>>>>>>>                  context_.configuration.agc.againMin) /\n>>>>>>>                 100.0;\n>>>>>>> -        if (!context_.configuration.black.level.has_value() &&\n>>>>>>> -            camHelper_->blackLevel().has_value()) {\n>>>>>>> +        if (camHelper_->blackLevel().has_value()) {\n>>>>>>>                 /*\n>>>>>>>                  * The black level from camHelper_ is a 16 bit value,\n>>>>>>> software ISP\n>>>>>>>                  * works with 8 bit pixel values, both regardless of\n>>>>>>> the actual","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 40158BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Dec 2024 09:40:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 48E2A66081;\n\tTue,  3 Dec 2024 10:40:57 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AFCB966072\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Dec 2024 10:40:55 +0100 (CET)","from mail-wr1-f70.google.com (mail-wr1-f70.google.com\n\t[209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-454-HZzo-6kpOE2hMBoBfxEgCw-1; Tue, 03 Dec 2024 04:40:53 -0500","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-385d4fa8e19so2766289f8f.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 03 Dec 2024 01:40:53 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-385fa330ca0sm2751313f8f.11.2024.12.03.01.40.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 03 Dec 2024 01:40:50 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"TggA52Ip\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1733218854;\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\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=zXbEEK7Aiek6lQzl1MuMIq16EnfmTQnD0CWQ2OEeVAE=;\n\tb=TggA52Ip5+8LFAQhj2RcwZ8DU44BWJ2+iM762T3d7U1f8kGnKgoj80rHkTBgYdpkI/fNYJ\n\tJ+yg7F6hED4ZnVZIdWLQnfBSqow2+3D8I4RFvQXL4MGJu9ZK/BfF1XpwRjKvGculG6AIVH\n\t3FzZm9YxyYrY6zzH5oaNE0612qWptmQ=","X-MC-Unique":"HZzo-6kpOE2hMBoBfxEgCw-1","X-Mimecast-MFC-AGG-ID":"HZzo-6kpOE2hMBoBfxEgCw","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733218851; x=1733823651;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=Fd1M5iDVcRDoIaddDWpEot73Ns7fEBZy9xGGM+kzB7I=;\n\tb=aY1i28skzAMyzKgOvT1teJqlgLyllWvMil9XjPMkhKEy2WCB3cgqFDBP2xryokgAjo\n\ths7AZSMQQ0pAsZbs8eX79VMSYX5bhw09RqvLF/xpNAFxY9TGqKQ+7MLBKEyS1E5djreX\n\tx1bcdkzWYTh+I9fxE1pYYe3VMrFiKJLnzdO9BtyS4qwTc6stZUl3+bJRztxws5L1tJZ0\n\tdedXhGB5WL8v+wbclJe6M++6YsncMcRywLi6fQMNch9pyVFMGsLahyDqeF+/CsxOXd9/\n\tunIZE4NLLNeWogkOk/hUp+WvtgRgw34BUIH3jMV1BjjcS/kiSjYGBgc7Iwf5w01KEtAU\n\t0FHQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUozQsCTpzx/z6NiR/KOio8n7vmHkBZdhOsSVl7QpUflrjC3QW2oKDX2Q8sHnEP43d/Gy6OL86I8GY74j3CLZo=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yz0Y7BozO0eHshbXmXKAvzyP4I1tS1loyGf8dr+CPnUQr4KdHWD\n\tpiJVzRi6k2Oau0WzC7sDaFpcYqyzAmiPBnyfo0F0T5yIDyN/LpOUVtrfWSck/K7w8MHyEa71ZNR\n\t9oDDnKjpqtKGXSnUtf3XNIclzYsHxX8oU571XNqZeNJ3g2ejKnTZKx2ju01njmfJZ7pk+Cgzuv2\n\tERIB2N+W4e3lgONOxaoQxkkxYjNFSRS7MaEUoVPrYt1P59OuwxHbB3zcs=","X-Gm-Gg":"ASbGncsP9PXYDibtKMj6Go/Z/z/VPI+WK2OC4ZqNUPd66ZIe11RrGzHQSAVuQkqhfwe\n\tJR1busTA6+8yHLN9gnG8vICvKHlZx1vX0DK9fp/tFkR2mOCDF9FOmd9ptEMfxqyLtFBnIE0AFat\n\tC3jJZasWE3kQbBqXQohHYtPIVp1XMlnV/87y9hdiwqAr8PCxfJOY42rqKcJBHRvn6lRLDlMBAmw\n\tjFgCqsK3yDW1UNRCw9+2fJC1IwoYBR0IDgveSdT/CnfIOvO9ddTFXZwpx0RcXqRXFrm7Q==","X-Received":["by 2002:a05:6000:1f8b:b0:385:df17:2157 with SMTP id\n\tffacd0b85a97d-385fd42a5e9mr1345327f8f.39.1733218851390; \n\tTue, 03 Dec 2024 01:40:51 -0800 (PST)","by 2002:a05:6000:1f8b:b0:385:df17:2157 with SMTP id\n\tffacd0b85a97d-385fd42a5e9mr1345301f8f.39.1733218850896; \n\tTue, 03 Dec 2024 01:40:50 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEJrnbSqPIKsiLao5/NvVY1oeln3FboR/pPDJqq8XQLFgs69lHmXaHyonIDG0FEIdG2m4kzww==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Robert Mader <robert.mader@collabora.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/1] libcamera: software_isp: Actually apply black level\n\tfrom tuning data","In-Reply-To":"<6e5b4caf-5d75-40c1-bc86-8580c3eacbb4@collabora.com> (Robert\n\tMader's message of \"Mon, 2 Dec 2024 18:49:20 +0100\")","References":"<20241108094333.332643-1-mzamazal@redhat.com>\n\t<20241108094333.332643-2-mzamazal@redhat.com>\n\t<34eb4ca4-ea40-46c4-975d-44342e92c596@collabora.com>\n\t<5a688652-3caf-44ad-8d53-ed59a27a8050@collabora.com>\n\t<173296091728.1810449.825331126323016294@ping.linuxembedded.co.uk>\n\t<2d030cbb-5beb-4614-9bb2-8bdf1fe91338@collabora.com>\n\t<87o71ujqoe.fsf@redhat.com>\n\t<6e5b4caf-5d75-40c1-bc86-8580c3eacbb4@collabora.com>","Date":"Tue, 03 Dec 2024 10:40:49 +0100","Message-ID":"<87r06pnly6.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"0S1YE1KJFGkqoSLToHpzf2kFnU56vn1odt2BA6G0UCY_1733218852","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]