[libcamera-devel] [PATCH 1/1] libcamera: controls: Add DigitalGain control

[Jacopo Mondi]

Hi Kieran,

On Mon, Nov 16, 2020 at 10:40:25AM +0000, Kieran Bingham wrote:
> Hi David,
>
> On 27/10/2020 14:12, David Plowman wrote:
> > This control reports the global digital gain applied by the pipeline
> > as a whole, after capturing a raw image from the sensor.
> >
> > Signed-off-by: David Plowman <david.plowman at 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 c8874fa9..e6362c74 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -530,4 +530,15 @@ controls:
> >          This control is only present when the pipeline supports scaling. Its
> >          maximum valid value is given by the properties::ScalerCropMaximum
> >          property, and the two can be used to implement digital zoom.
> > +
> > +  - DigitalGain:
> > +      type: float
> > +      description: |
> > +        Global digital gain value applied to the image during all the
> > +        processing steps after capturing the image from the sensor. Any raw
> > +        images, therefore, will not include this gain, but the final images
> > +        output by the imaging pipeline as a whole will include it.
> > +
> > +        This control is intended to report the value used by the image
> > +        processing pipeline.
>
>
> If this is a per-stream thing anyway, I guess it will then be up to
> pipeline handlers to set this to the appropriate value for each stream
> when it completes. The fact that this value would not be applicable to a
> RAW stream makes me think it certainly should be a per-stream metadata
> style value.
>
>
> I'd hope this could be handled by a common helper in that instance so it
> doesn't get left out of some pipeline handlers, but included in some,
> and become inconsistent. Not yet sure how we can handle that, but that
> will be a core issue anyway.
>
>
> I wonder if we should mark this somehow as read-only, at least until we
> determine that someone needs to set it.
>
> We could introduce a control property between type: and description:
>   read-only: true

Isn't a read-only control just a metadata ?

Wouldn't it be enough for a pipeline that does not support changing
the control value from applications not reporting it in the list of
supported Camera's controls, but only report it as part of a completed
request's metadata ?

Thanks
  j

>
>
> Otherwise, I see no objections currently. I think we're just waiting on
> top-level thoughts from Laurent. (And perhaps per-stream controls, but
> that brings it's own questions )
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
> >  ...
> >
>
> --
> Regards
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel