[libcamera-devel] [PATCH v2 5/6] ipa: raspberrypi: Add a DenoiseAlgorithm class to the Controller
Naushir Patuck
naush at raspberrypi.com
Fri Jan 22 13:46:07 CET 2021
Hi Kieran,
Thank you for your review feedback.
On Fri, 22 Jan 2021 at 11:50, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:
> Hi Naush,
>
> On 22/01/2021 09:25, 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>
> > ---
> > .../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..df1f35cc9e5f
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/controller/denoise_algorithm.hpp
>
> There's a bit of a mix of .h and .hpp within src/ipa/raspberrypi/ but I
> don't think that matters to much here.
>
Sadly, yes there is. Our controller code uses .hpp throughout, and this
file really ought to follow. We do have a non-trivial task of reformatting
all our controller code to better match with the libcamera formatting
style, so we ought to add renaming .hpp to .h to that list.
Regards,
Naush
>
> Otherwise, nothing controversial.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
> > @@ -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 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..f85a09afb380 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::ColourFast)
> > {
> > }
> >
> > @@ -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 = 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 ¶ms) 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
> --
> Kieran
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210122/521762d6/attachment.htm>
More information about the libcamera-devel
mailing list