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

15464 diff mbox series

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@ideasonboard.com>
---
 src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

Hi JM

Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-17 09:19:38)
> 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@ideasonboard.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index fd8fecb0..9e66f8fd 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,7 +344,7 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>                       const ipa::RPi::IPAConfig &ipaConfig,
>                       ControlList *controls)
>  {
> -       if (entityControls.size() != 2) {
> +       if (entityControls.size() < 2) {
>                 LOG(IPARPI, Error) << "No ISP or sensor controls found.";
>                 return -1;
>         }
> @@ -360,6 +362,14 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>                 return -1;
>         }
>  
> +       /* Lens may not be present, don't make it an hard assumption. */
> +       auto lensControl = entityControls.find(2);

entityControls is already a map for std::map<unsigned int, ControlInfoMap>

This would be a lot clearer if there was an enum defining what the int
means.

enum entityControlType {
	SensorControls = 0,
	ISPControls = 1,
	LensControls = 2,
};

I presume it's then possible to make the map a 
	std::map<entityControlType, ControlInfoMap>

> +       if (lensControl != entityControls.end()) {
> +               lensCtrls_ = lensControl->second;
> +               if (!validateLensControls())
> +                       LOG(IPARPI, Error) << "Lens control validation failed.";
> +       }
> +
>         maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();
>  
>         /* Setup a metadata ControlList to output metadata. */
> @@ -578,6 +588,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 +1093,18 @@ void IPARPi::processStats(unsigned int bufferId)
>  
>                 setDelayedControls.emit(ctrls);
>         }
> +
> +       /*
> +        * Set the focus position
> +        * \todo Use an AF algorithm and replace the arbitrary value
> +        */
> +       struct FocusStatus focusStatus;
> +       if (rpiMetadata_.Get("focus.status", focusStatus) == 0) {
> +               ControlList lensCtrls(lensCtrls_);
> +               lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, static_cast<int32_t>(123));
> +               setLensControls.emit(lensCtrls);

I think we should probably introduce an algorithm to determine a value
before we set one.

I can't see a reason to set an aribtrary value here right now except as
an example, which posting this patch has provided - but I wouldn't like
to merge this as is, I'd prefer to see this rebased on top of the
algorithm being posted first, even if simplistic, and I assume that's
not far off.

--
Kieran


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