[libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up NoiseReductionMode values

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:
> Remove the NoiseReductionMode values that the raspberrypi IPA does not
> support. The ControlInfo values that the IPA reports will be used for
> capability detection, so values that it does not support shall be
> removed.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  include/libcamera/ipa/raspberrypi.h | 8 +++++++-
>  src/ipa/raspberrypi/raspberrypi.cpp | 2 --
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index e0dc6f5e..6e97ef53 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -18,6 +18,12 @@ namespace libcamera {
>  
>  namespace RPi {
>  
> +const std::array<const ControlValue, 3> RPiNoiseReductionModeValues = {

Can this be constexpr ?

> +	static_cast<int32_t>(controls::NoiseReductionModeOff),
> +	static_cast<int32_t>(controls::NoiseReductionModeFast),
> +	static_cast<int32_t>(controls::NoiseReductionModeHighQuality),
> +};

I'm wondering, now that we have the ability to pass control info from
the IPA to the pipeline handler, could we move the control info map to
the IPA before adding more static data ?

Naush, is this something you have looked at by any chance ?

> +
>  /*
>   * List of controls handled by the Raspberry Pi IPA
>   *
> @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({
>  		{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
>  		{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>  		{ &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> -		{ &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }
> +		{ &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }
>  	}, controls::controls);
>  
>  } /* namespace RPi */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 8d44ab0a..daef1c2d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -608,8 +608,6 @@ static const std::map<int32_t, RPiController::DenoiseMode> DenoiseModeTable = {
>  	{ controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },
>  	{ controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },
>  	{ controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },
> -	{ controls::NoiseReductionModeMinimal, RPiController::DenoiseMode::ColourOff },
> -	{ controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },
>  };
>  
>  void IPARPi::queueRequest(const ControlList &controls)

-- 
Regards,

Laurent Pinchart