[{"id":24601,"web_url":"https://patchwork.libcamera.org/comment/24601/","msgid":"<20220816053055.GV311202@pyrite.rasen.tech>","date":"2022-08-16T05:30:55","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipa: raspberry: Port to the\n\tnew AEGC controls","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Jacopo,\n\nOn Thu, Aug 11, 2022 at 05:02:17PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> The newly introduced controls to driver the AEGC algorithm allow\n\ns/driver/drive/\n\n> to control the computation of the exposure time and analogue gain\n\ns/to control/controlling/\n\n> separately.\n> \n> The RPi AEGC implementation already computes the shutter and gain\n> values separately but does not expose separate functions to control\n> them.\n> \n> Augment the AgcAlgorithm interface to allow to pause/resume the shutter\n\ns#to pause/resume#pausing/resuming#\n\n> and gain automatic computations separately and plumb them to the newly\n> introduced controls.\n> \n> Add safety checks to ignore ExposureTime and AnalogueGain values if the\n> algorithms are not paused, and report the correct AeState value by\n> checking if both algorithms have been paused or if they have converged.\n> \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Paul Elder \n\n> ---\n> .../raspberrypi/controller/agc_algorithm.h | 6 ++\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 24 +++++-\n> src/ipa/raspberrypi/controller/rpi/agc.h | 8 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 76 ++++++++++++++++---\n> 4 files changed, 99 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> index 3a91444c3a61..bf9c501db553 100644\n> --- a/src/ipa/raspberrypi/controller/agc_algorithm.h\n> +++ b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> @@ -17,6 +17,12 @@ class AgcAlgorithm : public Algorithm\n> public:\n> \tAgcAlgorithm(Controller *controller) : Algorithm(controller) {}\n> \t/* An AGC algorithm must provide the following: */\n> +\tvirtual void pauseShutter();\n> +\tvirtual void resumeShutter();\n> +\tvirtual bool isShutterPaused() const;\n> +\tvirtual void pauseGain();\n> +\tvirtual void resumeGain();\n> +\tvirtual bool isGainPaused() const;\n> \tvirtual unsigned int getConvergenceFrames() const = 0;\n> \tvirtual void setEv(double ev) = 0;\n> \tvirtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index bd54a639d637..3d04724349f7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -275,18 +275,36 @@ bool Agc::isPaused() const\n> \treturn false;\n> }\n> \n> -void Agc::pause()\n> +void Agc::pauseShutter()\n> {\n> \tfixedShutter_ = status_.shutterTime;\n> -\tfixedAnalogueGain_ = status_.analogueGain;\n> }\n> \n> -void Agc::resume()\n> +void Agc::resumeShutter()\n> {\n> \tfixedShutter_ = 0s;\n> +}\n> +\n> +bool Agc::isShutterPaused() const\n> +{\n> +\treturn fixedShutter_ != 0s;\n> +}\n> +\n> +void Agc::pauseGain()\n> +{\n> +\tfixedAnalogueGain_ = status_.analogueGain;\n> +}\n> +\n> +void Agc::resumeGain()\n> +{\n> \tfixedAnalogueGain_ = 0;\n> }\n> \n> +bool Agc::isGainPaused() const\n> +{\n> +\treturn fixedAnalogueGain_ != 0;\n> +}\n> +\n> unsigned int Agc::getConvergenceFrames() const\n> {\n> \t/*\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\n> index 6d6b0e5ad857..cfb57f41848b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> @@ -77,8 +77,12 @@ public:\n> \tint read(const libcamera::YamlObject ¶ms) override;\n> \t/* AGC handles \"pausing\" for itself. */\n> \tbool isPaused() const override;\n> -\tvoid pause() override;\n> -\tvoid resume() override;\n> +\tvoid pauseShutter() override;\n> +\tvoid resumeShutter() override;\n> +\tbool isShutterPaused() const override;\n> +\tvoid pauseGain() override;\n> +\tvoid resumeGain() override;\n> +\tbool isGainPaused() const override;\n> \tunsigned int getConvergenceFrames() const override;\n> \tvoid setEv(double ev) override;\n> \tvoid setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 69c73f8c780a..c041aac008eb 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -74,8 +74,13 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;\n> \n> /* List of controls handled by the Raspberry Pi IPA */\n> static const ControlInfoMap::Map ipaControls{\n> -\t{ &controls::AeEnable, ControlInfo(false, true) },\n> +\t{ &controls::ExposureTimeMode,\n> +\t ControlInfo(static_cast(controls::ExposureTimeModeAuto),\n> +\t\t static_cast(controls::ExposureTimeModeManual)) },\n> \t{ &controls::ExposureTime, ControlInfo(0, 66666) },\n> +\t{ &controls::AnalogueGainMode,\n> +\t ControlInfo(static_cast(controls::AnalogueGainModeAuto),\n> +\t\t static_cast(controls::AnalogueGainModeManual)) },\n> \t{ &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },\n> \t{ &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },\n> \t{ &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },\n> @@ -556,9 +561,18 @@ void IPARPi::reportMetadata()\n> \t}\n> \n> \tAgcStatus *agcStatus = rpiMetadata_.getLocked(\"agc.status\");\n> -\tif (agcStatus) {\n> -\t\tlibcameraMetadata_.set(controls::AeLocked, agcStatus->locked);\n> +\tif (agcStatus)\n> \t\tlibcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);\n> +\n> +\tRPiController::AgcAlgorithm *agc = dynamic_cast(\n> +\t\tcontroller_.getAlgorithm(\"agc\"));\n> +\tif (agc) {\n> +\t\tif (agc->isShutterPaused() && agc->isGainPaused())\n> +\t\t\tlibcameraMetadata_.set(controls::AeState, controls::AeStateIdle);\n> +\t\telse if (agcStatus)\n> +\t\t\tlibcameraMetadata_.set(controls::AeState,\n> +\t\t\t\t\t agcStatus->locked ? controls::AeStateConverged\n> +\t\t\t\t\t\t\t\t : controls::AeStateSearching);\n> \t}\n> \n> \tLuxStatus *luxStatus = rpiMetadata_.getLocked(\"lux.status\");\n> @@ -703,20 +717,22 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\t\t << \" = \" << ctrl.second.toString();\n> \n> \t\tswitch (ctrl.first) {\n> -\t\tcase controls::AE_ENABLE: {\n> -\t\t\tRPiController::Algorithm *agc = controller_.getAlgorithm(\"agc\");\n> +\t\tcase controls::EXPOSURE_TIME_MODE: {\n> +\t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast(\n> +\t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n> \t\t\tif (!agc) {\n> \t\t\t\tLOG(IPARPI, Warning)\n> -\t\t\t\t\t<< \"Could not set AE_ENABLE - no AGC algorithm\";\n> +\t\t\t\t\t<< \"Could not set EXPOSURE_TIME_MODE - no AGC algorithm\";\n> \t\t\t\tbreak;\n> \t\t\t}\n> \n> -\t\t\tif (ctrl.second.get() == false)\n> -\t\t\t\tagc->pause();\n> +\t\t\tif (ctrl.second.get() == controls::ExposureTimeModeManual)\n> +\t\t\t\tagc->pauseShutter();\n> \t\t\telse\n> -\t\t\t\tagc->resume();\n> +\t\t\t\tagc->resumeShutter();\n> \n> -\t\t\tlibcameraMetadata_.set(controls::AeEnable, ctrl.second.get());\n> +\t\t\tlibcameraMetadata_.set(controls::ExposureTimeMode,\n> +\t\t\t\t\t ctrl.second.get());\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -729,6 +745,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\t\tbreak;\n> \t\t\t}\n> \n> +\t\t\t/*\n> +\t\t\t * Ignore manual exposure time when the auto exposure\n> +\t\t\t * algorithm is running.\n> +\t\t\t */\n> +\t\t\tif (!agc->isShutterPaused())\n> +\t\t\t\tbreak;\n> +\n> \t\t\t/* The control provides units of microseconds. */\n> \t\t\tagc->setFixedShutter(ctrl.second.get() * 1.0us);\n> \n> @@ -736,6 +759,25 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\tbreak;\n> \t\t}\n> \n> +\t\tcase controls::ANALOGUE_GAIN_MODE: {\n> +\t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast(\n> +\t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n> +\t\t\tif (!agc) {\n> +\t\t\t\tLOG(IPARPI, Warning)\n> +\t\t\t\t\t<< \"Could not set ANALOGUE_GAIN_MODE - no AGC algorithm\";\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\tif (ctrl.second.get() == controls::AnalogueGainModeManual)\n> +\t\t\t\tagc->pauseGain();\n> +\t\t\telse\n> +\t\t\t\tagc->resumeGain();\n> +\n> +\t\t\tlibcameraMetadata_.set(controls::AnalogueGainMode,\n> +\t\t\t\t\t ctrl.second.get());\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> \t\tcase controls::ANALOGUE_GAIN: {\n> \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast(\n> \t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n> @@ -745,6 +787,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\t\tbreak;\n> \t\t\t}\n> \n> +\t\t\t/*\n> +\t\t\t * Ignore manual analogue gain value when the auto gain\n> +\t\t\t * algorithm is running.\n> +\t\t\t */\n> +\t\t\tif (!agc->isGainPaused())\n> +\t\t\t\tbreak;\n> +\n> \t\t\tagc->setFixedAnalogueGain(ctrl.second.get());\n> \n> \t\t\tlibcameraMetadata_.set(controls::AnalogueGain,\n> @@ -801,6 +850,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\t\tbreak;\n> \t\t\t}\n> \n> +\t\t\t/*\n> +\t\t\t * Ignore AE_EXPOSURE_MODE if the shutter or the gain\n> +\t\t\t * are in auto mode.\n> +\t\t\t */\n> +\t\t\tif (!agc->isShutterPaused() || !agc->isGainPaused())\n> +\t\t\t\tbreak;\n> +\n> \t\t\tint32_t idx = ctrl.second.get();\n> \t\t\tif (ExposureModeTable.count(idx)) {\n> \t\t\t\tagc->setExposureMode(ExposureModeTable.at(idx));\n> -- \n> 2.37.1\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 7B188C3272\n\tfor ;\n\tTue, 16 Aug 2022 05:31:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B84D461FC0;\n\tTue, 16 Aug 2022 07:31:03 +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 3EF84603E3\n\tfor ;\n\tTue, 16 Aug 2022 07:31:02 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C934C496;\n\tTue, 16 Aug 2022 07:31:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660627863;\n\tbh=58fH6oLHbkyEvQ85bqkB4j8vclDCC5kJpjfJQddaGJw=;\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=hYPntiuAuSsFUEjNHb2lDFQgBHNCuRpeJ1aWJRqko5UWXsBYC7yBW0cw8tYeWIlaX\n\tc2umk9bO6FdRhr5D2p7i+xozb2Lbi2Ti2c+oYNyYJ/neipnigiN6o7OGxsRZVZfs7V\n\tFcIylTQ93g0XP21DXtfM0Aqkook3M1X7REwl8bYavbSSvNjoGjlM+Y3MXMru4YYDc3\n\ty/GIkoOjcVq94rYbdhQLvG1iDzOxk4zNClNrRu3te17HFucPYV3p7K44Q8mWiL0Gy8\n\thtp2PAefd0NzmAXwB0Mu9Y8NnuKqv+prz13oZn34SUAwQlo6k44HtxPRp3Ga+rQ+Bo\n\twsWyPLS+yUFgg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660627861;\n\tbh=58fH6oLHbkyEvQ85bqkB4j8vclDCC5kJpjfJQddaGJw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PeJ9bEjZ+slHdp12dm0xLAKCJqApRLaQjn4cBsDDGXj6HSoO8YfqcZqGhNbIRFndx\n\ttlg4IdiDJXRzRQRyM08KaTLiNFjMOurhyifu1ydz0op5vyqMOsSgXoUFdwnv0lp478\n\tyeJflS31+dioXh5Su28xZmyKg+WkcqvMvCkOREiE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PeJ9bEjZ\"; dkim-atps=neutral","Date":"Tue, 16 Aug 2022 14:30:55 +0900","To":"Jacopo Mondi ","Message-ID":"<20220816053055.GV311202@pyrite.rasen.tech>","References":"<20220811150219.62066-1-jacopo@jmondi.org>\n\t<20220811150219.62066-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220811150219.62066-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipa: raspberry: Port to the\n\tnew AEGC 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","From":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24650,"web_url":"https://patchwork.libcamera.org/comment/24650/","msgid":"","date":"2022-08-18T14:25:35","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipa: raspberry: Port to the\n\tnew AEGC controls","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for doing this work! I think mostly this seemed reasonable to\nme, there were just a few things I wanted to clarify about the desired\nbehaviour - it seems such a long time ago that we first discussed\nthis.\n\n1. How to turn AEGC on/off\n\nAs I understand it now, you have to pause both the \"shutter\" and\n\"gain\" explicitly. I think we talked about having some way to \"turn\neverything on/off\", given that an application might not know exactly\nhow many different things there are that it has to worry about.\n\nHas there been any further thought about this? Actually I'm less\ninterested in the C++ API these days, so maybe it's up to me to make\nthis work nicely in Python-land.\n\n2 Setting manual values\n\nMy reading of this is now that manual values are ignored unless you're\nin the \"manual\" mode. I guess this is OK so long as you can send\n\"manual mode\" along with the \"manual value\".\n\nActually I wasn't quite sure if the Raspberry Pi implementation would\nbehave correctly in this respect, there's maybe an issue related to\nprocessing controls in the correct order. So perhaps something to look\nat there.\n\nHere too I wonder about doing something to make the interface a bit\neasier for our Python users, but that sounds like my problem.\n\nI think that was pretty much everything that came to mind, so thanks\nfor doing this work!\n\nBest regards\nDavid\n\nOn Tue, 16 Aug 2022 at 06:31, Paul Elder via libcamera-devel\n wrote:\n>\n> Hi Jacopo,\n>\n> On Thu, Aug 11, 2022 at 05:02:17PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > The newly introduced controls to driver the AEGC algorithm allow\n>\n> s/driver/drive/\n>\n> > to control the computation of the exposure time and analogue gain\n>\n> s/to control/controlling/\n>\n> > separately.\n> >\n> > The RPi AEGC implementation already computes the shutter and gain\n> > values separately but does not expose separate functions to control\n> > them.\n> >\n> > Augment the AgcAlgorithm interface to allow to pause/resume the shutter\n>\n> s#to pause/resume#pausing/resuming#\n>\n> > and gain automatic computations separately and plumb them to the newly\n> > introduced controls.\n> >\n> > Add safety checks to ignore ExposureTime and AnalogueGain values if the\n> > algorithms are not paused, and report the correct AeState value by\n> > checking if both algorithms have been paused or if they have converged.\n> >\n> > Signed-off-by: Jacopo Mondi \n>\n> Reviewed-by: Paul Elder \n>\n> > ---\n> > .../raspberrypi/controller/agc_algorithm.h | 6 ++\n> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 24 +++++-\n> > src/ipa/raspberrypi/controller/rpi/agc.h | 8 +-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 76 ++++++++++++++++---\n> > 4 files changed, 99 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > index 3a91444c3a61..bf9c501db553 100644\n> > --- a/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > +++ b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > @@ -17,6 +17,12 @@ class AgcAlgorithm : public Algorithm\n> > public:\n> > AgcAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > /* An AGC algorithm must provide the following: */\n> > + virtual void pauseShutter();\n> > + virtual void resumeShutter();\n> > + virtual bool isShutterPaused() const;\n> > + virtual void pauseGain();\n> > + virtual void resumeGain();\n> > + virtual bool isGainPaused() const;\n> > virtual unsigned int getConvergenceFrames() const = 0;\n> > virtual void setEv(double ev) = 0;\n> > virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > index bd54a639d637..3d04724349f7 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > @@ -275,18 +275,36 @@ bool Agc::isPaused() const\n> > return false;\n> > }\n> >\n> > -void Agc::pause()\n> > +void Agc::pauseShutter()\n> > {\n> > fixedShutter_ = status_.shutterTime;\n> > - fixedAnalogueGain_ = status_.analogueGain;\n> > }\n> >\n> > -void Agc::resume()\n> > +void Agc::resumeShutter()\n> > {\n> > fixedShutter_ = 0s;\n> > +}\n> > +\n> > +bool Agc::isShutterPaused() const\n> > +{\n> > + return fixedShutter_ != 0s;\n> > +}\n> > +\n> > +void Agc::pauseGain()\n> > +{\n> > + fixedAnalogueGain_ = status_.analogueGain;\n> > +}\n> > +\n> > +void Agc::resumeGain()\n> > +{\n> > fixedAnalogueGain_ = 0;\n> > }\n> >\n> > +bool Agc::isGainPaused() const\n> > +{\n> > + return fixedAnalogueGain_ != 0;\n> > +}\n> > +\n> > unsigned int Agc::getConvergenceFrames() const\n> > {\n> > /*\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > index 6d6b0e5ad857..cfb57f41848b 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > @@ -77,8 +77,12 @@ public:\n> > int read(const libcamera::YamlObject ¶ms) override;\n> > /* AGC handles \"pausing\" for itself. */\n> > bool isPaused() const override;\n> > - void pause() override;\n> > - void resume() override;\n> > + void pauseShutter() override;\n> > + void resumeShutter() override;\n> > + bool isShutterPaused() const override;\n> > + void pauseGain() override;\n> > + void resumeGain() override;\n> > + bool isGainPaused() const override;\n> > unsigned int getConvergenceFrames() const override;\n> > void setEv(double ev) override;\n> > void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 69c73f8c780a..c041aac008eb 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -74,8 +74,13 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;\n> >\n> > /* List of controls handled by the Raspberry Pi IPA */\n> > static const ControlInfoMap::Map ipaControls{\n> > - { &controls::AeEnable, ControlInfo(false, true) },\n> > + { &controls::ExposureTimeMode,\n> > + ControlInfo(static_cast(controls::ExposureTimeModeAuto),\n> > + static_cast(controls::ExposureTimeModeManual)) },\n> > { &controls::ExposureTime, ControlInfo(0, 66666) },\n> > + { &controls::AnalogueGainMode,\n> > + ControlInfo(static_cast(controls::AnalogueGainModeAuto),\n> > + static_cast(controls::AnalogueGainModeManual)) },\n> > { &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },\n> > { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },\n> > { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },\n> > @@ -556,9 +561,18 @@ void IPARPi::reportMetadata()\n> > }\n> >\n> > AgcStatus *agcStatus = rpiMetadata_.getLocked(\"agc.status\");\n> > - if (agcStatus) {\n> > - libcameraMetadata_.set(controls::AeLocked, agcStatus->locked);\n> > + if (agcStatus)\n> > libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);\n> > +\n> > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > + controller_.getAlgorithm(\"agc\"));\n> > + if (agc) {\n> > + if (agc->isShutterPaused() && agc->isGainPaused())\n> > + libcameraMetadata_.set(controls::AeState, controls::AeStateIdle);\n> > + else if (agcStatus)\n> > + libcameraMetadata_.set(controls::AeState,\n> > + agcStatus->locked ? controls::AeStateConverged\n> > + : controls::AeStateSearching);\n> > }\n> >\n> > LuxStatus *luxStatus = rpiMetadata_.getLocked(\"lux.status\");\n> > @@ -703,20 +717,22 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > << \" = \" << ctrl.second.toString();\n> >\n> > switch (ctrl.first) {\n> > - case controls::AE_ENABLE: {\n> > - RPiController::Algorithm *agc = controller_.getAlgorithm(\"agc\");\n> > + case controls::EXPOSURE_TIME_MODE: {\n> > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > + controller_.getAlgorithm(\"agc\"));\n> > if (!agc) {\n> > LOG(IPARPI, Warning)\n> > - << \"Could not set AE_ENABLE - no AGC algorithm\";\n> > + << \"Could not set EXPOSURE_TIME_MODE - no AGC algorithm\";\n> > break;\n> > }\n> >\n> > - if (ctrl.second.get() == false)\n> > - agc->pause();\n> > + if (ctrl.second.get() == controls::ExposureTimeModeManual)\n> > + agc->pauseShutter();\n> > else\n> > - agc->resume();\n> > + agc->resumeShutter();\n> >\n> > - libcameraMetadata_.set(controls::AeEnable, ctrl.second.get());\n> > + libcameraMetadata_.set(controls::ExposureTimeMode,\n> > + ctrl.second.get());\n> > break;\n> > }\n> >\n> > @@ -729,6 +745,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > break;\n> > }\n> >\n> > + /*\n> > + * Ignore manual exposure time when the auto exposure\n> > + * algorithm is running.\n> > + */\n> > + if (!agc->isShutterPaused())\n> > + break;\n> > +\n> > /* The control provides units of microseconds. */\n> > agc->setFixedShutter(ctrl.second.get() * 1.0us);\n> >\n> > @@ -736,6 +759,25 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > break;\n> > }\n> >\n> > + case controls::ANALOGUE_GAIN_MODE: {\n> > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > + controller_.getAlgorithm(\"agc\"));\n> > + if (!agc) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set ANALOGUE_GAIN_MODE - no AGC algorithm\";\n> > + break;\n> > + }\n> > +\n> > + if (ctrl.second.get() == controls::AnalogueGainModeManual)\n> > + agc->pauseGain();\n> > + else\n> > + agc->resumeGain();\n> > +\n> > + libcameraMetadata_.set(controls::AnalogueGainMode,\n> > + ctrl.second.get());\n> > + break;\n> > + }\n> > +\n> > case controls::ANALOGUE_GAIN: {\n> > RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > controller_.getAlgorithm(\"agc\"));\n> > @@ -745,6 +787,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > break;\n> > }\n> >\n> > + /*\n> > + * Ignore manual analogue gain value when the auto gain\n> > + * algorithm is running.\n> > + */\n> > + if (!agc->isGainPaused())\n> > + break;\n> > +\n> > agc->setFixedAnalogueGain(ctrl.second.get());\n> >\n> > libcameraMetadata_.set(controls::AnalogueGain,\n> > @@ -801,6 +850,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > break;\n> > }\n> >\n> > + /*\n> > + * Ignore AE_EXPOSURE_MODE if the shutter or the gain\n> > + * are in auto mode.\n> > + */\n> > + if (!agc->isShutterPaused() || !agc->isGainPaused())\n> > + break;\n> > +\n> > int32_t idx = ctrl.second.get();\n> > if (ExposureModeTable.count(idx)) {\n> > agc->setExposureMode(ExposureModeTable.at(idx));\n> > --\n> > 2.37.1\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 61CB7C3272\n\tfor ;\n\tThu, 18 Aug 2022 14:26:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 236B561FC2;\n\tThu, 18 Aug 2022 16:26:00 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3BE1C61FBC\n\tfor ;\n\tThu, 18 Aug 2022 16:25:58 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id t21so1855743ljc.13\n\tfor ;\n\tThu, 18 Aug 2022 07:25:58 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660832760;\n\tbh=mHhCcO4q4QaQx0c+BiaxkrtW668D/6EM7ZRKuzRlXNs=;\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=GVxSxJRnSr6vg3FShGHEChw/MFJLR/stjCOCcjPGKWgHb2geqmUrCZOv+GOilJmTW\n\tKhjShLntcNG/MOPdCUIkyJT9PbrsLJ2+yn63LwIAfVL2GUgl3UwDoEqgbzVJsmJHkT\n\t4EYFAAUaKzx49zx5wbnBzlE6/Ekbw9gfz6GaHTsEswy5+4aW1KTcD5sokc6Av6i7ad\n\to1Gxw0jS7xDjVEDYZ/5E5P++yEEYOPQe2vdWmHQibLhsw4hqZvr5TSD260gSDJDQzC\n\tJxNEMUNRv+nr9cf5cZi3n1f14aLobEM3/27Nn8Ck2JXTETw8gV52707krY0a1D/QQK\n\tFt2gZSwUfdr6Q==","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;\n\tbh=VkZKujDCrIJ0QJgGhFPS86apmj0j6tbiocht/5RgjhA=;\n\tb=epVvyIWyQR2SKMf4HFxU+EvkMDdlh/w+4Sow64r6SDeyH7oN5iqobmNdS9QJB/Z9ab\n\tK7EqrepSo3rTKrWzPoeFnHK9IhVMk55vY6bc43TIirS4DsMedYRNvaJnIvg9Yfk1hNRs\n\twil0S+TMnufmirc3ria4FnDNR0yC7tfxm3VoY0OzAF7ZWs4GurGVyeF+JBKW9IU5Jw7s\n\tIuym0Vs1NV2lPKj/BEDBnWZzAr8zSTF7nxMm5jnnEz0CmfSspmLAyByxxTntQKjJtQCH\n\tdE+vq1tXt9E6teuxW/l5nOX7fHDLL6rambqRQhFgSwuOquiXQDXLr/5hw0oZPFokoEp/\n\tyRww=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"epVvyIWy\"; 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;\n\tbh=VkZKujDCrIJ0QJgGhFPS86apmj0j6tbiocht/5RgjhA=;\n\tb=rtcNDozz5xGUc8iN3tbfNtMi4ps6HMmZ/PmstNe3UGQ/ObaLcloPzheHR3OybbxEyA\n\tK3PD5rZsy7QsXadSn5x+vH9ySXAw3hraZ123uO/UcpajQPHA1iDG93AhnQx85E97rJtw\n\tYwreMZMEM0FpOSdCFuyTUfzULI3P923kXvAG/u5mNJNqvVFcAh91fE/sXkZ2zOQVk2mI\n\tZlQ32q+NFsdOcfQ9i2hF46My8R1wPL76ozc0L0tAziKvC0O3p4h4FwX78nK1eUPNIVAM\n\tareIIZ/zckKmDyCmFJMAgcJgO5HhsCrJiyKmpq4fDkj5tBTJasOxdOCvCpfunm+J8WHI\n\tTe3g==","X-Gm-Message-State":"ACgBeo3X2m6BoD5ILBVUxxx6hbs+P7rPDFfMInDOPD8G4P8n5uTF/zfF\n\t5xowVwtYuQLeTy9xm/bFjhxxPLhCbnjUuHpaDOY9c3zfzCQ=","X-Google-Smtp-Source":"AA6agR51bJm9e+q4pdUzSC9dAVRjw2PmjopheVMeSUKQdvwU+WX/Ldk9PMPypt82X72U3AUJn6EcSk8CChK39NEbo6w=","X-Received":"by 2002:a05:6402:26c1:b0:43d:afb9:220c with SMTP id\n\tx1-20020a05640226c100b0043dafb9220cmr2522127edd.26.1660832746542;\n\tThu, 18 Aug 2022 07:25:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20220811150219.62066-1-jacopo@jmondi.org>\n\t<20220811150219.62066-5-jacopo@jmondi.org>\n\t<20220816053055.GV311202@pyrite.rasen.tech>","In-Reply-To":"<20220816053055.GV311202@pyrite.rasen.tech>","Date":"Thu, 18 Aug 2022 15:25:35 +0100","Message-ID":"","To":"Paul Elder ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipa: raspberry: Port to the\n\tnew AEGC 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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24675,"web_url":"https://patchwork.libcamera.org/comment/24675/","msgid":"<20220818155818.m62nf23c2cskkybe@uno.localdomain>","date":"2022-08-18T15:58:18","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipa: raspberry: Port to the\n\tnew AEGC controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David\n\nOn Thu, Aug 18, 2022 at 03:25:35PM +0100, David Plowman wrote:\n> Hi Jacopo\n>\n> Thanks for doing this work! I think mostly this seemed reasonable to\n> me, there were just a few things I wanted to clarify about the desired\n> behaviour - it seems such a long time ago that we first discussed\n> this.\n>\n> 1. How to turn AEGC on/off\n>\n> As I understand it now, you have to pause both the \"shutter\" and\n> \"gain\" explicitly. I think we talked about having some way to \"turn\n> everything on/off\", given that an application might not know exactly\n> how many different things there are that it has to worry about.\n>\n> Has there been any further thought about this? Actually I'm less\n> interested in the C++ API these days, so maybe it's up to me to make\n> this work nicely in Python-land.\n>\n\nYes, we discussed about having a single big button to get into full\nmanual mode. In the same way we can have an helper to implement\n\"aperture\" and \"shutter\" priorities. My understanding is those belong\nto the upper layers. In your case the libcamera-apps or the python\nbindings.\n\nI wouldn't be against having those helpers as part of libcamera\nitself, the design is not clear to me yet but I wouldn't mind\n\n> 2 Setting manual values\n>\n> My reading of this is now that manual values are ignored unless you're\n> in the \"manual\" mode. I guess this is OK so long as you can send\n> \"manual mode\" along with the \"manual value\".\n\nYes you can send both of them as part of the same Request. True that\nwithout precise per-frame control the AECG machinery would be switched\nto manual mode immediately and the desired exposure time/gain would be\nrealized only a few frames later. The IPA can be made smart and handle\nthis case with some ad-hoc code.\n\n>\n> Actually I wasn't quite sure if the Raspberry Pi implementation would\n> behave correctly in this respect, there's maybe an issue related to\n> processing controls in the correct order. So perhaps something to look\n> at there.\n>\n\nIf there's a way to trigger such behaviour I can help\n\n> Here too I wonder about doing something to make the interface a bit\n> easier for our Python users, but that sounds like my problem.\n>\n> I think that was pretty much everything that came to mind, so thanks\n> for doing this work!\n>\n> Best regards\n> David\n>\n> On Tue, 16 Aug 2022 at 06:31, Paul Elder via libcamera-devel\n> wrote:\n> >\n> > Hi Jacopo,\n> >\n> > On Thu, Aug 11, 2022 at 05:02:17PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > > The newly introduced controls to driver the AEGC algorithm allow\n> >\n> > s/driver/drive/\n> >\n> > > to control the computation of the exposure time and analogue gain\n> >\n> > s/to control/controlling/\n> >\n> > > separately.\n> > >\n> > > The RPi AEGC implementation already computes the shutter and gain\n> > > values separately but does not expose separate functions to control\n> > > them.\n> > >\n> > > Augment the AgcAlgorithm interface to allow to pause/resume the shutter\n> >\n> > s#to pause/resume#pausing/resuming#\n> >\n> > > and gain automatic computations separately and plumb them to the newly\n> > > introduced controls.\n> > >\n> > > Add safety checks to ignore ExposureTime and AnalogueGain values if the\n> > > algorithms are not paused, and report the correct AeState value by\n> > > checking if both algorithms have been paused or if they have converged.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> >\n> > Reviewed-by: Paul Elder \n> >\n> > > ---\n> > > .../raspberrypi/controller/agc_algorithm.h | 6 ++\n> > > src/ipa/raspberrypi/controller/rpi/agc.cpp | 24 +++++-\n> > > src/ipa/raspberrypi/controller/rpi/agc.h | 8 +-\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 76 ++++++++++++++++---\n> > > 4 files changed, 99 insertions(+), 15 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > > index 3a91444c3a61..bf9c501db553 100644\n> > > --- a/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > > +++ b/src/ipa/raspberrypi/controller/agc_algorithm.h\n> > > @@ -17,6 +17,12 @@ class AgcAlgorithm : public Algorithm\n> > > public:\n> > > AgcAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > > /* An AGC algorithm must provide the following: */\n> > > + virtual void pauseShutter();\n> > > + virtual void resumeShutter();\n> > > + virtual bool isShutterPaused() const;\n> > > + virtual void pauseGain();\n> > > + virtual void resumeGain();\n> > > + virtual bool isGainPaused() const;\n> > > virtual unsigned int getConvergenceFrames() const = 0;\n> > > virtual void setEv(double ev) = 0;\n> > > virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > index bd54a639d637..3d04724349f7 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > @@ -275,18 +275,36 @@ bool Agc::isPaused() const\n> > > return false;\n> > > }\n> > >\n> > > -void Agc::pause()\n> > > +void Agc::pauseShutter()\n> > > {\n> > > fixedShutter_ = status_.shutterTime;\n> > > - fixedAnalogueGain_ = status_.analogueGain;\n> > > }\n> > >\n> > > -void Agc::resume()\n> > > +void Agc::resumeShutter()\n> > > {\n> > > fixedShutter_ = 0s;\n> > > +}\n> > > +\n> > > +bool Agc::isShutterPaused() const\n> > > +{\n> > > + return fixedShutter_ != 0s;\n> > > +}\n> > > +\n> > > +void Agc::pauseGain()\n> > > +{\n> > > + fixedAnalogueGain_ = status_.analogueGain;\n> > > +}\n> > > +\n> > > +void Agc::resumeGain()\n> > > +{\n> > > fixedAnalogueGain_ = 0;\n> > > }\n> > >\n> > > +bool Agc::isGainPaused() const\n> > > +{\n> > > + return fixedAnalogueGain_ != 0;\n> > > +}\n> > > +\n> > > unsigned int Agc::getConvergenceFrames() const\n> > > {\n> > > /*\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > > index 6d6b0e5ad857..cfb57f41848b 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > > @@ -77,8 +77,12 @@ public:\n> > > int read(const libcamera::YamlObject ¶ms) override;\n> > > /* AGC handles \"pausing\" for itself. */\n> > > bool isPaused() const override;\n> > > - void pause() override;\n> > > - void resume() override;\n> > > + void pauseShutter() override;\n> > > + void resumeShutter() override;\n> > > + bool isShutterPaused() const override;\n> > > + void pauseGain() override;\n> > > + void resumeGain() override;\n> > > + bool isGainPaused() const override;\n> > > unsigned int getConvergenceFrames() const override;\n> > > void setEv(double ev) override;\n> > > void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index 69c73f8c780a..c041aac008eb 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -74,8 +74,13 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;\n> > >\n> > > /* List of controls handled by the Raspberry Pi IPA */\n> > > static const ControlInfoMap::Map ipaControls{\n> > > - { &controls::AeEnable, ControlInfo(false, true) },\n> > > + { &controls::ExposureTimeMode,\n> > > + ControlInfo(static_cast(controls::ExposureTimeModeAuto),\n> > > + static_cast(controls::ExposureTimeModeManual)) },\n> > > { &controls::ExposureTime, ControlInfo(0, 66666) },\n> > > + { &controls::AnalogueGainMode,\n> > > + ControlInfo(static_cast(controls::AnalogueGainModeAuto),\n> > > + static_cast(controls::AnalogueGainModeManual)) },\n> > > { &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },\n> > > { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },\n> > > { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },\n> > > @@ -556,9 +561,18 @@ void IPARPi::reportMetadata()\n> > > }\n> > >\n> > > AgcStatus *agcStatus = rpiMetadata_.getLocked(\"agc.status\");\n> > > - if (agcStatus) {\n> > > - libcameraMetadata_.set(controls::AeLocked, agcStatus->locked);\n> > > + if (agcStatus)\n> > > libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);\n> > > +\n> > > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > > + controller_.getAlgorithm(\"agc\"));\n> > > + if (agc) {\n> > > + if (agc->isShutterPaused() && agc->isGainPaused())\n> > > + libcameraMetadata_.set(controls::AeState, controls::AeStateIdle);\n> > > + else if (agcStatus)\n> > > + libcameraMetadata_.set(controls::AeState,\n> > > + agcStatus->locked ? controls::AeStateConverged\n> > > + : controls::AeStateSearching);\n> > > }\n> > >\n> > > LuxStatus *luxStatus = rpiMetadata_.getLocked(\"lux.status\");\n> > > @@ -703,20 +717,22 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > > << \" = \" << ctrl.second.toString();\n> > >\n> > > switch (ctrl.first) {\n> > > - case controls::AE_ENABLE: {\n> > > - RPiController::Algorithm *agc = controller_.getAlgorithm(\"agc\");\n> > > + case controls::EXPOSURE_TIME_MODE: {\n> > > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > > + controller_.getAlgorithm(\"agc\"));\n> > > if (!agc) {\n> > > LOG(IPARPI, Warning)\n> > > - << \"Could not set AE_ENABLE - no AGC algorithm\";\n> > > + << \"Could not set EXPOSURE_TIME_MODE - no AGC algorithm\";\n> > > break;\n> > > }\n> > >\n> > > - if (ctrl.second.get() == false)\n> > > - agc->pause();\n> > > + if (ctrl.second.get() == controls::ExposureTimeModeManual)\n> > > + agc->pauseShutter();\n> > > else\n> > > - agc->resume();\n> > > + agc->resumeShutter();\n> > >\n> > > - libcameraMetadata_.set(controls::AeEnable, ctrl.second.get());\n> > > + libcameraMetadata_.set(controls::ExposureTimeMode,\n> > > + ctrl.second.get());\n> > > break;\n> > > }\n> > >\n> > > @@ -729,6 +745,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > > break;\n> > > }\n> > >\n> > > + /*\n> > > + * Ignore manual exposure time when the auto exposure\n> > > + * algorithm is running.\n> > > + */\n> > > + if (!agc->isShutterPaused())\n> > > + break;\n> > > +\n> > > /* The control provides units of microseconds. */\n> > > agc->setFixedShutter(ctrl.second.get() * 1.0us);\n> > >\n> > > @@ -736,6 +759,25 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > > break;\n> > > }\n> > >\n> > > + case controls::ANALOGUE_GAIN_MODE: {\n> > > + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > > + controller_.getAlgorithm(\"agc\"));\n> > > + if (!agc) {\n> > > + LOG(IPARPI, Warning)\n> > > + << \"Could not set ANALOGUE_GAIN_MODE - no AGC algorithm\";\n> > > + break;\n> > > + }\n> > > +\n> > > + if (ctrl.second.get() == controls::AnalogueGainModeManual)\n> > > + agc->pauseGain();\n> > > + else\n> > > + agc->resumeGain();\n> > > +\n> > > + libcameraMetadata_.set(controls::AnalogueGainMode,\n> > > + ctrl.second.get());\n> > > + break;\n> > > + }\n> > > +\n> > > case controls::ANALOGUE_GAIN: {\n> > > RPiController::AgcAlgorithm *agc = dynamic_cast(\n> > > controller_.getAlgorithm(\"agc\"));\n> > > @@ -745,6 +787,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > > break;\n> > > }\n> > >\n> > > + /*\n> > > + * Ignore manual analogue gain value when the auto gain\n> > > + * algorithm is running.\n> > > + */\n> > > + if (!agc->isGainPaused())\n> > > + break;\n> > > +\n> > > agc->setFixedAnalogueGain(ctrl.second.get());\n> > >\n> > > libcameraMetadata_.set(controls::AnalogueGain,\n> > > @@ -801,6 +850,13 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > > break;\n> > > }\n> > >\n> > > + /*\n> > > + * Ignore AE_EXPOSURE_MODE if the shutter or the gain\n> > > + * are in auto mode.\n> > > + */\n> > > + if (!agc->isShutterPaused() || !agc->isGainPaused())\n> > > + break;\n> > > +\n> > > int32_t idx = ctrl.second.get();\n> > > if (ExposureModeTable.count(idx)) {\n> > > agc->setExposureMode(ExposureModeTable.at(idx));\n> > > --\n> > > 2.37.1\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 64669C3272\n\tfor ;\n\tThu, 18 Aug 2022 15:58:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C24C61FC0;\n\tThu, 18 Aug 2022 17:58:24 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::224])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1DBE061FA7\n\tfor ;\n\tThu, 18 Aug 2022 17:58:23 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 86802E0002;\n\tThu, 18 Aug 2022 15:58:21 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660838304;\n\tbh=7E70FBEV1dQst2yRxueBMsoArC9+fyd/+vhyljsS2Q4=;\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=UhFJeFMJ4k+UQArF9v/DGPEMzkP4rkjdooKoy6G/xclateqWJd8I1dQCqj9cc0gHQ\n\t2x3DgjrqT+O7YtPLgCZGP+3m1d0stTNTLmoz9c0cMutCAAt0l/TABHVDLX6SU0846m\n\tWkSSMxNHwrypA1HL9qRDQ4xW30zIhCPERMCb0PDw/ZVH4Ow1LXdBdh7uKzi2118QXA\n\tTXo9cdL0qxVpK3j+7UeHX+Balk3bzcKqYxDBYth1htWXkOsxsmyLlN+SLAcoZkCMq9\n\tGwZQAGq0xN+26tRkDNNV5P064peeJcfvRHgyYc2dBBC1R/r0vJvB/3fpog4y2c6jyt\n\tD9ihWek3Wl2Yw==","Date":"Thu, 18 Aug 2022 17:58:18 +0200","To":"David Plowman ","Message-ID":"<20220818155818.m62nf23c2cskkybe@uno.localdomain>","References":"<20220811150219.62066-1-jacopo@jmondi.org>\n\t<20220811150219.62066-5-jacopo@jmondi.org>\n\t<20220816053055.GV311202@pyrite.rasen.tech>\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 4/6] ipa: raspberry: Port to the\n\tnew AEGC 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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]