[{"id":25496,"web_url":"https://patchwork.libcamera.org/comment/25496/","msgid":"<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>","date":"2022-10-20T07:40:39","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Wed, Oct 19, 2022 at 11:56:14PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> function. This removes the need to fill metadata manually in the IPA\n> module's processStatsBuffer() function.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> src/ipa/ipu3/ipa_context.cpp | 3 +++\n> src/ipa/ipu3/ipa_context.h | 1 +\n> src/ipa/ipu3/ipu3.cpp | 13 +------------\n> 5 files changed, 30 insertions(+), 13 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index f4e559bf8b84..b5309bdbea25 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> #include \"libipa/histogram.h\"\n> @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\t IPAFrameContext &frameContext,\n> \t\t const ipu3_uapi_stats_3a *stats,\n> -\t\t [[maybe_unused]] ControlList &metadata)\n> +\t\t ControlList &metadata)\n> {\n> \t/*\n> \t * Estimate the gain needed to have the proportion of pixels in a given\n> @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>\n> \tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> \tframeCount_++;\n> +\n> +\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> +\t\t\t\t * frameContext.sensor.exposure;\n> +\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> +\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> +\n> +\t/* \\todo Use VBlank value calculated from each frame exposure. */\n\nWhat block will adjust the frame duration ? AEGC for sure can ends up\nenlarging blankings, what other component will handle control of\nvblank ?\n\nNothing that interefere with this patch\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\tuint32_t vTotal = context.configuration.sensor.size.height\n> +\t\t\t+ context.configuration.sensor.defVBlank;\n> +\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> +\t\t\t\t * vTotal;\n> +\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> +\n> }\n>\n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 6452b6a1acd2..dd7ebc07c534 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -11,6 +11,8 @@\n>\n> #include \n>\n> +#include \n> +\n> /**\n> * \\file awb.h\n> */\n> @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tcontext.activeState.awb.gains.green = asyncResults_.greenGain;\n> \tcontext.activeState.awb.gains.red = asyncResults_.redGain;\n> \tcontext.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> +\n> +\tmetadata.set(controls::AwbEnable, true);\n> +\tmetadata.set(controls::ColourGains, {\n> +\t\t\tstatic_cast(context.activeState.awb.gains.red),\n> +\t\t\tstatic_cast(context.activeState.awb.gains.blue)\n> +\t\t});\n> +\tmetadata.set(controls::ColourTemperature,\n> +\t\t context.activeState.awb.temperatureK);\n> }\n>\n> constexpr uint16_t Awb::threshold(float value)\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 68f017b04751..959f314f5452 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> *\n> * \\var IPASessionConfiguration::sensor.defVBlank\n> * \\brief The default vblank value of the sensor\n> + *\n> + * \\var IPASessionConfiguration::sensor.size\n> + * \\brief Sensor output resolution\n> */\n>\n> /**\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 36099353e9f2..e9a3863b1693 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> \tstruct {\n> \t\tint32_t defVBlank;\n> \t\tutils::Duration lineDuration;\n> +\t\tSize size;\n> \t} sensor;\n> };\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index bc0f6007baca..a9a2b49ca95b 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \t/* Initialise the sensor configuration. */\n> \tcontext_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> \t\t\t\t\t\t * 1.0s / sensorInfo_.pixelRate;\n> +\tcontext_.configuration.sensor.size = sensorInfo_.outputSize;\n>\n> \t/*\n> \t * Compute the sensor V4L2 controls to be used by the algorithms and\n> @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>\n> -\tdouble lineDuration = context_.configuration.sensor.lineDuration.get();\n> -\tint32_t vBlank = context_.configuration.sensor.defVBlank;\n> \tControlList metadata(controls::controls);\n>\n> \tfor (auto const &algo : algorithms())\n> @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>\n> \tsetControls(frame);\n>\n> -\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> -\tint64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;\n> -\tmetadata.set(controls::FrameDuration, frameDuration);\n> -\n> -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> -\n> -\tmetadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n> -\n> -\tmetadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);\n> -\n> \t/*\n> \t * \\todo The Metadata provides a path to getting extended data\n> \t * out to the application. Further data such as a simplifed Histogram\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 F3B2EBD16B\n\tfor ;\n\tThu, 20 Oct 2022 07:40:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76B6362E98;\n\tThu, 20 Oct 2022 09:40:42 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AFFF262DFF\n\tfor ;\n\tThu, 20 Oct 2022 09:40:41 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 09CAC20007;\n\tThu, 20 Oct 2022 07:40:40 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666251642;\n\tbh=HFdVguuucBnR5uPrHdB6ZXNFbMZvJTIVlsshYuw7hMM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rXbDiA1nuXvXYJuHT4C5r3Z2Se5hTH+1VrfQGWqnGVnfjYS18X6hoVuJ4tSBD3WOg\n\tfyKROxeV5oxC7/9Vo5YEBXcpTZT6bFETyxOecDMXlgrII0oX59b+C826pzOd3chBEt\n\tqPb7xl5TSZ29CAEQsdFZgTGbw6RUoW8WwZ0jtHtN7d2cF7Vu/SB9HScjcGzCt8lKMk\n\t/dGjbwKJeEzrxUL3ZfQkO7CGsrfpo6dSCmpr7J74AouU2PsKkrksbPqBWnURKZaRYa\n\t4GIAIg0wTBRXhd/ugKvsX/bg7ZSFrCSV98QfN/KQd14304IiJipBUImOR8cAL082Rm\n\txdlyae58box3w==","Date":"Thu, 20 Oct 2022 09:40:39 +0200","To":"Laurent Pinchart ","Message-ID":"<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25506,"web_url":"https://patchwork.libcamera.org/comment/25506/","msgid":"","date":"2022-10-20T09:25:50","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThis change doesn't affect the RPi platform, but I just wanted to give a\nquick comment.\n\nOn Wed, 19 Oct 2022 at 21:56, Laurent Pinchart via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> function. This removes the need to fill metadata manually in the IPA\n> module's processStatsBuffer() function.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> src/ipa/ipu3/ipa_context.cpp | 3 +++\n> src/ipa/ipu3/ipa_context.h | 1 +\n> src/ipa/ipu3/ipu3.cpp | 13 +------------\n> 5 files changed, 30 insertions(+), 13 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp\n> b/src/ipa/ipu3/algorithms/agc.cpp\n> index f4e559bf8b84..b5309bdbea25 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> #include \"libipa/histogram.h\"\n> @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState\n> &activeState,\n> void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t\n> frame,\n> IPAFrameContext &frameContext,\n> const ipu3_uapi_stats_3a *stats,\n> - [[maybe_unused]] ControlList &metadata)\n> + ControlList &metadata)\n> {\n> /*\n> * Estimate the gain needed to have the proportion of pixels in a\n> given\n> @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n>\n> computeExposure(context, frameContext, yGain, iqMeanGain);\n> frameCount_++;\n> +\n> + utils::Duration exposureTime =\n> context.configuration.sensor.lineDuration\n> + * frameContext.sensor.exposure;\n> + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> + metadata.set(controls::ExposureTime,\n> exposureTime.get());\n>\n\nPutting the metadata.set() calls into the algorithm source code like above\nwould\nmean that the metadata is populated with the values the AGC requested. This\ncould be wrong because:\n\n1) If the sensor quantises the gain and/or shutter speed values from what\nAGC asked for.\n2) Unless I am mistaken, this does not account for sensor delays.\n\nI am assuming the metadata list here is what is returned back via the\ncompleted\nrequest, is that correct? Of course, my understanding of this change could\nbe completely\nwrong, so please ignore the noise :-)\n\nRegards,\nNaush\n\n\n> +\n> + /* \\todo Use VBlank value calculated from each frame exposure. */\n> + uint32_t vTotal = context.configuration.sensor.size.height\n> + + context.configuration.sensor.defVBlank;\n> + utils::Duration frameDuration =\n> context.configuration.sensor.lineDuration\n> + * vTotal;\n> + metadata.set(controls::FrameDuration,\n> frameDuration.get());\n> +\n> }\n>\n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp\n> b/src/ipa/ipu3/algorithms/awb.cpp\n> index 6452b6a1acd2..dd7ebc07c534 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -11,6 +11,8 @@\n>\n> #include \n>\n> +#include \n> +\n> /**\n> * \\file awb.h\n> */\n> @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> context.activeState.awb.gains.green = asyncResults_.greenGain;\n> context.activeState.awb.gains.red = asyncResults_.redGain;\n> context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> +\n> + metadata.set(controls::AwbEnable, true);\n> + metadata.set(controls::ColourGains, {\n> +\n> static_cast(context.activeState.awb.gains.red),\n> +\n> static_cast(context.activeState.awb.gains.blue)\n> + });\n> + metadata.set(controls::ColourTemperature,\n> + context.activeState.awb.temperatureK);\n> }\n>\n> constexpr uint16_t Awb::threshold(float value)\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 68f017b04751..959f314f5452 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> *\n> * \\var IPASessionConfiguration::sensor.defVBlank\n> * \\brief The default vblank value of the sensor\n> + *\n> + * \\var IPASessionConfiguration::sensor.size\n> + * \\brief Sensor output resolution\n> */\n>\n> /**\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 36099353e9f2..e9a3863b1693 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> struct {\n> int32_t defVBlank;\n> utils::Duration lineDuration;\n> + Size size;\n> } sensor;\n> };\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index bc0f6007baca..a9a2b49ca95b 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> /* Initialise the sensor configuration. */\n> context_.configuration.sensor.lineDuration =\n> sensorInfo_.minLineLength\n> * 1.0s /\n> sensorInfo_.pixelRate;\n> + context_.configuration.sensor.size = sensorInfo_.outputSize;\n>\n> /*\n> * Compute the sensor V4L2 controls to be used by the algorithms\n> and\n> @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> frameContext.sensor.exposure =\n> sensorControls.get(V4L2_CID_EXPOSURE).get();\n> frameContext.sensor.gain =\n> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>\n> - double lineDuration =\n> context_.configuration.sensor.lineDuration.get();\n> - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> ControlList metadata(controls::controls);\n>\n> for (auto const &algo : algorithms())\n> @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>\n> setControls(frame);\n>\n> - /* \\todo Use VBlank value calculated from each frame exposure. */\n> - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) *\n> lineDuration;\n> - metadata.set(controls::FrameDuration, frameDuration);\n> -\n> - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> -\n> - metadata.set(controls::ColourTemperature,\n> context_.activeState.awb.temperatureK);\n> -\n> - metadata.set(controls::ExposureTime, frameContext.sensor.exposure\n> * lineDuration);\n> -\n> /*\n> * \\todo The Metadata provides a path to getting extended data\n> * out to the application. Further data such as a simplifed\n> Histogram\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"","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 DF2F1C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 09:26:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9619962EA4;\n\tThu, 20 Oct 2022 11:26:08 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1012B62E9C\n\tfor ;\n\tThu, 20 Oct 2022 11:26:07 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id f37so32480698lfv.8\n\tfor ;\n\tThu, 20 Oct 2022 02:26:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666257968;\n\tbh=cktV/um8mEgoh6RfjP8PBLG8UBpgZmqhlHMQa8BBJGg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=2LqPo0bckBsmlPa65ndelUPnSGQQxL5kumBny8g9q6W5hH95NunR6yqcaBJQpUZp5\n\tMbFRB9Toasn1xMuELM2dczoGORHX0hBQ44nZI+FqgSKFmGJUNocTMCMFpZI/iI+Jqm\n\tWYGNeWUO8GMcy9si49G+lks7CJf2ig0Cy0oUCADChEcZbY4WKxsHtfAd6mT8bjzS+K\n\tP2WZHvI5YzK3hLKitPRK1zzSDFXLZIHuSR25Yz/+P9YX6gVQrIWoqVNoWmZ21jNCd8\n\tpKRDUygDUxxsyvpxKe6tzM9C7bxx2u5fnqVtSG2nNjzWlbY2BZ1JFgpERzBPoghj2E\n\tHSwZ2Efqf3ElA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=WLzfxYqnkwkFXzKusfPf6lf0qxz+577lxbJcgVi6IpM=;\n\tb=QaqV+iV4vQPcRlESOS80hrJG5L4CN25ccpcwI7DuzYW4EspNuuW7yBFWgxlrma9BS9\n\tOugkqgOFOckA5okZsKIWXRFuD8k/ILzmVWrT1IMrN3klfMm/Keitqknevq1T9iCxcCOg\n\t/yc/Ib6PZFlScCrKkDjJ/2IcLAtGhD3UPRP8xuuDcJ+DzD0R+uOXf4xcB2CMuZ3Dpjf0\n\tCFevFcPiROPVBjdfk+fsBAStmrk0BMWMwfmRQkVizgN0SSJJmWAO1ZyYDojolVxH1eiI\n\txassmy6L2VUe1iI/N5OAvUYA0UMXYl9o0QVkjlkEYfgRYFtZ+odscnKb+jmWzEt7jG3n\n\tjRNw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"QaqV+iV4\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=WLzfxYqnkwkFXzKusfPf6lf0qxz+577lxbJcgVi6IpM=;\n\tb=CJD9YzTyNifjx+BJcFFXZPrUWk1rAaMf+YK9DckXfV8USf2BCcipkHDhBH/JbI+9R1\n\tslY/ILHSKRKdNrjTbhS2OhR2RKA6Jn2idcFuK/2magpkoPYW+iSVSg7Lpigf1PWtKQRT\n\tGnTjI4IBUUA+FEqv3RHgs97F59ng9YdhpQOxWjcXnDN5eA2MjGhmVffUIeiwV7WV8r5D\n\t+CHUb7C1BETdKMzHH307/tp8v2wK9sX6ouuuajThLx3fEk/MiUYYPfnxMOLGDfkkgB+U\n\tshiimgNolsIdX4Jj5yHtyO9/xxgtNKAOmtr/+izIV3ymuDNa/N2D0TuYxiHrNLUV5Ewx\n\ttw6Q==","X-Gm-Message-State":"ACrzQf0K8i9hh+GJMM5RynMp7sPvJswlWI1qZ76lRqZpCEAVF/g+7NPn\n\tKTori6t3l3ATjUGziZc33YMzOZRfJFfcr4nShVyRmtawxfg=","X-Google-Smtp-Source":"AMsMyM5VVxExLMFS4yNlKhirrCssuf3llCB+L1RT8LQoktD6PEbpFEkzs2EPFTFsEDcAFZnOKBOibvvTQq309b53FTc=","X-Received":"by 2002:ac2:4c03:0:b0:4a2:2273:89c6 with SMTP id\n\tt3-20020ac24c03000000b004a2227389c6mr3994833lfq.245.1666257966333;\n\tThu, 20 Oct 2022 02:26:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>","Date":"Thu, 20 Oct 2022 10:25:50 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000003022fb05eb73ea2f\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25512,"web_url":"https://patchwork.libcamera.org/comment/25512/","msgid":"","date":"2022-10-20T10:33:55","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Oct 20, 2022 at 09:40:39AM +0200, Jacopo Mondi wrote:\n> On Wed, Oct 19, 2022 at 11:56:14PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > function. This removes the need to fill metadata manually in the IPA\n> > module's processStatsBuffer() function.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > src/ipa/ipu3/ipa_context.h | 1 +\n> > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > 5 files changed, 30 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > index f4e559bf8b84..b5309bdbea25 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -14,6 +14,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > #include \"libipa/histogram.h\"\n> > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > \t\t IPAFrameContext &frameContext,\n> > \t\t const ipu3_uapi_stats_3a *stats,\n> > -\t\t [[maybe_unused]] ControlList &metadata)\n> > +\t\t ControlList &metadata)\n> > {\n> > \t/*\n> > \t * Estimate the gain needed to have the proportion of pixels in a given\n> > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> >\n> > \tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> > \tframeCount_++;\n> > +\n> > +\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > +\t\t\t\t * frameContext.sensor.exposure;\n> > +\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > +\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> > +\n> > +\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> \n> What block will adjust the frame duration ? AEGC for sure can ends up\n> enlarging blankings, what other component will handle control of\n> vblank ?\n\nI think it will be AGC only.\n\n> Nothing that interefere with this patch\n> Reviewed-by: Jacopo Mondi \n> \n> > +\tuint32_t vTotal = context.configuration.sensor.size.height\n> > +\t\t\t+ context.configuration.sensor.defVBlank;\n> > +\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> > +\t\t\t\t * vTotal;\n> > +\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> > +\n> > }\n> >\n> > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > index 6452b6a1acd2..dd7ebc07c534 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -11,6 +11,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > /**\n> > * \\file awb.h\n> > */\n> > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > \tcontext.activeState.awb.gains.green = asyncResults_.greenGain;\n> > \tcontext.activeState.awb.gains.red = asyncResults_.redGain;\n> > \tcontext.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> > +\n> > +\tmetadata.set(controls::AwbEnable, true);\n> > +\tmetadata.set(controls::ColourGains, {\n> > +\t\t\tstatic_cast(context.activeState.awb.gains.red),\n> > +\t\t\tstatic_cast(context.activeState.awb.gains.blue)\n> > +\t\t});\n> > +\tmetadata.set(controls::ColourTemperature,\n> > +\t\t context.activeState.awb.temperatureK);\n> > }\n> >\n> > constexpr uint16_t Awb::threshold(float value)\n> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > index 68f017b04751..959f314f5452 100644\n> > --- a/src/ipa/ipu3/ipa_context.cpp\n> > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > *\n> > * \\var IPASessionConfiguration::sensor.defVBlank\n> > * \\brief The default vblank value of the sensor\n> > + *\n> > + * \\var IPASessionConfiguration::sensor.size\n> > + * \\brief Sensor output resolution\n> > */\n> >\n> > /**\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 36099353e9f2..e9a3863b1693 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > \tstruct {\n> > \t\tint32_t defVBlank;\n> > \t\tutils::Duration lineDuration;\n> > +\t\tSize size;\n> > \t} sensor;\n> > };\n> >\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index bc0f6007baca..a9a2b49ca95b 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > \t/* Initialise the sensor configuration. */\n> > \tcontext_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> > \t\t\t\t\t\t * 1.0s / sensorInfo_.pixelRate;\n> > +\tcontext_.configuration.sensor.size = sensorInfo_.outputSize;\n> >\n> > \t/*\n> > \t * Compute the sensor V4L2 controls to be used by the algorithms and\n> > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> >\n> > -\tdouble lineDuration = context_.configuration.sensor.lineDuration.get();\n> > -\tint32_t vBlank = context_.configuration.sensor.defVBlank;\n> > \tControlList metadata(controls::controls);\n> >\n> > \tfor (auto const &algo : algorithms())\n> > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> >\n> > \tsetControls(frame);\n> >\n> > -\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> > -\tint64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;\n> > -\tmetadata.set(controls::FrameDuration, frameDuration);\n> > -\n> > -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > -\n> > -\tmetadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n> > -\n> > -\tmetadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);\n> > -\n> > \t/*\n> > \t * \\todo The Metadata provides a path to getting extended data\n> > \t * out to the application. Further data such as a simplifed Histogram","headers":{"Return-Path":"","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 28B16BD16B\n\tfor ;\n\tThu, 20 Oct 2022 10:34:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8524E62EA5;\n\tThu, 20 Oct 2022 12:34: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 9C9D0604DB\n\tfor ;\n\tThu, 20 Oct 2022 12:34:20 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E4F8570;\n\tThu, 20 Oct 2022 12:34:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666262061;\n\tbh=iOyjwP1OpTDZHBXPemOHcQRzk52E5a52pTJOmcoZnXM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PDNVEZfgyFOEaMnUI1OFb81gUdLJAM5LzTf1b0NsW9okgC52pze5RTeohWHcp9HyH\n\tknABN0X6j0abVutKcXPlIhYlq7ukEZkz8D/FnXqMmA3NLhVxA/5Zce6tZCwljbUuZf\n\t+el9JeLSFpVIhBEzKsbKW7NzlA4kphVqeMoKOoiewX47aRl4lNrHN9J4M6/5pYQI4a\n\t3+ku+sDFY4ZKNWPHkioB/zt7bAIByHQgHFJtJWtj+JxCy7aSD761chFvoQIGcdL4zE\n\tCV+KKsvvV+a8sCs9j6EbptY0eL09B+fFRLQr3Ip2s5B+sxN6GP0jnsh4j2ObXU004/\n\tCzYp6aa4nMONQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666262060;\n\tbh=iOyjwP1OpTDZHBXPemOHcQRzk52E5a52pTJOmcoZnXM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Q1VAVRgf4IV17kZqOZIBRxSVrNCRV718qNAXYhKt2LaKFy0X6sRLN0glyTeDOLnpE\n\tUGyfxbeOHHq4MdJQ7yAWr4heZVaCPWPf0OBJXyFh5Ylzf95eNRoZz6Iw1Rtsv3pH/3\n\tYGCxsOFwIHHdNywHNFgv7aHJf1joEmMo7hEQ7iys="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Q1VAVRgf\"; dkim-atps=neutral","Date":"Thu, 20 Oct 2022 13:33:55 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25516,"web_url":"https://patchwork.libcamera.org/comment/25516/","msgid":"<166628167926.2560709.9764677047577550538@Monstersaurus>","date":"2022-10-20T16:01:19","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2022-10-20 10:25:50)\n> Hi Laurent,\n> \n> This change doesn't affect the RPi platform, but I just wanted to give a\n> quick comment.\n> \n> On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart via libcamera-devel <\n> libcamera-devel@lists.libcamera.org> wrote:\n> \n> > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > function. This removes the need to fill metadata manually in the IPA\n> > module's processStatsBuffer() function.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > src/ipa/ipu3/ipa_context.h | 1 +\n> > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > 5 files changed, 30 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp\n> > b/src/ipa/ipu3/algorithms/agc.cpp\n> > index f4e559bf8b84..b5309bdbea25 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -14,6 +14,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > #include \"libipa/histogram.h\"\n> > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState\n> > &activeState,\n> > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t\n> > frame,\n> > IPAFrameContext &frameContext,\n> > const ipu3_uapi_stats_3a *stats,\n> > - [[maybe_unused]] ControlList &metadata)\n> > + ControlList &metadata)\n> > {\n> > /*\n> > * Estimate the gain needed to have the proportion of pixels in a\n> > given\n> > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context,\n> > [[maybe_unused]] const uint32_t frame,\n> >\n> > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > frameCount_++;\n> > +\n> > + utils::Duration exposureTime =\n> > context.configuration.sensor.lineDuration\n> > + * frameContext.sensor.exposure;\n> > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > + metadata.set(controls::ExposureTime,\n> > exposureTime.get());\n> >\n> \n> Putting the metadata.set() calls into the algorithm source code like above\n> would\n> mean that the metadata is populated with the values the AGC requested. This\n> could be wrong because:\n\nThe frame context stores multiple copies of the exposure and gain.\n\nframeContext.sensor.{exposure,gain} are the values that are determined\nto have been set according to delayed controls. These get set currently\nin\n\nvoid IPAIPU3::processStatsBuffer(const uint32_t frame,\n [[maybe_unused]] const int64_t frameTimestamp,\n const uint32_t bufferId, const ControlList &sensorControls)\n\n\nIt looks like we don't actually store what the algorithm specifically\nwanted on that frame, so we're only storing what the agc 'currently'\nwould like (in the active state) and here we get from delayed controls\nwhat was 'actually' set for the frame.\n\nSo I believe it's correct.\n\n> \n> 1) If the sensor quantises the gain and/or shutter speed values from what\n> AGC asked for.\n> 2) Unless I am mistaken, this does not account for sensor delays.\n\nThe results for the frame are passed from the pipeline handler to the\nIPA from delayed controls. So it should all be handled.\n\n \n> I am assuming the metadata list here is what is returned back via the\n> completed\n> request, is that correct? Of course, my understanding of this change could\n> be completely\n> wrong, so please ignore the noise :-)\n\nYes, this completed metadata is what the application should receive.\nIn this instance, it's a bit long winded, as the exposure/gains are\nbeing sent from the pipeline handler, into the IPA, stored in the frame\ncontext.sensor{} ... and then assigned to the metadata by the AGC\nalgorithm, which then returns the completed metadata back to the\npipeline handler for the request to be delivered to the application.\n\n--\nKieran\n\n\n> \n> Regards,\n> Naush\n> \n> \n> > +\n> > + /* \\todo Use VBlank value calculated from each frame exposure. */\n> > + uint32_t vTotal = context.configuration.sensor.size.height\n> > + + context.configuration.sensor.defVBlank;\n> > + utils::Duration frameDuration =\n> > context.configuration.sensor.lineDuration\n> > + * vTotal;\n> > + metadata.set(controls::FrameDuration,\n> > frameDuration.get());\n> > +\n> > }\n> >\n> > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp\n> > b/src/ipa/ipu3/algorithms/awb.cpp\n> > index 6452b6a1acd2..dd7ebc07c534 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -11,6 +11,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > /**\n> > * \\file awb.h\n> > */\n> > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context,\n> > [[maybe_unused]] const uint32_t frame,\n> > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> > +\n> > + metadata.set(controls::AwbEnable, true);\n> > + metadata.set(controls::ColourGains, {\n> > +\n> > static_cast(context.activeState.awb.gains.red),\n> > +\n> > static_cast(context.activeState.awb.gains.blue)\n> > + });\n> > + metadata.set(controls::ColourTemperature,\n> > + context.activeState.awb.temperatureK);\n> > }\n> >\n> > constexpr uint16_t Awb::threshold(float value)\n> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > index 68f017b04751..959f314f5452 100644\n> > --- a/src/ipa/ipu3/ipa_context.cpp\n> > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > *\n> > * \\var IPASessionConfiguration::sensor.defVBlank\n> > * \\brief The default vblank value of the sensor\n> > + *\n> > + * \\var IPASessionConfiguration::sensor.size\n> > + * \\brief Sensor output resolution\n> > */\n> >\n> > /**\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 36099353e9f2..e9a3863b1693 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > struct {\n> > int32_t defVBlank;\n> > utils::Duration lineDuration;\n> > + Size size;\n> > } sensor;\n> > };\n> >\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index bc0f6007baca..a9a2b49ca95b 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > /* Initialise the sensor configuration. */\n> > context_.configuration.sensor.lineDuration =\n> > sensorInfo_.minLineLength\n> > * 1.0s /\n> > sensorInfo_.pixelRate;\n> > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> >\n> > /*\n> > * Compute the sensor V4L2 controls to be used by the algorithms\n> > and\n> > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > frameContext.sensor.exposure =\n> > sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > frameContext.sensor.gain =\n> > camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> >\n> > - double lineDuration =\n> > context_.configuration.sensor.lineDuration.get();\n> > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > ControlList metadata(controls::controls);\n> >\n> > for (auto const &algo : algorithms())\n> > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> >\n> > setControls(frame);\n> >\n> > - /* \\todo Use VBlank value calculated from each frame exposure. */\n> > - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) *\n> > lineDuration;\n> > - metadata.set(controls::FrameDuration, frameDuration);\n> > -\n> > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > -\n> > - metadata.set(controls::ColourTemperature,\n> > context_.activeState.awb.temperatureK);\n> > -\n> > - metadata.set(controls::ExposureTime, frameContext.sensor.exposure\n> > * lineDuration);\n> > -\n> > /*\n> > * \\todo The Metadata provides a path to getting extended data\n> > * out to the application. Further data such as a simplifed\n> > Histogram\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >\n> >","headers":{"Return-Path":"","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 848A5BD16B\n\tfor ;\n\tThu, 20 Oct 2022 16:01:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A0D7962EAC;\n\tThu, 20 Oct 2022 18:01:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2250C604DB\n\tfor ;\n\tThu, 20 Oct 2022 18:01:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89D73D38;\n\tThu, 20 Oct 2022 18:01:22 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666281684;\n\tbh=+cU+rSTvdxByz/eDkV+c9o5ibTZSo/wylGEOTGDwOm8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fCsdJqQEcfgCT99i/sU78JN4AukcxkXDHoG9Dqh+YHWOTNMU3dvvehPyd1pME5W0m\n\tUfDcmrnGxJNUkCL2YBdhdqkyqK3gx8vQovtx1aPQ2PCvSGGDVAkntFb1ROZyNz4G5S\n\tg6jwwcZxcg80CLXcyI38GNlghHoXaihd9F2GQicBOqJvModzS6zYDsb+0R/65GL3xs\n\tUznaos+bxW0zsT/40tj8pBX1ROFIOXl3cLXm+91jsbjSZZzED9JPMfMYP5Ax9vOQaX\n\t1JY46xZFAVgIWYTZEC2WnvgOLkqgD+L6SuxeWm0n6jTmnfi3B2SSyrv9HMotFFylSd\n\tH58yD5UmUo2Xg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666281682;\n\tbh=+cU+rSTvdxByz/eDkV+c9o5ibTZSo/wylGEOTGDwOm8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=X46zSB08XxeDl4lDhV5H8ENx0oVHWRQkoqLeWuHq/789O1vlUelqgV7rC2J0dPLwa\n\t+q9uMcWHos6l6Hy4MWFjlAR0nYLAlWFOXDUET7QHbukW0G/Kjv0aPWbl6NSwcPj2vb\n\tCNIi2PxEhgzPX3shB+yTB6o+wqQrtJLT/8u1opVs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"X46zSB08\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t","To":"Laurent Pinchart ,\n\tNaushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Thu, 20 Oct 2022 17:01:19 +0100","Message-ID":"<166628167926.2560709.9764677047577550538@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25517,"web_url":"https://patchwork.libcamera.org/comment/25517/","msgid":"<166628230239.2560709.17471437859147544451@Monstersaurus>","date":"2022-10-20T16:11:42","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-10-20 11:33:55)\n> Hi Jacopo,\n> \n> On Thu, Oct 20, 2022 at 09:40:39AM +0200, Jacopo Mondi wrote:\n> > On Wed, Oct 19, 2022 at 11:56:14PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > > function. This removes the need to fill metadata manually in the IPA\n> > > module's processStatsBuffer() function.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > > src/ipa/ipu3/ipa_context.h | 1 +\n> > > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > > 5 files changed, 30 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > > index f4e559bf8b84..b5309bdbea25 100644\n> > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > @@ -14,6 +14,7 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \"libipa/histogram.h\"\n> > > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > IPAFrameContext &frameContext,\n> > > const ipu3_uapi_stats_3a *stats,\n> > > - [[maybe_unused]] ControlList &metadata)\n> > > + ControlList &metadata)\n> > > {\n> > > /*\n> > > * Estimate the gain needed to have the proportion of pixels in a given\n> > > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > >\n> > > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > > frameCount_++;\n> > > +\n> > > + utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > > + * frameContext.sensor.exposure;\n> > > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > + metadata.set(controls::ExposureTime, exposureTime.get());\n> > > +\n> > > + /* \\todo Use VBlank value calculated from each frame exposure. */\n> > \n> > What block will adjust the frame duration ? AEGC for sure can ends up\n> > enlarging blankings, what other component will handle control of\n> > vblank ?\n> \n> I think it will be AGC only.\n\nYes, I don't think we would split AEC and AGC. This agc algorithm deals\nwith both auto-exposure, and auto-gain. Perhaps it should be named aegc\n... ?\n\n\n> \n> > Nothing that interefere with this patch\n> > Reviewed-by: Jacopo Mondi \n> > \n> > > + uint32_t vTotal = context.configuration.sensor.size.height\n> > > + + context.configuration.sensor.defVBlank;\n> > > + utils::Duration frameDuration = context.configuration.sensor.lineDuration\n> > > + * vTotal;\n> > > + metadata.set(controls::FrameDuration, frameDuration.get());\n> > > +\n> > > }\n> > >\n> > > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > > index 6452b6a1acd2..dd7ebc07c534 100644\n> > > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > > @@ -11,6 +11,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > /**\n> > > * \\file awb.h\n> > > */\n> > > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > > context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> > > +\n> > > + metadata.set(controls::AwbEnable, true);\n> > > + metadata.set(controls::ColourGains, {\n> > > + static_cast(context.activeState.awb.gains.red),\n> > > + static_cast(context.activeState.awb.gains.blue)\n> > > + });\n> > > + metadata.set(controls::ColourTemperature,\n> > > + context.activeState.awb.temperatureK);\n\nThanks,\n\nI think this all makes sense for the moment.\n\n\n\n> > > }\n> > >\n> > > constexpr uint16_t Awb::threshold(float value)\n> > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > > index 68f017b04751..959f314f5452 100644\n> > > --- a/src/ipa/ipu3/ipa_context.cpp\n> > > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > > *\n> > > * \\var IPASessionConfiguration::sensor.defVBlank\n> > > * \\brief The default vblank value of the sensor\n> > > + *\n> > > + * \\var IPASessionConfiguration::sensor.size\n> > > + * \\brief Sensor output resolution\n> > > */\n> > >\n> > > /**\n> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > index 36099353e9f2..e9a3863b1693 100644\n> > > --- a/src/ipa/ipu3/ipa_context.h\n> > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > > struct {\n> > > int32_t defVBlank;\n> > > utils::Duration lineDuration;\n> > > + Size size;\n> > > } sensor;\n> > > };\n> > >\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index bc0f6007baca..a9a2b49ca95b 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > > /* Initialise the sensor configuration. */\n> > > context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> > > * 1.0s / sensorInfo_.pixelRate;\n> > > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> > >\n> > > /*\n> > > * Compute the sensor V4L2 controls to be used by the algorithms and\n> > > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > > frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > >\n> > > - double lineDuration = context_.configuration.sensor.lineDuration.get();\n> > > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > > ControlList metadata(controls::controls);\n> > >\n> > > for (auto const &algo : algorithms())\n> > > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > >\n> > > setControls(frame);\n> > >\n> > > - /* \\todo Use VBlank value calculated from each frame exposure. */\n> > > - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;\n> > > - metadata.set(controls::FrameDuration, frameDuration);\n> > > -\n> > > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > -\n> > > - metadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n> > > -\n> > > - metadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);\n> > > -\n\nI'm half worrying about what would happen if the agc was optional and\ndisabled... But I also think it shouldn't be all too optional right now.\n\nReviewed-by: Kieran Bingham \n\n> > > /*\n> > > * \\todo The Metadata provides a path to getting extended data\n> > > * out to the application. Further data such as a simplifed Histogram\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","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 276E1C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 16:11:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E57362EAD;\n\tThu, 20 Oct 2022 18:11:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CDD3604DB\n\tfor ;\n\tThu, 20 Oct 2022 18:11:45 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DD983D38;\n\tThu, 20 Oct 2022 18:11:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666282306;\n\tbh=8X5zUEXPJVe4bErcPD8ADRyBYzrg43cBi6GTioEKdYg=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=lNkaRUiJwgY/VRv/DU9zDqIHzVpInPbT40N9O/x5wKJfGlbpgVJkbOr6NN2So9ih5\n\twcyPDDcPXOxoButg1kZ92P/C6AFwS7xjk5oYlFuKPO3UtcnJjNZ0axZtyQxN7cdR+6\n\tpM6oymeN4b6MKiy8pxUGtupr8a7DUOR6pHXMqY8jcQPEMIBlL63jJD9h/H347vqSP0\n\tJyNAr/TyHLvENrJ9/nay9o/BpLtPdckbfwiRhi9tFx5+6noZKy7I7rBDGOY9VkMmr5\n\t3VQEhyxDMJBl5BotPyXb6WCsVkFqwWLKIB+RXLfU1bOmk6NAn5MRqdQsqQl06cSmS1\n\tuSav5zGSMwTRg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666282305;\n\tbh=8X5zUEXPJVe4bErcPD8ADRyBYzrg43cBi6GTioEKdYg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rt3dVggZLMELLqYbFtpo9+5pu4GFrwg80K6ucJISEwFEH15UQqxPAOvDsIcxDSFuy\n\tERJSgchCZbRNSURVLVc2MDVmTWAc2/32FsskOApGGvVcrmbh2oet8dxoug53X4LQ9D\n\tTaWkq3xJT6vamzVPc8KKxqg2BN1RNbKwrPqzX1ZA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rt3dVggZ\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>\n\t","To":"Jacopo Mondi ,\n\tLaurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t","Date":"Thu, 20 Oct 2022 17:11:42 +0100","Message-ID":"<166628230239.2560709.17471437859147544451@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25518,"web_url":"https://patchwork.libcamera.org/comment/25518/","msgid":"","date":"2022-10-20T16:55:47","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 20 Oct 2022 at 17:01, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck via libcamera-devel (2022-10-20 10:25:50)\n> > Hi Laurent,\n> >\n> > This change doesn't affect the RPi platform, but I just wanted to give a\n> > quick comment.\n> >\n> > On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart via libcamera-devel <\n> > libcamera-devel@lists.libcamera.org> wrote:\n> >\n> > > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > > function. This removes the need to fill metadata manually in the IPA\n> > > module's processStatsBuffer() function.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > > src/ipa/ipu3/ipa_context.h | 1 +\n> > > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > > 5 files changed, 30 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp\n> > > b/src/ipa/ipu3/algorithms/agc.cpp\n> > > index f4e559bf8b84..b5309bdbea25 100644\n> > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > @@ -14,6 +14,7 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \"libipa/histogram.h\"\n> > > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState\n> > > &activeState,\n> > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t\n> > > frame,\n> > > IPAFrameContext &frameContext,\n> > > const ipu3_uapi_stats_3a *stats,\n> > > - [[maybe_unused]] ControlList &metadata)\n> > > + ControlList &metadata)\n> > > {\n> > > /*\n> > > * Estimate the gain needed to have the proportion of pixels\n> in a\n> > > given\n> > > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context,\n> > > [[maybe_unused]] const uint32_t frame,\n> > >\n> > > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > > frameCount_++;\n> > > +\n> > > + utils::Duration exposureTime =\n> > > context.configuration.sensor.lineDuration\n> > > + * frameContext.sensor.exposure;\n> > > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > + metadata.set(controls::ExposureTime,\n> > > exposureTime.get());\n> > >\n> >\n> > Putting the metadata.set() calls into the algorithm source code like\n> above\n> > would\n> > mean that the metadata is populated with the values the AGC requested.\n> This\n> > could be wrong because:\n>\n> The frame context stores multiple copies of the exposure and gain.\n>\n> frameContext.sensor.{exposure,gain} are the values that are determined\n> to have been set according to delayed controls. These get set currently\n> in\n>\n> void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> [[maybe_unused]] const int64_t\n> frameTimestamp,\n> const uint32_t bufferId, const\n> ControlList &sensorControls)\n>\n>\n> It looks like we don't actually store what the algorithm specifically\n> wanted on that frame, so we're only storing what the agc 'currently'\n> would like (in the active state) and here we get from delayed controls\n> what was 'actually' set for the frame.\n>\n> So I believe it's correct.\n>\n\nAh, that all makes sense now, thanks for explaining!\n\nNaush\n\n\n> >\n> > 1) If the sensor quantises the gain and/or shutter speed values from what\n> > AGC asked for.\n> > 2) Unless I am mistaken, this does not account for sensor delays.\n>\n> The results for the frame are passed from the pipeline handler to the\n> IPA from delayed controls. So it should all be handled.\n>\n>\n> > I am assuming the metadata list here is what is returned back via the\n> > completed\n> > request, is that correct? Of course, my understanding of this change\n> could\n> > be completely\n> > wrong, so please ignore the noise :-)\n>\n> Yes, this completed metadata is what the application should receive.\n> In this instance, it's a bit long winded, as the exposure/gains are\n> being sent from the pipeline handler, into the IPA, stored in the frame\n> context.sensor{} ... and then assigned to the metadata by the AGC\n> algorithm, which then returns the completed metadata back to the\n> pipeline handler for the request to be delivered to the application.\n>\n> --\n> Kieran\n>\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > > +\n> > > + /* \\todo Use VBlank value calculated from each frame exposure.\n> */\n> > > + uint32_t vTotal = context.configuration.sensor.size.height\n> > > + + context.configuration.sensor.defVBlank;\n> > > + utils::Duration frameDuration =\n> > > context.configuration.sensor.lineDuration\n> > > + * vTotal;\n> > > + metadata.set(controls::FrameDuration,\n> > > frameDuration.get());\n> > > +\n> > > }\n> > >\n> > > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp\n> > > b/src/ipa/ipu3/algorithms/awb.cpp\n> > > index 6452b6a1acd2..dd7ebc07c534 100644\n> > > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > > @@ -11,6 +11,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > /**\n> > > * \\file awb.h\n> > > */\n> > > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context,\n> > > [[maybe_unused]] const uint32_t frame,\n> > > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > > context.activeState.awb.temperatureK =\n> asyncResults_.temperatureK;\n> > > +\n> > > + metadata.set(controls::AwbEnable, true);\n> > > + metadata.set(controls::ColourGains, {\n> > > +\n> > > static_cast(context.activeState.awb.gains.red),\n> > > +\n> > > static_cast(context.activeState.awb.gains.blue)\n> > > + });\n> > > + metadata.set(controls::ColourTemperature,\n> > > + context.activeState.awb.temperatureK);\n> > > }\n> > >\n> > > constexpr uint16_t Awb::threshold(float value)\n> > > diff --git a/src/ipa/ipu3/ipa_context.cpp\n> b/src/ipa/ipu3/ipa_context.cpp\n> > > index 68f017b04751..959f314f5452 100644\n> > > --- a/src/ipa/ipu3/ipa_context.cpp\n> > > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > > *\n> > > * \\var IPASessionConfiguration::sensor.defVBlank\n> > > * \\brief The default vblank value of the sensor\n> > > + *\n> > > + * \\var IPASessionConfiguration::sensor.size\n> > > + * \\brief Sensor output resolution\n> > > */\n> > >\n> > > /**\n> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > index 36099353e9f2..e9a3863b1693 100644\n> > > --- a/src/ipa/ipu3/ipa_context.h\n> > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > > struct {\n> > > int32_t defVBlank;\n> > > utils::Duration lineDuration;\n> > > + Size size;\n> > > } sensor;\n> > > };\n> > >\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index bc0f6007baca..a9a2b49ca95b 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo\n> &configInfo,\n> > > /* Initialise the sensor configuration. */\n> > > context_.configuration.sensor.lineDuration =\n> > > sensorInfo_.minLineLength\n> > > * 1.0s /\n> > > sensorInfo_.pixelRate;\n> > > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> > >\n> > > /*\n> > > * Compute the sensor V4L2 controls to be used by the\n> algorithms\n> > > and\n> > > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t\n> frame,\n> > > frameContext.sensor.exposure =\n> > > sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > > frameContext.sensor.gain =\n> > >\n> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > >\n> > > - double lineDuration =\n> > > context_.configuration.sensor.lineDuration.get();\n> > > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > > ControlList metadata(controls::controls);\n> > >\n> > > for (auto const &algo : algorithms())\n> > > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t\n> frame,\n> > >\n> > > setControls(frame);\n> > >\n> > > - /* \\todo Use VBlank value calculated from each frame exposure.\n> */\n> > > - int64_t frameDuration = (vBlank +\n> sensorInfo_.outputSize.height) *\n> > > lineDuration;\n> > > - metadata.set(controls::FrameDuration, frameDuration);\n> > > -\n> > > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > -\n> > > - metadata.set(controls::ColourTemperature,\n> > > context_.activeState.awb.temperatureK);\n> > > -\n> > > - metadata.set(controls::ExposureTime,\n> frameContext.sensor.exposure\n> > > * lineDuration);\n> > > -\n> > > /*\n> > > * \\todo The Metadata provides a path to getting extended data\n> > > * out to the application. Further data such as a simplifed\n> > > Histogram\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart\n> > >\n> > >\n>","headers":{"Return-Path":"","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 24CE0C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 16:56:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B99462EAD;\n\tThu, 20 Oct 2022 18:56:06 +0200 (CEST)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45015604DB\n\tfor ;\n\tThu, 20 Oct 2022 18:56:05 +0200 (CEST)","by mail-lf1-x12a.google.com with SMTP id bu25so441843lfb.3\n\tfor ;\n\tThu, 20 Oct 2022 09:56:05 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666284966;\n\tbh=UnsQEww12cWeShuRLliJ9s8qkIEdx68Udt867wh8imE=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cOKUokc4h/CAocgqsEzrUHAe67g5jj+n/ZEBpKmZ+89eDG+eq8IN99oUjLDdZMwrg\n\th0BEUtReYLtwvBYn6U5WNx/HCNSO8aVdxiblFiGmejspT5Yixb7yMLuJteKNJJg/Ev\n\t+pR22eJDiIE9xrTx9R/yTxFEQwHITxntoZZ+GwY8qKzTgt57YRoQRMcqJNEnqU2LnQ\n\tAZffP+cVHLVkarWWV9y2Qn7BmxtlkqrSZKzlY8rhP+TV77k/E16diwAO7a3//eH/u9\n\t9M4vJVFAuoGoHU9rUheCISQXF4iOjt0m1Pqb7hNQ8C6CvYN+aatNK1czyyWtScFF/h\n\tCKAeG3aNAqOXw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=1KaKfyIjUCSWYA17GgFXdlHONvQ/MMZTObNCSrYS4VE=;\n\tb=JP8YmExXuHQA73DyGzmcRxkWaAxdQwZhZ890K/mTx4uvy/mVLgRWXk6z2tlRqMKl14\n\tUBtvmWnPATePB//hbr61GMmul//JBLw1syYhnrCHktfasFHKiegxWSS5oNU5b2qm51Lj\n\thdsTaZSPVRbcuCPwKVzkLvRb8VLswN7DklhpB9O1WYFSfxtIloSS1qYQR52cOmtJp9nI\n\t8Nndauj9pdI1GV0jivPfanV3yPEiWIMLq15VUg2uqaQuukDAgi3nppmyRocGUMxVMVK0\n\tvxN4zfvu3Ilnii8AQdk/HRokfb/sQ7T3XqNqbTxnNk4TSOt39iQLgVqFfiRzBnQYlnEF\n\tFpRw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"JP8YmExX\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=1KaKfyIjUCSWYA17GgFXdlHONvQ/MMZTObNCSrYS4VE=;\n\tb=gB8VGJlgCcHA4XUpGk21XKEAPTBrPZdqN5meqea/ppxmZEBJgAo2qPb1JX+8SgN8hc\n\t/Fp4XzDqhDx3vsAkNAsIbvvvT8wcr1MvBUOIB6UP6GwBANINI6Vga2QC1uK9JeaRpHbY\n\tNsSlgYL9YXJsrGQ/89nxg7Be1gIQhN3l/jjOaS9v8DSyOn/GZcIhI0j4DrgvASDO9dmP\n\tKIjva8qLiquND/dSOfUl3mFaIF9hLbC62tMKUqip1DG3U6W/cTIEWAjvnlVmQvqSHiF/\n\t1Wtq1Gl4y2bhcUKv82l7iva4cFWSFaGobrKz3lTA4W8cEk0hy91tOeOdQpTqr83haTKc\n\tXpXQ==","X-Gm-Message-State":"ACrzQf1q3zaaPbvi06jZcUu6tgpy28mCNnDWKhnpYTWeCQyz36YfsRhz\n\t8UltgBhMX5BHLJrgKRVduiINAWAkxSudYwSXhYyXAA==","X-Google-Smtp-Source":"AMsMyM7onU/eWftKbPZUi3nULZJCIz+mWHw6yz98xwLSv2oGoXjEKEUfxrPkRb96H3D6jNtTzU5W5mjbfGtNIVvWonw=","X-Received":"by 2002:a19:7709:0:b0:4a4:769c:fccb with SMTP id\n\ts9-20020a197709000000b004a4769cfccbmr5660125lfc.356.1666284964382;\n\tThu, 20 Oct 2022 09:56:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t\n\t<166628167926.2560709.9764677047577550538@Monstersaurus>","In-Reply-To":"<166628167926.2560709.9764677047577550538@Monstersaurus>","Date":"Thu, 20 Oct 2022 17:55:47 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"00000000000065b04f05eb7a3386\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Naushir Patuck via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25519,"web_url":"https://patchwork.libcamera.org/comment/25519/","msgid":"<20221020180020.6jrjje7pedudpabi@uno.localdomain>","date":"2022-10-20T18:00:20","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran, Laurent\n\nOn Thu, Oct 20, 2022 at 05:11:42PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-10-20 11:33:55)\n> > Hi Jacopo,\n> >\n> > On Thu, Oct 20, 2022 at 09:40:39AM +0200, Jacopo Mondi wrote:\n> > > On Wed, Oct 19, 2022 at 11:56:14PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > > > function. This removes the need to fill metadata manually in the IPA\n> > > > module's processStatsBuffer() function.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > > > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > > > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > > > src/ipa/ipu3/ipa_context.h | 1 +\n> > > > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > > > 5 files changed, 30 insertions(+), 13 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > > > index f4e559bf8b84..b5309bdbea25 100644\n> > > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > > @@ -14,6 +14,7 @@\n> > > > #include \n> > > > #include \n> > > >\n> > > > +#include \n> > > > #include \n> > > >\n> > > > #include \"libipa/histogram.h\"\n> > > > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> > > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > > IPAFrameContext &frameContext,\n> > > > const ipu3_uapi_stats_3a *stats,\n> > > > - [[maybe_unused]] ControlList &metadata)\n> > > > + ControlList &metadata)\n> > > > {\n> > > > /*\n> > > > * Estimate the gain needed to have the proportion of pixels in a given\n> > > > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > >\n> > > > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > > > frameCount_++;\n> > > > +\n> > > > + utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > > > + * frameContext.sensor.exposure;\n> > > > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > > + metadata.set(controls::ExposureTime, exposureTime.get());\n> > > > +\n> > > > + /* \\todo Use VBlank value calculated from each frame exposure. */\n> > >\n> > > What block will adjust the frame duration ? AEGC for sure can ends up\n> > > enlarging blankings, what other component will handle control of\n> > > vblank ?\n> >\n> > I think it will be AGC only.\n>\n> Yes, I don't think we would split AEC and AGC. This agc algorithm deals\n> with both auto-exposure, and auto-gain. Perhaps it should be named aegc\n> ... ?\n>\n\nActually I was thinking about handling the of\ncontrols::FrameDurationLimits which I presume should clamp the\nexposure and the total frame length (hence changing the vblank here\nmentioned in the comment).\n\nRPi handles that in the IPA main module for example, that's why I was\nwondering if the comment actually applies. Nothing relevant, I was mostly\nthinking out loud..\n\n>\n> >\n> > > Nothing that interefere with this patch\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > + uint32_t vTotal = context.configuration.sensor.size.height\n> > > > + + context.configuration.sensor.defVBlank;\n> > > > + utils::Duration frameDuration = context.configuration.sensor.lineDuration\n> > > > + * vTotal;\n> > > > + metadata.set(controls::FrameDuration, frameDuration.get());\n> > > > +\n> > > > }\n> > > >\n> > > > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> > > > index 6452b6a1acd2..dd7ebc07c534 100644\n> > > > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > > > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > > > @@ -11,6 +11,8 @@\n> > > >\n> > > > #include \n> > > >\n> > > > +#include \n> > > > +\n> > > > /**\n> > > > * \\file awb.h\n> > > > */\n> > > > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > > > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > > > context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> > > > +\n> > > > + metadata.set(controls::AwbEnable, true);\n> > > > + metadata.set(controls::ColourGains, {\n> > > > + static_cast(context.activeState.awb.gains.red),\n> > > > + static_cast(context.activeState.awb.gains.blue)\n> > > > + });\n> > > > + metadata.set(controls::ColourTemperature,\n> > > > + context.activeState.awb.temperatureK);\n>\n> Thanks,\n>\n> I think this all makes sense for the moment.\n>\n>\n>\n> > > > }\n> > > >\n> > > > constexpr uint16_t Awb::threshold(float value)\n> > > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > > > index 68f017b04751..959f314f5452 100644\n> > > > --- a/src/ipa/ipu3/ipa_context.cpp\n> > > > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > > > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > > > *\n> > > > * \\var IPASessionConfiguration::sensor.defVBlank\n> > > > * \\brief The default vblank value of the sensor\n> > > > + *\n> > > > + * \\var IPASessionConfiguration::sensor.size\n> > > > + * \\brief Sensor output resolution\n> > > > */\n> > > >\n> > > > /**\n> > > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > > index 36099353e9f2..e9a3863b1693 100644\n> > > > --- a/src/ipa/ipu3/ipa_context.h\n> > > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > > > struct {\n> > > > int32_t defVBlank;\n> > > > utils::Duration lineDuration;\n> > > > + Size size;\n> > > > } sensor;\n> > > > };\n> > > >\n> > > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > > index bc0f6007baca..a9a2b49ca95b 100644\n> > > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > > > /* Initialise the sensor configuration. */\n> > > > context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> > > > * 1.0s / sensorInfo_.pixelRate;\n> > > > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> > > >\n> > > > /*\n> > > > * Compute the sensor V4L2 controls to be used by the algorithms and\n> > > > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > > frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > > > frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > > >\n> > > > - double lineDuration = context_.configuration.sensor.lineDuration.get();\n> > > > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > > > ControlList metadata(controls::controls);\n> > > >\n> > > > for (auto const &algo : algorithms())\n> > > > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > >\n> > > > setControls(frame);\n> > > >\n> > > > - /* \\todo Use VBlank value calculated from each frame exposure. */\n> > > > - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;\n> > > > - metadata.set(controls::FrameDuration, frameDuration);\n> > > > -\n> > > > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > > -\n> > > > - metadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n> > > > -\n> > > > - metadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);\n> > > > -\n>\n> I'm half worrying about what would happen if the agc was optional and\n> disabled... But I also think it shouldn't be all too optional right now.\n>\n> Reviewed-by: Kieran Bingham \n>\n> > > > /*\n> > > > * \\todo The Metadata provides a path to getting extended data\n> > > > * out to the application. Further data such as a simplifed Histogram\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","headers":{"Return-Path":"","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 7354CBD16B\n\tfor ;\n\tThu, 20 Oct 2022 18:00:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9307A62EAE;\n\tThu, 20 Oct 2022 20:00:25 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::221])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CFC1604DB\n\tfor ;\n\tThu, 20 Oct 2022 20:00:23 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 6DD1F240003;\n\tThu, 20 Oct 2022 18:00:22 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666288825;\n\tbh=MwBqeEVl3vLYhz59vuyUkzJlIYuVD3Oeyu7R2uNuyLQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=FQ9yqZwvgVBBuOT96EaP4UjKvlZ3XiPvkUZ6zJAOTvzK6CbuL+qcCmQADGdrcXEPF\n\tqszintdZCKJ8gQbr555+lQnzh/HDljShKrKY06TRnU7BdPTe0j14DCoP9PWZwOT78q\n\t6Isq9lNXGyWlubkI5f2gr6Yz/TT1iqEXYdRBH0Q/TFtdKlu8A7pHtj/SBbRhgPKlTq\n\trF1svUfiAhHWZE+fQtzDDtrFdB/0oGysvAPu0/lAvvv2G9aV4DNrwLgouW7fE0t92T\n\t4PwFfpJ67Zb9DOQDqyMXKJgILuSsodrq7WbJJCHI7ryNEBLFxqcXTHu31yYFGng1eE\n\thsgqhg2IAv/lA==","Date":"Thu, 20 Oct 2022 20:00:20 +0200","To":"Kieran Bingham ","Message-ID":"<20221020180020.6jrjje7pedudpabi@uno.localdomain>","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t<20221020074039.kgtomdk3y3xgzdad@uno.localdomain>\n\t\n\t<166628230239.2560709.17471437859147544451@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166628230239.2560709.17471437859147544451@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25520,"web_url":"https://patchwork.libcamera.org/comment/25520/","msgid":"","date":"2022-10-20T21:06:07","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Oct 20, 2022 at 10:25:50AM +0100, Naushir Patuck wrote:\n> Hi Laurent,\n> \n> This change doesn't affect the RPi platform, but I just wanted to give a\n> quick comment.\n\nThat makes me even more thankful for your review :-)\n\n> On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart wrote:\n> \n> > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > function. This removes the need to fill metadata manually in the IPA\n> > module's processStatsBuffer() function.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > src/ipa/ipu3/ipa_context.h | 1 +\n> > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > 5 files changed, 30 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp\n> > b/src/ipa/ipu3/algorithms/agc.cpp\n> > index f4e559bf8b84..b5309bdbea25 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -14,6 +14,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > #include \"libipa/histogram.h\"\n> > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > const ipu3_uapi_stats_3a *stats,\n> > - [[maybe_unused]] ControlList &metadata)\n> > + ControlList &metadata)\n> > {\n> > /*\n> > * Estimate the gain needed to have the proportion of pixels in a given\n> > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> >\n> > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > frameCount_++;\n> > +\n> > + utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > + * frameContext.sensor.exposure;\n> > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > + metadata.set(controls::ExposureTime, exposureTime.get());\n> >\n> \n> Putting the metadata.set() calls into the algorithm source code like above would\n> mean that the metadata is populated with the values the AGC requested. This\n> could be wrong because:\n> \n> 1) If the sensor quantises the gain and/or shutter speed values from what\n> AGC asked for.\n> 2) Unless I am mistaken, this does not account for sensor delays.\n\nThe metadata is filled from the gain and exposure time store in\nframeContext.sensor. These values are different from the ones computed\nby the AGC algorithm, which are stored in frameContext.agc. The former\nis set in IPARkISP1::processStatsBuffer() from the V4L2 control values\npassed by the pipeline handler, which come from the DelayedControls\nclass. I thus believe the code here is correct.\n\n> I am assuming the metadata list here is what is returned back via the completed\n> request, is that correct?\n\nThat's correct.\n\n> Of course, my understanding of this change could be completely\n> wrong, so please ignore the noise :-)\n> \n> > +\n> > + /* \\todo Use VBlank value calculated from each frame exposure. */\n> > + uint32_t vTotal = context.configuration.sensor.size.height\n> > + + context.configuration.sensor.defVBlank;\n> > + utils::Duration frameDuration = context.configuration.sensor.lineDuration\n> > + * vTotal;\n> > + metadata.set(controls::FrameDuration, frameDuration.get());\n> > +\n> > }\n> >\n> > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp\n> > b/src/ipa/ipu3/algorithms/awb.cpp\n> > index 6452b6a1acd2..dd7ebc07c534 100644\n> > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > @@ -11,6 +11,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > /**\n> > * \\file awb.h\n> > */\n> > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > context.activeState.awb.temperatureK = asyncResults_.temperatureK;\n> > +\n> > + metadata.set(controls::AwbEnable, true);\n> > + metadata.set(controls::ColourGains, {\n> > + static_cast(context.activeState.awb.gains.red),\n> > + static_cast(context.activeState.awb.gains.blue)\n> > + });\n> > + metadata.set(controls::ColourTemperature,\n> > + context.activeState.awb.temperatureK);\n> > }\n> >\n> > constexpr uint16_t Awb::threshold(float value)\n> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > index 68f017b04751..959f314f5452 100644\n> > --- a/src/ipa/ipu3/ipa_context.cpp\n> > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > *\n> > * \\var IPASessionConfiguration::sensor.defVBlank\n> > * \\brief The default vblank value of the sensor\n> > + *\n> > + * \\var IPASessionConfiguration::sensor.size\n> > + * \\brief Sensor output resolution\n> > */\n> >\n> > /**\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 36099353e9f2..e9a3863b1693 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > struct {\n> > int32_t defVBlank;\n> > utils::Duration lineDuration;\n> > + Size size;\n> > } sensor;\n> > };\n> >\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index bc0f6007baca..a9a2b49ca95b 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > /* Initialise the sensor configuration. */\n> > context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> > * 1.0s / sensorInfo_.pixelRate;\n> > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> >\n> > /*\n> > * Compute the sensor V4L2 controls to be used by the algorithms and\n> > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> >\n> > - double lineDuration = context_.configuration.sensor.lineDuration.get();\n> > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > ControlList metadata(controls::controls);\n> >\n> > for (auto const &algo : algorithms())\n> > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> >\n> > setControls(frame);\n> >\n> > - /* \\todo Use VBlank value calculated from each frame exposure. */\n> > - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;\n> > - metadata.set(controls::FrameDuration, frameDuration);\n> > -\n> > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > -\n> > - metadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);\n> > -\n> > - metadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);\n> > -\n> > /*\n> > * \\todo The Metadata provides a path to getting extended data\n> > * out to the application. Further data such as a simplifed Histogram","headers":{"Return-Path":"","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 6CFA2C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 21:06:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A68DB62EB7;\n\tThu, 20 Oct 2022 23:06:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F3B0C62EA3\n\tfor ;\n\tThu, 20 Oct 2022 23:06:31 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 722F0D38;\n\tThu, 20 Oct 2022 23:06:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666299993;\n\tbh=+r947kMnrgtSS9hAjoyayEqNEhRg/BfeiMiAKLY8FXY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=aF/Dj63III9cmDGNzqQa3c4zilL/6zSt6hlwfTM8rRSLE8hFq5DWs9WSC+6JXYGHV\n\t3S8V0RCnqfS3qZftvFApKcWj2FDHmGb3UHciwTS36EmSX269zrdw00meSEqE/ptdfx\n\t5KTr6jwunvJbiDz/pHzRDUzq1Dv+r6JRi0oXtB52rI+IbUaRQDyd5yCCBmOZziCFTC\n\tjHGws6h/ofEH856AtBRHueYjNmsNzGxymGoDbjOTC0dBl6Zr6v8AhTrCQzbcjURp8U\n\temIPW93I/PXpkUhcarJ1r5w8IQk/YAmzwwGWXAMQ8QDUngxpuP5o+2SMLPcwyRLgoe\n\tDcNrCN95irSTQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666299991;\n\tbh=+r947kMnrgtSS9hAjoyayEqNEhRg/BfeiMiAKLY8FXY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YXS/u2MA/OGpgWtCmtEg43M9NCBjmsAOqZDZL8j7DMbaeZW6DVm2QUor3fNsksoCe\n\t3puDiifzIDAV4Z8a3YVNmVeBNrStWrpi1tOixRM7Z2IyvHJ4MIpq9gtFlsHI+4FLlD\n\tHqXf52SOoXQC+TUnHSAxzF0dmEzW3+1sHaAXFVHg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YXS/u2MA\"; dkim-atps=neutral","Date":"Fri, 21 Oct 2022 00:06:07 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25522,"web_url":"https://patchwork.libcamera.org/comment/25522/","msgid":"","date":"2022-10-21T07:32:57","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 20 Oct 2022 at 22:06, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Thu, Oct 20, 2022 at 10:25:50AM +0100, Naushir Patuck wrote:\n> > Hi Laurent,\n> >\n> > This change doesn't affect the RPi platform, but I just wanted to give a\n> > quick comment.\n>\n> That makes me even more thankful for your review :-)\n>\n> > On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart wrote:\n> >\n> > > Fill the frame metadata in the AGC and AWB algorithm's prepare()\n> > > function. This removes the need to fill metadata manually in the IPA\n> > > module's processStatsBuffer() function.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-\n> > > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++\n> > > src/ipa/ipu3/ipa_context.cpp | 3 +++\n> > > src/ipa/ipu3/ipa_context.h | 1 +\n> > > src/ipa/ipu3/ipu3.cpp | 13 +------------\n> > > 5 files changed, 30 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp\n> > > b/src/ipa/ipu3/algorithms/agc.cpp\n> > > index f4e559bf8b84..b5309bdbea25 100644\n> > > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > > @@ -14,6 +14,7 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \"libipa/histogram.h\"\n> > > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState\n> &activeState,\n> > > void Agc::process(IPAContext &context, [[maybe_unused]] const\n> uint32_t frame,\n> > > IPAFrameContext &frameContext,\n> > > const ipu3_uapi_stats_3a *stats,\n> > > - [[maybe_unused]] ControlList &metadata)\n> > > + ControlList &metadata)\n> > > {\n> > > /*\n> > > * Estimate the gain needed to have the proportion of pixels\n> in a given\n> > > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> > >\n> > > computeExposure(context, frameContext, yGain, iqMeanGain);\n> > > frameCount_++;\n> > > +\n> > > + utils::Duration exposureTime =\n> context.configuration.sensor.lineDuration\n> > > + * frameContext.sensor.exposure;\n> > > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > + metadata.set(controls::ExposureTime,\n> exposureTime.get());\n> > >\n> >\n> > Putting the metadata.set() calls into the algorithm source code like\n> above would\n> > mean that the metadata is populated with the values the AGC requested.\n> This\n> > could be wrong because:\n> >\n> > 1) If the sensor quantises the gain and/or shutter speed values from what\n> > AGC asked for.\n> > 2) Unless I am mistaken, this does not account for sensor delays.\n>\n> The metadata is filled from the gain and exposure time store in\n> frameContext.sensor. These values are different from the ones computed\n> by the AGC algorithm, which are stored in frameContext.agc. The former\n> is set in IPARkISP1::processStatsBuffer() from the V4L2 control values\n> passed by the pipeline handler, which come from the DelayedControls\n> class. I thus believe the code here is correct.\n>\n> > I am assuming the metadata list here is what is returned back via the\n> completed\n> > request, is that correct?\n>\n> That's correct.\n>\n\nThanks for the clarification Laurent. So my understanding was indeed\nincorrect :-)\n\n\n>\n> > Of course, my understanding of this change could be completely\n> > wrong, so please ignore the noise :-)\n> >\n> > > +\n> > > + /* \\todo Use VBlank value calculated from each frame exposure.\n> */\n> > > + uint32_t vTotal = context.configuration.sensor.size.height\n> > > + + context.configuration.sensor.defVBlank;\n> > > + utils::Duration frameDuration =\n> context.configuration.sensor.lineDuration\n> > > + * vTotal;\n> > > + metadata.set(controls::FrameDuration,\n> frameDuration.get());\n> > > +\n> > > }\n> > >\n> > > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp\n> > > b/src/ipa/ipu3/algorithms/awb.cpp\n> > > index 6452b6a1acd2..dd7ebc07c534 100644\n> > > --- a/src/ipa/ipu3/algorithms/awb.cpp\n> > > +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> > > @@ -11,6 +11,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > /**\n> > > * \\file awb.h\n> > > */\n> > > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context,\n> [[maybe_unused]] const uint32_t frame,\n> > > context.activeState.awb.gains.green = asyncResults_.greenGain;\n> > > context.activeState.awb.gains.red = asyncResults_.redGain;\n> > > context.activeState.awb.temperatureK =\n> asyncResults_.temperatureK;\n> > > +\n> > > + metadata.set(controls::AwbEnable, true);\n> > > + metadata.set(controls::ColourGains, {\n> > > +\n> static_cast(context.activeState.awb.gains.red),\n> > > +\n> static_cast(context.activeState.awb.gains.blue)\n> > > + });\n> > > + metadata.set(controls::ColourTemperature,\n> > > + context.activeState.awb.temperatureK);\n> > > }\n> > >\n> > > constexpr uint16_t Awb::threshold(float value)\n> > > diff --git a/src/ipa/ipu3/ipa_context.cpp\n> b/src/ipa/ipu3/ipa_context.cpp\n> > > index 68f017b04751..959f314f5452 100644\n> > > --- a/src/ipa/ipu3/ipa_context.cpp\n> > > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {\n> > > *\n> > > * \\var IPASessionConfiguration::sensor.defVBlank\n> > > * \\brief The default vblank value of the sensor\n> > > + *\n> > > + * \\var IPASessionConfiguration::sensor.size\n> > > + * \\brief Sensor output resolution\n> > > */\n> > >\n> > > /**\n> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > index 36099353e9f2..e9a3863b1693 100644\n> > > --- a/src/ipa/ipu3/ipa_context.h\n> > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {\n> > > struct {\n> > > int32_t defVBlank;\n> > > utils::Duration lineDuration;\n> > > + Size size;\n> > > } sensor;\n> > > };\n> > >\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index bc0f6007baca..a9a2b49ca95b 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo\n> &configInfo,\n> > > /* Initialise the sensor configuration. */\n> > > context_.configuration.sensor.lineDuration =\n> sensorInfo_.minLineLength\n> > > * 1.0s /\n> sensorInfo_.pixelRate;\n> > > + context_.configuration.sensor.size = sensorInfo_.outputSize;\n> > >\n> > > /*\n> > > * Compute the sensor V4L2 controls to be used by the\n> algorithms and\n> > > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t\n> frame,\n> > > frameContext.sensor.exposure =\n> sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > > frameContext.sensor.gain =\n> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > >\n> > > - double lineDuration =\n> context_.configuration.sensor.lineDuration.get();\n> > > - int32_t vBlank = context_.configuration.sensor.defVBlank;\n> > > ControlList metadata(controls::controls);\n> > >\n> > > for (auto const &algo : algorithms())\n> > > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t\n> frame,\n> > >\n> > > setControls(frame);\n> > >\n> > > - /* \\todo Use VBlank value calculated from each frame exposure.\n> */\n> > > - int64_t frameDuration = (vBlank +\n> sensorInfo_.outputSize.height) * lineDuration;\n> > > - metadata.set(controls::FrameDuration, frameDuration);\n> > > -\n> > > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> > > -\n> > > - metadata.set(controls::ColourTemperature,\n> context_.activeState.awb.temperatureK);\n> > > -\n> > > - metadata.set(controls::ExposureTime,\n> frameContext.sensor.exposure * lineDuration);\n> > > -\n> > > /*\n> > > * \\todo The Metadata provides a path to getting extended data\n> > > * out to the application. Further data such as a simplifed\n> Histogram\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 9FD54C0DA4\n\tfor ;\n\tFri, 21 Oct 2022 07:33:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF88762EBC;\n\tFri, 21 Oct 2022 09:33:14 +0200 (CEST)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3C2F604E8\n\tfor ;\n\tFri, 21 Oct 2022 09:33:13 +0200 (CEST)","by mail-lf1-x135.google.com with SMTP id f37so3612369lfv.8\n\tfor ;\n\tFri, 21 Oct 2022 00:33:13 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666337595;\n\tbh=bkaYQHNeoVBaDjb6/z2Z66kR6E4baqCxPxRQ44VTJrc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=a+PHIQiZECBEtvF7lOGJrHbxCS2mhc9RRrPL+rOyMPW09k16Ur1xx8DzceoXG/nBJ\n\ti8siFACyfheQf+m0naykP/dkDUxEuoipel+K04s4PAtc4LDfdufhyr0iE6WnZS7oe2\n\tRNfz/R5TGdew4Q5tjXGbvbYazGEPmuMNMRvciP/dcQ6VqDC2/ORmcM3CuMWmtiOkBs\n\ta/e4Vt2sZY8Pf7MuJwocR1ru3XxddLFwO5pX3UP9lj3sN295FFI6QHlfO9HCGlwI3J\n\t05qMue8VuwBy8OfCUxPGGvIZvw+9H12dRkjXMJJkBEgMiw5buTLkOyy9k9N0VNieY8\n\tOo67374JA44AQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=LQxwTBhNcEcdee8ex+CC82f7jL5jyyc0hY5dbth0yfw=;\n\tb=tI4yJTAPke7zl0vfAU7a5sTv/L10K5QYIJswyzQ+sazfVYJWKDPZ1jUxFH/sBivdyt\n\tpDJFBWBkMYkPFKZYQuatSiGQsJRvjtd/wjx47npt0W4HlpJ+XaxJGBBgwCtpk12Zizty\n\tSOMElOf0k0vXCJ4eur+AnU5jKwybdsp5tsNfgnDc6ZskknkDzO8vMX0X4bZLPIB91R5F\n\tJJym3wY8r7WoBWwTjeVgGJRlZJtwq1DMP/s76VT6mX4EjLCcOcuHyxLX0IJRc0e/M8MG\n\tck1fAl/Odk+aF+a1ZrkqWj6QOA/By2DQzdnUwotOz28RrDTUI4l+xom7ehQIZcfYkiom\n\tse4g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"tI4yJTAP\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=LQxwTBhNcEcdee8ex+CC82f7jL5jyyc0hY5dbth0yfw=;\n\tb=o+tZLTDIXJbOEhmNIUWDZhZ8RviGUZqDMBeBh86FPst70E3gF1EnMC+XtUQHM8FQbJ\n\tDE+OsKBq+dwMOe5Q38Hgaj8jtvuKvI/+KBGKcAHr5YhjXSwSM8zb/Bnpod8gI7tQjcux\n\t//Kja6fY027b5MV/rYginxC4cpKOfmLYaFXbNdkmG12HonUXwoMA5WalmIBm5plfVJUt\n\t2B0qgHG0Xxk4P4KOM3iISK3xyP0wuOcI1517BvLBgpd732qZQhSnb/hE95UDhqo9C309\n\tE0TpoNYRS3tgE2grFrsdgHttjvK/HJY4EmYWyFWHEP/MV2rQFMCpe9c6l8hYgLBxmbrd\n\tWRiA==","X-Gm-Message-State":"ACrzQf0YT7AWcl6RIdL5+T3jcvQdjbpIZLD1PAPu0647DPsvSFqdLGzC\n\tL9KStVjTSpdQnTJ1bthsmzJLiLXxVrrMCMJ/vd/NuceWOCI=","X-Google-Smtp-Source":"AMsMyM7EQweum91/mx7WmoO0oS/DAjKhGT647DYraUIfdNHsPdT3+EurVisVwdl0Ui8mBPcbTjC6BxYZVmpd3WE6mnI=","X-Received":"by 2002:ac2:4c03:0:b0:4a2:2273:89c6 with SMTP id\n\tt3-20020ac24c03000000b004a2227389c6mr5692198lfq.245.1666337592860;\n\tFri, 21 Oct 2022 00:33:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20221019205614.25751-1-laurent.pinchart@ideasonboard.com>\n\t<20221019205614.25751-4-laurent.pinchart@ideasonboard.com>\n\t\n\t","In-Reply-To":"","Date":"Fri, 21 Oct 2022 08:32:57 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000004c7ab905eb86749a\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB\n\tmetadata in algorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]