[libcamera-devel,1/3] libcamera: add a sharpness strength control

4099 diff mbox series

The control is a single float value with minimum, default and maximum
values. Please read the description for more details.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/libcamera/control_ids.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

Hi David,

Thank you for the patch.

On Fri, Jun 19, 2020 at 10:27:23AM +0100, David Plowman wrote:
> The control is a single float value with minimum, default and maximum
> values. Please read the description for more details.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/libcamera/control_ids.yaml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index 77ebc3f..1bc1b10 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -239,4 +239,15 @@ controls:
>          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels
>          control can only be returned in metadata.
>        size: [4]
> +
> +  - Sharpness:
> +      type: float
> +      description:  |
> +        The strength of the sharpening to be applied.

Should we specify that sharpening is not applied to RAW streams ? Or do
you expect hardware that would be able to apply sharpening in the Bayer
domain ?

> The minimum value
> +        means minimal (or preferably no) sharpening, the maximum should
> +        signify extremely high levels of sharpening (higher than anyone could
> +        reasonably want), and the default value should give a "reasonable"
> +        level, suitable for many use cases. We recommand that the amount
> +        of sharpening applied should be "approximately" proportional to this
> +        parameter.

I propose detailing this a bit further, to explain that 0 means no
sharpening. How about the following ?

	A value of 0.0 means no sharpening. The minimum value means
	minimal sharpening, and shall be 0.0 unless the camera can't
	disable sharpening completely. The default value shall give a
	"reasonable" level or sharpening, suitable for most use cases.
	The maximum value may apply extremely high levels of sharpening,
	higher than anyone could reasonably want. Negative values are
	not allowed.

>  ...

Hi Laurent

Thanks for the feedback.

On Mon, 22 Jun 2020 at 03:07, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Fri, Jun 19, 2020 at 10:27:23AM +0100, David Plowman wrote:
> > The control is a single float value with minimum, default and maximum
> > values. Please read the description for more details.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 77ebc3f..1bc1b10 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -239,4 +239,15 @@ controls:
> >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels
> >          control can only be returned in metadata.
> >        size: [4]
> > +
> > +  - Sharpness:
> > +      type: float
> > +      description:  |
> > +        The strength of the sharpening to be applied.
>
> Should we specify that sharpening is not applied to RAW streams ? Or do
> you expect hardware that would be able to apply sharpening in the Bayer
> domain ?

Agreed. Sharpening a raw stream would seem strange. You'd probably
have to have done all kinds of other stuff before sharpening would make
much sense, at which point it's hardly raw any more.

>
> > The minimum value
> > +        means minimal (or preferably no) sharpening, the maximum should
> > +        signify extremely high levels of sharpening (higher than anyone could
> > +        reasonably want), and the default value should give a "reasonable"
> > +        level, suitable for many use cases. We recommand that the amount
> > +        of sharpening applied should be "approximately" proportional to this
> > +        parameter.
>
> I propose detailing this a bit further, to explain that 0 means no
> sharpening. How about the following ?
>
>         A value of 0.0 means no sharpening. The minimum value means
>         minimal sharpening, and shall be 0.0 unless the camera can't
>         disable sharpening completely. The default value shall give a
>         "reasonable" level or sharpening, suitable for most use cases.
>         The maximum value may apply extremely high levels of sharpening,
>         higher than anyone could reasonably want. Negative values are
>         not allowed.

Yes, I like that. I'll put this all in v2 of the patch.

Best regards
David

>
> >  ...
>
> --
> Regards,
>
> Laurent Pinchart