[{"id":15038,"web_url":"https://patchwork.libcamera.org/comment/15038/","msgid":"","date":"2021-02-07T14:07:05","subject":"Re: [libcamera-devel] [PATCH 4/7] ipa: rasberrypi: contrast: Remove\n\tunnecessary atomic variables","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 Thu, Feb 04, 2021 at 09:34:54AM +0000, David Plowman wrote:\n> SetBrightness() and SetContrast() are only called synchronously so\n> there is no need for brightness_ and contrast_ to be atomic.\n> \n> Signed-off-by: David Plowman \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/raspberrypi/controller/rpi/contrast.cpp | 9 ++++-----\n> src/ipa/raspberrypi/controller/rpi/contrast.hpp | 5 ++---\n> 2 files changed, 6 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> index 05ed139f..2bc43027 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> @@ -150,7 +150,6 @@ Pwl apply_manual_contrast(Pwl const &gamma_curve, double brightness,\n> void Contrast::Process(StatisticsPtr &stats,\n> \t\t [[maybe_unused]] Metadata *image_metadata)\n> {\n> -\tdouble brightness = brightness_, contrast = contrast_;\n> \tHistogram histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);\n> \t// We look at the histogram and adjust the gamma curve in the following\n> \t// ways: 1. Adjust the gamma curve so as to pull the start of the\n> @@ -165,13 +164,13 @@ void Contrast::Process(StatisticsPtr &stats,\n> \t}\n> \t// 2. Finally apply any manually selected brightness/contrast\n> \t// adjustment.\n> -\tif (brightness != 0 || contrast != 1.0)\n> -\t\tgamma_curve = apply_manual_contrast(gamma_curve, brightness,\n> -\t\t\t\t\t\t contrast);\n> +\tif (brightness_ != 0 || contrast_ != 1.0)\n> +\t\tgamma_curve = apply_manual_contrast(gamma_curve, brightness_,\n> +\t\t\t\t\t\t contrast_);\n> \t// And fill in the status for output. Use more points towards the bottom\n> \t// of the curve.\n> \tContrastStatus status;\n> -\tfill_in_status(status, brightness, contrast, gamma_curve);\n> +\tfill_in_status(status, brightness_, contrast_, gamma_curve);\n> \t{\n> \t\tstd::unique_lock lock(mutex_);\n> \t\tstatus_ = status;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.hpp b/src/ipa/raspberrypi/controller/rpi/contrast.hpp\n> index 6836f181..85624539 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/contrast.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.hpp\n> @@ -6,7 +6,6 @@\n> */\n> #pragma once\n> \n> -#include \n> #include \n> \n> #include \"../contrast_algorithm.hpp\"\n> @@ -42,8 +41,8 @@ public:\n> \n> private:\n> \tContrastConfig config_;\n> -\tstd::atomic brightness_;\n> -\tstd::atomic contrast_;\n> +\tdouble brightness_;\n> +\tdouble contrast_;\n> \tContrastStatus status_;\n> \tstd::mutex mutex_;\n> };","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 AC434BD162\n\tfor ;\n\tSun, 7 Feb 2021 14:07:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B156601B1;\n\tSun, 7 Feb 2021 15:07:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B56D601AB\n\tfor ;\n\tSun, 7 Feb 2021 15:07:28 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EF0E7F3;\n\tSun, 7 Feb 2021 15:07:27 +0100 (CET)"],"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=\"Zuo061xt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612706848;\n\tbh=q6mgsICFOsWiDtp7tMWZjDzcjQyP3+/82nDvrbCzlNA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zuo061xtFt3O0IfYIGfXSyVLcvmhjFgtt7ipSfB1fb2wFC3zsZ2B+bXgf15v5ndmm\n\tKa9XDcInyr+Yn6WKxqG1M8rn/w419fXgLK+3goDNKEJ4pn1reA0/gyFN4b9vVgchwt\n\twqwv50YUYcmsspzKsCQdRH+075c7BmDZpasvyN7U=","Date":"Sun, 7 Feb 2021 16:07:05 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210204093457.6879-1-david.plowman@raspberrypi.com>\n\t<20210204093457.6879-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210204093457.6879-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 4/7] ipa: rasberrypi: contrast: Remove\n\tunnecessary atomic variables","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\" "}}]