[{"id":23072,"web_url":"https://patchwork.libcamera.org/comment/23072/","msgid":"<20220518192628.ypu4fdfmad7oup2t@uno.localdomain>","date":"2022-05-18T19:26:28","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Wed, May 18, 2022 at 03:47:26PM +0200, Paul Elder via libcamera-devel wrote:\n> We have multiple goals:\n> - we need a lock of some sort, to instruct the AEGC to not update output\n> results\n> - we need manual modes, to override the values computed by the AEGC\n> - we need to support seamless transitions from auto -> manual, and do so\n> without flickering\n> - we need custom minimum values for the manual controls, that is no\n> magic values for enabling/disabling auto\n> - all of these need to be done with AE sub-controls (exposure time,\n> analogue gain)\n>\n> To achieve these goals, we introduce mode controls for the AE\n> sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto\n> state, and a disabled state. The disabled state has an internal one-way\n> state change from locked to manual, triggered by the presence of the\n> value-controls (ExposureTime and AnalogueGain).\n>\n> We then remove the AeEnable control, as it is a redundant control in the\n> face of these two mode controls.\n>\n> We also remove AeLocked, as it is insufficient for reporting the AE\n> state, and we promote AeState to non-draft to fill its role. Notably,\n> the locked state is removed, since this information can be obtained from\n> the aforementioned mode controls.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> Signed-off-by: Paul Elder \n>\n\nBe prepared: a lot of bikeshedding ahead\n\nI think the content is good and these are the last refinements. Take\nthem as suggestions.\n\n> ---\n> Changes in v4:\n> - remove FlashRequired and Precapture from AeState\n> - upgrade documentation of all the controls\n>\n> Changes in v3:\n> - improve wording of the control descriptions\n> - make more succinct and clear\n> - add description of how to do a flickerless transition\n>\n> Changes in v2:\n> - No changes, just resubmitting at the head of this series so that it's\n> together and so that /people will actually see it/\n>\n> Initial version:\n> Still RFC as I haven't updated the users of the control yet, and I want\n> to check that these are the controls and docs that we want.\n>\n> We've decided that the \"master AE control\" will be implemented by a\n> helper... but looking at uvcvideo and the V4L2 controls I'm wondering if\n> such helper should come earlier than later?\n> ---\n> src/libcamera/control_ids.yaml | 262 +++++++++++++++++++++++++--------\n> 1 file changed, 200 insertions(+), 62 deletions(-)\n>\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 9d4638ae..9f5ce5e8 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -7,23 +7,46 @@\n> # Unless otherwise stated, all controls are bi-directional, i.e. they can be\n> # set through Request::controls() and returned out through Request::metadata().\n> controls:\n> - - AeEnable:\n> - type: bool\n> + - AeState:\n> + type: int32_t\n> description: |\n> - Enable or disable the AE.\n> + Control to report the AE algorithm state associated with the capture\n> + result.\n\nI know you added the last part to highlight that this control is only\nreported in metadata. To be honest I would defer this to a later\nseries where the 'direction' of controls will be more clearly defined\nand just define this\n\n \"Control to report the AE algorithm state'\n>\n> - \\sa ExposureTime AnalogueGain\n> + The state is still reported even if ExposureTimeMode or\n> + AnalogueGainMode is set to Disabled.\n>\n> - - AeLocked:\n> - type: bool\n> - description: |\n> - Report the lock status of a running AE algorithm.\n> + \\sa AnalogueGain\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTime\n> + \\sa ExposureTimeMode\n>\n> - If the AE algorithm is locked the value shall be set to true, if it's\n> - converging it shall be set to false. If the AE algorithm is not\n> - running the control shall not be present in the metadata control list.\n> + enum:\n\nThe AF controls use the term \"Idle\" instead of inactive. I think it\nwould be nice if we use the same term to express the same concept.\n\n> + - name: AeStateInactive\n> + value: 0\n> + description: |\n> + The AE algorithm is inactive.\n>\n> - \\sa AeEnable\n> + This state should be returned if both AnalogueGainMode and\n> + ExposureTimeMode are set to disabled (or one, if the camera only\n> + supports one of the two controls).\n\nDo you think it would be better to write this slightly differently, to\ntell what it means to application, instead of suggesting to IPA\ndevelopers what they have to report (this part can be kept as well,\nmaybe implicitly suggested).\n\nTo explain what I mean, I would make this along the lines of:\n\n This state is returned when the AE algorithm is not\n actively computing new values for the exposure time and\n analogue gain. This happens when both AnalogueGainMode\n and ExposureTimeMode are set to disabled or when the\n algorithm has converged and has not yet initiated a new\n scan.\n\n If either ExposureTimeMode or AnalogueGainMode are set\n to auto, the AE algorithm might spontaneously initiate\n a new scan in which case the state is moved to be\n AeStateSearching.\n\n> + - name: AeStateSearching\n> + value: 1\n> + description: |\n> + The AE algorithm has not converged yet.\n\n The AE algorithm is actively computing new values and\n has not converged yet.\n\n> + This state should be returned if at least one of AnalogueGainMode\n> + or ExposureTimeMode is set to auto, and the AE algorithm hasn't\n> + converged yet. If the AE algorithm converges, the state shall go to\n> + AeStateConverged.\n\n This state is only returned when at least one of\n AnalogueGainMode or ExposureTimeMode is set to auto and\n the AE algorithm has spontaneously started a scan and\n has not convergd yet. If the AE algorithm converges,\n the state is moved to AeStateConverged.\n\nFor the same reason of telling applications how they should interepet\nit instead of telling IPA developers how they should behave.\n\nDo we need a AeStateFailed state to report failed scans or it\nshouldn't happen at all ?\n\n> + - name: AeStateConverged\n> + value: 2\n> + description: |\n> + The AE algorithm has converged.\n> +\n> + This state should be returned if at least one of AnalogueGainMode\n> + or ExposureTimeMode is set to auto, and the AE algorithm has\n> + converged.\n\nSame \"This state is returned if ..\" instead of \"should be\"\n\nI hope my reasoning is right here and I'm not trying to correct a\nnative speaker with my bad english.\n\n>\n> # AeMeteringMode needs further attention:\n> # - Auto-generate max enum value.\n> @@ -93,6 +116,13 @@ controls:\n> how the desired total exposure is divided between the shutter time\n> and the sensor's analogue gain. The exposure modes are platform\n> specific, and not all exposure modes may be supported.\n> +\n> + When one of AnalogueGainMode or ExposureTimeMode is set to Disabled,\n> + the fixed values will override any choices made by AeExposureMode.\n> +\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTimeMode\n> +\n> enum:\n> - name: ExposureNormal\n> value: 0\n> @@ -111,13 +141,15 @@ controls:\n> type: float\n> description: |\n> Specify an Exposure Value (EV) parameter. The EV parameter will only be\n> - applied if the AE algorithm is currently enabled.\n> + applied if the AE algorithm is currently enabled, that is, at least one\n> + of AnalogueGainMode and ExposureTimeMode are auto.\n>\n> By convention EV adjusts the exposure as log2. For example\n> EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment\n> of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n>\n> - \\sa AeEnable\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTimeMode\n>\n> - ExposureTime:\n> type: int32_t\n> @@ -125,17 +157,85 @@ controls:\n> Exposure time (shutter speed) for the frame applied in the sensor\n> device. This value is specified in micro-seconds.\n>\n> - Setting this value means that it is now fixed and the AE algorithm may\n> - not change it. Setting it back to zero returns it to the control of the\n> - AE algorithm.\n> + This control will only take effect if ExposureTimeMode is Disabled. If\n> + this control is set when ExposureTimeMode is Auto, the value will be\n> + ignored and will not be retained.\n> +\n> + When reported in metadata, this control indicates what exposure time\n> + was used for the current request, regardless of ExposureTimeMode.\n\ns/for the current request//\n\nmetadata are always associated to requests :)\n\n> + ExposureTimeMode will indicate the source of the exposure time value,\n> + whether it came from the AE algorithm or not.\n> +\n> + \\sa AnalogueGain\n> + \\sa ExposureTimeMode\n> +\n> + - ExposureTimeMode:\n> + type: int32_t\n> + description: |\n> + Controls the source of the exposure time that is applied to the image\n> + sensor. When set to Auto, the AE algorithm computes the exposure time\n> + and configures the image sensor accordingly. When set to Disabled,\n> + exposure time specified in ExposureTime is applied to the image sensor.\n> + If ExposureTime is not set, then the value last computed by the AE\n\nI think you mean \"When transitioning from Auto to Manual mode and no\nExposureTime is provided, then...\"\n\n\n> + algorithm when the mode was Auto will be used.\n> +\n> + If ExposureTime is not set and the mode is ExposureTimeModeDisabled and\n> + AE was never Auto (either because the camera started in Disabled mode,\n> + or Auto is not supported by the camera), the camera should use a\n> + best-effort default value.\n> +\n> + When ExposureTimeMode is set Auto, the value set in ExposureTime is\n> + ignored and is not retained. This means that if ExposureTimeMode is set\n\n\"is later set to Disabled\"\n\nto make it clear it is a state transition\n\n> + to Disabled and ExposureTime is not also set, the exposure time that\n> + was last computed by the AE algorithm while the mode was Auto will be\n> + applied to the sensor.\n\nAh, isn't this also described above ?\n\n> +\n> + If ExposureTimeModeDisabled is supported, the ExposureTime control must\n> + also be supported.\n\nThis is a note for implementer, it doesn't hurt though, but I would\nrather move it to the design document you have prepared.\n\n> +\n> + The set of ExposureTimeMode modes that are supported by the camera must\n> + have an intersection with the supported set of AnalogueGainMode modes.\n>\n> - \\sa AnalogueGain AeEnable\n> + As it takes a few frames to apply the exposure time, there is a period of\n> + time between submitting a request with ExposureTimeMode set to Disabled\n> + and the exposure time component of the AE actually being disabled,\n> + during which the AE algorithm can still update the exposure time. If an\n> + application is switching from automatic and manual control and wishes\n> + to eliminate any flicker during the switch, the following procedure is\n> + recommended.\n>\n> - \\todo Document the interactions between AeEnable and setting a fixed\n> - value for this control. Consider interactions with other AE features,\n> - such as aperture and aperture/shutter priority mode, and decide if\n> - control of which features should be automatically adjusted shouldn't\n> - better be handled through a separate AE mode control.\n> + 1. Start with ExposureTimeMode set to Auto\n> +\n> + 2. Set ExposureTimeMode to Disabled\n> +\n> + 3. Wait for the first request to be output that has ExposureTimeMode\n> + set to Disabled\n> +\n> + 4. Copy the value reported in ExposureTime into a new request, and\n> + submit it\n> +\n> + 5. Proceed to run manual exposure time\n\nI dare to repropose the text I suggested on v2:\n\n Applications that transition from auto ExposureTimeMode\n to the direct control of the exposure time should\n aim to do so by selecting an ExposureTime value as close\n as possible to the last value computed by the auto\n exposure algorithm in order to avoid any visible\n flickering.\n\n To select the correct value to use as ExposureTime value,\n applications should accommodate the natural delay in\n applying controls caused by the capture pipeline frame\n depth.\n\n When switching to manual exposure mode, applications\n should not immediately specify an ExposureTime value in\n the same request where ExposureTimeMode is set to\n Disabled. They should instead wait for the first Request\n where ExposureTimeMode is reported as Disabled in the\n Request metadata, and use the there reported exposure to\n populate the ExposureTime control value in the next\n Request to be queued to the Camera.\n\n The implementation of the auto-exposure algorithm\n should equally try to minimize flickering and when\n transitioning from manual exposure mode to auto\n exposure use the last value provided by the application\n as starting point.\n\nThis is not alternative to the list of steps you have proposed but\nprovides a more detailed introduction to them. Do you think it's not\nnecessary ?\n\n> +\n> + \\sa ExposureTime\n> + enum:\n> + - name: ExposureTimeModeAuto\n> + value: 0\n> + description: |\n> + The exposure time will be calculated automatically and set by the\n> + AE algorithm. If ExposureTime is set while this mode is active, it\n> + will be ignored, and it will also not be retained.\n> + - name: ExposureTimeModeDisabled\n> + value: 1\n> + description: |\n> + The exposure time will not be updated by the AE algorithm. It will\n> + come from the last calculated value when the mode was Auto, or from\n> + the value specified in ExposureTime.\n> +\n> + When transitioning from Auto to Disabled mode, the last computed\n> + exposure value is used until a new value is specified through the\n> + ExposureTime control. If an ExposureTime value is specified in the\n> + same request where the ExposureTimeMode is changed from Auto to\n> + Disabled, the provided ExposureTime is applied.\n\n\"is applied immediately\" ?\n\nThanks and sorry for being picky, we're almost there :)\n\n>\n> - AnalogueGain:\n> type: float\n> @@ -144,17 +244,85 @@ controls:\n> The value of the control specifies the gain multiplier applied to all\n> colour channels. This value cannot be lower than 1.0.\n>\n> - Setting this value means that it is now fixed and the AE algorithm may\n> - not change it. Setting it back to zero returns it to the control of the\n> - AE algorithm.\n> + This control will only take effect if AnalogueGainMode is Disabled. If\n> + this control is set when AnalogueGainMode is Auto, the value will be\n> + ignored and will not be retained.\n> +\n> + When reported in metadata, this control indicates what analogue gain\n> + was used for the current request, regardless of AnalogueGainMode.\n> + AnalogueGainMode will indicate the source of the analogue gain value,\n> + whether it came from the AE algorithm or not.\n> +\n> + \\sa ExposureTime\n> + \\sa AnalogueGainMode\n> +\n> + - AnalogueGainMode:\n> + type: int32_t\n> + description: |\n> + Controls the source of the analogue gain that is applied to the image\n> + sensor. When set to Auto, the AE algorithm computes the analogue gain\n> + and configures the image sensor accordingly. When set to Disabled,\n> + analogue gain specified in AnalogueGain is applied to the image sensor.\n> + If AnalogueGain is not set, then the value last computed by the AE\n> + algorithm when the mode was Auto will be used.\n> +\n> + If AnalogueGain is not set and the mode is AnalogueGainModeDisabled and\n> + AE was never Auto (either because the camera started in Disabled mode,\n> + or Auto is not supported by the camera), the camera should use a\n> + best-effort default value.\n> +\n> + When AnalogueGainMode is set Auto, the value set in AnalogueGain is\n> + ignored and is not retained. This means that if AnalogueGainMode is set\n> + to Disabled and AnalogueGain is not also set, the analogue gain that\n> + was last computed by the AE algorithm while the mode was Auto will be\n> + applied to the sensor.\n>\n> - \\sa ExposureTime AeEnable\n> + If AnalogueGainModeDisabled is supported, the AnalogueGain control must\n> + also be supported.\n> +\n> + The set of AnalogueGainMode modes that are supported by the camera must\n> + have an intersection with the supported set of ExposureTimeMode modes.\n> +\n> + As it takes a few frames to apply the analogue gain, there is a period of\n> + time between submitting a request with AnalogueGainMode set to Disabled\n> + and the analogue gain component of the AE actually being disabled,\n> + during which the AE algorithm can still update the analogue gain. If an\n> + application is switching from automatic and manual control and wishes\n> + to eliminate any flicker during the switch, the following procedure is\n> + recommended.\n> +\n> + 1. Start with AnalogueGainMode set to Auto\n> +\n> + 2. Set AnalogueGainMode to Disabled\n> +\n> + 3. Wait for the first request to be output that has AnalogueGainMode\n> + set to Disabled\n> +\n> + 4. Copy the value reported in AnalogueGain into a new request, and\n> + submit it\n> +\n> + 5. Proceed to run manual analogue gain\n> +\n> + \\sa AnalogueGain\n> + enum:\n> + - name: AnalogueGainModeAuto\n> + value: 0\n> + description: |\n> + The analogue gain will be calculated automatically and set by the\n> + AE algorithm. If AnalogueGain is set while this mode is active, it\n> + will be ignored, and it will also not be retained.\n> + - name: AnalogueGainModeDisabled\n> + value: 1\n> + description: |\n> + The analogue gain will not be updated by the AE algorithm. It will\n> + come from the last calculated value when the mode was Auto, or from\n> + the value specified in AnalogueGain.\n>\n> - \\todo Document the interactions between AeEnable and setting a fixed\n> - value for this control. Consider interactions with other AE features,\n> - such as aperture and aperture/shutter priority mode, and decide if\n> - control of which features should be automatically adjusted shouldn't\n> - better be handled through a separate AE mode control.\n> + When transitioning from Auto to Disabled mode the last computed\n> + gain value is used until a new value is specified through the\n> + AnalogueGain control. If an AnalogueGain value is specified in the\n> + same request where the AnalogueGainMode is set to Disabled, the\n> + provided AnalogueGain is applied.\n>\n> - Brightness:\n> type: float\n> @@ -477,36 +645,6 @@ controls:\n> High quality aberration correction which might reduce the frame\n> rate.\n>\n> - - AeState:\n> - type: int32_t\n> - draft: true\n> - description: |\n> - Control to report the current AE algorithm state. Currently identical to\n> - ANDROID_CONTROL_AE_STATE.\n> -\n> - Current state of the AE algorithm.\n> - enum:\n> - - name: AeStateInactive\n> - value: 0\n> - description: The AE algorithm is inactive.\n> - - name: AeStateSearching\n> - value: 1\n> - description: The AE algorithm has not converged yet.\n> - - name: AeStateConverged\n> - value: 2\n> - description: The AE algorithm has converged.\n> - - name: AeStateLocked\n> - value: 3\n> - description: The AE algorithm is locked.\n> - - name: AeStateFlashRequired\n> - value: 4\n> - description: The AE algorithm would need a flash for good results\n> - - name: AeStatePrecapture\n> - value: 5\n> - description: |\n> - The AE algorithm has started a pre-capture metering session.\n> - \\sa AePrecaptureTrigger\n> -\n> - AfState:\n> type: int32_t\n> draft: true\n> --\n> 2.30.2\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 25E96C0F2A\n\tfor ;\n\tWed, 18 May 2022 19:26:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FB8665659;\n\tWed, 18 May 2022 21:26:32 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::223])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 258F665656\n\tfor ;\n\tWed, 18 May 2022 21:26:31 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 21A2260002;\n\tWed, 18 May 2022 19:26:29 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652901992;\n\tbh=rz2aeWTivh8NbReTgyd2DmA1bI4BWBAKYJnARLBbQbc=;\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=tuDN7Vl1Rytc43LR81PreXwl+hZiOQhUeq560JRlcFKOfiwBntUesEL6yWVps7iY6\n\t8BcaldXidRKqAGeoGd25Y70/1Q/sURHInF7axs6pDB6HZzewHVIW9AnugchxXUITsG\n\tQteVFaQubJKGTLs43W/b/eSxDD29H2WB8qUzgqLIVYKD0KDMEFO+xlaPefbEwA1cys\n\tS1FaCLyZKy2vw+omhbcsVdAWoVBXJwYKgyDk5LTlZK2yz2EurbduNm2OYKhR9Cw/DI\n\tMA7iJ7P81bgHpq2G8cZyOqnGAbzeaOCNx9z0WFvVSjgbIp8C/zOnIsvQ4jGBbo8zFA\n\t/RUE1uXNnvtrA==","Date":"Wed, 18 May 2022 21:26:28 +0200","To":"Paul Elder ","Message-ID":"<20220518192628.ypu4fdfmad7oup2t@uno.localdomain>","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220518134728.777709-2-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related 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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23717,"web_url":"https://patchwork.libcamera.org/comment/23717/","msgid":"","date":"2022-07-04T10:02:47","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Paul,\n\nThank you for your work. It's nice to see these changes to solidify the AE\ncontrols.\nI do have a few thoughts/comments below.\n\n\nOn Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> We have multiple goals:\n> - we need a lock of some sort, to instruct the AEGC to not update output\n> results\n> - we need manual modes, to override the values computed by the AEGC\n> - we need to support seamless transitions from auto -> manual, and do so\n> without flickering\n> - we need custom minimum values for the manual controls, that is no\n> magic values for enabling/disabling auto\n> - all of these need to be done with AE sub-controls (exposure time,\n> analogue gain)\n>\n> To achieve these goals, we introduce mode controls for the AE\n> sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto\n> state, and a disabled state. The disabled state has an internal one-way\n> state change from locked to manual, triggered by the presence of the\n> value-controls (ExposureTime and AnalogueGain).\n>\n> We then remove the AeEnable control, as it is a redundant control in the\n> face of these two mode controls.\n>\n> We also remove AeLocked, as it is insufficient for reporting the AE\n> state, and we promote AeState to non-draft to fill its role. Notably,\n> the locked state is removed, since this information can be obtained from\n> the aforementioned mode controls.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v4:\n> - remove FlashRequired and Precapture from AeState\n> - upgrade documentation of all the controls\n>\n> Changes in v3:\n> - improve wording of the control descriptions\n> - make more succinct and clear\n> - add description of how to do a flickerless transition\n>\n> Changes in v2:\n> - No changes, just resubmitting at the head of this series so that it's\n> together and so that /people will actually see it/\n>\n> Initial version:\n> Still RFC as I haven't updated the users of the control yet, and I want\n> to check that these are the controls and docs that we want.\n>\n> We've decided that the \"master AE control\" will be implemented by a\n> helper... but looking at uvcvideo and the V4L2 controls I'm wondering if\n> such helper should come earlier than later?\n>\n\nYes, I agree having the \"master AE control\" earlier will be beneficial for\napplication developers.\n\n\n> ---\n> src/libcamera/control_ids.yaml | 262 +++++++++++++++++++++++++--------\n> 1 file changed, 200 insertions(+), 62 deletions(-)\n>\n> diff --git a/src/libcamera/control_ids.yaml\n> b/src/libcamera/control_ids.yaml\n> index 9d4638ae..9f5ce5e8 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -7,23 +7,46 @@\n> # Unless otherwise stated, all controls are bi-directional, i.e. they can\n> be\n> # set through Request::controls() and returned out through\n> Request::metadata().\n> controls:\n> - - AeEnable:\n> - type: bool\n> + - AeState:\n> + type: int32_t\n> description: |\n> - Enable or disable the AE.\n> + Control to report the AE algorithm state associated with the\n> capture\n> + result.\n>\n> - \\sa ExposureTime AnalogueGain\n> + The state is still reported even if ExposureTimeMode or\n> + AnalogueGainMode is set to Disabled.\n>\n> - - AeLocked:\n> - type: bool\n> - description: |\n> - Report the lock status of a running AE algorithm.\n> + \\sa AnalogueGain\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTime\n> + \\sa ExposureTimeMode\n>\n> - If the AE algorithm is locked the value shall be set to true, if\n> it's\n> - converging it shall be set to false. If the AE algorithm is not\n> - running the control shall not be present in the metadata control\n> list.\n> + enum:\n> + - name: AeStateInactive\n> + value: 0\n> + description: |\n> + The AE algorithm is inactive.\n>\n> - \\sa AeEnable\n> + This state should be returned if both AnalogueGainMode and\n> + ExposureTimeMode are set to disabled (or one, if the camera\n> only\n> + supports one of the two controls).\n> + - name: AeStateSearching\n> + value: 1\n> + description: |\n> + The AE algorithm has not converged yet.\n> +\n> + This state should be returned if at least one of\n> AnalogueGainMode\n> + or ExposureTimeMode is set to auto, and the AE algorithm\n> hasn't\n> + converged yet. If the AE algorithm converges, the state shall\n> go to\n> + AeStateConverged.\n> + - name: AeStateConverged\n> + value: 2\n> + description: |\n> + The AE algorithm has converged.\n> +\n> + This state should be returned if at least one of\n> AnalogueGainMode\n> + or ExposureTimeMode is set to auto, and the AE algorithm has\n> + converged.\n>\n> # AeMeteringMode needs further attention:\n> # - Auto-generate max enum value.\n> @@ -93,6 +116,13 @@ controls:\n> how the desired total exposure is divided between the shutter time\n> and the sensor's analogue gain. The exposure modes are platform\n> specific, and not all exposure modes may be supported.\n> +\n> + When one of AnalogueGainMode or ExposureTimeMode is set to\n> Disabled,\n> + the fixed values will override any choices made by AeExposureMode.\n> +\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTimeMode\n> +\n> enum:\n> - name: ExposureNormal\n> value: 0\n> @@ -111,13 +141,15 @@ controls:\n> type: float\n> description: |\n> Specify an Exposure Value (EV) parameter. The EV parameter will\n> only be\n> - applied if the AE algorithm is currently enabled.\n> + applied if the AE algorithm is currently enabled, that is, at\n> least one\n> + of AnalogueGainMode and ExposureTimeMode are auto.\n>\n> By convention EV adjusts the exposure as log2. For example\n> EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment\n> of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n>\n> - \\sa AeEnable\n> + \\sa AnalogueGainMode\n> + \\sa ExposureTimeMode\n>\n> - ExposureTime:\n> type: int32_t\n> @@ -125,17 +157,85 @@ controls:\n> Exposure time (shutter speed) for the frame applied in the sensor\n> device. This value is specified in micro-seconds.\n>\n> - Setting this value means that it is now fixed and the AE\n> algorithm may\n> - not change it. Setting it back to zero returns it to the control\n> of the\n> - AE algorithm.\n> + This control will only take effect if ExposureTimeMode is\n> Disabled. If\n> + this control is set when ExposureTimeMode is Auto, the value will\n> be\n> + ignored and will not be retained.\n> +\n> + When reported in metadata, this control indicates what exposure\n> time\n> + was used for the current request, regardless of ExposureTimeMode.\n> + ExposureTimeMode will indicate the source of the exposure time\n> value,\n> + whether it came from the AE algorithm or not.\n> +\n> + \\sa AnalogueGain\n> + \\sa ExposureTimeMode\n> +\n> + - ExposureTimeMode:\n> + type: int32_t\n> + description: |\n> + Controls the source of the exposure time that is applied to the\n> image\n> + sensor. When set to Auto, the AE algorithm computes the exposure\n> time\n> + and configures the image sensor accordingly. When set to Disabled,\n> + exposure time specified in ExposureTime is applied to the image\n> sensor.\n> + If ExposureTime is not set, then the value last computed by the AE\n> + algorithm when the mode was Auto will be used.\n>\n\nCan we un-set ExposureTime? If it ever gets set once at any point in the\napplication,\nthen ExposureTimeModeDisabled will always use the last set value for\nExposureTime.\n\n\n> +\n> + If ExposureTime is not set and the mode is\n> ExposureTimeModeDisabled and\n> + AE was never Auto (either because the camera started in Disabled\n> mode,\n> + or Auto is not supported by the camera), the camera should use a\n> + best-effort default value.\n> +\n> + When ExposureTimeMode is set Auto, the value set in ExposureTime\n> is\n> + ignored and is not retained. This means that if ExposureTimeMode\n> is set\n> + to Disabled and ExposureTime is not also set, the exposure time\n> that\n> + was last computed by the AE algorithm while the mode was Auto\n> will be\n> + applied to the sensor.\n> +\n> + If ExposureTimeModeDisabled is supported, the ExposureTime\n> control must\n> + also be supported.\n> +\n> + The set of ExposureTimeMode modes that are supported by the\n> camera must\n> + have an intersection with the supported set of AnalogueGainMode\n> modes.\n>\n> - \\sa AnalogueGain AeEnable\n> + As it takes a few frames to apply the exposure time, there is a\n> period of\n> + time between submitting a request with ExposureTimeMode set to\n> Disabled\n> + and the exposure time component of the AE actually being disabled,\n> + during which the AE algorithm can still update the exposure time.\n> If an\n> + application is switching from automatic and manual control and\n> wishes\n> + to eliminate any flicker during the switch, the following\n> procedure is\n> + recommended.\n>\n\nI'm a bit confused by this bit to be honest. If a user switches\nExposureTimeMode from\nAuto to Disabled with the intention of setting a manual ExposureTime, how\ncan we ever\navoid a glitch in the brightness (unless we also change AnalogueGain\nappropriately)?\n\n\n> - \\todo Document the interactions between AeEnable and setting a\n> fixed\n> - value for this control. Consider interactions with other AE\n> features,\n> - such as aperture and aperture/shutter priority mode, and decide if\n> - control of which features should be automatically adjusted\n> shouldn't\n> - better be handled through a separate AE mode control.\n> + 1. Start with ExposureTimeMode set to Auto\n> +\n> + 2. Set ExposureTimeMode to Disabled\n> +\n> + 3. Wait for the first request to be output that has\n> ExposureTimeMode\n> + set to Disabled\n>\n\nHow would the application know this time point? Would the AE algorithm\nhave to\ncount frames once it has been given a ExposureTimeModeDisabled ctrl then\nreturn out the same in the metadata when it knows that it's last requested\nexposure\ntime change has been applied?\n\n\n> +\n> + 4. Copy the value reported in ExposureTime into a new request, and\n> + submit it\n> +\n> + 5. Proceed to run manual exposure time\n>\n\nAgain, I am unclear how this avoids glitches. Say the AE chooses an\nexposure\ntime of 33ms, then the user wants to switch to 15ms. There is always going\nto\nbe a jump in brightness. Perhaps my interpretation of this glitch is not\nthe same\nas what you are describing?\n\nDitto comments for the AnalogueGain changes.\n\nRegards,\nNaush\n\n\n> +\n> + \\sa ExposureTime\n> + enum:\n> + - name: ExposureTimeModeAuto\n> + value: 0\n> + description: |\n> + The exposure time will be calculated automatically and set by\n> the\n> + AE algorithm. If ExposureTime is set while this mode is\n> active, it\n> + will be ignored, and it will also not be retained.\n> + - name: ExposureTimeModeDisabled\n> + value: 1\n> + description: |\n> + The exposure time will not be updated by the AE algorithm. It\n> will\n> + come from the last calculated value when the mode was Auto,\n> or from\n> + the value specified in ExposureTime.\n> +\n> + When transitioning from Auto to Disabled mode, the last\n> computed\n> + exposure value is used until a new value is specified through\n> the\n> + ExposureTime control. If an ExposureTime value is specified\n> in the\n> + same request where the ExposureTimeMode is changed from Auto\n> to\n> + Disabled, the provided ExposureTime is applied.\n>\n> - AnalogueGain:\n> type: float\n> @@ -144,17 +244,85 @@ controls:\n> The value of the control specifies the gain multiplier applied to\n> all\n> colour channels. This value cannot be lower than 1.0.\n>\n> - Setting this value means that it is now fixed and the AE\n> algorithm may\n> - not change it. Setting it back to zero returns it to the control\n> of the\n> - AE algorithm.\n> + This control will only take effect if AnalogueGainMode is\n> Disabled. If\n> + this control is set when AnalogueGainMode is Auto, the value will\n> be\n> + ignored and will not be retained.\n> +\n> + When reported in metadata, this control indicates what analogue\n> gain\n> + was used for the current request, regardless of AnalogueGainMode.\n> + AnalogueGainMode will indicate the source of the analogue gain\n> value,\n> + whether it came from the AE algorithm or not.\n> +\n> + \\sa ExposureTime\n> + \\sa AnalogueGainMode\n> +\n> + - AnalogueGainMode:\n> + type: int32_t\n> + description: |\n> + Controls the source of the analogue gain that is applied to the\n> image\n> + sensor. When set to Auto, the AE algorithm computes the analogue\n> gain\n> + and configures the image sensor accordingly. When set to Disabled,\n> + analogue gain specified in AnalogueGain is applied to the image\n> sensor.\n> + If AnalogueGain is not set, then the value last computed by the AE\n> + algorithm when the mode was Auto will be used.\n> +\n> + If AnalogueGain is not set and the mode is\n> AnalogueGainModeDisabled and\n> + AE was never Auto (either because the camera started in Disabled\n> mode,\n> + or Auto is not supported by the camera), the camera should use a\n> + best-effort default value.\n> +\n> + When AnalogueGainMode is set Auto, the value set in AnalogueGain\n> is\n> + ignored and is not retained. This means that if AnalogueGainMode\n> is set\n> + to Disabled and AnalogueGain is not also set, the analogue gain\n> that\n> + was last computed by the AE algorithm while the mode was Auto\n> will be\n> + applied to the sensor.\n>\n> - \\sa ExposureTime AeEnable\n> + If AnalogueGainModeDisabled is supported, the AnalogueGain\n> control must\n> + also be supported.\n> +\n> + The set of AnalogueGainMode modes that are supported by the\n> camera must\n> + have an intersection with the supported set of ExposureTimeMode\n> modes.\n> +\n> + As it takes a few frames to apply the analogue gain, there is a\n> period of\n> + time between submitting a request with AnalogueGainMode set to\n> Disabled\n> + and the analogue gain component of the AE actually being disabled,\n> + during which the AE algorithm can still update the analogue gain.\n> If an\n> + application is switching from automatic and manual control and\n> wishes\n> + to eliminate any flicker during the switch, the following\n> procedure is\n> + recommended.\n> +\n> + 1. Start with AnalogueGainMode set to Auto\n> +\n> + 2. Set AnalogueGainMode to Disabled\n> +\n> + 3. Wait for the first request to be output that has\n> AnalogueGainMode\n> + set to Disabled\n> +\n> + 4. Copy the value reported in AnalogueGain into a new request, and\n> + submit it\n> +\n> + 5. Proceed to run manual analogue gain\n>\n+\n> + \\sa AnalogueGain\n> + enum:\n> + - name: AnalogueGainModeAuto\n> + value: 0\n> + description: |\n> + The analogue gain will be calculated automatically and set by\n> the\n> + AE algorithm. If AnalogueGain is set while this mode is\n> active, it\n> + will be ignored, and it will also not be retained.\n> + - name: AnalogueGainModeDisabled\n> + value: 1\n> + description: |\n> + The analogue gain will not be updated by the AE algorithm. It\n> will\n> + come from the last calculated value when the mode was Auto,\n> or from\n> + the value specified in AnalogueGain.\n>\n> - \\todo Document the interactions between AeEnable and setting a\n> fixed\n> - value for this control. Consider interactions with other AE\n> features,\n> - such as aperture and aperture/shutter priority mode, and decide if\n> - control of which features should be automatically adjusted\n> shouldn't\n> - better be handled through a separate AE mode control.\n> + When transitioning from Auto to Disabled mode the last\n> computed\n> + gain value is used until a new value is specified through the\n> + AnalogueGain control. If an AnalogueGain value is specified\n> in the\n> + same request where the AnalogueGainMode is set to Disabled,\n> the\n> + provided AnalogueGain is applied.\n>\n> - Brightness:\n> type: float\n> @@ -477,36 +645,6 @@ controls:\n> High quality aberration correction which might reduce the\n> frame\n> rate.\n>\n> - - AeState:\n> - type: int32_t\n> - draft: true\n> - description: |\n> - Control to report the current AE algorithm state. Currently\n> identical to\n> - ANDROID_CONTROL_AE_STATE.\n> -\n> - Current state of the AE algorithm.\n> - enum:\n> - - name: AeStateInactive\n> - value: 0\n> - description: The AE algorithm is inactive.\n> - - name: AeStateSearching\n> - value: 1\n> - description: The AE algorithm has not converged yet.\n> - - name: AeStateConverged\n> - value: 2\n> - description: The AE algorithm has converged.\n> - - name: AeStateLocked\n> - value: 3\n> - description: The AE algorithm is locked.\n> - - name: AeStateFlashRequired\n> - value: 4\n> - description: The AE algorithm would need a flash for good\n> results\n> - - name: AeStatePrecapture\n> - value: 5\n> - description: |\n> - The AE algorithm has started a pre-capture metering session.\n> - \\sa AePrecaptureTrigger\n> -\n> - AfState:\n> type: int32_t\n> draft: true\n> --\n> 2.30.2\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 D1166BD808\n\tfor ;\n\tMon, 4 Jul 2022 10:03:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 173806564A;\n\tMon, 4 Jul 2022 12:03:05 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB1CA61FB1\n\tfor ;\n\tMon, 4 Jul 2022 12:03:02 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id v9so10418098ljk.10\n\tfor ;\n\tMon, 04 Jul 2022 03:03:02 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656928985;\n\tbh=+yl/jF46/vA5goTT3EXp5sxXdeNMQ2tHREMsNmVAqaU=;\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=WwmpVzWqa9p6/zw/C0bpJekMU5PuQ6jwT7F5aIC1QCuSYvvW1NoK4JltxJjhRRlzB\n\tIUZyjfln95Iw4iBWD7oDu6bWU/5fcNPBEvgWlpkyeozI/98iSiKT6dJh4yUm+k1h6S\n\tQx+qY2X36LIl4qhbxIsrMQKp0XwvqTm1a4qlBNpw9Iy4di1vJngVYyOGZi0Bj0xfBX\n\tpDzndbLQZszNyb/wDMoOXtzw0KNC46vt8cdIHI84Og3jk8aWvKMLYGwd55TsouHCnh\n\t8682WBAKqu6FurhDgbo0LBFIO7WkFsHWIcFn2MAdv78rE6WNC6nULMSZZImUdr5mbC\n\tSY5zuclc+gmTQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=wh4XtzEktxC9ercWS0HvrXTOAQC4UN/XQf0jG2xjrYI=;\n\tb=KD0NZbbTgIYBWP30SyvC0a4GoEWilfUNruylUeeeiWkqxSVsPsjORbyVJh2S1P/2Th\n\tAWFqBq3pBmhhEzxtFOoSh6MZjnTjeX3gZ4rufz4FbUWR8t208Q7DxX1NLW3xBMf5dM4J\n\tMsQIMgoFW0QzRfmbM9coPrzJB7O3TlmCLejGovEKTrnlpRgri7ntxm093iDjoIyBUPu9\n\ti4yrgMaZXYOaw4U2zr/z3zvEyA2giekT4RFP0F5K+38EmpJlm1W22H2KZBWy4RNBZyeD\n\tf/X0d6UmGDLe+y4tNB1it0x0aFMJAzA9Mo68dokeP41fMULXzim1CbGV+iu8JRUHAfqV\n\tZDwA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"KD0NZbbT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=wh4XtzEktxC9ercWS0HvrXTOAQC4UN/XQf0jG2xjrYI=;\n\tb=1J6myGKAB/sHI3XWi6BteUIMvOZ3TFigwyWr2G/dJy8CZXTN59OIEZWhHeNUKfQyBE\n\t5MLFwSmEGl8WcQRT7EdyriLvbvlO1x5r55ZasmsekqjQAYivewOCugadlidze3seLCAR\n\tIHVZcvwWL8ue/vNNJff65TEzy2800bkRQgbAPVM/7K6ANCQ+zAzJTePLgFLhEJWNlIhu\n\tjXBQM6hLpu2lkNnuBMSHpibyDnA2R72JNcbe+g6lmlx3Q2PjeY72OUc9lMSepxC94+B1\n\tUP5Iy640ngKr2QPYyMIid25XY2Vl7rkODZYhQfZw/k5OcV6nl7onLfD4aL4n72VDSajU\n\tABzA==","X-Gm-Message-State":"AJIora9aemfYRUz7e/8K75YP/VNm7GbX6oI2fiLAYfCjNxa3gPh1VsEx\n\tsJqNKuv30GvBqPRp+CVRM8/DK6CtXP6QCc4O3aXjPCRMPVhIWdow","X-Google-Smtp-Source":"AGRyM1s5OMZ6xzThMDA+9v1qJYR8uKlak3oDRhGKRz1WxYRTl1DNn8MUN3QpBGYghh02pyK2vhbZkFlCLXNO3Bcrj08=","X-Received":"by 2002:a2e:b911:0:b0:25a:9942:4171 with SMTP id\n\tb17-20020a2eb911000000b0025a99424171mr15344833ljb.426.1656928981950;\n\tMon, 04 Jul 2022 03:03:01 -0700 (PDT)","MIME-Version":"1.0","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@ideasonboard.com>","In-Reply-To":"<20220518134728.777709-2-paul.elder@ideasonboard.com>","Date":"Mon, 4 Jul 2022 11:02:47 +0100","Message-ID":"","To":"Paul Elder ","Content-Type":"multipart/alternative; boundary=\"000000000000633baf05e2f7d78f\"","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related 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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23719,"web_url":"https://patchwork.libcamera.org/comment/23719/","msgid":"<20220704153024.xzolxogrxxxwrry4@uno.localdomain>","date":"2022-07-04T15:30:24","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n sorry Paul if I reply in your place, but I just got through this so\nI might have it slightly fresher\n\nOn Mon, Jul 04, 2022 at 11:02:47AM +0100, Naushir Patuck via libcamera-devel wrote:\n> Hi Paul,\n>\n> Thank you for your work. It's nice to see these changes to solidify the AE\n> controls.\n> I do have a few thoughts/comments below.\n\nCan I ask you to have a look at the final result with the fixups I\nsent in reply applied on top of this patch ?\n>\n>\n> On Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <\n> libcamera-devel@lists.libcamera.org> wrote:\n>\n> > We have multiple goals:\n> > - we need a lock of some sort, to instruct the AEGC to not update output\n> > results\n> > - we need manual modes, to override the values computed by the AEGC\n> > - we need to support seamless transitions from auto -> manual, and do so\n> > without flickering\n> > - we need custom minimum values for the manual controls, that is no\n> > magic values for enabling/disabling auto\n> > - all of these need to be done with AE sub-controls (exposure time,\n> > analogue gain)\n> >\n> > To achieve these goals, we introduce mode controls for the AE\n> > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an auto\n> > state, and a disabled state. The disabled state has an internal one-way\n> > state change from locked to manual, triggered by the presence of the\n> > value-controls (ExposureTime and AnalogueGain).\n> >\n> > We then remove the AeEnable control, as it is a redundant control in the\n> > face of these two mode controls.\n> >\n> > We also remove AeLocked, as it is insufficient for reporting the AE\n> > state, and we promote AeState to non-draft to fill its role. Notably,\n> > the locked state is removed, since this information can be obtained from\n> > the aforementioned mode controls.\n> >\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v4:\n> > - remove FlashRequired and Precapture from AeState\n> > - upgrade documentation of all the controls\n> >\n> > Changes in v3:\n> > - improve wording of the control descriptions\n> > - make more succinct and clear\n> > - add description of how to do a flickerless transition\n> >\n> > Changes in v2:\n> > - No changes, just resubmitting at the head of this series so that it's\n> > together and so that /people will actually see it/\n> >\n> > Initial version:\n> > Still RFC as I haven't updated the users of the control yet, and I want\n> > to check that these are the controls and docs that we want.\n> >\n> > We've decided that the \"master AE control\" will be implemented by a\n> > helper... but looking at uvcvideo and the V4L2 controls I'm wondering if\n> > such helper should come earlier than later?\n> >\n>\n> Yes, I agree having the \"master AE control\" earlier will be beneficial for\n> application developers.\n>\n\nDo you envision this as something that could be part of your\napplications in example, or in a layer part of libcamera itself ?\n\n>\n> > ---\n> > src/libcamera/control_ids.yaml | 262 +++++++++++++++++++++++++--------\n> > 1 file changed, 200 insertions(+), 62 deletions(-)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml\n> > b/src/libcamera/control_ids.yaml\n> > index 9d4638ae..9f5ce5e8 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -7,23 +7,46 @@\n> > # Unless otherwise stated, all controls are bi-directional, i.e. they can\n> > be\n> > # set through Request::controls() and returned out through\n> > Request::metadata().\n> > controls:\n> > - - AeEnable:\n> > - type: bool\n> > + - AeState:\n> > + type: int32_t\n> > description: |\n> > - Enable or disable the AE.\n> > + Control to report the AE algorithm state associated with the\n> > capture\n> > + result.\n> >\n> > - \\sa ExposureTime AnalogueGain\n> > + The state is still reported even if ExposureTimeMode or\n> > + AnalogueGainMode is set to Disabled.\n> >\n> > - - AeLocked:\n> > - type: bool\n> > - description: |\n> > - Report the lock status of a running AE algorithm.\n> > + \\sa AnalogueGain\n> > + \\sa AnalogueGainMode\n> > + \\sa ExposureTime\n> > + \\sa ExposureTimeMode\n> >\n> > - If the AE algorithm is locked the value shall be set to true, if\n> > it's\n> > - converging it shall be set to false. If the AE algorithm is not\n> > - running the control shall not be present in the metadata control\n> > list.\n> > + enum:\n> > + - name: AeStateInactive\n> > + value: 0\n> > + description: |\n> > + The AE algorithm is inactive.\n> >\n> > - \\sa AeEnable\n> > + This state should be returned if both AnalogueGainMode and\n> > + ExposureTimeMode are set to disabled (or one, if the camera\n> > only\n> > + supports one of the two controls).\n> > + - name: AeStateSearching\n> > + value: 1\n> > + description: |\n> > + The AE algorithm has not converged yet.\n> > +\n> > + This state should be returned if at least one of\n> > AnalogueGainMode\n> > + or ExposureTimeMode is set to auto, and the AE algorithm\n> > hasn't\n> > + converged yet. If the AE algorithm converges, the state shall\n> > go to\n> > + AeStateConverged.\n> > + - name: AeStateConverged\n> > + value: 2\n> > + description: |\n> > + The AE algorithm has converged.\n> > +\n> > + This state should be returned if at least one of\n> > AnalogueGainMode\n> > + or ExposureTimeMode is set to auto, and the AE algorithm has\n> > + converged.\n> >\n> > # AeMeteringMode needs further attention:\n> > # - Auto-generate max enum value.\n> > @@ -93,6 +116,13 @@ controls:\n> > how the desired total exposure is divided between the shutter time\n> > and the sensor's analogue gain. The exposure modes are platform\n> > specific, and not all exposure modes may be supported.\n> > +\n> > + When one of AnalogueGainMode or ExposureTimeMode is set to\n> > Disabled,\n> > + the fixed values will override any choices made by AeExposureMode.\n> > +\n> > + \\sa AnalogueGainMode\n> > + \\sa ExposureTimeMode\n> > +\n> > enum:\n> > - name: ExposureNormal\n> > value: 0\n> > @@ -111,13 +141,15 @@ controls:\n> > type: float\n> > description: |\n> > Specify an Exposure Value (EV) parameter. The EV parameter will\n> > only be\n> > - applied if the AE algorithm is currently enabled.\n> > + applied if the AE algorithm is currently enabled, that is, at\n> > least one\n> > + of AnalogueGainMode and ExposureTimeMode are auto.\n> >\n> > By convention EV adjusts the exposure as log2. For example\n> > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure adjustment\n> > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n> >\n> > - \\sa AeEnable\n> > + \\sa AnalogueGainMode\n> > + \\sa ExposureTimeMode\n> >\n> > - ExposureTime:\n> > type: int32_t\n> > @@ -125,17 +157,85 @@ controls:\n> > Exposure time (shutter speed) for the frame applied in the sensor\n> > device. This value is specified in micro-seconds.\n> >\n> > - Setting this value means that it is now fixed and the AE\n> > algorithm may\n> > - not change it. Setting it back to zero returns it to the control\n> > of the\n> > - AE algorithm.\n> > + This control will only take effect if ExposureTimeMode is\n> > Disabled. If\n> > + this control is set when ExposureTimeMode is Auto, the value will\n> > be\n> > + ignored and will not be retained.\n> > +\n> > + When reported in metadata, this control indicates what exposure\n> > time\n> > + was used for the current request, regardless of ExposureTimeMode.\n> > + ExposureTimeMode will indicate the source of the exposure time\n> > value,\n> > + whether it came from the AE algorithm or not.\n> > +\n> > + \\sa AnalogueGain\n> > + \\sa ExposureTimeMode\n> > +\n> > + - ExposureTimeMode:\n> > + type: int32_t\n> > + description: |\n> > + Controls the source of the exposure time that is applied to the\n> > image\n> > + sensor. When set to Auto, the AE algorithm computes the exposure\n> > time\n> > + and configures the image sensor accordingly. When set to Disabled,\n> > + exposure time specified in ExposureTime is applied to the image\n> > sensor.\n> > + If ExposureTime is not set, then the value last computed by the AE\n> > + algorithm when the mode was Auto will be used.\n> >\n>\n> Can we un-set ExposureTime? If it ever gets set once at any point in the\n> application,\n> then ExposureTimeModeDisabled will always use the last set value for\n> ExposureTime.\n>\n\nIf I interpret your question right, are you wondering if the\nExposureTime value is retained if an application sends it when the\nAEGC is actually in auto mode (and so the ExposureTime from application\nis not applied) ?\n\nWe discussed this, and I think Paul tried to clarify it in the\nExposureTime documentation by saying:\n\n - name: ExposureTimeModeDisabled\n If ExposureTime is set while this mode is active, it\n will be ignored, and it will also not be retained.\n\nwhich means that by design, the ExposureTime is just ignored if sent\nwhen the AEGC is in auto mode.\n\nDo you think that's not the expected behaviour ?\n\n>\n> > +\n> > + If ExposureTime is not set and the mode is\n> > ExposureTimeModeDisabled and\n> > + AE was never Auto (either because the camera started in Disabled\n> > mode,\n> > + or Auto is not supported by the camera), the camera should use a\n> > + best-effort default value.\n> > +\n> > + When ExposureTimeMode is set Auto, the value set in ExposureTime\n> > is\n> > + ignored and is not retained. This means that if ExposureTimeMode\n> > is set\n> > + to Disabled and ExposureTime is not also set, the exposure time\n> > that\n> > + was last computed by the AE algorithm while the mode was Auto\n> > will be\n> > + applied to the sensor.\n> > +\n> > + If ExposureTimeModeDisabled is supported, the ExposureTime\n> > control must\n> > + also be supported.\n> > +\n> > + The set of ExposureTimeMode modes that are supported by the\n> > camera must\n> > + have an intersection with the supported set of AnalogueGainMode\n> > modes.\n> >\n> > - \\sa AnalogueGain AeEnable\n> > + As it takes a few frames to apply the exposure time, there is a\n> > period of\n> > + time between submitting a request with ExposureTimeMode set to\n> > Disabled\n> > + and the exposure time component of the AE actually being disabled,\n> > + during which the AE algorithm can still update the exposure time.\n> > If an\n> > + application is switching from automatic and manual control and\n> > wishes\n> > + to eliminate any flicker during the switch, the following\n> > procedure is\n> > + recommended.\n> >\n>\n> I'm a bit confused by this bit to be honest. If a user switches\n> ExposureTimeMode from\n> Auto to Disabled with the intention of setting a manual ExposureTime, how\n> can we ever\n> avoid a glitch in the brightness (unless we also change AnalogueGain\n> appropriately)?\n>\n\nSee below\n\n>\n> > - \\todo Document the interactions between AeEnable and setting a\n> > fixed\n> > - value for this control. Consider interactions with other AE\n> > features,\n> > - such as aperture and aperture/shutter priority mode, and decide if\n> > - control of which features should be automatically adjusted\n> > shouldn't\n> > - better be handled through a separate AE mode control.\n> > + 1. Start with ExposureTimeMode set to Auto\n> > +\n> > + 2. Set ExposureTimeMode to Disabled\n> > +\n> > + 3. Wait for the first request to be output that has\n> > ExposureTimeMode\n> > + set to Disabled\n> >\n>\n> How would the application know this time point? Would the AE algorithm\n> have to\n> count frames once it has been given a ExposureTimeModeDisabled ctrl then\n> return out the same in the metadata when it knows that it's last requested\n> exposure\n> time change has been applied?\n>\n\nNot sure this is going to answer your question, but let's start by\ndefining what a \"glitch\" is for us in this definition. I think it's\nuseful to validate our understanding against your experience of providing\nthis features to the vast number of users you have.\n\nThe idea is that applications willing to control the exposure time\nexplicitly might want to do so by minimizing the difference between\nthe last value computed by the AEGC algorithm and their newly set\nvalue, to avoid a sudden change in exposure and gain which result in a\nvisible \"glitch\". En example, suddenly moving the exposure time and\ngain to the opposite of the spectrum of what the AEGC was computing\nwill result in images going very bright or very dark in just a few\nframes.\n\nThe way to implement a smooth transition is to start from the values\nlastly computed by the AEGC (as available in metadata) and then \"slowly\"\nmoving towards the desired manual value. Of course this is not\nmandatory, application might desire a sudden change of exposure, or simply\nwon't care about smooth transitions. If they do, however, they have to\nconsider that there will always be a number of requests in the queue\nthat will be processed by the camera before the one with\n\"ExposureTimeDisabled\" gets to be processed.\n\nDuring the processing of those requests in the queue, the AEGC will still\nbe active and might still change the exposure time (and gain) to values quite\ndifferent from the ones visible at the application at the time it\nqueued the request with \"ExposureTimeModeDisabled\".\n\nThe steps proposed here suggest to applications to wait until the\n\"ExposureTimeModeDisabled\" request is returned and the AEGC is\nactually off. From the definitions we gave here, this mean the\nexposure time (and gain) won't be updated by the now inactive AEGC\nblock until an \"ExposureTime\" value is submitted by applications (more\nor less like your agc::pause()/resume() work, if I recall correctly).\n\nWhen the \"ExposureTimeModeDisabled\" request has completed,\napplications know that the exposure time won't be updated from that\npoint on, and can use the ExposureTime and AnalogueGain metadata values\nas a \"stable\" starting point for their values.\n\nDoes this make sense to you ?\n\nIn my fixups I proposed a rework of the introduction section of this\npart, could you have a look to see if that's more clear ?\n\n>\n> > +\n> > + 4. Copy the value reported in ExposureTime into a new request, and\n> > + submit it\n> > +\n> > + 5. Proceed to run manual exposure time\n> >\n>\n> Again, I am unclear how this avoids glitches. Say the AE chooses an\n> exposure\n> time of 33ms, then the user wants to switch to 15ms. There is always going\n> to\n> be a jump in brightness. Perhaps my interpretation of this glitch is not\n> the same\n> as what you are describing?\n\nIf an application decides not to care and halves the exposure time\nfrom one request to the following one, the above procedure is useless\nindeed.\n\nBut as explained above, an application might want to approach 15ms more\nsmoothly and the above text suggests how to do so.\n\nI feel like this is mostly directed to applications that wants to\ndrive the AEGC with some sort of algorithm instead of application\nthat simply take a value in from users and apply it directly. In\nthis latter case the values input from the user might very well be 1ms\nhence approaching it slowly might not even be desired.\n\nThanks for commenting!\n\n>\n> Ditto comments for the AnalogueGain changes.\n>\n> Regards,\n> Naush\n>\n>\n> > +\n> > + \\sa ExposureTime\n> > + enum:\n> > + - name: ExposureTimeModeAuto\n> > + value: 0\n> > + description: |\n> > + The exposure time will be calculated automatically and set by\n> > the\n> > + AE algorithm. If ExposureTime is set while this mode is\n> > active, it\n> > + will be ignored, and it will also not be retained.\n> > + - name: ExposureTimeModeDisabled\n> > + value: 1\n> > + description: |\n> > + The exposure time will not be updated by the AE algorithm. It\n> > will\n> > + come from the last calculated value when the mode was Auto,\n> > or from\n> > + the value specified in ExposureTime.\n> > +\n> > + When transitioning from Auto to Disabled mode, the last\n> > computed\n> > + exposure value is used until a new value is specified through\n> > the\n> > + ExposureTime control. If an ExposureTime value is specified\n> > in the\n> > + same request where the ExposureTimeMode is changed from Auto\n> > to\n> > + Disabled, the provided ExposureTime is applied.\n> >\n> > - AnalogueGain:\n> > type: float\n> > @@ -144,17 +244,85 @@ controls:\n> > The value of the control specifies the gain multiplier applied to\n> > all\n> > colour channels. This value cannot be lower than 1.0.\n> >\n> > - Setting this value means that it is now fixed and the AE\n> > algorithm may\n> > - not change it. Setting it back to zero returns it to the control\n> > of the\n> > - AE algorithm.\n> > + This control will only take effect if AnalogueGainMode is\n> > Disabled. If\n> > + this control is set when AnalogueGainMode is Auto, the value will\n> > be\n> > + ignored and will not be retained.\n> > +\n> > + When reported in metadata, this control indicates what analogue\n> > gain\n> > + was used for the current request, regardless of AnalogueGainMode.\n> > + AnalogueGainMode will indicate the source of the analogue gain\n> > value,\n> > + whether it came from the AE algorithm or not.\n> > +\n> > + \\sa ExposureTime\n> > + \\sa AnalogueGainMode\n> > +\n> > + - AnalogueGainMode:\n> > + type: int32_t\n> > + description: |\n> > + Controls the source of the analogue gain that is applied to the\n> > image\n> > + sensor. When set to Auto, the AE algorithm computes the analogue\n> > gain\n> > + and configures the image sensor accordingly. When set to Disabled,\n> > + analogue gain specified in AnalogueGain is applied to the image\n> > sensor.\n> > + If AnalogueGain is not set, then the value last computed by the AE\n> > + algorithm when the mode was Auto will be used.\n> > +\n> > + If AnalogueGain is not set and the mode is\n> > AnalogueGainModeDisabled and\n> > + AE was never Auto (either because the camera started in Disabled\n> > mode,\n> > + or Auto is not supported by the camera), the camera should use a\n> > + best-effort default value.\n> > +\n> > + When AnalogueGainMode is set Auto, the value set in AnalogueGain\n> > is\n> > + ignored and is not retained. This means that if AnalogueGainMode\n> > is set\n> > + to Disabled and AnalogueGain is not also set, the analogue gain\n> > that\n> > + was last computed by the AE algorithm while the mode was Auto\n> > will be\n> > + applied to the sensor.\n> >\n> > - \\sa ExposureTime AeEnable\n> > + If AnalogueGainModeDisabled is supported, the AnalogueGain\n> > control must\n> > + also be supported.\n> > +\n> > + The set of AnalogueGainMode modes that are supported by the\n> > camera must\n> > + have an intersection with the supported set of ExposureTimeMode\n> > modes.\n> > +\n> > + As it takes a few frames to apply the analogue gain, there is a\n> > period of\n> > + time between submitting a request with AnalogueGainMode set to\n> > Disabled\n> > + and the analogue gain component of the AE actually being disabled,\n> > + during which the AE algorithm can still update the analogue gain.\n> > If an\n> > + application is switching from automatic and manual control and\n> > wishes\n> > + to eliminate any flicker during the switch, the following\n> > procedure is\n> > + recommended.\n> > +\n> > + 1. Start with AnalogueGainMode set to Auto\n> > +\n> > + 2. Set AnalogueGainMode to Disabled\n> > +\n> > + 3. Wait for the first request to be output that has\n> > AnalogueGainMode\n> > + set to Disabled\n> > +\n> > + 4. Copy the value reported in AnalogueGain into a new request, and\n> > + submit it\n> > +\n> > + 5. Proceed to run manual analogue gain\n> >\n> +\n> > + \\sa AnalogueGain\n> > + enum:\n> > + - name: AnalogueGainModeAuto\n> > + value: 0\n> > + description: |\n> > + The analogue gain will be calculated automatically and set by\n> > the\n> > + AE algorithm. If AnalogueGain is set while this mode is\n> > active, it\n> > + will be ignored, and it will also not be retained.\n> > + - name: AnalogueGainModeDisabled\n> > + value: 1\n> > + description: |\n> > + The analogue gain will not be updated by the AE algorithm. It\n> > will\n> > + come from the last calculated value when the mode was Auto,\n> > or from\n> > + the value specified in AnalogueGain.\n> >\n> > - \\todo Document the interactions between AeEnable and setting a\n> > fixed\n> > - value for this control. Consider interactions with other AE\n> > features,\n> > - such as aperture and aperture/shutter priority mode, and decide if\n> > - control of which features should be automatically adjusted\n> > shouldn't\n> > - better be handled through a separate AE mode control.\n> > + When transitioning from Auto to Disabled mode the last\n> > computed\n> > + gain value is used until a new value is specified through the\n> > + AnalogueGain control. If an AnalogueGain value is specified\n> > in the\n> > + same request where the AnalogueGainMode is set to Disabled,\n> > the\n> > + provided AnalogueGain is applied.\n> >\n> > - Brightness:\n> > type: float\n> > @@ -477,36 +645,6 @@ controls:\n> > High quality aberration correction which might reduce the\n> > frame\n> > rate.\n> >\n> > - - AeState:\n> > - type: int32_t\n> > - draft: true\n> > - description: |\n> > - Control to report the current AE algorithm state. Currently\n> > identical to\n> > - ANDROID_CONTROL_AE_STATE.\n> > -\n> > - Current state of the AE algorithm.\n> > - enum:\n> > - - name: AeStateInactive\n> > - value: 0\n> > - description: The AE algorithm is inactive.\n> > - - name: AeStateSearching\n> > - value: 1\n> > - description: The AE algorithm has not converged yet.\n> > - - name: AeStateConverged\n> > - value: 2\n> > - description: The AE algorithm has converged.\n> > - - name: AeStateLocked\n> > - value: 3\n> > - description: The AE algorithm is locked.\n> > - - name: AeStateFlashRequired\n> > - value: 4\n> > - description: The AE algorithm would need a flash for good\n> > results\n> > - - name: AeStatePrecapture\n> > - value: 5\n> > - description: |\n> > - The AE algorithm has started a pre-capture metering session.\n> > - \\sa AePrecaptureTrigger\n> > -\n> > - AfState:\n> > type: int32_t\n> > draft: true\n> > --\n> > 2.30.2\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 6BE5FBE173\n\tfor ;\n\tMon, 4 Jul 2022 15:30:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B75BF6564E;\n\tMon, 4 Jul 2022 17:30:28 +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 D1FBC61FB1\n\tfor ;\n\tMon, 4 Jul 2022 17:30:26 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 163EDE0007;\n\tMon, 4 Jul 2022 15:30:25 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656948628;\n\tbh=5bkc38mAcgfwRsjU4Ujk2mTlswpJrnXxEBXxf4kuw08=;\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=fwubdgHQRZ8E0cO8TmEqYPxqc0wtyq724H04Dtuo5GlyH2BzWFlhMcThlSDfzAusa\n\tOSMAF9WlSGY1n/thmrRy2c2IEIs7ZVSvxlxxt3TKbeQ02kqG8DucnqB8az58IEFGrN\n\t3FhqSHDT5xSaGAQvK3KC87PJnWVC60E8HHIPhWCGfAh33aKoPlbXPcfrjBneQjyctw\n\tTAm3n9HU3rYBhg56Ftuydyit3o4M7+G48iStcLzk5IsSWUUBpAbeAelJvxkqL0l0jm\n\ty0XzkFCL2pEhBN/iriCRcKX/ruaVroNIKMGgsGDguTCmQgrMz8i5HxlrW6YYfX+Lyp\n\t/2+JK4rimbd5w==","Date":"Mon, 4 Jul 2022 17:30:24 +0200","To":"Naushir Patuck ","Message-ID":"<20220704153024.xzolxogrxxxwrry4@uno.localdomain>","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@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 v4 1/3] controls: Reorganize the\n\tAE-related 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\" "}},{"id":23736,"web_url":"https://patchwork.libcamera.org/comment/23736/","msgid":"","date":"2022-07-05T08:06:39","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo and Paul,\n\nOn Mon, 4 Jul 2022 at 16:30, Jacopo Mondi wrote:\n>\n> Hi Naush,\n> sorry Paul if I reply in your place, but I just got through this so\n> I might have it slightly fresher\n>\n> On Mon, Jul 04, 2022 at 11:02:47AM +0100, Naushir Patuck via\nlibcamera-devel wrote:\n> > Hi Paul,\n> >\n> > Thank you for your work. It's nice to see these changes to solidify\nthe AE\n> > controls.\n> > I do have a few thoughts/comments below.\n>\n> Can I ask you to have a look at the final result with the fixups I\n> sent in reply applied on top of this patch ?\n\n\nYes will do!\n\n\n>\n> >\n> >\n> > On Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <\n> > libcamera-devel@lists.libcamera.org> wrote:\n> >\n> > > We have multiple goals:\n> > > - we need a lock of some sort, to instruct the AEGC to not update\noutput\n> > > results\n> > > - we need manual modes, to override the values computed by the AEGC\n>\n> > > - we need to support seamless transitions from auto -> manual, and do\nso\n> > > without flickering\n> > > - we need custom minimum values for the manual controls, that is no\n> > > magic values for enabling/disabling auto\n> > > - all of these need to be done with AE sub-controls (exposure time,\n> > > analogue gain)\n> > >\n> > > To achieve these goals, we introduce mode controls for the AE\n> > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an\nauto\n> > > state, and a disabled state. The disabled state has an internal\none-way\n> > > state change from locked to manual, triggered by the presence of the\n> > > value-controls (ExposureTime and AnalogueGain).\n> > >\n> > > We then remove the AeEnable control, as it is a redundant control in\nthe\n> > > face of these two mode controls.\n> > >\n> > > We also remove AeLocked, as it is insufficient for reporting the AE\n> > > state, and we promote AeState to non-draft to fill its role. Notably,\n> > > the locked state is removed, since this information can be obtained\nfrom\n> > > the aforementioned mode controls.\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> > > Signed-off-by: Paul Elder \n> > >\n> > > ---\n> > > Changes in v4:\n> > > - remove FlashRequired and Precapture from AeState\n> > > - upgrade documentation of all the controls\n> > >\n> > > Changes in v3:\n> > > - improve wording of the control descriptions\n> > > - make more succinct and clear\n> > > - add description of how to do a flickerless transition\n> > >\n> > > Changes in v2:\n> > > - No changes, just resubmitting at the head of this series so that\nit's\n> > > together and so that /people will actually see it/\n> > >\n> > > Initial version:\n> > > Still RFC as I haven't updated the users of the control yet, and I\nwant\n> > > to check that these are the controls and docs that we want.\n> > >\n> > > We've decided that the \"master AE control\" will be implemented by a\n> > > helper... but looking at uvcvideo and the V4L2 controls I'm wondering\nif\n> > > such helper should come earlier than later?\n> > >\n> >\n> > Yes, I agree having the \"master AE control\" earlier will be beneficial\nfor\n> > application developers.\n> >\n>\n> Do you envision this as something that could be part of your\n> applications in example, or in a layer part of libcamera itself ?\n\n\nMy preference would be to have a helper in libcamera do this if possible.\nThis way, applications don't need to duplicate code to set all AE controls.\nBut either way, it's not too big of a deal to put it in the application.\n\n>\n>\n> >\n> > > ---\n> > > src/libcamera/control_ids.yaml | 262\n+++++++++++++++++++++++++--------\n> > > 1 file changed, 200 insertions(+), 62 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/control_ids.yaml\n> > > b/src/libcamera/control_ids.yaml\n> > > index 9d4638ae..9f5ce5e8 100644\n> > > --- a/src/libcamera/control_ids.yaml\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -7,23 +7,46 @@\n> > > # Unless otherwise stated, all controls are bi-directional, i.e.\nthey can\n> > > be\n> > > # set through Request::controls() and returned out through\n> > > Request::metadata().\n> > > controls:\n> > > - - AeEnable:\n> > > - type: bool\n> > > + - AeState:\n> > > + type: int32_t\n> > > description: |\n> > > - Enable or disable the AE.\n> > > + Control to report the AE algorithm state associated with the\n> > > capture\n> > > + result.\n> > >\n> > > - \\sa ExposureTime AnalogueGain\n> > > + The state is still reported even if ExposureTimeMode or\n> > > + AnalogueGainMode is set to Disabled.\n> > >\n> > > - - AeLocked:\n> > > - type: bool\n> > > - description: |\n> > > - Report the lock status of a running AE algorithm.\n> > > + \\sa AnalogueGain\n> > > + \\sa AnalogueGainMode\n> > > + \\sa ExposureTime\n> > > + \\sa ExposureTimeMode\n> > >\n> > > - If the AE algorithm is locked the value shall be set to\ntrue, if\n> > > it's\n> > > - converging it shall be set to false. If the AE algorithm is\nnot\n> > > - running the control shall not be present in the metadata\ncontrol\n> > > list.\n> > > + enum:\n> > > + - name: AeStateInactive\n> > > + value: 0\n> > > + description: |\n> > > + The AE algorithm is inactive.\n> > >\n> > > - \\sa AeEnable\n> > > + This state should be returned if both AnalogueGainMode\nand\n> > > + ExposureTimeMode are set to disabled (or one, if the\ncamera\n> > > only\n> > > + supports one of the two controls).\n> > > + - name: AeStateSearching\n> > > + value: 1\n> > > + description: |\n> > > + The AE algorithm has not converged yet.\n> > > +\n> > > + This state should be returned if at least one of\n> > > AnalogueGainMode\n> > > + or ExposureTimeMode is set to auto, and the AE algorithm\n> > > hasn't\n> > > + converged yet. If the AE algorithm converges, the state\nshall\n> > > go to\n> > > + AeStateConverged.\n> > > + - name: AeStateConverged\n> > > + value: 2\n> > > + description: |\n> > > + The AE algorithm has converged.\n> > > +\n> > > + This state should be returned if at least one of\n> > > AnalogueGainMode\n> > > + or ExposureTimeMode is set to auto, and the AE algorithm\nhas\n> > > + converged.\n> > >\n> > > # AeMeteringMode needs further attention:\n> > > # - Auto-generate max enum value.\n> > > @@ -93,6 +116,13 @@ controls:\n> > > how the desired total exposure is divided between the\nshutter time\n> > > and the sensor's analogue gain. The exposure modes are\nplatform\n> > > specific, and not all exposure modes may be supported.\n> > > +\n> > > + When one of AnalogueGainMode or ExposureTimeMode is set to\n> > > Disabled,\n> > > + the fixed values will override any choices made by\nAeExposureMode.\n> > > +\n> > > + \\sa AnalogueGainMode\n> > > + \\sa ExposureTimeMode\n> > > +\n> > > enum:\n> > > - name: ExposureNormal\n> > > value: 0\n> > > @@ -111,13 +141,15 @@ controls:\n> > > type: float\n> > > description: |\n> > > Specify an Exposure Value (EV) parameter. The EV parameter\nwill\n> > > only be\n> > > - applied if the AE algorithm is currently enabled.\n> > > + applied if the AE algorithm is currently enabled, that is, at\n> > > least one\n> > > + of AnalogueGainMode and ExposureTimeMode are auto.\n> > >\n> > > By convention EV adjusts the exposure as log2. For example\n> > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure\nadjustment\n> > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n> > >\n> > > - \\sa AeEnable\n> > > + \\sa AnalogueGainMode\n> > > + \\sa ExposureTimeMode\n> > >\n> > > - ExposureTime:\n> > > type: int32_t\n> > > @@ -125,17 +157,85 @@ controls:\n> > > Exposure time (shutter speed) for the frame applied in the\nsensor\n> > > device. This value is specified in micro-seconds.\n> > >\n> > > - Setting this value means that it is now fixed and the AE\n> > > algorithm may\n> > > - not change it. Setting it back to zero returns it to the\ncontrol\n> > > of the\n> > > - AE algorithm.\n> > > + This control will only take effect if ExposureTimeMode is\n> > > Disabled. If\n> > > + this control is set when ExposureTimeMode is Auto, the value\nwill\n> > > be\n> > > + ignored and will not be retained.\n> > > +\n> > > + When reported in metadata, this control indicates what\nexposure\n> > > time\n> > > + was used for the current request, regardless of\nExposureTimeMode.\n> > > + ExposureTimeMode will indicate the source of the exposure\ntime\n> > > value,\n> > > + whether it came from the AE algorithm or not.\n> > > +\n> > > + \\sa AnalogueGain\n> > > + \\sa ExposureTimeMode\n> > > +\n> > > + - ExposureTimeMode:\n> > > + type: int32_t\n> > > + description: |\n> > > + Controls the source of the exposure time that is applied to\nthe\n> > > image\n> > > + sensor. When set to Auto, the AE algorithm computes the\nexposure\n> > > time\n> > > + and configures the image sensor accordingly. When set to\nDisabled,\n> > > + exposure time specified in ExposureTime is applied to the\nimage\n> > > sensor.\n> > > + If ExposureTime is not set, then the value last computed by\nthe AE\n> > > + algorithm when the mode was Auto will be used.\n> > >\n> >\n> > Can we un-set ExposureTime? If it ever gets set once at any point in\nthe\n> > application,\n> > then ExposureTimeModeDisabled will always use the last set value for\n> > ExposureTime.\n> >\n>\n> If I interpret your question right, are you wondering if the\n> ExposureTime value is retained if an application sends it when the\n> AEGC is actually in auto mode (and so the ExposureTime from application\n> is not applied) ?\n>\n> We discussed this, and I think Paul tried to clarify it in the\n> ExposureTime documentation by saying:\n>\n> - name: ExposureTimeModeDisabled\n> If ExposureTime is set while this mode is active, it\n> will be ignored, and it will also not be retained.\n>\n> which means that by design, the ExposureTime is just ignored if sent\n> when the AEGC is in auto mode.\n>\n> Do you think that's not the expected behaviour ?\n\n\nNot exactly... I was considering the following sequence:\n\n1) ExposureTimeMode is Auto - AE adjusts exposure time as needed.\n2) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.\nNo ExposureTime value set yet, so keep the last AE exposure time.\n3) Set ExposureTime to some desired value.\n4) ExposureTimeMode set to Auto - AE adjusts exposure time as needed again.\n5) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.\n\nIn step 5, does the IPA switch back to the ExposureTime set in step 3, or\ndoes\nsetting ExposureTimeMode to Auto invalidate the existing ExposureTime\nvalue? If\nthe ExposureTime value is still valid, should we consider a way to\ninvalidate\n(unset) it if the application wanted to just use the AE selected exposure\ntime\nat step 5?\n\n>\n>\n> >\n> > > +\n> > > + If ExposureTime is not set and the mode is\n> > > ExposureTimeModeDisabled and\n> > > + AE was never Auto (either because the camera started in\nDisabled\n> > > mode,\n> > > + or Auto is not supported by the camera), the camera should\nuse a\n> > > + best-effort default value.\n> > > +\n> > > + When ExposureTimeMode is set Auto, the value set in\nExposureTime\n> > > is\n> > > + ignored and is not retained. This means that if\nExposureTimeMode\n> > > is set\n> > > + to Disabled and ExposureTime is not also set, the exposure\ntime\n> > > that\n> > > + was last computed by the AE algorithm while the mode was Auto\n> > > will be\n> > > + applied to the sensor.\n> > > +\n> > > + If ExposureTimeModeDisabled is supported, the ExposureTime\n> > > control must\n> > > + also be supported.\n> > > +\n> > > + The set of ExposureTimeMode modes that are supported by the\n> > > camera must\n> > > + have an intersection with the supported set of\nAnalogueGainMode\n> > > modes.\n> > >\n> > > - \\sa AnalogueGain AeEnable\n> > > + As it takes a few frames to apply the exposure time, there\nis a\n> > > period of\n> > > + time between submitting a request with ExposureTimeMode set\nto\n> > > Disabled\n> > > + and the exposure time component of the AE actually being\ndisabled,\n> > > + during which the AE algorithm can still update the exposure\ntime.\n> > > If an\n> > > + application is switching from automatic and manual control\nand\n> > > wishes\n> > > + to eliminate any flicker during the switch, the following\n> > > procedure is\n> > > + recommended.\n> > >\n> >\n> > I'm a bit confused by this bit to be honest. If a user switches\n> > ExposureTimeMode from\n> > Auto to Disabled with the intention of setting a manual ExposureTime,\nhow\n> > can we ever\n> > avoid a glitch in the brightness (unless we also change AnalogueGain\n> > appropriately)?\n> >\n>\n> See below\n>\n> >\n> > > - \\todo Document the interactions between AeEnable and setting\na\n> > > fixed\n> > > - value for this control. Consider interactions with other AE\n> > > features,\n> > > - such as aperture and aperture/shutter priority mode, and\ndecide if\n> > > - control of which features should be automatically adjusted\n> > > shouldn't\n> > > - better be handled through a separate AE mode control.\n> > > + 1. Start with ExposureTimeMode set to Auto\n> > > +\n> > > + 2. Set ExposureTimeMode to Disabled\n> > > +\n> > > + 3. Wait for the first request to be output that has\n> > > ExposureTimeMode\n> > > + set to Disabled\n> > >\n> >\n> > How would the application know this time point? Would the AE algorithm\n> > have to\n> > count frames once it has been given a ExposureTimeModeDisabled ctrl then\n> > return out the same in the metadata when it knows that it's last\nrequested\n> > exposure\n> > time change has been applied?\n> >\n>\n> Not sure this is going to answer your question, but let's start by\n> defining what a \"glitch\" is for us in this definition. I think it's\n> useful to validate our understanding against your experience of providing\n> this features to the vast number of users you have.\n>\n> The idea is that applications willing to control the exposure time\n> explicitly might want to do so by minimizing the difference between\n> the last value computed by the AEGC algorithm and their newly set\n> value, to avoid a sudden change in exposure and gain which result in a\n> visible \"glitch\". En example, suddenly moving the exposure time and\n> gain to the opposite of the spectrum of what the AEGC was computing\n> will result in images going very bright or very dark in just a few\n> frames.\n>\n> The way to implement a smooth transition is to start from the values\n> lastly computed by the AEGC (as available in metadata) and then \"slowly\"\n> moving towards the desired manual value. Of course this is not\n> mandatory, application might desire a sudden change of exposure, or simply\n> won't care about smooth transitions. If they do, however, they have to\n> consider that there will always be a number of requests in the queue\n> that will be processed by the camera before the one with\n> \"ExposureTimeDisabled\" gets to be processed.\n>\n> During the processing of those requests in the queue, the AEGC will still\n> be active and might still change the exposure time (and gain) to values\nquite\n> different from the ones visible at the application at the time it\n> queued the request with \"ExposureTimeModeDisabled\".\n>\n> The steps proposed here suggest to applications to wait until the\n> \"ExposureTimeModeDisabled\" request is returned and the AEGC is\n> actually off. From the definitions we gave here, this mean the\n> exposure time (and gain) won't be updated by the now inactive AEGC\n> block until an \"ExposureTime\" value is submitted by applications (more\n> or less like your agc::pause()/resume() work, if I recall correctly).\n>\n> When the \"ExposureTimeModeDisabled\" request has completed,\n> applications know that the exposure time won't be updated from that\n> point on, and can use the ExposureTime and AnalogueGain metadata values\n> as a \"stable\" starting point for their values.\n>\n> Does this make sense to you ?\n\nYes it does, thanks for the clarification!\n\nOne question still remains - how does the application know when the AE has\nactullay finished - i.e. all the AE adjusted frames have been delivered?\nShould\nthe IPA report \"ExposureTimeModeDisabled\" only when the AE adjust frames\nhave\nbeen completed, or report it immediately? The former will require AE to\nhold\nextra state and keep \"running\" even when it might be disabled, but it's not\ntoo\nbig of a problem I suppose.\n\n>\n> In my fixups I proposed a rework of the introduction section of this\n> part, could you have a look to see if that's more clear ?\n>\n> >\n> > > +\n> > > + 4. Copy the value reported in ExposureTime into a new\nrequest, and\n> > > + submit it\n> > > +\n> > > + 5. Proceed to run manual exposure time\n> > >\n> >\n> > Again, I am unclear how this avoids glitches. Say the AE chooses an\n> > exposure\n> > time of 33ms, then the user wants to switch to 15ms. There is always\ngoing\n> > to\n> > be a jump in brightness. Perhaps my interpretation of this glitch is\nnot\n> > the same\n> > as what you are describing?\n>\n> If an application decides not to care and halves the exposure time\n> from one request to the following one, the above procedure is useless\n> indeed.\n>\n> But as explained above, an application might want to approach 15ms more\n> smoothly and the above text suggests how to do so.\n>\n> I feel like this is mostly directed to applications that wants to\n> drive the AEGC with some sort of algorithm instead of application\n> that simply take a value in from users and apply it directly. In\n> this latter case the values input from the user might very well be 1ms\n> hence approaching it slowly might not even be desired.\n\nThanks, I do understand when this might be needed now. But I struggle to\nsee\nwhy any application might want to do something like this, but that's not to\nsay\nthey won't :-)\n\nRegards,\nNaush\n\n>\n> Thanks for commenting!\n>\n> >\n> > Ditto comments for the AnalogueGain changes.\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > > +\n> > > + \\sa ExposureTime\n> > > + enum:\n> > > + - name: ExposureTimeModeAuto\n> > > + value: 0\n> > > + description: |\n> > > + The exposure time will be calculated automatically and\nset by\n> > > the\n> > > + AE algorithm. If ExposureTime is set while this mode is\n> > > active, it\n> > > + will be ignored, and it will also not be retained.\n> > > + - name: ExposureTimeModeDisabled\n> > > + value: 1\n> > > + description: |\n> > > + The exposure time will not be updated by the AE\nalgorithm. It\n> > > will\n> > > + come from the last calculated value when the mode was\nAuto,\n> > > or from\n> > > + the value specified in ExposureTime.\n> > > +\n> > > + When transitioning from Auto to Disabled mode, the last\n> > > computed\n> > > + exposure value is used until a new value is specified\nthrough\n> > > the\n> > > + ExposureTime control. If an ExposureTime value is\nspecified\n> > > in the\n> > > + same request where the ExposureTimeMode is changed from\nAuto\n> > > to\n> > > + Disabled, the provided ExposureTime is applied.\n> > >\n> > > - AnalogueGain:\n> > > type: float\n> > > @@ -144,17 +244,85 @@ controls:\n> > > The value of the control specifies the gain multiplier\napplied to\n> > > all\n> > > colour channels. This value cannot be lower than 1.0.\n> > >\n> > > - Setting this value means that it is now fixed and the AE\n> > > algorithm may\n> > > - not change it. Setting it back to zero returns it to the\ncontrol\n> > > of the\n> > > - AE algorithm.\n> > > + This control will only take effect if AnalogueGainMode is\n> > > Disabled. If\n> > > + this control is set when AnalogueGainMode is Auto, the value\nwill\n> > > be\n> > > + ignored and will not be retained.\n> > > +\n> > > + When reported in metadata, this control indicates what\nanalogue\n> > > gain\n> > > + was used for the current request, regardless of\nAnalogueGainMode.\n> > > + AnalogueGainMode will indicate the source of the analogue\ngain\n> > > value,\n> > > + whether it came from the AE algorithm or not.\n> > > +\n> > > + \\sa ExposureTime\n> > > + \\sa AnalogueGainMode\n> > > +\n> > > + - AnalogueGainMode:\n> > > + type: int32_t\n> > > + description: |\n> > > + Controls the source of the analogue gain that is applied to\nthe\n> > > image\n> > > + sensor. When set to Auto, the AE algorithm computes the\nanalogue\n> > > gain\n> > > + and configures the image sensor accordingly. When set to\nDisabled,\n> > > + analogue gain specified in AnalogueGain is applied to the\nimage\n> > > sensor.\n> > > + If AnalogueGain is not set, then the value last computed by\nthe AE\n> > > + algorithm when the mode was Auto will be used.\n> > > +\n> > > + If AnalogueGain is not set and the mode is\n> > > AnalogueGainModeDisabled and\n> > > + AE was never Auto (either because the camera started in\nDisabled\n> > > mode,\n> > > + or Auto is not supported by the camera), the camera should\nuse a\n> > > + best-effort default value.\n> > > +\n> > > + When AnalogueGainMode is set Auto, the value set in\nAnalogueGain\n> > > is\n> > > + ignored and is not retained. This means that if\nAnalogueGainMode\n> > > is set\n> > > + to Disabled and AnalogueGain is not also set, the analogue\ngain\n> > > that\n> > > + was last computed by the AE algorithm while the mode was Auto\n> > > will be\n> > > + applied to the sensor.\n> > >\n> > > - \\sa ExposureTime AeEnable\n> > > + If AnalogueGainModeDisabled is supported, the AnalogueGain\n> > > control must\n> > > + also be supported.\n> > > +\n> > > + The set of AnalogueGainMode modes that are supported by the\n> > > camera must\n> > > + have an intersection with the supported set of\nExposureTimeMode\n> > > modes.\n> > > +\n> > > + As it takes a few frames to apply the analogue gain, there\nis a\n> > > period of\n> > > + time between submitting a request with AnalogueGainMode set\nto\n> > > Disabled\n> > > + and the analogue gain component of the AE actually being\ndisabled,\n> > > + during which the AE algorithm can still update the analogue\ngain.\n> > > If an\n> > > + application is switching from automatic and manual control\nand\n> > > wishes\n> > > + to eliminate any flicker during the switch, the following\n> > > procedure is\n> > > + recommended.\n> > > +\n> > > + 1. Start with AnalogueGainMode set to Auto\n> > > +\n> > > + 2. Set AnalogueGainMode to Disabled\n> > > +\n> > > + 3. Wait for the first request to be output that has\n> > > AnalogueGainMode\n> > > + set to Disabled\n> > > +\n> > > + 4. Copy the value reported in AnalogueGain into a new\nrequest, and\n> > > + submit it\n> > > +\n> > > + 5. Proceed to run manual analogue gain\n> > >\n> > +\n> > > + \\sa AnalogueGain\n> > > + enum:\n> > > + - name: AnalogueGainModeAuto\n> > > + value: 0\n> > > + description: |\n> > > + The analogue gain will be calculated automatically and\nset by\n> > > the\n> > > + AE algorithm. If AnalogueGain is set while this mode is\n> > > active, it\n> > > + will be ignored, and it will also not be retained.\n> > > + - name: AnalogueGainModeDisabled\n> > > + value: 1\n> > > + description: |\n> > > + The analogue gain will not be updated by the AE\nalgorithm. It\n> > > will\n> > > + come from the last calculated value when the mode was\nAuto,\n> > > or from\n> > > + the value specified in AnalogueGain.\n> > >\n> > > - \\todo Document the interactions between AeEnable and setting\na\n> > > fixed\n> > > - value for this control. Consider interactions with other AE\n> > > features,\n> > > - such as aperture and aperture/shutter priority mode, and\ndecide if\n> > > - control of which features should be automatically adjusted\n> > > shouldn't\n> > > - better be handled through a separate AE mode control.\n> > > + When transitioning from Auto to Disabled mode the last\n> > > computed\n> > > + gain value is used until a new value is specified\nthrough the\n> > > + AnalogueGain control. If an AnalogueGain value is\nspecified\n> > > in the\n> > > + same request where the AnalogueGainMode is set to\nDisabled,\n> > > the\n> > > + provided AnalogueGain is applied.\n> > >\n> > > - Brightness:\n> > > type: float\n> > > @@ -477,36 +645,6 @@ controls:\n> > > High quality aberration correction which might reduce the\n> > > frame\n> > > rate.\n> > >\n> > > - - AeState:\n> > > - type: int32_t\n> > > - draft: true\n> > > - description: |\n> > > - Control to report the current AE algorithm state. Currently\n> > > identical to\n> > > - ANDROID_CONTROL_AE_STATE.\n> > > -\n> > > - Current state of the AE algorithm.\n> > > - enum:\n> > > - - name: AeStateInactive\n> > > - value: 0\n> > > - description: The AE algorithm is inactive.\n> > > - - name: AeStateSearching\n> > > - value: 1\n> > > - description: The AE algorithm has not converged yet.\n> > > - - name: AeStateConverged\n> > > - value: 2\n> > > - description: The AE algorithm has converged.\n> > > - - name: AeStateLocked\n> > > - value: 3\n> > > - description: The AE algorithm is locked.\n> > > - - name: AeStateFlashRequired\n> > > - value: 4\n> > > - description: The AE algorithm would need a flash for good\n> > > results\n> > > - - name: AeStatePrecapture\n> > > - value: 5\n> > > - description: |\n> > > - The AE algorithm has started a pre-capture metering\nsession.\n> > > - \\sa AePrecaptureTrigger\n> > > -\n> > > - AfState:\n> > > type: int32_t\n> > > draft: true\n> > > --\n> > > 2.30.2\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 41AD3BD1F1\n\tfor ;\n\tTue, 5 Jul 2022 08:06:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 873036330E;\n\tTue, 5 Jul 2022 10:06:58 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F4E060401\n\tfor ;\n\tTue, 5 Jul 2022 10:06:56 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id q8so1090727ljj.10\n\tfor ;\n\tTue, 05 Jul 2022 01:06:56 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657008418;\n\tbh=QotLe7vZW+/QDOqyfPxL8mN+e3r5XYkFzQUtnFPeH0E=;\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=zgEIXLkZ72SN6ZVggqIOT9j+jZuT21uOPtwAG98xDVaEuN0iE6/JnkBF2Wjop2NLR\n\tv/wyj8h8iQ56sFHhtoPR4EXtoj6W2Ab43EKcjqbiU/Ioe3X8uakEezKJj95bfxHfbf\n\tMJ3fpKLkDHSzAms072MJWY6MZ+sBpkRoZH2FkhIWV1rkV31kFJl53TVHk/u6lMnS+T\n\tJhviEm5b70EpZX3oraPXjuXJLZXLvZdEN6+AuNDiZuZZ8dcGvIJorGYB+kakma8btN\n\tr/LYC1XPTMXsNkbdzQFA15GKilIOu5FnTD9GwvAfOpQB6L2zvZtIpoztle2uvbeIHv\n\tMMtpvXU+Fjswg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=aCIhdP9yS8AaqClq4YAOLYR+Ds4n/gjjS53sf03EFRA=;\n\tb=Yj4LS6KAeLeky8HhHnpfZ9P7326FqUe7CQ/eHqEWNDBEyMGL5WdPfUvsrSC8sUEz+E\n\tSiwLUxvkLCw4XfEoiWuHjZ77nEpzf3ygzUc2/H7PjekOvVMHLycVlZStxb0F+UisGWrJ\n\t8752ZS7yTmHuK8E0177gQYQwBiC0xFoxtE+8azNT643M/tl/zdzrZSebkyouRIo1veY0\n\tCxyxTenIy0DT9lzWijeF3SZr/q2AOpC1dxQ5QswiRAtU2ShKVfL3DoQqf5Zob0SrP4jk\n\t1Xs7K5sqtbi/2EMfuUYNxV7G/GZMRKjJ76aO7azprDsb8GBXNAcRG/unCsrze3CP83/c\n\tuO7Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Yj4LS6KA\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=aCIhdP9yS8AaqClq4YAOLYR+Ds4n/gjjS53sf03EFRA=;\n\tb=H9RCwa8i/8RJVXB7yKz2syoWti/Px0rLDA8WUHC57QLmSBxXgqUqVCDrZds8KrgmXa\n\tYiESM1BrD9lLEiae78nuu5GilSFROIW40pQl4+H7tw85jqKpJb/CmAV0ik1Xr1C61yPg\n\tnmSsZOIToZ/+GqCZ68oJrudV6ZKDUA47wslpnxF3wkOqxT1zhx3xre7whPLFagBzFkvv\n\t7FUC51sYX2a9PRkF6Wn22JAIaUVW48BsmA7lrmX8+MXr5DUwTUD472QZhPriaja7Dwn5\n\tAnPyBVVYmTYLqTm1VyGEXo8dsrL42fOniuLOrHv2i05u7b7MNIOMwNADR6rG2P6puuIb\n\t4VNw==","X-Gm-Message-State":"AJIora+pDYadM5qOcB87BxJFATvUf6trB8SwItn/jVdOFBGdSQmmcQf/\n\tLqmYebFpkfhkikS9M0nb6SrcTqU0saKihSq2cjJROo4LOL9mxMey","X-Google-Smtp-Source":"AGRyM1tQiuva2WcVrdCWpmrl6zRXXV8U3IMXqYBsLmU+78Y6im+UJqsersNmKLHvfXP1Jr5/xRPmGAEJ2uSu1+hILxI=","X-Received":"by 2002:a2e:b911:0:b0:25a:9942:4171 with SMTP id\n\tb17-20020a2eb911000000b0025a99424171mr18301780ljb.426.1657008415620;\n\tTue, 05 Jul 2022 01:06:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@ideasonboard.com>\n\t\n\t<20220704153024.xzolxogrxxxwrry4@uno.localdomain>","In-Reply-To":"<20220704153024.xzolxogrxxxwrry4@uno.localdomain>","Date":"Tue, 5 Jul 2022 09:06:39 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000000dad105e30a56fd\"","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related 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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23737,"web_url":"https://patchwork.libcamera.org/comment/23737/","msgid":"<20220705091702.kglucpzutivmgclu@uno.localdomain>","date":"2022-07-05T09:17:02","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush,\n\nOn Tue, Jul 05, 2022 at 09:06:39AM +0100, Naushir Patuck wrote:\n> Hi Jacopo and Paul,\n>\n> On Mon, 4 Jul 2022 at 16:30, Jacopo Mondi wrote:\n> >\n> > Hi Naush,\n> > sorry Paul if I reply in your place, but I just got through this so\n> > I might have it slightly fresher\n> >\n> > On Mon, Jul 04, 2022 at 11:02:47AM +0100, Naushir Patuck via\n> libcamera-devel wrote:\n> > > Hi Paul,\n> > >\n> > > Thank you for your work. It's nice to see these changes to solidify\n> the AE\n> > > controls.\n> > > I do have a few thoughts/comments below.\n> >\n> > Can I ask you to have a look at the final result with the fixups I\n> > sent in reply applied on top of this patch ?\n>\n>\n> Yes will do!\n>\n>\n> >\n> > >\n> > >\n> > > On Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <\n> > > libcamera-devel@lists.libcamera.org> wrote:\n> > >\n> > > > We have multiple goals:\n> > > > - we need a lock of some sort, to instruct the AEGC to not update\n> output\n> > > > results\n> > > > - we need manual modes, to override the values computed by the AEGC\n> >\n> > > > - we need to support seamless transitions from auto -> manual, and do\n> so\n> > > > without flickering\n> > > > - we need custom minimum values for the manual controls, that is no\n> > > > magic values for enabling/disabling auto\n> > > > - all of these need to be done with AE sub-controls (exposure time,\n> > > > analogue gain)\n> > > >\n> > > > To achieve these goals, we introduce mode controls for the AE\n> > > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an\n> auto\n> > > > state, and a disabled state. The disabled state has an internal\n> one-way\n> > > > state change from locked to manual, triggered by the presence of the\n> > > > value-controls (ExposureTime and AnalogueGain).\n> > > >\n> > > > We then remove the AeEnable control, as it is a redundant control in\n> the\n> > > > face of these two mode controls.\n> > > >\n> > > > We also remove AeLocked, as it is insufficient for reporting the AE\n> > > > state, and we promote AeState to non-draft to fill its role. Notably,\n> > > > the locked state is removed, since this information can be obtained\n> from\n> > > > the aforementioned mode controls.\n> > > >\n> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> > > > Signed-off-by: Paul Elder \n> > > >\n> > > > ---\n> > > > Changes in v4:\n> > > > - remove FlashRequired and Precapture from AeState\n> > > > - upgrade documentation of all the controls\n> > > >\n> > > > Changes in v3:\n> > > > - improve wording of the control descriptions\n> > > > - make more succinct and clear\n> > > > - add description of how to do a flickerless transition\n> > > >\n> > > > Changes in v2:\n> > > > - No changes, just resubmitting at the head of this series so that\n> it's\n> > > > together and so that /people will actually see it/\n> > > >\n> > > > Initial version:\n> > > > Still RFC as I haven't updated the users of the control yet, and I\n> want\n> > > > to check that these are the controls and docs that we want.\n> > > >\n> > > > We've decided that the \"master AE control\" will be implemented by a\n> > > > helper... but looking at uvcvideo and the V4L2 controls I'm wondering\n> if\n> > > > such helper should come earlier than later?\n> > > >\n> > >\n> > > Yes, I agree having the \"master AE control\" earlier will be beneficial\n> for\n> > > application developers.\n> > >\n> >\n> > Do you envision this as something that could be part of your\n> > applications in example, or in a layer part of libcamera itself ?\n>\n>\n> My preference would be to have a helper in libcamera do this if possible.\n> This way, applications don't need to duplicate code to set all AE controls.\n> But either way, it's not too big of a deal to put it in the application.\n>\n> >\n> >\n> > >\n> > > > ---\n> > > > src/libcamera/control_ids.yaml | 262\n> +++++++++++++++++++++++++--------\n> > > > 1 file changed, 200 insertions(+), 62 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/control_ids.yaml\n> > > > b/src/libcamera/control_ids.yaml\n> > > > index 9d4638ae..9f5ce5e8 100644\n> > > > --- a/src/libcamera/control_ids.yaml\n> > > > +++ b/src/libcamera/control_ids.yaml\n> > > > @@ -7,23 +7,46 @@\n> > > > # Unless otherwise stated, all controls are bi-directional, i.e.\n> they can\n> > > > be\n> > > > # set through Request::controls() and returned out through\n> > > > Request::metadata().\n> > > > controls:\n> > > > - - AeEnable:\n> > > > - type: bool\n> > > > + - AeState:\n> > > > + type: int32_t\n> > > > description: |\n> > > > - Enable or disable the AE.\n> > > > + Control to report the AE algorithm state associated with the\n> > > > capture\n> > > > + result.\n> > > >\n> > > > - \\sa ExposureTime AnalogueGain\n> > > > + The state is still reported even if ExposureTimeMode or\n> > > > + AnalogueGainMode is set to Disabled.\n> > > >\n> > > > - - AeLocked:\n> > > > - type: bool\n> > > > - description: |\n> > > > - Report the lock status of a running AE algorithm.\n> > > > + \\sa AnalogueGain\n> > > > + \\sa AnalogueGainMode\n> > > > + \\sa ExposureTime\n> > > > + \\sa ExposureTimeMode\n> > > >\n> > > > - If the AE algorithm is locked the value shall be set to\n> true, if\n> > > > it's\n> > > > - converging it shall be set to false. If the AE algorithm is\n> not\n> > > > - running the control shall not be present in the metadata\n> control\n> > > > list.\n> > > > + enum:\n> > > > + - name: AeStateInactive\n> > > > + value: 0\n> > > > + description: |\n> > > > + The AE algorithm is inactive.\n> > > >\n> > > > - \\sa AeEnable\n> > > > + This state should be returned if both AnalogueGainMode\n> and\n> > > > + ExposureTimeMode are set to disabled (or one, if the\n> camera\n> > > > only\n> > > > + supports one of the two controls).\n> > > > + - name: AeStateSearching\n> > > > + value: 1\n> > > > + description: |\n> > > > + The AE algorithm has not converged yet.\n> > > > +\n> > > > + This state should be returned if at least one of\n> > > > AnalogueGainMode\n> > > > + or ExposureTimeMode is set to auto, and the AE algorithm\n> > > > hasn't\n> > > > + converged yet. If the AE algorithm converges, the state\n> shall\n> > > > go to\n> > > > + AeStateConverged.\n> > > > + - name: AeStateConverged\n> > > > + value: 2\n> > > > + description: |\n> > > > + The AE algorithm has converged.\n> > > > +\n> > > > + This state should be returned if at least one of\n> > > > AnalogueGainMode\n> > > > + or ExposureTimeMode is set to auto, and the AE algorithm\n> has\n> > > > + converged.\n> > > >\n> > > > # AeMeteringMode needs further attention:\n> > > > # - Auto-generate max enum value.\n> > > > @@ -93,6 +116,13 @@ controls:\n> > > > how the desired total exposure is divided between the\n> shutter time\n> > > > and the sensor's analogue gain. The exposure modes are\n> platform\n> > > > specific, and not all exposure modes may be supported.\n> > > > +\n> > > > + When one of AnalogueGainMode or ExposureTimeMode is set to\n> > > > Disabled,\n> > > > + the fixed values will override any choices made by\n> AeExposureMode.\n> > > > +\n> > > > + \\sa AnalogueGainMode\n> > > > + \\sa ExposureTimeMode\n> > > > +\n> > > > enum:\n> > > > - name: ExposureNormal\n> > > > value: 0\n> > > > @@ -111,13 +141,15 @@ controls:\n> > > > type: float\n> > > > description: |\n> > > > Specify an Exposure Value (EV) parameter. The EV parameter\n> will\n> > > > only be\n> > > > - applied if the AE algorithm is currently enabled.\n> > > > + applied if the AE algorithm is currently enabled, that is, at\n> > > > least one\n> > > > + of AnalogueGainMode and ExposureTimeMode are auto.\n> > > >\n> > > > By convention EV adjusts the exposure as log2. For example\n> > > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure\n> adjustment\n> > > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n> > > >\n> > > > - \\sa AeEnable\n> > > > + \\sa AnalogueGainMode\n> > > > + \\sa ExposureTimeMode\n> > > >\n> > > > - ExposureTime:\n> > > > type: int32_t\n> > > > @@ -125,17 +157,85 @@ controls:\n> > > > Exposure time (shutter speed) for the frame applied in the\n> sensor\n> > > > device. This value is specified in micro-seconds.\n> > > >\n> > > > - Setting this value means that it is now fixed and the AE\n> > > > algorithm may\n> > > > - not change it. Setting it back to zero returns it to the\n> control\n> > > > of the\n> > > > - AE algorithm.\n> > > > + This control will only take effect if ExposureTimeMode is\n> > > > Disabled. If\n> > > > + this control is set when ExposureTimeMode is Auto, the value\n> will\n> > > > be\n> > > > + ignored and will not be retained.\n> > > > +\n> > > > + When reported in metadata, this control indicates what\n> exposure\n> > > > time\n> > > > + was used for the current request, regardless of\n> ExposureTimeMode.\n> > > > + ExposureTimeMode will indicate the source of the exposure\n> time\n> > > > value,\n> > > > + whether it came from the AE algorithm or not.\n> > > > +\n> > > > + \\sa AnalogueGain\n> > > > + \\sa ExposureTimeMode\n> > > > +\n> > > > + - ExposureTimeMode:\n> > > > + type: int32_t\n> > > > + description: |\n> > > > + Controls the source of the exposure time that is applied to\n> the\n> > > > image\n> > > > + sensor. When set to Auto, the AE algorithm computes the\n> exposure\n> > > > time\n> > > > + and configures the image sensor accordingly. When set to\n> Disabled,\n> > > > + exposure time specified in ExposureTime is applied to the\n> image\n> > > > sensor.\n> > > > + If ExposureTime is not set, then the value last computed by\n> the AE\n> > > > + algorithm when the mode was Auto will be used.\n> > > >\n> > >\n> > > Can we un-set ExposureTime? If it ever gets set once at any point in\n> the\n> > > application,\n> > > then ExposureTimeModeDisabled will always use the last set value for\n> > > ExposureTime.\n> > >\n> >\n> > If I interpret your question right, are you wondering if the\n> > ExposureTime value is retained if an application sends it when the\n> > AEGC is actually in auto mode (and so the ExposureTime from application\n> > is not applied) ?\n> >\n> > We discussed this, and I think Paul tried to clarify it in the\n> > ExposureTime documentation by saying:\n> >\n> > - name: ExposureTimeModeDisabled\n> > If ExposureTime is set while this mode is active, it\n> > will be ignored, and it will also not be retained.\n> >\n> > which means that by design, the ExposureTime is just ignored if sent\n> > when the AEGC is in auto mode.\n> >\n> > Do you think that's not the expected behaviour ?\n>\n>\n> Not exactly... I was considering the following sequence:\n>\n> 1) ExposureTimeMode is Auto - AE adjusts exposure time as needed.\n> 2) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.\n> No ExposureTime value set yet, so keep the last AE exposure time.\n> 3) Set ExposureTime to some desired value.\n> 4) ExposureTimeMode set to Auto - AE adjusts exposure time as needed again.\n> 5) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.\n>\n> In step 5, does the IPA switch back to the ExposureTime set in step 3, or\n> does\n> setting ExposureTimeMode to Auto invalidate the existing ExposureTime\n> value? If\n\nI would expect step 3 to re-cycle the state machine and invalidate the\nmanually programmed exposure time\n\nIs this what application developers would expect in your opinion ?\n\n> the ExposureTime value is still valid, should we consider a way to\n> invalidate\n> (unset) it if the application wanted to just use the AE selected exposure\n> time\n> at step 5?\n>\n> >\n> >\n> > >\n> > > > +\n> > > > + If ExposureTime is not set and the mode is\n> > > > ExposureTimeModeDisabled and\n> > > > + AE was never Auto (either because the camera started in\n> Disabled\n> > > > mode,\n> > > > + or Auto is not supported by the camera), the camera should\n> use a\n> > > > + best-effort default value.\n> > > > +\n> > > > + When ExposureTimeMode is set Auto, the value set in\n> ExposureTime\n> > > > is\n> > > > + ignored and is not retained. This means that if\n> ExposureTimeMode\n> > > > is set\n> > > > + to Disabled and ExposureTime is not also set, the exposure\n> time\n> > > > that\n> > > > + was last computed by the AE algorithm while the mode was Auto\n> > > > will be\n> > > > + applied to the sensor.\n> > > > +\n> > > > + If ExposureTimeModeDisabled is supported, the ExposureTime\n> > > > control must\n> > > > + also be supported.\n> > > > +\n> > > > + The set of ExposureTimeMode modes that are supported by the\n> > > > camera must\n> > > > + have an intersection with the supported set of\n> AnalogueGainMode\n> > > > modes.\n> > > >\n> > > > - \\sa AnalogueGain AeEnable\n> > > > + As it takes a few frames to apply the exposure time, there\n> is a\n> > > > period of\n> > > > + time between submitting a request with ExposureTimeMode set\n> to\n> > > > Disabled\n> > > > + and the exposure time component of the AE actually being\n> disabled,\n> > > > + during which the AE algorithm can still update the exposure\n> time.\n> > > > If an\n> > > > + application is switching from automatic and manual control\n> and\n> > > > wishes\n> > > > + to eliminate any flicker during the switch, the following\n> > > > procedure is\n> > > > + recommended.\n> > > >\n> > >\n> > > I'm a bit confused by this bit to be honest. If a user switches\n> > > ExposureTimeMode from\n> > > Auto to Disabled with the intention of setting a manual ExposureTime,\n> how\n> > > can we ever\n> > > avoid a glitch in the brightness (unless we also change AnalogueGain\n> > > appropriately)?\n> > >\n> >\n> > See below\n> >\n> > >\n> > > > - \\todo Document the interactions between AeEnable and setting\n> a\n> > > > fixed\n> > > > - value for this control. Consider interactions with other AE\n> > > > features,\n> > > > - such as aperture and aperture/shutter priority mode, and\n> decide if\n> > > > - control of which features should be automatically adjusted\n> > > > shouldn't\n> > > > - better be handled through a separate AE mode control.\n> > > > + 1. Start with ExposureTimeMode set to Auto\n> > > > +\n> > > > + 2. Set ExposureTimeMode to Disabled\n> > > > +\n> > > > + 3. Wait for the first request to be output that has\n> > > > ExposureTimeMode\n> > > > + set to Disabled\n> > > >\n> > >\n> > > How would the application know this time point? Would the AE algorithm\n> > > have to\n> > > count frames once it has been given a ExposureTimeModeDisabled ctrl then\n> > > return out the same in the metadata when it knows that it's last\n> requested\n> > > exposure\n> > > time change has been applied?\n> > >\n> >\n> > Not sure this is going to answer your question, but let's start by\n> > defining what a \"glitch\" is for us in this definition. I think it's\n> > useful to validate our understanding against your experience of providing\n> > this features to the vast number of users you have.\n> >\n> > The idea is that applications willing to control the exposure time\n> > explicitly might want to do so by minimizing the difference between\n> > the last value computed by the AEGC algorithm and their newly set\n> > value, to avoid a sudden change in exposure and gain which result in a\n> > visible \"glitch\". En example, suddenly moving the exposure time and\n> > gain to the opposite of the spectrum of what the AEGC was computing\n> > will result in images going very bright or very dark in just a few\n> > frames.\n> >\n> > The way to implement a smooth transition is to start from the values\n> > lastly computed by the AEGC (as available in metadata) and then \"slowly\"\n> > moving towards the desired manual value. Of course this is not\n> > mandatory, application might desire a sudden change of exposure, or simply\n> > won't care about smooth transitions. If they do, however, they have to\n> > consider that there will always be a number of requests in the queue\n> > that will be processed by the camera before the one with\n> > \"ExposureTimeDisabled\" gets to be processed.\n> >\n> > During the processing of those requests in the queue, the AEGC will still\n> > be active and might still change the exposure time (and gain) to values\n> quite\n> > different from the ones visible at the application at the time it\n> > queued the request with \"ExposureTimeModeDisabled\".\n> >\n> > The steps proposed here suggest to applications to wait until the\n> > \"ExposureTimeModeDisabled\" request is returned and the AEGC is\n> > actually off. From the definitions we gave here, this mean the\n> > exposure time (and gain) won't be updated by the now inactive AEGC\n> > block until an \"ExposureTime\" value is submitted by applications (more\n> > or less like your agc::pause()/resume() work, if I recall correctly).\n> >\n> > When the \"ExposureTimeModeDisabled\" request has completed,\n> > applications know that the exposure time won't be updated from that\n> > point on, and can use the ExposureTime and AnalogueGain metadata values\n> > as a \"stable\" starting point for their values.\n> >\n> > Does this make sense to you ?\n>\n> Yes it does, thanks for the clarification!\n>\n> One question still remains - how does the application know when the AE has\n> actullay finished - i.e. all the AE adjusted frames have been delivered?\n> Should\n> the IPA report \"ExposureTimeModeDisabled\" only when the AE adjust frames\n> have\n> been completed, or report it immediately? The former will require AE to\n\nI do expect IPAs to process all requests with AE activated until they\ndon't get to crunch the one that has \"ExposureTimeModeDisabled\" in the\ncontrols list, then \"pause\" their AGC and set\n\"ExposureTimeModeDisabled\" in metadata (with the currently programmed\nexposure time in \"ExposureTime\"). From that point on the\nexposure time won't be updated anymore by AEGC.\n\nBetween the time an application sends a request with\n\"ExposureTimeModeDisabled\" and the time it should send one with an\n\"ExposureTime\" specified there will be a number of requests completed\nwith the AEGC still reported as active.\n\n> hold\n> extra state and keep \"running\" even when it might be disabled, but it's not\n> too\n> big of a problem I suppose.\n>\n\nI'm missing why the AEGC should keep a state, so I'm probably missing\npart of your reasoning\n\n> >\n> > In my fixups I proposed a rework of the introduction section of this\n> > part, could you have a look to see if that's more clear ?\n> >\n> > >\n> > > > +\n> > > > + 4. Copy the value reported in ExposureTime into a new\n> request, and\n> > > > + submit it\n> > > > +\n> > > > + 5. Proceed to run manual exposure time\n> > > >\n> > >\n> > > Again, I am unclear how this avoids glitches. Say the AE chooses an\n> > > exposure\n> > > time of 33ms, then the user wants to switch to 15ms. There is always\n> going\n> > > to\n> > > be a jump in brightness. Perhaps my interpretation of this glitch is\n> not\n> > > the same\n> > > as what you are describing?\n> >\n> > If an application decides not to care and halves the exposure time\n> > from one request to the following one, the above procedure is useless\n> > indeed.\n> >\n> > But as explained above, an application might want to approach 15ms more\n> > smoothly and the above text suggests how to do so.\n> >\n> > I feel like this is mostly directed to applications that wants to\n> > drive the AEGC with some sort of algorithm instead of application\n> > that simply take a value in from users and apply it directly. In\n> > this latter case the values input from the user might very well be 1ms\n> > hence approaching it slowly might not even be desired.\n>\n> Thanks, I do understand when this might be needed now. But I struggle to\n> see\n> why any application might want to do something like this, but that's not to\n> say\n> they won't :-)\n\nTo be honest the idea to have such a section in documentation comes\nfrom the fact android does describe it, so I presumed it was kind of a\ncommon parameter.\n\nThanks for review\n\n>\n> Regards,\n> Naush\n>\n> >\n> > Thanks for commenting!\n> >\n> > >\n> > > Ditto comments for the AnalogueGain changes.\n> > >\n> > > Regards,\n> > > Naush\n> > >\n> > >\n> > > > +\n> > > > + \\sa ExposureTime\n> > > > + enum:\n> > > > + - name: ExposureTimeModeAuto\n> > > > + value: 0\n> > > > + description: |\n> > > > + The exposure time will be calculated automatically and\n> set by\n> > > > the\n> > > > + AE algorithm. If ExposureTime is set while this mode is\n> > > > active, it\n> > > > + will be ignored, and it will also not be retained.\n> > > > + - name: ExposureTimeModeDisabled\n> > > > + value: 1\n> > > > + description: |\n> > > > + The exposure time will not be updated by the AE\n> algorithm. It\n> > > > will\n> > > > + come from the last calculated value when the mode was\n> Auto,\n> > > > or from\n> > > > + the value specified in ExposureTime.\n> > > > +\n> > > > + When transitioning from Auto to Disabled mode, the last\n> > > > computed\n> > > > + exposure value is used until a new value is specified\n> through\n> > > > the\n> > > > + ExposureTime control. If an ExposureTime value is\n> specified\n> > > > in the\n> > > > + same request where the ExposureTimeMode is changed from\n> Auto\n> > > > to\n> > > > + Disabled, the provided ExposureTime is applied.\n> > > >\n> > > > - AnalogueGain:\n> > > > type: float\n> > > > @@ -144,17 +244,85 @@ controls:\n> > > > The value of the control specifies the gain multiplier\n> applied to\n> > > > all\n> > > > colour channels. This value cannot be lower than 1.0.\n> > > >\n> > > > - Setting this value means that it is now fixed and the AE\n> > > > algorithm may\n> > > > - not change it. Setting it back to zero returns it to the\n> control\n> > > > of the\n> > > > - AE algorithm.\n> > > > + This control will only take effect if AnalogueGainMode is\n> > > > Disabled. If\n> > > > + this control is set when AnalogueGainMode is Auto, the value\n> will\n> > > > be\n> > > > + ignored and will not be retained.\n> > > > +\n> > > > + When reported in metadata, this control indicates what\n> analogue\n> > > > gain\n> > > > + was used for the current request, regardless of\n> AnalogueGainMode.\n> > > > + AnalogueGainMode will indicate the source of the analogue\n> gain\n> > > > value,\n> > > > + whether it came from the AE algorithm or not.\n> > > > +\n> > > > + \\sa ExposureTime\n> > > > + \\sa AnalogueGainMode\n> > > > +\n> > > > + - AnalogueGainMode:\n> > > > + type: int32_t\n> > > > + description: |\n> > > > + Controls the source of the analogue gain that is applied to\n> the\n> > > > image\n> > > > + sensor. When set to Auto, the AE algorithm computes the\n> analogue\n> > > > gain\n> > > > + and configures the image sensor accordingly. When set to\n> Disabled,\n> > > > + analogue gain specified in AnalogueGain is applied to the\n> image\n> > > > sensor.\n> > > > + If AnalogueGain is not set, then the value last computed by\n> the AE\n> > > > + algorithm when the mode was Auto will be used.\n> > > > +\n> > > > + If AnalogueGain is not set and the mode is\n> > > > AnalogueGainModeDisabled and\n> > > > + AE was never Auto (either because the camera started in\n> Disabled\n> > > > mode,\n> > > > + or Auto is not supported by the camera), the camera should\n> use a\n> > > > + best-effort default value.\n> > > > +\n> > > > + When AnalogueGainMode is set Auto, the value set in\n> AnalogueGain\n> > > > is\n> > > > + ignored and is not retained. This means that if\n> AnalogueGainMode\n> > > > is set\n> > > > + to Disabled and AnalogueGain is not also set, the analogue\n> gain\n> > > > that\n> > > > + was last computed by the AE algorithm while the mode was Auto\n> > > > will be\n> > > > + applied to the sensor.\n> > > >\n> > > > - \\sa ExposureTime AeEnable\n> > > > + If AnalogueGainModeDisabled is supported, the AnalogueGain\n> > > > control must\n> > > > + also be supported.\n> > > > +\n> > > > + The set of AnalogueGainMode modes that are supported by the\n> > > > camera must\n> > > > + have an intersection with the supported set of\n> ExposureTimeMode\n> > > > modes.\n> > > > +\n> > > > + As it takes a few frames to apply the analogue gain, there\n> is a\n> > > > period of\n> > > > + time between submitting a request with AnalogueGainMode set\n> to\n> > > > Disabled\n> > > > + and the analogue gain component of the AE actually being\n> disabled,\n> > > > + during which the AE algorithm can still update the analogue\n> gain.\n> > > > If an\n> > > > + application is switching from automatic and manual control\n> and\n> > > > wishes\n> > > > + to eliminate any flicker during the switch, the following\n> > > > procedure is\n> > > > + recommended.\n> > > > +\n> > > > + 1. Start with AnalogueGainMode set to Auto\n> > > > +\n> > > > + 2. Set AnalogueGainMode to Disabled\n> > > > +\n> > > > + 3. Wait for the first request to be output that has\n> > > > AnalogueGainMode\n> > > > + set to Disabled\n> > > > +\n> > > > + 4. Copy the value reported in AnalogueGain into a new\n> request, and\n> > > > + submit it\n> > > > +\n> > > > + 5. Proceed to run manual analogue gain\n> > > >\n> > > +\n> > > > + \\sa AnalogueGain\n> > > > + enum:\n> > > > + - name: AnalogueGainModeAuto\n> > > > + value: 0\n> > > > + description: |\n> > > > + The analogue gain will be calculated automatically and\n> set by\n> > > > the\n> > > > + AE algorithm. If AnalogueGain is set while this mode is\n> > > > active, it\n> > > > + will be ignored, and it will also not be retained.\n> > > > + - name: AnalogueGainModeDisabled\n> > > > + value: 1\n> > > > + description: |\n> > > > + The analogue gain will not be updated by the AE\n> algorithm. It\n> > > > will\n> > > > + come from the last calculated value when the mode was\n> Auto,\n> > > > or from\n> > > > + the value specified in AnalogueGain.\n> > > >\n> > > > - \\todo Document the interactions between AeEnable and setting\n> a\n> > > > fixed\n> > > > - value for this control. Consider interactions with other AE\n> > > > features,\n> > > > - such as aperture and aperture/shutter priority mode, and\n> decide if\n> > > > - control of which features should be automatically adjusted\n> > > > shouldn't\n> > > > - better be handled through a separate AE mode control.\n> > > > + When transitioning from Auto to Disabled mode the last\n> > > > computed\n> > > > + gain value is used until a new value is specified\n> through the\n> > > > + AnalogueGain control. If an AnalogueGain value is\n> specified\n> > > > in the\n> > > > + same request where the AnalogueGainMode is set to\n> Disabled,\n> > > > the\n> > > > + provided AnalogueGain is applied.\n> > > >\n> > > > - Brightness:\n> > > > type: float\n> > > > @@ -477,36 +645,6 @@ controls:\n> > > > High quality aberration correction which might reduce the\n> > > > frame\n> > > > rate.\n> > > >\n> > > > - - AeState:\n> > > > - type: int32_t\n> > > > - draft: true\n> > > > - description: |\n> > > > - Control to report the current AE algorithm state. Currently\n> > > > identical to\n> > > > - ANDROID_CONTROL_AE_STATE.\n> > > > -\n> > > > - Current state of the AE algorithm.\n> > > > - enum:\n> > > > - - name: AeStateInactive\n> > > > - value: 0\n> > > > - description: The AE algorithm is inactive.\n> > > > - - name: AeStateSearching\n> > > > - value: 1\n> > > > - description: The AE algorithm has not converged yet.\n> > > > - - name: AeStateConverged\n> > > > - value: 2\n> > > > - description: The AE algorithm has converged.\n> > > > - - name: AeStateLocked\n> > > > - value: 3\n> > > > - description: The AE algorithm is locked.\n> > > > - - name: AeStateFlashRequired\n> > > > - value: 4\n> > > > - description: The AE algorithm would need a flash for good\n> > > > results\n> > > > - - name: AeStatePrecapture\n> > > > - value: 5\n> > > > - description: |\n> > > > - The AE algorithm has started a pre-capture metering\n> session.\n> > > > - \\sa AePrecaptureTrigger\n> > > > -\n> > > > - AfState:\n> > > > type: int32_t\n> > > > draft: true\n> > > > --\n> > > > 2.30.2\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 D826BBD1F1\n\tfor ;\n\tTue, 5 Jul 2022 09:17:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDDA86330E;\n\tTue, 5 Jul 2022 11:17:05 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2AEE60401\n\tfor ;\n\tTue, 5 Jul 2022 11:17:04 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 08440FF811;\n\tTue, 5 Jul 2022 09:17:03 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657012626;\n\tbh=Kv9ftJOLEbJzYKyKBiEWrAb4T8hZJ/k2xEYJ5RBqJy4=;\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=0XEqn6/eFd1kJGTmvJPzn1TI91tUFrupD6Ds0bzdht+TyqTDug3A/B8GVJcdFwXNZ\n\t63en7ogcP5pwjQ3PwT3qWEbLJzZzD6lFqSux5Di0BhruDHUjBh5ErV2RqriOr9HnC4\n\toTqOE2Ckpxo2dVTJBc9YDcwGMrpEPlrpRHRId0Fe/Hg5/gIJLLpsp9iZF+pW1143K/\n\tK95LdpVutnmIZ8vSObxxa80h8ZmnTfw4PqqS50q5o/WpuKW1ROGlXaX6LG1o6JIcwu\n\t1M+KBtmqbRHu6+V9tteIpJ7SbIl+FIMVl38ObC5eNBnx0g8PJ0HREYWNPHai1VUx+P\n\tEHJIHmUT898Gg==","Date":"Tue, 5 Jul 2022 11:17:02 +0200","To":"Naushir Patuck ","Message-ID":"<20220705091702.kglucpzutivmgclu@uno.localdomain>","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@ideasonboard.com>\n\t\n\t<20220704153024.xzolxogrxxxwrry4@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related 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\" "}},{"id":23739,"web_url":"https://patchwork.libcamera.org/comment/23739/","msgid":"","date":"2022-07-05T09:43:46","subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo\n\nOn Tue, 5 Jul 2022 at 10:17, Jacopo Mondi wrote:\n>\n> Hi Naush,\n>\n> On Tue, Jul 05, 2022 at 09:06:39AM +0100, Naushir Patuck wrote:\n> > Hi Jacopo and Paul,\n> >\n> > On Mon, 4 Jul 2022 at 16:30, Jacopo Mondi wrote:\n> > >\n> > > Hi Naush,\n> > > sorry Paul if I reply in your place, but I just got through this so\n> > > I might have it slightly fresher\n> > >\n> > > On Mon, Jul 04, 2022 at 11:02:47AM +0100, Naushir Patuck via\n> > libcamera-devel wrote:\n> > > > Hi Paul,\n> > > >\n> > > > Thank you for your work. It's nice to see these changes to solidify\n> > the AE\n> > > > controls.\n> > > > I do have a few thoughts/comments below.\n> > >\n> > > Can I ask you to have a look at the final result with the fixups I\n> > > sent in reply applied on top of this patch ?\n> >\n> >\n> > Yes will do!\n> >\n> >\n> > >\n> > > >\n> > > >\n> > > > On Wed, 18 May 2022 at 14:47, Paul Elder via libcamera-devel <\n> > > > libcamera-devel@lists.libcamera.org> wrote:\n> > > >\n> > > > > We have multiple goals:\n> > > > > - we need a lock of some sort, to instruct the AEGC to not update\n> > output\n> > > > > results\n> > > > > - we need manual modes, to override the values computed by the\nAEGC\n> > >\n> > > > > - we need to support seamless transitions from auto -> manual,\nand do\n> > so\n> > > > > without flickering\n> > > > > - we need custom minimum values for the manual controls, that is\nno\n> > > > > magic values for enabling/disabling auto\n> > > > > - all of these need to be done with AE sub-controls (exposure\ntime,\n> > > > > analogue gain)\n> > > > >\n> > > > > To achieve these goals, we introduce mode controls for the AE\n> > > > > sub-controls: ExposureTimeMode and AnalogueGainMode. These have an\n> > auto\n> > > > > state, and a disabled state. The disabled state has an internal\n> > one-way\n> > > > > state change from locked to manual, triggered by the presence of\nthe\n> > > > > value-controls (ExposureTime and AnalogueGain).\n> > > > >\n> > > > > We then remove the AeEnable control, as it is a redundant control\nin\n> > the\n> > > > > face of these two mode controls.\n> > > > >\n> > > > > We also remove AeLocked, as it is insufficient for reporting the\nAE\n> > > > > state, and we promote AeState to non-draft to fill its role.\nNotably,\n> > > > > the locked state is removed, since this information can be\nobtained\n> > from\n> > > > > the aforementioned mode controls.\n> > > > >\n> > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> > > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> > > > > Signed-off-by: Paul Elder \n> > > > >\n> > > > > ---\n> > > > > Changes in v4:\n> > > > > - remove FlashRequired and Precapture from AeState\n> > > > > - upgrade documentation of all the controls\n> > > > >\n> > > > > Changes in v3:\n> > > > > - improve wording of the control descriptions\n> > > > > - make more succinct and clear\n> > > > > - add description of how to do a flickerless transition\n> > > > >\n> > > > > Changes in v2:\n> > > > > - No changes, just resubmitting at the head of this series so that\n> > it's\n> > > > > together and so that /people will actually see it/\n> > > > >\n> > > > > Initial version:\n> > > > > Still RFC as I haven't updated the users of the control yet, and I\n> > want\n> > > > > to check that these are the controls and docs that we want.\n> > > > >\n> > > > > We've decided that the \"master AE control\" will be implemented by\na\n> > > > > helper... but looking at uvcvideo and the V4L2 controls I'm\nwondering\n> > if\n> > > > > such helper should come earlier than later?\n> > > > >\n> > > >\n> > > > Yes, I agree having the \"master AE control\" earlier will be\nbeneficial\n> > for\n> > > > application developers.\n> > > >\n> > >\n> > > Do you envision this as something that could be part of your\n> > > applications in example, or in a layer part of libcamera itself ?\n> >\n> >\n> > My preference would be to have a helper in libcamera do this if\npossible.\n> > This way, applications don't need to duplicate code to set all AE\ncontrols.\n> > But either way, it's not too big of a deal to put it in the application.\n> >\n> > >\n> > >\n> > > >\n> > > > > ---\n> > > > > src/libcamera/control_ids.yaml | 262\n> > +++++++++++++++++++++++++--------\n> > > > > 1 file changed, 200 insertions(+), 62 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/control_ids.yaml\n> > > > > b/src/libcamera/control_ids.yaml\n> > > > > index 9d4638ae..9f5ce5e8 100644\n> > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > @@ -7,23 +7,46 @@\n> > > > > # Unless otherwise stated, all controls are bi-directional, i.e.\n> > they can\n> > > > > be\n> > > > > # set through Request::controls() and returned out through\n> > > > > Request::metadata().\n> > > > > controls:\n> > > > > - - AeEnable:\n> > > > > - type: bool\n> > > > > + - AeState:\n> > > > > + type: int32_t\n> > > > > description: |\n> > > > > - Enable or disable the AE.\n> > > > > + Control to report the AE algorithm state associated with\nthe\n> > > > > capture\n> > > > > + result.\n> > > > >\n> > > > > - \\sa ExposureTime AnalogueGain\n> > > > > + The state is still reported even if ExposureTimeMode or\n> > > > > + AnalogueGainMode is set to Disabled.\n> > > > >\n> > > > > - - AeLocked:\n> > > > > - type: bool\n> > > > > - description: |\n> > > > > - Report the lock status of a running AE algorithm.\n> > > > > + \\sa AnalogueGain\n> > > > > + \\sa AnalogueGainMode\n> > > > > + \\sa ExposureTime\n> > > > > + \\sa ExposureTimeMode\n> > > > >\n> > > > > - If the AE algorithm is locked the value shall be set to\n> > true, if\n> > > > > it's\n> > > > > - converging it shall be set to false. If the AE algorithm\nis\n> > not\n> > > > > - running the control shall not be present in the metadata\n> > control\n> > > > > list.\n> > > > > + enum:\n> > > > > + - name: AeStateInactive\n> > > > > + value: 0\n> > > > > + description: |\n> > > > > + The AE algorithm is inactive.\n> > > > >\n> > > > > - \\sa AeEnable\n> > > > > + This state should be returned if both\nAnalogueGainMode\n> > and\n> > > > > + ExposureTimeMode are set to disabled (or one, if the\n> > camera\n> > > > > only\n> > > > > + supports one of the two controls).\n> > > > > + - name: AeStateSearching\n> > > > > + value: 1\n> > > > > + description: |\n> > > > > + The AE algorithm has not converged yet.\n> > > > > +\n> > > > > + This state should be returned if at least one of\n> > > > > AnalogueGainMode\n> > > > > + or ExposureTimeMode is set to auto, and the AE\nalgorithm\n> > > > > hasn't\n> > > > > + converged yet. If the AE algorithm converges, the\nstate\n> > shall\n> > > > > go to\n> > > > > + AeStateConverged.\n> > > > > + - name: AeStateConverged\n> > > > > + value: 2\n> > > > > + description: |\n> > > > > + The AE algorithm has converged.\n> > > > > +\n> > > > > + This state should be returned if at least one of\n> > > > > AnalogueGainMode\n> > > > > + or ExposureTimeMode is set to auto, and the AE\nalgorithm\n> > has\n> > > > > + converged.\n> > > > >\n> > > > > # AeMeteringMode needs further attention:\n> > > > > # - Auto-generate max enum value.\n> > > > > @@ -93,6 +116,13 @@ controls:\n> > > > > how the desired total exposure is divided between the\n> > shutter time\n> > > > > and the sensor's analogue gain. The exposure modes are\n> > platform\n> > > > > specific, and not all exposure modes may be supported.\n> > > > > +\n> > > > > + When one of AnalogueGainMode or ExposureTimeMode is set\nto\n> > > > > Disabled,\n> > > > > + the fixed values will override any choices made by\n> > AeExposureMode.\n> > > > > +\n> > > > > + \\sa AnalogueGainMode\n> > > > > + \\sa ExposureTimeMode\n> > > > > +\n> > > > > enum:\n> > > > > - name: ExposureNormal\n> > > > > value: 0\n> > > > > @@ -111,13 +141,15 @@ controls:\n> > > > > type: float\n> > > > > description: |\n> > > > > Specify an Exposure Value (EV) parameter. The EV\nparameter\n> > will\n> > > > > only be\n> > > > > - applied if the AE algorithm is currently enabled.\n> > > > > + applied if the AE algorithm is currently enabled, that\nis, at\n> > > > > least one\n> > > > > + of AnalogueGainMode and ExposureTimeMode are auto.\n> > > > >\n> > > > > By convention EV adjusts the exposure as log2. For\nexample\n> > > > > EV = [-2, -1, 0.5, 0, 0.5, 1, 2] results in an exposure\n> > adjustment\n> > > > > of [1/4x, 1/2x, 1/sqrt(2)x, 1x, sqrt(2)x, 2x, 4x].\n> > > > >\n> > > > > - \\sa AeEnable\n> > > > > + \\sa AnalogueGainMode\n> > > > > + \\sa ExposureTimeMode\n> > > > >\n> > > > > - ExposureTime:\n> > > > > type: int32_t\n> > > > > @@ -125,17 +157,85 @@ controls:\n> > > > > Exposure time (shutter speed) for the frame applied in\nthe\n> > sensor\n> > > > > device. This value is specified in micro-seconds.\n> > > > >\n> > > > > - Setting this value means that it is now fixed and the AE\n> > > > > algorithm may\n> > > > > - not change it. Setting it back to zero returns it to the\n> > control\n> > > > > of the\n> > > > > - AE algorithm.\n> > > > > + This control will only take effect if ExposureTimeMode is\n> > > > > Disabled. If\n> > > > > + this control is set when ExposureTimeMode is Auto, the\nvalue\n> > will\n> > > > > be\n> > > > > + ignored and will not be retained.\n> > > > > +\n> > > > > + When reported in metadata, this control indicates what\n> > exposure\n> > > > > time\n> > > > > + was used for the current request, regardless of\n> > ExposureTimeMode.\n> > > > > + ExposureTimeMode will indicate the source of the exposure\n> > time\n> > > > > value,\n> > > > > + whether it came from the AE algorithm or not.\n> > > > > +\n> > > > > + \\sa AnalogueGain\n> > > > > + \\sa ExposureTimeMode\n> > > > > +\n> > > > > + - ExposureTimeMode:\n> > > > > + type: int32_t\n> > > > > + description: |\n> > > > > + Controls the source of the exposure time that is applied\nto\n> > the\n> > > > > image\n> > > > > + sensor. When set to Auto, the AE algorithm computes the\n> > exposure\n> > > > > time\n> > > > > + and configures the image sensor accordingly. When set to\n> > Disabled,\n> > > > > + exposure time specified in ExposureTime is applied to the\n> > image\n> > > > > sensor.\n> > > > > + If ExposureTime is not set, then the value last computed\nby\n> > the AE\n> > > > > + algorithm when the mode was Auto will be used.\n> > > > >\n> > > >\n> > > > Can we un-set ExposureTime? If it ever gets set once at any point\nin\n> > the\n> > > > application,\n> > > > then ExposureTimeModeDisabled will always use the last set value for\n> > > > ExposureTime.\n> > > >\n> > >\n> > > If I interpret your question right, are you wondering if the\n> > > ExposureTime value is retained if an application sends it when the\n> > > AEGC is actually in auto mode (and so the ExposureTime from\napplication\n> > > is not applied) ?\n> > >\n> > > We discussed this, and I think Paul tried to clarify it in the\n> > > ExposureTime documentation by saying:\n> > >\n> > > - name: ExposureTimeModeDisabled\n> > > If ExposureTime is set while this mode is active, it\n> > > will be ignored, and it will also not be retained.\n> > >\n> > > which means that by design, the ExposureTime is just ignored if sent\n> > > when the AEGC is in auto mode.\n> > >\n> > > Do you think that's not the expected behaviour ?\n> >\n> >\n> > Not exactly... I was considering the following sequence:\n> >\n> > 1) ExposureTimeMode is Auto - AE adjusts exposure time as needed.\n> > 2) ExposureTimeMode set to Disabled - AE stops adjusting exposure time.\n> > No ExposureTime value set yet, so keep the last AE exposure time.\n> > 3) Set ExposureTime to some desired value.\n> > 4) ExposureTimeMode set to Auto - AE adjusts exposure time as needed\nagain.\n> > 5) ExposureTimeMode set to Disabled - AE stops adjusting exposure\ntime.\n> >\n> > In step 5, does the IPA switch back to the ExposureTime set in step 3,\nor\n> > does\n> > setting ExposureTimeMode to Auto invalidate the existing ExposureTime\n> > value? If\n>\n> I would expect step 3 to re-cycle the state machine and invalidate the\n> manually programmed exposure time\n>\n> Is this what application developers would expect in your opinion ?\n\nThat seems reasonable to me, but perhaps worth adding some text to the\ndocumention clarifying this.\n\n>\n> > the ExposureTime value is still valid, should we consider a way to\n> > invalidate\n> > (unset) it if the application wanted to just use the AE selected\nexposure\n> > time\n> > at step 5?\n> >\n> > >\n> > >\n> > > >\n> > > > > +\n> > > > > + If ExposureTime is not set and the mode is\n> > > > > ExposureTimeModeDisabled and\n> > > > > + AE was never Auto (either because the camera started in\n> > Disabled\n> > > > > mode,\n> > > > > + or Auto is not supported by the camera), the camera\nshould\n> > use a\n> > > > > + best-effort default value.\n> > > > > +\n> > > > > + When ExposureTimeMode is set Auto, the value set in\n> > ExposureTime\n> > > > > is\n> > > > > + ignored and is not retained. This means that if\n> > ExposureTimeMode\n> > > > > is set\n> > > > > + to Disabled and ExposureTime is not also set, the\nexposure\n> > time\n> > > > > that\n> > > > > + was last computed by the AE algorithm while the mode was\nAuto\n> > > > > will be\n> > > > > + applied to the sensor.\n> > > > > +\n> > > > > + If ExposureTimeModeDisabled is supported, the\nExposureTime\n> > > > > control must\n> > > > > + also be supported.\n> > > > > +\n> > > > > + The set of ExposureTimeMode modes that are supported by\nthe\n> > > > > camera must\n> > > > > + have an intersection with the supported set of\n> > AnalogueGainMode\n> > > > > modes.\n> > > > >\n> > > > > - \\sa AnalogueGain AeEnable\n> > > > > + As it takes a few frames to apply the exposure time,\nthere\n> > is a\n> > > > > period of\n> > > > > + time between submitting a request with ExposureTimeMode\nset\n> > to\n> > > > > Disabled\n> > > > > + and the exposure time component of the AE actually being\n> > disabled,\n> > > > > + during which the AE algorithm can still update the\nexposure\n> > time.\n> > > > > If an\n> > > > > + application is switching from automatic and manual\ncontrol\n> > and\n> > > > > wishes\n> > > > > + to eliminate any flicker during the switch, the following\n> > > > > procedure is\n> > > > > + recommended.\n> > > > >\n> > > >\n> > > > I'm a bit confused by this bit to be honest. If a user switches\n> > > > ExposureTimeMode from\n> > > > Auto to Disabled with the intention of setting a manual\nExposureTime,\n> > how\n> > > > can we ever\n> > > > avoid a glitch in the brightness (unless we also change AnalogueGain\n> > > > appropriately)?\n> > > >\n> > >\n> > > See below\n> > >\n> > > >\n> > > > > - \\todo Document the interactions between AeEnable and\nsetting\n> > a\n> > > > > fixed\n> > > > > - value for this control. Consider interactions with other\nAE\n> > > > > features,\n> > > > > - such as aperture and aperture/shutter priority mode, and\n> > decide if\n> > > > > - control of which features should be automatically\nadjusted\n> > > > > shouldn't\n> > > > > - better be handled through a separate AE mode control.\n> > > > > + 1. Start with ExposureTimeMode set to Auto\n> > > > > +\n> > > > > + 2. Set ExposureTimeMode to Disabled\n> > > > > +\n> > > > > + 3. Wait for the first request to be output that has\n> > > > > ExposureTimeMode\n> > > > > + set to Disabled\n> > > > >\n> > > >\n> > > > How would the application know this time point? Would the AE\nalgorithm\n> > > > have to\n> > > > count frames once it has been given a ExposureTimeModeDisabled ctrl\nthen\n> > > > return out the same in the metadata when it knows that it's last\n> > requested\n> > > > exposure\n> > > > time change has been applied?\n> > > >\n> > >\n> > > Not sure this is going to answer your question, but let's start by\n> > > defining what a \"glitch\" is for us in this definition. I think it's\n> > > useful to validate our understanding against your experience of\nproviding\n> > > this features to the vast number of users you have.\n> > >\n> > > The idea is that applications willing to control the exposure time\n> > > explicitly might want to do so by minimizing the difference between\n> > > the last value computed by the AEGC algorithm and their newly set\n> > > value, to avoid a sudden change in exposure and gain which result in a\n> > > visible \"glitch\". En example, suddenly moving the exposure time and\n> > > gain to the opposite of the spectrum of what the AEGC was computing\n> > > will result in images going very bright or very dark in just a few\n> > > frames.\n> > >\n> > > The way to implement a smooth transition is to start from the values\n> > > lastly computed by the AEGC (as available in metadata) and then\n\"slowly\"\n> > > moving towards the desired manual value. Of course this is not\n> > > mandatory, application might desire a sudden change of exposure, or\nsimply\n> > > won't care about smooth transitions. If they do, however, they have to\n> > > consider that there will always be a number of requests in the queue\n> > > that will be processed by the camera before the one with\n> > > \"ExposureTimeDisabled\" gets to be processed.\n> > >\n> > > During the processing of those requests in the queue, the AEGC will\nstill\n> > > be active and might still change the exposure time (and gain) to\nvalues\n> > quite\n> > > different from the ones visible at the application at the time it\n> > > queued the request with \"ExposureTimeModeDisabled\".\n> > >\n> > > The steps proposed here suggest to applications to wait until the\n> > > \"ExposureTimeModeDisabled\" request is returned and the AEGC is\n> > > actually off. From the definitions we gave here, this mean the\n> > > exposure time (and gain) won't be updated by the now inactive AEGC\n> > > block until an \"ExposureTime\" value is submitted by applications (more\n> > > or less like your agc::pause()/resume() work, if I recall correctly).\n> > >\n> > > When the \"ExposureTimeModeDisabled\" request has completed,\n> > > applications know that the exposure time won't be updated from that\n> > > point on, and can use the ExposureTime and AnalogueGain metadata\nvalues\n> > > as a \"stable\" starting point for their values.\n> > >\n> > > Does this make sense to you ?\n> >\n> > Yes it does, thanks for the clarification!\n> >\n> > One question still remains - how does the application know when the AE\nhas\n> > actullay finished - i.e. all the AE adjusted frames have been delivered?\n> > Should\n> > the IPA report \"ExposureTimeModeDisabled\" only when the AE adjust frames\n> > have\n> > been completed, or report it immediately? The former will require AE to\n>\n> I do expect IPAs to process all requests with AE activated until they\n> don't get to crunch the one that has \"ExposureTimeModeDisabled\" in the\n> controls list, then \"pause\" their AGC and set\n> \"ExposureTimeModeDisabled\" in metadata (with the currently programmed\n> exposure time in \"ExposureTime\"). From that point on the\n> exposure time won't be updated anymore by AEGC.\n>\n> Between the time an application sends a request with\n> \"ExposureTimeModeDisabled\" and the time it should send one with an\n> \"ExposureTime\" specified there will be a number of requests completed\n> with the AEGC still reported as active.\n\nThat answers my question!\n\n>\n> > hold\n> > extra state and keep \"running\" even when it might be disabled, but it's\nnot\n> > too\n> > big of a problem I suppose.\n> >\n>\n> I'm missing why the AEGC should keep a state, so I'm probably missing\n> part of your reasoning\n\nActually, I was wrong, it's not EGC that keeps state, it is the IPA that\ndoes to\nknow when to send \"ExposureTimeModeDisabled\" metadat.\n\nThanks for the clarification.\n\nRegards,\nNaush\n\n\n>\n> > >\n> > > In my fixups I proposed a rework of the introduction section of this\n> > > part, could you have a look to see if that's more clear ?\n> > >\n> > > >\n> > > > > +\n> > > > > + 4. Copy the value reported in ExposureTime into a new\n> > request, and\n> > > > > + submit it\n> > > > > +\n> > > > > + 5. Proceed to run manual exposure time\n> > > > >\n> > > >\n> > > > Again, I am unclear how this avoids glitches. Say the AE chooses an\n> > > > exposure\n> > > > time of 33ms, then the user wants to switch to 15ms. There is\nalways\n> > going\n> > > > to\n> > > > be a jump in brightness. Perhaps my interpretation of this glitch\nis\n> > not\n> > > > the same\n> > > > as what you are describing?\n> > >\n> > > If an application decides not to care and halves the exposure time\n> > > from one request to the following one, the above procedure is useless\n> > > indeed.\n> > >\n> > > But as explained above, an application might want to approach 15ms\nmore\n> > > smoothly and the above text suggests how to do so.\n> > >\n> > > I feel like this is mostly directed to applications that wants to\n> > > drive the AEGC with some sort of algorithm instead of application\n> > > that simply take a value in from users and apply it directly. In\n> > > this latter case the values input from the user might very well be 1ms\n> > > hence approaching it slowly might not even be desired.\n> >\n> > Thanks, I do understand when this might be needed now. But I struggle\nto\n> > see\n> > why any application might want to do something like this, but that's\nnot to\n> > say\n> > they won't :-)\n>\n> To be honest the idea to have such a section in documentation comes\n> from the fact android does describe it, so I presumed it was kind of a\n> common parameter.\n>\n> Thanks for review\n>\n> >\n> > Regards,\n> > Naush\n> >\n> > >\n> > > Thanks for commenting!\n> > >\n> > > >\n> > > > Ditto comments for the AnalogueGain changes.\n> > > >\n> > > > Regards,\n> > > > Naush\n> > > >\n> > > >\n> > > > > +\n> > > > > + \\sa ExposureTime\n> > > > > + enum:\n> > > > > + - name: ExposureTimeModeAuto\n> > > > > + value: 0\n> > > > > + description: |\n> > > > > + The exposure time will be calculated automatically\nand\n> > set by\n> > > > > the\n> > > > > + AE algorithm. If ExposureTime is set while this mode\nis\n> > > > > active, it\n> > > > > + will be ignored, and it will also not be retained.\n> > > > > + - name: ExposureTimeModeDisabled\n> > > > > + value: 1\n> > > > > + description: |\n> > > > > + The exposure time will not be updated by the AE\n> > algorithm. It\n> > > > > will\n> > > > > + come from the last calculated value when the mode was\n> > Auto,\n> > > > > or from\n> > > > > + the value specified in ExposureTime.\n> > > > > +\n> > > > > + When transitioning from Auto to Disabled mode, the\nlast\n> > > > > computed\n> > > > > + exposure value is used until a new value is specified\n> > through\n> > > > > the\n> > > > > + ExposureTime control. If an ExposureTime value is\n> > specified\n> > > > > in the\n> > > > > + same request where the ExposureTimeMode is changed\nfrom\n> > Auto\n> > > > > to\n> > > > > + Disabled, the provided ExposureTime is applied.\n> > > > >\n> > > > > - AnalogueGain:\n> > > > > type: float\n> > > > > @@ -144,17 +244,85 @@ controls:\n> > > > > The value of the control specifies the gain multiplier\n> > applied to\n> > > > > all\n> > > > > colour channels. This value cannot be lower than 1.0.\n> > > > >\n> > > > > - Setting this value means that it is now fixed and the AE\n> > > > > algorithm may\n> > > > > - not change it. Setting it back to zero returns it to the\n> > control\n> > > > > of the\n> > > > > - AE algorithm.\n> > > > > + This control will only take effect if AnalogueGainMode is\n> > > > > Disabled. If\n> > > > > + this control is set when AnalogueGainMode is Auto, the\nvalue\n> > will\n> > > > > be\n> > > > > + ignored and will not be retained.\n> > > > > +\n> > > > > + When reported in metadata, this control indicates what\n> > analogue\n> > > > > gain\n> > > > > + was used for the current request, regardless of\n> > AnalogueGainMode.\n> > > > > + AnalogueGainMode will indicate the source of the analogue\n> > gain\n> > > > > value,\n> > > > > + whether it came from the AE algorithm or not.\n> > > > > +\n> > > > > + \\sa ExposureTime\n> > > > > + \\sa AnalogueGainMode\n> > > > > +\n> > > > > + - AnalogueGainMode:\n> > > > > + type: int32_t\n> > > > > + description: |\n> > > > > + Controls the source of the analogue gain that is applied\nto\n> > the\n> > > > > image\n> > > > > + sensor. When set to Auto, the AE algorithm computes the\n> > analogue\n> > > > > gain\n> > > > > + and configures the image sensor accordingly. When set to\n> > Disabled,\n> > > > > + analogue gain specified in AnalogueGain is applied to the\n> > image\n> > > > > sensor.\n> > > > > + If AnalogueGain is not set, then the value last computed\nby\n> > the AE\n> > > > > + algorithm when the mode was Auto will be used.\n> > > > > +\n> > > > > + If AnalogueGain is not set and the mode is\n> > > > > AnalogueGainModeDisabled and\n> > > > > + AE was never Auto (either because the camera started in\n> > Disabled\n> > > > > mode,\n> > > > > + or Auto is not supported by the camera), the camera\nshould\n> > use a\n> > > > > + best-effort default value.\n> > > > > +\n> > > > > + When AnalogueGainMode is set Auto, the value set in\n> > AnalogueGain\n> > > > > is\n> > > > > + ignored and is not retained. This means that if\n> > AnalogueGainMode\n> > > > > is set\n> > > > > + to Disabled and AnalogueGain is not also set, the\nanalogue\n> > gain\n> > > > > that\n> > > > > + was last computed by the AE algorithm while the mode was\nAuto\n> > > > > will be\n> > > > > + applied to the sensor.\n> > > > >\n> > > > > - \\sa ExposureTime AeEnable\n> > > > > + If AnalogueGainModeDisabled is supported, the\nAnalogueGain\n> > > > > control must\n> > > > > + also be supported.\n> > > > > +\n> > > > > + The set of AnalogueGainMode modes that are supported by\nthe\n> > > > > camera must\n> > > > > + have an intersection with the supported set of\n> > ExposureTimeMode\n> > > > > modes.\n> > > > > +\n> > > > > + As it takes a few frames to apply the analogue gain,\nthere\n> > is a\n> > > > > period of\n> > > > > + time between submitting a request with AnalogueGainMode\nset\n> > to\n> > > > > Disabled\n> > > > > + and the analogue gain component of the AE actually being\n> > disabled,\n> > > > > + during which the AE algorithm can still update the\nanalogue\n> > gain.\n> > > > > If an\n> > > > > + application is switching from automatic and manual\ncontrol\n> > and\n> > > > > wishes\n> > > > > + to eliminate any flicker during the switch, the following\n> > > > > procedure is\n> > > > > + recommended.\n> > > > > +\n> > > > > + 1. Start with AnalogueGainMode set to Auto\n> > > > > +\n> > > > > + 2. Set AnalogueGainMode to Disabled\n> > > > > +\n> > > > > + 3. Wait for the first request to be output that has\n> > > > > AnalogueGainMode\n> > > > > + set to Disabled\n> > > > > +\n> > > > > + 4. Copy the value reported in AnalogueGain into a new\n> > request, and\n> > > > > + submit it\n> > > > > +\n> > > > > + 5. Proceed to run manual analogue gain\n> > > > >\n> > > > +\n> > > > > + \\sa AnalogueGain\n> > > > > + enum:\n> > > > > + - name: AnalogueGainModeAuto\n> > > > > + value: 0\n> > > > > + description: |\n> > > > > + The analogue gain will be calculated automatically\nand\n> > set by\n> > > > > the\n> > > > > + AE algorithm. If AnalogueGain is set while this mode\nis\n> > > > > active, it\n> > > > > + will be ignored, and it will also not be retained.\n> > > > > + - name: AnalogueGainModeDisabled\n> > > > > + value: 1\n> > > > > + description: |\n> > > > > + The analogue gain will not be updated by the AE\n> > algorithm. It\n> > > > > will\n> > > > > + come from the last calculated value when the mode was\n> > Auto,\n> > > > > or from\n> > > > > + the value specified in AnalogueGain.\n> > > > >\n> > > > > - \\todo Document the interactions between AeEnable and\nsetting\n> > a\n> > > > > fixed\n> > > > > - value for this control. Consider interactions with other\nAE\n> > > > > features,\n> > > > > - such as aperture and aperture/shutter priority mode, and\n> > decide if\n> > > > > - control of which features should be automatically\nadjusted\n> > > > > shouldn't\n> > > > > - better be handled through a separate AE mode control.\n> > > > > + When transitioning from Auto to Disabled mode the\nlast\n> > > > > computed\n> > > > > + gain value is used until a new value is specified\n> > through the\n> > > > > + AnalogueGain control. If an AnalogueGain value is\n> > specified\n> > > > > in the\n> > > > > + same request where the AnalogueGainMode is set to\n> > Disabled,\n> > > > > the\n> > > > > + provided AnalogueGain is applied.\n> > > > >\n> > > > > - Brightness:\n> > > > > type: float\n> > > > > @@ -477,36 +645,6 @@ controls:\n> > > > > High quality aberration correction which might\nreduce the\n> > > > > frame\n> > > > > rate.\n> > > > >\n> > > > > - - AeState:\n> > > > > - type: int32_t\n> > > > > - draft: true\n> > > > > - description: |\n> > > > > - Control to report the current AE algorithm state.\nCurrently\n> > > > > identical to\n> > > > > - ANDROID_CONTROL_AE_STATE.\n> > > > > -\n> > > > > - Current state of the AE algorithm.\n> > > > > - enum:\n> > > > > - - name: AeStateInactive\n> > > > > - value: 0\n> > > > > - description: The AE algorithm is inactive.\n> > > > > - - name: AeStateSearching\n> > > > > - value: 1\n> > > > > - description: The AE algorithm has not converged yet.\n> > > > > - - name: AeStateConverged\n> > > > > - value: 2\n> > > > > - description: The AE algorithm has converged.\n> > > > > - - name: AeStateLocked\n> > > > > - value: 3\n> > > > > - description: The AE algorithm is locked.\n> > > > > - - name: AeStateFlashRequired\n> > > > > - value: 4\n> > > > > - description: The AE algorithm would need a flash for\ngood\n> > > > > results\n> > > > > - - name: AeStatePrecapture\n> > > > > - value: 5\n> > > > > - description: |\n> > > > > - The AE algorithm has started a pre-capture metering\n> > session.\n> > > > > - \\sa AePrecaptureTrigger\n> > > > > -\n> > > > > - AfState:\n> > > > > type: int32_t\n> > > > > draft: true\n> > > > > --\n> > > > > 2.30.2\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 040C6BE173\n\tfor ;\n\tTue, 5 Jul 2022 09:44:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38CCA6330E;\n\tTue, 5 Jul 2022 11:44:04 +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 D2B2C60401\n\tfor ;\n\tTue, 5 Jul 2022 11:44:02 +0200 (CEST)","by mail-lf1-x12a.google.com with SMTP id t25so19602191lfg.7\n\tfor ;\n\tTue, 05 Jul 2022 02:44:02 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657014244;\n\tbh=M7kUr8gTbkDPgEWTPhYpnSHHC4R/FQFgwfNCUujMBz0=;\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=YJ61M5RVSXT2RFChbLMj46f24YfS6QBoGHrDTi7WD4/zOrMx3tKEInmtYKgo0gQev\n\tf8XR/TyTjkvY9haC2wgtlMwS9PUmzIKleRwpiYv/UVqjxRq167FTzIn0IPHusbBPWz\n\t46YB/avtJZTT2zyOL8LDYT1ciIsfbsVf5ml0MHKoG1B/sChfwy9QDywTIGPKELqRFK\n\tzX8KmC5eaVA2L5ZIk4CfwYGQspG3WC1nGkmp8MTAM6wnVOxj/yfcMSHIfSNBjLsUwT\n\tbMSaUjHaWdGLEnSRvv1PJ1jJcbMIew3171u3vS363HJZE1SDk2PSk/9wY1bLNBKsfv\n\t01xhJuWifkyiA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ek8caprqwuIQBU14rt1SrG6evxbqxVp0hFSZIrqaA9w=;\n\tb=a7nNU+xguXqx/Xw5f8LX6T5w6q7zdT3ZWmyC6CfgF37+/XuuHFaNew2Q7SFolc9Aca\n\tncX714XSUxOFi/t6SNZ/ucLpfyRiGhYi7tayuWU2YVNZjuVVfYsT4G4GEel4Gr7GsU8f\n\tQJA3PYrzTj/Dj+o5nLmVijCB9a67gmTn8rqu4itztMn41Aiw3Z81y7e1heE5Wk+Tbu6u\n\tL1f1dUzoT1If4Vo+WatIYW5hYFOlqsHlpnXyEpOwtVvdErfWm4zcGYKirt48DC/z8Ykm\n\tp9QDaOciP2dqzSeEGfJsLQGtEK22TmNYGMujsmYJsh6BvTDFFKuqITzuOH3O34V2c7be\n\tm1KA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"a7nNU+xg\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ek8caprqwuIQBU14rt1SrG6evxbqxVp0hFSZIrqaA9w=;\n\tb=2UMTZABzm/Ik7F2cjU8+n6RYmH7MhLTmhUa+Z2LLh1rRObbQgpoqyie1Cnxf2vGv8i\n\tZ6HigHahPG0kJK8DwiaVcoDDfmZgLl4ooZbI1l26XN7bDE6biqH8MlOXoTjJup695exQ\n\tBUZQtPAa0EzkvX4abkvQGolyuWuhmWskkGGVLq2zZwzgeYR0IQj8LB3tWGet6Wt+Ca7C\n\tQDKKDPXi1NCua4+eQ0Xr0QFxHkirV2Hwfiz0K1LzZa1NnjHPPSw1dDIW28Q8pB95tYtk\n\tSpCVVlqiOe1pljUvzcyjY3bLDufYWss0K7h92lLw9cHZ3NOG+rasO27gAzO4i/89LU7H\n\tTZ6A==","X-Gm-Message-State":"AJIora8FdlTxycjBPuiN7NBl3AXeHN6Gw/2Fz2/3QtR05uBPOj8IdjyC\n\tdWpireUCo7cZE4FWhc719oJn6slVb4Cmd37yvvrwqF3CeHTegSqT","X-Google-Smtp-Source":"AGRyM1tSHGU1+juUpZ6kaBGz2tpYaYxvLHM05ySnmB1kiEIpB77U3jvntWFiW8C+AC5cxsVNqDpe52q6fsmOk1iYOgE=","X-Received":"by 2002:a05:6512:3581:b0:47f:6e4d:bf6a with SMTP id\n\tm1-20020a056512358100b0047f6e4dbf6amr21072213lfr.63.1657014242023;\n\tTue, 05 Jul 2022 02:44:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20220518134728.777709-1-paul.elder@ideasonboard.com>\n\t<20220518134728.777709-2-paul.elder@ideasonboard.com>\n\t\n\t<20220704153024.xzolxogrxxxwrry4@uno.localdomain>\n\t\n\t<20220705091702.kglucpzutivmgclu@uno.localdomain>","In-Reply-To":"<20220705091702.kglucpzutivmgclu@uno.localdomain>","Date":"Tue, 5 Jul 2022 10:43:46 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000048ae3805e30bb1c4\"","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] controls: Reorganize the\n\tAE-related 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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]