Patch Detail
Show a patch.
GET /api/1.1/patches/10469/?format=api
{ "id": 10469, "url": "https://patchwork.libcamera.org/api/1.1/patches/10469/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10469/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20201123073804.3125-11-david.plowman@raspberrypi.com>", "date": "2020-11-23T07:38:04", "name": "[libcamera-devel,v2,10/10] libcamera: src: ipa: raspberrypi: agc: Improve AE locked logic", "commit_ref": "1ea9ce9bdd1f42d468f3a8e71282215f46a2464c", "pull_url": null, "state": "accepted", "archived": false, "hash": "7bff1dd1b1a3445549c867684c3e2c1b1ff3da3a", "submitter": { "id": 42, "url": "https://patchwork.libcamera.org/api/1.1/people/42/?format=api", "name": "David Plowman", "email": "david.plowman@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/10469/mbox/", "series": [ { "id": 1475, "url": "https://patchwork.libcamera.org/api/1.1/series/1475/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1475", "date": "2020-11-23T07:37:54", "name": "Raspberry Pi AGC", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1475/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10469/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10469/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1D4A3BE177\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Nov 2020 07:38:42 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D9F1D63359;\n\tMon, 23 Nov 2020 08:38:41 +0100 (CET)", "from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com\n\t[IPv6:2a00:1450:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE69263325\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Nov 2020 08:38:39 +0100 (CET)", "by mail-wr1-x42f.google.com with SMTP id 23so17535485wrc.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 22 Nov 2020 23:38:39 -0800 (PST)", "from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72])\n\tby smtp.gmail.com with ESMTPSA id\n\th15sm17841822wrw.15.2020.11.22.23.38.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 22 Nov 2020 23:38:38 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"DuVqlZUW\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=xxsqE/MmwUS9M8cOxXNB6MzXfTJijIjyM9MywW0cEkY=;\n\tb=DuVqlZUWSTLxFVLr53wowlMaiE2XB3oeYiEOVEpqbxwPBJL+Cgq6UMtWenyT+U0myg\n\t7L9J0ARHnIqA9KVhyIh3COaeYkdxopU2lLSVzk7chrtZKcvzooAKqjR06KBxCczN4e1u\n\tWg6uzuEL2fBLKmi+8ARIqgNzYGi8/SvcPphWXOagh9B+yWP6rU61gMfnEHeBReqb5PjL\n\tCUHLhHX5iLcva+4zYrobbHRPELI5qr2qqy6npmrztz7SbC+Go1QcUBbaYU8ekeZkvkjy\n\t2wBsMp4NROaqXAE/txbvNe/t8biXkq5lSklaG/EoRSnt5yKFC6ekaR2WVcRw5IjbxNnA\n\tVayQ==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=xxsqE/MmwUS9M8cOxXNB6MzXfTJijIjyM9MywW0cEkY=;\n\tb=CUZv7jyvWLA1liXo2wy1slzPUIBblpEXY6Os7O7Jr/C39lKPRGNILJDL91Fvn0cVvC\n\t+WsBJACLMbY2q07jH3KCHbMj68QKK2NPjUa631YNMuPMfv9oxm1O/z9BVqcovKoLq22G\n\tksqzSgS0y1YSZRWupj5wSerdTwAlVJO6+xm2gmHJK4TuXeFofVGW2HYRghFMF+WUj9Gz\n\tt2G/p46Y9Zs61ciVemqhSqTyomeHP3sD5jyVhH68IczSV1xR8sstgVPyKSktxPGlfdYg\n\tJlRHv7DE1EuqQLPeS7LTp7nR9uoeROIVI/fZfH9BumiUllk6Q9Iat4IXQiWbihyd4/V8\n\t+NLw==", "X-Gm-Message-State": "AOAM533Zd8cpNkFB/RHm662D0Q5H1IWUr6BY6o9bPG/JmqH4Vxnbdaen\n\tCWkMkUm4IO4gRL2Rme79TsaMkL4GYZtV1cy9", "X-Google-Smtp-Source": "ABdhPJzTdJCSbsrmKDH2+W+UIkpVEiEdAIwknfXK1QFoGBRWnJvb9CM+i0QmeTbE6bYrYhTvmQhJHA==", "X-Received": "by 2002:a05:6000:1292:: with SMTP id\n\tf18mr29029212wrx.196.1606117119205; \n\tSun, 22 Nov 2020 23:38:39 -0800 (PST)", "From": "David Plowman <david.plowman@raspberrypi.com>", "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:\n\traspberrypi: agc: Improve AE locked logic", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Previously we required that the sensor absolutely reaches the target\nexposure, but this can fail if frame rates or analogue gains are\nlimited. Instead insist only that we get several frames with the same\nexposure time, analogue gain and that the algorithm's target exposure\nhasn't changed either.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n---\n src/ipa/raspberrypi/controller/rpi/agc.cpp | 66 ++++++++++++++--------\n src/ipa/raspberrypi/controller/rpi/agc.hpp | 3 +\n 2 files changed, 44 insertions(+), 25 deletions(-)", "diff": "diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\nindex 37806055..4c56bdc9 100644\n--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n@@ -154,6 +154,7 @@ Agc::Agc(Controller *controller)\n \t: AgcAlgorithm(controller), metering_mode_(nullptr),\n \t exposure_mode_(nullptr), constraint_mode_(nullptr),\n \t frame_count_(0), lock_count_(0),\n+\t last_target_exposure_(0.0),\n \t ev_(1.0), flicker_period_(0.0),\n \t fixed_shutter_(0), fixed_analogue_gain_(0.0)\n {\n@@ -162,6 +163,7 @@ Agc::Agc(Controller *controller)\n \t// it's not been calculated yet (i.e. Process hasn't yet run).\n \tmemset(&status_, 0, sizeof(status_));\n \tstatus_.ev = ev_;\n+\tmemset(&last_device_status_, 0, sizeof(last_device_status_));\n }\n \n char const *Agc::Name() const\n@@ -262,8 +264,6 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,\n \n void Agc::Prepare(Metadata *image_metadata)\n {\n-\tint lock_count = lock_count_;\n-\tlock_count_ = 0;\n \tstatus_.digital_gain = 1.0;\n \tfetchAwbStatus(image_metadata); // always fetch it so that Process knows it's been done\n \n@@ -287,31 +287,10 @@ void Agc::Prepare(Metadata *image_metadata)\n \t\t\t\tLOG(RPiAgc, Debug) << \"Use digital_gain \" << status_.digital_gain;\n \t\t\t\tLOG(RPiAgc, Debug) << \"Effective exposure \" << actual_exposure * status_.digital_gain;\n \t\t\t\t// Decide whether AEC/AGC has converged.\n-\t\t\t\t// Insist AGC is steady for MAX_LOCK_COUNT\n-\t\t\t\t// frames before we say we are \"locked\".\n-\t\t\t\t// (The hard-coded constants may need to\n-\t\t\t\t// become customisable.)\n-\t\t\t\tif (status_.target_exposure_value) {\n-#define MAX_LOCK_COUNT 3\n-\t\t\t\t\tdouble err = 0.10 * status_.target_exposure_value + 200;\n-\t\t\t\t\tif (actual_exposure <\n-\t\t\t\t\t\t status_.target_exposure_value + err &&\n-\t\t\t\t\t actual_exposure >\n-\t\t\t\t\t\t status_.target_exposure_value - err)\n-\t\t\t\t\t\tlock_count_ =\n-\t\t\t\t\t\t\tstd::min(lock_count + 1,\n-\t\t\t\t\t\t\t\t MAX_LOCK_COUNT);\n-\t\t\t\t\telse if (actual_exposure <\n-\t\t\t\t\t\t\t status_.target_exposure_value + 1.5 * err &&\n-\t\t\t\t\t\t actual_exposure >\n-\t\t\t\t\t\t\t status_.target_exposure_value - 1.5 * err)\n-\t\t\t\t\t\tlock_count_ = lock_count;\n-\t\t\t\t\tLOG(RPiAgc, Debug) << \"Lock count: \" << lock_count_;\n-\t\t\t\t}\n+\t\t\t\tupdateLockStatus(device_status);\n \t\t\t}\n \t\t} else\n-\t\t\tLOG(RPiAgc, Debug) << Name() << \": no device metadata\";\n-\t\tstatus_.locked = lock_count_ >= MAX_LOCK_COUNT;\n+\t\t\tLOG(RPiAgc, Warning) << Name() << \": no device metadata\";\n \t\timage_metadata->Set(\"agc.status\", status_);\n \t}\n }\n@@ -342,6 +321,43 @@ void Agc::Process(StatisticsPtr &stats, Metadata *image_metadata)\n \twriteAndFinish(image_metadata, desaturate);\n }\n \n+void Agc::updateLockStatus(DeviceStatus const &device_status)\n+{\n+\tconst double ERROR_FACTOR = 0.10; // make these customisable?\n+\tconst int MAX_LOCK_COUNT = 5;\n+\t// Reset \"lock count\" when we exceed this multiple of ERROR_FACTOR\n+\tconst double RESET_MARGIN = 1.5;\n+\n+\t// Add 200us to the exposure time error to allow for line quantisation.\n+\tdouble exposure_error = last_device_status_.shutter_speed * ERROR_FACTOR + 200;\n+\tdouble gain_error = last_device_status_.analogue_gain * ERROR_FACTOR;\n+\tdouble target_error = last_target_exposure_ * ERROR_FACTOR;\n+\n+\t// Note that we don't know the exposure/gain limits of the sensor, so\n+\t// the values we keep requesting may be unachievable. For this reason\n+\t// we only insist that we're close to values in the past few frames.\n+\tif (device_status.shutter_speed > last_device_status_.shutter_speed - exposure_error &&\n+\t device_status.shutter_speed < last_device_status_.shutter_speed + exposure_error &&\n+\t device_status.analogue_gain > last_device_status_.analogue_gain - gain_error &&\n+\t device_status.analogue_gain < last_device_status_.analogue_gain + gain_error &&\n+\t status_.target_exposure_value > last_target_exposure_ - target_error &&\n+\t status_.target_exposure_value < last_target_exposure_ + target_error)\n+\t\tlock_count_ = std::min(lock_count_ + 1, MAX_LOCK_COUNT);\n+\telse if (device_status.shutter_speed < last_device_status_.shutter_speed - RESET_MARGIN * exposure_error ||\n+\t\t device_status.shutter_speed > last_device_status_.shutter_speed + RESET_MARGIN * exposure_error ||\n+\t\t device_status.analogue_gain < last_device_status_.analogue_gain - RESET_MARGIN * gain_error ||\n+\t\t device_status.analogue_gain > last_device_status_.analogue_gain + RESET_MARGIN * gain_error ||\n+\t\t status_.target_exposure_value < last_target_exposure_ - RESET_MARGIN * target_error ||\n+\t\t status_.target_exposure_value > last_target_exposure_ + RESET_MARGIN * target_error)\n+\t\tlock_count_ = 0;\n+\n+\tlast_device_status_ = device_status;\n+\tlast_target_exposure_ = status_.target_exposure_value;\n+\n+\tLOG(RPiAgc, Debug) << \"Lock count updated to \" << lock_count_;\n+\tstatus_.locked = lock_count_ == MAX_LOCK_COUNT;\n+}\n+\n static void copy_string(std::string const &s, char *d, size_t size)\n {\n \tsize_t length = s.copy(d, size - 1);\ndiff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp\nindex 859a9650..47ebb324 100644\n--- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n+++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n@@ -82,6 +82,7 @@ public:\n \tvoid Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n \n private:\n+\tvoid updateLockStatus(DeviceStatus const &device_status);\n \tAgcConfig config_;\n \tvoid housekeepConfig();\n \tvoid fetchCurrentExposure(Metadata *image_metadata);\n@@ -111,6 +112,8 @@ private:\n \tExposureValues filtered_; // these values are filtered towards target\n \tAgcStatus status_;\n \tint lock_count_;\n+\tDeviceStatus last_device_status_;\n+\tdouble last_target_exposure_;\n \t// Below here the \"settings\" that applications can change.\n \tstd::string metering_mode_name_;\n \tstd::string exposure_mode_name_;\n", "prefixes": [ "libcamera-devel", "v2", "10/10" ] }