[libcamera-devel] [PATCH v4 4/5] libcamera: controls: Improve documentation for ExposureTime and AnalogueGain

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Tue, Dec 01, 2020 at 05:55:35PM +0000, David Plowman wrote:
> Setting these controls "fixes" them and the AE may not change them;
> setting them back to zero returns them to the control of the AE
> algorithm.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index a883e27e..fba1f545 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -125,8 +125,15 @@ controls:
>          Exposure time (shutter speed) for the frame applied in the sensor
>          device. This value is specified in micro-seconds.
>  
> +        Setting this value means that it is now fixed and the AE algorithm may
> +        not change it. Setting it back to zero returns it to the control of the
> +        AE algorithm.
> +
>          \sa AnalogueGain AeEnable
>  
> +        \todo Consider how setting the exposure time interacts with other AE
> +              features, such as aperture, aperture/shutter priority modes etc.

No need for the additional indentation. I'd also expand this a bit to
mention AeEnable. Maybe something along the lines of the following ?

        \todo Document the interactions between AeEnable and setting a fixed
        value for this control. Consider interactions with other AE features,
        such as aperture and aperture/shutter priority mode, and decide if
        control of which features should be automatically adjusted shouldn't
        better be handled through a separate AE mode control.

If you're fine with this, I'll use this text when applying, there's no
need to resend this patch.

> +
>    - AnalogueGain:
>        type: float
>        description: |
> @@ -134,8 +141,15 @@ controls:
>          The value of the control specifies the gain multiplier applied to all
>          colour channels. This value cannot be lower than 1.0.
>  
> +        Setting this value means that it is now fixed and the AE algorithm may
> +        not change it. Setting it back to zero returns it to the control of the
> +        AE algorithm.
> +
>          \sa ExposureTime AeEnable
>  
> +        \todo Consider how setting the analogue gain interacts with other AE
> +              features, such as aperture, aperture/shutter priority modes etc.
> +
>    - Brightness:
>        type: float
>        description: |

-- 
Regards,

Laurent Pinchart