[{"id":26368,"web_url":"https://patchwork.libcamera.org/comment/26368/","msgid":"<167508367752.42371.1437722328871301628@Monstersaurus>","date":"2023-01-30T13:01:17","subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Handle\n\tAEC/AGC flicker controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman via libcamera-devel (2023-01-26 13:45:59)\n> We handle the flicker modes by passing the correct period to the\n> AEC/AGC algorithm which already contains the necessary code. The\n> \"Auto\" mode is currently unsupported.\n> \n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 80 +++++++++++++++++++++++++++++\n> 1 file changed, 80 insertions(+)\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 5f7397e2..477d72ba 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -86,6 +86,8 @@ static const ControlInfoMap::Map ipaControls{\n> { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },\n> { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },\n> { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },\n> + { &controls::AeFlickerMode, ControlInfo(controls::AeFlickerModeValues) },\n> + { &controls::AeFlickerPeriod, ControlInfo(0, 1000000) },\n> { &controls::AwbEnable, ControlInfo(false, true) },\n> { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> @@ -218,6 +220,12 @@ private:\n> \n> /* Maximum gain code for the sensor. */\n> uint32_t maxSensorGainCode_;\n> +\n> + /* The current state of flicker avoidance. */\n> + struct FlickerState {\n> + int32_t mode;\n> + Duration period;\n> + } flickerState_;\n> };\n> \n> int IPARPi::init(const IPASettings &settings, bool lensPresent, IPAInitResult *result)\n> @@ -946,6 +954,78 @@ void IPARPi::queueRequest(const ControlList &controls)\n> break;\n> }\n> \n> + case controls::AE_FLICKER_MODE: {\n> + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> + controller_.getAlgorithm(\"agc\"));\n> + if (!agc) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set AE_FLICKER_MODE - no AGC algorithm\";\n\nHrm. I believe we use AE_FLICKER_MODE because we can't switch on\n&controls::AeFlickerMode (it's difficult to get a constexpr of the id)\n\nBut users/developers who read this warning won't see\nAE_FLICKER_MODE, they would be using the controls::AeFlickerMode name,\nso it might be better to use that?\n\nOtherwise, seems good to me.\n\nReviewed-by: Kieran Bingham \n\n\n\n> + break;\n> + }\n> +\n> + int32_t mode = ctrl.second.get();\n> + bool modeValid = true;\n> +\n> + switch (mode) {\n> + case controls::FlickerOff: {\n> + agc->setFlickerPeriod(0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerFreq50Hz: {\n> + agc->setFlickerPeriod(10000 * 1.0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerFreq60Hz: {\n> + agc->setFlickerPeriod(8333.333 * 1.0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerCustom: {\n> + agc->setFlickerPeriod(flickerState_.period);\n> +\n> + break;\n> + }\n> +\n> + default:\n> + LOG(IPARPI, Error) << \"Flicker mode \" << mode << \" is not supported\";\n> + modeValid = false;\n> +\n> + break;\n> + }\n> +\n> + if (modeValid)\n> + flickerState_.mode = mode;\n> +\n> + break;\n> + }\n> +\n> + case controls::AE_FLICKER_PERIOD: {\n> + RPiController::AgcAlgorithm *agc = dynamic_cast(\n> + controller_.getAlgorithm(\"agc\"));\n> + if (!agc) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set AE_FLICKER_PERIOD - no AGC algorithm\";\n> + break;\n> + }\n> +\n> + uint32_t period = ctrl.second.get();\n> + flickerState_.period = period * 1.0us;\n> +\n> + /*\n> + * We note that it makes no difference if the mode gets set to \"custom\"\n> + * first, and the period updated after, or vice versa.\n> + */\n> + if (flickerState_.mode == controls::FlickerCustom)\n> + agc->setFlickerPeriod(flickerState_.period);\n> +\n> + break;\n> + }\n> +\n> case controls::AWB_ENABLE: {\n> RPiController::AwbAlgorithm *awb = dynamic_cast(\n> controller_.getAlgorithm(\"awb\"));\n> -- \n> 2.30.2\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 CD251BEFBE\n\tfor ;\n\tMon, 30 Jan 2023 13:01:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 390EF625D8;\n\tMon, 30 Jan 2023 14:01:22 +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 7179760482\n\tfor ;\n\tMon, 30 Jan 2023 14:01:20 +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 E22888B8;\n\tMon, 30 Jan 2023 14:01:19 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675083682;\n\tbh=iJuwOL2e8aSOc8RtfbFXaleGb/iasTJRdVZaUaSb+7o=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=2BuO/+5WorGh6bX4ckFwZvyxxmWEkdFG+Un2wx22xcbvsVQKNmpLV+5CXFm+KySsT\n\tAW+at9O9vUbrfJTkgAb2DTvKgjA0GZ7q70e0LgIBIBtqRCewcEncgTbs1uQW7G0CNu\n\t+5OOtXEHeB4xfwa/YOBPv/8ezoiSSjJuSlRc/rVtLZl32eqUiH4TcXWEEMMDOZutwA\n\tLMWeyeVdR4X+dx/vdCMlejSEhxLx3WIj/E1e096ddVcpnrPvHtkc9LxbNpyyWU40p/\n\tDROw9ySJOTYVXeYS1s+nnzQKWrciTPwY1JUJPi+kFidBEl4KLtVapkeNq7o9ycQ168\n\ta/s9iaWMUlhGA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675083680;\n\tbh=iJuwOL2e8aSOc8RtfbFXaleGb/iasTJRdVZaUaSb+7o=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=piA/LULA4ozlROXIw2VcZHgZWHSDv7dvzIx1N0kqrdYUhZw2s/HLQIUTo7JcQpEHv\n\t7DTBChZY6t/NL7spAxTIqu1PNEGapLOIR1HXZmKvjY81IHKLPP/ZFwiVdDyWeIsh4h\n\tag0vRaO3fRVcrXJDNJ5IFsw1UW6lg7SLSjJam0VU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"piA/LULA\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230126134559.3323-3-david.plowman@raspberrypi.com>","References":"<20230126134559.3323-1-david.plowman@raspberrypi.com>\n\t<20230126134559.3323-3-david.plowman@raspberrypi.com>","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 30 Jan 2023 13:01:17 +0000","Message-ID":"<167508367752.42371.1437722328871301628@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Handle\n\tAEC/AGC flicker controls","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26652,"web_url":"https://patchwork.libcamera.org/comment/26652/","msgid":"","date":"2023-03-13T09:06:25","subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Handle\n\tAEC/AGC flicker controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThanks for the work!\n\nOn Thu, 26 Jan 2023 at 13:46, David Plowman via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> We handle the flicker modes by passing the correct period to the\n> AEC/AGC algorithm which already contains the necessary code. The\n> \"Auto\" mode is currently unsupported.\n>\n> Signed-off-by: David Plowman \n>\n\nReviewed-by: Naushir Patuck \n\n\n\n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 80 +++++++++++++++++++++++++++++\n> 1 file changed, 80 insertions(+)\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 5f7397e2..477d72ba 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -86,6 +86,8 @@ static const ControlInfoMap::Map ipaControls{\n> { &controls::AeConstraintMode,\n> ControlInfo(controls::AeConstraintModeValues) },\n> { &controls::AeExposureMode,\n> ControlInfo(controls::AeExposureModeValues) },\n> { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },\n> + { &controls::AeFlickerMode,\n> ControlInfo(controls::AeFlickerModeValues) },\n> + { &controls::AeFlickerPeriod, ControlInfo(0, 1000000) },\n> { &controls::AwbEnable, ControlInfo(false, true) },\n> { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> @@ -218,6 +220,12 @@ private:\n>\n> /* Maximum gain code for the sensor. */\n> uint32_t maxSensorGainCode_;\n> +\n> + /* The current state of flicker avoidance. */\n> + struct FlickerState {\n> + int32_t mode;\n> + Duration period;\n> + } flickerState_;\n> };\n>\n> int IPARPi::init(const IPASettings &settings, bool lensPresent,\n> IPAInitResult *result)\n> @@ -946,6 +954,78 @@ void IPARPi::queueRequest(const ControlList &controls)\n> break;\n> }\n>\n> + case controls::AE_FLICKER_MODE: {\n> + RPiController::AgcAlgorithm *agc =\n> dynamic_cast(\n> + controller_.getAlgorithm(\"agc\"));\n> + if (!agc) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set AE_FLICKER_MODE\n> - no AGC algorithm\";\n> + break;\n> + }\n> +\n> + int32_t mode = ctrl.second.get();\n> + bool modeValid = true;\n> +\n> + switch (mode) {\n> + case controls::FlickerOff: {\n> + agc->setFlickerPeriod(0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerFreq50Hz: {\n> + agc->setFlickerPeriod(10000 * 1.0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerFreq60Hz: {\n> + agc->setFlickerPeriod(8333.333 * 1.0us);\n> +\n> + break;\n> + }\n> +\n> + case controls::FlickerCustom: {\n> +\n> agc->setFlickerPeriod(flickerState_.period);\n> +\n> + break;\n> + }\n> +\n> + default:\n> + LOG(IPARPI, Error) << \"Flicker mode \" <<\n> mode << \" is not supported\";\n> + modeValid = false;\n> +\n> + break;\n> + }\n> +\n> + if (modeValid)\n> + flickerState_.mode = mode;\n> +\n> + break;\n> + }\n> +\n> + case controls::AE_FLICKER_PERIOD: {\n> + RPiController::AgcAlgorithm *agc =\n> dynamic_cast(\n> + controller_.getAlgorithm(\"agc\"));\n> + if (!agc) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set\n> AE_FLICKER_PERIOD - no AGC algorithm\";\n> + break;\n> + }\n> +\n> + uint32_t period = ctrl.second.get();\n> + flickerState_.period = period * 1.0us;\n> +\n> + /*\n> + * We note that it makes no difference if the mode\n> gets set to \"custom\"\n> + * first, and the period updated after, or vice\n> versa.\n> + */\n> + if (flickerState_.mode == controls::FlickerCustom)\n> +\n> agc->setFlickerPeriod(flickerState_.period);\n> +\n> + break;\n> + }\n> +\n> case controls::AWB_ENABLE: {\n> RPiController::AwbAlgorithm *awb =\n> dynamic_cast(\n> controller_.getAlgorithm(\"awb\"));\n> --\n> 2.30.2\n>\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 E257ABD80A\n\tfor ;\n\tMon, 13 Mar 2023 09:06:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5539662707;\n\tMon, 13 Mar 2023 10:06:16 +0100 (CET)","from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com\n\t[IPv6:2607:f8b0:4864:20::b32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF89262706\n\tfor ;\n\tMon, 13 Mar 2023 10:06:14 +0100 (CET)","by mail-yb1-xb32.google.com with SMTP id y144so10936304yby.12\n\tfor ;\n\tMon, 13 Mar 2023 02:06:14 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678698376;\n\tbh=F3UvZ1X11XeJLUmnaAEFPh0ndN4wqVHteVEPPg0PcOk=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=k/RS3hGGsbj9bnh0lfBwC2RrMw2JimkyxiQmlce5ciqryHgPJwDRrDsWCJeZWw3Up\n\tAKkA7gy9YqHUSSxtaJwMd8vRGtrd1kIwq0L7qa6M83nc4YQwbE/1fDLzKDsIqdTT2j\n\t34PLktUFdY6VuuO7U8LjeM6IttQYz6b1meV8inw0OlwLZGTcQW9ck+LjCXlPPFPv5b\n\tgbwBdaVrG/7vz6+YTf1IPVUcEWCl36qfZKY3xgyl5JDurQeXSf1cTZY1EqAKduhHOu\n\tqxrCWVLcQsFuS9zPPQ9pC0H8bXe1k0CSyC94D23yO4+MihbdMf3QLTmDN0zr7zA0hf\n\tWW2ffJkrhyFjw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1678698373;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=t1AvSCPrNbEA3DhpWmvy/BUjh4OFFmaTsx8U8+dfI9s=;\n\tb=K/4ULZznfpza9B1bTelFteG69qhpDNgJ/7T/kLTMEAN+OY617/MaA+CrmkD8zOsJxe\n\tEjRH2QEV3NHRJmHE2zggu6D19ezB8cqgPX94ljJejrLBVZcnIxYjDnQVeVWugK0f4M3Z\n\tN5+8lHE21Oog3Fgt8JKllPUVV+oceCrrRpJV2j9g2C52uYidRwH9KLfg19AHkwKcxTYE\n\tMsOnqmMg2AitpeEaE71xEM3CnuiA4N9a9U9mjPTUxg/v5bBqgp2h0HmxfYwd0VQQXbx4\n\t2oPFGymIbSlN4JnqizCiI8iYCEUIWZZPJ2uZMADdVYnXSNuwmMUsHFDvvuDuJPo+OBlN\n\tlJ7w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"K/4ULZzn\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1678698373;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=t1AvSCPrNbEA3DhpWmvy/BUjh4OFFmaTsx8U8+dfI9s=;\n\tb=tjQ38URsnJyjUc3iBZHf2XxOfAW1H+jWswxz18T3ee5ctBRGIaqohpr46A+Msy3W4z\n\taJ3WClcO4NW2PegW2TRpVq88owC52i3I4m12Oi+NR0QOXrLHdO8IEwaCOLsHr+H5A8rs\n\tN01I9VD/ojfCY5fahFwRlG/iDiTD7/CVIjS5OKHVp6hV3NN9VuBXK1Av+iiQUOQzxV9U\n\tD3N4/JVZB8ekbSNdZOqmLnfU2stulRim3dv5Wxk2BAfYEaG52x7wBBSOLRoncnZDntOv\n\tJwSSVDMsBRfJ8gx5uC8j5hHG6Bcuk6eaH/f8my5rbHhTbvEPIrIan0Bo3NmHFc+s/lxY\n\t8Kcg==","X-Gm-Message-State":"AO0yUKUKatG0xZIPm87A7pOwn5CiazzfVarhPgJUPL+pLOum1SM9q4wN\n\thkTY0JON6/1tESmbQsS9IFd5GK1IaKNM8OVX8Td9PApeJ1QNehz5ZEA=","X-Google-Smtp-Source":"AK7set/r1t3ybPWy4aCKfRkime9QKNC4/CNyGTjzk/18aCs8TsnXqIlhN99wwzNVY0sB1J2PIa/dQpvHxKi4vVJDJ5o=","X-Received":"by 2002:a25:9c43:0:b0:94a:ebba:cba6 with SMTP id\n\tx3-20020a259c43000000b0094aebbacba6mr16147354ybo.9.1678698373716;\n\tMon, 13 Mar 2023 02:06:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20230126134559.3323-1-david.plowman@raspberrypi.com>\n\t<20230126134559.3323-3-david.plowman@raspberrypi.com>","In-Reply-To":"<20230126134559.3323-3-david.plowman@raspberrypi.com>","Date":"Mon, 13 Mar 2023 09:06:25 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"00000000000040339405f6c46c85\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Handle\n\tAEC/AGC flicker controls","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]