[libcamera-devel] [PATCH 1/4] libcamera: controls: Add focus Figure of Merit (FoM) control

[Laurent Pinchart]

Hi Naush,

On Mon, Jun 29, 2020 at 11:41:45AM +0100, Naushir Patuck wrote:
> On Mon, 29 Jun 2020 at 01:15, Laurent Pinchart wrote:
> > On Fri, Jun 26, 2020 at 11:25:28AM +0100, Naushir Patuck wrote:
> > > Provide a control to allow the IPA to return a FoM to indicate how
> > > in-focus a scene is. Note, this is not to be used as a means to
> > > implement a focus algorithm by the application, rather an indication of
> > > how in-focus a scene is.
> > >
> > > Signed-off-by: Naushir Patuck <naush 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 8c3e4c72..b46693ec 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -251,4 +251,15 @@ controls:
> > >          higher than anyone could reasonably want. Negative values are
> > >          not allowed. Note also that sharpening is not applied to raw
> > >          streams.
> > > +
> > > +  - FocusFoM:
> > > +      type: int32_t
> > > +      description: |
> > > +        Reports a Figure of Merit (FoM) to indicate how in-focus the frame is.
> > > +        A larger FocusFoM value indicates a more in-focus frame. This control
> > > +        depends on the IPA to gather ISP statistics from the defined focus
> > > +        region, and combine them in a suitable way to generate a FocusFoM value.
> > > +        In this respect, it is not necessarily aimed a providing a way to
> > > +        implement a focus algorithm by the application, rather an indication of
> > > +        how in-focus a frame is.
> >
> > Do you think this would be sufficient for applications, or would it be
> > better to use an array control that would return an array of values ?
> > Looking a 4/4 your ISP returns a grid of 4x3 values, so that could fit
> > in a control without a large performance impact.
> 
> David and I did discuss this at length.  We concluded that a single
> value here was good because it simplified what the application wants
> to know from this control, i.e. how good is the focus in this scene.
> Anything more complicated, like a real focus algorithm would need
> access to all of the grid, and that can live happily in the IPAs.

Agreed, auto-focus should live in the IPA.

I'm curious, what are the use cases for applications, how would they
consume this value ?

Also, do you see any issue with the FocusFoM control as defined here
when used with sensors that produce PDAF data ? I have no particular
concern, but you have more experience than I do in this area.

Another question, I assume that in most (all ?) case the focus
statistics are a measure of sharpness, and that two images with the same
focus quality but with a very different level of details in the scene
would have different focus FoM values, right ? I'd be tempted to name
the control to refer to sharpness instead of focus then, but we already
have a sharpness control that is completely unrelated. We can of course
always change names later, but I'd like your opinion here. Also, is my
understanding applicable to PDAF too, or would there be differences
there ?

> I am ok to extend this to an array to provide all 12 values, but if we
> do that, arguably we should add another control like FocusFomRegions
> that list the region coordinates for each FoM.  This will allow the
> control to be more generic for other HW vendors.

Yes, I agree that would make sense.

> > This leads to the next question of whether we should then report all
> > statistics in a buffer instead, and possibly even standardize the
> > statistics format. That could open pandora's box though, it's a rabbit
> > hole that could end up being pretty deep. I tried to look around for
> > other implementations, and there's support for histogram and focus
> > (sharpness) data in the Android camera HAL API, but marked as
> > "future". It seems to have been work in progress pretty much from the
> > start, without much interest in finalizing it. Looking at OpenKCam could
> > be interesting too, but there's no public spec I could find. Are you
> > aware of any other attempt at standardizing something in this area ?
> 
> I recall we talked about standard formats for statistics a year (or
> so) ago and got very nervous about doing such a thing :)  I was
> briefly involved in the OpenKCam effort, and we did talk about exactly
> this.  If I recall, the consensus between hardware vendors was the
> same - we might spend too much effort (and add lots of complexity) to
> define a generic statistics format, which might never get used by
> users, and vendors never implement.

I share the same concern. It may quickly become too complicated to
really be useful.

> Part of me feels that is still the case with libcamera.  Vendors like
> Raspberry Pi do have the statistics buffer documented.  In the rare
> case a user does want statistics to do some magic stuff in the
> application, they can request the stats buffer (when the code is
> available), and resign to the fact that how they handle the statistics
> data will not be portable with other vendors.

I think this makes sense. If we decide to standardize statistics, it
would probably be better implemented as a library that parses vendor
statistics formats, instead of implemented in the IPA.

I however don't rule out, for some statistics, the possibility of having
the IPA reporting metadata about the statistics format, in order to ease
usage in applications, or even to allow coding sharing of algorithms
between different vendors. That's a topic for later though.

> > >  ...

-- 
Regards,

Laurent Pinchart