[{"id":25890,"web_url":"https://patchwork.libcamera.org/comment/25890/","msgid":"<20221124122504.qm47onphifv4aq3x@uno.localdomain>","date":"2022-11-24T12:25:04","subject":"Re: [libcamera-devel] [PATCH v4 5/9] ipa: rkisp1: agc: Support raw\n\tcapture","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Nov 24, 2022 at 04:51:29AM +0200, Laurent Pinchart via libcamera-devel wrote:\n> Support raw capture by allowing manual control of the exposure time and\n> analogue gain.\n>\n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Paul Elder \n\nLGTM\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> Changes since v3:\n>\n> - Drop unneeded params null check\n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 49 ++++++++++++++++++++-----------\n> src/ipa/rkisp1/algorithms/agc.h | 2 ++\n> 2 files changed, 34 insertions(+), 17 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index ccbcd4a9583d..6bf92743f4cf 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -62,6 +62,7 @@ static constexpr double kRelativeLuminanceTarget = 0.4;\n> Agc::Agc()\n> \t: frameCount_(0), numCells_(0), numHistBins_(0), filteredExposure_(0s)\n> {\n> +\tsupportsRaw_ = true;\n> }\n>\n> /**\n> @@ -81,7 +82,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> \t\t10ms / context.configuration.sensor.lineDuration;\n> \tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n> \tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n> -\tcontext.activeState.agc.autoEnabled = true;\n> +\tcontext.activeState.agc.autoEnabled = !context.configuration.raw;\n>\n> \t/*\n> \t * According to the RkISP1 documentation:\n> @@ -123,12 +124,15 @@ void Agc::queueRequest(IPAContext &context,\n> {\n> \tauto &agc = context.activeState.agc;\n>\n> -\tconst auto &agcEnable = controls.get(controls::AeEnable);\n> -\tif (agcEnable && *agcEnable != agc.autoEnabled) {\n> -\t\tagc.autoEnabled = *agcEnable;\n> +\tif (!context.configuration.raw) {\n> +\t\tconst auto &agcEnable = controls.get(controls::AeEnable);\n> +\t\tif (agcEnable && *agcEnable != agc.autoEnabled) {\n> +\t\t\tagc.autoEnabled = *agcEnable;\n>\n> -\t\tLOG(RkISP1Agc, Debug)\n> -\t\t\t<< (agc.autoEnabled ? \"Enabling\" : \"Disabling\") << \" AGC\";\n> +\t\t\tLOG(RkISP1Agc, Debug)\n> +\t\t\t\t<< (agc.autoEnabled ? \"Enabling\" : \"Disabling\")\n> +\t\t\t\t<< \" AGC\";\n> +\t\t}\n> \t}\n>\n> \tconst auto &exposure = controls.get(controls::ExposureTime);\n> @@ -368,6 +372,22 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n> \treturn histogram.interQuantileMean(0.98, 1.0);\n> }\n>\n> +void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> +\t\t ControlList &metadata)\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> +\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> * \\brief Process RkISP1 statistics, and run AGC operations\n> * \\param[in] context The shared IPA context\n> @@ -383,6 +403,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\t IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,\n> \t\t ControlList &metadata)\n> {\n> +\tif (!stats) {\n> +\t\tfillMetadata(context, frameContext, metadata);\n> +\t\treturn;\n> +\t}\n> +\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -425,17 +450,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\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> -\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> +\tfillMetadata(context, frameContext, metadata);\n> }\n>\n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index a228d0c37768..8a22263741b6 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -44,6 +44,8 @@ private:\n> \tutils::Duration filterExposure(utils::Duration exposureValue);\n> \tdouble estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);\n> \tdouble measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;\n> +\tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> +\t\t\t ControlList &metadata);\n>\n> \tuint64_t frameCount_;\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 C7C43BDE6B\n\tfor ;\n\tThu, 24 Nov 2022 12:25:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 31FE76331A;\n\tThu, 24 Nov 2022 13:25:07 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::226])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 073BE632EA\n\tfor ;\n\tThu, 24 Nov 2022 13:25:06 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 78E9AC0010;\n\tThu, 24 Nov 2022 12:25:05 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669292707;\n\tbh=FZQtuUq76Drt8x8e62rwRkgbW9Hrw3j2y39qkYmf9SY=;\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=UMGMqtse0ReiOYB6gS2tn1LAg0v4ZCDKizM/qQEli7djJM0KBvZhx90dTf/h+hImY\n\tvmYemNzAM+dmKFoM7u4sJqvJvA6rdDxmZq643khqeqPeGy8aC5XLOgs1kl3ceMPK2X\n\t1iPg5FYMwadx3gQmKzDCEaOYGCNyxWrylnQQftHFfoeTBs5oNwYlAKFud/1HhM8GMD\n\tNzRxvLeFn/GfFGz+rHSTK2k44VsRYWpJL4UFEzSIneTMIwisL/Z0Kk+1fRoXTm11gS\n\t+oLKwCCxeytTDDXVf28VGqlotCPJi7Zp2N918jEt9GpFAmNgDYbKGvWuQAS21KXIAs\n\td+qvM6898vj1g==","Date":"Thu, 24 Nov 2022 13:25:04 +0100","To":"Laurent Pinchart ","Message-ID":"<20221124122504.qm47onphifv4aq3x@uno.localdomain>","References":"<20221124025133.17875-1-laurent.pinchart@ideasonboard.com>\n\t<20221124025133.17875-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221124025133.17875-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 5/9] ipa: rkisp1: agc: Support raw\n\tcapture","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\" "}}]