Patch Detail
Show a patch.
GET /api/patches/13256/?format=api
{ "id": 13256, "url": "https://patchwork.libcamera.org/api/patches/13256/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13256/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>", "date": "2021-08-09T09:20:07", "name": "[libcamera-devel,RFC,5/5] ipa: ipu3: Move IPU3 agc into algorithms", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "a7a9f72775249bae6f05421e42c0c75867ec4ce5", "submitter": { "id": 75, "url": "https://patchwork.libcamera.org/api/people/75/?format=api", "name": "Jean-Michel Hautbois", "email": "jeanmichel.hautbois@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/13256/mbox/", "series": [ { "id": 2320, "url": "https://patchwork.libcamera.org/api/series/2320/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2320", "date": "2021-08-09T09:20:02", "name": "IPU3: Introduce modularity for algorithms", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2320/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13256/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13256/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 E2043C3243\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 9 Aug 2021 09:20:21 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7791668894;\n\tMon, 9 Aug 2021 11:20:21 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE25768826\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 9 Aug 2021 11:20:16 +0200 (CEST)", "from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:14c5:c0eb:b1fb:db22])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 79BF6EE;\n\tMon, 9 Aug 2021 11:20:16 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W4lyKNZt\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628500816;\n\tbh=q3xLYhlQWLqVZGaLtxBXVXGidaoqPRlpYIJTIY6j2P4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=W4lyKNZtBa7yNUA3XyizOJJOj4q1eSn0TE98Y7VLQb1ODLMMGDtsqTiv27uDzzRW9\n\tcz4DhFHZhHkvVtyRPfy0XJr4DYsjKOEtAyYGwPN1E824HByLFE4nREZxpoCjb5B+Ei\n\tdt4zT2LQSCKWOhGO9cpAtgC1zcxKEfnehv5q7qE4=", "From": "Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 9 Aug 2021 11:20:07 +0200", "Message-Id": "<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20210809092007.79067-1-jeanmichel.hautbois@ideasonboard.com>", "References": "<20210809092007.79067-1-jeanmichel.hautbois@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into\n\talgorithms", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Use the Context class and algorithm interface to properly call the AGC\nalgorithm from IPAIPU3.\nWe need to pass shutter speed and analogue gain through Context. Add a\ndedicated AGC structure for that.\n\nSigned-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n---\n .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 56 ++++++++++---------\n src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 31 +++++-----\n src/ipa/ipu3/algorithms/meson.build | 1 +\n src/ipa/ipu3/ipa_context.h | 11 ++++\n src/ipa/ipu3/ipu3.cpp | 25 +++++----\n src/ipa/ipu3/meson.build | 1 -\n 6 files changed, 70 insertions(+), 55 deletions(-)\n rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (81%)\n rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (61%)", "diff": "diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\nsimilarity index 81%\nrename from src/ipa/ipu3/ipu3_agc.cpp\nrename to src/ipa/ipu3/algorithms/agc.cpp\nindex 733814dd..1146a34a 100644\n--- a/src/ipa/ipu3/ipu3_agc.cpp\n+++ b/src/ipa/ipu3/algorithms/agc.cpp\n@@ -2,26 +2,25 @@\n /*\n * Copyright (C) 2021, Ideas On Board\n *\n- * ipu3_agc.cpp - AGC/AEC control algorithm\n+ * agc.cpp - AGC/AEC control algorithm\n */\n \n-#include \"ipu3_agc.h\"\n+#include \"agc.h\"\n \n-#include <algorithm>\n #include <cmath>\n+\n+#include <algorithm>\n #include <numeric>\n \n #include <libcamera/base/log.h>\n \n-#include <libcamera/ipa/core_ipa_interface.h>\n-\n #include \"libipa/histogram.h\"\n \n namespace libcamera {\n \n using namespace std::literals::chrono_literals;\n \n-namespace ipa::ipu3 {\n+namespace ipa::ipu3::algorithms {\n \n LOG_DEFINE_CATEGORY(IPU3Agc)\n \n@@ -36,8 +35,8 @@ static constexpr uint32_t kMaxISO = 1500;\n \n /* Maximum analogue gain value\n * \\todo grab it from a camera helper */\n-static constexpr uint32_t kMinGain = kMinISO / 100;\n-static constexpr uint32_t kMaxGain = kMaxISO / 100;\n+static constexpr double kMinGain = kMinISO / 100;\n+static constexpr double kMaxGain = kMaxISO / 100;\n \n /* \\todo use calculated value based on sensor */\n static constexpr uint32_t kMinExposure = 1;\n@@ -50,24 +49,24 @@ static constexpr double kEvGainTarget = 0.5;\n /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */\n static constexpr uint8_t kCellSize = 8;\n \n-IPU3Agc::IPU3Agc()\n+Agc::Agc()\n \t: frameCount_(0), lastFrame_(0), converged_(false),\n-\t updateControls_(false), iqMean_(0.0),\n-\t lineDuration_(0s), maxExposureTime_(0s),\n+\t updateControls_(false), iqMean_(0.0), maxExposureTime_(0s),\n \t prevExposure_(0s), prevExposureNoDg_(0s),\n \t currentExposure_(0s), currentExposureNoDg_(0s)\n {\n }\n \n-void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo)\n+int Agc::configure(IPAContext &context)\n {\n-\taeGrid_ = bdsGrid;\n+\t/* The AGC algorithm uses the AWB statistics */\n+\taeGrid_ = context.awb.grid.bdsGrid;\n+\tmaxExposureTime_ = kMaxExposure * context.agc.lineDuration;\n \n-\tlineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;\n-\tmaxExposureTime_ = kMaxExposure * lineDuration_;\n+\treturn 0;\n }\n \n-void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n+void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n {\n \tconst struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;\n \tRectangle aeRegion = { statsAeGrid.x_start,\n@@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n \tiqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);\n }\n \n-void IPU3Agc::filterExposure()\n+void Agc::filterExposure()\n {\n \tdouble speed = 0.2;\n \tif (prevExposure_ == 0s) {\n@@ -128,7 +127,7 @@ void IPU3Agc::filterExposure()\n \t\tprevExposure_ = speed * currentExposure_ +\n \t\t\t\tprevExposure_ * (1.0 - speed);\n \t\tprevExposureNoDg_ = speed * currentExposureNoDg_ +\n-\t\t\t\tprevExposureNoDg_ * (1.0 - speed);\n+\t\t\t\t prevExposureNoDg_ * (1.0 - speed);\n \t}\n \t/*\n \t * We can't let the no_dg exposure deviate too far below the\n@@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()\n \tLOG(IPU3Agc, Debug) << \"After filtering, total_exposure \" << prevExposure_;\n }\n \n-void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n+void Agc::lockExposureGain(IPAContext &context)\n {\n \tupdateControls_ = false;\n \n@@ -160,7 +159,9 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n \n \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n-\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n+\t\tlibcamera::utils::Duration currentShutter = context.agc.shutterTime;\n+\t\tuint32_t exposure = currentShutter / context.agc.lineDuration;\n+\t\tdouble &gain = context.agc.analogueGain;\n \t\tcurrentExposureNoDg_ = currentShutter * gain;\n \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n \t\t\t\t << \" Shutter speed \" << currentShutter\n@@ -177,26 +178,27 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n \t\tif (currentShutter < maxExposureTime_) {\n \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n \t\t\tnewExposure = currentExposure_ / exposure;\n-\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n+\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n \t\t\tupdateControls_ = true;\n \t\t} else if (currentShutter >= maxExposureTime_) {\n-\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n+\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n \t\t\tnewExposure = currentExposure_ / gain;\n \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);\n \t\t\tupdateControls_ = true;\n \t\t}\n-\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * lineDuration_ << \" and gain \" << gain;\n+\t\tcontext.agc.shutterTime = exposure * context.agc.lineDuration;\n+\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * context.agc.lineDuration << \" and gain \" << gain;\n \t}\n \tlastFrame_ = frameCount_;\n }\n \n-void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain)\n+void Agc::process(IPAContext &context)\n {\n-\tprocessBrightness(stats);\n-\tlockExposureGain(exposure, gain);\n+\tprocessBrightness(context.stats);\n+\tlockExposureGain(context);\n \tframeCount_++;\n }\n \n-} /* namespace ipa::ipu3 */\n+} /* namespace ipa::ipu3::algorithms */\n \n } /* namespace libcamera */\ndiff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h\nsimilarity index 61%\nrename from src/ipa/ipu3/ipu3_agc.h\nrename to src/ipa/ipu3/algorithms/agc.h\nindex 3b7f781e..e87a06f3 100644\n--- a/src/ipa/ipu3/ipu3_agc.h\n+++ b/src/ipa/ipu3/algorithms/agc.h\n@@ -4,44 +4,44 @@\n *\n * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n */\n-#ifndef __LIBCAMERA_IPU3_AGC_H__\n-#define __LIBCAMERA_IPU3_AGC_H__\n+#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n+#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n+\n+#include <linux/intel-ipu3.h>\n \n #include <array>\n #include <unordered_map>\n \n-#include <linux/intel-ipu3.h>\n-\n #include <libcamera/base/utils.h>\n \n #include <libcamera/geometry.h>\n \n-#include \"algorithms/algorithm.h\"\n-#include \"libipa/algorithm.h\"\n+#include \"algorithm.h\"\n \n namespace libcamera {\n \n struct IPACameraSensorInfo;\n \n-namespace ipa::ipu3 {\n+namespace ipa::ipu3::algorithms {\n \n using utils::Duration;\n \n-class IPU3Agc : public ipa::Algorithm\n+class Agc : public Algorithm\n {\n public:\n-\tIPU3Agc();\n-\t~IPU3Agc() = default;\n+\tAgc();\n+\t~Agc() = default;\n+\n+\tint configure(IPAContext &context) override;\n+\tvoid process(IPAContext &context) override;\n \n-\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo);\n-\tvoid process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain);\n \tbool converged() { return converged_; }\n \tbool updateControls() { return updateControls_; }\n \n private:\n \tvoid processBrightness(const ipu3_uapi_stats_3a *stats);\n \tvoid filterExposure();\n-\tvoid lockExposureGain(uint32_t &exposure, double &gain);\n+\tvoid lockExposureGain(IPAContext &context);\n \n \tstruct ipu3_uapi_grid_config aeGrid_;\n \n@@ -53,7 +53,6 @@ private:\n \n \tdouble iqMean_;\n \n-\tDuration lineDuration_;\n \tDuration maxExposureTime_;\n \n \tDuration prevExposure_;\n@@ -62,8 +61,8 @@ private:\n \tDuration currentExposureNoDg_;\n };\n \n-} /* namespace ipa::ipu3 */\n+} /* namespace ipa::ipu3::algorithms */\n \n } /* namespace libcamera */\n \n-#endif /* __LIBCAMERA_IPU3_AGC_H__ */\n+#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */\ndiff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\nindex df36d719..3faf913e 100644\n--- a/src/ipa/ipu3/algorithms/meson.build\n+++ b/src/ipa/ipu3/algorithms/meson.build\n@@ -1,6 +1,7 @@\n # SPDX-License-Identifier: CC0-1.0\n \n ipu3_ipa_algorithms = files([\n+ 'agc.cpp',\n 'awb.cpp',\n 'contrast.cpp',\n ])\ndiff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\nindex c43b275b..1d7a1984 100644\n--- a/src/ipa/ipu3/ipa_context.h\n+++ b/src/ipa/ipu3/ipa_context.h\n@@ -11,10 +11,14 @@\n \n #include <linux/intel-ipu3.h>\n \n+#include <libcamera/base/utils.h>\n+\n #include <libcamera/geometry.h>\n \n namespace libcamera {\n \n+using libcamera::utils::Duration;\n+\n namespace ipa::ipu3 {\n \n struct IPAContext {\n@@ -33,6 +37,13 @@ struct IPAContext {\n \t\t\tSize bdsOutputSize;\n \t\t} grid;\n \t} awb;\n+\n+\t/* AGC specific parameters to share */\n+\tstruct Agc {\n+\t\tDuration lineDuration;\n+\t\tDuration shutterTime;\n+\t\tdouble analogueGain;\n+\t} agc;\n };\n \n } /* namespace ipa::ipu3 */\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 82506461..07b1d11c 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -22,12 +22,12 @@\n \n #include \"libcamera/internal/framebuffer.h\"\n \n+#include \"algorithms/agc.h\"\n #include \"algorithms/algorithm.h\"\n #include \"algorithms/awb.h\"\n #include \"algorithms/contrast.h\"\n #include \"ipa_context.h\"\n \n-#include \"ipu3_agc.h\"\n #include \"libipa/camera_sensor_helper.h\"\n \n static constexpr uint32_t kMaxCellWidthPerSet = 160;\n@@ -37,6 +37,8 @@ namespace libcamera {\n \n LOG_DEFINE_CATEGORY(IPAIPU3)\n \n+using namespace std::literals::chrono_literals;\n+\n namespace ipa::ipu3 {\n \n class IPAIPU3 : public IPAIPU3Interface\n@@ -81,8 +83,6 @@ private:\n \tuint32_t minGain_;\n \tuint32_t maxGain_;\n \n-\t/* Interface to the AEC/AGC algorithm */\n-\tstd::unique_ptr<IPU3Agc> agcAlgo_;\n \t/* Interface to the Camera Helper */\n \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n \n@@ -103,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings)\n \t}\n \n \t/* Construct our Algorithms */\n+\talgorithms_.emplace_back(new algorithms::Agc());\n \talgorithms_.emplace_back(new algorithms::Awb());\n \talgorithms_.emplace_back(new algorithms::Contrast());\n \n@@ -214,11 +215,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n \tcalculateBdsGrid(configInfo.bdsOutputSize);\n \tcontext_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize;\n \n+\t/* Prepare AGC parameters */\n+\tcontext_.agc.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n+\tcontext_.agc.shutterTime = exposure_ * context_.agc.lineDuration;\n+\tcontext_.agc.analogueGain = camHelper_->gain(gain_);\n+\n \tconfigureAlgorithms();\n \n \tbdsGrid_ = context_.awb.grid.bdsGrid;\n-\tagcAlgo_ = std::make_unique<IPU3Agc>();\n-\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n \n \treturn 0;\n }\n@@ -341,12 +345,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n \t/* Run the process for each algorithm on the stats */\n \tprocessAlgorithms(stats);\n \n-\tdouble gain = camHelper_->gain(gain_);\n-\tagcAlgo_->process(stats, exposure_, gain);\n-\tgain_ = camHelper_->gainCode(gain);\n-\n-\tif (agcAlgo_->updateControls())\n-\t\tsetControls(frame);\n+\tsetControls(frame);\n \n \t/* \\todo Use VBlank value calculated from each frame exposure. */\n \tint64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /\n@@ -365,6 +364,10 @@ void IPAIPU3::setControls(unsigned int frame)\n \tIPU3Action op;\n \top.op = ActionSetSensorControls;\n \n+\t/* Convert gain and exposure */\n+\tgain_ = camHelper_->gainCode(context_.agc.analogueGain);\n+\texposure_ = context_.agc.shutterTime / context_.agc.lineDuration;\n+\n \tControlList ctrls(ctrls_);\n \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\ndiff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\nindex d1126947..39320980 100644\n--- a/src/ipa/ipu3/meson.build\n+++ b/src/ipa/ipu3/meson.build\n@@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'\n \n ipu3_ipa_sources = files([\n 'ipu3.cpp',\n- 'ipu3_agc.cpp',\n ])\n \n ipu3_ipa_sources += ipu3_ipa_algorithms\n", "prefixes": [ "libcamera-devel", "RFC", "5/5" ] }