From patchwork Tue Apr 23 18:19:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19930 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 F0166BE08B for ; Tue, 23 Apr 2024 18:20:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A7D89633EC; Tue, 23 Apr 2024 20:20:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="VsEsCQNp"; 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 8EA6661AC9 for ; Tue, 23 Apr 2024 20:20:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713896420; 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=ej4J5SFxOLBaVAU2/GxZbptxItgAs3voArS0ckiAhY4=; b=VsEsCQNp63ayZflM6eQOnzShfVxuq8gTvjAfCYpIl5L5tyWtaa3Hqsp3XAA0npUrwJonfl Axz2GBUsyhDXS0dX+iwfK/OqwHpps3Xzxpmi7phiMgYNifHE30fnRsCEE8BnWNtXftaNVa 6/E1szXtRYw6EO5tflah+c5oNwnzLM4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-227-WcX68h_iOFOJ9vT1dNlL_Q-1; Tue, 23 Apr 2024 14:20:18 -0400 X-MC-Unique: WcX68h_iOFOJ9vT1dNlL_Q-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id A09901049C93 for ; Tue, 23 Apr 2024 18:20:18 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.63]) by smtp.corp.redhat.com (Postfix) with ESMTP id E2187151EF; Tue, 23 Apr 2024 18:20:17 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 1/5] libcamera: software_isp: Use a specific integer type for black level Date: Tue, 23 Apr 2024 20:19:56 +0200 Message-ID: <20240423182000.1527425-2-mzamazal@redhat.com> In-Reply-To: <20240423182000.1527425-1-mzamazal@redhat.com> References: <20240423182000.1527425-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 documented range of values corresponds to uint8_t, so let's use that type. Signed-off-by: Milan Zamazal Reviewed-by: Laurent Pinchart --- src/ipa/simple/black_level.cpp | 3 ++- src/ipa/simple/black_level.h | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp index c7e8d8b7..a9ab12a3 100644 --- a/src/ipa/simple/black_level.cpp +++ b/src/ipa/simple/black_level.cpp @@ -8,6 +8,7 @@ #include "black_level.h" #include +#include #include @@ -43,7 +44,7 @@ BlackLevel::BlackLevel() * \return The black level, in the range from 0 (minimum) to 255 (maximum). * If the black level couldn't be determined yet, return 0. */ -unsigned int BlackLevel::get() const +uint8_t BlackLevel::get() const { return blackLevelSet_ ? blackLevel_ : 0; } diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h index 7e37757e..85bc5d8e 100644 --- a/src/ipa/simple/black_level.h +++ b/src/ipa/simple/black_level.h @@ -17,11 +17,11 @@ class BlackLevel { public: BlackLevel(); - unsigned int get() const; + uint8_t get() const; void update(SwIspStats::Histogram &yHistogram); private: - unsigned int blackLevel_; + uint8_t blackLevel_; bool blackLevelSet_; }; From patchwork Tue Apr 23 18:19:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19931 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 E8441BE08B for ; Tue, 23 Apr 2024 18:20:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 87468633F5; Tue, 23 Apr 2024 20:20:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="AYMMg8W4"; 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 6473D61B17 for ; Tue, 23 Apr 2024 20:20:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713896422; 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=v9K/zX61MGjH9nw2VP8CZIwhNLdceCV48IGeJqmdDIk=; b=AYMMg8W4a+PiLlVVpiH/mn5jm7OZh/rxnhHBZgLAD6g8XG1t07aEVhSvcWMm8NBA6I/SyL mAQBhmvqf8s+uZQBFM9HSaAdsrnGlhDqyad+EjpDqK/Jhvp+uEKbMzmVtFBKcCHCW1JYjr gsBIJR+qUu0lj8g5PifIBh2PJf1uK74= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-531-lKzI_-VkPsaC_UF_pP7ChQ-1; Tue, 23 Apr 2024 14:20:21 -0400 X-MC-Unique: lKzI_-VkPsaC_UF_pP7ChQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id D87AD1049CA4 for ; Tue, 23 Apr 2024 18:20:19 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.63]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0CA0449102; Tue, 23 Apr 2024 18:20:18 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 2/5] libcamera: software_isp: Honor black level in AWB Date: Tue, 23 Apr 2024 20:19:57 +0200 Message-ID: <20240423182000.1527425-3-mzamazal@redhat.com> In-Reply-To: <20240423182000.1527425-1-mzamazal@redhat.com> References: <20240423182000.1527425-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 white balance computation didn't consider black level. This is wrong because then the computed ratios are off when they are computed from the whole brightness range rather than the sensor range. This patch adjusts white balance computation for the black level. There is no need to change white balance application in debayering as this is already applied after black level correction. Exposure computation already subtracts black level, no changes are needed there. Signed-off-by: Milan Zamazal --- src/ipa/simple/soft_simple.cpp | 43 ++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index b9fb58b5..30956a19 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -5,6 +5,8 @@ * soft_simple.cpp - Simple Software Image Processing Algorithm module */ +#include +#include #include #include @@ -240,28 +242,45 @@ void IPASoftSimple::stop() void IPASoftSimple::processStats(const ControlList &sensorControls) { + SwIspStats::Histogram histogram = stats_->yHistogram; + if (ignoreUpdates_ > 0) + blackLevel_.update(histogram); + const uint8_t blackLevel = blackLevel_.get(); + params_->blackLevel = blackLevel; + /* * Calculate red and blue gains for AWB. * Clamp max gain at 4.0, this also avoids 0 division. */ - if (stats_->sumR_ <= stats_->sumG_ / 4) - params_->gainR = 1024; - else - params_->gainR = 256 * stats_->sumG_ / stats_->sumR_; + { + const uint64_t nPixels = std::accumulate( + histogram.begin(), histogram.end(), 0); + auto subtractBlackLevel = [blackLevel, nPixels]( + uint64_t sum, uint64_t div) + -> uint64_t { + uint64_t reducedSum = sum - blackLevel * nPixels / div; + uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); + return spreadSum; + }; + const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); + const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); + const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); + + if (sumR <= sumG / 4) + params_->gainR = 1024; + else + params_->gainR = 256 * sumG / sumR; - if (stats_->sumB_ <= stats_->sumG_ / 4) - params_->gainB = 1024; - else - params_->gainB = 256 * stats_->sumG_ / stats_->sumB_; + if (sumB <= sumG / 4) + params_->gainB = 1024; + else + params_->gainB = 256 * sumG / sumB; + } /* Green gain and gamma values are fixed */ params_->gainG = 256; params_->gamma = 0.5; - if (ignoreUpdates_ > 0) - blackLevel_.update(stats_->yHistogram); - params_->blackLevel = blackLevel_.get(); - setIspParams.emit(); /* \todo Switch to the libipa/algorithm.h API someday. */ From patchwork Tue Apr 23 18:19:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19932 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 CF15EBE08B for ; Tue, 23 Apr 2024 18:20:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 812C2633FC; Tue, 23 Apr 2024 20:20:27 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="ihNhN7Or"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D8A92633F8 for ; Tue, 23 Apr 2024 20:20:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713896423; 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=+onVB9djwQpKEjveedl/LlBR5C1s7AQt2hGdPONjIPM=; b=ihNhN7OrLQxapeqy2DaHaTQhhcR7TqnlnX03mKxkveucuY+o3Jy5UUh1YghG/jiTN5YfdD Erjgu67lFc/z9RZKTa2CEfe51FtvAw4TjH+WVQmODWu3N2aeYHdfPJsJGrURNANU5a/kom lsfcIsKhYFt04LDGKtGv5lRhxeMoZZs= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-508-DHwMvoJCPDK-gIbql2LQ2w-1; Tue, 23 Apr 2024 14:20:21 -0400 X-MC-Unique: DHwMvoJCPDK-gIbql2LQ2w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4EAFC3C13A90 for ; Tue, 23 Apr 2024 18:20:21 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.63]) by smtp.corp.redhat.com (Postfix) with ESMTP id 448F1151EF; Tue, 23 Apr 2024 18:20:20 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 3/5] libcamera: software_isp: Move color mappings out of debayering Date: Tue, 23 Apr 2024 20:19:58 +0200 Message-ID: <20240423182000.1527425-4-mzamazal@redhat.com> In-Reply-To: <20240423182000.1527425-1-mzamazal@redhat.com> References: <20240423182000.1527425-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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" Constructing the color mapping tables is related to stats rather than debayering, where they are applied. Let's move the corresponding code to stats processing. This is a preliminary step towards building this functionality on top of libipa/algorithm.h, which should follow. Signed-off-by: Milan Zamazal --- .../internal/software_isp/debayer_params.h | 18 ++-- src/ipa/simple/soft_simple.cpp | 84 ++++++++++++------- src/libcamera/software_isp/debayer.cpp | 29 ++++--- src/libcamera/software_isp/debayer_cpu.cpp | 41 ++------- src/libcamera/software_isp/debayer_cpu.h | 9 +- src/libcamera/software_isp/software_isp.cpp | 4 +- 6 files changed, 87 insertions(+), 98 deletions(-) diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h index 32cd448a..09f4ff00 100644 --- a/include/libcamera/internal/software_isp/debayer_params.h +++ b/include/libcamera/internal/software_isp/debayer_params.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* - * Copyright (C) 2023, Red Hat Inc. + * Copyright (C) 2023, 2024 Red Hat Inc. * * Authors: * Hans de Goede @@ -10,20 +10,20 @@ #pragma once +#include +#include + namespace libcamera { struct DebayerParams { static constexpr unsigned int kGain10 = 256; + static constexpr unsigned int kRGBLookupSize = 256; - unsigned int gainR; - unsigned int gainG; - unsigned int gainB; + using ColorLookupTable = std::array; - float gamma; - /** - * \brief Level of the black point, 0..255, 0 is no correction. - */ - unsigned int blackLevel; + ColorLookupTable red; + ColorLookupTable green; + ColorLookupTable blue; }; } /* namespace libcamera */ diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 30956a19..c5085f71 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -5,6 +5,7 @@ * soft_simple.cpp - Simple Software Image Processing Algorithm module */ +#include #include #include #include @@ -84,6 +85,10 @@ private: ControlInfoMap sensorInfoMap_; BlackLevel blackLevel_; + static constexpr unsigned int kGammaLookupSize = 1024; + std::array gammaTable_; + int lastBlackLevel_ = -1; + int32_t exposureMin_, exposureMax_; int32_t exposure_; double againMin_, againMax_, againMinStep_; @@ -246,40 +251,59 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) if (ignoreUpdates_ > 0) blackLevel_.update(histogram); const uint8_t blackLevel = blackLevel_.get(); - params_->blackLevel = blackLevel; /* * Calculate red and blue gains for AWB. * Clamp max gain at 4.0, this also avoids 0 division. */ - { - const uint64_t nPixels = std::accumulate( - histogram.begin(), histogram.end(), 0); - auto subtractBlackLevel = [blackLevel, nPixels]( - uint64_t sum, uint64_t div) - -> uint64_t { - uint64_t reducedSum = sum - blackLevel * nPixels / div; - uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); - return spreadSum; - }; - const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); - const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); - const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); - - if (sumR <= sumG / 4) - params_->gainR = 1024; - else - params_->gainR = 256 * sumG / sumR; - - if (sumB <= sumG / 4) - params_->gainB = 1024; - else - params_->gainB = 256 * sumG / sumB; + const uint64_t nPixels = std::accumulate( + histogram.begin(), histogram.end(), 0); + auto subtractBlackLevel = [blackLevel, nPixels]( + uint64_t sum, uint64_t div) + -> uint64_t { + const uint64_t reducedSum = sum - blackLevel * nPixels / div; + const uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); + return spreadSum; + }; + const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); + const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); + const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); + + /* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */ + unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR; + unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB; + /* Green gain and gamma values are fixed */ + constexpr unsigned int gainG = 256; + /* gamma == 1.0 means no correction */ + constexpr float gamma = 0.5; + + /* Update the gamma table if needed */ + if (blackLevel != lastBlackLevel_) { + const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256; + std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0); + const float divisor = kGammaLookupSize - blackIndex - 1.0; + for (unsigned int i = blackIndex; i < kGammaLookupSize; i++) + gammaTable_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, gamma); + + lastBlackLevel_ = blackLevel; } - /* Green gain and gamma values are fixed */ - params_->gainG = 256; - params_->gamma = 0.5; + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { + constexpr unsigned int div = + DebayerParams::kRGBLookupSize * DebayerParams::kGain10 / + kGammaLookupSize; + unsigned int idx; + + /* Apply gamma after gain! */ + idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) }); + params_->red[i] = gammaTable_[idx]; + + idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) }); + params_->green[i] = gammaTable_[idx]; + + idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) }); + params_->blue[i] = gammaTable_[idx]; + } setIspParams.emit(); @@ -300,7 +324,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf */ const unsigned int blackLevelHistIdx = - params_->blackLevel / (256 / SwIspStats::kYHistogramSize); + blackLevel / (256 / SwIspStats::kYHistogramSize); const unsigned int histogramSize = SwIspStats::kYHistogramSize - blackLevelHistIdx; const unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount; @@ -348,8 +372,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV << " exp " << exposure_ << " again " << again_ - << " gain R/B " << params_->gainR << "/" << params_->gainB - << " black level " << params_->blackLevel; + << " gain R/B " << gainR << "/" << gainB + << " black level " << blackLevel; } void IPASoftSimple::updateExposure(double exposureMSV) diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp index 1c035e9b..ac438a33 100644 --- a/src/libcamera/software_isp/debayer.cpp +++ b/src/libcamera/software_isp/debayer.cpp @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* * Copyright (C) 2023, Linaro Ltd - * Copyright (C) 2023, Red Hat Inc. + * Copyright (C) 2023, 2024 Red Hat Inc. * * Authors: * Hans de Goede @@ -24,29 +24,28 @@ namespace libcamera { */ /** - * \var DebayerParams::gainR - * \brief Red gain - * - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. + * \var DebayerParams::kRGBLookupSize + * \brief Size of a color lookup table */ /** - * \var DebayerParams::gainG - * \brief Green gain - * - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. + * \typedef DebayerParams::ColorLookupTable + * \brief Type of the lookup tables for red, green, blue values */ /** - * \var DebayerParams::gainB - * \brief Blue gain - * - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. + * \var DebayerParams::red + * \brief Lookup table for red color, mapping input values to output values + */ + +/** + * \var DebayerParams::green + * \brief Lookup table for green color, mapping input values to output values */ /** - * \var DebayerParams::gamma - * \brief Gamma correction, 1.0 is no correction + * \var DebayerParams::blue + * \brief Lookup table for blue color, mapping input values to output values */ /** diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 88d6578b..8b2b2f40 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -11,7 +11,6 @@ #include "debayer_cpu.h" -#include #include #include @@ -47,9 +46,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats) */ enableInputMemcpy_ = true; - /* Initialize gamma to 1.0 curve */ - for (unsigned int i = 0; i < kGammaLookupSize; i++) - gamma_[i] = i / (kGammaLookupSize / kRGBLookupSize); + /* Initialize color lookup tables */ + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) + red_[i] = green_[i] = blue_[i] = i; for (unsigned int i = 0; i < kMaxLineBuffers; i++) lineBuffers_[i] = nullptr; @@ -698,37 +697,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime); } - /* Apply DebayerParams */ - if (params.gamma != gammaCorrection_ || params.blackLevel != blackLevel_) { - const unsigned int blackIndex = - params.blackLevel * kGammaLookupSize / 256; - std::fill(gamma_.begin(), gamma_.begin() + blackIndex, 0); - const float divisor = kGammaLookupSize - blackIndex - 1.0; - for (unsigned int i = blackIndex; i < kGammaLookupSize; i++) - gamma_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, params.gamma); - - gammaCorrection_ = params.gamma; - blackLevel_ = params.blackLevel; - } - - if (swapRedBlueGains_) - std::swap(params.gainR, params.gainB); - - for (unsigned int i = 0; i < kRGBLookupSize; i++) { - constexpr unsigned int div = - kRGBLookupSize * DebayerParams::kGain10 / kGammaLookupSize; - unsigned int idx; - - /* Apply gamma after gain! */ - idx = std::min({ i * params.gainR / div, (kGammaLookupSize - 1) }); - red_[i] = gamma_[idx]; - - idx = std::min({ i * params.gainG / div, (kGammaLookupSize - 1) }); - green_[i] = gamma_[idx]; - - idx = std::min({ i * params.gainB / div, (kGammaLookupSize - 1) }); - blue_[i] = gamma_[idx]; - } + green_ = params.green; + red_ = swapRedBlueGains_ ? params.blue : params.red; + blue_ = swapRedBlueGains_ ? params.red : params.blue; /* Copy metadata from the input buffer */ FrameMetadata &metadata = output->_d()->metadata(); diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index 689c1075..47373426 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -122,15 +122,12 @@ private: void process2(const uint8_t *src, uint8_t *dst); void process4(const uint8_t *src, uint8_t *dst); - static constexpr unsigned int kGammaLookupSize = 1024; - static constexpr unsigned int kRGBLookupSize = 256; /* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */ static constexpr unsigned int kMaxLineBuffers = 5; - std::array gamma_; - std::array red_; - std::array green_; - std::array blue_; + DebayerParams::ColorLookupTable red_; + DebayerParams::ColorLookupTable green_; + DebayerParams::ColorLookupTable blue_; debayerFn debayer0_; debayerFn debayer1_; debayerFn debayer2_; diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index e4e56086..3e07453d 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -63,9 +63,7 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) * handler */ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) - : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, - DebayerParams::kGain10, 0.5f, 0 }, - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) + : dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) { if (!dmaHeap_.isValid()) { LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object"; From patchwork Tue Apr 23 18:19:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19933 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 0BE60C328D for ; Tue, 23 Apr 2024 18:20:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 51107633FE; Tue, 23 Apr 2024 20:20:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="QlJtMV4m"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BB5F2633EC for ; Tue, 23 Apr 2024 20:20:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713896424; 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=9iPHvaerPCvm1J8fb11HBL7RWIiLE6dX/qwpyR82Kfw=; b=QlJtMV4mBAF2/z98RZ1JeV6bW4DAhm7mZFwrN/UQjNcqHbnG384W4c6ytirG6p5W5Ynwau 3YhehECfyqHtfN4CfoEAmdHyZeI/n09U6eJvV8quGYyB59P8wpNOGdFfb3Lz06Topfx6JW +ZQP4yzl6ZMYDO+lLlPeTBo5oLe/1gU= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-18-UUBtCbdGM8eGlgW3UzAiBA-1; Tue, 23 Apr 2024 14:20:22 -0400 X-MC-Unique: UUBtCbdGM8eGlgW3UzAiBA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 87FBA29AC00A for ; Tue, 23 Apr 2024 18:20:22 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.63]) by smtp.corp.redhat.com (Postfix) with ESMTP id AE81649102; Tue, 23 Apr 2024 18:20:21 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 4/5] libcamera: software_isp: Remove TODO #13 Date: Tue, 23 Apr 2024 20:19:59 +0200 Message-ID: <20240423182000.1527425-5-mzamazal@redhat.com> In-Reply-To: <20240423182000.1527425-1-mzamazal@redhat.com> References: <20240423182000.1527425-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 current software ISP debayering implementation uses color lookup tables that apply black level subtraction, color gain and gamma correction in a single step, while performing debayering. In theory, black level subtraction and color gains should be applied before debayering and gamma correction after debayering. But because black level subtraction and color gain corrections are currently linear and we average only same-color pixels in debayering, we can apply all the operations after debayering. The only difference is with clipping where out-of-range pixels contribute more than they deserve but this is not generally significant. Combining all the operations at once is conceptually not ideal but it is not incorrect in this case and saves CPU cycles, which is critical for software ISP CPU implementation (it may be less important with future GPU implementation). This means we need the current implementation. It may get replaced or become optional (configurable) in future, for example if the separation is needed due to introducing other image processing operations. Under these circumstances and considering that the lookup tables construction has been moved out of the debayering code in the preceding patch, it makes no sense to keep software ISP TODO #13. Let's remove it in favor of a clarifying code comment. Signed-off-by: Milan Zamazal --- src/ipa/simple/soft_simple.cpp | 10 ++++++++++ src/libcamera/software_isp/TODO | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index c5085f71..b2f4d308 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -247,6 +247,16 @@ void IPASoftSimple::stop() void IPASoftSimple::processStats(const ControlList &sensorControls) { + /* + * Here black level subtraction, color gains and gamma correction are + * combined into a single operation (table lookup) to save CPU cycles. + * This works because black level subtraction and color gains are currently + * linear operations and we average only same-color pixels in debayering. + * If we change anything on this or introduce other operations in between, + * it may be needed to split the operations and perform black and gain + * corrections before debayering and gamma correction after debayering. + */ + SwIspStats::Histogram histogram = stats_->yHistogram; if (ignoreUpdates_ > 0) blackLevel_.update(histogram); diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index 4fcee39b..fcb02588 100644 --- a/src/libcamera/software_isp/TODO +++ b/src/libcamera/software_isp/TODO @@ -267,13 +267,3 @@ This could be handled better with DelayedControls. > V4L2_CID_EXPOSURE })); You should use the DelayedControls class. - ---- - -13. Improve black level and colour gains application - -I think the black level should eventually be moved before debayering, and -ideally the colour gains as well. I understand the need for optimizations to -lower the CPU consumption, but at the same time I don't feel comfortable -building up on top of an implementation that may work a bit more by chance than -by correctness, as that's not very maintainable. From patchwork Tue Apr 23 18:20:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19934 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 382E8BE08B for ; Tue, 23 Apr 2024 18:20:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C7A93633F3; Tue, 23 Apr 2024 20:20:30 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="Rtn7u+fi"; 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 276F1633EC for ; Tue, 23 Apr 2024 20:20:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713896425; 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=7138dUVxFMU7AXzdbakfQfOpBfpMWwkz4oaLCF5Pk/U=; b=Rtn7u+fiBHmBI9ksH/IuNpf2j/TZLAWW9xXNLXTM5RLi1fcmyzshZrDh5K+LvZwQMyGLlJ yKQM6WbboVMhG7LLTklNmPUJbR3TpsTSMqdFiw1vaGMW+ofOPTEz25RvYKGJSGPXEt5viH buDRcF/o7cIccNUI3k7f5biB2OzVQCo= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-683-5yQPMmqpNH2ttyvioeNpzw-1; Tue, 23 Apr 2024 14:20:23 -0400 X-MC-Unique: 5yQPMmqpNH2ttyvioeNpzw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id A526929AC012 for ; Tue, 23 Apr 2024 18:20:23 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.63]) by smtp.corp.redhat.com (Postfix) with ESMTP id E6C9649102; Tue, 23 Apr 2024 18:20:22 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 5/5] libcamera: software_isp: Remove DebayerParams::kGain10 Date: Tue, 23 Apr 2024 20:20:00 +0200 Message-ID: <20240423182000.1527425-6-mzamazal@redhat.com> In-Reply-To: <20240423182000.1527425-1-mzamazal@redhat.com> References: <20240423182000.1527425-1-mzamazal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 constant is used in a single place internally and doesn't belong to DebayerParams anymore. Let's use 256 directly. Completes software ISP TODO #4. Signed-off-by: Milan Zamazal --- .../internal/software_isp/debayer_params.h | 1 - src/ipa/simple/soft_simple.cpp | 3 +-- src/libcamera/software_isp/TODO | 13 ------------- src/libcamera/software_isp/debayer.cpp | 5 ----- 4 files changed, 1 insertion(+), 21 deletions(-) diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h index 09f4ff00..6aaa7d4c 100644 --- a/include/libcamera/internal/software_isp/debayer_params.h +++ b/include/libcamera/internal/software_isp/debayer_params.h @@ -16,7 +16,6 @@ namespace libcamera { struct DebayerParams { - static constexpr unsigned int kGain10 = 256; static constexpr unsigned int kRGBLookupSize = 256; using ColorLookupTable = std::array; diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index b2f4d308..5298836c 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -300,8 +300,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { constexpr unsigned int div = - DebayerParams::kRGBLookupSize * DebayerParams::kGain10 / - kGammaLookupSize; + DebayerParams::kRGBLookupSize * 256 / kGammaLookupSize; unsigned int idx; /* Apply gamma after gain! */ diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index fcb02588..f8f00139 100644 --- a/src/libcamera/software_isp/TODO +++ b/src/libcamera/software_isp/TODO @@ -72,19 +72,6 @@ stats in hardware, such as the i.MX7), but please keep it on your radar. --- -4. Hide internal representation of gains from callers - -> struct DebayerParams { -> static constexpr unsigned int kGain10 = 256; - -Forcing the caller to deal with the internal representation of gains -isn't nice, especially given that it precludes implementing gains of -different precisions in different backend. Wouldn't it be better to pass -the values as floating point numbers, and convert them to the internal -representation in the implementation of process() before using them ? - ---- - 5. Store ISP parameters in per-frame buffers > /** diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp index ac438a33..548c2ccd 100644 --- a/src/libcamera/software_isp/debayer.cpp +++ b/src/libcamera/software_isp/debayer.cpp @@ -18,11 +18,6 @@ namespace libcamera { * \brief Struct to hold the debayer parameters. */ -/** - * \var DebayerParams::kGain10 - * \brief const value for 1.0 gain - */ - /** * \var DebayerParams::kRGBLookupSize * \brief Size of a color lookup table