From patchwork Tue Dec 3 09:38:13 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 22154 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 41F28BDC71 for ; Tue, 3 Dec 2024 09:38:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id ECB2466081; Tue, 3 Dec 2024 10:38:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="i7sxdosm"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F136E66078 for ; Tue, 3 Dec 2024 10:38:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733218705; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nOoYMUv+/30pnE+RFHHBwaFJ4WuhvSn+J09Cvm5nJSo=; b=i7sxdosmyllTq0R/RfbVXTwsvabWQXRRGjK0QfC1/cm+8MTe7LqAHgegy6VrvaXKwiAeR+ kiGcAIbJSnVZOvqWXaANqF7fEBl7dUNT+nkClU0ajgI0aT7BXee5/DJh8N3qqorJmJ9tvx jKszTQBsrR0dqclsEqCQXOA04oN6FNY= Received: from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-587-gbenQodLMrGimH7RN6G3bA-1; Tue, 03 Dec 2024 04:38:24 -0500 X-MC-Unique: gbenQodLMrGimH7RN6G3bA-1 X-Mimecast-MFC-AGG-ID: gbenQodLMrGimH7RN6G3bA Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 60EA91955DC8; Tue, 3 Dec 2024 09:38:23 +0000 (UTC) Received: from nuthatch.brq.redhat.com (unknown [10.43.17.31]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 00FB230000DF; Tue, 3 Dec 2024 09:38:21 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal , Kieran Bingham , Robert Mader Subject: [PATCH v2 1/1] libcamera: software_isp: Actually apply black level from tuning data Date: Tue, 3 Dec 2024 10:38:13 +0100 Message-ID: <20241203093814.67984-2-mzamazal@redhat.com> In-Reply-To: <20241203093814.67984-1-mzamazal@redhat.com> References: <20241203093814.67984-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: GvFU0FE8T4-LDLe7c2XrqheluqW9kWS_R3-JAeoFUsg_1733218703 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The black level obtained from the tuning file in software ISP is retrieved in init (because this is the standard algorithm method with access to tuning data) and stored into context. But the context gets reset in configure and the black level is lost and never applied. Let's store the black level from the tuning file into an algorithm instance variable and put it into the context only later in configure. This is similar to what rkisp1 IPA does with the values obtained from the tuning file. Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 ("libcamera: software_isp: Clear IPA context on configure and stop") Signed-off-by: Milan Zamazal Tested-by: Robert Mader Reviewed-by: Stanislaw Gruszka --- src/ipa/simple/algorithms/blc.cpp | 7 +++++-- src/ipa/simple/algorithms/blc.h | 4 ++++ src/ipa/simple/soft_simple.cpp | 3 +-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp index b4e32fe1..1d7d370b 100644 --- a/src/ipa/simple/algorithms/blc.cpp +++ b/src/ipa/simple/algorithms/blc.cpp @@ -21,7 +21,8 @@ BlackLevel::BlackLevel() { } -int BlackLevel::init(IPAContext &context, const YamlObject &tuningData) +int BlackLevel::init([[maybe_unused]] IPAContext &context, + const YamlObject &tuningData) { auto blackLevel = tuningData["blackLevel"].get(); if (blackLevel.has_value()) { @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData) * Convert 16 bit values from the tuning file to 8 bit black * level for the SoftISP. */ - context.configuration.black.level = blackLevel.value() >> 8; + definedLevel_ = blackLevel.value() >> 8; } return 0; } @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData) int BlackLevel::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo) { + if (definedLevel_.has_value()) + context.configuration.black.level = definedLevel_; context.activeState.blc.level = context.configuration.black.level.value_or(255); return 0; diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h index 67c688ae..52d59cab 100644 --- a/src/ipa/simple/algorithms/blc.h +++ b/src/ipa/simple/algorithms/blc.h @@ -7,6 +7,9 @@ #pragma once +#include +#include + #include "algorithm.h" namespace libcamera { @@ -29,6 +32,7 @@ public: private: int32_t exposure_; double gain_; + std::optional definedLevel_; }; } /* namespace ipa::soft::algorithms */ diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index ba3d5265..e1b6d3af 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) (context_.configuration.agc.againMax - context_.configuration.agc.againMin) / 100.0; - if (!context_.configuration.black.level.has_value() && - camHelper_->blackLevel().has_value()) { + if (camHelper_->blackLevel().has_value()) { /* * The black level from camHelper_ is a 16 bit value, software ISP * works with 8 bit pixel values, both regardless of the actual