[{"id":20844,"web_url":"https://patchwork.libcamera.org/comment/20844/","msgid":"","date":"2021-11-11T08:16:11","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: Fix -nan exception","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kate,\n\nThanks for the patch !\n\nOn 11/11/2021 09:10, Kate Hsuan wrote:\n> The user may equip a cam cover to make sure security.If they want to\n> disable the cam, the cam cover could be physically closed to ensure\n> the image will not be delivered. In this situation, the image will be\n> all black and it triggers the -nan exception for some values.\n> Eventually, the exposure will be locked. As a result, we only can get\n> a black image.\n> \n> evGain is calculated by kEvGainTarget * knumHistogramBins / iqMean_.\n> For a black image, a -nan of iqMean_ will lead evGain miss calculated.\n> Consequently, the exposure value can not be correctly estimated and be\n> locked at -nan as well. In this situation, evGain is set to 1.0 and try\n> to estimate again through the next frame. Also, an additional check\n> makes sure the exposure value is between minTotalExposure and\n> maxTotalExposure.\n> \n> Signed-off-by: Kate Hsuan \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index b5d736c1..d965febe 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -191,6 +191,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \n> \t/* Estimate the gain needed to have the proportion wanted */\n> \tdouble evGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> +\tif(std::isnan(evGain))\n> +\t\tevGain = 1.0;\n\nThis has been found on our side too :-) but we solved it in a different \nway, improving the AGC behaviour. Maybe would you like to have a look to \nthe \"[PATCH v2 00/14] IPA: IPU3: Introduce per-frame controls\" series ?\n\nTo be honest, I hesitated between testing nan, patching the Histogram \nclass and the third option you can find in \"ipa: ipu3: agc: Limit the \nnumber of saturated cells\".\n\n> \n> \t/* extracted from Rpi::Agc::computeTargetExposure */\n> \t/* Calculate the shutter time in seconds */\n> @@ -210,7 +212,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \n> \t/* Clamp the exposure value to the min and max authorized */\n> \tutils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;\n> -\tcurrentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> +\tutils::Duration minTotalExposure = minShutterSpeed * minAnalogueGain_;\n> +\n> +\tcurrentExposure_ = std::clamp(currentExposure_, minTotalExposure, maxTotalExposure);\n> \tLOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_\n> \t\t\t << \", maximum is \" << maxTotalExposure;\n> \n> @@ -232,7 +236,6 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \tLOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> \t\t\t << shutterTime << \" and \"\n> \t\t\t << stepGain;\n> -\n> \texposure = shutterTime / lineDuration_;\n> \tanalogueGain = stepGain;\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 3607CBDB1C\n\tfor ;\n\tThu, 11 Nov 2021 08:16:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A60D560345;\n\tThu, 11 Nov 2021 09:16:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20D516032C\n\tfor ;\n\tThu, 11 Nov 2021 09:16:14 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:e627:8337:a781:d98] (unknown\n\t[IPv6:2a01:e0a:169:7140:e627:8337:a781:d98])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C0622DEE;\n\tThu, 11 Nov 2021 09:16:13 +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=\"U0lK8oBl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636618573;\n\tbh=2GbwsQQewQd9PXi6jRDYhVM/unyet2dS9wh33JUTxsg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=U0lK8oBlEmJCye6tMKzXcAQWKfayY4MaQ9w/dwMKW3BfyHpNdwm+jYs58DaFSv10x\n\t4aRxIEUupXSDS66ZEVR/slgBtueXZRQVNfq9hc1vqHJNVvONZO4hNNOyJIFbxgvypk\n\thbBsj2eaeBk1iEFuQr84jyAFXNc+HKk7Rky6Ln5w=","Message-ID":"","Date":"Thu, 11 Nov 2021 09:16:11 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.2.1","Content-Language":"en-US","To":"Kate Hsuan ,\n\tlibcamera devel ","References":"<20211111081055.61127-1-hpa@redhat.com>","From":"Jean-Michel Hautbois ","In-Reply-To":"<20211111081055.61127-1-hpa@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: Fix -nan exception","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":20894,"web_url":"https://patchwork.libcamera.org/comment/20894/","msgid":"","date":"2021-11-12T07:17:58","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: Fix -nan exception","submitter":{"id":105,"url":"https://patchwork.libcamera.org/api/people/105/","name":"Kate Hsuan","email":"hpa@redhat.com"},"content":"Hi Jean-Michel,\n\nOn Thu, Nov 11, 2021 at 4:25 PM Jean-Michel Hautbois\n wrote:\n>\n> Hi Kate,\n>\n> Thanks for the patch !\n>\n> On 11/11/2021 09:10, Kate Hsuan wrote:\n> > The user may equip a cam cover to make sure security.If they want to\n> > disable the cam, the cam cover could be physically closed to ensure\n> > the image will not be delivered. In this situation, the image will be\n> > all black and it triggers the -nan exception for some values.\n> > Eventually, the exposure will be locked. As a result, we only can get\n> > a black image.\n> >\n> > evGain is calculated by kEvGainTarget * knumHistogramBins / iqMean_.\n> > For a black image, a -nan of iqMean_ will lead evGain miss calculated.\n> > Consequently, the exposure value can not be correctly estimated and be\n> > locked at -nan as well. In this situation, evGain is set to 1.0 and try\n> > to estimate again through the next frame. Also, an additional check\n> > makes sure the exposure value is between minTotalExposure and\n> > maxTotalExposure.\n> >\n> > Signed-off-by: Kate Hsuan \n> > ---\n> > src/ipa/ipu3/algorithms/agc.cpp | 7 +++++--\n> > 1 file changed, 5 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > index b5d736c1..d965febe 100644\n> > --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > @@ -191,6 +191,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> >\n> > /* Estimate the gain needed to have the proportion wanted */\n> > double evGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> > + if(std::isnan(evGain))\n> > + evGain = 1.0;\n>\n> This has been found on our side too :-) but we solved it in a different\n> way, improving the AGC behaviour. Maybe would you like to have a look to\n> the \"[PATCH v2 00/14] IPA: IPU3: Introduce per-frame controls\" series ?\n>\n> To be honest, I hesitated between testing nan, patching the Histogram\n> class and the third option you can find in \"ipa: ipu3: agc: Limit the\n> number of saturated cells\".\n\nThank you for noticing me this :)\n\nWe are working on the same issue.\n\n>\n> >\n> > /* extracted from Rpi::Agc::computeTargetExposure */\n> > /* Calculate the shutter time in seconds */\n> > @@ -210,7 +212,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> >\n> > /* Clamp the exposure value to the min and max authorized */\n> > utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;\n> > - currentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> > + utils::Duration minTotalExposure = minShutterSpeed * minAnalogueGain_;\n> > +\n> > + currentExposure_ = std::clamp(currentExposure_, minTotalExposure, maxTotalExposure);\n> > LOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_\n> > << \", maximum is \" << maxTotalExposure;\n> >\n> > @@ -232,7 +236,6 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> > LOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> > << shutterTime << \" and \"\n> > << stepGain;\n> > -\n> > exposure = shutterTime / lineDuration_;\n> > analogueGain = stepGain;\n> >\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 8A87EBDB1C\n\tfor ;\n\tFri, 12 Nov 2021 07:18:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6A7B6034D;\n\tFri, 12 Nov 2021 08:18:15 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C617B60232\n\tfor ;\n\tFri, 12 Nov 2021 08:18:14 +0100 (CET)","from mail-lf1-f69.google.com (mail-lf1-f69.google.com\n\t[209.85.167.69]) (Using TLS) by relay.mimecast.com with ESMTP id\n\tus-mta-365-ShEILgJaMSeond_rDADQxg-1; Fri, 12 Nov 2021 02:18:12 -0500","by mail-lf1-f69.google.com with SMTP id\n\tb23-20020a0565120b9700b00403a044bfcdso2305748lfv.13\n\tfor ;\n\tThu, 11 Nov 2021 23:18:11 -0800 (PST)"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"eIbOEyA1\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hpa@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1636701493;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=CThnGJC7iUVA2+9yCGJdrz8JjcdX8FwjiVcktiebKsU=;\n\tb=eIbOEyA1832ALnxz/70H5LPLMXN7XShfz64ZAwQ+JgcyB4mK21iWR9qnEOCJc2T4KeD9Wm\n\tmx5PExgUQYiJO8K8j2aBgbhUsZgqKXeF3nKxolwn7swNfw1iOY7f8yHfQfSFKfeECPJobl\n\tJqoHAp5AlFzvAbEPZQjyvqVt8QCi1kc=","X-MC-Unique":"ShEILgJaMSeond_rDADQxg-1","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=CThnGJC7iUVA2+9yCGJdrz8JjcdX8FwjiVcktiebKsU=;\n\tb=SjGckr5ekzphElxrYFVHWPKb+Lwzoozfaz6LJUjw8tm8AMwld3mx2UXBJgde/XUac3\n\ttMvqJ2BRJRYDmrCRetoPIGI85GAQDTZs5T8RSW1SQK+x1TH3c46oXqUX1cx+oDlhv2mo\n\tBFeZ7U63KXcxcKDvpMnJJNJq4IkvJC+oJPJcrz/GlYP3SWxQu+ao284BrfjXtSq93Iv5\n\t0sk8diHvci3tJaGdHNJdrHYRFn2g1SMX9Wo6Dg/bEVa+7GTsJ/rWmPbAU50pgTxllyhk\n\tZK+R9Qd5iTGZZ1Sw9ZQBDcGL5qM84pA5pYCJBNk0BmOfI1g3mb7+NmKuK+fATvVISLeA\n\tQbqQ==","X-Gm-Message-State":"AOAM533tZjllMJYu6SKbCO1V7PKdtu8eV5UA8ePN5GzNO+nfoIVe8AK3\n\tqtZaTqaaMdONWGu+zAjQLj0Lt52po2uBBdCaqkvRF1at8Md9NmO5QTvMJMwHF1mHt3eLxfn9LEJ\n\tHDZ2aTY5YuBB50bIi5J7bwNUc/p2H6CZxXfgyDApJK9jguBsWKQ==","X-Received":["by 2002:ac2:4292:: with SMTP id\n\tm18mr12009775lfh.539.1636701490270; \n\tThu, 11 Nov 2021 23:18:10 -0800 (PST)","by 2002:ac2:4292:: with SMTP id\n\tm18mr12009730lfh.539.1636701489795; \n\tThu, 11 Nov 2021 23:18:09 -0800 (PST)"],"X-Google-Smtp-Source":"ABdhPJwoTaJVM27kNF1pwvxOWw17cs+dUQXQEhcBKNYzVGvDOPzaOpLpqEgCGSG8ms65PyFwRi7EDNqsfgu73pbkJjk=","MIME-Version":"1.0","References":"<20211111081055.61127-1-hpa@redhat.com>\n\t","In-Reply-To":"","From":"Kate Hsuan ","Date":"Fri, 12 Nov 2021 15:17:58 +0800","Message-ID":"","To":"Jean-Michel Hautbois ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: Fix -nan exception","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\" "}}]