[PATCH v4 1/6] libcamera: controls: Update the ColourTemperature control to be writable

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Tue, Aug 13, 2024 at 10:44:18AM +0200, Stefan Klug wrote:
> For manual control it is helpful to be able to specify a fixed colour
> temperature. It also provides an easy way to apply the temperature
> specific CCMs and colour gains that are contained in the tuning files.
> 
> Document this and update the control dependencies.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/libcamera/control_ids_core.yaml | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index cf40771d3896..04dcc4af67fc 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -252,9 +252,12 @@ controls:
>    - AwbEnable:
>        type: bool
>        description: |
> -        Enable or disable the AWB.
> +        Enable or disable the AWB. Disabling AWB stops updates to the
> +        ColourGains and to the ColourCorrectionMatrix.

Please split this in two paragraphs. The first paragraph is translated
to a \brief doxygen command, and should be a single sentence. Same
below.

I would like to clarify this a bit. Here's an attempt, is it what you
mean ?

	Enable or disable the AWB.

	When AWB is enabled, the algorithm estimates the colour	temperature of
	the scene, and computes colour gains and the colour correction matrix
	automatically. The computed colour temperate, gains and correction
	matrix are reported in metadata. The corresponding controls are ignored
	if set in a request.

	When AWB is disabled, the colour temperature, gains and correction
	matrix are not updated automatically and can be set manually in
	requests.

>  
> +        \sa ColourCorrectionMatrix
>          \sa ColourGains
> +        \sa ColourTemperature
>  
>    # AwbMode needs further attention:
>    # - Auto-generate max enum value.
> @@ -309,13 +312,24 @@ controls:
>          disabled.
>  
>          \sa AwbEnable
> +        \sa ColourTemperature
>        size: [2]
>  
>    - ColourTemperature:
>        type: int32_t
> -      description: Report the current estimate of the colour temperature, in
> -        kelvin, for this frame. The ColourTemperature control can only be
> -        returned in metadata.
> +      description: |
> +        Report the current estimate of the colour temperature, in kelvin, for
> +        this frame. An implementation may also allow this control to be set when

s/also //

> +        AWB is disabled. In that case ColourGains and the ColourCorrectionMatrix
> +        get set accordingly. If either ColourGains or ColourCorrectionMatrix are
> +        specified at the same time, they take precedence.

maybe "they take precedence, and the ColourTemperature is ignored" ? Or,
if an application sets ColourTemperature and ColourGains but not
ColourCorrectionMatrix, do you expect the implementation to apply the
requested ColourGains and set ColourCorrectionMatrix based on the
temperature ?

> +
> +        The metadata will only report measured colour temperature values when
> +        available, even if set manually.

I'm not sure to understand this.

> +
> +        \sa AwbEnable
> +        \sa ColourCorrectionMatrix
> +        \sa ColourGains
>  
>    - Saturation:
>        type: float
> @@ -365,6 +379,8 @@ controls:
>          transformation. The 3x3 matrix is stored in conventional reading
>          order in an array of 9 floating point values.
>  
> +        \sa AwbEnable
> +        \sa ColourTemperature
>        size: [3,3]
>  
>    - ScalerCrop:

-- 
Regards,

Laurent Pinchart