[{"id":32173,"web_url":"https://patchwork.libcamera.org/comment/32173/","msgid":"","date":"2024-11-14T14:36:41","subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch. \n\nOn Wed, Nov 13, 2024 at 10:12:50PM +0900, Paul Elder wrote:\n> Document the design and rationale for the AE-related controls.\n> Also add documentation for the controls.\n> \n> Signed-off-by: Paul Elder \n> Signed-off-by: Jacopo Mondi \n> \n> ---\n> Changes in v3:\n> - merge the control documentation into the same document (including the\n> patch)\n> - this is because it was a bit unwieldy to put it in\n> control_ids.cpp.in, now that it's used generically to generate\n> control ids of all namespaces\n> ---\n> Documentation/design/ae.rst | 348 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 348 insertions(+)\n> create mode 100644 Documentation/design/ae.rst\n> \n> diff --git a/Documentation/design/ae.rst b/Documentation/design/ae.rst\n> new file mode 100644\n> index 000000000000..f121afecea5a\n> --- /dev/null\n> +++ b/Documentation/design/ae.rst\n> @@ -0,0 +1,348 @@\n> +.. SPDX-License-Identifier: CC-BY-SA-4.0\n> +\n> +Design of Exposure and Gain controls\n> +====================================\n> +\n> +This document explains the design and rationale of the controls related to\n> +exposure and gain. This includes the all-encompassing auto-exposure (AE), the\n> +manual exposure control, and the manual gain control.\n> +\n> +Description of the problem\n> +--------------------------\n> +\n> +Sub controls\n> +^^^^^^^^^^^^\n> +\n> +There are more than one control that make up exposure: exposure, gain, and\n> +aperture (though for now we will not consider aperture). We already had\n> +individual controls for setting the values of manual exposure and manual gain,\n> +but for switching between auto mode and manual mode we only had a high-level\n> +boolean AeEnable control that would set *both* exposure and gain to auto mode\n> +or manual mode; we had no way to set one to auto and the other to manual.\n> +\n> +So, we need to introduce two new controls to act as \"levers\" to indicate\n> +individually for exposure and gain if the value would come from AEGC or if it\n> +would come from the manual control value.\n> +\n> +Aperture priority\n> +^^^^^^^^^^^^^^^^^\n> +\n> +We eventually will support aperture, and so whatever our solution is for having\n> +only some controls on auto and the others on manual needs to be extensible.\n> +\n> +Flickering when going from auto to manual\n> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n> +\n> +When a manual exposure or gain value is requested by the application, it costs\n> +a few frames worth of time for them to take effect. This means that during a\n> +transition from auto to manual, there would be flickering in the control values\n> +and the transition won't be smooth.\n> +\n> +Take for instance the following flow, where we start on auto exposure (which\n> +for the purposes of the example increments by 1 each frame) and we want to\n> +switch seamlessly to manual exposure, which involves copying the exposure value\n> +computed by the auto exposure algorithm:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> +\n> + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> + Exp requested: N/A N/A N/A 2 2 2 2\n> + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> +\n> + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> + Exp used: 0 1 2 3 4 2 2\n> +\n> +As we can see, after frame N+2 completes, we copy the exposure value that was\n> +used for frame N+2 (which was computed by AE algorithm), and queue that value\n> +into request N+3 with manual mode on. However, as it takes two frames for the\n> +exposure to be set, the exposure still changes since it is set by AE, and we\n> +get a flicker in the exposure during the switch from auto to manual.\n> +\n> +A solution is to *not submit* any exposure value when manual mode is enabled,\n> +and wait until the manual mode as been \"applied\" before copying the exposure\n> +value:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> +\n> + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> + Exp requested: N/A N/A N/A None None None 5\n> + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> +\n> + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> + Exp used: 0 1 2 3 4 5 5\n> +\n> +In practice, this works. However, libcamera has a policy where once a control\n> +is submitted, its value is saved and does not need to be resubmitted (even\n> +though this isn't implemented yet). So if the manual exposure value was set\n> +while auto mode was on, in theory the value would be saved, so when manual mode\n> +is enabled, the exposure value that was previously set would immediately be\n> +used. Clearly this solution isn't correct, but it can serve as the basis for a\n> +proper solution, with some more rigorous rules.\n> +\n> +Existing solutions\n> +------------------\n> +\n> +Raspberry Pi\n> +^^^^^^^^^^^^\n> +\n> +The raspberry pi IPA got around the lack of individual AeEnable controls for\n> +exposure and gain by using magic values. When AeEnable was false, if one of the\n> +manual control values was set to 0 then the value computed by AEGC would be\n> +used for just that control. This solution isn't desirable, as it prevents\n> +that magic value from being used as a valid value.\n> +\n> +To get around the flickering issue, when AeEnable was false, the raspberry pi\n> +AEGC would simply stop updating the values to be set. As mentioned above, since\n> +the value retention mechanism hasn't actually been implemented yet, this\n> +worked. But, it's not a proper solution.\n> +\n> +Android\n> +^^^^^^^\n> +\n> +The Android HAL specification requires that exposure and gain (sensitivity)\n> +must both be manual or both be auto. It cannot be that one is manual while the\n> +other is auto, so they simply don't support sub controls.\n> +\n> +For the flickering issue, the Android HAL has an AeLock control. To transition\n> +from auto to manual, the application would keep AE on auto, and turn on the\n> +lock. Once the lock has propagated through, then the value can be copied from\n> +the result into the request and the lock disabled and the mode set to manual.\n> +\n> +The problem with this solution is, besides the extra complexity, that it is\n> +ambiguous what happens if there is a state transition from manual to locked\n> +(even though it's a state transition that doesn't make sense). If locked is\n> +defined to \"use the last automatically computed values\" then it could use the\n> +values from the last time it AE was set to auto, or it would be undefined if AE\n> +was never auto (eg. it started out as manual), or if AE is implemented to run\n> +in the background it could just use the current values that are computed. If\n> +locked is defined to \"use the last value that was set\" there would be less\n> +ambiguity. Still, it's better if we can make it impossible to execute this\n> +nonsensical state transition, and if we can reduce the complexity of having\n> +this extra control or extra setting on a lever.\n> +\n> +Summary of goals\n> +----------------\n> +\n> + - We need a lock of some sort, to instruct the AEGC to not update output\n> + results\n> +\n> + - We need manual modes, to override the values computed by the AEGC\n> +\n> + - We need to support seamless transitions from auto to manual, and do so\n> + without flickering\n> +\n> + - We need custom minimum values for the manual controls; that is, no magic\n> + values for enabling/disabling auto\n> +\n> + - All of these need to be done with AE sub-controls (exposure time, analogue\n> + gain) and be extensible to aperture in the future\n> +\n> +Our solution\n> +------------\n> +\n> +A diagram of our solution:\n> +\n> +::\n> +\n> + +----------------------------+-------------+------------------+-----------------+\n> + | INPUT | ALGORITHM | RESULT | OUTPUT |\n> + +----------------------------+-------------+------------------+-----------------+\n> +\n> + ExposureTimeMode ExposureTimeMode\n> + ---------------------+----------------------------------------+----------------->\n> + 0: Auto | |\n> + 1: Manual | V\n> + | |\\\n> + | | \\\n> + | /----------------------------------> | 1| ExposureTime\n> + | | +-------------+ exposure time | | -------------->\n> + \\--)--> | | --------------> | 0|\n> + ExposureTime | | | | /\n> + ------------------------+--> | | |/\n> + | | AeState\n> + | AEGC | ----------------------------------->\n> + AnalogueGain | |\n> + ------------------------+--> | | |\\\n> + | | | | \\\n> + /--)--> | | --------------> | 0| AnalogueGain\n> + | | +-------------+ analogue gain | | -------------->\n> + | \\----------------------------------> | 1|\n> + | | /\n> + | |/\n> + | ^\n> + AnalogueGainMode | | AnalogueGainMode\n> + ---------------------+----------------------------------------+----------------->\n> + 0: Auto\n> + 1: Manual\n> +\n> +\n> +The diagram is divided in four sections horizontally:\n> +\n> + - Input: The values received from the request controls\n> +\n> + - Algorithm: The algorithm itself\n> +\n> + - Result: The values calculated by the algorithm\n> +\n> + - Output: The values that sent in result metadata and applied to the device\n> +\n> +The four input controls are divided between manual values (ExposureTime and\n> +AnalogueGain), and operation modes (ExposureTimeMode and AnalogueGainMode). The\n> +former are the manual values, the latter control how they're applied. The two\n> +modes are independent from each other, and each can take one of two values:\n> +\n> + - Auto (0): The AGC computes the value normally. The AGC result is applied\n> + to the output. The manual value is ignored *and is not retained*.\n> +\n> + - Manual (1): The AGC uses the manual value internally. The corresponding\n> + manual control from the request is applied to the output. The AGC result\n> + is ignored.\n> +\n> +The AeState control reports the state of the unified AEGC block. If both\n> +ExposureTimeMode and AnalogueGainMode are set to disabled then it will report\n> +Idle. If at least one of the two is set to auto, then AeState will report\n> +if the AEGC has Converged or not (Searching). This control replaces the old\n> +AeLocked control, as it was insufficient for reporting the AE state.\n> +\n> +There is a caveat to the disabled mode: the manual control value is not\n> +retained if it is set during auto mode. This means that if the disabled mode is\n> +entered without also setting the manual value, then it will enter a state\n> +similar to \"locked\", where the last automatically computed value while the mode\n> +was auto will be used. Once the manual value is set, then that will be used and\n> +retained as usual.\n> +\n> +This simulates an auto -> locked -> manual or auto -> manual state transition,\n> +and makes it impossible to do the nonsensical manual -> locked state\n> +transition.\n> +\n> +We specifically do not have a \"master AE control\" like the old AeEnable. This\n> +is because we have the individual mode controls, and if we had a master AE\n> +control it would be a \"control that sets other controls\", which could easily\n> +get out of control.\n> +\n> +With this solution, the earlier example would become:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N+2 | | N+3 | | N+4 | | N+5 | | N+6 | | N+7 | | N+8 | | N+9 | | N+10|\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + Mode requested: Auto Disab Disab Disab Disab Disab Disab Disab Disab\n> + Exp requested: N/A None None None None 10 None 10 10\n> + Set in Frame: N+4 N+5 N+6 N+7 N+8 N+9 N+10 N+10 N+10\n> +\n> + Mode used: Auto Auto Auto Disab Disab Disab Disab Disab Disab\n> + Exp used: 2 3 4 5 5 5 5 10 10\n> +\n> +This example is extended by a few frames to exhibit the simulated \"locked\"\n> +state. At frame N+5 the application has confirmed that the auto mode has been\n> +disabled, but does not provide a manual value until request N+7. Thus, the\n> +value that is used in requests N+5 and N+6 (where the mode is disabled), comes\n> +from the last value that was used when the mode was auto, which comes from\n> +frame N+4.\n> +\n> +Then, in N+7, a manual value of 10 is supplied. It takes until frame N+9 for\n> +the exposure to be applied. N+8 does not supply a manual value, but the last\n> +supplied value is retained, so a manual value of 10 is still used and set in\n> +frame N+10.\n> +\n> +Although this behavior is the same as what we had with waiting for the manual\n> +mode to propagate (in the section \"Description of the problem\"), this time it\n> +is correct as we have defined specifically that if a manual value was specified\n> +while the mode was auto, it will not be retained.\n> +\n> +Description of the controls\n> +===========================\n> +\n> + libcamera offers the following controls related to exposure and gain:\n> +\n> + - AnalogueGain\n> +\n> + - AnalogueGainMode\n> +\n> + - ExposureTime\n> +\n> + - ExposureTimeMode\n> +\n> + - AeState\n> +\n> + Auto-exposure and auto-gain can be enabled and disabled separately using the\n> + ExposureTimeMode and AnalogueGainMode controls respectively. There is no\n> + overarching AeEnable control.\n> +\n> + For each of exposure and gain, we can model it with three states: auto,\n> + locked, and manual. Note that AnalogueGainMode and ExposureTimeMode only\n> + have two values, as the locked state is simulated.\n> +\n> + ::\n> +\n> + /---------------------------------\\\n> + | |\n> + V |\n> + +--------+ +--------+\n> + | | ---------------------> | |\n> + | Auto | +--------+ | Manual |\n> + | | ----> | Locked | ----> | |\n> + +--------+ +--------+ +--------+\n> + ^ |\n> + | |\n> + \\----------------/\n> +\n> + Notice from the state diagram that locked to manual is a one-way state\n> + change, as the reverse direction is nonsensical (see the design document for\n> + more details on this topic).\n> +\n> + The exposure/gain is in the Auto state when\n> + ExposureTimeMode/AnalogueGainMode is set to Auto. In this state, the value\n> + that is computed by the AE algorithm is applied to the image sensor. Any\n> + value that is supplied in the ExposureTime/AnalogueGain control is ignored\n> + and is not retained.\n> +\n> + If ExposureTimeMode/AnalogueGainMode is set to Manual, it can put us in\n> + either the Locked or Manual state. The difference is in if\n> + ExposureTime/AnalogueGain has been supplied. If it has not yet been\n> + supplied, then we are in the Locked state. If it has been supplied, then we\n> + are in the Manual state.\n> +\n> + In both the Locked state and the Manual state the exposure/gain value does\n> + not come from the AE algorithm. In the Locked state the value comes from the\n> + last value computed by the AE algorithm while the state was Auto, or if the\n> + state was never Auto (e.g. we started in Locked, or the camera doesn't\n> + support Auto), then the value should be a best-effort default value. In the\n> + Manual state the value comes from the value supplied in the\n> + ExposureTime/AnalogueGain control.\n> +\n> + To transition from the Locked state to the Manual state, a value needs to be\n> + submitted in ExposureTime/AnalogueGain. Once the state has transitioned to\n> + Manual, then this value will be retained, and doesn't need to be resubmitted\n> + if it doesn't change.\n> +\n> + To transition to the Auto state, simply set\n> + ExposureTimeMode/AnalogueGainMode to Auto.\n> +\n> +\n> + The AeState metadata reports the state of the AE algorithm. As AE cannot\n> + compute exposure and gain separately, the state of the AE component is\n> + unified. There are three states: Idle, Searching, and Converged.\n> +\n> + The state shall be Idle if both ExposureTimeMode and AnalogueGainMode\n> + are set to Manual. If the camera only supports one of the two controls,\n> + then the state shall be Idle if that one control is set to Manual. If\n> + the camera does not support Manual for at least one of the two controls,\n> + then the state will never be Idle, as AE will always be running.\n> +\n> + The state shall be Searching if at least one of exposure or gain calculated\n> + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> + *and* the value(s) have not converged yet.\n> +\n> + The state shall be Converged if at least one of exposure or gain calculated\n> + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> + *and* the value(s) have converged.\n\nThat is a lot of stuff to go through. I like the model. One thing that\ncame to my mind: What is the reason to use a enum for the ExposureTimeMode\ncontrol instead of a bool? Are there further extensions planned? GenICam\ndevices often have an additional mode \"Once\" that runs the control loop\nuntil converged and then snaps back to off/manual.\n\nBut overall I believe we should get that it.\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\n> -- \n> 2.39.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 E7E03C324C\n\tfor ;\n\tThu, 14 Nov 2024 14:37:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DDF5065848;\n\tThu, 14 Nov 2024 15:37:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A8D9657E0\n\tfor ;\n\tThu, 14 Nov 2024 15:36:44 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:cd64:b5ef:1d95:ef1c])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 764C6827;\n\tThu, 14 Nov 2024 15:36:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IoTCYfM+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731594990;\n\tbh=Mz6lJSH6/3WPHTaLFBMNGraa4WlhvxMMjo0wCGny73o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IoTCYfM+AzFJIW0dv8oIQSyTK1/0s1bQegOSQ4yBwrZOMCb6EbJmaWCFDr9jLPptR\n\tuKqKakWvmkHtwaZI/spvIr8kFWKJ4ImdKcDdI6ntoyl/H3zzDObVCHtfLh2yO/SWKo\n\tjS4LpmaLhBQHc2wt+rRS+T5kdeI2ogkrV47gX0F4=","Date":"Thu, 14 Nov 2024 15:36:41 +0100","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com, \n\tjacopo.mondi@ideasonboard.com, naush@raspberrypi.com,\n\tdavid.plowman@raspberrypi.com, Jacopo Mondi ","Subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","Message-ID":"","References":"<20241113131256.3170817-1-paul.elder@ideasonboard.com>\n\t<20241113131256.3170817-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241113131256.3170817-3-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32189,"web_url":"https://patchwork.libcamera.org/comment/32189/","msgid":"","date":"2024-11-15T13:13:02","subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Nov 14, 2024 at 03:36:41PM +0100, Stefan Klug wrote:\n> Hi Paul,\n> \n> Thank you for the patch. \n> \n> On Wed, Nov 13, 2024 at 10:12:50PM +0900, Paul Elder wrote:\n> > Document the design and rationale for the AE-related controls.\n> > Also add documentation for the controls.\n> > \n> > Signed-off-by: Paul Elder \n> > Signed-off-by: Jacopo Mondi \n> > \n> > ---\n> > Changes in v3:\n> > - merge the control documentation into the same document (including the\n> > patch)\n> > - this is because it was a bit unwieldy to put it in\n> > control_ids.cpp.in, now that it's used generically to generate\n> > control ids of all namespaces\n> > ---\n> > Documentation/design/ae.rst | 348 ++++++++++++++++++++++++++++++++++++\n> > 1 file changed, 348 insertions(+)\n> > create mode 100644 Documentation/design/ae.rst\n> > \n> > diff --git a/Documentation/design/ae.rst b/Documentation/design/ae.rst\n> > new file mode 100644\n> > index 000000000000..f121afecea5a\n> > --- /dev/null\n> > +++ b/Documentation/design/ae.rst\n> > @@ -0,0 +1,348 @@\n> > +.. SPDX-License-Identifier: CC-BY-SA-4.0\n> > +\n> > +Design of Exposure and Gain controls\n> > +====================================\n> > +\n> > +This document explains the design and rationale of the controls related to\n> > +exposure and gain. This includes the all-encompassing auto-exposure (AE), the\n> > +manual exposure control, and the manual gain control.\n> > +\n> > +Description of the problem\n> > +--------------------------\n> > +\n> > +Sub controls\n> > +^^^^^^^^^^^^\n> > +\n> > +There are more than one control that make up exposure: exposure, gain, and\n> > +aperture (though for now we will not consider aperture). We already had\n> > +individual controls for setting the values of manual exposure and manual gain,\n> > +but for switching between auto mode and manual mode we only had a high-level\n> > +boolean AeEnable control that would set *both* exposure and gain to auto mode\n> > +or manual mode; we had no way to set one to auto and the other to manual.\n> > +\n> > +So, we need to introduce two new controls to act as \"levers\" to indicate\n> > +individually for exposure and gain if the value would come from AEGC or if it\n> > +would come from the manual control value.\n> > +\n> > +Aperture priority\n> > +^^^^^^^^^^^^^^^^^\n> > +\n> > +We eventually will support aperture, and so whatever our solution is for having\n> > +only some controls on auto and the others on manual needs to be extensible.\n> > +\n> > +Flickering when going from auto to manual\n> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n> > +\n> > +When a manual exposure or gain value is requested by the application, it costs\n> > +a few frames worth of time for them to take effect. This means that during a\n> > +transition from auto to manual, there would be flickering in the control values\n> > +and the transition won't be smooth.\n> > +\n> > +Take for instance the following flow, where we start on auto exposure (which\n> > +for the purposes of the example increments by 1 each frame) and we want to\n> > +switch seamlessly to manual exposure, which involves copying the exposure value\n> > +computed by the auto exposure algorithm:\n> > +\n> > +::\n> > +\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > +\n> > + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> > + Exp requested: N/A N/A N/A 2 2 2 2\n> > + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> > +\n> > + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> > + Exp used: 0 1 2 3 4 2 2\n> > +\n> > +As we can see, after frame N+2 completes, we copy the exposure value that was\n> > +used for frame N+2 (which was computed by AE algorithm), and queue that value\n> > +into request N+3 with manual mode on. However, as it takes two frames for the\n> > +exposure to be set, the exposure still changes since it is set by AE, and we\n> > +get a flicker in the exposure during the switch from auto to manual.\n> > +\n> > +A solution is to *not submit* any exposure value when manual mode is enabled,\n> > +and wait until the manual mode as been \"applied\" before copying the exposure\n> > +value:\n> > +\n> > +::\n> > +\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > +\n> > + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> > + Exp requested: N/A N/A N/A None None None 5\n> > + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> > +\n> > + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> > + Exp used: 0 1 2 3 4 5 5\n> > +\n> > +In practice, this works. However, libcamera has a policy where once a control\n> > +is submitted, its value is saved and does not need to be resubmitted (even\n> > +though this isn't implemented yet). So if the manual exposure value was set\n> > +while auto mode was on, in theory the value would be saved, so when manual mode\n> > +is enabled, the exposure value that was previously set would immediately be\n> > +used. Clearly this solution isn't correct, but it can serve as the basis for a\n> > +proper solution, with some more rigorous rules.\n> > +\n> > +Existing solutions\n> > +------------------\n> > +\n> > +Raspberry Pi\n> > +^^^^^^^^^^^^\n> > +\n> > +The raspberry pi IPA got around the lack of individual AeEnable controls for\n> > +exposure and gain by using magic values. When AeEnable was false, if one of the\n> > +manual control values was set to 0 then the value computed by AEGC would be\n> > +used for just that control. This solution isn't desirable, as it prevents\n> > +that magic value from being used as a valid value.\n> > +\n> > +To get around the flickering issue, when AeEnable was false, the raspberry pi\n> > +AEGC would simply stop updating the values to be set. As mentioned above, since\n> > +the value retention mechanism hasn't actually been implemented yet, this\n> > +worked. But, it's not a proper solution.\n> > +\n> > +Android\n> > +^^^^^^^\n> > +\n> > +The Android HAL specification requires that exposure and gain (sensitivity)\n> > +must both be manual or both be auto. It cannot be that one is manual while the\n> > +other is auto, so they simply don't support sub controls.\n> > +\n> > +For the flickering issue, the Android HAL has an AeLock control. To transition\n> > +from auto to manual, the application would keep AE on auto, and turn on the\n> > +lock. Once the lock has propagated through, then the value can be copied from\n> > +the result into the request and the lock disabled and the mode set to manual.\n> > +\n> > +The problem with this solution is, besides the extra complexity, that it is\n> > +ambiguous what happens if there is a state transition from manual to locked\n> > +(even though it's a state transition that doesn't make sense). If locked is\n> > +defined to \"use the last automatically computed values\" then it could use the\n> > +values from the last time it AE was set to auto, or it would be undefined if AE\n> > +was never auto (eg. it started out as manual), or if AE is implemented to run\n> > +in the background it could just use the current values that are computed. If\n> > +locked is defined to \"use the last value that was set\" there would be less\n> > +ambiguity. Still, it's better if we can make it impossible to execute this\n> > +nonsensical state transition, and if we can reduce the complexity of having\n> > +this extra control or extra setting on a lever.\n> > +\n> > +Summary of goals\n> > +----------------\n> > +\n> > + - We need a lock of some sort, to instruct the AEGC to not update output\n> > + results\n> > +\n> > + - We need manual modes, to override the values computed by the AEGC\n> > +\n> > + - We need to support seamless transitions from auto to manual, and do so\n> > + without flickering\n> > +\n> > + - We need custom minimum values for the manual controls; that is, no magic\n> > + values for enabling/disabling auto\n> > +\n> > + - All of these need to be done with AE sub-controls (exposure time, analogue\n> > + gain) and be extensible to aperture in the future\n> > +\n> > +Our solution\n> > +------------\n> > +\n> > +A diagram of our solution:\n> > +\n> > +::\n> > +\n> > + +----------------------------+-------------+------------------+-----------------+\n> > + | INPUT | ALGORITHM | RESULT | OUTPUT |\n> > + +----------------------------+-------------+------------------+-----------------+\n> > +\n> > + ExposureTimeMode ExposureTimeMode\n> > + ---------------------+----------------------------------------+----------------->\n> > + 0: Auto | |\n> > + 1: Manual | V\n> > + | |\\\n> > + | | \\\n> > + | /----------------------------------> | 1| ExposureTime\n> > + | | +-------------+ exposure time | | -------------->\n> > + \\--)--> | | --------------> | 0|\n> > + ExposureTime | | | | /\n> > + ------------------------+--> | | |/\n> > + | | AeState\n> > + | AEGC | ----------------------------------->\n> > + AnalogueGain | |\n> > + ------------------------+--> | | |\\\n> > + | | | | \\\n> > + /--)--> | | --------------> | 0| AnalogueGain\n> > + | | +-------------+ analogue gain | | -------------->\n> > + | \\----------------------------------> | 1|\n> > + | | /\n> > + | |/\n> > + | ^\n> > + AnalogueGainMode | | AnalogueGainMode\n> > + ---------------------+----------------------------------------+----------------->\n> > + 0: Auto\n> > + 1: Manual\n> > +\n> > +\n> > +The diagram is divided in four sections horizontally:\n> > +\n> > + - Input: The values received from the request controls\n> > +\n> > + - Algorithm: The algorithm itself\n> > +\n> > + - Result: The values calculated by the algorithm\n> > +\n> > + - Output: The values that sent in result metadata and applied to the device\n> > +\n> > +The four input controls are divided between manual values (ExposureTime and\n> > +AnalogueGain), and operation modes (ExposureTimeMode and AnalogueGainMode). The\n> > +former are the manual values, the latter control how they're applied. The two\n> > +modes are independent from each other, and each can take one of two values:\n> > +\n> > + - Auto (0): The AGC computes the value normally. The AGC result is applied\n> > + to the output. The manual value is ignored *and is not retained*.\n> > +\n> > + - Manual (1): The AGC uses the manual value internally. The corresponding\n> > + manual control from the request is applied to the output. The AGC result\n> > + is ignored.\n> > +\n> > +The AeState control reports the state of the unified AEGC block. If both\n> > +ExposureTimeMode and AnalogueGainMode are set to disabled then it will report\n> > +Idle. If at least one of the two is set to auto, then AeState will report\n> > +if the AEGC has Converged or not (Searching). This control replaces the old\n> > +AeLocked control, as it was insufficient for reporting the AE state.\n> > +\n> > +There is a caveat to the disabled mode: the manual control value is not\n> > +retained if it is set during auto mode. This means that if the disabled mode is\n> > +entered without also setting the manual value, then it will enter a state\n> > +similar to \"locked\", where the last automatically computed value while the mode\n> > +was auto will be used. Once the manual value is set, then that will be used and\n> > +retained as usual.\n> > +\n> > +This simulates an auto -> locked -> manual or auto -> manual state transition,\n> > +and makes it impossible to do the nonsensical manual -> locked state\n> > +transition.\n> > +\n> > +We specifically do not have a \"master AE control\" like the old AeEnable. This\n> > +is because we have the individual mode controls, and if we had a master AE\n> > +control it would be a \"control that sets other controls\", which could easily\n> > +get out of control.\n> > +\n> > +With this solution, the earlier example would become:\n> > +\n> > +::\n> > +\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > + | N+2 | | N+3 | | N+4 | | N+5 | | N+6 | | N+7 | | N+8 | | N+9 | | N+10|\n> > + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> > + Mode requested: Auto Disab Disab Disab Disab Disab Disab Disab Disab\n> > + Exp requested: N/A None None None None 10 None 10 10\n> > + Set in Frame: N+4 N+5 N+6 N+7 N+8 N+9 N+10 N+10 N+10\n> > +\n> > + Mode used: Auto Auto Auto Disab Disab Disab Disab Disab Disab\n> > + Exp used: 2 3 4 5 5 5 5 10 10\n> > +\n> > +This example is extended by a few frames to exhibit the simulated \"locked\"\n> > +state. At frame N+5 the application has confirmed that the auto mode has been\n> > +disabled, but does not provide a manual value until request N+7. Thus, the\n> > +value that is used in requests N+5 and N+6 (where the mode is disabled), comes\n> > +from the last value that was used when the mode was auto, which comes from\n> > +frame N+4.\n> > +\n> > +Then, in N+7, a manual value of 10 is supplied. It takes until frame N+9 for\n> > +the exposure to be applied. N+8 does not supply a manual value, but the last\n> > +supplied value is retained, so a manual value of 10 is still used and set in\n> > +frame N+10.\n> > +\n> > +Although this behavior is the same as what we had with waiting for the manual\n> > +mode to propagate (in the section \"Description of the problem\"), this time it\n> > +is correct as we have defined specifically that if a manual value was specified\n> > +while the mode was auto, it will not be retained.\n> > +\n> > +Description of the controls\n> > +===========================\n> > +\n> > + libcamera offers the following controls related to exposure and gain:\n> > +\n> > + - AnalogueGain\n> > +\n> > + - AnalogueGainMode\n> > +\n> > + - ExposureTime\n> > +\n> > + - ExposureTimeMode\n> > +\n> > + - AeState\n> > +\n> > + Auto-exposure and auto-gain can be enabled and disabled separately using the\n> > + ExposureTimeMode and AnalogueGainMode controls respectively. There is no\n> > + overarching AeEnable control.\n> > +\n> > + For each of exposure and gain, we can model it with three states: auto,\n> > + locked, and manual. Note that AnalogueGainMode and ExposureTimeMode only\n> > + have two values, as the locked state is simulated.\n> > +\n> > + ::\n> > +\n> > + /---------------------------------\\\n> > + | |\n> > + V |\n> > + +--------+ +--------+\n> > + | | ---------------------> | |\n> > + | Auto | +--------+ | Manual |\n> > + | | ----> | Locked | ----> | |\n> > + +--------+ +--------+ +--------+\n> > + ^ |\n> > + | |\n> > + \\----------------/\n> > +\n> > + Notice from the state diagram that locked to manual is a one-way state\n> > + change, as the reverse direction is nonsensical (see the design document for\n> > + more details on this topic).\n> > +\n> > + The exposure/gain is in the Auto state when\n> > + ExposureTimeMode/AnalogueGainMode is set to Auto. In this state, the value\n> > + that is computed by the AE algorithm is applied to the image sensor. Any\n> > + value that is supplied in the ExposureTime/AnalogueGain control is ignored\n> > + and is not retained.\n> > +\n> > + If ExposureTimeMode/AnalogueGainMode is set to Manual, it can put us in\n> > + either the Locked or Manual state. The difference is in if\n> > + ExposureTime/AnalogueGain has been supplied. If it has not yet been\n> > + supplied, then we are in the Locked state. If it has been supplied, then we\n> > + are in the Manual state.\n> > +\n> > + In both the Locked state and the Manual state the exposure/gain value does\n> > + not come from the AE algorithm. In the Locked state the value comes from the\n> > + last value computed by the AE algorithm while the state was Auto, or if the\n> > + state was never Auto (e.g. we started in Locked, or the camera doesn't\n> > + support Auto), then the value should be a best-effort default value. In the\n> > + Manual state the value comes from the value supplied in the\n> > + ExposureTime/AnalogueGain control.\n> > +\n> > + To transition from the Locked state to the Manual state, a value needs to be\n> > + submitted in ExposureTime/AnalogueGain. Once the state has transitioned to\n> > + Manual, then this value will be retained, and doesn't need to be resubmitted\n> > + if it doesn't change.\n> > +\n> > + To transition to the Auto state, simply set\n> > + ExposureTimeMode/AnalogueGainMode to Auto.\n> > +\n> > +\n> > + The AeState metadata reports the state of the AE algorithm. As AE cannot\n> > + compute exposure and gain separately, the state of the AE component is\n> > + unified. There are three states: Idle, Searching, and Converged.\n> > +\n> > + The state shall be Idle if both ExposureTimeMode and AnalogueGainMode\n> > + are set to Manual. If the camera only supports one of the two controls,\n> > + then the state shall be Idle if that one control is set to Manual. If\n> > + the camera does not support Manual for at least one of the two controls,\n> > + then the state will never be Idle, as AE will always be running.\n> > +\n> > + The state shall be Searching if at least one of exposure or gain calculated\n> > + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> > + *and* the value(s) have not converged yet.\n> > +\n> > + The state shall be Converged if at least one of exposure or gain calculated\n> > + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> > + *and* the value(s) have converged.\n> \n> That is a lot of stuff to go through. I like the model. One thing that\n\n\\o/\n\n> came to my mind: What is the reason to use a enum for the ExposureTimeMode\n> control instead of a bool? Are there further extensions planned? GenICam\n\ntbh I can't recall any specific reason. Maybe that it's more explicit\nthan AutoExposureTime/AutoAnalogueGain == true/false?\n\n> devices often have an additional mode \"Once\" that runs the control loop\n> until converged and then snaps back to off/manual.\n\nOh neat! Maybe in the future...\n\n> \n> But overall I believe we should get that it.\n> Reviewed-by: Stefan Klug \n\n\nThanks,\n\nPaul","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 96F34C3257\n\tfor ;\n\tFri, 15 Nov 2024 13:13:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ADE996588F;\n\tFri, 15 Nov 2024 14:13:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 672F965882\n\tfor ;\n\tFri, 15 Nov 2024 14:13:10 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:bd6c:1638:cb26:1bfa])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4DFE09CE;\n\tFri, 15 Nov 2024 14:12:53 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vOtvKSpw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731676375;\n\tbh=VIk1S4hD+mWAXwCkCOzqZiekjzDocsh5pcQ3JIS4sXs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vOtvKSpwwNtvABlaKLLjIleHyTNPyG+Tl+gcXa+40Z1CkU76qwz94o/0Vha8UvC5O\n\tx03IwPj5RlvTxyAlV7EIwApstGjOjgDtGqBiNOfTWlabvNF3a3Pea+fab/xBKb0uSA\n\tjAyn1nrHSp3mhlxX1g8CkFu/BsMwA/JbS2WsGxsY=","Date":"Fri, 15 Nov 2024 22:13:02 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org, laurent.pinchart@ideasonboard.com, \n\tjacopo.mondi@ideasonboard.com, naush@raspberrypi.com,\n\tdavid.plowman@raspberrypi.com, Jacopo Mondi ","Subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","Message-ID":"","References":"<20241113131256.3170817-1-paul.elder@ideasonboard.com>\n\t<20241113131256.3170817-3-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32307,"web_url":"https://patchwork.libcamera.org/comment/32307/","msgid":"<20241120134213.GR12409@pendragon.ideasonboard.com>","date":"2024-11-20T13:42:13","subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Nov 13, 2024 at 10:12:50PM +0900, Paul Elder wrote:\n> Document the design and rationale for the AE-related controls.\n> Also add documentation for the controls.\n\nThat's a great document, thank you. I'm getting a few build errors\nthough:\n\n[518/519] Generating Documentation/documentation with a custom command\nFAILED: Documentation/html\n/usr/bin/sphinx-build -D release=v0.3.2+123-1f1977af -q -W -b html Documentation Documentation/html\nDocumentation/design/ae.rst:290: WARNING: Literal block ends without a blank line; unexpected unindent. [docutils]\nDocumentation/design/ae.rst:295: ERROR: Unexpected indentation. [docutils]\nDocumentation/design/ae.rst: WARNING: document isn't included in any toctree\n\n> Signed-off-by: Paul Elder \n> Signed-off-by: Jacopo Mondi \n> \n> ---\n> Changes in v3:\n> - merge the control documentation into the same document (including the\n> patch)\n> - this is because it was a bit unwieldy to put it in\n> control_ids.cpp.in, now that it's used generically to generate\n> control ids of all namespaces\n> ---\n> Documentation/design/ae.rst | 348 ++++++++++++++++++++++++++++++++++++\n\nDon't forget the entry in Documentation/meson.build, in addition to\nwiring this in the toc tree.\n\n> 1 file changed, 348 insertions(+)\n> create mode 100644 Documentation/design/ae.rst\n> \n> diff --git a/Documentation/design/ae.rst b/Documentation/design/ae.rst\n> new file mode 100644\n> index 000000000000..f121afecea5a\n> --- /dev/null\n> +++ b/Documentation/design/ae.rst\n> @@ -0,0 +1,348 @@\n> +.. SPDX-License-Identifier: CC-BY-SA-4.0\n> +\n> +Design of Exposure and Gain controls\n> +====================================\n> +\n> +This document explains the design and rationale of the controls related to\n> +exposure and gain. This includes the all-encompassing auto-exposure (AE), the\n> +manual exposure control, and the manual gain control.\n> +\n> +Description of the problem\n> +--------------------------\n> +\n> +Sub controls\n> +^^^^^^^^^^^^\n> +\n> +There are more than one control that make up exposure: exposure, gain, and\n\nWe need to standardize vocabulary. I've sent '[PATCH v2] libcamera:\nRename \"shutter speed\" to \"exposure time\"' that addresses the low\nhanging fruit of consistently using \"exposure time\" when we talk about,\nwell, exposure time. Here that would be\n\nThere are more than one control that make up exposure: exposure time, gain, and\naperture (...).\n\n(The first use of \"exposure\" isn't correct I think, at least according to\nhttps://en.wikipedia.org/wiki/Exposure_(photography). I'm not sure what\na better term would be. We could also have our own definition of\n\"exposure\".)\n\nCould you rename \"exposure\" to \"exposure time\" through the series when\nit refers to the exposure time ? We will address the remaining usage of\n\"exposure\" separately as I don't want to delay the AE rework.\n\n> +aperture (though for now we will not consider aperture). We already had\n> +individual controls for setting the values of manual exposure and manual gain,\n> +but for switching between auto mode and manual mode we only had a high-level\n> +boolean AeEnable control that would set *both* exposure and gain to auto mode\n> +or manual mode; we had no way to set one to auto and the other to manual.\n> +\n> +So, we need to introduce two new controls to act as \"levers\" to indicate\n> +individually for exposure and gain if the value would come from AEGC or if it\n> +would come from the manual control value.\n> +\n> +Aperture priority\n> +^^^^^^^^^^^^^^^^^\n> +\n> +We eventually will support aperture, and so whatever our solution is for having\n\nMaybe s/will/may need to/\n\n> +only some controls on auto and the others on manual needs to be extensible.\n> +\n> +Flickering when going from auto to manual\n> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n> +\n> +When a manual exposure or gain value is requested by the application, it costs\n> +a few frames worth of time for them to take effect. This means that during a\n> +transition from auto to manual, there would be flickering in the control values\n> +and the transition won't be smooth.\n> +\n> +Take for instance the following flow, where we start on auto exposure (which\n> +for the purposes of the example increments by 1 each frame) and we want to\n> +switch seamlessly to manual exposure, which involves copying the exposure value\n> +computed by the auto exposure algorithm:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> +\n> + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> + Exp requested: N/A N/A N/A 2 2 2 2\n> + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> +\n> + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> + Exp used: 0 1 2 3 4 2 2\n> +\n> +As we can see, after frame N+2 completes, we copy the exposure value that was\n> +used for frame N+2 (which was computed by AE algorithm), and queue that value\n> +into request N+3 with manual mode on. However, as it takes two frames for the\n> +exposure to be set, the exposure still changes since it is set by AE, and we\n> +get a flicker in the exposure during the switch from auto to manual.\n> +\n> +A solution is to *not submit* any exposure value when manual mode is enabled,\n> +and wait until the manual mode as been \"applied\" before copying the exposure\n> +value:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N | | N+1 | | N+2 | | N+3 | | N+4 | | N+5 | | N+6 |\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> +\n> + Mode requested: Auto Auto Auto Manual Manual Manual Manual\n> + Exp requested: N/A N/A N/A None None None 5\n> + Set in Frame: N+2 N+3 N+4 N+5 N+6 N+7 N+8\n> +\n> + Mode used: Auto Auto Auto Auto Auto Manual Manual\n> + Exp used: 0 1 2 3 4 5 5\n> +\n> +In practice, this works. However, libcamera has a policy where once a control\n> +is submitted, its value is saved and does not need to be resubmitted (even\n> +though this isn't implemented yet). So if the manual exposure value was set\n\nIs the \"not implemented yet\" part universally true ? And \"yet\" implies\nwe plan to implement it. I'd drop the part between parentheses.\n\ns/So if/If/ (as there's another \"so\" further in the sentence)\n\n> +while auto mode was on, in theory the value would be saved, so when manual mode\n> +is enabled, the exposure value that was previously set would immediately be\n> +used. Clearly this solution isn't correct, but it can serve as the basis for a\n> +proper solution, with some more rigorous rules.\n> +\n> +Existing solutions\n> +------------------\n> +\n> +Raspberry Pi\n> +^^^^^^^^^^^^\n> +\n> +The raspberry pi IPA got around the lack of individual AeEnable controls for\n\ns/raspberry pi/Raspberry Pi/\n\n> +exposure and gain by using magic values. When AeEnable was false, if one of the\n\nMaybe use the present tense (s/was/is/) here as the section is named\n\"Existing solutions\" ?\n\n> +manual control values was set to 0 then the value computed by AEGC would be\n> +used for just that control. This solution isn't desirable, as it prevents\n> +that magic value from being used as a valid value.\n> +\n> +To get around the flickering issue, when AeEnable was false, the raspberry pi\n\ns/raspberry pi/Raspberry Pi/\n\n> +AEGC would simply stop updating the values to be set. As mentioned above, since\n> +the value retention mechanism hasn't actually been implemented yet, this\n\nAh, I see why you need the \"not implemented yet\" above.\n\nMaybe\n\nTo get around the flickering issue, when AeEnable is false, the\nRaspberry Pi AEGC simply stops updating the values to be set, without\nrestoring the previously set manual exposure time and gain. This works,\nbut is not a proper solution.\n\n> +worked. But, it's not a proper solution.\n> +\n> +Android\n> +^^^^^^^\n> +\n> +The Android HAL specification requires that exposure and gain (sensitivity)\n> +must both be manual or both be auto. It cannot be that one is manual while the\n> +other is auto, so they simply don't support sub controls.\n> +\n> +For the flickering issue, the Android HAL has an AeLock control. To transition\n> +from auto to manual, the application would keep AE on auto, and turn on the\n> +lock. Once the lock has propagated through, then the value can be copied from\n> +the result into the request and the lock disabled and the mode set to manual.\n> +\n> +The problem with this solution is, besides the extra complexity, that it is\n> +ambiguous what happens if there is a state transition from manual to locked\n> +(even though it's a state transition that doesn't make sense). If locked is\n> +defined to \"use the last automatically computed values\" then it could use the\n> +values from the last time it AE was set to auto, or it would be undefined if AE\n> +was never auto (eg. it started out as manual), or if AE is implemented to run\n> +in the background it could just use the current values that are computed. If\n> +locked is defined to \"use the last value that was set\" there would be less\n> +ambiguity. Still, it's better if we can make it impossible to execute this\n> +nonsensical state transition, and if we can reduce the complexity of having\n> +this extra control or extra setting on a lever.\n> +\n> +Summary of goals\n> +----------------\n> +\n> + - We need a lock of some sort, to instruct the AEGC to not update output\n> + results\n> +\n> + - We need manual modes, to override the values computed by the AEGC\n> +\n> + - We need to support seamless transitions from auto to manual, and do so\n> + without flickering\n> +\n> + - We need custom minimum values for the manual controls; that is, no magic\n> + values for enabling/disabling auto\n> +\n> + - All of these need to be done with AE sub-controls (exposure time, analogue\n> + gain) and be extensible to aperture in the future\n\nYou can lower the indentation here. Same below for other lists.\n\n> +\n> +Our solution\n> +------------\n> +\n> +A diagram of our solution:\n> +\n> +::\n> +\n> + +----------------------------+-------------+------------------+-----------------+\n> + | INPUT | ALGORITHM | RESULT | OUTPUT |\n> + +----------------------------+-------------+------------------+-----------------+\n> +\n> + ExposureTimeMode ExposureTimeMode\n> + ---------------------+----------------------------------------+----------------->\n> + 0: Auto | |\n> + 1: Manual | V\n\nThere seems to be a small formatting issue here.\n\n> + | |\\\n> + | | \\\n> + | /----------------------------------> | 1| ExposureTime\n> + | | +-------------+ exposure time | | -------------->\n> + \\--)--> | | --------------> | 0|\n> + ExposureTime | | | | /\n> + ------------------------+--> | | |/\n> + | | AeState\n> + | AEGC | ----------------------------------->\n> + AnalogueGain | |\n> + ------------------------+--> | | |\\\n> + | | | | \\\n> + /--)--> | | --------------> | 0| AnalogueGain\n> + | | +-------------+ analogue gain | | -------------->\n> + | \\----------------------------------> | 1|\n> + | | /\n> + | |/\n> + | ^\n> + AnalogueGainMode | | AnalogueGainMode\n> + ---------------------+----------------------------------------+----------------->\n> + 0: Auto\n> + 1: Manual\n> +\n> +\n> +The diagram is divided in four sections horizontally:\n> +\n> + - Input: The values received from the request controls\n> +\n> + - Algorithm: The algorithm itself\n> +\n> + - Result: The values calculated by the algorithm\n> +\n> + - Output: The values that sent in result metadata and applied to the device\n\n\"that sent\" ?\n\n> +\n> +The four input controls are divided between manual values (ExposureTime and\n> +AnalogueGain), and operation modes (ExposureTimeMode and AnalogueGainMode). The\n> +former are the manual values, the latter control how they're applied. The two\n> +modes are independent from each other, and each can take one of two values:\n> +\n> + - Auto (0): The AGC computes the value normally. The AGC result is applied\n> + to the output. The manual value is ignored *and is not retained*.\n> +\n> + - Manual (1): The AGC uses the manual value internally. The corresponding\n> + manual control from the request is applied to the output. The AGC result\n> + is ignored.\n> +\n> +The AeState control reports the state of the unified AEGC block. If both\n> +ExposureTimeMode and AnalogueGainMode are set to disabled then it will report\n\nHere you mention \"disabled\" but above the controls are defined as \"auto\"\nor \"manual\". Should \"disabled\" be replaced with \"manual\", here and below\n?\n\n> +Idle. If at least one of the two is set to auto, then AeState will report\n> +if the AEGC has Converged or not (Searching). This control replaces the old\n> +AeLocked control, as it was insufficient for reporting the AE state.\n> +\n> +There is a caveat to the disabled mode: the manual control value is not\n> +retained if it is set during auto mode. This means that if the disabled mode is\n> +entered without also setting the manual value, then it will enter a state\n> +similar to \"locked\", where the last automatically computed value while the mode\n> +was auto will be used. Once the manual value is set, then that will be used and\n> +retained as usual.\n> +\n> +This simulates an auto -> locked -> manual or auto -> manual state transition,\n> +and makes it impossible to do the nonsensical manual -> locked state\n> +transition.\n> +\n> +We specifically do not have a \"master AE control\" like the old AeEnable. This\n> +is because we have the individual mode controls, and if we had a master AE\n> +control it would be a \"control that sets other controls\", which could easily\n> +get out of control.\n\nNo pun intended ? :-)\n\n> +\n> +With this solution, the earlier example would become:\n> +\n> +::\n> +\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + | N+2 | | N+3 | | N+4 | | N+5 | | N+6 | | N+7 | | N+8 | | N+9 | | N+10|\n> + +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+ +-----+\n> + Mode requested: Auto Disab Disab Disab Disab Disab Disab Disab Disab\n> + Exp requested: N/A None None None None 10 None 10 10\n> + Set in Frame: N+4 N+5 N+6 N+7 N+8 N+9 N+10 N+10 N+10\n\nI think the last two N+10 should be N+11 and N+12.\n\n> +\n> + Mode used: Auto Auto Auto Disab Disab Disab Disab Disab Disab\n> + Exp used: 2 3 4 5 5 5 5 10 10\n> +\n> +This example is extended by a few frames to exhibit the simulated \"locked\"\n> +state. At frame N+5 the application has confirmed that the auto mode has been\n> +disabled, but does not provide a manual value until request N+7. Thus, the\n> +value that is used in requests N+5 and N+6 (where the mode is disabled), comes\n> +from the last value that was used when the mode was auto, which comes from\n> +frame N+4.\n> +\n> +Then, in N+7, a manual value of 10 is supplied. It takes until frame N+9 for\n> +the exposure to be applied. N+8 does not supply a manual value, but the last\n> +supplied value is retained, so a manual value of 10 is still used and set in\n> +frame N+10.\n> +\n> +Although this behavior is the same as what we had with waiting for the manual\n> +mode to propagate (in the section \"Description of the problem\"), this time it\n> +is correct as we have defined specifically that if a manual value was specified\n> +while the mode was auto, it will not be retained.\n> +\n> +Description of the controls\n> +===========================\n\nThis uses the same heading level as the top-level title, is that\nintentional ?\n\n> +\n> + libcamera offers the following controls related to exposure and gain:\n\nThe indentation is wrong for the whole block, everything should be on\nthe left-most column.\n\n> +\n> + - AnalogueGain\n> +\n> + - AnalogueGainMode\n> +\n> + - ExposureTime\n> +\n> + - ExposureTimeMode\n> +\n> + - AeState\n> +\n> + Auto-exposure and auto-gain can be enabled and disabled separately using the\n> + ExposureTimeMode and AnalogueGainMode controls respectively. There is no\n> + overarching AeEnable control.\n> +\n> + For each of exposure and gain, we can model it with three states: auto,\n> + locked, and manual. Note that AnalogueGainMode and ExposureTimeMode only\n> + have two values, as the locked state is simulated.\n> +\n> + ::\n> +\n> + /---------------------------------\\\n> + | |\n> + V |\n> + +--------+ +--------+\n> + | | ---------------------> | |\n> + | Auto | +--------+ | Manual |\n> + | | ----> | Locked | ----> | |\n> + +--------+ +--------+ +--------+\n> + ^ |\n> + | |\n> + \\----------------/\n> +\n> + Notice from the state diagram that locked to manual is a one-way state\n> + change, as the reverse direction is nonsensical (see the design document for\n> + more details on this topic).\n\nIsn't this the design document ?\n\n> +\n> + The exposure/gain is in the Auto state when\n> + ExposureTimeMode/AnalogueGainMode is set to Auto. In this state, the value\n> + that is computed by the AE algorithm is applied to the image sensor. Any\n> + value that is supplied in the ExposureTime/AnalogueGain control is ignored\n> + and is not retained.\n\nI expect it will be easier to implement this by accumulating the manual\nexposure time and analog gain controls as we do for all other controls,\nand override the internal storage with the last value computed by the\nAGC algorithm when transitioning to manual. That's an implementation\ndetail, the behaviour will be the same from a user point of view, but if\nwe expect AGC to be implemented that way, it may be nicer to also\ndescribe it the same way here.\n\n> +\n> + If ExposureTimeMode/AnalogueGainMode is set to Manual, it can put us in\n> + either the Locked or Manual state. The difference is in if\n> + ExposureTime/AnalogueGain has been supplied. If it has not yet been\n> + supplied, then we are in the Locked state. If it has been supplied, then we\n> + are in the Manual state.\n> +\n> + In both the Locked state and the Manual state the exposure/gain value does\n> + not come from the AE algorithm. In the Locked state the value comes from the\n> + last value computed by the AE algorithm while the state was Auto, or if the\n> + state was never Auto (e.g. we started in Locked, or the camera doesn't\n> + support Auto), then the value should be a best-effort default value. In the\n\nCan we start in the locked state, as it's not a real state ? I'd rather\nsay that if the user starts the camera in manual mode without setting a\ncorresponding manual value then a best-effort default value is used.\n\n> + Manual state the value comes from the value supplied in the\n> + ExposureTime/AnalogueGain control.\n> +\n> + To transition from the Locked state to the Manual state, a value needs to be\n> + submitted in ExposureTime/AnalogueGain. Once the state has transitioned to\n> + Manual, then this value will be retained, and doesn't need to be resubmitted\n> + if it doesn't change.\n\nI understand where all of this come from, but I think the documentation\ncould be simplified if we explained the behaviour as copying the last\nvalue computed by the algorithm to the manual value when transitioning\nfrom auto to manual (and also mentioning of course that if the same\nrequest provides a manual value, it will override the value copied from\nthe algorithm). That way we could avoid introducing a locked\npseudo-state, and I think the result would be easier to understand for\nthe reader.\n\nPlease let me know what you think, I value your opinion.\n\n> +\n> + To transition to the Auto state, simply set\n> + ExposureTimeMode/AnalogueGainMode to Auto.\n> +\n> +\n\nExtra blank line.\n\n> + The AeState metadata reports the state of the AE algorithm. As AE cannot\n> + compute exposure and gain separately, the state of the AE component is\n> + unified. There are three states: Idle, Searching, and Converged.\n> +\n> + The state shall be Idle if both ExposureTimeMode and AnalogueGainMode\n> + are set to Manual. If the camera only supports one of the two controls,\n> + then the state shall be Idle if that one control is set to Manual. If\n> + the camera does not support Manual for at least one of the two controls,\n> + then the state will never be Idle, as AE will always be running.\n> +\n> + The state shall be Searching if at least one of exposure or gain calculated\n> + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> + *and* the value(s) have not converged yet.\n> +\n> + The state shall be Converged if at least one of exposure or gain calculated\n> + by the AE algorithm is used (that is, at least one of the two modes is Auto),\n> + *and* the value(s) have converged.","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 E786DC32F5\n\tfor ;\n\tWed, 20 Nov 2024 13:42:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED21765F4C;\n\tWed, 20 Nov 2024 14:42:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4B8165898\n\tfor ;\n\tWed, 20 Nov 2024 14:42:23 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E91275A;\n\tWed, 20 Nov 2024 14:42:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Nof8Mw9s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732110124;\n\tbh=xNC6l+azI4rvIP7M0LM/taYBBn1FqySOzK9dQTW+IAo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Nof8Mw9sI7ygV92e4q1pKdN06AYXbc0mJr9CONR3CbWtDvTOIMEm7p+7mvKmWSmxe\n\tSh5uDHyRNxbR8PcPTik9C7Uc8kBXr00PaZW3dU5EpxwSV/Tx+da3sGTA/6AsGkstAG\n\tVTrKGKn6IL3InxbtWNTu08SGbkIN3rs2TBT8f7GY=","Date":"Wed, 20 Nov 2024 15:42:13 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, jacopo.mondi@ideasonboard.com,\n\tnaush@raspberrypi.com, david.plowman@raspberrypi.com,\n\tJacopo Mondi ","Subject":"Re: [PATCH v3 2/8] Documentation: design: ae: Document the design\n\tfor AE controls","Message-ID":"<20241120134213.GR12409@pendragon.ideasonboard.com>","References":"<20241113131256.3170817-1-paul.elder@ideasonboard.com>\n\t<20241113131256.3170817-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241113131256.3170817-3-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]