[{"id":19695,"web_url":"https://patchwork.libcamera.org/comment/19695/","msgid":"","date":"2021-09-15T01:48:39","subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:\n> Remove the NoiseReductionMode values that the raspberrypi IPA does not\n> support. The ControlInfo values that the IPA reports will be used for\n> capability detection, so values that it does not support shall be\n> removed.\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/ipa/raspberrypi.h | 8 +++++++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 2 --\n> 2 files changed, 7 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index e0dc6f5e..6e97ef53 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -18,6 +18,12 @@ namespace libcamera {\n> \n> namespace RPi {\n> \n> +const std::array RPiNoiseReductionModeValues = {\n\nCan this be constexpr ?\n\n> +\tstatic_cast(controls::NoiseReductionModeOff),\n> +\tstatic_cast(controls::NoiseReductionModeFast),\n> +\tstatic_cast(controls::NoiseReductionModeHighQuality),\n> +};\n\nI'm wondering, now that we have the ability to pass control info from\nthe IPA to the pipeline handler, could we move the control info map to\nthe IPA before adding more static data ?\n\nNaush, is this something you have looked at by any chance ?\n\n> +\n> /*\n> * List of controls handled by the Raspberry Pi IPA\n> *\n> @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({\n> \t\t{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> \t\t{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> \t\t{ &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> -\t\t{ &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> +\t\t{ &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n> \t}, controls::controls);\n> \n> } /* namespace RPi */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 8d44ab0a..daef1c2d 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -608,8 +608,6 @@ static const std::map DenoiseModeTable = {\n> \t{ controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> \t{ controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> \t{ controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> -\t{ controls::NoiseReductionModeMinimal, RPiController::DenoiseMode::ColourOff },\n> -\t{ controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> };\n> \n> void IPARPi::queueRequest(const ControlList &controls)","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 C6157BDC71\n\tfor ;\n\tWed, 15 Sep 2021 01:49:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3683569189;\n\tWed, 15 Sep 2021 03:49:06 +0200 (CEST)","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 E1F9860247\n\tfor ;\n\tWed, 15 Sep 2021 03:49:04 +0200 (CEST)","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 4ECFC24F;\n\tWed, 15 Sep 2021 03:49:04 +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=\"s7X4VsCN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631670544;\n\tbh=NuDAR4bg4TPsKdFY2wCwn5px/6ykyFM/otacp5z7o80=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s7X4VsCNxH4meN0QUTSuOwGo7M6Aj03wVOJO5w/mPeqCroh7t8jakzrFzWmHTO4jY\n\tOpcVUeHQOI24Y9/SeFUTXMVIY5qvattB/d1bkMO94+OrbXXklsAGP7Z0z+/1ydh2/l\n\tLa2NrshSw2gC9qOHA44HqJ8I1hoU+Lco2YZffKr8=","Date":"Wed, 15 Sep 2021 04:48:39 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210913102007.2303225-2-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19699,"web_url":"https://patchwork.libcamera.org/comment/19699/","msgid":"<20210915030844.GG1857410@pyrite.rasen.tech>","date":"2021-09-15T03:08:44","subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Wed, Sep 15, 2021 at 04:48:39AM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:\n> > Remove the NoiseReductionMode values that the raspberrypi IPA does not\n> > support. The ControlInfo values that the IPA reports will be used for\n> > capability detection, so values that it does not support shall be\n> > removed.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > include/libcamera/ipa/raspberrypi.h | 8 +++++++-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 2 --\n> > 2 files changed, 7 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > index e0dc6f5e..6e97ef53 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -18,6 +18,12 @@ namespace libcamera {\n> > \n> > namespace RPi {\n> > \n> > +const std::array RPiNoiseReductionModeValues = {\n> \n> Can this be constexpr ?\n\nYeah.\n\n> \n> > +\tstatic_cast(controls::NoiseReductionModeOff),\n> > +\tstatic_cast(controls::NoiseReductionModeFast),\n> > +\tstatic_cast(controls::NoiseReductionModeHighQuality),\n> > +};\n> \n> I'm wondering, now that we have the ability to pass control info from\n> the IPA to the pipeline handler, could we move the control info map to\n> the IPA before adding more static data ?\n\nThat's probably a better idea. I think it should be done before this\nthen.\n\n\nPaul\n\n> \n> Naush, is this something you have looked at by any chance ?\n> \n> > +\n> > /*\n> > * List of controls handled by the Raspberry Pi IPA\n> > *\n> > @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({\n> > \t\t{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> > \t\t{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > \t\t{ &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > -\t\t{ &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> > +\t\t{ &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n> > \t}, controls::controls);\n> > \n> > } /* namespace RPi */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 8d44ab0a..daef1c2d 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -608,8 +608,6 @@ static const std::map DenoiseModeTable = {\n> > \t{ controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> > \t{ controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> > \t{ controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> > -\t{ controls::NoiseReductionModeMinimal, RPiController::DenoiseMode::ColourOff },\n> > -\t{ controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> > };\n> > \n> > void IPARPi::queueRequest(const ControlList &controls)","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 0F6F6BDC71\n\tfor ;\n\tWed, 15 Sep 2021 03:08:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D260669189;\n\tWed, 15 Sep 2021 05:08:53 +0200 (CEST)","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 9332060247\n\tfor ;\n\tWed, 15 Sep 2021 05:08:52 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD03F24F;\n\tWed, 15 Sep 2021 05:08:50 +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=\"GomN+fPm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631675332;\n\tbh=h1TxpHW0HE0kOkE8M1PwKCTRzzuWoyDEw1BqoOzMkTU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GomN+fPmtLMc0FdfWng0lzywbgYAZd7mZXGX5TYaRI1CfM6TYq6h0BPaQBTFI5IgC\n\tyoT4N0rgXrUwre5T1dHK9atfyHfLQ71ejGWOFUb2T9gpdtVTdMX2/BBSjljsMwtafC\n\twEr+QzrLabyhAuW24wAc38doNerRNmIWXRzitTcU=","Date":"Wed, 15 Sep 2021 12:08:44 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210915030844.GG1857410@pyrite.rasen.tech>","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-2-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19703,"web_url":"https://patchwork.libcamera.org/comment/19703/","msgid":"","date":"2021-09-15T07:53:50","subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Wed, 15 Sept 2021 at 02:49, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Paul,\n>\n> Thank you for the patch.\n>\n> On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:\n> > Remove the NoiseReductionMode values that the raspberrypi IPA does not\n> > support. The ControlInfo values that the IPA reports will be used for\n> > capability detection, so values that it does not support shall be\n> > removed.\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> > include/libcamera/ipa/raspberrypi.h | 8 +++++++-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 2 --\n> > 2 files changed, 7 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> > index e0dc6f5e..6e97ef53 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -18,6 +18,12 @@ namespace libcamera {\n> >\n> > namespace RPi {\n> >\n> > +const std::array RPiNoiseReductionModeValues = {\n>\n> Can this be constexpr ?\n>\n> > + static_cast(controls::NoiseReductionModeOff),\n> > + static_cast(controls::NoiseReductionModeFast),\n> > + static_cast(controls::NoiseReductionModeHighQuality),\n> > +};\n>\n> I'm wondering, now that we have the ability to pass control info from\n> the IPA to the pipeline handler, could we move the control info map to\n> the IPA before adding more static data ?\n>\n> Naush, is this something you have looked at by any chance ?\n>\n\nI haven't looked into this, but it makes sense to do it this way.\nIt would be almost trivial to pass the control info from ipa::init().\n\nThanks,\nNaush\n\n\n>\n> > +\n> > /*\n> > * List of controls handled by the Raspberry Pi IPA\n> > *\n> > @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({\n> > { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f,\n> 16.0f) },\n> > { &controls::ScalerCrop, ControlInfo(Rectangle{},\n> Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > { &controls::FrameDurationLimits,\n> ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > - { &controls::NoiseReductionMode,\n> ControlInfo(controls::NoiseReductionModeValues) }\n> > + { &controls::NoiseReductionMode,\n> ControlInfo(RPiNoiseReductionModeValues) }\n> > }, controls::controls);\n> >\n> > } /* namespace RPi */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 8d44ab0a..daef1c2d 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -608,8 +608,6 @@ static const std::map RPiController::DenoiseMode> DenoiseModeTable = {\n> > { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off\n> },\n> > { controls::NoiseReductionModeFast,\n> RPiController::DenoiseMode::ColourFast },\n> > { controls::NoiseReductionModeHighQuality,\n> RPiController::DenoiseMode::ColourHighQuality },\n> > - { controls::NoiseReductionModeMinimal,\n> RPiController::DenoiseMode::ColourOff },\n> > - { controls::NoiseReductionModeZSL,\n> RPiController::DenoiseMode::ColourHighQuality },\n> > };\n> >\n> > void IPARPi::queueRequest(const ControlList &controls)\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 2F818BDC71\n\tfor ;\n\tWed, 15 Sep 2021 07:54:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CC066918B;\n\tWed, 15 Sep 2021 09:54:09 +0200 (CEST)","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 1B71960134\n\tfor ;\n\tWed, 15 Sep 2021 09:54:07 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id w4so3509556ljh.13\n\tfor ;\n\tWed, 15 Sep 2021 00:54:07 -0700 (PDT)"],"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=\"KVsgTe5K\"; 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=56j2xOsQgGHky79P8W0BHgw/fEmYUNyeTp9bg+pxK/8=;\n\tb=KVsgTe5K13EfSbSbHMIB8aCuygKu5TUIjDWFU85jVz/E+MYaUIs3V6SkeiNHg/Tt9D\n\tAbREtRahkQG6IGCwu5BuTnKMdNx6uL2Cm+mSmZP6ym/t3FBqRy93CL+apQ3aD1dHSU4j\n\tnBTIyoBcsvKMc32wfXDU2AeEDEo+wuK3prb6Fx3tcgCX18CmWMPBGePtqqctNyWEafSy\n\tkcoknzOMBECC0QfDeuNRlyehg4ndNFGY3bHOli2Cmq+OOq5rY+6q1GtGWQ1M5+muTxmG\n\tGaVCaJsrhhCUReRf6CuX+ibscde92ljjfrKR2aUh+9wPPQ+qmwogdjjYyH127Q9/jnFS\n\tvjTg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=56j2xOsQgGHky79P8W0BHgw/fEmYUNyeTp9bg+pxK/8=;\n\tb=gDjKXvNXvEfmxtAcNiXdTBrKJ05wxqWibArxeStfNWpvtacmylz+90Avp777CFjkP8\n\thQ8PvsNP4T8Otb/zt61/gFLhwYY3x958bFS937i/zmLCZmfcRE1lWKcGC/uVrnvVBAhK\n\t11dGN9NeSWwR0c+97mGEaREJf0eF0JMRlbBk7FWZ8nFit4aD0TtDAx2Mj1qBCD7M1rGv\n\twTQGr9OGIQT0EMsERBXxOhTfeUhlFjLyY4Yt8cg0Z1OxaPHDB893nHo0k+Y3uBgoxugi\n\t0df1Rex2BnWhx015egX7V61iUu5M6ws8Loq/sETHrgQE5LzlZae/8PdClez3raXKUEOx\n\twhnA==","X-Gm-Message-State":"AOAM531kdjlN25Fm/PSUqLYdw05ck74Xmu3kfjI8y/OJtxwpOy9bi+LN\n\tWijbQ+ttRoGN31dMGVttljsxZklpGsVlqhKYqWuxYk7gMXs=","X-Google-Smtp-Source":"ABdhPJwUy4vM7MQ/vL/CDamtQsvNFumJNqNvoMwuiT7YuKJr+1/UryFG/n2bkyMVKn7bFPfc1EUw7EnclpTWGV/Puxo=","X-Received":"by 2002:a2e:a486:: with SMTP id\n\th6mr19735845lji.208.1631692446424; \n\tWed, 15 Sep 2021 00:54:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-2-paul.elder@ideasonboard.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 15 Sep 2021 08:53:50 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000a6f1cf05cc0400be\"","Subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19704,"web_url":"https://patchwork.libcamera.org/comment/19704/","msgid":"","date":"2021-09-15T08:28:40","subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Sep 15, 2021 at 08:53:50AM +0100, Naushir Patuck wrote:\n> On Wed, 15 Sept 2021 at 02:49, Laurent Pinchart wrote:\n> > On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:\n> > > Remove the NoiseReductionMode values that the raspberrypi IPA does not\n> > > support. The ControlInfo values that the IPA reports will be used for\n> > > capability detection, so values that it does not support shall be\n> > > removed.\n> > >\n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > include/libcamera/ipa/raspberrypi.h | 8 +++++++-\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 2 --\n> > > 2 files changed, 7 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > > index e0dc6f5e..6e97ef53 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.h\n> > > +++ b/include/libcamera/ipa/raspberrypi.h\n> > > @@ -18,6 +18,12 @@ namespace libcamera {\n> > >\n> > > namespace RPi {\n> > >\n> > > +const std::array RPiNoiseReductionModeValues = {\n> >\n> > Can this be constexpr ?\n> >\n> > > + static_cast(controls::NoiseReductionModeOff),\n> > > + static_cast(controls::NoiseReductionModeFast),\n> > > + static_cast(controls::NoiseReductionModeHighQuality),\n> > > +};\n> >\n> > I'm wondering, now that we have the ability to pass control info from\n> > the IPA to the pipeline handler, could we move the control info map to\n> > the IPA before adding more static data ?\n> >\n> > Naush, is this something you have looked at by any chance ?\n> \n> I haven't looked into this, but it makes sense to do it this way.\n> It would be almost trivial to pass the control info from ipa::init().\n\nSounds good to me. Let's avoid Paul and you implementing this in\nparallel, is this something you plan to work on ?\n\n> > > +\n> > > /*\n> > > * List of controls handled by the Raspberry Pi IPA\n> > > *\n> > > @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({\n> > > { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f,\n> > 16.0f) },\n> > > { &controls::ScalerCrop, ControlInfo(Rectangle{},\n> > Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > { &controls::FrameDurationLimits,\n> > ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > > - { &controls::NoiseReductionMode,\n> > ControlInfo(controls::NoiseReductionModeValues) }\n> > > + { &controls::NoiseReductionMode,\n> > ControlInfo(RPiNoiseReductionModeValues) }\n> > > }, controls::controls);\n> > >\n> > > } /* namespace RPi */\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> > b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index 8d44ab0a..daef1c2d 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -608,8 +608,6 @@ static const std::map > RPiController::DenoiseMode> DenoiseModeTable = {\n> > > { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off\n> > },\n> > > { controls::NoiseReductionModeFast,\n> > RPiController::DenoiseMode::ColourFast },\n> > > { controls::NoiseReductionModeHighQuality,\n> > RPiController::DenoiseMode::ColourHighQuality },\n> > > - { controls::NoiseReductionModeMinimal,\n> > RPiController::DenoiseMode::ColourOff },\n> > > - { controls::NoiseReductionModeZSL,\n> > RPiController::DenoiseMode::ColourHighQuality },\n> > > };\n> > >\n> > > void IPARPi::queueRequest(const ControlList &controls)","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 9F6C1BDB1D\n\tfor ;\n\tWed, 15 Sep 2021 08:29:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D8F869189;\n\tWed, 15 Sep 2021 10:29:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ECBDA60134\n\tfor ;\n\tWed, 15 Sep 2021 10:29:08 +0200 (CEST)","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 46E8724F;\n\tWed, 15 Sep 2021 10:29:08 +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=\"GVli7ZBI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631694548;\n\tbh=T/UyyISiDseJdTpTuIHIw4O8AsCuAi3CdyHIlL9B3EQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GVli7ZBIq8GJcYx5J8ROKxVKK8kbLnsbGUT9Ddkb8SbPiYWETosveT5dvGVbp9vTZ\n\tpM7w9kDfPpF6PtznDh9lc0T0+EWQIpqGmh1NSEvtw+662/+yZdPBSWHdF9pxKwxbD5\n\tAQuVodpEKNAquOXjpq9i04lZPzIuoDmTZoY5w744=","Date":"Wed, 15 Sep 2021 11:28:40 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-2-paul.elder@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19705,"web_url":"https://patchwork.libcamera.org/comment/19705/","msgid":"","date":"2021-09-15T09:26:48","subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 15 Sept 2021 at 09:29, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Wed, Sep 15, 2021 at 08:53:50AM +0100, Naushir Patuck wrote:\n> > On Wed, 15 Sept 2021 at 02:49, Laurent Pinchart wrote:\n> > > On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:\n> > > > Remove the NoiseReductionMode values that the raspberrypi IPA does\n> not\n> > > > support. The ControlInfo values that the IPA reports will be used for\n> > > > capability detection, so values that it does not support shall be\n> > > > removed.\n> > > >\n> > > > Signed-off-by: Paul Elder \n> > > > ---\n> > > > include/libcamera/ipa/raspberrypi.h | 8 +++++++-\n> > > > src/ipa/raspberrypi/raspberrypi.cpp | 2 --\n> > > > 2 files changed, 7 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/ipa/raspberrypi.h\n> b/include/libcamera/ipa/raspberrypi.h\n> > > > index e0dc6f5e..6e97ef53 100644\n> > > > --- a/include/libcamera/ipa/raspberrypi.h\n> > > > +++ b/include/libcamera/ipa/raspberrypi.h\n> > > > @@ -18,6 +18,12 @@ namespace libcamera {\n> > > >\n> > > > namespace RPi {\n> > > >\n> > > > +const std::array RPiNoiseReductionModeValues\n> = {\n> > >\n> > > Can this be constexpr ?\n> > >\n> > > > + static_cast(controls::NoiseReductionModeOff),\n> > > > + static_cast(controls::NoiseReductionModeFast),\n> > > > + static_cast(controls::NoiseReductionModeHighQuality),\n> > > > +};\n> > >\n> > > I'm wondering, now that we have the ability to pass control info from\n> > > the IPA to the pipeline handler, could we move the control info map to\n> > > the IPA before adding more static data ?\n> > >\n> > > Naush, is this something you have looked at by any chance ?\n> >\n> > I haven't looked into this, but it makes sense to do it this way.\n> > It would be almost trivial to pass the control info from ipa::init().\n>\n> Sounds good to me. Let's avoid Paul and you implementing this in\n> parallel, is this something you plan to work on ?\n>\n\nI could do the change (hopefully) by the end of the week.\n\nRegards,\nNaush\n\n\n\n>\n> > > > +\n> > > > /*\n> > > > * List of controls handled by the Raspberry Pi IPA\n> > > > *\n> > > > @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({\n> > > > { &controls::ColourCorrectionMatrix,\n> ControlInfo(-16.0f,\n> > > 16.0f) },\n> > > > { &controls::ScalerCrop, ControlInfo(Rectangle{},\n> > > Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > { &controls::FrameDurationLimits,\n> > > ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > > > - { &controls::NoiseReductionMode,\n> > > ControlInfo(controls::NoiseReductionModeValues) }\n> > > > + { &controls::NoiseReductionMode,\n> > > ControlInfo(RPiNoiseReductionModeValues) }\n> > > > }, controls::controls);\n> > > >\n> > > > } /* namespace RPi */\n> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > index 8d44ab0a..daef1c2d 100644\n> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > @@ -608,8 +608,6 @@ static const std::map > > RPiController::DenoiseMode> DenoiseModeTable = {\n> > > > { controls::NoiseReductionModeOff,\n> RPiController::DenoiseMode::Off\n> > > },\n> > > > { controls::NoiseReductionModeFast,\n> > > RPiController::DenoiseMode::ColourFast },\n> > > > { controls::NoiseReductionModeHighQuality,\n> > > RPiController::DenoiseMode::ColourHighQuality },\n> > > > - { controls::NoiseReductionModeMinimal,\n> > > RPiController::DenoiseMode::ColourOff },\n> > > > - { controls::NoiseReductionModeZSL,\n> > > RPiController::DenoiseMode::ColourHighQuality },\n> > > > };\n> > > >\n> > > > void IPARPi::queueRequest(const ControlList &controls)\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 9A51FBDC71\n\tfor ;\n\tWed, 15 Sep 2021 09:27:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 281FC69189;\n\tWed, 15 Sep 2021 11:27:06 +0200 (CEST)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3EF3760134\n\tfor ;\n\tWed, 15 Sep 2021 11:27:05 +0200 (CEST)","by mail-lf1-x134.google.com with SMTP id b18so3565139lfb.1\n\tfor ;\n\tWed, 15 Sep 2021 02:27:05 -0700 (PDT)"],"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=\"jJ5T0EY4\"; 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=y8W5XBBTch8jWdSvcQDz7YmfGUrPOK7CluJ1/GPd2jk=;\n\tb=jJ5T0EY4rz8XGHRyaFNbB7eOMNanUpTnmFI4KuPhn40R5fabDYxcCNLTtYK9OA8ahc\n\tpp/SluEWSypl6ECHkKPZ2X9lqG5BAJIEMiGj97HGjqGMFLDIAkpEYK1CQNIIgGfgC1G6\n\t4wEsDjm06kQEMrZFYC/rICrnmqRKOCbKh41SkmQnGu4itNib4FdCF0PKGUHUhL/yoDjz\n\tYaCa6Q8Xdn6ejbavBYxljHrjjlgYdldYpprv7ozd2HaWahBbaHZK0yLxXKHySamDDECV\n\tPPM//D+oqzqC3A3XZ+keBnmkkB+dH8bPEqbkVq8bc6uOmNNiPnmls7Y525ZuWTUSONv3\n\tyWpw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=y8W5XBBTch8jWdSvcQDz7YmfGUrPOK7CluJ1/GPd2jk=;\n\tb=DvkLdfJy1NioTLqW2D9nfS/CVSnjhf/qSfRg1d+cb72uL/KZ59FLs0+LhtFr7uK5SM\n\tlyB366sVi3nyn7Bvlx3llEebZS9tbAswCs2m4glFvgx6hrxwq+GQe0dd+0pjQEGGhPse\n\t3c7bK8DBEN8ftvVYOV+BRfgSrVJ5DDJNL5xfRLdoDedjer0QE3vnkjg75aQIl2rpkMVh\n\t9o1B38ymCLRi0XjiKNIE6qEX7UjDVLpkZZ4a+Hgmx5UUfJkqW8Ox2GOZff8DT4wqSQt2\n\tAV5lZlm79uO+nrfNYuiYIRpQQo/BF5asQs/dpPOhahVT14gjpaVLPZEkX8PVAJfg1xY8\n\tHXtg==","X-Gm-Message-State":"AOAM533ZFY8rsJcgxo65u4vfZGv8oZWgrwTPPQSThPqW6ZxwbOlHFFdR\n\tNU3/NlHlDGqd5AdaEccG8+oqYM6AIT7DyFp4yXh3dg==","X-Google-Smtp-Source":"ABdhPJy5QdAppDwWIkiJ6KyB8Aisof9VSWFLDM1f32szSg1CmjmXODeJ1V8+WDfXnMsXYR7EKUXiEBkVljmcq8YGNRY=","X-Received":"by 2002:a05:6512:b20:: with SMTP id\n\tw32mr16719230lfu.315.1631698024443; \n\tWed, 15 Sep 2021 02:27:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-2-paul.elder@ideasonboard.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 15 Sep 2021 10:26:48 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000020cb9c05cc054dd2\"","Subject":"Re: [libcamera-devel] [PATCH 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]