[libcamera-devel,v2,4/6] ipa: raspberrypi: Rename SdnStatus to DenoiseStatus
diff mbox series

Message ID 20210122092537.708375-5-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Colour denoise
Related show

Commit Message

Naushir Patuck Jan. 22, 2021, 9:25 a.m. UTC 
This change is in anticipation of the addition of a DenoiseAlgorithm
base class which the SDN class will derive from. We want to match the
metadata object name with the base class algorithm name.

This renames:
- SdnStatus metadata object to DenoiseStatus
- "sdn.status" metadata string key to "denoise.status"
- sdn_status.h header file to denoise_status.h

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 .../controller/{sdn_status.h => denoise_status.h}         | 6 +++---
 src/ipa/raspberrypi/controller/rpi/sdn.cpp                | 6 +++---
 src/ipa/raspberrypi/raspberrypi.cpp                       | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)
 rename src/ipa/raspberrypi/controller/{sdn_status.h => denoise_status.h} (65%)

Comments

David Plowman Jan. 22, 2021, 11 a.m. UTC | #1 
Hi Naush

Thanks for this patch. No complaints from me, so:

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

David

On Fri, 22 Jan 2021 at 09:25, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> This change is in anticipation of the addition of a DenoiseAlgorithm
> base class which the SDN class will derive from. We want to match the
> metadata object name with the base class algorithm name.
>
> This renames:
> - SdnStatus metadata object to DenoiseStatus
> - "sdn.status" metadata string key to "denoise.status"
> - sdn_status.h header file to denoise_status.h
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  .../controller/{sdn_status.h => denoise_status.h}         | 6 +++---
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp                | 6 +++---
>  src/ipa/raspberrypi/raspberrypi.cpp                       | 8 ++++----
>  3 files changed, 10 insertions(+), 10 deletions(-)
>  rename src/ipa/raspberrypi/controller/{sdn_status.h => denoise_status.h} (65%)
>
> diff --git a/src/ipa/raspberrypi/controller/sdn_status.h b/src/ipa/raspberrypi/controller/denoise_status.h
> similarity index 65%
> rename from src/ipa/raspberrypi/controller/sdn_status.h
> rename to src/ipa/raspberrypi/controller/denoise_status.h
> index 871e0b62af1f..06d7cfb91ae8 100644
> --- a/src/ipa/raspberrypi/controller/sdn_status.h
> +++ b/src/ipa/raspberrypi/controller/denoise_status.h
> @@ -1,8 +1,8 @@
>  /* SPDX-License-Identifier: BSD-2-Clause */
>  /*
> - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
>   *
> - * sdn_status.h - SDN (spatial denoise) control algorithm status
> + * denoise_status.h - Spatial Denoise control algorithm status
>   */
>  #pragma once
>
> @@ -12,7 +12,7 @@
>  extern "C" {
>  #endif
>
> -struct SdnStatus {
> +struct DenoiseStatus {
>         double noise_constant;
>         double noise_slope;
>         double strength;
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> index aa82830b02b4..d8c1521a6633 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -5,8 +5,8 @@
>   * sdn.cpp - SDN (spatial denoise) control algorithm
>   */
>
> +#include "../denoise_status.h"
>  #include "../noise_status.h"
> -#include "../sdn_status.h"
>
>  #include "sdn.hpp"
>
> @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)
>         RPI_LOG("Noise profile: constant " << noise_status.noise_constant
>                                            << " slope "
>                                            << noise_status.noise_slope);
> -       struct SdnStatus status;
> +       struct DenoiseStatus status;
>         status.noise_constant = noise_status.noise_constant * deviation_;
>         status.noise_slope = noise_status.noise_slope * deviation_;
>         status.strength = strength_;
> -       image_metadata->Set("sdn.status", status);
> +       image_metadata->Set("denoise.status", status);
>         RPI_LOG("Sdn: programmed constant " << status.noise_constant
>                                             << " slope " << status.noise_slope
>                                             << " strength "
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 5ccc7a6551f5..220cf174aa4f 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -43,13 +43,13 @@
>  #include "contrast_algorithm.hpp"
>  #include "contrast_status.h"
>  #include "controller.hpp"
> +#include "denoise_status.h"
>  #include "dpc_status.h"
>  #include "focus_status.h"
>  #include "geq_status.h"
>  #include "lux_status.h"
>  #include "metadata.hpp"
>  #include "noise_status.h"
> -#include "sdn_status.h"
>  #include "sharpen_algorithm.hpp"
>  #include "sharpen_status.h"
>
> @@ -109,7 +109,7 @@ private:
>         void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);
>         void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);
>         void applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls);
> -       void applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls);
> +       void applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls);
>         void applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls);
>         void applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls);
>         void applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls);
> @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)
>                 if (geqStatus)
>                         applyGEQ(geqStatus, ctrls);
>
> -               SdnStatus *denoiseStatus = rpiMetadata_.GetLocked<SdnStatus>("sdn.status");
> +               DenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked<DenoiseStatus>("denoise.status");
>                 if (denoiseStatus)
>                         applyDenoise(denoiseStatus, ctrls);
>
> @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)
>         ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);
>  }
>
> -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls)
> +void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls)
>  {
>         bcm2835_isp_denoise denoise;
>
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Jan. 22, 2021, 11:46 a.m. UTC | #2 
Hi Naush,

On 22/01/2021 09:25, Naushir Patuck wrote:
> This change is in anticipation of the addition of a DenoiseAlgorithm
> base class which the SDN class will derive from. We want to match the
> metadata object name with the base class algorithm name.
> 
> This renames:
> - SdnStatus metadata object to DenoiseStatus
> - "sdn.status" metadata string key to "denoise.status"
> - sdn_status.h header file to denoise_status.h
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  .../controller/{sdn_status.h => denoise_status.h}         | 6 +++---
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp                | 6 +++---
>  src/ipa/raspberrypi/raspberrypi.cpp                       | 8 ++++----
>  3 files changed, 10 insertions(+), 10 deletions(-)
>  rename src/ipa/raspberrypi/controller/{sdn_status.h => denoise_status.h} (65%)
> 
> diff --git a/src/ipa/raspberrypi/controller/sdn_status.h b/src/ipa/raspberrypi/controller/denoise_status.h
> similarity index 65%
> rename from src/ipa/raspberrypi/controller/sdn_status.h
> rename to src/ipa/raspberrypi/controller/denoise_status.h
> index 871e0b62af1f..06d7cfb91ae8 100644
> --- a/src/ipa/raspberrypi/controller/sdn_status.h
> +++ b/src/ipa/raspberrypi/controller/denoise_status.h
> @@ -1,8 +1,8 @@
>  /* SPDX-License-Identifier: BSD-2-Clause */
>  /*
> - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
>   *
> - * sdn_status.h - SDN (spatial denoise) control algorithm status
> + * denoise_status.h - Spatial Denoise control algorithm status
>   */
>  #pragma once
>  
> @@ -12,7 +12,7 @@
>  extern "C" {
>  #endif
>  
> -struct SdnStatus {
> +struct DenoiseStatus {
>  	double noise_constant;
>  	double noise_slope;
>  	double strength;
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp

This is just the status rename then, so I guess no need to do a full
rename of all SDN->denoise?

Anyway, it's just a query.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> index aa82830b02b4..d8c1521a6633 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -5,8 +5,8 @@
>   * sdn.cpp - SDN (spatial denoise) control algorithm
>   */
>  
> +#include "../denoise_status.h"
>  #include "../noise_status.h"
> -#include "../sdn_status.h"
>  
>  #include "sdn.hpp"
>  
> @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)
>  	RPI_LOG("Noise profile: constant " << noise_status.noise_constant
>  					   << " slope "
>  					   << noise_status.noise_slope);
> -	struct SdnStatus status;
> +	struct DenoiseStatus status;
>  	status.noise_constant = noise_status.noise_constant * deviation_;
>  	status.noise_slope = noise_status.noise_slope * deviation_;
>  	status.strength = strength_;
> -	image_metadata->Set("sdn.status", status);
> +	image_metadata->Set("denoise.status", status);
>  	RPI_LOG("Sdn: programmed constant " << status.noise_constant
>  					    << " slope " << status.noise_slope
>  					    << " strength "
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 5ccc7a6551f5..220cf174aa4f 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -43,13 +43,13 @@
>  #include "contrast_algorithm.hpp"
>  #include "contrast_status.h"
>  #include "controller.hpp"
> +#include "denoise_status.h"
>  #include "dpc_status.h"
>  #include "focus_status.h"
>  #include "geq_status.h"
>  #include "lux_status.h"
>  #include "metadata.hpp"
>  #include "noise_status.h"
> -#include "sdn_status.h"
>  #include "sharpen_algorithm.hpp"
>  #include "sharpen_status.h"
>  
> @@ -109,7 +109,7 @@ private:
>  	void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);
>  	void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);
>  	void applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls);
> -	void applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls);
> +	void applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls);
>  	void applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls);
>  	void applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls);
>  	void applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls);
> @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)
>  		if (geqStatus)
>  			applyGEQ(geqStatus, ctrls);
>  
> -		SdnStatus *denoiseStatus = rpiMetadata_.GetLocked<SdnStatus>("sdn.status");
> +		DenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked<DenoiseStatus>("denoise.status");
>  		if (denoiseStatus)
>  			applyDenoise(denoiseStatus, ctrls);
>  
> @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)
>  	ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);
>  }
>  
> -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls)
> +void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls)
>  {
>  	bcm2835_isp_denoise denoise;
>  
>
Naushir Patuck Jan. 22, 2021, 12:02 p.m. UTC | #3 
Hi Kieran,

Thank you for your review feedback.

On Fri, 22 Jan 2021 at 11:46, Kieran Bingham <
kieran.bingham@ideasonboard.com> wrote:

> Hi Naush,
>
> On 22/01/2021 09:25, Naushir Patuck wrote:
> > This change is in anticipation of the addition of a DenoiseAlgorithm
> > base class which the SDN class will derive from. We want to match the
> > metadata object name with the base class algorithm name.
> >
> > This renames:
> > - SdnStatus metadata object to DenoiseStatus
> > - "sdn.status" metadata string key to "denoise.status"
> > - sdn_status.h header file to denoise_status.h
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  .../controller/{sdn_status.h => denoise_status.h}         | 6 +++---
> >  src/ipa/raspberrypi/controller/rpi/sdn.cpp                | 6 +++---
> >  src/ipa/raspberrypi/raspberrypi.cpp                       | 8 ++++----
> >  3 files changed, 10 insertions(+), 10 deletions(-)
> >  rename src/ipa/raspberrypi/controller/{sdn_status.h =>
> denoise_status.h} (65%)
> >
> > diff --git a/src/ipa/raspberrypi/controller/sdn_status.h
> b/src/ipa/raspberrypi/controller/denoise_status.h
> > similarity index 65%
> > rename from src/ipa/raspberrypi/controller/sdn_status.h
> > rename to src/ipa/raspberrypi/controller/denoise_status.h
> > index 871e0b62af1f..06d7cfb91ae8 100644
> > --- a/src/ipa/raspberrypi/controller/sdn_status.h
> > +++ b/src/ipa/raspberrypi/controller/denoise_status.h
> > @@ -1,8 +1,8 @@
> >  /* SPDX-License-Identifier: BSD-2-Clause */
> >  /*
> > - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> > + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
> >   *
> > - * sdn_status.h - SDN (spatial denoise) control algorithm status
> > + * denoise_status.h - Spatial Denoise control algorithm status
> >   */
> >  #pragma once
> >
> > @@ -12,7 +12,7 @@
> >  extern "C" {
> >  #endif
> >
> > -struct SdnStatus {
> > +struct DenoiseStatus {
> >       double noise_constant;
> >       double noise_slope;
> >       double strength;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
>
> This is just the status rename then, so I guess no need to do a full
> rename of all SDN->denoise?
>
> Anyway, it's just a query.
>

No, we do not want a wholesale rename.  David and I did discuss this
separately.  The goal is to mach the name "Denoise" with the
"DenoiseAlgorithm" class in the next patch set.  SDN as a controller is
currently the only derived denoise controller, but that may not be the case
in the future, if that makes sense?

Regards,
Naush



>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>
> > index aa82830b02b4..d8c1521a6633 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > @@ -5,8 +5,8 @@
> >   * sdn.cpp - SDN (spatial denoise) control algorithm
> >   */
> >
> > +#include "../denoise_status.h"
> >  #include "../noise_status.h"
> > -#include "../sdn_status.h"
> >
> >  #include "sdn.hpp"
> >
> > @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)
> >       RPI_LOG("Noise profile: constant " << noise_status.noise_constant
> >                                          << " slope "
> >                                          << noise_status.noise_slope);
> > -     struct SdnStatus status;
> > +     struct DenoiseStatus status;
> >       status.noise_constant = noise_status.noise_constant * deviation_;
> >       status.noise_slope = noise_status.noise_slope * deviation_;
> >       status.strength = strength_;
> > -     image_metadata->Set("sdn.status", status);
> > +     image_metadata->Set("denoise.status", status);
> >       RPI_LOG("Sdn: programmed constant " << status.noise_constant
> >                                           << " slope " <<
> status.noise_slope
> >                                           << " strength "
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 5ccc7a6551f5..220cf174aa4f 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -43,13 +43,13 @@
> >  #include "contrast_algorithm.hpp"
> >  #include "contrast_status.h"
> >  #include "controller.hpp"
> > +#include "denoise_status.h"
> >  #include "dpc_status.h"
> >  #include "focus_status.h"
> >  #include "geq_status.h"
> >  #include "lux_status.h"
> >  #include "metadata.hpp"
> >  #include "noise_status.h"
> > -#include "sdn_status.h"
> >  #include "sharpen_algorithm.hpp"
> >  #include "sharpen_status.h"
> >
> > @@ -109,7 +109,7 @@ private:
> >       void applyBlackLevel(const struct BlackLevelStatus
> *blackLevelStatus, ControlList &ctrls);
> >       void applyGamma(const struct ContrastStatus *contrastStatus,
> ControlList &ctrls);
> >       void applyGEQ(const struct GeqStatus *geqStatus, ControlList
> &ctrls);
> > -     void applyDenoise(const struct SdnStatus *denoiseStatus,
> ControlList &ctrls);
> > +     void applyDenoise(const struct DenoiseStatus *denoiseStatus,
> ControlList &ctrls);
> >       void applySharpen(const struct SharpenStatus *sharpenStatus,
> ControlList &ctrls);
> >       void applyDPC(const struct DpcStatus *dpcStatus, ControlList
> &ctrls);
> >       void applyLS(const struct AlscStatus *lsStatus, ControlList
> &ctrls);
> > @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)
> >               if (geqStatus)
> >                       applyGEQ(geqStatus, ctrls);
> >
> > -             SdnStatus *denoiseStatus =
> rpiMetadata_.GetLocked<SdnStatus>("sdn.status");
> > +             DenoiseStatus *denoiseStatus =
> rpiMetadata_.GetLocked<DenoiseStatus>("denoise.status");
> >               if (denoiseStatus)
> >                       applyDenoise(denoiseStatus, ctrls);
> >
> > @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus
> *geqStatus, ControlList &ctrls)
> >       ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);
> >  }
> >
> > -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus,
> ControlList &ctrls)
> > +void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus,
> ControlList &ctrls)
> >  {
> >       bcm2835_isp_denoise denoise;
> >
> >
>
> --
> Regards
> --
> Kieran
>
Kieran Bingham Jan. 22, 2021, 12:04 p.m. UTC | #4 
Hi Naush,

On 22/01/2021 12:02, Naushir Patuck wrote:
> Hi Kieran,
> 
> Thank you for your review feedback.
> 
> On Fri, 22 Jan 2021 at 11:46, Kieran Bingham
> <kieran.bingham@ideasonboard.com
> <mailto:kieran.bingham@ideasonboard.com>> wrote:
> 
>     Hi Naush,
> 
>     On 22/01/2021 09:25, Naushir Patuck wrote:
>     > This change is in anticipation of the addition of a DenoiseAlgorithm
>     > base class which the SDN class will derive from. We want to match the
>     > metadata object name with the base class algorithm name.
>     >
>     > This renames:
>     > - SdnStatus metadata object to DenoiseStatus
>     > - "sdn.status" metadata string key to "denoise.status"
>     > - sdn_status.h header file to denoise_status.h
>     >
>     > Signed-off-by: Naushir Patuck <naush@raspberrypi.com
>     <mailto:naush@raspberrypi.com>>
>     > ---
>     >  .../controller/{sdn_status.h => denoise_status.h}         | 6 +++---
>     >  src/ipa/raspberrypi/controller/rpi/sdn.cpp                | 6 +++---
>     >  src/ipa/raspberrypi/raspberrypi.cpp                       | 8
>     ++++----
>     >  3 files changed, 10 insertions(+), 10 deletions(-)
>     >  rename src/ipa/raspberrypi/controller/{sdn_status.h =>
>     denoise_status.h} (65%)
>     >
>     > diff --git a/src/ipa/raspberrypi/controller/sdn_status.h
>     b/src/ipa/raspberrypi/controller/denoise_status.h
>     > similarity index 65%
>     > rename from src/ipa/raspberrypi/controller/sdn_status.h
>     > rename to src/ipa/raspberrypi/controller/denoise_status.h
>     > index 871e0b62af1f..06d7cfb91ae8 100644
>     > --- a/src/ipa/raspberrypi/controller/sdn_status.h
>     > +++ b/src/ipa/raspberrypi/controller/denoise_status.h
>     > @@ -1,8 +1,8 @@
>     >  /* SPDX-License-Identifier: BSD-2-Clause */
>     >  /*
>     > - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
>     > + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
>     >   *
>     > - * sdn_status.h - SDN (spatial denoise) control algorithm status
>     > + * denoise_status.h - Spatial Denoise control algorithm status
>     >   */
>     >  #pragma once
>     > 
>     > @@ -12,7 +12,7 @@
>     >  extern "C" {
>     >  #endif
>     > 
>     > -struct SdnStatus {
>     > +struct DenoiseStatus {
>     >       double noise_constant;
>     >       double noise_slope;
>     >       double strength;
>     > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
>     b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> 
>     This is just the status rename then, so I guess no need to do a full
>     rename of all SDN->denoise?
> 
>     Anyway, it's just a query.
> 
> 
> No, we do not want a wholesale rename.  David and I did discuss this
> separately.  The goal is to mach the name "Denoise" with the
> "DenoiseAlgorithm" class in the next patch set.  SDN as a controller is
> currently the only derived denoise controller, but that may not be the
> case in the future, if that makes sense?

Certainly, no worries there.
--
Kieran


> 
> Regards,
> Naush
> 
>  
> 
> 
>     Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com
>     <mailto:kieran.bingham@ideasonboard.com>>
> 
> 
>     > index aa82830b02b4..d8c1521a6633 100644
>     > --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
>     > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
>     > @@ -5,8 +5,8 @@
>     >   * sdn.cpp - SDN (spatial denoise) control algorithm
>     >   */
>     > 
>     > +#include "../denoise_status.h"
>     >  #include "../noise_status.h"
>     > -#include "../sdn_status.h"
>     > 
>     >  #include "sdn.hpp"
>     > 
>     > @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)
>     >       RPI_LOG("Noise profile: constant " <<
>     noise_status.noise_constant
>     >                                          << " slope "
>     >                                          << noise_status.noise_slope);
>     > -     struct SdnStatus status;
>     > +     struct DenoiseStatus status;
>     >       status.noise_constant = noise_status.noise_constant *
>     deviation_;
>     >       status.noise_slope = noise_status.noise_slope * deviation_;
>     >       status.strength = strength_;
>     > -     image_metadata->Set("sdn.status", status);
>     > +     image_metadata->Set("denoise.status", status);
>     >       RPI_LOG("Sdn: programmed constant " << status.noise_constant
>     >                                           << " slope " <<
>     status.noise_slope
>     >                                           << " strength "
>     > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
>     b/src/ipa/raspberrypi/raspberrypi.cpp
>     > index 5ccc7a6551f5..220cf174aa4f 100644
>     > --- a/src/ipa/raspberrypi/raspberrypi.cpp
>     > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>     > @@ -43,13 +43,13 @@
>     >  #include "contrast_algorithm.hpp"
>     >  #include "contrast_status.h"
>     >  #include "controller.hpp"
>     > +#include "denoise_status.h"
>     >  #include "dpc_status.h"
>     >  #include "focus_status.h"
>     >  #include "geq_status.h"
>     >  #include "lux_status.h"
>     >  #include "metadata.hpp"
>     >  #include "noise_status.h"
>     > -#include "sdn_status.h"
>     >  #include "sharpen_algorithm.hpp"
>     >  #include "sharpen_status.h"
>     > 
>     > @@ -109,7 +109,7 @@ private:
>     >       void applyBlackLevel(const struct BlackLevelStatus
>     *blackLevelStatus, ControlList &ctrls);
>     >       void applyGamma(const struct ContrastStatus *contrastStatus,
>     ControlList &ctrls);
>     >       void applyGEQ(const struct GeqStatus *geqStatus, ControlList
>     &ctrls);
>     > -     void applyDenoise(const struct SdnStatus *denoiseStatus,
>     ControlList &ctrls);
>     > +     void applyDenoise(const struct DenoiseStatus *denoiseStatus,
>     ControlList &ctrls);
>     >       void applySharpen(const struct SharpenStatus *sharpenStatus,
>     ControlList &ctrls);
>     >       void applyDPC(const struct DpcStatus *dpcStatus, ControlList
>     &ctrls);
>     >       void applyLS(const struct AlscStatus *lsStatus, ControlList
>     &ctrls);
>     > @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)
>     >               if (geqStatus)
>     >                       applyGEQ(geqStatus, ctrls);
>     > 
>     > -             SdnStatus *denoiseStatus =
>     rpiMetadata_.GetLocked<SdnStatus>("sdn.status");
>     > +             DenoiseStatus *denoiseStatus =
>     rpiMetadata_.GetLocked<DenoiseStatus>("denoise.status");
>     >               if (denoiseStatus)
>     >                       applyDenoise(denoiseStatus, ctrls);
>     > 
>     > @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus
>     *geqStatus, ControlList &ctrls)
>     >       ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);
>     >  }
>     > 
>     > -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus,
>     ControlList &ctrls)
>     > +void IPARPi::applyDenoise(const struct DenoiseStatus
>     *denoiseStatus, ControlList &ctrls)
>     >  {
>     >       bcm2835_isp_denoise denoise;
>     > 
>     >
> 
>     -- 
>     Regards
>     --
>     Kieran
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/controller/sdn_status.h b/src/ipa/raspberrypi/controller/denoise_status.h
similarity index 65%
rename from src/ipa/raspberrypi/controller/sdn_status.h
rename to src/ipa/raspberrypi/controller/denoise_status.h
index 871e0b62af1f..06d7cfb91ae8 100644
--- a/src/ipa/raspberrypi/controller/sdn_status.h
+++ b/src/ipa/raspberrypi/controller/denoise_status.h
@@ -1,8 +1,8 @@ 
 /* SPDX-License-Identifier: BSD-2-Clause */
 /*
- * Copyright (C) 2019, Raspberry Pi (Trading) Limited
+ * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
  *
- * sdn_status.h - SDN (spatial denoise) control algorithm status
+ * denoise_status.h - Spatial Denoise control algorithm status
  */
 #pragma once
 
@@ -12,7 +12,7 @@ 
 extern "C" {
 #endif
 
-struct SdnStatus {
+struct DenoiseStatus {
 	double noise_constant;
 	double noise_slope;
 	double strength;
diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
index aa82830b02b4..d8c1521a6633 100644
--- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
@@ -5,8 +5,8 @@ 
  * sdn.cpp - SDN (spatial denoise) control algorithm
  */
 
+#include "../denoise_status.h"
 #include "../noise_status.h"
-#include "../sdn_status.h"
 
 #include "sdn.hpp"
 
@@ -44,11 +44,11 @@  void Sdn::Prepare(Metadata *image_metadata)
 	RPI_LOG("Noise profile: constant " << noise_status.noise_constant
 					   << " slope "
 					   << noise_status.noise_slope);
-	struct SdnStatus status;
+	struct DenoiseStatus status;
 	status.noise_constant = noise_status.noise_constant * deviation_;
 	status.noise_slope = noise_status.noise_slope * deviation_;
 	status.strength = strength_;
-	image_metadata->Set("sdn.status", status);
+	image_metadata->Set("denoise.status", status);
 	RPI_LOG("Sdn: programmed constant " << status.noise_constant
 					    << " slope " << status.noise_slope
 					    << " strength "
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 5ccc7a6551f5..220cf174aa4f 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -43,13 +43,13 @@ 
 #include "contrast_algorithm.hpp"
 #include "contrast_status.h"
 #include "controller.hpp"
+#include "denoise_status.h"
 #include "dpc_status.h"
 #include "focus_status.h"
 #include "geq_status.h"
 #include "lux_status.h"
 #include "metadata.hpp"
 #include "noise_status.h"
-#include "sdn_status.h"
 #include "sharpen_algorithm.hpp"
 #include "sharpen_status.h"
 
@@ -109,7 +109,7 @@  private:
 	void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);
 	void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);
 	void applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls);
-	void applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls);
+	void applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls);
 	void applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls);
 	void applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls);
 	void applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls);
@@ -899,7 +899,7 @@  void IPARPi::prepareISP(unsigned int bufferId)
 		if (geqStatus)
 			applyGEQ(geqStatus, ctrls);
 
-		SdnStatus *denoiseStatus = rpiMetadata_.GetLocked<SdnStatus>("sdn.status");
+		DenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked<DenoiseStatus>("denoise.status");
 		if (denoiseStatus)
 			applyDenoise(denoiseStatus, ctrls);
 
@@ -1083,7 +1083,7 @@  void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)
 	ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);
 }
 
-void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls)
+void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls)
 {
 	bcm2835_isp_denoise denoise;