[libcamera-devel] [PATCH v2 5/6] libcamera: controls: Update usage and description for existing controls

[Kieran Bingham]

Hi Naush,

On 09/03/2020 12:33, Naushir Patuck wrote:
> Switch to using floats for constrast and saturation control to be more

s/constrast/contrast/

> in-line with end-user expectations.
> 
> Expand on the descrption for the brightness, contrast and saturation

s/descrption/description/

I really should find some time to resume work on adding spell check to
checkstyle.py :-S - it would be useful to everyone I'm sure.

Maybe a task to give to an internship or put on our todo pages ;-)

> controls.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 9a33094a..3d1b058f 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -192,13 +192,20 @@ controls:
>  
>    - Brightness:
>        type: int32_t
> -      description: Specify a fixed brightness parameter
> +      description: |
> +        Specify a fixed brightness parameter. Positive values (up to 65535)
> +        produce brighter images; negative values (up to -65536) produce darker

I think 'up to' still makes sense here ... but only asking to be devils
advocate... should this be 'down to -65536' ?

I assume it's then up to the pipelines to convert that brightness range
to whatever capabilities the hardware has anyway.

Wouldn't the control specify the range through a ControlRange to
indicate what range is actually supported?

> +        images and 0 leaves pixels unchanged.
>  
>    - Contrast:
> -      type: int32_t
> -      description: Specify a fixed contrast parameter
> +      type: float
> +      description:  |
> +        Specify a fixed contrast parameter. Normal contrast is given by the
> +        value 1.0; larger values produce images with more contrast.

What does a value less than 1.0 provide? Do we need to document that?

If normal contrast is 1.0, and increasing the contrast is the range:
  (1.0 > ~)
does that mean 'decreasing' the contrast is only the range:
  0.0 > 1.0 ?

Presumably 0 contrast would then give a single colour image?

Would this be more effective to use a scale of 0 == no change, > 0
increases contrast, and < 0 decreases?

>  
>    - Saturation:
> -      type: int32_t
> -      description: Specify a fixed saturation parameter
> +      type: float
> +      description:  |
> +        Specify a fixed saturation parameter. Normal saturation is given by
> +        the value 1.0; larger values produce more saturated colours.
>  ...

Same comments I as above I think.

> 

-- 
Regards
--
Kieran