[libcamera-devel] [PATCH] ipa: rpi: Fix the reporting of Focus FoMs

[David Plowman]

Hi Kieran

Thanks for the review!

On Thu, 27 Jul 2023 at 11:13, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting David Plowman via libcamera-devel (2023-07-20 13:48:23)
> > The FocusFom metadata was no longer being reported back because the
> > "focus.status" metadata was never being created.
> >
> > Additionally, the scaling of the focus FoMs was over-zealous, rounding
> > just about everything down to zero.
> >
> > Fixes: ac7511dc4c59 ("ipa: raspberrypi: Generalise the focus reporting code")
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/rpi/common/ipa_base.cpp | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> > index f40f2e71..2a033264 100644
> > --- a/src/ipa/rpi/common/ipa_base.cpp
> > +++ b/src/ipa/rpi/common/ipa_base.cpp
> > @@ -458,6 +458,9 @@ void IpaBase::processStats(const ProcessParams &params)
> >
> >                 RPiController::StatisticsPtr statistics = platformProcessStats(it->second.planes()[0]);
> >
> > +               /* reportMetadata() will pick this up and set the FocusFoM metadata */
> > +               rpiMetadata.set("focus.status", statistics->focusRegions);
> > +
> >                 helper_->process(statistics, rpiMetadata);
> >                 controller_.process(statistics, &rpiMetadata);
> >
> > @@ -1197,7 +1200,7 @@ void IpaBase::reportMetadata(unsigned int ipaContext)
> >                         }
> >                 }
> >
> > -               uint32_t focusFoM = (sum / numRegions) >> 16;
> > +               uint32_t focusFoM = sum / numRegions;
>
> There's no specific units for this control anyway is there, so this
> still seems reasonable. I wonder if we should have platforms scale the
> values to a defined range somehow - but I don't know what that range
> would be or if it would make much difference anyway.
>
> But having a usable value is better than not so...

Yes, it is an utterly random unitless number that comes out of the
hardware. It's basically the accumulated energy in the response of
some arbitrary sharpening filter, I think.

The most typical use case is to watch it while you twiddle your manual
focus lens, for which it's fine!

Thanks
David

>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> >                 libcameraMetadata_.set(controls::FocusFoM, focusFoM);
> >         }
> >
> > --
> > 2.30.2
> >