[libcamera-devel] [PATCH v3 3/5] libcamera: controls: Reorder and update description of existing controls

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Fri, Apr 03, 2020 at 03:53:03PM +0100, Naushir Patuck wrote:
> Group AE, AWB, etc. controls together for accessibility.
> 
> Update descriptions for Contrast, Brightness, and Saturation controls.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 45 ++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 839eea76..64e81520 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -23,25 +23,6 @@ controls:
>  
>          \sa AeEnable
>  
> -  - AwbEnable:
> -      type: bool
> -      description: |
> -        Enable or disable the AWB.
> -
> -        \sa ManualGain
> -
> -  - Brightness:
> -      type: int32_t
> -      description: Specify a fixed brightness parameter
> -
> -  - Contrast:
> -      type: int32_t
> -      description: Specify a fixed contrast parameter
> -
> -  - Saturation:
> -      type: int32_t
> -      description: Specify a fixed saturation parameter
> -
>    - ExposureTime:
>        type: int32_t
>        description: |
> @@ -58,4 +39,30 @@ controls:
>          colour channels. This value cannot be lower than 1.0.
>  
>          \sa ExposureTime AeEnable
> +
> +  - Brightness:
> +      type: int32_t
> +      description: |
> +        Specify a fixed brightness parameter. Positive values (up to 65535)
> +        produce brighter images; negative values (up to -65536) produce darker
> +        images and 0 leaves pixels unchanged.
> +
> +  - Contrast:
> +      type: int32_t
> +      description:  |
> +        Specify a fixed contrast parameter. Normal contrast is given by the
> +        value 1.0; larger values produce images with more contrast.

Contrast is an int32_t type. Should it be changed to a float, or use a
fixed-point value like brightness ? I think both should use the same
type, and if we go for fixed-point, 1.0 isn't the right default :-)

The uvcvideo pipeline handler should be adapted, I can handle that once
we agree on what types and ranges are best.

> +
> +  - AwbEnable:
> +      type: bool
> +      description: |
> +        Enable or disable the AWB.
> +
> +        \sa ManualGain
> +
> +  - Saturation:
> +      type: int32_t
> +      description:  |
> +        Specify a fixed saturation parameter. Normal saturation is given by
> +        the value 1.0; larger values produce more saturated colours.

Same here.

>  ...

-- 
Regards,

Laurent Pinchart