[{"id":14992,"web_url":"https://patchwork.libcamera.org/comment/14992/","msgid":"","date":"2021-02-04T21:36:18","subject":"Re: [libcamera-devel] [PATCH v5 4/6] ipa: raspberrypi: Rename\n\tSdnStatus to DenoiseStatus","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Fri, Jan 29, 2021 at 03:11:52PM +0000, Naushir Patuck wrote:\n> This change is in anticipation of the addition of a DenoiseAlgorithm\n> base class which the SDN class will derive from. We want to match the\n> metadata object name with the base class algorithm name.\n> \n> This renames:\n> - SdnStatus metadata object to DenoiseStatus\n> - \"sdn.status\" metadata string key to \"denoise.status\"\n> - sdn_status.h header file to denoise_status.h\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> Reviewed-by: Kieran Bingham \n> ---\n> .../controller/{sdn_status.h => denoise_status.h} | 6 +++---\n> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +++---\n> src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++----\n> 3 files changed, 10 insertions(+), 10 deletions(-)\n> rename src/ipa/raspberrypi/controller/{sdn_status.h => denoise_status.h} (65%)\n> \n> diff --git a/src/ipa/raspberrypi/controller/sdn_status.h b/src/ipa/raspberrypi/controller/denoise_status.h\n> similarity index 65%\n> rename from src/ipa/raspberrypi/controller/sdn_status.h\n> rename to src/ipa/raspberrypi/controller/denoise_status.h\n> index 871e0b62af1f..06d7cfb91ae8 100644\n> --- a/src/ipa/raspberrypi/controller/sdn_status.h\n> +++ b/src/ipa/raspberrypi/controller/denoise_status.h\n> @@ -1,8 +1,8 @@\n> /* SPDX-License-Identifier: BSD-2-Clause */\n> /*\n> - * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited\n> *\n> - * sdn_status.h - SDN (spatial denoise) control algorithm status\n> + * denoise_status.h - Spatial Denoise control algorithm status\n\nShould \"Spatial\" be dropped here ?\n\nReviewed-by: Laurent Pinchart \n\n> */\n> #pragma once\n> \n> @@ -12,7 +12,7 @@\n> extern \"C\" {\n> #endif\n> \n> -struct SdnStatus {\n> +struct DenoiseStatus {\n> \tdouble noise_constant;\n> \tdouble noise_slope;\n> \tdouble strength;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> index aa82830b02b4..d8c1521a6633 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> @@ -5,8 +5,8 @@\n> * sdn.cpp - SDN (spatial denoise) control algorithm\n> */\n> \n> +#include \"../denoise_status.h\"\n> #include \"../noise_status.h\"\n> -#include \"../sdn_status.h\"\n> \n> #include \"sdn.hpp\"\n> \n> @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)\n> \tRPI_LOG(\"Noise profile: constant \" << noise_status.noise_constant\n> \t\t\t\t\t << \" slope \"\n> \t\t\t\t\t << noise_status.noise_slope);\n> -\tstruct SdnStatus status;\n> +\tstruct DenoiseStatus status;\n> \tstatus.noise_constant = noise_status.noise_constant * deviation_;\n> \tstatus.noise_slope = noise_status.noise_slope * deviation_;\n> \tstatus.strength = strength_;\n> -\timage_metadata->Set(\"sdn.status\", status);\n> +\timage_metadata->Set(\"denoise.status\", status);\n> \tRPI_LOG(\"Sdn: programmed constant \" << status.noise_constant\n> \t\t\t\t\t << \" slope \" << status.noise_slope\n> \t\t\t\t\t << \" strength \"\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 5ccc7a6551f5..220cf174aa4f 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -43,13 +43,13 @@\n> #include \"contrast_algorithm.hpp\"\n> #include \"contrast_status.h\"\n> #include \"controller.hpp\"\n> +#include \"denoise_status.h\"\n> #include \"dpc_status.h\"\n> #include \"focus_status.h\"\n> #include \"geq_status.h\"\n> #include \"lux_status.h\"\n> #include \"metadata.hpp\"\n> #include \"noise_status.h\"\n> -#include \"sdn_status.h\"\n> #include \"sharpen_algorithm.hpp\"\n> #include \"sharpen_status.h\"\n> \n> @@ -109,7 +109,7 @@ private:\n> \tvoid applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);\n> \tvoid applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);\n> \tvoid applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls);\n> -\tvoid applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls);\n> +\tvoid applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls);\n> \tvoid applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls);\n> \tvoid applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls);\n> \tvoid applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls);\n> @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)\n> \t\tif (geqStatus)\n> \t\t\tapplyGEQ(geqStatus, ctrls);\n> \n> -\t\tSdnStatus *denoiseStatus = rpiMetadata_.GetLocked(\"sdn.status\");\n> +\t\tDenoiseStatus *denoiseStatus = rpiMetadata_.GetLocked(\"denoise.status\");\n> \t\tif (denoiseStatus)\n> \t\t\tapplyDenoise(denoiseStatus, ctrls);\n> \n> @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)\n> \tctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);\n> }\n> \n> -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls)\n> +void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus, ControlList &ctrls)\n> {\n> \tbcm2835_isp_denoise denoise;\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 04055BD162\n\tfor ;\n\tThu, 4 Feb 2021 21:36:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 817076146C;\n\tThu, 4 Feb 2021 22:36:42 +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 4F21761430\n\tfor ;\n\tThu, 4 Feb 2021 22:36:41 +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 C0F4D45D;\n\tThu, 4 Feb 2021 22:36:40 +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=\"mPYIn7Ax\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612474601;\n\tbh=WY/aSH4mepS7HltCW6bZMIOviBw4EyDVwWdolyYUQd4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mPYIn7AxwGn5e00E5ELTRKY2HriashhHc62XkUIIvxfKc4kI5/4Y3Jb7892VRMO7/\n\tvSBKPPClAHSYIQzrWWijHnX+xZOhe5KV/fw7NaJJS+JpuetH8HEyGB0CpJUKaemf+K\n\tfNdwr64Z7Qzjr9TMxu5zqwIGbVVFmWab9jauip3M=","Date":"Thu, 4 Feb 2021 23:36:18 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210129151154.1051163-1-naush@raspberrypi.com>\n\t<20210129151154.1051163-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210129151154.1051163-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v5 4/6] ipa: raspberrypi: Rename\n\tSdnStatus to DenoiseStatus","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\" "}},{"id":15028,"web_url":"https://patchwork.libcamera.org/comment/15028/","msgid":"","date":"2021-02-05T13:44:09","subject":"Re: [libcamera-devel] [PATCH v5 4/6] ipa: raspberrypi: Rename\n\tSdnStatus to DenoiseStatus","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review feedback.\n\nOn Thu, 4 Feb 2021 at 21:36, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Fri, Jan 29, 2021 at 03:11:52PM +0000, Naushir Patuck wrote:\n> > This change is in anticipation of the addition of a DenoiseAlgorithm\n> > base class which the SDN class will derive from. We want to match the\n> > metadata object name with the base class algorithm name.\n> >\n> > This renames:\n> > - SdnStatus metadata object to DenoiseStatus\n> > - \"sdn.status\" metadata string key to \"denoise.status\"\n> > - sdn_status.h header file to denoise_status.h\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > .../controller/{sdn_status.h => denoise_status.h} | 6 +++---\n> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +++---\n> > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++----\n> > 3 files changed, 10 insertions(+), 10 deletions(-)\n> > rename src/ipa/raspberrypi/controller/{sdn_status.h =>\n> denoise_status.h} (65%)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/sdn_status.h\n> b/src/ipa/raspberrypi/controller/denoise_status.h\n> > similarity index 65%\n> > rename from src/ipa/raspberrypi/controller/sdn_status.h\n> > rename to src/ipa/raspberrypi/controller/denoise_status.h\n> > index 871e0b62af1f..06d7cfb91ae8 100644\n> > --- a/src/ipa/raspberrypi/controller/sdn_status.h\n> > +++ b/src/ipa/raspberrypi/controller/denoise_status.h\n> > @@ -1,8 +1,8 @@\n> > /* SPDX-License-Identifier: BSD-2-Clause */\n> > /*\n> > - * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> > + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited\n> > *\n> > - * sdn_status.h - SDN (spatial denoise) control algorithm status\n> > + * denoise_status.h - Spatial Denoise control algorithm status\n>\n> Should \"Spatial\" be dropped here ?\n>\n\nMakes sense, I will drop \"Spatial\".\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > */\n> > #pragma once\n> >\n> > @@ -12,7 +12,7 @@\n> > extern \"C\" {\n> > #endif\n> >\n> > -struct SdnStatus {\n> > +struct DenoiseStatus {\n> > double noise_constant;\n> > double noise_slope;\n> > double strength;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > index aa82830b02b4..d8c1521a6633 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > @@ -5,8 +5,8 @@\n> > * sdn.cpp - SDN (spatial denoise) control algorithm\n> > */\n> >\n> > +#include \"../denoise_status.h\"\n> > #include \"../noise_status.h\"\n> > -#include \"../sdn_status.h\"\n> >\n> > #include \"sdn.hpp\"\n> >\n> > @@ -44,11 +44,11 @@ void Sdn::Prepare(Metadata *image_metadata)\n> > RPI_LOG(\"Noise profile: constant \" << noise_status.noise_constant\n> > << \" slope \"\n> > << noise_status.noise_slope);\n> > - struct SdnStatus status;\n> > + struct DenoiseStatus status;\n> > status.noise_constant = noise_status.noise_constant * deviation_;\n> > status.noise_slope = noise_status.noise_slope * deviation_;\n> > status.strength = strength_;\n> > - image_metadata->Set(\"sdn.status\", status);\n> > + image_metadata->Set(\"denoise.status\", status);\n> > RPI_LOG(\"Sdn: programmed constant \" << status.noise_constant\n> > << \" slope \" <<\n> status.noise_slope\n> > << \" strength \"\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 5ccc7a6551f5..220cf174aa4f 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -43,13 +43,13 @@\n> > #include \"contrast_algorithm.hpp\"\n> > #include \"contrast_status.h\"\n> > #include \"controller.hpp\"\n> > +#include \"denoise_status.h\"\n> > #include \"dpc_status.h\"\n> > #include \"focus_status.h\"\n> > #include \"geq_status.h\"\n> > #include \"lux_status.h\"\n> > #include \"metadata.hpp\"\n> > #include \"noise_status.h\"\n> > -#include \"sdn_status.h\"\n> > #include \"sharpen_algorithm.hpp\"\n> > #include \"sharpen_status.h\"\n> >\n> > @@ -109,7 +109,7 @@ private:\n> > void applyBlackLevel(const struct BlackLevelStatus\n> *blackLevelStatus, ControlList &ctrls);\n> > void applyGamma(const struct ContrastStatus *contrastStatus,\n> ControlList &ctrls);\n> > void applyGEQ(const struct GeqStatus *geqStatus, ControlList\n> &ctrls);\n> > - void applyDenoise(const struct SdnStatus *denoiseStatus,\n> ControlList &ctrls);\n> > + void applyDenoise(const struct DenoiseStatus *denoiseStatus,\n> ControlList &ctrls);\n> > void applySharpen(const struct SharpenStatus *sharpenStatus,\n> ControlList &ctrls);\n> > void applyDPC(const struct DpcStatus *dpcStatus, ControlList\n> &ctrls);\n> > void applyLS(const struct AlscStatus *lsStatus, ControlList\n> &ctrls);\n> > @@ -899,7 +899,7 @@ void IPARPi::prepareISP(unsigned int bufferId)\n> > if (geqStatus)\n> > applyGEQ(geqStatus, ctrls);\n> >\n> > - SdnStatus *denoiseStatus =\n> rpiMetadata_.GetLocked(\"sdn.status\");\n> > + DenoiseStatus *denoiseStatus =\n> rpiMetadata_.GetLocked(\"denoise.status\");\n> > if (denoiseStatus)\n> > applyDenoise(denoiseStatus, ctrls);\n> >\n> > @@ -1083,7 +1083,7 @@ void IPARPi::applyGEQ(const struct GeqStatus\n> *geqStatus, ControlList &ctrls)\n> > ctrls.set(V4L2_CID_USER_BCM2835_ISP_GEQ, c);\n> > }\n> >\n> > -void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus,\n> ControlList &ctrls)\n> > +void IPARPi::applyDenoise(const struct DenoiseStatus *denoiseStatus,\n> ControlList &ctrls)\n> > {\n> > bcm2835_isp_denoise denoise;\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 32328BD162\n\tfor ;\n\tFri, 5 Feb 2021 13:44:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92F3E614BB;\n\tFri, 5 Feb 2021 14:44:27 +0100 (CET)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C9A9614B0\n\tfor ;\n\tFri, 5 Feb 2021 14:44:26 +0100 (CET)","by mail-lj1-x22a.google.com with SMTP id y14so7773194ljn.8\n\tfor ;\n\tFri, 05 Feb 2021 05:44:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"CkSR0DQf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=vbLbTJ2d/fAOFa06ety9p+gz9du6kFReqmDI/gW+Nwo=;\n\tb=CkSR0DQfC4wVPgoBKG8E2hZ7qDD8kO2Br4Af5KhJPlz5q5+KbgVUCIjvCfaPrYFyuR\n\tg2deIBl62QB/mpWahX1KVPt9WkGlyAed7S0y//YqauOsUuHERI5JLpccxbP/mgQwUa3x\n\tBn2r+SQvBIl0SYqDYAzKugXxtV37gUjH/Zn/R4rm5R1f7V4j+7qg/KD7djIAa33bi+ok\n\tNqnRwLfueyJksXDKxCoeuQJ+jWgVO0+iZ0UOod29Rrnpy7WrfJISrwqL3hlevyRgdBgV\n\t+FDjJ/8FLuWTFfFy5jkAaUngwluOzNlzkB+YLQUutn8wT3IJv+dvM86pTWndSBJlsjo9\n\tHfAw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=vbLbTJ2d/fAOFa06ety9p+gz9du6kFReqmDI/gW+Nwo=;\n\tb=AjvoFp6bjshoMH4NbgZccwcvEYxwl6lRJTE9MriNJgatQMd/54lasfrsfK2u7XlLfU\n\tJ5Z4G/4wSeb8wn/SdPJCC5ZaVXDPTgFBd/Z6GK4EMy5tGCMlzrqLgef3pF6iGaaQAhck\n\tQ5OmHn3FnUV4LCpumXzCj0aQRdqQCt2qbmXuKxmW8IELw6QYD1D+UmHT/+ZWSTQzlp5G\n\tndEJVFScPNYpHxUk+mLEdxrZdQktztsk6oJqM930I1kq0F3m/txVA+M/DIhnCrMWszJX\n\t2g7tBMUk/lzjsXn/LyxEiDrlPImZrGIf2geMG11scexi5XGXB3DO+5mXEkOdA+DzlOGR\n\tDP8Q==","X-Gm-Message-State":"AOAM530AGK5LcoGQa+jOIjAVSNT7vGKUAaOWPNm0M5YlhGRndBTr+t+K\n\tZhHvHJzVTAaJLAjUxlmZUZ6dBH8J2YLXrMgEKo1PRg==","X-Google-Smtp-Source":"ABdhPJyKgn7jzCqPJj30rmmlI0k5NVI44YYQhwpYMpmipH+uvFxGWtCeJYpMOBym6jUM3YNF9LGOD9gtOFf9Gkjn0yY=","X-Received":"by 2002:a2e:7605:: with SMTP id r5mr2603237ljc.299.1612532665906;\n\tFri, 05 Feb 2021 05:44:25 -0800 (PST)","MIME-Version":"1.0","References":"<20210129151154.1051163-1-naush@raspberrypi.com>\n\t<20210129151154.1051163-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 5 Feb 2021 13:44:09 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v5 4/6] ipa: raspberrypi: Rename\n\tSdnStatus to DenoiseStatus","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============7782394934503550587==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]