From patchwork Mon Nov 16 16:49:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 10439 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 0D9E4BE082 for ; Mon, 16 Nov 2020 16:49:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CF907632D6; Mon, 16 Nov 2020 17:49: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="U74fOlTh"; dkim-atps=neutral Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 85835632D1 for ; Mon, 16 Nov 2020 17:49:34 +0100 (CET) Received: by mail-wm1-x332.google.com with SMTP id s13so24199948wmh.4 for ; Mon, 16 Nov 2020 08:49:34 -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=bc8pIXR1oLN0cPR7o76WwaFmRCJS7IeSD16dgScTV5Y=; b=U74fOlTh9+gRoGsA7I1ew8UZ8Gp0YnQ33ebmfHCu1139xp01lcQZxb9OJpt/4nYyoA t9WcREbC7Cy2IztwsFjvMWwaRhwTsoZPWPBIzf/uvksPkZm+oZHk3b7KUJX5bHmb4TZx jtw7kQyAuZFpH37rVTHRlL8Ytq4lRQZsW2jnz5avObmw0SfxURyHb8FBRunFXzH9j6ij Sjk/HmXgFpD4cd5NNdrJlx/IF40Ld+VSUxeflPJ9ytjrLWukU0NRcOWQH+fSHtLz8uRJ FXpUSvOkkUCZn3jL90uLLEjaPqx+t+GQFHhsx+33xKTEXcqWm5houfMi3sG3iKfz3DI1 p0XA== 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=bc8pIXR1oLN0cPR7o76WwaFmRCJS7IeSD16dgScTV5Y=; b=HJjxV42W7uDyxOAKK2j6N1o3nhASgwXCNdclXGwC2X6vAWOYwgr3O0sQlraYmnUwiH HUNzYp8pdV2xlD5UWj/2H+j/cst1TA8E3w2LBQh5X9xpdrJnv0fSbFDbYeixElRaAU/r ryNMvBnCIWXdmRRNnjtX6LkwmPAZ+8qFxpmfV0zK+T9whWkMqn2mULTEH48u7gT2WbHN EUQQpJVXZwjlty3iq9Zqj/4nwE9aEheq4f89sZALpuaIb5RTYf/489ZmLzcnQV78oK7/ BRxWEPEk7sOHMN4oOFdh9f2Fut0fslRVvDhBBxJIatSXCNnaKhCdXcSUs4uuz6mgVWon vljA== X-Gm-Message-State: AOAM5332MSFGntw0FN5df0HfjR2cayhqByWbzHJOMUoffT0Rrdzs8FSE dezBq9/graIb+598MKmEkjtIc8xoUnaGzg== X-Google-Smtp-Source: ABdhPJzQczI9hMe766aBmspH5aNNtuon3uCub+duQ7RrywWrnOo6iU15nndMmoSjCjWCYnoSNyd4Lw== X-Received: by 2002:a1c:3d05:: with SMTP id k5mr17131635wma.151.1605545373897; Mon, 16 Nov 2020 08:49:33 -0800 (PST) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id q16sm23716973wrn.13.2020.11.16.08.49.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Nov 2020 08:49:33 -0800 (PST) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Mon, 16 Nov 2020 16:49:18 +0000 Message-Id: <20201116164918.2055-11-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201116164918.2055-1-david.plowman@raspberrypi.com> References: <20201116164918.2055-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 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 | 65 +++++++++++++--------- src/ipa/raspberrypi/controller/rpi/agc.hpp | 3 + 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index 93b46a28..7be74763 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -164,6 +164,8 @@ Agc::Agc(Controller *controller) flicker_period_ = 0.0; fixed_shutter_ = 0; fixed_analogue_gain_ = 0.0; + memset(&last_device_status_, 0, sizeof(last_device_status_)); + last_target_exposure_ = 0.0; } char const *Agc::Name() const @@ -264,8 +266,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 @@ -289,32 +289,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; - //printf("%s\n", status_.locked ? "+++++++++" : "-"); + LOG(RPiAgc, Warning) << Name() << ": no device metadata"; image_metadata->Set("agc.status", status_); } } @@ -345,6 +323,41 @@ 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; + + // 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 - 1.5 * exposure_error || + device_status.shutter_speed > last_device_status_.shutter_speed + 1.5 * exposure_error || + device_status.analogue_gain < last_device_status_.analogue_gain - 1.5 * gain_error || + device_status.analogue_gain > last_device_status_.analogue_gain + 1.5 * gain_error || + status_.target_exposure_value < last_target_exposure_ - 1.5 * target_error || + status_.target_exposure_value > last_target_exposure_ + 1.5 * 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_;