[{"id":22063,"web_url":"https://patchwork.libcamera.org/comment/22063/","msgid":"","date":"2022-01-24T11:32:29","subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch!\n\nOn Mon, 24 Jan 2022 at 10:31, Naushir Patuck wrote:\n>\n> Limit the gain code to the maximum value reported by the sensor controls when\n> sending to DelayedControls. The AGC algorithm will handle a lower gain used by\n> the sensor, provided provided it knows the actual gain used. This change ensures\n> that DelayedControls will never report an unclipped gain used.\n\nApart from s/provided provided/provided/:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 0ed41385018a..a72d516f84ee 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -173,6 +173,9 @@ private:\n> /* Frame duration (1/fps) limits. */\n> Duration minFrameDuration_;\n> Duration maxFrameDuration_;\n> +\n> + /* Maximum gain code for the sensor. */\n> + uint32_t maxSensorGainCode_;\n> };\n>\n> int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n> @@ -357,6 +360,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n> return -1;\n> }\n>\n> + maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get();\n> +\n> /* Setup a metadata ControlList to output metadata. */\n> libcameraMetadata_ = ControlList(controls::controls);\n>\n> @@ -1113,6 +1118,13 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> {\n> int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n>\n> + /*\n> + * Ensure anything larger than the max gain code will not be passed to\n> + * DelayedControls. The AGC will correctly handle a lower gain returned\n> + * by the sensor, provided it knows the actual gain used.\n> + */\n> + gainCode = std::min(gainCode, maxSensorGainCode_);\n> +\n> /* GetVBlanking might clip exposure time to the fps limits. */\n> Duration exposure = agcStatus->shutter_time;\n> int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);\n> --\n> 2.25.1\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 ED628BDCBF\n\tfor ;\n\tMon, 24 Jan 2022 11:32:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 472BA60987;\n\tMon, 24 Jan 2022 12:32:43 +0100 (CET)","from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com\n\t[IPv6:2a00:1450:4864:20::42e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02A6E6020E\n\tfor ;\n\tMon, 24 Jan 2022 12:32:40 +0100 (CET)","by mail-wr1-x42e.google.com with SMTP id f17so12965983wrx.1\n\tfor ;\n\tMon, 24 Jan 2022 03:32:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"FdDTAbWn\"; 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=ma4kVJt+N84eucoIBaWsgdjIm727Frd7zB/sBTti364=;\n\tb=FdDTAbWnGJHowegXTMDvraFSQkj4gaMGHBUjcReCe0+lXNQZOlHFzDXIoHT597CCRV\n\tV0qy/xWxxFn5gthg9nqpxysRR7T/lnk+pnAA5lHHVyc0Z9GypFgtfvpImhcB5Pjms7gr\n\tBuzWor29tIuiYEaDc0/UZBcDVwmDpHSNGHDDA6fLTMmi7VuZYcnBQhoVSwysyH59vNt/\n\tfx/WjwVfybypx7Z4Vz/8hmTeyO0xct5QlFhR86zkDeDU5iIHZO2h1/OzR7gC8LMz2+8/\n\tSXVAj8yCk9asr5kQqJVh6gO2+oAUX8WjNVS98B1Wc+9c0LqIFeckAyKpNfl1CgJMfRha\n\tRlMQ==","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=ma4kVJt+N84eucoIBaWsgdjIm727Frd7zB/sBTti364=;\n\tb=SQz3XywmbIoLGZVqOixOlIqsCX7clLTkxNyxCb8rto1C/J4Phyy+nNDqFo/YSUONTB\n\t4n6NYAcd6lDXsuGYMIaT4loPzIc018GEVehaabff+YWzu2BzGygs+AcM51UdCB3crSKR\n\toWuGGFEw5TMoS0r8GENWDWs5HHoJKL5lBmBh44PLoZ/FdETIBBH2Rm7xuKz11SI7pUs8\n\tXuGFBRoRcreFVEE/XsxPiItbILT55ZDiKSnyT5BSTjdjHsOTWDlB9l4m9oJxWvCfzVmz\n\tJp8dc9AA0FkVm9BLRqJg+Y4vLOb1qo3riZZBL6cyyE3p+urNrzEWRj2LR84KUGOmxg0e\n\tNYcA==","X-Gm-Message-State":"AOAM5339ZUhChJHuF9a2bgH/zliUiWOsOS2df98oAhvpG2sI0/5nsFDQ\n\twCtQkKNyK/gDtzn29CsuYEXTcVQ2uQuGtWrmk1Aawg==","X-Google-Smtp-Source":"ABdhPJzffo6DKwK+HrxEM+9ppb9VY3/5by/mYUs3rUuArRlWtHR3ZlCoD9nMzFb56Qs1sGf1kMmHSoPSXDNI5CgD83A=","X-Received":"by 2002:a5d:6d8b:: with SMTP id l11mr2004119wrs.24.1643023960674;\n\tMon, 24 Jan 2022 03:32:40 -0800 (PST)","MIME-Version":"1.0","References":"<20220124103107.1799464-1-naush@raspberrypi.com>","In-Reply-To":"<20220124103107.1799464-1-naush@raspberrypi.com>","From":"David Plowman ","Date":"Mon, 24 Jan 2022 11:32:29 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","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":22078,"web_url":"https://patchwork.libcamera.org/comment/22078/","msgid":"<164367360435.115113.16459541156495425499@Monstersaurus>","date":"2022-02-01T00:00:04","subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nQuoting Naushir Patuck (2022-01-24 10:31:06)\n> Limit the gain code to the maximum value reported by the sensor controls when\n> sending to DelayedControls. The AGC algorithm will handle a lower gain used by\n> the sensor, provided provided it knows the actual gain used. This change ensures\n> that DelayedControls will never report an unclipped gain used.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 0ed41385018a..a72d516f84ee 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -173,6 +173,9 @@ private:\n> /* Frame duration (1/fps) limits. */\n> Duration minFrameDuration_;\n> Duration maxFrameDuration_;\n> +\n> + /* Maximum gain code for the sensor. */\n> + uint32_t maxSensorGainCode_;\n\nAny risk of needing to clamp the minimum value too?\n\n\n> };\n> \n> int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n> @@ -357,6 +360,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n> return -1;\n> }\n> \n> + maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get();\n> +\n> /* Setup a metadata ControlList to output metadata. */\n> libcameraMetadata_ = ControlList(controls::controls);\n> \n> @@ -1113,6 +1118,13 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> {\n> int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> \n> + /*\n> + * Ensure anything larger than the max gain code will not be passed to\n> + * DelayedControls. The AGC will correctly handle a lower gain returned\n> + * by the sensor, provided it knows the actual gain used.\n> + */\n> + gainCode = std::min(gainCode, maxSensorGainCode_);\n> +\n\nSounds ok to me though.\n\nReviewed-by: Kieran Bingham \n\n> /* GetVBlanking might clip exposure time to the fps limits. */\n> Duration exposure = agcStatus->shutter_time;\n> int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);\n> -- \n> 2.25.1\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 AF311BDCBF\n\tfor ;\n\tTue, 1 Feb 2022 00:00:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 25E3B609AC;\n\tTue, 1 Feb 2022 01:00:09 +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 B7922604F1\n\tfor ;\n\tTue, 1 Feb 2022 01:00:07 +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 4F6CAD88;\n\tTue, 1 Feb 2022 01:00:07 +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=\"CII+GzRs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1643673607;\n\tbh=FYkUWUU9ECcMkVMQE5sSLRI+OJVkVzn4wHM/2EU4UEY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=CII+GzRsMpeekAbCPwBfyilR8yCkMCXe7DvPDyz8gBMnFlCjZCV50ICvl4neIadlB\n\tOrLfTj3W60ff3bLWNzxGpcWzdbxe7cD2zGwluQXqvbfv43gNWyZGum3m6EptGc4UhI\n\tABtYkUcGCcJta2vtRUrroQ29lKbWbD5S8M0zUGU4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220124103107.1799464-1-naush@raspberrypi.com>","References":"<20220124103107.1799464-1-naush@raspberrypi.com>","From":"Kieran Bingham ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 01 Feb 2022 00:00:04 +0000","Message-ID":"<164367360435.115113.16459541156495425499@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","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":22083,"web_url":"https://patchwork.libcamera.org/comment/22083/","msgid":"","date":"2022-02-01T08:48:42","subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThanks for the review (this + all the other patches as well)!\n\nOn Tue, 1 Feb 2022 at 00:00, Kieran Bingham \nwrote:\n\n> Hi Naush,\n>\n> Quoting Naushir Patuck (2022-01-24 10:31:06)\n> > Limit the gain code to the maximum value reported by the sensor controls\n> when\n> > sending to DelayedControls. The AGC algorithm will handle a lower gain\n> used by\n> > the sensor, provided provided it knows the actual gain used. This change\n> ensures\n> > that DelayedControls will never report an unclipped gain used.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++++\n> > 1 file changed, 12 insertions(+)\n> >\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 0ed41385018a..a72d516f84ee 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -173,6 +173,9 @@ private:\n> > /* Frame duration (1/fps) limits. */\n> > Duration minFrameDuration_;\n> > Duration maxFrameDuration_;\n> > +\n> > + /* Maximum gain code for the sensor. */\n> > + uint32_t maxSensorGainCode_;\n>\n> Any risk of needing to clamp the minimum value too?\n>\n\nThis is a good point.\nI made the assumption that a minimum gain of 1.0 is universal, but perhaps\nnot...\nDavid, what do you think? I recall you had played with devices where this\nmay possible\nnot be true?\n\nRegards,\nNaush\n\n\n>\n> > };\n> >\n> > int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig\n> *sensorConfig)\n> > @@ -357,6 +360,8 @@ int IPARPi::configure(const IPACameraSensorInfo\n> &sensorInfo,\n> > return -1;\n> > }\n> >\n> > + maxSensorGainCode_ =\n> sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get();\n> > +\n> > /* Setup a metadata ControlList to output metadata. */\n> > libcameraMetadata_ = ControlList(controls::controls);\n> >\n> > @@ -1113,6 +1118,13 @@ void IPARPi::applyAGC(const struct AgcStatus\n> *agcStatus, ControlList &ctrls)\n> > {\n> > int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> >\n> > + /*\n> > + * Ensure anything larger than the max gain code will not be\n> passed to\n> > + * DelayedControls. The AGC will correctly handle a lower gain\n> returned\n> > + * by the sensor, provided it knows the actual gain used.\n> > + */\n> > + gainCode = std::min(gainCode, maxSensorGainCode_);\n> > +\n>\n> Sounds ok to me though.\n>\n> Reviewed-by: Kieran Bingham \n>\n> > /* GetVBlanking might clip exposure time to the fps limits. */\n> > Duration exposure = agcStatus->shutter_time;\n> > int32_t vblanking = helper_->GetVBlanking(exposure,\n> minFrameDuration_, maxFrameDuration_);\n> > --\n> > 2.25.1\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 58AAABF415\n\tfor ;\n\tTue, 1 Feb 2022 08:49:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FB86609C9;\n\tTue, 1 Feb 2022 09:49:01 +0100 (CET)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC97360986\n\tfor ;\n\tTue, 1 Feb 2022 09:48:59 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id u6so32255315lfm.10\n\tfor ;\n\tTue, 01 Feb 2022 00:48:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"nCp5JuhL\"; 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=151CJ6fT86ONlbXH2M5JuYZpLMIuDNaRUZ8bVSMaV2k=;\n\tb=nCp5JuhLDmEjuejytwNYr881G3I5D8RQVGKf0umkvE8ZKXsGYVc4CO2OBJ5Y1EjgvQ\n\tTtwFTMDnJfkNcMrfBJurO4YWqmqnhid4g4GC3Po5BarQvmhHBo1mhyJtRc4tbwD7+HHl\n\ttNqaRWO6DUM9EBA7CxJSZ0zpFBy86+RHfg1IAp8B14CfZO/LSr3vpCiiE1Bh8z9KFt8n\n\t+4XkBoPhD8Hv2Ujyn/vLImKhCsBPH1ovb3HWGEMhefbnIKrIs9nUl57l3bhFuua/MmFW\n\t8tYo0egAWAuXEkyK5JrdU0cze/wzmzzdm9smw1dQEB+np6vck/wWONVUwntvH6IYNOq3\n\tXELQ==","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=151CJ6fT86ONlbXH2M5JuYZpLMIuDNaRUZ8bVSMaV2k=;\n\tb=vmG5yPCbe/U9EePiwgTwNxJ62GGwnH+OombLzW2U/bbswIHItnn5a2e5afU55pvBwf\n\tMQrCtJV6ttmR8fBYEIgy7s5HenKTT7GpfmIfC/IGPtpzl7fxNEiPZ4Ao/je27/yLL4pq\n\tX4tVIV2RDkYCG8ODN1Tuxa9FIbLlxwSpNxQ5ywyebbPBb/M2uyKFdrgp6AjCWvqcWnSr\n\tF1uk2h4FT/qURBQt669oQLCeWjmmrwaOFJzT3BhSV3ONMqd+3JdWB+OMMA4mJ1DpnZDw\n\t5eKo5KKK0EE6CO5fgDvH7gJl8+En2Qk6DG9qtifj/CGprRgK0+Ne3l26uqYAhut61grZ\n\tCA0g==","X-Gm-Message-State":"AOAM530KH5dR3D9XXw8lOa66SLL65pCDIfyxdtsiDnkXiHgAo7Fbunyp\n\tdjzu15Clu6MfkXrr5fzl2FJVU4BruITiV9b5TnYrwg==","X-Google-Smtp-Source":"ABdhPJx/uBNCY+ELs2tvy5IhUYPf/v6fx6s7k8nAql2vMjAhL6ErgZ7SKhVl4Hi7pdOi3No/ODyCzo60GVw7+gj7SpI=","X-Received":"by 2002:a05:6512:441:: with SMTP id\n\ty1mr18032601lfk.315.1643705339166; \n\tTue, 01 Feb 2022 00:48:59 -0800 (PST)","MIME-Version":"1.0","References":"<20220124103107.1799464-1-naush@raspberrypi.com>\n\t<164367360435.115113.16459541156495425499@Monstersaurus>","In-Reply-To":"<164367360435.115113.16459541156495425499@Monstersaurus>","From":"Naushir Patuck ","Date":"Tue, 1 Feb 2022 08:48:42 +0000","Message-ID":"","To":"Kieran Bingham , \n\tDavid Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000db587a05d6f0f846\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","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":22086,"web_url":"https://patchwork.libcamera.org/comment/22086/","msgid":"","date":"2022-02-01T09:37:19","subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nOn Tue, 1 Feb 2022 at 08:48, Naushir Patuck wrote:\n>\n> Hi Kieran,\n>\n> Thanks for the review (this + all the other patches as well)!\n>\n> On Tue, 1 Feb 2022 at 00:00, Kieran Bingham wrote:\n>>\n>> Hi Naush,\n>>\n>> Quoting Naushir Patuck (2022-01-24 10:31:06)\n>> > Limit the gain code to the maximum value reported by the sensor controls when\n>> > sending to DelayedControls. The AGC algorithm will handle a lower gain used by\n>> > the sensor, provided provided it knows the actual gain used. This change ensures\n>> > that DelayedControls will never report an unclipped gain used.\n>> >\n>> > Signed-off-by: Naushir Patuck \n>> > ---\n>> > src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++++\n>> > 1 file changed, 12 insertions(+)\n>> >\n>> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > index 0ed41385018a..a72d516f84ee 100644\n>> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > @@ -173,6 +173,9 @@ private:\n>> > /* Frame duration (1/fps) limits. */\n>> > Duration minFrameDuration_;\n>> > Duration maxFrameDuration_;\n>> > +\n>> > + /* Maximum gain code for the sensor. */\n>> > + uint32_t maxSensorGainCode_;\n>>\n>> Any risk of needing to clamp the minimum value too?\n>\n>\n> This is a good point.\n> I made the assumption that a minimum gain of 1.0 is universal, but perhaps not...\n> David, what do you think? I recall you had played with devices where this may possible\n> not be true?\n>\n\nIt's a fair question, but I'm not inclined to clamp the minimum as\nwell. I think handling that would require extra code in the AGC (I\nhave other things to tackle right now...) or maybe editing exposure\nprofiles on a per-sensor basis (also undesirable,). Actually I have\none sensor which has a minimum gain of 1.12 for *some*, but not all,\nreadout modes, but I take care of all that in the cam helper. So\nanyway, I'd rather fix the problem that we really have and leave this\nquestion on the back burner for now. We could re-consider it if it\never becomes a real thing.\n\nDavid\n\n> Regards,\n> Naush\n>\n>>\n>>\n>> > };\n>> >\n>> > int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n>> > @@ -357,6 +360,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n>> > return -1;\n>> > }\n>> >\n>> > + maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get();\n>> > +\n>> > /* Setup a metadata ControlList to output metadata. */\n>> > libcameraMetadata_ = ControlList(controls::controls);\n>> >\n>> > @@ -1113,6 +1118,13 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n>> > {\n>> > int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n>> >\n>> > + /*\n>> > + * Ensure anything larger than the max gain code will not be passed to\n>> > + * DelayedControls. The AGC will correctly handle a lower gain returned\n>> > + * by the sensor, provided it knows the actual gain used.\n>> > + */\n>> > + gainCode = std::min(gainCode, maxSensorGainCode_);\n>> > +\n>>\n>> Sounds ok to me though.\n>>\n>> Reviewed-by: Kieran Bingham \n>>\n>> > /* GetVBlanking might clip exposure time to the fps limits. */\n>> > Duration exposure = agcStatus->shutter_time;\n>> > int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);\n>> > --\n>> > 2.25.1\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 E6B55BF415\n\tfor ;\n\tTue, 1 Feb 2022 09:37:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 47C26609E2;\n\tTue, 1 Feb 2022 10:37:31 +0100 (CET)","from mail-wm1-x334.google.com (mail-wm1-x334.google.com\n\t[IPv6:2a00:1450:4864:20::334])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31377609AC\n\tfor ;\n\tTue, 1 Feb 2022 10:37:30 +0100 (CET)","by mail-wm1-x334.google.com with SMTP id\n\tl12-20020a7bc34c000000b003467c58cbdfso1407453wmj.2\n\tfor ;\n\tTue, 01 Feb 2022 01:37:30 -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=\"EnKnDXom\"; 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=RtV86z5wBKYcIrzYdVmPYJ57fMmXXmyDVirxhRloVRk=;\n\tb=EnKnDXomAyS9RLGQaNNXr/Ax7O4XcOUE9gmpyhBlDPaV2NWAzDucX/lGrZ/7QL8Rth\n\t4Nr24jiy0rWT3+zjG1I3O69AgyFiOY33wjrN+ly33nEpenUYh5GeuWAE5/AoINBaW2RE\n\tccUzu4xh9MVhiSs0iw4StbOPX/ya8DeBVGU9cq6KIkbFoZ27uUHOAb+eACKZkKzQL2Em\n\tp00zbiXkcf9SF0EGQqO9tUZmHf8LgYmh/9zXz1UKQJTHcbDjBtz1Ydv2+8lSZjWq5MMY\n\tu95inwAzn9yZ7EAt+Df+uPfypHw7oFZaCnLtp9HL/SxxrojZCF4yi0uHRhCgn6NHbk+m\n\tutsQ==","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=RtV86z5wBKYcIrzYdVmPYJ57fMmXXmyDVirxhRloVRk=;\n\tb=snJ93k952pZTm2HMX8Mqd5Fc1/024TKVZWA/udt2srvo0MnFCdZRyv213xNEMyUEs5\n\t6Vi68eG62etqaQUXmMH0s5lTJxPluN1qwf7MrmZ4UhayYSmXIedCruIQH9pjr+c70cG+\n\tolbxfbWFHKKADTADzdS3ojOVpCzfnuWardVqre7jw14abeI4zdZzVRSV7iD3ug+rbOTT\n\tugNNZsObAOAhCLyKNss3sV9cQ1MsTT/gf6OJikS71QTvDPZOKTARo/sBi1d0QGnm1Dz5\n\t624YtBCzhbeiqWCU7OJzHEmLpIaZFonbsmbf9tk0P9mKw6Ky0RZv97TFhNBLYGXvZOHD\n\tNIZQ==","X-Gm-Message-State":"AOAM530uIgORoAUfRj9D+UMB1uTppJWF+VUinrIAStgXuuJjbF7DmMcv\n\tmJ1KDi5I86NRUUZVf4d5thImiWhKcag090/v80+eq0POigHGig==","X-Google-Smtp-Source":"ABdhPJwmsnwFPnw66xhK3We+gKSNLrnHk7wOyM7Ve8e1V3d/FOeNMFzpXlXcz3zoXr2EVLoE2btr151rBRJ0XoFqY2s=","X-Received":"by 2002:a05:600c:491:: with SMTP id\n\td17mr960591wme.130.1643708249755; \n\tTue, 01 Feb 2022 01:37:29 -0800 (PST)","MIME-Version":"1.0","References":"<20220124103107.1799464-1-naush@raspberrypi.com>\n\t<164367360435.115113.16459541156495425499@Monstersaurus>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 1 Feb 2022 09:37:19 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] ipa: raspberrypi: Limit the\n\tmaximum sensor gain used","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\" "}}]