[libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Control the lens position

[Kieran Bingham]

Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-16 09:39:51)
> Now that the ancillary links are configured, we can use the CameraLens
> class and control the VCM through the IPA.
> For now, force a default value for the lens position, until the AF
> algorithm is introduced.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 38 +++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index fd8fecb0..0a0b5a3a 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -108,6 +108,7 @@ private:
>         void setMode(const IPACameraSensorInfo &sensorInfo);
>         bool validateSensorControls();
>         bool validateIspControls();
> +       bool validateLensControls();
>         void queueRequest(const ControlList &controls);
>         void returnEmbeddedBuffer(unsigned int bufferId);
>         void prepareISP(const ipa::RPi::ISPConfig &data);
> @@ -132,6 +133,7 @@ private:
>  
>         ControlInfoMap sensorCtrls_;
>         ControlInfoMap ispCtrls_;
> +       ControlInfoMap lensCtrls_;
>         ControlList libcameraMetadata_;
>  
>         /* Camera sensor params. */
> @@ -342,13 +344,14 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>                       const ipa::RPi::IPAConfig &ipaConfig,
>                       ControlList *controls)
>  {
> -       if (entityControls.size() != 2) {
> -               LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> +       if (entityControls.size() != 3) {

Ok - so that explains the '2' in the previous patch.

Feels a bit arbitrary though ... I wonder if it needs to be a map to
make it more explicit to which control list is which...

> +               LOG(IPARPI, Error) << "No ISP, lens or sensor controls found.";

Again, what happens on a device with no lens ...


>                 return -1;
>         }
>  
>         sensorCtrls_ = entityControls.at(0);
>         ispCtrls_ = entityControls.at(1);
> +       lensCtrls_ = entityControls.at(2);
>  
>         if (!validateSensorControls()) {
>                 LOG(IPARPI, Error) << "Sensor control validation failed.";
> @@ -360,6 +363,11 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>                 return -1;
>         }
>  
> +       if (!validateLensControls()) {

What happens here if there is no lens ...


> +               LOG(IPARPI, Error) << "Lens control validation failed.";
> +               return -1;
> +       }
> +
>         maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
>  
>         /* Setup a metadata ControlList to output metadata. */
> @@ -578,6 +586,23 @@ bool IPARPi::validateIspControls()
>         return true;
>  }
>  
> +bool IPARPi::validateLensControls()
> +{
> +       static const uint32_t ctrls[] = {
> +               V4L2_CID_FOCUS_ABSOLUTE,
> +       };
> +
> +       for (auto c : ctrls) {
> +               if (lensCtrls_.find(c) == lensCtrls_.end()) {
> +                       LOG(IPARPI, Error) << "Unable to find lens control "
> +                                          << utils::hex(c);
> +                       return false;
> +               }
> +       }
> +
> +       return true;
> +}
> +
>  /*
>   * Converting between enums (used in the libcamera API) and the names that
>   * we use to identify different modes. Unfortunately, the conversion tables
> @@ -1066,6 +1091,15 @@ void IPARPi::processStats(unsigned int bufferId)
>  
>                 setDelayedControls.emit(ctrls);
>         }
> +
> +       /*
> +        * Set the focus position
> +        * \todo Use an AF algorithm and replace the arbitrary value
> +        */
> +       ControlList lensCtrls(lensCtrls_);
> +       lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, static_cast<int32_t>(123));
> +       setLensControls.emit(lensCtrls);
> +

no need for extra blank line here.

But I guess this is just a 'sample' for the moment anyway.

It really makes me think changing the IPA API to use a libcamera control
list would be better too - so that the plumbing between is cleaner. But
that's a whole chunk of rework...


>  }
>  
>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
> -- 
> 2.32.0
>