From patchwork Mon Nov 23 07:38:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 10469 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 1D4A3BE177 for ; Mon, 23 Nov 2020 07:38:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D9F1D63359; Mon, 23 Nov 2020 08:38:41 +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="DuVqlZUW"; dkim-atps=neutral Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BE69263325 for ; Mon, 23 Nov 2020 08:38:39 +0100 (CET) Received: by mail-wr1-x42f.google.com with SMTP id 23so17535485wrc.8 for ; Sun, 22 Nov 2020 23:38:39 -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=xxsqE/MmwUS9M8cOxXNB6MzXfTJijIjyM9MywW0cEkY=; b=DuVqlZUWSTLxFVLr53wowlMaiE2XB3oeYiEOVEpqbxwPBJL+Cgq6UMtWenyT+U0myg 7L9J0ARHnIqA9KVhyIh3COaeYkdxopU2lLSVzk7chrtZKcvzooAKqjR06KBxCczN4e1u Wg6uzuEL2fBLKmi+8ARIqgNzYGi8/SvcPphWXOagh9B+yWP6rU61gMfnEHeBReqb5PjL CUHLhHX5iLcva+4zYrobbHRPELI5qr2qqy6npmrztz7SbC+Go1QcUBbaYU8ekeZkvkjy 2wBsMp4NROaqXAE/txbvNe/t8biXkq5lSklaG/EoRSnt5yKFC6ekaR2WVcRw5IjbxNnA VayQ== 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=xxsqE/MmwUS9M8cOxXNB6MzXfTJijIjyM9MywW0cEkY=; b=CUZv7jyvWLA1liXo2wy1slzPUIBblpEXY6Os7O7Jr/C39lKPRGNILJDL91Fvn0cVvC +WsBJACLMbY2q07jH3KCHbMj68QKK2NPjUa631YNMuPMfv9oxm1O/z9BVqcovKoLq22G ksqzSgS0y1YSZRWupj5wSerdTwAlVJO6+xm2gmHJK4TuXeFofVGW2HYRghFMF+WUj9Gz t2G/p46Y9Zs61ciVemqhSqTyomeHP3sD5jyVhH68IczSV1xR8sstgVPyKSktxPGlfdYg JlRHv7DE1EuqQLPeS7LTp7nR9uoeROIVI/fZfH9BumiUllk6Q9Iat4IXQiWbihyd4/V8 +NLw== X-Gm-Message-State: AOAM533Zd8cpNkFB/RHm662D0Q5H1IWUr6BY6o9bPG/JmqH4Vxnbdaen CWkMkUm4IO4gRL2Rme79TsaMkL4GYZtV1cy9 X-Google-Smtp-Source: ABdhPJzTdJCSbsrmKDH2+W+UIkpVEiEdAIwknfXK1QFoGBRWnJvb9CM+i0QmeTbE6bYrYhTvmQhJHA== X-Received: by 2002:a05:6000:1292:: with SMTP id f18mr29029212wrx.196.1606117119205; Sun, 22 Nov 2020 23:38:39 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id h15sm17841822wrw.15.2020.11.22.23.38.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 22 Nov 2020 23:38:38 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Mon, 23 Nov 2020 07:38:04 +0000 Message-Id: <20201123073804.3125-11-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201123073804.3125-1-david.plowman@raspberrypi.com> References: <20201123073804.3125-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/10] libcamera: src: ipa: raspberrypi: agc: Improve AE locked logic 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" Previously we required that the sensor absolutely reaches the target exposure, but this can fail if frame rates or analogue gains are limited. Instead insist only that we get several frames with the same exposure time, analogue gain and that the algorithm's target exposure hasn't changed either. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/controller/rpi/agc.cpp | 66 ++++++++++++++-------- src/ipa/raspberrypi/controller/rpi/agc.hpp | 3 + 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index 37806055..4c56bdc9 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -154,6 +154,7 @@ Agc::Agc(Controller *controller) : AgcAlgorithm(controller), metering_mode_(nullptr), exposure_mode_(nullptr), constraint_mode_(nullptr), frame_count_(0), lock_count_(0), + last_target_exposure_(0.0), ev_(1.0), flicker_period_(0.0), fixed_shutter_(0), fixed_analogue_gain_(0.0) { @@ -162,6 +163,7 @@ Agc::Agc(Controller *controller) // it's not been calculated yet (i.e. Process hasn't yet run). memset(&status_, 0, sizeof(status_)); status_.ev = ev_; + memset(&last_device_status_, 0, sizeof(last_device_status_)); } char const *Agc::Name() const @@ -262,8 +264,6 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode, void Agc::Prepare(Metadata *image_metadata) { - int lock_count = lock_count_; - lock_count_ = 0; status_.digital_gain = 1.0; fetchAwbStatus(image_metadata); // always fetch it so that Process knows it's been done @@ -287,31 +287,10 @@ void Agc::Prepare(Metadata *image_metadata) LOG(RPiAgc, Debug) << "Use digital_gain " << status_.digital_gain; LOG(RPiAgc, Debug) << "Effective exposure " << actual_exposure * status_.digital_gain; // Decide whether AEC/AGC has converged. - // Insist AGC is steady for MAX_LOCK_COUNT - // frames before we say we are "locked". - // (The hard-coded constants may need to - // become customisable.) - if (status_.target_exposure_value) { -#define MAX_LOCK_COUNT 3 - double err = 0.10 * status_.target_exposure_value + 200; - if (actual_exposure < - status_.target_exposure_value + err && - actual_exposure > - status_.target_exposure_value - err) - lock_count_ = - std::min(lock_count + 1, - MAX_LOCK_COUNT); - else if (actual_exposure < - status_.target_exposure_value + 1.5 * err && - actual_exposure > - status_.target_exposure_value - 1.5 * err) - lock_count_ = lock_count; - LOG(RPiAgc, Debug) << "Lock count: " << lock_count_; - } + updateLockStatus(device_status); } } else - LOG(RPiAgc, Debug) << Name() << ": no device metadata"; - status_.locked = lock_count_ >= MAX_LOCK_COUNT; + LOG(RPiAgc, Warning) << Name() << ": no device metadata"; image_metadata->Set("agc.status", status_); } } @@ -342,6 +321,43 @@ void Agc::Process(StatisticsPtr &stats, Metadata *image_metadata) writeAndFinish(image_metadata, desaturate); } +void Agc::updateLockStatus(DeviceStatus const &device_status) +{ + const double ERROR_FACTOR = 0.10; // make these customisable? + const int MAX_LOCK_COUNT = 5; + // Reset "lock count" when we exceed this multiple of ERROR_FACTOR + const double RESET_MARGIN = 1.5; + + // Add 200us to the exposure time error to allow for line quantisation. + double exposure_error = last_device_status_.shutter_speed * ERROR_FACTOR + 200; + double gain_error = last_device_status_.analogue_gain * ERROR_FACTOR; + double target_error = last_target_exposure_ * ERROR_FACTOR; + + // Note that we don't know the exposure/gain limits of the sensor, so + // the values we keep requesting may be unachievable. For this reason + // we only insist that we're close to values in the past few frames. + if (device_status.shutter_speed > last_device_status_.shutter_speed - exposure_error && + device_status.shutter_speed < last_device_status_.shutter_speed + exposure_error && + device_status.analogue_gain > last_device_status_.analogue_gain - gain_error && + device_status.analogue_gain < last_device_status_.analogue_gain + gain_error && + status_.target_exposure_value > last_target_exposure_ - target_error && + status_.target_exposure_value < last_target_exposure_ + target_error) + lock_count_ = std::min(lock_count_ + 1, MAX_LOCK_COUNT); + else if (device_status.shutter_speed < last_device_status_.shutter_speed - RESET_MARGIN * exposure_error || + device_status.shutter_speed > last_device_status_.shutter_speed + RESET_MARGIN * exposure_error || + device_status.analogue_gain < last_device_status_.analogue_gain - RESET_MARGIN * gain_error || + device_status.analogue_gain > last_device_status_.analogue_gain + RESET_MARGIN * gain_error || + status_.target_exposure_value < last_target_exposure_ - RESET_MARGIN * target_error || + status_.target_exposure_value > last_target_exposure_ + RESET_MARGIN * target_error) + lock_count_ = 0; + + last_device_status_ = device_status; + last_target_exposure_ = status_.target_exposure_value; + + LOG(RPiAgc, Debug) << "Lock count updated to " << lock_count_; + status_.locked = lock_count_ == MAX_LOCK_COUNT; +} + static void copy_string(std::string const &s, char *d, size_t size) { size_t length = s.copy(d, size - 1); diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp index 859a9650..47ebb324 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp @@ -82,6 +82,7 @@ public: void Process(StatisticsPtr &stats, Metadata *image_metadata) override; private: + void updateLockStatus(DeviceStatus const &device_status); AgcConfig config_; void housekeepConfig(); void fetchCurrentExposure(Metadata *image_metadata); @@ -111,6 +112,8 @@ private: ExposureValues filtered_; // these values are filtered towards target AgcStatus status_; int lock_count_; + DeviceStatus last_device_status_; + double last_target_exposure_; // Below here the "settings" that applications can change. std::string metering_mode_name_; std::string exposure_mode_name_;