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

[Laurent Pinchart]

Hello,

On Mon, Nov 23, 2020 at 09:17:33AM +0000, Kieran Bingham wrote:
> On 23/11/2020 08:58, Jacopo Mondi wrote:
> > On Mon, Nov 16, 2020 at 10:40:25AM +0000, Kieran Bingham wrote:
> >> 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 ?
> 
> Ah, yes of course - because if the control is not listed as supported it
> won't be there to set in the first place! I forgot about that.
> 
> So - indeed, no requirement to mark anything as read-only. That will be
> implicit.

There could still be value in specifying this in the yaml file, to
document which controls are meant to be read-only. Instead of writing
"This control is read-only" or "This control is meant to be used in
metadata only", a read-only property (I'm sure we'll bikeshed the name,
maybe "type: metadata" would be better) would allow compile-time and
runtime sanity checks. That's not a prerequisite for this patch though.

> >> 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
> 
> -- 
> Regards
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,

Laurent Pinchart