[libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi: Control the lens from pipeline
Naushir Patuck
naush at raspberrypi.com
Mon Mar 28 09:50:36 CEST 2022
Hi,
On Sun, 27 Mar 2022 at 23:01, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> On Thu, Mar 24, 2022 at 10:28:33AM +0000, Naushir Patuck wrote:
> > On Thu, 24 Mar 2022 at 08:22, Jean-Michel Hautbois wrote:
> > > 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.
>
> I think we'll need that. VCM actuators can be very tricky, especially
> when they don't have a control loop. Large swing values will result in
> overshoot and ringing. Some open-loop VCM actuators have hardware
> features to generate a ramp instead of a step, which can already help
> (the ramp parameters can be programmable).
>
> > 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.
>
> I don't think DelayedControl should handle this, it will be very
> VCM-specific. A VCM helper (or rather, most likely, VCM helper*s*) would
> be better.
>
I agree that DelayedControls is not the right place for this. Perhaps a
new member
function CameraLens::getrFocusPosition that takes in the frame
sequence number
and returns the expected position through some dead-reckoning calculations?
Regards,
Naush
>
> > > >> +}
> > > >> +
> > > >> void RPiCameraData::setSensorControls(ControlList &controls)
> > > >> {
> > > >> /*
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220328/7fc38f2d/attachment-0001.htm>
More information about the libcamera-devel
mailing list