[libcamera-devel] [PATCH v2 04/11] pipeline: rkisp1: Add basic AF controls to the supported controls list

[Kieran Bingham]

Quoting Jacopo Mondi via libcamera-devel (2022-07-14 19:30:03)
> Hi Daniel,
> 
> On Wed, Jul 13, 2022 at 10:43:10AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > This will expose the AF controls and will allow controlling them using
> > the top level API.
> >
> > Signed-off-by: Daniel Semkowicz <dse at thaumatec.com>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 7ee80192..99d66b1d 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -958,10 +958,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >       std::unique_ptr<RkISP1CameraData> data =
> >               std::make_unique<RkISP1CameraData>(this, &mainPath_, &selfPath_);
> >
> > -     ControlInfoMap::Map ctrls;
> > -     ctrls.emplace(std::piecewise_construct,
> > -                   std::forward_as_tuple(&controls::AeEnable),
> > -                   std::forward_as_tuple(false, true));
> > +     ControlInfoMap::Map ctrls({
> > +             { &controls::AeEnable, ControlInfo(false, true) },
> > +             { &controls::AfMode, ControlInfo(controls::AfModeValues) },
> > +             { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },
> > +             { &controls::AfPause, ControlInfo(controls::AfPauseValues) }
> 
> Does the algorithm support continuous auto-focus ? Has it been tested ?
> I've been told by those who tried implement it (RPi) that without PDAF
> statistics from the sensor it's rather hard to get it right.
> 
> If the algorithm does not support it I would drop AfPause as it's
> only useful for CAF (and the associated functions should probably be
> removed along the class hierarchy as well maybe)

As this algorithm implementation is going into libipa, I wonder if we
can get it (the common algorithm implementation) to report the controls
it supports and merge that into the ControlList of supported controls
exposed by the IPA?

That would help towards keeping the support modular, and choosing a
different algorithm implementation (perhaps at runtime) could then
correctly reflect the supported controls?

--
Kieran


> 
> > +     });
> >
> >       data->controlInfo_ = ControlInfoMap(std::move(ctrls),
> >                                           controls::controls);
> > --
> > 2.34.1
> >