[libcamera-devel] [PATCH v4 2/2] ipa: ipu3: Initialize CameraSensorHelper at IPU3 init stage

[Umang Jain]

Hi JM,

On 6/15/21 7:05 PM, Jean-Michel Hautbois wrote:
> In order for the CameraSensorHelper to be instantiated, we need to find
> its factory using the camera sensor model name stored in
> IPASettings::sensorModel. As we don't need to do it at each configure
> call (the sensor is not changing in-between), implement the init call in
> IPAIPU3 to do that.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>   src/ipa/ipu3/ipu3.cpp | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 8b4c7351..e68e9bb3 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -23,6 +23,7 @@
>   
>   #include "ipu3_agc.h"
>   #include "ipu3_awb.h"
> +#include "libipa/camera_sensor_helper.h"
>   
>   static constexpr uint32_t kMaxCellWidthPerSet = 160;
>   static constexpr uint32_t kMaxCellHeightPerSet = 56;
> @@ -36,10 +37,7 @@ namespace ipa::ipu3 {
>   class IPAIPU3 : public IPAIPU3Interface
>   {
>   public:
> -	int init([[maybe_unused]] const IPASettings &settings) override
> -	{
> -		return 0;
> -	}
> +	int init(const IPASettings &settings) override;
>   	int start() override;
>   	void stop() override {}
>   
> @@ -78,6 +76,8 @@ private:
>   	std::unique_ptr<IPU3Awb> awbAlgo_;
>   	/* Interface to the AEC/AGC algorithm */
>   	std::unique_ptr<IPU3Agc> agcAlgo_;
> +	/* Interface to the Camera Helper */
> +	std::unique_ptr<CameraSensorHelper> camHelper_;
>   
>   	/* Local parameter storage */
>   	struct ipu3_uapi_params params_;
> @@ -85,6 +85,16 @@ private:
>   	struct ipu3_uapi_grid_config bdsGrid_;
>   };
>   
> +int IPAIPU3::init(const IPASettings &settings)
> +{
> +	camHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);
> +	if (camHelper_ == nullptr) {
> +		LOG(IPAIPU3, Error) << "Failed to create camera sensor helper for " << settings.sensorModel;
> +		return -1;

When can this fail? Probably when we are missing some sensorModel's 
CameraSensorHelper class right? So in this that case, I would return:

     return -EINVAL;


> +	}

New line here:

With that,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> +	return 0;
> +}
> +
>   int IPAIPU3::start()
>   {
>   	setControls(0);