[libcamera-devel] [PATCH v2 3/3] ipu3: Apply shading adapter as part of AIQ::run2a()

[Umang Jain]

Hi Han-Lin

Thank you for the patch.

On 10/7/21 12:51 PM, Han-Lin Chen wrote:
> Apply shading adapter to correct lens shading for both camera.


both?

The SA will be applied to whichever camera is running with the IPA 
right? I guess we only need a generic statement here like:

     Populate shading adapter input parameters and run it as a part of 
run2a()
     to correct lens shading for the camera.

>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>   aiq/aiq.cpp                  |  3 +++
>   aiq/aiq_input_parameters.cpp | 12 ++++++++++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp
> index 708e9d6..24c61cb 100644
> --- a/aiq/aiq.cpp
> +++ b/aiq/aiq.cpp
> @@ -154,6 +154,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters &params,
>   	params.paParams.exposure_params = results.ae()->exposures[0].exposure;
>   	parameterAdapterRun(params.paParams, results);
>   
> +	params.saParams.awb_results = results.awb();
> +	shadingAdapterRun(params.saParams, results);
> +
>   	afRun(params.afParams, results);
>   
>   	return 0;
> diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp
> index 8a53849..36e2b07 100644
> --- a/aiq/aiq_input_parameters.cpp
> +++ b/aiq/aiq_input_parameters.cpp
> @@ -89,6 +89,15 @@ int AiqInputParameters::configure(const IPAConfigInfo &configInfo)
>   	/* Guess from hal-configs-nautilus/files/camera3_profiles.xml#263 */
>   	sensorDescriptor.coarse_integration_time_max_margin = 10;
>   
> +	sensorFrameParams.horizontal_crop_offset = 0;
> +	sensorFrameParams.vertical_crop_offset = 0;
> +	sensorFrameParams.cropped_image_width = configInfo.sensorInfo.analogCrop.width;
> +	sensorFrameParams.cropped_image_height = configInfo.sensorInfo.analogCrop.height;
> +	sensorFrameParams.horizontal_scaling_numerator = 1;
> +	sensorFrameParams.horizontal_scaling_denominator = 1;
> +	sensorFrameParams.vertical_scaling_numerator = 1;
> +	sensorFrameParams.vertical_scaling_denominator = 1;
> +
>   	return 0;
>   }
>   
> @@ -165,6 +174,9 @@ void AiqInputParameters::setAeAwbAfDefaults()
>   	gbceParams.tone_map_level = ia_aiq_tone_map_level_default;
>   	gbceParams.frame_use = ia_aiq_frame_use_still;
>   	gbceParams.ev_shift = 0;
> +
> +	/* SA Params */
> +	saParams.frame_use = ia_aiq_frame_use_still;


I think we also need here:

     +  saParams.sensor_frame_params = &sensorFrameParams;

To actually pass sensor frame params to the SA run.

Other than that, I think we are almost there for merging this series. 
I'll let Kieran come back and take a look and we can handle the merge 
(fixing the above suggestions as well)

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

>   }
>   
>   } /* namespace ipa::ipu3::aiq */