[libcamera-devel] [PATCH v5 5/6] ipa: raspberrypi: Add a DenoiseAlgorithm class to the Controller

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Fri, Jan 29, 2021 at 03:11:53PM +0000, Naushir Patuck wrote:
> This denoise algorithm class will be used to pass in the user requested
> denoise operating mode to the controller. The existing Denoise
> controller will derive from this new DenoiseAlgorithm class.
> 
> Add a denoise mode field in the denoise status metadata object for the
> IPA to use when configuring the ISP.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
>  .../controller/denoise_algorithm.hpp          | 23 +++++++++++++++++++
>  .../raspberrypi/controller/denoise_status.h   |  1 +
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp    | 11 +++++++--
>  src/ipa/raspberrypi/controller/rpi/sdn.hpp    |  5 +++-
>  4 files changed, 37 insertions(+), 3 deletions(-)
>  create mode 100644 src/ipa/raspberrypi/controller/denoise_algorithm.hpp
> 
> diff --git a/src/ipa/raspberrypi/controller/denoise_algorithm.hpp b/src/ipa/raspberrypi/controller/denoise_algorithm.hpp
> new file mode 100644
> index 000000000000..39fcd7e94df2
> --- /dev/null
> +++ b/src/ipa/raspberrypi/controller/denoise_algorithm.hpp
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * denoise.hpp - Denoise control algorithm interface
> + */
> +#pragma once
> +
> +#include "algorithm.hpp"
> +
> +namespace RPiController {
> +
> +enum class DenoiseMode { Off, ColourOff, ColourFast, ColourHighQuality };
> +
> +class DenoiseAlgorithm : public Algorithm
> +{
> +public:
> +	DenoiseAlgorithm(Controller *controller) : Algorithm(controller) {}
> +	// A Denoise algorithm must provide the following:
> +	virtual void SetMode(DenoiseMode mode) = 0;
> +};
> +
> +} // namespace RPiController
> diff --git a/src/ipa/raspberrypi/controller/denoise_status.h b/src/ipa/raspberrypi/controller/denoise_status.h
> index 06d7cfb91ae8..6064cc2c192e 100644
> --- a/src/ipa/raspberrypi/controller/denoise_status.h
> +++ b/src/ipa/raspberrypi/controller/denoise_status.h
> @@ -16,6 +16,7 @@ struct DenoiseStatus {
>  	double noise_constant;
>  	double noise_slope;
>  	double strength;
> +	unsigned int mode;
>  };
>  
>  #ifdef __cplusplus
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> index d8c1521a6633..8e66fc7ec8ac 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: BSD-2-Clause */
>  /*
> - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
>   *
>   * sdn.cpp - SDN (spatial denoise) control algorithm
>   */
> @@ -18,7 +18,7 @@ using namespace RPiController;
>  #define NAME "rpi.sdn"
>  
>  Sdn::Sdn(Controller *controller)
> -	: Algorithm(controller)
> +	: DenoiseAlgorithm(controller), mode_(DenoiseMode::ColourOff)
>  {
>  }
>  
> @@ -48,6 +48,7 @@ void Sdn::Prepare(Metadata *image_metadata)
>  	status.noise_constant = noise_status.noise_constant * deviation_;
>  	status.noise_slope = noise_status.noise_slope * deviation_;
>  	status.strength = strength_;
> +	status.mode = static_cast<std::underlying_type_t<DenoiseMode>>(mode_);
>  	image_metadata->Set("denoise.status", status);
>  	RPI_LOG("Sdn: programmed constant " << status.noise_constant
>  					    << " slope " << status.noise_slope
> @@ -55,6 +56,12 @@ void Sdn::Prepare(Metadata *image_metadata)
>  					    << status.strength);
>  }
>  
> +void Sdn::SetMode(DenoiseMode mode)
> +{
> +	// We only distinguish between off and all other modes.
> +	mode_ = mode;
> +}
> +
>  // Register algorithm with the system.
>  static Algorithm *Create(Controller *controller)
>  {
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.hpp b/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> index 486c000d7b77..2371ce04163f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> @@ -7,12 +7,13 @@
>  #pragma once
>  
>  #include "../algorithm.hpp"
> +#include "../denoise_algorithm.hpp"
>  
>  namespace RPiController {
>  
>  // Algorithm to calculate correct spatial denoise (SDN) settings.
>  
> -class Sdn : public Algorithm
> +class Sdn : public DenoiseAlgorithm
>  {
>  public:
>  	Sdn(Controller *controller = NULL);
> @@ -20,10 +21,12 @@ public:
>  	void Read(boost::property_tree::ptree const &params) override;
>  	void Initialise() override;
>  	void Prepare(Metadata *image_metadata) override;
> +	void SetMode(DenoiseMode mode) override;
>  
>  private:
>  	double deviation_;
>  	double strength_;
> +	DenoiseMode mode_;
>  };
>  
>  } // namespace RPiController

-- 
Regards,

Laurent Pinchart