[{"id":32991,"web_url":"https://patchwork.libcamera.org/comment/32991/","msgid":"<20250109214047.GF6159@pendragon.ideasonboard.com>","date":"2025-01-09T21:40:47","subject":"Re: [PATCH v6 09/12] controls: Redefine AeEnable","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jan 08, 2025 at 06:09:39PM -0600, Paul Elder wrote:\n> In the redesign of the AE-related controls, the AeEnable control was\n> intended to be removed in favor of more specific sub-controls for\n> analogue gain and exposure time. However this will cause problems if an\n> aperture sub-controls is introduced, and an application from a\n\ns/controls/control/ (or s/if an/if/ on the previous line)\n\n> pre-aperture era uses a camera that supports aperture.\n> \n> If there is no AeEnable control, then a pre-aperture era application\n> might set analogue gain and exposure time to manual, while aperture\n> silently stays auto since that's the default mode. Thus aperture would\n> be uncontrollable by the application.\n> \n> With an AeEnable control, then a pre-aperture era application can set\n> AeEnable to manual, and under the hood all three of analogue gain and\n> exposure time and aperture will be set to manual. The application won't\n> be able to set the manual aperture, however.\n> \n> Although the above scenario is expected to be rare, as applications are\n> expected to recompile on a libcamera version change, the scenario with\n\nIt's not a matter of recompiling applications. Even if recompiled, an\napplication that doesn't know about aperture will not set the related\ncontrols. I think you can just drop the second clause above (from \"as\napplications\" to \"version chage, \").\n\n> an AeEnable control seems less detrimental. With an AeEnable control at\n> least the aperture would be static at a reasonably usable value, whereas\n> without an AeEnable the aperture would be more-or-less uncontrolable and\n> could go to extreme values as the AEGC algorithm tries to compensate for\n> the manual analogue gain and exposure time values.\n> \n> Thus we redefine the AeEnable control, available only as a control and\n> not in metadata. It will be preprocessed by the Camera class so that the\n> relevant sub-controls are set. No pipline handler nor IPA shall act on\n> the AeEnable control. The IPA still has to report the control as\n> available, however.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> New in v6\n> ---\n> Documentation/design/ae.rst | 23 +++++++++++++++++------\n> src/libcamera/control_ids_core.yaml | 12 +++++++++---\n> 2 files changed, 26 insertions(+), 9 deletions(-)\n> \n> diff --git a/Documentation/design/ae.rst b/Documentation/design/ae.rst\n> index f8b2c887c..bbb6bb1d1 100644\n> --- a/Documentation/design/ae.rst\n> +++ b/Documentation/design/ae.rst\n> @@ -185,6 +185,10 @@ A diagram of our solution:\n> 0: Auto\n> 1: Manual\n> \n> + AeEnable\n> + - True -> ExposureTimeMode:Auto + AnalogueGainMode:Auto\n> + - False -> ExposureTimeMode:Manual + AnalogueGainMode:Manual\n> +\n> \n> The diagram is divided in four sections horizontally:\n> \n> @@ -225,10 +229,14 @@ This simulates an auto -> locked -> manual or auto -> manual state transition,\n> and makes it impossible to do the nonsensical manual -> locked state\n> transition.\n> \n> -We specifically do not have a \"master AE control\" like the old AeEnable. This\n> -is because we have the individual mode controls, and if we had a master AE\n> -control it would be a \"control that sets other controls\", which could easily\n> -get out of control.\n> +AeEnable still exists to allow applications to set the mode of all the\n> +sub-controls at once. Besides being for convenience, this will also be useful\n> +when we eventually implement an aperture control. This is because applications\n> +there were made before aperture was available would still be able to set\n\nthat are written before aperture is available will still be able to set\n\n> +aperture mode to auto or manual, as opposed to having the aperture stuck at\n> +auto while the application really wanted manual. Although the aperture would\n> +still be stuck at an uncontrollable value, at least it would be at a static\n> +usable value as opposed to varying via the AEGC algorithm.\n> \n> With this solution, the earlier example would become:\n> \n> @@ -277,9 +285,12 @@ and gain:\n> \n> - AeState\n> \n> +- AeEnable\n> +\n> Auto-exposure and auto-gain can be enabled and disabled separately using the\n> -ExposureTimeMode and AnalogueGainMode controls respectively. There is no\n> -overarching AeEnable control.\n> +ExposureTimeMode and AnalogueGainMode controls respectively. The AeEnable\n> +control can also be used, as it sets both of the modes simultaneously. The\n\ns/as it/and/\n\nReviewed-by: Laurent Pinchart \n\n> +AeEnable control is not returned in metadata.\n> \n> When the respective mode is set to auto, the respective value that is computed\n> by the AEGC algorithm is applied to the image sensor. Any value that is\n> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> index d88c1852e..aa7448645 100644\n> --- a/src/libcamera/control_ids_core.yaml\n> +++ b/src/libcamera/control_ids_core.yaml\n> @@ -10,11 +10,17 @@ vendor: libcamera\n> controls:\n> - AeEnable:\n> type: bool\n> - direction: inout\n> + direction: in\n> description: |\n> - Enable or disable the AE.\n> + Enable or disable the AEGC algorithm. When this control is set to true,\n> + both ExposureTimeMode and AnalogueGainMode are set to auto, and if this\n> + control is set to false then both are set to manual.\n> +\n> + If ExposureTimeMode or AnalogueGainMode are also set in the same\n> + request as AeEnable, then the modes supplied by ExposureTimeMode or\n> + AnalogueGainMode will take precedence.\n> \n> - \\sa ExposureTime AnalogueGain\n> + \\sa ExposureTimeMode AnalogueGainMode\n> \n> - AeState:\n> type: int32_t","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 45D1AC32EF\n\tfor ;\n\tThu, 9 Jan 2025 21:40:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75F35684EA;\n\tThu, 9 Jan 2025 22:40:53 +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 6D80B61880\n\tfor ;\n\tThu, 9 Jan 2025 22:40:51 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 594176DC;\n\tThu, 9 Jan 2025 22:39:57 +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=\"fzA6Symc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1736458797;\n\tbh=C08NzfCtOaGXTGu0Uwlgv/XYndALTWcmAsJEimAzsOM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fzA6Symc9hAXLJu+yRQrWuJ9PnnccUBuuqQt4H9eQfq4ncZTla98Xv7MjKupMbDPL\n\tDEHtYn+K46+A5UvWWxr1av3Bz6rOhMl0CuvwabjLmH7enqUCr0I614lEWQbxejAsR3\n\t1PTLmayPouGMIIHaOJRpm7SQm3+kBz53n3niCrhU=","Date":"Thu, 9 Jan 2025 23:40:47 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, stefan.klug@ideasonboard.com","Subject":"Re: [PATCH v6 09/12] controls: Redefine AeEnable","Message-ID":"<20250109214047.GF6159@pendragon.ideasonboard.com>","References":"<20250109000942.1616565-1-paul.elder@ideasonboard.com>\n\t<20250109000942.1616565-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250109000942.1616565-10-paul.elder@ideasonboard.com>","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\" "}}]