[{"id":20731,"web_url":"https://patchwork.libcamera.org/comment/20731/","msgid":"<163638957215.275423.2446536828402243202@Monstersaurus>","date":"2021-11-08T16:39:32","subject":"Re: [libcamera-devel] [PATCH 18/22] ipa: ipu3: Move the sensor\n\tupdate controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-08 13:13:46)\n> We want the setControls() to use the cached exposure_ and gain_ values,\n> so move the frame context update of those variables into\n> frameCompleted() call instead.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/ipu3.cpp | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index b60ab7e7..9b20e8ab 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -534,6 +534,10 @@ void IPAIPU3::frameStarted([[maybe_unused]] unsigned int frame)\n> \n> void IPAIPU3::frameCompleted([[maybe_unused]] unsigned int frame)\n> {\n> + /* Apply the exposure and gain updated values */\n> + exposure_ = context_.frameContext->agc.exposure;\n> + gain_ = camHelper_->gainCode(context_.frameContext->agc.gain);\n> +\n\nFollowing this through I fear we might have to split these values more.\n\nWe need to store Exposure and Gain (EG from here) in several places, and\na couple of different points when it's calculated.\n\n\nFor frame X\n\n- processControls:\n\tEG 'might' be set as manual requested values here\n\tfrom the request. Otherwise they have no use afaict.\n\tIf they are set in here, then they should be stored in the\n\tIPU3IPA state to be used when calling FillParams (next)\n\n- FillParams:\n\tEG needs to be the 'desired next EG values for this frame'\n\tThese values were calculated on the previous iteration of\n\tprocessStatistics if it has happened or some sane initial\n\tdefaults if we are just starting up from frame 0.\n\n- processStatistics:\n\tThe metadata needs to retrieve the EG that was used (from\n\tdelayedControls or sensor metadata?) , and make\n\tsure those values are set in the outgoing request metadata.\n\n\tThe algorithms need to compare the EG that was used for this\n\tframe against the statistics of the frame.\n\tThen ... the algorithms will generate 'new' EG values. (EG++)\n\n\tAt this point, I think EG++ needs to be stored (not in the\n\tFrameContext) probably in the IPU3IPA state or such ready to be\n\tset in the next FillParams.\n\nIs my understanding of the sequences correct above?\n\n\nLooking at IPAIPU3::parseStatistics(), I can see a call to\nsetControls() after the algorithms run, so perhaps there isn't any\nwaiting around for fillParams to occur, and they get set quickly which\nthrows my understanding of the sequencing out a bit.\n\n> /*\n> * Remove the pointer from the queue, it should not be accessed\n> * anymore and delete it.\n> @@ -711,10 +715,6 @@ void IPAIPU3::setControls(unsigned int frame)\n> IPU3Action op;\n> op.op = ActionSetSensorControls;\n> \n> - /* Apply the exposure and gain updated values */\n> - exposure_ = context_.frameContext->agc.exposure;\n> - gain_ = camHelper_->gainCode(context_.frameContext->agc.gain);\n> -\n> ControlList ctrls(ctrls_);\n> ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_));\n> -- \n> 2.32.0\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 8A184BF415\n\tfor ;\n\tMon, 8 Nov 2021 16:39:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7CF96035D;\n\tMon, 8 Nov 2021 17:39:35 +0100 (CET)","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 367C06032C\n\tfor ;\n\tMon, 8 Nov 2021 17:39:35 +0100 (CET)","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 D4FC98BB;\n\tMon, 8 Nov 2021 17:39:34 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"D1Wfyfw3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636389574;\n\tbh=8FnPhlC4PVuVoJ9lN2PJysWhwBdsgOjDw0yPObGR9yc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=D1Wfyfw3xcU5fgxi8fBsgmm5k0mV0cNB73WItSW+P85FsYN1su3EZb9R/AaXiTdN8\n\tuCY1iIFq1KM8eSljYmRt/ZJi0xk6JgSVH3dTDgiK+Pvvkp5dGWUGir6GyxkDUmeb1Z\n\tJoz2zmHl/C9N/r5q9OibCotWLU84QIzaiL4Ro90Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211108131350.130665-19-jeanmichel.hautbois@ideasonboard.com>","References":"<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211108131350.130665-19-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 08 Nov 2021 16:39:32 +0000","Message-ID":"<163638957215.275423.2446536828402243202@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 18/22] ipa: ipu3: Move the sensor\n\tupdate controls","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]