[libcamera-devel,v5,6/6] ipa: raspberrypi: Handle control::NoiseReductionMode in the controller

11066 diff mbox series

The application provided noise reduction mode gets passed into the
denoise controller. The denoise controller in turn returns the mode to
the IPA which now sets up the colour denoise processing appropriately.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/ipa/raspberrypi.h |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp | 51 ++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

Hi Naush,

Thank you for the patch.

On Fri, Jan 29, 2021 at 03:11:54PM +0000, Naushir Patuck wrote:
> The application provided noise reduction mode gets passed into the
> denoise controller. The denoise controller in turn returns the mode to
> the IPA which now sets up the colour denoise processing appropriately.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  include/libcamera/ipa/raspberrypi.h |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp | 51 ++++++++++++++++++++++++++++-
>  2 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 1de36039cee0..3be79fc5ed73 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -66,6 +66,7 @@ static const ControlInfoMap Controls = {
>  	{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
>  	{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>  	{ &controls::FrameDurations, ControlInfo(1000, 1000000000) },
> +	{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>  };
>  
>  } /* namespace RPi */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 220cf174aa4f..862c52bde082 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -43,6 +43,7 @@
>  #include "contrast_algorithm.hpp"
>  #include "contrast_status.h"
>  #include "controller.hpp"
> +#include "denoise_algorithm.hpp"
>  #include "denoise_status.h"
>  #include "dpc_status.h"
>  #include "focus_status.h"
> @@ -565,6 +566,7 @@ bool IPARPi::validateIspControls()
>  		V4L2_CID_USER_BCM2835_ISP_SHARPEN,
>  		V4L2_CID_USER_BCM2835_ISP_DPC,
>  		V4L2_CID_USER_BCM2835_ISP_LENS_SHADING,
> +		V4L2_CID_USER_BCM2835_ISP_CDN,
>  	};
>  
>  	for (auto c : ctrls) {
> @@ -614,6 +616,14 @@ static const std::map<int32_t, std::string> AwbModeTable = {
>  	{ controls::AwbCustom, "custom" },
>  };
>  
> +static const std::map<int32_t, RPiController::DenoiseMode> DenoiseModeTable = {
> +	{ controls::draft::NoiseReductionModeOff, RPiController::DenoiseMode::Off },
> +	{ controls::draft::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },
> +	{ controls::draft::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },
> +	{ controls::draft::NoiseReductionModeMinimal, RPiController::DenoiseMode::ColourOff },
> +	{ controls::draft::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },
> +};
> +
>  void IPARPi::queueRequest(const ControlList &controls)
>  {
>  	/* Clear the return metadata buffer. */
> @@ -836,6 +846,23 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			break;
>  		}
>  
> +		case controls::NOISE_REDUCTION_MODE: {
> +			RPiController::DenoiseAlgorithm *sdn = dynamic_cast<RPiController::DenoiseAlgorithm *>(
> +				controller_.GetAlgorithm("SDN"));
> +			ASSERT(sdn);
> +
> +			int32_t idx = ctrl.second.get<int32_t>();
> +			auto mode = DenoiseModeTable.find(idx);
> +			if (mode != DenoiseModeTable.end()) {
> +				sdn->SetMode(mode->second);
> +				libcameraMetadata_.set(controls::draft::NoiseReductionMode, idx);
> +			} else {
> +				LOG(IPARPI, Error) << "Noise reduction mode " << idx
> +						   << " not recognised";
> +			}
> +			break;
> +		}
> +
>  		default:
>  			LOG(IPARPI, Warning)
>  				<< "Ctrl " << controls::controls.at(ctrl.first)->name()
> @@ -1085,18 +1112,40 @@ void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)
>  
>  void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls)
>  {
> +	using RPiController::DenoiseMode;
> +
>  	bcm2835_isp_denoise denoise;
> +	DenoiseMode mode = static_cast<DenoiseMode>(denoiseStatus->mode);
>  
> -	denoise.enabled = 1;
> +	denoise.enabled = mode != DenoiseMode::Off;
>  	denoise.constant = denoiseStatus->noise_constant;
>  	denoise.slope.num = 1000 * denoiseStatus->noise_slope;
>  	denoise.slope.den = 1000;
>  	denoise.strength.num = 1000 * denoiseStatus->strength;
>  	denoise.strength.den = 1000;
>  
> +	/* Set the CDN mode to match the SDN operating mode. */
> +	bcm2835_isp_cdn cdn;
> +	switch (mode) {
> +	case DenoiseMode::ColourFast:
> +		cdn.enabled = 1;
> +		cdn.mode = CDN_MODE_FAST;
> +		break;
> +	case DenoiseMode::ColourHighQuality:
> +		cdn.enabled = 1;
> +		cdn.mode = CDN_MODE_HIGH_QUALITY;
> +		break;
> +	default:
> +		cdn.enabled = 0;
> +	}
> +
>  	ControlValue c(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(&denoise),
>  					    sizeof(denoise) });
>  	ctrls.set(V4L2_CID_USER_BCM2835_ISP_DENOISE, c);
> +
> +	c = ControlValue(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(&cdn),
> +					      sizeof(cdn) });
> +	ctrls.set(V4L2_CID_USER_BCM2835_ISP_CDN, c);
>  }
>  
>  void IPARPi::applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls)