[{"id":32491,"web_url":"https://patchwork.libcamera.org/comment/32491/","msgid":"<21af89c6-33d7-4616-8e6d-0d7c419d7747@collabora.com>","date":"2024-12-03T10:08:49","subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Tested-by: Robert Mader <robert.mader@collabora.com>\n\nOn 03.12.24 10:38, 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 b4e32fe1..1d7d370b 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 67c688ae..52d59cab 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>   \tint32_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 ba3d5265..e1b6d3af 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 F083CBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Dec 2024 10:08:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA3D066081;\n\tTue,  3 Dec 2024 11:08:57 +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 1840C66072\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Dec 2024 11:08:55 +0100 (CET)","by mx.zohomail.com with SMTPS id 1733220532426149.47365204945743; \n\tTue, 3 Dec 2024 02:08:52 -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=\"SkpdIZgU\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1733220533; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=UqnTtpHkF4Tlh93qGgpJab73NmRztP6KHJDcpZyWzzfqB85WV4oILQME+MUsvoE1BjDPkbP8TjAQlVaR2negeTX8FWCcTadb4/t/g+uDFh+ljoUi3pOFTLvij9bnWxSzZa3ZEs9kQrkWz0olHBcFl3s1sBrUjLmO0ReeesF1PwM=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1733220533;\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=2743TkK9MOzZdD3PoX+W6bN1lyh5NODgJy24u/Wjl3Q=; \n\tb=FbfJ+a8rizdzkcYzuYWg6pCYcpWp3bEno11rYJfwIOw1BbFBdpj/r3H37mcFONBvalobAkG+1FRwzn60Zc/JnJEZB04J/0HhAwBjk8B+wlA8E5Ni+j55dVD6L7oNy3If1GCF0DSHeQkCD11CVQ9Y4M6kEvZTTs+CHBCcNPRUYW0=","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=1733220533;\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=2743TkK9MOzZdD3PoX+W6bN1lyh5NODgJy24u/Wjl3Q=;\n\tb=SkpdIZgUht9+yypCVtr65pBxTirQ2sjMRuI0N3Gg2rH/4PQu/9gOChHiQdnfamEr\n\t9X8UeqZpbFggkNIkU1Dq8nuZulsvFoKI62VmFbAGB8SOgY/qeuDR9+48z+hZu2X6k7+\n\t/GewXZKRxE7pg8fxkXUpNCjq+fI2an75Afnr4uxg=","Message-ID":"<21af89c6-33d7-4616-8e6d-0d7c419d7747@collabora.com>","Date":"Tue, 3 Dec 2024 11:08:49 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20241203093814.67984-1-mzamazal@redhat.com>\n\t<20241203093814.67984-2-mzamazal@redhat.com>","Content-Language":"en-US","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<20241203093814.67984-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":32495,"web_url":"https://patchwork.libcamera.org/comment/32495/","msgid":"<87mshdnhr6.fsf@redhat.com>","date":"2024-12-03T11:11:25","subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Robert Mader <robert.mader@collabora.com> writes:\n\n> Tested-by: Robert Mader <robert.mader@collabora.com>\n\nThanks for testing!\n\n> On 03.12.24 10:38, 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 b4e32fe1..1d7d370b 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 &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 67c688ae..52d59cab 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>>   \tint32_t exposure_;\n>>   \tdouble gain_;\n>> +\tstd::optional<uint8_t> definedLevel_;\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 ba3d5265..e1b6d3af 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 CA09CBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Dec 2024 11:11:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC7B76608A;\n\tTue,  3 Dec 2024 12:11:33 +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 8DAE166041\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Dec 2024 12:11:31 +0100 (CET)","from mail-lf1-f69.google.com (mail-lf1-f69.google.com\n\t[209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-97-_o-9dV2ROPyNgmP17_zPQA-1; Tue, 03 Dec 2024 06:11:29 -0500","by mail-lf1-f69.google.com with SMTP id\n\t2adb3069b0e04-53e152253e4so416246e87.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 03 Dec 2024 03:11:29 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-434aa74edb1sm221530565e9.3.2024.12.03.03.11.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 03 Dec 2024 03:11:26 -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=\"heaZLI4G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1733224290;\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=09LSsJYySVp5U4fzlkVpWL2dtSHqzBrsEHa7nHfS+fQ=;\n\tb=heaZLI4GPrwjIv5hW+U77aPzYtGPZsjvoFJa8zLcUTCZ7CJG797Bgw5R5g6q4ouTv+Fayi\n\tLx9oXHfun33Zb4rTAK8v9AwGPwFZ4uFKQZ4DArWupyT4nqvjJkOcbuZqqJW+PjupUijjlR\n\tpq2eytp69nqUYSdPNwn9HBfkOm98GgY=","X-MC-Unique":"_o-9dV2ROPyNgmP17_zPQA-1","X-Mimecast-MFC-AGG-ID":"_o-9dV2ROPyNgmP17_zPQA","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733224287; x=1733829087;\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=09LSsJYySVp5U4fzlkVpWL2dtSHqzBrsEHa7nHfS+fQ=;\n\tb=H8HfADTtgd5s42nf6o2qRdE5hqt6eK03inmb0yOut6Nz7KDCxCaA6VrUn1/7TBvAKt\n\tPP6yLQQuOKlg0g9gioKu7PXGombH+jmfJtqKgdFQpBwpe/Gbm2l7timT5T0RKXgsmH1K\n\t/Rg+zNYqCIWv2J+GQgmuKRpvfKL9ITRDapBPlrVPIoHiFGw/jBr8Yy6RRQavfsnHOSLm\n\tIcdRb1AlD4+lYuWBFOqSBihETYbMyXOMkaryZKImz/XxFyLBdBRUB/KKGX2rCNg6wb8E\n\tYAQfPo2dQl6sDgFtCPT5CMyP8CQNdvzfqrd2vy2fRFPJD64elqItYUOk0Tan+pKe7jMv\n\tU8BA==","X-Gm-Message-State":"AOJu0YxQFdDjU8dAr5VTRvNA0E82FWP95v2Srzelh82aoezxZLZ2LE/K\n\t3brbJ1AITpwdLcY0N3uYJ27e+7spbCP0EUF2knn0kzujcUvd+TJvh8ZmGGnTqiMwK9h9DWjuUNj\n\tQ6r6i06cYH2qS8j96Ez116h+mc1Duw2nAE22CdvMSHqYekYEnjvmWP5QPjdYLZLKpqnusAALPfu\n\taPDSk=","X-Gm-Gg":"ASbGnctb1RjEuHLdJiQXOr5jqOjP6nNdARyT5zI6dTYwgNhNPvDW2hE5vhomNNISBqv\n\tbNyOqGHtJlfOpEh0PXnrEbCgoPhW/Vt0wiuGdDX+TEyEl5DsH1GuGkHvMLh9ghDz7zr2F28+YzO\n\tSVp08noBdQnL34LXE7YarNTuJ1FOjpPkX/5cy+01hAmJlTYZx4d5qSL/H0avg25/cUb07fHba+v\n\t2CTsblNzzq1m6RrDZKBgDX1+90yEUVEosNBMxMrsk4+cLfbckaywAm4aq26f7XsH2QZDA==","X-Received":["by 2002:a05:6512:3e03:b0:53e:14f7:4945 with SMTP id\n\t2adb3069b0e04-53e14f749f3mr843631e87.27.1733224287592; \n\tTue, 03 Dec 2024 03:11:27 -0800 (PST)","by 2002:a05:6512:3e03:b0:53e:14f7:4945 with SMTP id\n\t2adb3069b0e04-53e14f749f3mr843611e87.27.1733224287135; \n\tTue, 03 Dec 2024 03:11:27 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEmdCo+WhXDmq2y7Z1YNgMiVZwmPojBAjnypJNnSQlxt0MvZLL9vnV6Yfg9TcUUK2rOqU31Nw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Robert Mader <robert.mader@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","In-Reply-To":"<21af89c6-33d7-4616-8e6d-0d7c419d7747@collabora.com> (Robert\n\tMader's message of \"Tue, 3 Dec 2024 11:08:49 +0100\")","References":"<20241203093814.67984-1-mzamazal@redhat.com>\n\t<20241203093814.67984-2-mzamazal@redhat.com>\n\t<21af89c6-33d7-4616-8e6d-0d7c419d7747@collabora.com>","Date":"Tue, 03 Dec 2024 12:11:25 +0100","Message-ID":"<87mshdnhr6.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":"zyHihpxnqpeqU6KRXQUvFHH1CHIjEm9uBNHC-gueGZo_1733224288","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":"<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":32496,"web_url":"https://patchwork.libcamera.org/comment/32496/","msgid":"<Z07p5/man6D3pFvY@linux.intel.com>","date":"2024-12-03T11:22:15","subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"On Tue, Dec 03, 2024 at 10:38:13AM +0100, 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>\nReviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n\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 b4e32fe1..1d7d370b 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 67c688ae..52d59cab 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>  \tint32_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 ba3d5265..e1b6d3af 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\n> -- \n> 2.44.2\n>","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 C05DDBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Dec 2024 11:22:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AB3F6608A;\n\tTue,  3 Dec 2024 12:22:23 +0100 (CET)","from mgamail.intel.com (mgamail.intel.com [192.198.163.8])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7EE0C65FD6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Dec 2024 12:22:20 +0100 (CET)","from orviesa010.jf.intel.com ([10.64.159.150])\n\tby fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t03 Dec 2024 03:22:19 -0800","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.246.21.136]) by orviesa010-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Dec 2024 03:22:17 -0800"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"HWrOf27G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1733224941; x=1764760941;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=++7f2E/JnbNBT5+DgLjBgJTUxKu3fiH/XvwzhMhr3gA=;\n\tb=HWrOf27G/HKjo7Sz9++bt1hwMCJv22/S7PTBRnt5/FPKWjqCBlDA4u43\n\tzNoqsgAwWI3I1Dd4tcyqiYa2/1rAn0MALQIdhYK6qDsKGxyQOFhGFTzS8\n\tFmCmHoJXeKtw9sz0PKdD0arV/ypeIF/ObOwko7bv0IjQb5FI1tAnrx0WP\n\tNcZcAWYBljb4Awxd+lN96aowCS31WKkbPxudi04WU0eo1l9lamN0Fs3NK\n\t/FlWU8fQEm9AlgtnLIc4m/TN9XfLQTUcNh3tgTEDlm+lvq6by+iL7aOmq\n\ttVELDuKb8iaO1bcs+UzFaKPiB5KSk2VZZZVC/UTpAPxwP75gHkwXQj3si w==;","X-CSE-ConnectionGUID":["2+H+D8QRThGQA40eAGRRsw==","G4IksThvS/6bZq0YBEdlmQ=="],"X-CSE-MsgGUID":["wckxQgfhQ1G9PFv6lulKOg==","3I3O8c9pR86yd6jkgew+5w=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11274\"; a=\"50957340\"","E=Sophos;i=\"6.12,205,1728975600\"; d=\"scan'208\";a=\"50957340\"","E=Sophos;i=\"6.12,205,1728975600\"; d=\"scan'208\";a=\"93300550\""],"X-ExtLoop1":"1","Date":"Tue, 3 Dec 2024 12:22:15 +0100","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tRobert Mader <robert.mader@collabora.com>","Subject":"Re: [PATCH v2 1/1] libcamera: software_isp: Actually apply black\n\tlevel from tuning data","Message-ID":"<Z07p5/man6D3pFvY@linux.intel.com>","References":"<20241203093814.67984-1-mzamazal@redhat.com>\n\t<20241203093814.67984-2-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241203093814.67984-2-mzamazal@redhat.com>","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>"}}]