Message ID | 20200623091404.15155-4-david.plowman@raspberrypi.com |
---|---|
State | Accepted |
Commit | 0dbc6a507c682db1590105765119b7fa59f6493e |
Headers | show |
Series |
|
Related | show |
Hi David, Thank you for the patch. On Tue, Jun 23, 2020 at 10:14:03AM +0100, David Plowman wrote: > The sharpness control is, loosely speaking, a gain applied to > the amount of sharpening added to an image. We also report the > sharpness setting used back to the caller in metadata. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > .../raspberrypi/controller/rpi/sharpen.cpp | 28 +++++++++++++++---- > .../raspberrypi/controller/rpi/sharpen.hpp | 6 ++-- > .../controller/sharpen_algorithm.hpp | 21 ++++++++++++++ > .../raspberrypi/controller/sharpen_status.h | 2 ++ > 4 files changed, 50 insertions(+), 7 deletions(-) > create mode 100644 src/ipa/raspberrypi/controller/sharpen_algorithm.hpp > > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp > index 4c2fdb3..3bae01f 100644 > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp > @@ -17,7 +17,7 @@ using namespace RPi; > #define NAME "rpi.sharpen" > > Sharpen::Sharpen(Controller *controller) > - : Algorithm(controller) > + : SharpenAlgorithm(controller), user_strength_(1.0) > { > } > > @@ -42,14 +42,32 @@ void Sharpen::Read(boost::property_tree::ptree const ¶ms) > limit_ = params.get<double>("limit", 1.0); > } > > +void Sharpen::SetStrength(double strength) > +{ > + // Note that this method is how an application sets the overall > + // sharpening "strength". We call this the "user strength" field > + // as there already is a strength_ field - being an internal gain > + // parameter that gets passed to the ISP control code. Negative > + // values are not allowed - coerce them to zero (no sharpening). > + user_strength_ = std::max(0.0, strength); > +} > + > void Sharpen::Prepare(Metadata *image_metadata) > { > + // The user_strength_ affects the algorithm's internal gain directly, but > + // we adjust the limit and threshold less aggressively. Using a sqrt > + // function is an arbitrary but gentle way of accomplishing this. > + double user_strength_sqrt = sqrt(user_strength_); > struct SharpenStatus status; > // Binned modes seem to need the sharpening toned down with this > - // pipeline. > - status.threshold = threshold_ * mode_factor_; > - status.strength = strength_ / mode_factor_; > - status.limit = limit_ / mode_factor_; > + // pipeline, thus we use the mode_factor here. Also avoid > + // divide-by-zero with the user_strength_sqrt. > + status.threshold = threshold_ * mode_factor_ / > + std::max(0.01, user_strength_sqrt); checkstyle.py reports that std::max should be aligned under threshold_. If you don't use it already, I recommend setting up checkstyle.py as a git hook. You can simply copy utils/hooks/post-commit or utils/hooks/pre-commit to .git/hooks/. I personally use the post-commit hook to make it easier to ignore false positives. With this small issue fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + status.strength = strength_ / mode_factor_ * user_strength_; > + status.limit = limit_ / mode_factor_ * user_strength_sqrt; > + // Finally, report any application-supplied parameters that were used. > + status.user_strength = user_strength_; > image_metadata->Set("sharpen.status", status); > } > > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp > index a3bf899..568521b 100644 > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp > @@ -6,20 +6,21 @@ > */ > #pragma once > > -#include "../algorithm.hpp" > +#include "../sharpen_algorithm.hpp" > #include "../sharpen_status.h" > > // This is our implementation of the "sharpen algorithm". > > namespace RPi { > > -class Sharpen : public Algorithm > +class Sharpen : public SharpenAlgorithm > { > public: > Sharpen(Controller *controller); > char const *Name() const override; > void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override; > void Read(boost::property_tree::ptree const ¶ms) override; > + void SetStrength(double strength) override; > void Prepare(Metadata *image_metadata) override; > > private: > @@ -27,6 +28,7 @@ private: > double strength_; > double limit_; > double mode_factor_; > + double user_strength_; > }; > > } // namespace RPi > diff --git a/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp > new file mode 100644 > index 0000000..3b27a74 > --- /dev/null > +++ b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: BSD-2-Clause */ > +/* > + * Copyright (C) 2020, Raspberry Pi (Trading) Limited > + * > + * sharpen_algorithm.hpp - sharpness control algorithm interface > + */ > +#pragma once > + > +#include "algorithm.hpp" > + > +namespace RPi { > + > +class SharpenAlgorithm : public Algorithm > +{ > +public: > + SharpenAlgorithm(Controller *controller) : Algorithm(controller) {} > + // A sharpness control algorithm must provide the following: > + virtual void SetStrength(double strength) = 0; > +}; > + > +} // namespace RPi > diff --git a/src/ipa/raspberrypi/controller/sharpen_status.h b/src/ipa/raspberrypi/controller/sharpen_status.h > index 6de80f4..7501b19 100644 > --- a/src/ipa/raspberrypi/controller/sharpen_status.h > +++ b/src/ipa/raspberrypi/controller/sharpen_status.h > @@ -19,6 +19,8 @@ struct SharpenStatus { > double strength; > // upper limit of the allowed sharpening response > double limit; > + // The sharpening strength requested by the user or application. > + double user_strength; > }; > > #ifdef __cplusplus
diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp index 4c2fdb3..3bae01f 100644 --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp @@ -17,7 +17,7 @@ using namespace RPi; #define NAME "rpi.sharpen" Sharpen::Sharpen(Controller *controller) - : Algorithm(controller) + : SharpenAlgorithm(controller), user_strength_(1.0) { } @@ -42,14 +42,32 @@ void Sharpen::Read(boost::property_tree::ptree const ¶ms) limit_ = params.get<double>("limit", 1.0); } +void Sharpen::SetStrength(double strength) +{ + // Note that this method is how an application sets the overall + // sharpening "strength". We call this the "user strength" field + // as there already is a strength_ field - being an internal gain + // parameter that gets passed to the ISP control code. Negative + // values are not allowed - coerce them to zero (no sharpening). + user_strength_ = std::max(0.0, strength); +} + void Sharpen::Prepare(Metadata *image_metadata) { + // The user_strength_ affects the algorithm's internal gain directly, but + // we adjust the limit and threshold less aggressively. Using a sqrt + // function is an arbitrary but gentle way of accomplishing this. + double user_strength_sqrt = sqrt(user_strength_); struct SharpenStatus status; // Binned modes seem to need the sharpening toned down with this - // pipeline. - status.threshold = threshold_ * mode_factor_; - status.strength = strength_ / mode_factor_; - status.limit = limit_ / mode_factor_; + // pipeline, thus we use the mode_factor here. Also avoid + // divide-by-zero with the user_strength_sqrt. + status.threshold = threshold_ * mode_factor_ / + std::max(0.01, user_strength_sqrt); + status.strength = strength_ / mode_factor_ * user_strength_; + status.limit = limit_ / mode_factor_ * user_strength_sqrt; + // Finally, report any application-supplied parameters that were used. + status.user_strength = user_strength_; image_metadata->Set("sharpen.status", status); } diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp index a3bf899..568521b 100644 --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp @@ -6,20 +6,21 @@ */ #pragma once -#include "../algorithm.hpp" +#include "../sharpen_algorithm.hpp" #include "../sharpen_status.h" // This is our implementation of the "sharpen algorithm". namespace RPi { -class Sharpen : public Algorithm +class Sharpen : public SharpenAlgorithm { public: Sharpen(Controller *controller); char const *Name() const override; void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override; void Read(boost::property_tree::ptree const ¶ms) override; + void SetStrength(double strength) override; void Prepare(Metadata *image_metadata) override; private: @@ -27,6 +28,7 @@ private: double strength_; double limit_; double mode_factor_; + double user_strength_; }; } // namespace RPi diff --git a/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp new file mode 100644 index 0000000..3b27a74 --- /dev/null +++ b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/* + * Copyright (C) 2020, Raspberry Pi (Trading) Limited + * + * sharpen_algorithm.hpp - sharpness control algorithm interface + */ +#pragma once + +#include "algorithm.hpp" + +namespace RPi { + +class SharpenAlgorithm : public Algorithm +{ +public: + SharpenAlgorithm(Controller *controller) : Algorithm(controller) {} + // A sharpness control algorithm must provide the following: + virtual void SetStrength(double strength) = 0; +}; + +} // namespace RPi diff --git a/src/ipa/raspberrypi/controller/sharpen_status.h b/src/ipa/raspberrypi/controller/sharpen_status.h index 6de80f4..7501b19 100644 --- a/src/ipa/raspberrypi/controller/sharpen_status.h +++ b/src/ipa/raspberrypi/controller/sharpen_status.h @@ -19,6 +19,8 @@ struct SharpenStatus { double strength; // upper limit of the allowed sharpening response double limit; + // The sharpening strength requested by the user or application. + double user_strength; }; #ifdef __cplusplus
The sharpness control is, loosely speaking, a gain applied to the amount of sharpening added to an image. We also report the sharpness setting used back to the caller in metadata. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- .../raspberrypi/controller/rpi/sharpen.cpp | 28 +++++++++++++++---- .../raspberrypi/controller/rpi/sharpen.hpp | 6 ++-- .../controller/sharpen_algorithm.hpp | 21 ++++++++++++++ .../raspberrypi/controller/sharpen_status.h | 2 ++ 4 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 src/ipa/raspberrypi/controller/sharpen_algorithm.hpp