From patchwork Wed Feb 10 17:58:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 11209 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 23CF9BD160 for ; Wed, 10 Feb 2021 17:58:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E4B6D61646; Wed, 10 Feb 2021 18:58:38 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="h8nxRuoh"; dkim-atps=neutral Received: from mail-ej1-x636.google.com (mail-ej1-x636.google.com [IPv6:2a00:1450:4864:20::636]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 08A1C6162E for ; Wed, 10 Feb 2021 18:58:36 +0100 (CET) Received: by mail-ej1-x636.google.com with SMTP id w1so5706301ejf.11 for ; Wed, 10 Feb 2021 09:58:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=W6MmJZJLGnwy72pggNNug9dowL58ofDKZhSOvEPLblo=; b=h8nxRuoh1SARBZMvIbkIUJtjZ3mv+G93EB0YtsDMHD2TmfL3K/BaE9FCr2XwsCalZ2 /MVSZ6IWqJAp/VmCP1R3JZDPs27t9JxADDZoI+KQci92ANsvx9GeunnCnhPe/C2NSI5I lfjG/19LxdiBRSkR5Cx/WBLMTI3re9ZzdhUxDw4Jeuxv5nDBJZGZFuonfXKECCZ0/U8V uMZRaUiCmGxhrwrAHSuyC0gvgJSfoU2ZG8X6wb0sL2AoAvde6wq6qbssfyMB7wjOU3GT ednNPUm8DCuAPMuvDbtw09Pu3+HaRysAZcnG9dOvpGJ2DnS/Yz6slDaxujr8Z7BpQilJ /CCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=W6MmJZJLGnwy72pggNNug9dowL58ofDKZhSOvEPLblo=; b=DvDdm2fGczXUzZM5aWubnkn5tCVQ4+4MG3OovfYGucW0MR3Z8UPkUWGhLBs226Ka75 TAAyDS6VgnAshCxGnrXOl0YDyFc33qWbhUNoY2ztSl0wsLAI8cPCaGCf5ws7LmkqKRq7 Wvjtjbm6nqk3cG4ER6/dmSNVANUtDcP6GhUyFdSHoZsQ16jeeHYK2KcgeTTDXC6xhPbQ 1MP0hVEsZtq1XlQbFSrtX7nYIkX8+E7xwTyvN7l79wiBz1qiWnBgOozqXgCae2h1WOrY Gy/SvowX+Gqvfnop3xJTPgjdHnWWk5EpGsGF/Z/69NnTN4vkIhw4EQPRfmeI3FlMfUca Mfdg== X-Gm-Message-State: AOAM531zYOzBTKfxbSL5TcIK+L+Z3aWC3vJ6t2WHcTrS8P/SFjJr7QiT nDQWHr2dxpzKRS+bAeXfyQUMNYhddW/JfVTd X-Google-Smtp-Source: ABdhPJyT8/ys/+BEdL3pXcuAb+cMB+4xo49RL9kSr2oFw4yXT+DYVxprW4PrTQcOgnPHt3I20WybFg== X-Received: by 2002:a17:906:d19b:: with SMTP id c27mr4159864ejz.234.1612979915186; Wed, 10 Feb 2021 09:58:35 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id hc13sm1660483ejc.5.2021.02.10.09.58.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Feb 2021 09:58:34 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Feb 2021 17:58:29 +0000 Message-Id: <20210210175830.32038-2-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210210175830.32038-1-david.plowman@raspberrypi.com> References: <20210210175830.32038-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/2] ipa: raspberrypi: AWB: Remove unnecessary frame count variable 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 variable frame_count2_ is not needed as Prepare() and Process() always run in lock step one after the other. Signed-off-by: David Plowman Reviewed-by: Laurent Pinchart Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/controller/rpi/awb.cpp | 6 ++---- src/ipa/raspberrypi/controller/rpi/awb.hpp | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp index 791b5108..1c65eda8 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp @@ -153,7 +153,7 @@ void Awb::Read(boost::property_tree::ptree const ¶ms) void Awb::Initialise() { - frame_count2_ = frame_count_ = frame_phase_ = 0; + frame_count_ = frame_phase_ = 0; // Put something sane into the status that we are filtering towards, // just in case the first few frames don't have anything meaningful in // them. @@ -288,11 +288,9 @@ void Awb::Process(StatisticsPtr &stats, Metadata *image_metadata) // Count frames since we last poked the async thread. if (frame_phase_ < (int)config_.frame_period) frame_phase_++; - if (frame_count2_ < (int)config_.startup_frames) - frame_count2_++; LOG(RPiAwb, Debug) << "frame_phase " << frame_phase_; if (frame_phase_ >= (int)config_.frame_period || - frame_count2_ < (int)config_.startup_frames) { + frame_count_ < (int)config_.startup_frames) { // Update any settings and any image metadata that we need. struct LuxStatus lux_status = {}; lux_status.lux = 400; // in case no metadata diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp index 1b39ab4b..f113c642 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp @@ -130,7 +130,6 @@ private: // counts up to frame_period before restarting the async thread int frame_phase_; int frame_count_; // counts up to startup_frames - int frame_count2_; // counts up to startup_frames for Process method AwbStatus sync_results_; AwbStatus prev_sync_results_; std::string mode_name_; From patchwork Wed Feb 10 17:58:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 11210 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 0E56EBD160 for ; Wed, 10 Feb 2021 17:58:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6C8C861639; Wed, 10 Feb 2021 18:58:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="mU8s4a3b"; dkim-atps=neutral Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1452261637 for ; Wed, 10 Feb 2021 18:58:37 +0100 (CET) Received: by mail-ej1-x62b.google.com with SMTP id l25so5734612eja.9 for ; Wed, 10 Feb 2021 09:58:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=s6vHWsx0xIwncY54ZT/xyjBtApBBVfKf/VoaVcjGM1Y=; b=mU8s4a3bqqGf7UnalEQAUeZigEsEXCOEcWb/1F6/kqi7LQhuJmY1+K/er1Rs++bwLK y0hoN9jtRA5dtGzRjqxjefkXQpNvLRMfRdzKTMFo+Tqvgr8bcu6qMXz7JemJlGW1+pNF xG3OuxwrGT5kGwUTNIbX3TFlS5CgiYeOl/o8oJ/nqfMDo6K/ECtZc0eHsBdGG+KCGQrn SZcZZceXrw2uIxT24S/0fYen2k8+LJg2K9V0DhXP2xTzqRUvVaxv2BFFGl6uuF8Yy+Id IxaCOaBV/YqoG/fZX3zjjK7Ofolh1l7tnQ+8Hs5CQLERR2EP1SmTm9G79ZXwOnRSdDsm AHuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=s6vHWsx0xIwncY54ZT/xyjBtApBBVfKf/VoaVcjGM1Y=; b=crgnM3D9UeIcqfWlbzo78fr0K5tjWY2NwRLqNfPoOkjtMzv/XQM/rwHsdfGmMy332k NXIVPCmEzwCq4G0jGH8oRcYTZuxj5mg0vT3hxCGP8DQNHRl3yXw9NAk4hT4Fr3skeuy6 Lt/Lw3W9vZAEVltAV2Ild6usUGd9xbfueHoxn12VY772Qyg6Kcn9GXesXZ30MGzJJ00G igITpJwSasH4moi/tOAaqqwobGzC3AyWYtiZyt6ziE+dJpoW6jF/HH6u4kUWvwvRe/tN Zdlo6iwbpJ8FXUt5jLbXiOUS8u1HezmJlD5c9v1QI/vram73w0PSI9/+IKygcWgpWwBE OHCg== X-Gm-Message-State: AOAM531O8C7o+krqAIEezGEaVcu6KI0ZkCpUOMbS0/hokfrNPmAr/ijt z1nzEQO+dzlq4O/UJ+1Juj5ygioFtHNn0rrB X-Google-Smtp-Source: ABdhPJye4g6mpZot2BurxR+24IlL90mhoitsCtmTes5NkquBm++w7M0Ji9ZuQ1Qi+/+T8MX7QoqPUA== X-Received: by 2002:a17:906:3656:: with SMTP id r22mr4059275ejb.14.1612979916360; Wed, 10 Feb 2021 09:58:36 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id hc13sm1660483ejc.5.2021.02.10.09.58.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Feb 2021 09:58:35 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Feb 2021 17:58:30 +0000 Message-Id: <20210210175830.32038-3-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210210175830.32038-1-david.plowman@raspberrypi.com> References: <20210210175830.32038-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: AWB: Fix race condition setting manual gains 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" Applying the manual_r_ and manual_b_ values is entirely removed from the asynchronous thread where their use constituted a race hazard. The main thread now deals with them entirely, involving the following changes. 1. SetManualGains() applies the new values directly to the "sync_results", meaning that Prepare() will jump to the new values immediately (which is a better behaviour). 2. Process() does not restart the asynchronous thread when manual gains are in force. 3. The asynchronous thread might be running when manual gains are set, so we ignore the results produced in this case. Signed-off-by: David Plowman Reviewed-by: Laurent Pinchart Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/controller/rpi/awb.cpp | 86 +++++++++++----------- src/ipa/raspberrypi/controller/rpi/awb.hpp | 1 + 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp index 1c65eda8..bb637f10 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp @@ -175,9 +175,9 @@ void Awb::Initialise() unsigned int Awb::GetConvergenceFrames() const { - // If colour gains have been explicitly set, there is no convergence + // If not in auto mode, there is no convergence // to happen, so no need to drop any frames - return zero. - if (manual_r_ && manual_b_) + if (!isAutoEnabled()) return 0; else return config_.convergence_frames; @@ -193,38 +193,47 @@ void Awb::SetManualGains(double manual_r, double manual_b) // If any of these are 0.0, we swich back to auto. manual_r_ = manual_r; manual_b_ = manual_b; + // If not in auto mode, set these values into the sync_results which + // means that Prepare() will adopt them immediately. + if (!isAutoEnabled()) { + sync_results_.gain_r = prev_sync_results_.gain_r = manual_r_; + sync_results_.gain_g = prev_sync_results_.gain_g = 1.0; + sync_results_.gain_b = prev_sync_results_.gain_b = manual_b_; + } } void Awb::SwitchMode([[maybe_unused]] CameraMode const &camera_mode, Metadata *metadata) { - // If fixed colour gains have been set, we should let other algorithms - // know by writing it into the image metadata. - if (manual_r_ != 0.0 && manual_b_ != 0.0) { - prev_sync_results_.gain_r = manual_r_; - prev_sync_results_.gain_g = 1.0; - prev_sync_results_.gain_b = manual_b_; - // If we're starting up for the first time, try and - // "dead reckon" the corresponding colour temperature. - if (first_switch_mode_ && config_.bayes) { - Pwl ct_r_inverse = config_.ct_r.Inverse(); - Pwl ct_b_inverse = config_.ct_b.Inverse(); - double ct_r = ct_r_inverse.Eval(ct_r_inverse.Domain().Clip(1 / manual_r_)); - double ct_b = ct_b_inverse.Eval(ct_b_inverse.Domain().Clip(1 / manual_b_)); - prev_sync_results_.temperature_K = (ct_r + ct_b) / 2; - } - sync_results_ = prev_sync_results_; + // On the first mode switch we'll have no meaningful colour + // temperature, so try to dead reckon one if in manual mode. + if (!isAutoEnabled() && first_switch_mode_ && config_.bayes) { + Pwl ct_r_inverse = config_.ct_r.Inverse(); + Pwl ct_b_inverse = config_.ct_b.Inverse(); + double ct_r = ct_r_inverse.Eval(ct_r_inverse.Domain().Clip(1 / manual_r_)); + double ct_b = ct_b_inverse.Eval(ct_b_inverse.Domain().Clip(1 / manual_b_)); + prev_sync_results_.temperature_K = (ct_r + ct_b) / 2; + sync_results_.temperature_K = prev_sync_results_.temperature_K; } + // Let other algorithms know the current white balance values. metadata->Set("awb.status", prev_sync_results_); first_switch_mode_ = false; } +bool Awb::isAutoEnabled() const +{ + return manual_r_ == 0.0 || manual_b_ == 0.0; +} + void Awb::fetchAsyncResults() { LOG(RPiAwb, Debug) << "Fetch AWB results"; async_finished_ = false; async_started_ = false; - sync_results_ = async_results_; + // It's possible manual gains could be set even while the async + // thread was running, so only copy the results if still in auto mode. + if (isAutoEnabled()) + sync_results_ = async_results_; } void Awb::restartAsync(StatisticsPtr &stats, double lux) @@ -289,8 +298,10 @@ void Awb::Process(StatisticsPtr &stats, Metadata *image_metadata) if (frame_phase_ < (int)config_.frame_period) frame_phase_++; LOG(RPiAwb, Debug) << "frame_phase " << frame_phase_; - if (frame_phase_ >= (int)config_.frame_period || - frame_count_ < (int)config_.startup_frames) { + // We do not restart the async thread if we're not in auto mode. + if (isAutoEnabled() && + (frame_phase_ >= (int)config_.frame_period || + frame_count_ < (int)config_.startup_frames)) { // Update any settings and any image metadata that we need. struct LuxStatus lux_status = {}; lux_status.lux = 400; // in case no metadata @@ -614,29 +625,18 @@ void Awb::awbGrey() void Awb::doAwb() { - if (manual_r_ != 0.0 && manual_b_ != 0.0) { - async_results_.temperature_K = 4500; // don't know what it is - async_results_.gain_r = manual_r_; - async_results_.gain_g = 1.0; - async_results_.gain_b = manual_b_; + prepareStats(); + LOG(RPiAwb, Debug) << "Valid zones: " << zones_.size(); + if (zones_.size() > config_.min_regions) { + if (config_.bayes) + awbBayes(); + else + awbGrey(); LOG(RPiAwb, Debug) - << "Using manual white balance: gain_r " - << async_results_.gain_r << " gain_b " - << async_results_.gain_b; - } else { - prepareStats(); - LOG(RPiAwb, Debug) << "Valid zones: " << zones_.size(); - if (zones_.size() > config_.min_regions) { - if (config_.bayes) - awbBayes(); - else - awbGrey(); - LOG(RPiAwb, Debug) - << "CT found is " - << async_results_.temperature_K - << " with gains r " << async_results_.gain_r - << " and b " << async_results_.gain_b; - } + << "CT found is " + << async_results_.temperature_K + << " with gains r " << async_results_.gain_r + << " and b " << async_results_.gain_b; } } diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp index f113c642..45ba9e25 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp @@ -108,6 +108,7 @@ public: }; private: + bool isAutoEnabled() const; // configuration is read-only, and available to both threads AwbConfig config_; std::thread async_thread_;