[libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi: Control the lens from pipeline

[Naushir Patuck]

Hi,

On Thu, 24 Mar 2022 at 08:22, Jean-Michel Hautbois via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:

> Hi Kieran,
>
> On 24/03/2022 00:54, Kieran Bingham wrote:
> > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:44)
> >> The lens focus is controled by a VCM, which is linked to the sensor
> >> using the ancillary links.
> >> Pass the control to the config info structure and make it possible to
> >> update by the IPA.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <
> jeanmichel.hautbois at ideasonboard.com>
> >> ---
> >>   include/libcamera/ipa/raspberrypi.mojom         |  1 +
> >>   .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++
> >>   2 files changed, 18 insertions(+)
> >>
> >> diff --git a/include/libcamera/ipa/raspberrypi.mojom
> b/include/libcamera/ipa/raspberrypi.mojom
> >> index acd3cafe..0c3922b0 100644
> >> --- a/include/libcamera/ipa/raspberrypi.mojom
> >> +++ b/include/libcamera/ipa/raspberrypi.mojom
> >> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {
> >>          embeddedComplete(uint32 bufferId);
> >>          setIspControls(libcamera.ControlList controls);
> >>          setDelayedControls(libcamera.ControlList controls);
> >> +       setLensControls(libcamera.ControlList controls);
> >>   };
> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> index c2230199..970f9fe7 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> @@ -33,6 +33,7 @@
> >>
> >>   #include "libcamera/internal/bayer_format.h"
> >>   #include "libcamera/internal/camera.h"
> >> +#include "libcamera/internal/camera_lens.h"
> >>   #include "libcamera/internal/camera_sensor.h"
> >>   #include "libcamera/internal/delayed_controls.h"
> >>   #include "libcamera/internal/device_enumerator.h"
> >> @@ -202,6 +203,7 @@ public:
> >>          void setIspControls(const ControlList &controls);
> >>          void setDelayedControls(const ControlList &controls);
> >>          void setSensorControls(ControlList &controls);
> >> +       void setLensControls(const ControlList &controls);
> >>
> >>          /* bufferComplete signal handlers. */
> >>          void unicamBufferDequeue(FrameBuffer *buffer);
> >> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig
> *sensorConfig)
> >>          ipa_->embeddedComplete.connect(this,
> &RPiCameraData::embeddedComplete);
> >>          ipa_->setIspControls.connect(this,
> &RPiCameraData::setIspControls);
> >>          ipa_->setDelayedControls.connect(this,
> &RPiCameraData::setDelayedControls);
> >> +       ipa_->setLensControls.connect(this,
> &RPiCameraData::setLensControls);
> >>
> >>          /*
> >>           * The configuration (tuning file) is made from the sensor
> name unless
> >> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >>          entityControls.emplace(0, sensor_->controls());
> >>          entityControls.emplace(1, isp_[Isp::Input].dev()->controls());
> >>
> >> +       CameraLens *lens = sensor_->focusLens();
> >> +       if (lens)
> >> +               entityControls.emplace(2, lens->controls());
> >> +
> >>          /* Always send the user transform to the IPA. */
> >>          ipaConfig.transform = static_cast<unsigned
> int>(config->transform);
> >>
> >> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const
> ControlList &controls)
> >>          handleState();
> >>   }
> >>
> >> +void RPiCameraData::setLensControls(const ControlList &ctrls)
> >> +{
> >> +       CameraLens *lens = sensor_->focusLens();
> >> +       if (!lens)
> >> +               return;
> >> +
> >> +       /* \todo Should we keep track of the latest value applied ? */
> >> +
>  lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());
> >
> > Does the algorithm currently take into account what position the lens is
> > at for the statistics it is considering? I'm concerned that it isn't, or
> > that it's considering the position / top of the hill as a value which is
> > mis-matched against the frame...?
> >
> > I presume there are delays that mean it might even be hard to determine
> > how long it takes to move the VCM to a given position - so that might
> > make it more difficult to state which frame had which position?
>
> That's a good remark, and I don't really have an answer to this yet. I
> suppose there is a delay, but I don't know how to measure it easily. And
> I am not sure it is a very big one, so it could very well not be a big
> issue (opened question !) ?
>

This is quite a tricky thing to do.  One option is have the af algorithm
count time/frames and assume it knows about the characteristics of the
actuator and knows when the lens stopped moving to sample the stats.

Another option would be to use the DelayedControls to return the "correct"
lens position for each frame.  The problem with this is the lens movement
time is dependent on displacement needed.  So, we need to adapt
DelayedControls to allow something like "set the control now but inform me
of the change N frames later." That would work, and keeps in line with how
delayed controls report shutter/gain from the sensor.

Naush


>
> >
> >
> >> +}
> >> +
> >>   void RPiCameraData::setSensorControls(ControlList &controls)
> >>   {
> >>          /*
> >> --
> >> 2.32.0
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220324/d9ab433b/attachment.htm>