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

[Jean-Michel Hautbois]

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 !) ?

> 
> 
>> +}
>> +
>>   void RPiCameraData::setSensorControls(ControlList &controls)
>>   {
>>          /*
>> -- 
>> 2.32.0
>>