[{"id":5392,"web_url":"https://patchwork.libcamera.org/comment/5392/","msgid":"<20200625032326.GC5980@pendragon.ideasonboard.com>","date":"2020-06-25T03:23:26","subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: ipa: raspberrypi:\n\tAdd sharpness strength control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Tue, Jun 23, 2020 at 10:14:03AM +0100, David Plowman wrote:\n> The sharpness control is, loosely speaking, a gain applied to\n> the amount of sharpening added to an image. We also report the\n> sharpness setting used back to the caller in metadata.\n> \n> Signed-off-by: David Plowman \n> ---\n> .../raspberrypi/controller/rpi/sharpen.cpp | 28 +++++++++++++++----\n> .../raspberrypi/controller/rpi/sharpen.hpp | 6 ++--\n> .../controller/sharpen_algorithm.hpp | 21 ++++++++++++++\n> .../raspberrypi/controller/sharpen_status.h | 2 ++\n> 4 files changed, 50 insertions(+), 7 deletions(-)\n> create mode 100644 src/ipa/raspberrypi/controller/sharpen_algorithm.hpp\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> index 4c2fdb3..3bae01f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> @@ -17,7 +17,7 @@ using namespace RPi;\n> #define NAME \"rpi.sharpen\"\n> \n> Sharpen::Sharpen(Controller *controller)\n> -\t: Algorithm(controller)\n> +\t: SharpenAlgorithm(controller), user_strength_(1.0)\n> {\n> }\n> \n> @@ -42,14 +42,32 @@ void Sharpen::Read(boost::property_tree::ptree const ¶ms)\n> \tlimit_ = params.get(\"limit\", 1.0);\n> }\n> \n> +void Sharpen::SetStrength(double strength)\n> +{\n> +\t// Note that this method is how an application sets the overall\n> +\t// sharpening \"strength\". We call this the \"user strength\" field\n> +\t// as there already is a strength_ field - being an internal gain\n> +\t// parameter that gets passed to the ISP control code. Negative\n> +\t// values are not allowed - coerce them to zero (no sharpening).\n> +\tuser_strength_ = std::max(0.0, strength);\n> +}\n> +\n> void Sharpen::Prepare(Metadata *image_metadata)\n> {\n> +\t// The user_strength_ affects the algorithm's internal gain directly, but\n> +\t// we adjust the limit and threshold less aggressively. Using a sqrt\n> +\t// function is an arbitrary but gentle way of accomplishing this.\n> +\tdouble user_strength_sqrt = sqrt(user_strength_);\n> \tstruct SharpenStatus status;\n> \t// Binned modes seem to need the sharpening toned down with this\n> -\t// pipeline.\n> -\tstatus.threshold = threshold_ * mode_factor_;\n> -\tstatus.strength = strength_ / mode_factor_;\n> -\tstatus.limit = limit_ / mode_factor_;\n> +\t// pipeline, thus we use the mode_factor here. Also avoid\n> +\t// divide-by-zero with the user_strength_sqrt.\n> +\tstatus.threshold = threshold_ * mode_factor_ /\n> +\t\tstd::max(0.01, user_strength_sqrt);\n\ncheckstyle.py reports that std::max should be aligned under threshold_.\nIf you don't use it already, I recommend setting up checkstyle.py as a\ngit hook. You can simply copy utils/hooks/post-commit or\nutils/hooks/pre-commit to .git/hooks/. I personally use the post-commit\nhook to make it easier to ignore false positives.\n\nWith this small issue fixed,\n\nReviewed-by: Laurent Pinchart \n\n> +\tstatus.strength = strength_ / mode_factor_ * user_strength_;\n> +\tstatus.limit = limit_ / mode_factor_ * user_strength_sqrt;\n> +\t// Finally, report any application-supplied parameters that were used.\n> +\tstatus.user_strength = user_strength_;\n> \timage_metadata->Set(\"sharpen.status\", status);\n> }\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> index a3bf899..568521b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> @@ -6,20 +6,21 @@\n> */\n> #pragma once\n> \n> -#include \"../algorithm.hpp\"\n> +#include \"../sharpen_algorithm.hpp\"\n> #include \"../sharpen_status.h\"\n> \n> // This is our implementation of the \"sharpen algorithm\".\n> \n> namespace RPi {\n> \n> -class Sharpen : public Algorithm\n> +class Sharpen : public SharpenAlgorithm\n> {\n> public:\n> \tSharpen(Controller *controller);\n> \tchar const *Name() const override;\n> \tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> \tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> +\tvoid SetStrength(double strength) override;\n> \tvoid Prepare(Metadata *image_metadata) override;\n> \n> private:\n> @@ -27,6 +28,7 @@ private:\n> \tdouble strength_;\n> \tdouble limit_;\n> \tdouble mode_factor_;\n> +\tdouble user_strength_;\n> };\n> \n> } // namespace RPi\n> diff --git a/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp\n> new file mode 100644\n> index 0000000..3b27a74\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/sharpen_algorithm.hpp\n> @@ -0,0 +1,21 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> + *\n> + * sharpen_algorithm.hpp - sharpness control algorithm interface\n> + */\n> +#pragma once\n> +\n> +#include \"algorithm.hpp\"\n> +\n> +namespace RPi {\n> +\n> +class SharpenAlgorithm : public Algorithm\n> +{\n> +public:\n> +\tSharpenAlgorithm(Controller *controller) : Algorithm(controller) {}\n> +\t// A sharpness control algorithm must provide the following:\n> +\tvirtual void SetStrength(double strength) = 0;\n> +};\n> +\n> +} // namespace RPi\n> diff --git a/src/ipa/raspberrypi/controller/sharpen_status.h b/src/ipa/raspberrypi/controller/sharpen_status.h\n> index 6de80f4..7501b19 100644\n> --- a/src/ipa/raspberrypi/controller/sharpen_status.h\n> +++ b/src/ipa/raspberrypi/controller/sharpen_status.h\n> @@ -19,6 +19,8 @@ struct SharpenStatus {\n> \tdouble strength;\n> \t// upper limit of the allowed sharpening response\n> \tdouble limit;\n> +\t// The sharpening strength requested by the user or application.\n> +\tdouble user_strength;\n> };\n> \n> #ifdef __cplusplus","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B156DC0101\n\tfor ;\n\tThu, 25 Jun 2020 03:23:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49463609A5;\n\tThu, 25 Jun 2020 05:23:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96D99603BB\n\tfor ;\n\tThu, 25 Jun 2020 05:23:27 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02D43521;\n\tThu, 25 Jun 2020 05:23:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pgV/HKu9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593055407;\n\tbh=Wv6Knlp7814yKQGRev+VPnKMnk4FKhWjhtyQbi6tfSQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pgV/HKu9SjN0Vic+dzi9Bxf7Xwo668i6N3J/ltDEEsAcxqQTSaWUKA1GzhAQrdQYQ\n\tVUS3mmeLx4ndS8htF1y/WyQCMXncdP16EYG7LTVo+WWldV+zXUtBysoEnswrRYsAtr\n\ttBh/sradDrDHcdaXIt1HK84lp4ubDqJUkWraekOs=","Date":"Thu, 25 Jun 2020 06:23:26 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200625032326.GC5980@pendragon.ideasonboard.com>","References":"<20200623091404.15155-1-david.plowman@raspberrypi.com>\n\t<20200623091404.15155-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200623091404.15155-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: ipa: raspberrypi:\n\tAdd sharpness strength control","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]