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

[David Plowman]

Hi everyone

Sounds like we're happy enough from the point of view of this thing
being read-only (for Raspberry Pi at least). Would anyone want any
changes to the wording? Perhaps the final sentence/paragraph might now
be better as

"This control is present in a request's ControlList only if the
pipeline supports setting the value. Even when it cannot be set by an
application, the pipeline may still report the actual value used in
the metadata returned with completed requests."

Any other thoughts?

Thanks
David

On Mon, 23 Nov 2020 at 09:17, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> On 23/11/2020 08:58, Jacopo Mondi wrote:
> > 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 ?
>
> 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.
>
> --
> Kieran
>
>
> >
> > 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
>
> --
> Regards
> --
> Kieran