From patchwork Wed Feb 10 11:17:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 11205 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 1E1BCBD16C for ; Wed, 10 Feb 2021 11:17:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DC14461631; Wed, 10 Feb 2021 12:17:50 +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="ppX+AkQA"; dkim-atps=neutral Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FF7760300 for ; Wed, 10 Feb 2021 12:17:49 +0100 (CET) Received: by mail-ej1-x62a.google.com with SMTP id w1so3391505ejf.11 for ; Wed, 10 Feb 2021 03:17:49 -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=2uPjPtRGwcM8UxYCkq6ukzmkPLRNZbgsghs2S7MjZSI=; b=ppX+AkQAiTlxp+VZzmI8rm60cAm/fb7weGpNj7GKQMeP0UvQb8N1Rya329ltnAZRTC UwDEJ7oF+46RjFBDsIWRFb1b9ciLm/gVwHeD1AWz5XP/v3lsGiDAVUNWg79n4I/b2dqF 8Q5hyBPyyy/Ip6ACOqiZ3L82LB8G1+Mx9fwPS94XVjWUyEc6OM47M9cRPd0qghjz4Ftg +0Fe7Sy9C4B6OTNbOlDc5vtsDEPgQ7u1x/nIda9pmiu+Gx+NjkpQmKJBzPVNahR0RNya 5v2wctEYVZYvIxgANrg02w56Y9pVfcV2gqzy3dkXXtRfxy54PFdPsB/hDzWvN7DLxFQS 6Q1w== 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=2uPjPtRGwcM8UxYCkq6ukzmkPLRNZbgsghs2S7MjZSI=; b=A+mMZ3BiEDcnHoDz8ZYzuhNl+wmmscZcPk74Mfgy+6S2ve10WpSvX4qh7ggGUqxWtt VnKuTQ4CrVUOVNnW/2w5HTZJlWsFtrKLFS0huPZnxJe24Q9KuYJMBRRuUdt+df5axfMI IgvHfVyv+fK3hCPSUd/eWTaalZPcrHE8dHGkD91tDhcGLRIavBkjv7afD0pEVGP4rsHg Trhh1sgJgsZj4cup2DfOPP6gVQw43WocpIGuwrtvNxKcWT07D0Qt8f0a3d52udtl2CPz AejKvN8vQBb/6z7Bacb+Y1V3miLO7F3jV6vHsptrBTCfDCgsWZbBhGDVLE2PjBuHpzCK 51zQ== X-Gm-Message-State: AOAM531dynWRHjyc8RENhfzZibVYmLP8knZcbIGchWhmm0yoZ3ecqAga qqfHYP4WjiY4pxQQQqbGBTTFAjfgdObeQ8zY X-Google-Smtp-Source: ABdhPJyeB0nrjZXlI0hch8rdJtOioMFNI1LrLSxaEYH48/AVwzpVdrTicfiCY/LIzzkyZdLmH5M6Jw== X-Received: by 2002:a17:907:2058:: with SMTP id pg24mr2381793ejb.441.1612955868540; Wed, 10 Feb 2021 03:17:48 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id dt17sm892659ejb.70.2021.02.10.03.17.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Feb 2021 03:17:48 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Feb 2021 11:17:42 +0000 Message-Id: <20210210111743.14374-2-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210210111743.14374-1-david.plowman@raspberrypi.com> References: <20210210111743.14374-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] ipa: raspberrypi: AWB: Remove unecessary 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 11:17:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 11207 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 BD217BD160 for ; Wed, 10 Feb 2021 11:17:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8AEDF61629; Wed, 10 Feb 2021 12:17:52 +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="J/PvRIvu"; dkim-atps=neutral Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 11337602FE for ; Wed, 10 Feb 2021 12:17:50 +0100 (CET) Received: by mail-ed1-x52e.google.com with SMTP id s5so2456219edw.8 for ; Wed, 10 Feb 2021 03:17:50 -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=wzFVQfU0T7qTGPk1WFMLhvt3/kvhwvTHUc836eiWnbw=; b=J/PvRIvuAvXS/eo0V2pL4W3MO4B7TRsoXv/Sa0o6J3mhYIufwsTbyee4DRtOcBuars vrBbMV3BGEZatiL2/9Wa1GT1tD2TM72z8+kYbnyXhy8+XGHEq2oVzAFo6Y0nzoGBZsq4 wDIOTeHNHkTuHfrZ/ZuKGNjspiHlAsv/eLtUJXeBpIDDhf7mrTDgVuWKhnn6TQt7Yizp PG5Fy0Mpq4MvwBIIfTbEdMbmjaZw04G/AACNt81DUekYqiBYV16joxYQdkWyl1dLVvZM AnWdZFhqVaN8YXFkrWD0QAaKCWwxKim7VUPq1ywunz2B/DhVlJE7SgQlVjHtIOrk7Pe3 ajaQ== 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=wzFVQfU0T7qTGPk1WFMLhvt3/kvhwvTHUc836eiWnbw=; b=HqGykOLQQkju9xqzMfBPm06yJdWSdp9G0hd9yJvdv+oHvN6ngbwIvRWXsKthOshnXZ /grNaDXnRChnfADfpd3Rb+MKAqiYWhBtnynNwR3GtTfj+NDl9XUwNzyX+jQESqTSkXny FB2oXyRQcC8Z5e4fJv3pQEy3ixKDydXE0lcaPCQAqZvGRw4tBRZ63QriRCz8z3B9wvLw dP+byQGdizwA/tnVX8KsglyR+UUQq7AW3RPU536dn55NhhUtWPti8j/uKfVvSiF2yHyQ lI4JsyuKXUq/c5c9dLjDF80h6d/VsAEEwDRkiBN/Od2vfJa8KK8iOb0gawumo9my9St4 5nHw== X-Gm-Message-State: AOAM532mwE7WLW66zRhJXyedq36jenTUTkXGHyCnSatV2D2qypXfh8wX KZxdDogiVISMI1MoqZnBJ24IhzBClcexPrYW X-Google-Smtp-Source: ABdhPJyvmVOI9E+OfjhxME2CgxGqT08663Z2UtDnILyMnbeUDvM2HuxmFGlwGaTeVgb8dg4BRLf4Ag== X-Received: by 2002:a50:bb47:: with SMTP id y65mr2691479ede.33.1612955869410; Wed, 10 Feb 2021 03:17:49 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id dt17sm892659ejb.70.2021.02.10.03.17.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Feb 2021 03:17:48 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Feb 2021 11:17:43 +0000 Message-Id: <20210210111743.14374-3-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210210111743.14374-1-david.plowman@raspberrypi.com> References: <20210210111743.14374-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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 | 78 ++++++++++------------ 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp index 1c65eda8..46a8ce2a 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp @@ -193,28 +193,30 @@ 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 non-zero, set these values into the sync_results which means that + // Prepare() will adopt them immediately. + if (manual_r_ != 0.0 && manual_b_ != 0.0) { + 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 (manual_r_ != 0.0 && manual_b_ != 0.0 && + 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; } @@ -224,7 +226,10 @@ 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. In this case, we ignore its results. + if (manual_r_ == 0.0 || manual_b_ == 0.0) + sync_results_ = async_results_; } void Awb::restartAsync(StatisticsPtr &stats, double lux) @@ -289,8 +294,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 when we have fixed colour gains. + if ((manual_r_ == 0.0 || manual_b_ == 0.0) && + (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 +621,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; } }