[v5,1/8] libcamera: controls: Update the ColourTemperature control to be writable

22218 diff mbox series

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@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v5:
- Improve documentation
---
 src/libcamera/control_ids_core.yaml | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

Quoting Stefan Klug (2024-12-06 14:52:21)
> 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@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> Changes in v5:
> - Improve documentation
> ---
>  src/libcamera/control_ids_core.yaml | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index d45cf8e56187..3795f7169911 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -283,7 +283,19 @@ controls:
>        description: |
>          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.
> @@ -341,14 +353,25 @@ controls:
>          ColourGains can only be applied in a Request when the AWB is disabled.
>  
>          \sa AwbEnable
> +        \sa ColourTemperature
>        size: [2]
>  
>    - ColourTemperature:
>        type: int32_t
>        description: |
> -        Report the estimate of the colour temperature for the frame, in kelvin.
> +        Report the current estimate of the colour temperature, in kelvin, for

I think I recall we need to keep the 'first line' as the brief and short
which gets used in the generated documentation.

Perhaps the first line could be simply:

"""
	   ColourTemperature of the Frame.
"""

With that (as in, 'a brief' - you can change it if you prefer)


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>



> +        this frame. An implementation may allow this control to be set when AWB
> +        is disabled. In that case ColourGains and the ColourCorrectionMatrix get
> +        set accordingly. If ColourGains and ColourTemperature are specified at
> +        the same time, ColourGains takes precedence. The same applies to
> +        ColourCorrectionMatrix.
> +
> +        The metadata will only report measured colour temperature values when
> +        available, even if set manually.
>  
> -        The ColourTemperature control can only be returned in metadata.
> +        \sa AwbEnable
> +        \sa ColourCorrectionMatrix
> +        \sa ColourGains
>  
>    - Saturation:
>        type: float
> @@ -405,6 +428,8 @@ controls:
>          stored in conventional reading order in an array of 9 floating point
>          values.
>  
> +        \sa AwbEnable
> +        \sa ColourTemperature
>        size: [3,3]
>  
>    - ScalerCrop:
> -- 
> 2.43.0
>

Hello,

On Fri, Dec 06, 2024 at 03:05:23PM +0000, Kieran Bingham wrote:
> Quoting Stefan Klug (2024-12-06 14:52:21)
> > 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@ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > 
> > ---
> > Changes in v5:
> > - Improve documentation
> > ---
> >  src/libcamera/control_ids_core.yaml | 29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > index d45cf8e56187..3795f7169911 100644
> > --- a/src/libcamera/control_ids_core.yaml
> > +++ b/src/libcamera/control_ids_core.yaml
> > @@ -283,7 +283,19 @@ controls:
> >        description: |
> >          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.
> > @@ -341,14 +353,25 @@ controls:
> >          ColourGains can only be applied in a Request when the AWB is disabled.
> >  
> >          \sa AwbEnable
> > +        \sa ColourTemperature
> >        size: [2]
> >  
> >    - ColourTemperature:
> >        type: int32_t
> >        description: |
> > -        Report the estimate of the colour temperature for the frame, in kelvin.
> > +        Report the current estimate of the colour temperature, in kelvin, for
> 
> I think I recall we need to keep the 'first line' as the brief and short
> which gets used in the generated documentation.
> 
> Perhaps the first line could be simply:
> 
> """
> 	   ColourTemperature of the Frame.

s/Frame/frame, in kelvin/

> """
> 
> With that (as in, 'a brief' - you can change it if you prefer)
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +        this frame. An implementation may allow this control to be set when AWB
> > +        is disabled. In that case ColourGains and the ColourCorrectionMatrix get
> > +        set accordingly. If ColourGains and ColourTemperature are specified at
> > +        the same time, ColourGains takes precedence. The same applies to
> > +        ColourCorrectionMatrix.
> > +
> > +        The metadata will only report measured colour temperature values when
> > +        available, even if set manually.

I had to re-read the whole discussion thread for the previous version of
this patch to understand this sentence, so we still have unresolved
issues with the control documentation.

I'll reply to the last e-mails in v4, there's unfinished business there.

> >  
> > -        The ColourTemperature control can only be returned in metadata.
> > +        \sa AwbEnable
> > +        \sa ColourCorrectionMatrix
> > +        \sa ColourGains
> >  
> >    - Saturation:
> >        type: float
> > @@ -405,6 +428,8 @@ controls:
> >          stored in conventional reading order in an array of 9 floating point
> >          values.
> >  
> > +        \sa AwbEnable
> > +        \sa ColourTemperature
> >        size: [3,3]
> >  
> >    - ScalerCrop: