[{"id":21931,"web_url":"https://patchwork.libcamera.org/comment/21931/","msgid":"<164125209751.682966.14015260273761984617@Monstersaurus>","date":"2022-01-03T23:21:37","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2021-12-21 05:10:22)\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> ---\n> No change in v2:\n> - constexpr did not work\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 548bfba0..593139c5 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> + static_cast(controls::NoiseReductionModeOff),\n> + static_cast(controls::NoiseReductionModeFast),\n> + static_cast(controls::NoiseReductionModeHighQuality),\n> +};\n> +\n> /*\n> * List of controls handled by the Raspberry Pi IPA\n> *\n> @@ -46,7 +52,7 @@ static const ControlInfoMap Controls({\n> { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> - { &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> + { &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n\nAha, I didn't realise we could liimit the exposed values from controls\nenums like this.\n\nI'll have to keep an eye on how things check to ensure the control value\nis supported, but it makes sense that it should only report the modes it\nsupports.\n\nReviewed-by: Kieran Bingham \n\n> }, controls::controls);\n> \n> } /* namespace RPi */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 3f497be1..e0685c69 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -618,8 +618,6 @@ static const std::map DenoiseModeTable = {\n> { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> { controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> { controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> - { controls::NoiseReductionModeRaw, RPiController::DenoiseMode::ColourOff },\n> - { controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> };\n> \n> void IPARPi::queueRequest(const ControlList &controls)\n> -- \n> 2.27.0\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 6218FBE080\n\tfor ;\n\tMon, 3 Jan 2022 23:21:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2DDB60868;\n\tTue, 4 Jan 2022 00:21:42 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9BC60604F8\n\tfor ;\n\tTue, 4 Jan 2022 00:21:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 14D3F501;\n\tTue, 4 Jan 2022 00:21:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"duBSP70c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641252100;\n\tbh=ZmYrNFdEtlzTmS6sd9cH7F6lXrZxv+wviK/BQSviRNg=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=duBSP70cPZSw5cPpKw8MBfqMMHTbJ8K8hchAYJ4Zl2cx8/97Vod7CFvT4yqJoDt+k\n\tcshGW2OSltlCR09ZH5uRGI2iCZ21BxGpLqrr2Jgdtax1PJILZEsjljqyEOm89rzmhr\n\t6dEi0DDHjU3yVFNoruQ5LSRpYUwBBaeUn3JujgdU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211221051023.2628625-3-paul.elder@ideasonboard.com>","References":"<20211221051023.2628625-1-paul.elder@ideasonboard.com>\n\t<20211221051023.2628625-3-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 03 Jan 2022 23:21:37 +0000","Message-ID":"<164125209751.682966.14015260273761984617@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21936,"web_url":"https://patchwork.libcamera.org/comment/21936/","msgid":"","date":"2022-01-04T08:27:49","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Paul, Kieran\n\nThank you for this patch!\n\nOn Mon, 3 Jan 2022 at 23:21, Kieran Bingham\n wrote:\n>\n> Quoting Paul Elder (2021-12-21 05:10:22)\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> > ---\n> > No change in v2:\n> > - constexpr did not work\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 548bfba0..593139c5 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> > + static_cast(controls::NoiseReductionModeOff),\n> > + static_cast(controls::NoiseReductionModeFast),\n> > + static_cast(controls::NoiseReductionModeHighQuality),\n> > +};\n\nJust one thing, didn't we support \"NoiseReductionModeMinimal\" and\n\"NoiseReductionMode ZSL\" too? (Though the former is more important to\nus.) So I'm thinking those need to be in the list here as well.\n\nBest regards\nDavid\n\n> > +\n> > /*\n> > * List of controls handled by the Raspberry Pi IPA\n> > *\n> > @@ -46,7 +52,7 @@ static const ControlInfoMap Controls({\n> > { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > - { &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> > + { &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n>\n> Aha, I didn't realise we could liimit the exposed values from controls\n> enums like this.\n>\n> I'll have to keep an eye on how things check to ensure the control value\n> is supported, but it makes sense that it should only report the modes it\n> supports.\n>\n> Reviewed-by: Kieran Bingham \n>\n> > }, controls::controls);\n> >\n> > } /* namespace RPi */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 3f497be1..e0685c69 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -618,8 +618,6 @@ static const std::map DenoiseModeTable = {\n> > { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> > { controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> > { controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> > - { controls::NoiseReductionModeRaw, RPiController::DenoiseMode::ColourOff },\n> > - { controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> > };\n> >\n> > void IPARPi::queueRequest(const ControlList &controls)\n> > --\n> > 2.27.0\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 CA4A8BE080\n\tfor ;\n\tTue, 4 Jan 2022 08:28:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 120396091C;\n\tTue, 4 Jan 2022 09:28:03 +0100 (CET)","from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com\n\t[IPv6:2a00:1450:4864:20::32c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE8BE604F2\n\tfor ;\n\tTue, 4 Jan 2022 09:28:00 +0100 (CET)","by mail-wm1-x32c.google.com with SMTP id\n\tb186-20020a1c1bc3000000b00345734afe78so1031298wmb.0\n\tfor ;\n\tTue, 04 Jan 2022 00:28:00 -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=\"cRupCcQk\"; 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=9p230qkvyop1ttsCZkdqhCoocQhzHHbmgcZko0n8sA8=;\n\tb=cRupCcQkgbD5WVPfkV5VJ1WxMO1PBUNpK1e+/wH4JPAuQLD88tMvehmMuROBiZRMIK\n\tqDp93v+WiDVUAa/Uws199fHuth5f8k7Le5YdyeFpRH42cJIN2dmOle2wC3Etixp45MJN\n\t/H9WtOWtm2FP8mfLFtlOf+NTgAti+W/GUqH/f9zvwEj+qF3vgZWnRF6gbUV1xFidJ6z9\n\t7hHOyN/lmcCmJnX7ucCxasOp/g3xyvcZEQLjyIiZDlDvv/cnEFg+jKIwbG4sxuqTpP3P\n\tXHwAfICx1956VgoJlw5QRa0R2Kkt0l94g8rJ0MzOEYZ6066OYw4+S3h39y5BN2f9DmdQ\n\taX6A==","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=9p230qkvyop1ttsCZkdqhCoocQhzHHbmgcZko0n8sA8=;\n\tb=TozGSt3zskieaGBN84X81n//xH0wmTryLrJ1pNRm9EY1SNbNX8/4rpLSoEb/XLtHiU\n\tR8Y95NdYhhrpT9bpWqQBHJALal+UDBvrsgQZZTjJDQfWHLN69jymq/WANKtLf/TUBAZ+\n\tIQvj4Mk5uVtvMlOo8crKAEz94bTeAa/eGjxJ6dmcgyc/bJeySdX4B6bXk7oWb51EnXxN\n\tYqoefwfJwZ8gFl094Wng/RyOF8UfWWnozkC5SKoe7UcIzFgrMk37ENyqOPLNdATLhbXz\n\t+r1RL2pR5z+l5xNqPYwnQK+Wiy6KinacdK+pL6uP6e0NhpmDvLgCJcHeoY8SlRIy3bhj\n\t54Qw==","X-Gm-Message-State":"AOAM530/PnxWJdyLkRlqDCro2yDoPiUcl0Hh6uD4tKWibCLVA1EGIUEn\n\tTxvbLX3YGsEX0IGogUciLo7aUKve+e7sc4E/UE/KWA==","X-Google-Smtp-Source":"ABdhPJxNkGzMnlwddtAG8Ufr27fQUx6dt/sgdj5fau86LVZL8Nwi1YzNDKS0inzkhfSO8QUZFVLWWgnkSLDTfx0FPlM=","X-Received":"by 2002:a05:600c:4fcc:: with SMTP id\n\to12mr40574071wmq.184.1641284880168; \n\tTue, 04 Jan 2022 00:28:00 -0800 (PST)","MIME-Version":"1.0","References":"<20211221051023.2628625-1-paul.elder@ideasonboard.com>\n\t<20211221051023.2628625-3-paul.elder@ideasonboard.com>\n\t<164125209751.682966.14015260273761984617@Monstersaurus>","In-Reply-To":"<164125209751.682966.14015260273761984617@Monstersaurus>","From":"David Plowman ","Date":"Tue, 4 Jan 2022 08:27:49 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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":21937,"web_url":"https://patchwork.libcamera.org/comment/21937/","msgid":"<164129238119.3986460.12425769236882556263@Monstersaurus>","date":"2022-01-04T10:33:01","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2022-01-04 08:27:49)\n> Hi Paul, Kieran\n> \n> Thank you for this patch!\n> \n> On Mon, 3 Jan 2022 at 23:21, Kieran Bingham\n> wrote:\n> >\n> > Quoting Paul Elder (2021-12-21 05:10:22)\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> > > ---\n> > > No change in v2:\n> > > - constexpr did not work\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 548bfba0..593139c5 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> > > + static_cast(controls::NoiseReductionModeOff),\n> > > + static_cast(controls::NoiseReductionModeFast),\n> > > + static_cast(controls::NoiseReductionModeHighQuality),\n> > > +};\n> \n> Just one thing, didn't we support \"NoiseReductionModeMinimal\" and\n\nIs 'Minimal' different to 'Fast' somehow?\n\n> \"NoiseReductionMode ZSL\" too? (Though the former is more important to\n> us.) So I'm thinking those need to be in the list here as well.\n\nIn this series, ZSL is defined as being used when reprocessing, so it\nwill only apply NoiseReduction on low resolution streams (view finders)\nassuming that a RAW stream will also be provided which will be de-noised\non only later processed frames.\n\nAs I understand it, this patch is directly removing the listing that ZSL\nis available, and listing direct matches of what is exposed by the RPi\nIPA modes?\n\n\n> \n> Best regards\n> David\n> \n> > > +\n> > > /*\n> > > * List of controls handled by the Raspberry Pi IPA\n> > > *\n> > > @@ -46,7 +52,7 @@ static const ControlInfoMap Controls({\n> > > { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> > > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > > - { &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> > > + { &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n> >\n> > Aha, I didn't realise we could liimit the exposed values from controls\n> > enums like this.\n> >\n> > I'll have to keep an eye on how things check to ensure the control value\n> > is supported, but it makes sense that it should only report the modes it\n> > supports.\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > > }, controls::controls);\n> > >\n> > > } /* namespace RPi */\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index 3f497be1..e0685c69 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -618,8 +618,6 @@ static const std::map DenoiseModeTable = {\n> > > { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> > > { controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> > > { controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> > > - { controls::NoiseReductionModeRaw, RPiController::DenoiseMode::ColourOff },\n> > > - { controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> > > };\n> > >\n> > > void IPARPi::queueRequest(const ControlList &controls)\n> > > --\n> > > 2.27.0\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 934E3BF415\n\tfor ;\n\tTue, 4 Jan 2022 10:33:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AABD760927;\n\tTue, 4 Jan 2022 11:33:04 +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 A5CBD60219\n\tfor ;\n\tTue, 4 Jan 2022 11:33:03 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5AA7C501;\n\tTue, 4 Jan 2022 11:33:03 +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=\"CEZNGleS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641292383;\n\tbh=IoNTfYa7HSnwgQhDO6pwaCODH14/4tg2VJOkdBsoRiY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=CEZNGleS/9lK+ZenN+uvsLVZa4hC6qOhXcJSw+R5vOlb4dfC516W5DTUi/bzCmrQi\n\t2LuWraYZiiB319/+x+W+VN2XBfjDbe24Pd6JybrKHB3PH2iGyxGjf9x9Ksz9IY+aHU\n\toitAzly1F+A+b1m+q4LE5VQwuBIUBUmjAMTpbdDI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211221051023.2628625-1-paul.elder@ideasonboard.com>\n\t<20211221051023.2628625-3-paul.elder@ideasonboard.com>\n\t<164125209751.682966.14015260273761984617@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"David Plowman ","Date":"Tue, 04 Jan 2022 10:33:01 +0000","Message-ID":"<164129238119.3986460.12425769236882556263@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 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":21943,"web_url":"https://patchwork.libcamera.org/comment/21943/","msgid":"","date":"2022-01-04T13:25:09","subject":"Re: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Clean up\n\tNoiseReductionMode values","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"HI Kieran\n\nThanks for the reply.\n\nOn Tue, 4 Jan 2022 at 10:33, Kieran Bingham\n wrote:\n>\n> Quoting David Plowman (2022-01-04 08:27:49)\n> > Hi Paul, Kieran\n> >\n> > Thank you for this patch!\n> >\n> > On Mon, 3 Jan 2022 at 23:21, Kieran Bingham\n> > wrote:\n> > >\n> > > Quoting Paul Elder (2021-12-21 05:10:22)\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> > > > ---\n> > > > No change in v2:\n> > > > - constexpr did not work\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 548bfba0..593139c5 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> > > > + static_cast(controls::NoiseReductionModeOff),\n> > > > + static_cast(controls::NoiseReductionModeFast),\n> > > > + static_cast(controls::NoiseReductionModeHighQuality),\n> > > > +};\n> >\n> > Just one thing, didn't we support \"NoiseReductionModeMinimal\" and\n>\n> Is 'Minimal' different to 'Fast' somehow?\n\nYes, in Raspberry PI world \"fast\" means to use the \"fast software\ncolour denoise algorithm\" whereas \"minimal\" means \"no software colour\ndenoise at all\". (There's also a \"high quality\" but slower version of\nsoftware colour denoise.)\n\nI think we need to keep \"minimal\" as an option, but as you say, ZSL\ndoesn't bother us for the moment.\n\nThanks\nDavid\n\n>\n> > \"NoiseReductionMode ZSL\" too? (Though the former is more important to\n> > us.) So I'm thinking those need to be in the list here as well.\n>\n> In this series, ZSL is defined as being used when reprocessing, so it\n> will only apply NoiseReduction on low resolution streams (view finders)\n> assuming that a RAW stream will also be provided which will be de-noised\n> on only later processed frames.\n>\n> As I understand it, this patch is directly removing the listing that ZSL\n> is available, and listing direct matches of what is exposed by the RPi\n> IPA modes?\n>\n>\n> >\n> > Best regards\n> > David\n> >\n> > > > +\n> > > > /*\n> > > > * List of controls handled by the Raspberry Pi IPA\n> > > > *\n> > > > @@ -46,7 +52,7 @@ static const ControlInfoMap Controls({\n> > > > { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> > > > { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },\n> > > > - { &controls::NoiseReductionMode, ControlInfo(controls::NoiseReductionModeValues) }\n> > > > + { &controls::NoiseReductionMode, ControlInfo(RPiNoiseReductionModeValues) }\n> > >\n> > > Aha, I didn't realise we could liimit the exposed values from controls\n> > > enums like this.\n> > >\n> > > I'll have to keep an eye on how things check to ensure the control value\n> > > is supported, but it makes sense that it should only report the modes it\n> > > supports.\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > > > }, controls::controls);\n> > > >\n> > > > } /* namespace RPi */\n> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > index 3f497be1..e0685c69 100644\n> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > @@ -618,8 +618,6 @@ static const std::map DenoiseModeTable = {\n> > > > { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off },\n> > > > { controls::NoiseReductionModeFast, RPiController::DenoiseMode::ColourFast },\n> > > > { controls::NoiseReductionModeHighQuality, RPiController::DenoiseMode::ColourHighQuality },\n> > > > - { controls::NoiseReductionModeRaw, RPiController::DenoiseMode::ColourOff },\n> > > > - { controls::NoiseReductionModeZSL, RPiController::DenoiseMode::ColourHighQuality },\n> > > > };\n> > > >\n> > > > void IPARPi::queueRequest(const ControlList &controls)\n> > > > --\n> > > > 2.27.0\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 4E68BBF415\n\tfor ;\n\tTue, 4 Jan 2022 13:25:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E3A760921;\n\tTue, 4 Jan 2022 14:25:22 +0100 (CET)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8E82604F4\n\tfor ;\n\tTue, 4 Jan 2022 14:25:20 +0100 (CET)","by mail-wm1-x329.google.com with SMTP id o30so20712025wms.4\n\tfor ;\n\tTue, 04 Jan 2022 05:25:20 -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=\"B+f1Mpi8\"; 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=JCAHkEK8VKMoBisgFyxaZjibeRHOt0LoA/SPDHgEvns=;\n\tb=B+f1Mpi8DdiKdTO7a14ubF35wctraJaABE8lAfEI4FIwNFaLPNlOWCCX3KibgZDd0a\n\taPAo6ejA65HoSwURfr/ibAVRXXRSgMYkLtSkvAZ/hBWZKYIibSjYyouB3A117pXOdg8b\n\tLDEO6fK2whs0huVo6dcSa4gy3nn2n+tEIeY63IYJ7gWlLx77XPesOXDPQY5505T8+WyM\n\tXHFDuTqrm6yhnMhyV9LPGWjTYKgRmFRXlJa9+L7aprnYEH/GD9EIOKj5aHdIEpr5Hr7z\n\tlxn2PLdnFwMUGrfC7bXgXAWSj3Zs1aQTAmU+iphBU0CL/51UwLTZB3FhXgLNe8jC//LU\n\tBWjw==","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=JCAHkEK8VKMoBisgFyxaZjibeRHOt0LoA/SPDHgEvns=;\n\tb=M1nwA2gT5tdq2GiHmhLo/mjSQv+wQGkUfHaw/R0gl+1XjdtIpK1tH6C8xGOrb1uBxP\n\tjPVQCuEqJjivpRECHtA4XZ9Qm9nb4uyz9pcK4JqwV9nq4qMKPQv0Jl3fNgfOv8VHLWk2\n\tpd7IEgnjl7EoGDL0zLUUk4ZUQPAPZMztjzJiCiClvAwGus5GEA9yatgVjJvKJix3kmhC\n\tYx7Fo2LXE1sJrlxI1A3O+zicKIQHewvThrlJkMFDxxbBSMGB5mrCLfI23S+E+rAWEuNm\n\tn2klFn1pZCyRsrslhvafzuoR5TTN2BobgHFzTj0Ea2scAgiugjS6EasAG/fVBbDBdzLl\n\tanlQ==","X-Gm-Message-State":"AOAM532lRfY8JUvgJxZsSxr+sYtSbZemuXbmaHjtT/UKUeCVcSv9+8Ux\n\tGmiqbxU4aGODUR91IHa/JIq95DX/o2Yd81W80PJoPikXqT4=","X-Google-Smtp-Source":"ABdhPJxBRLEyB7i/enwBLa7jgkpIie7Yv9c3CQtrVAcrbEfNXTG8u8SzMruRTTQEkmTfD2NljdvSq8M/aZbNbaCI+88=","X-Received":"by 2002:a05:600c:3b0e:: with SMTP id\n\tm14mr41677449wms.130.1641302720248; \n\tTue, 04 Jan 2022 05:25:20 -0800 (PST)","MIME-Version":"1.0","References":"<20211221051023.2628625-1-paul.elder@ideasonboard.com>\n\t<20211221051023.2628625-3-paul.elder@ideasonboard.com>\n\t<164125209751.682966.14015260273761984617@Monstersaurus>\n\t\n\t<164129238119.3986460.12425769236882556263@Monstersaurus>","In-Reply-To":"<164129238119.3986460.12425769236882556263@Monstersaurus>","From":"David Plowman ","Date":"Tue, 4 Jan 2022 13:25:09 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 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\" "}}]