[{"id":29979,"web_url":"https://patchwork.libcamera.org/comment/29979/","msgid":"<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>","date":"2024-06-17T09:29:18","subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-06-16 17:39:07)\n> The effective exposure value for each frame is split into shutter time,\n> analog gain and digital gain based on the AGC constraint mode and\n> exposure mode. The algorithm uses the modes from the active state, which\n> tracks the latest queued request, instead of the frame context, which\n> tracks the value of the controls requested for that frame. Fix it by\n> using the correct modes.\n\nThe state tracking at different points of time is going to be a\nrecurring tricky part isn't it.\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index a61201bb05c9..6a199b47c9ad 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> utils::Duration shutterTime;\n> double aGain, dGain;\n> std::tie(shutterTime, aGain, dGain) =\n> - calculateNewEv(context.activeState.agc.constraintMode,\n> - context.activeState.agc.exposureMode,\n> + calculateNewEv(frameContext.agc.constraintMode,\n> + frameContext.agc.exposureMode,\n> hist, effectiveExposureValue);\n> \n> LOG(RkISP1Agc, Debug)\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 BD951C3237\n\tfor ;\n\tMon, 17 Jun 2024 09:29:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD45D65491;\n\tMon, 17 Jun 2024 11:29:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DFD161A1C\n\tfor ;\n\tMon, 17 Jun 2024 11:29:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FA329C1;\n\tMon, 17 Jun 2024 11:29:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BWz5yjc0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718616545;\n\tbh=asR71814z+dJPn5mZNQGAghJ97X+G6/x99ZfoZQwbsU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=BWz5yjc0MVZZBIbOW8mhs5ibre8W7BUg+JEnMKqUrIgaUM5wVbLm9Ld1aBNxGITJJ\n\tSLK5k6fuoqoE5RT1SVB2Gqs0/7EMYsAsUbv5hz8h48H7iNtlLy/qXVT4Jp09DZ58lc\n\tBc2Jmv1VocqmM0fY8prHhKkH0Wychd8xFjMtyJf8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240616163910.5506-10-laurent.pinchart@ideasonboard.com>","References":"<20240616163910.5506-1-laurent.pinchart@ideasonboard.com>\n\t<20240616163910.5506-10-laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 17 Jun 2024 10:29:18 +0100","Message-ID":"<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29981,"web_url":"https://patchwork.libcamera.org/comment/29981/","msgid":"<20240617093244.GJ30324@pendragon.ideasonboard.com>","date":"2024-06-17T09:32:44","subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2024-06-16 17:39:07)\n> > The effective exposure value for each frame is split into shutter time,\n> > analog gain and digital gain based on the AGC constraint mode and\n> > exposure mode. The algorithm uses the modes from the active state, which\n> > tracks the latest queued request, instead of the frame context, which\n> > tracks the value of the controls requested for that frame. Fix it by\n> > using the correct modes.\n> \n> The state tracking at different points of time is going to be a\n> recurring tricky part isn't it.\n\nIt appears to be so. A recuring pain point, but at the same time, it\nmakes me happy to see a pattern emerging. Patterns mean we can try to\nfind structural solutions.\n\n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index a61201bb05c9..6a199b47c9ad 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > utils::Duration shutterTime;\n> > double aGain, dGain;\n> > std::tie(shutterTime, aGain, dGain) =\n> > - calculateNewEv(context.activeState.agc.constraintMode,\n> > - context.activeState.agc.exposureMode,\n> > + calculateNewEv(frameContext.agc.constraintMode,\n> > + frameContext.agc.exposureMode,\n> > hist, effectiveExposureValue);\n> > \n> > LOG(RkISP1Agc, Debug)","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 5E13ABD87C\n\tfor ;\n\tMon, 17 Jun 2024 09:33:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79BA665491;\n\tMon, 17 Jun 2024 11:33:08 +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 B0C3E65489\n\tfor ;\n\tMon, 17 Jun 2024 11:33:06 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A264F9C1;\n\tMon, 17 Jun 2024 11:32:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W3pzDMzS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718616769;\n\tbh=uje8z6vuMSVGsLF5oVH9fYvqlKnz1e7y/EwSpstX9Uk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W3pzDMzSrZOEMIRcCpaEWebIrk08195MInrMNItSaurriK2Djn8W+g6a1h7Aw4psO\n\tsueiLAk0mU1B4TI7JPUz1u14y4poEfwNiYYMuHHrYLHaWWYGnmSekislCh4KvzTBMb\n\tmW8cnAdFA63AX8hTjqoU6kUciJfLtqFzXMwmSWfc=","Date":"Mon, 17 Jun 2024 12:32:44 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","Message-ID":"<20240617093244.GJ30324@pendragon.ideasonboard.com>","References":"<20240616163910.5506-1-laurent.pinchart@ideasonboard.com>\n\t<20240616163910.5506-10-laurent.pinchart@ideasonboard.com>\n\t<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>","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":29998,"web_url":"https://patchwork.libcamera.org/comment/29998/","msgid":"","date":"2024-06-17T12:52:21","subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Jun 17, 2024 at 12:32:44PM +0300, Laurent Pinchart wrote:\n> On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2024-06-16 17:39:07)\n> > > The effective exposure value for each frame is split into shutter time,\n> > > analog gain and digital gain based on the AGC constraint mode and\n> > > exposure mode. The algorithm uses the modes from the active state, which\n> > > tracks the latest queued request, instead of the frame context, which\n> > > tracks the value of the controls requested for that frame. Fix it by\n> > > using the correct modes.\n> > \n> > The state tracking at different points of time is going to be a\n> > recurring tricky part isn't it.\n> \n> It appears to be so. A recuring pain point, but at the same time, it\n> makes me happy to see a pattern emerging. Patterns mean we can try to\n> find structural solutions.\n\nI can never remember which is the right one...\n\nReviewed-by: Paul Elder \n\n> \n> > Reviewed-by: Kieran Bingham \n> > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> > > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > index a61201bb05c9..6a199b47c9ad 100644\n> > > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > utils::Duration shutterTime;\n> > > double aGain, dGain;\n> > > std::tie(shutterTime, aGain, dGain) =\n> > > - calculateNewEv(context.activeState.agc.constraintMode,\n> > > - context.activeState.agc.exposureMode,\n> > > + calculateNewEv(frameContext.agc.constraintMode,\n> > > + frameContext.agc.exposureMode,\n> > > hist, effectiveExposureValue);\n> > > \n> > > LOG(RkISP1Agc, Debug)","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 4403DC3237\n\tfor ;\n\tMon, 17 Jun 2024 12:52:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DCB3965494;\n\tMon, 17 Jun 2024 14:52:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8181B61A20\n\tfor ;\n\tMon, 17 Jun 2024 14:52:28 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 782619C1;\n\tMon, 17 Jun 2024 14:52:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hf5gGENb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718628731;\n\tbh=vuyQV2GcEUBYX5AZ0pNa4TyKH5Zbw4cHnIm1ZkgeKFk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hf5gGENbZJBaCWRhCpb2FhQOHA/F2iwTtIdB20z4Lanpsn4kzFC7kqDEgqKb+7ha0\n\toDfxDgOwvRumGarfohOMCn7C1D1IcZkaoY5Q0OXwVTU4h4YPAJ24njZW8S6gFMOhg6\n\tNHEyVjnv9gOKm7JMRICOUVa6OuWVvYAP0S/szpqU=","Date":"Mon, 17 Jun 2024 21:52:21 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","Message-ID":"","References":"<20240616163910.5506-1-laurent.pinchart@ideasonboard.com>\n\t<20240616163910.5506-10-laurent.pinchart@ideasonboard.com>\n\t<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>\n\t<20240617093244.GJ30324@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240617093244.GJ30324@pendragon.ideasonboard.com>","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":29999,"web_url":"https://patchwork.libcamera.org/comment/29999/","msgid":"<20240617125454.GB10134@pendragon.ideasonboard.com>","date":"2024-06-17T12:54:54","subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 17, 2024 at 09:52:21PM +0900, Paul Elder wrote:\n> On Mon, Jun 17, 2024 at 12:32:44PM +0300, Laurent Pinchart wrote:\n> > On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote:\n> > > Quoting Laurent Pinchart (2024-06-16 17:39:07)\n> > > > The effective exposure value for each frame is split into shutter time,\n> > > > analog gain and digital gain based on the AGC constraint mode and\n> > > > exposure mode. The algorithm uses the modes from the active state, which\n> > > > tracks the latest queued request, instead of the frame context, which\n> > > > tracks the value of the controls requested for that frame. Fix it by\n> > > > using the correct modes.\n> > > \n> > > The state tracking at different points of time is going to be a\n> > > recurring tricky part isn't it.\n> > \n> > It appears to be so. A recuring pain point, but at the same time, it\n> > makes me happy to see a pattern emerging. Patterns mean we can try to\n> > find structural solutions.\n> \n> I can never remember which is the right one...\n\nI'll take that as a vote for a structural solution :-)\n\n> Reviewed-by: Paul Elder \n> \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> > > > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > > > \n> > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > > index a61201bb05c9..6a199b47c9ad 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > > utils::Duration shutterTime;\n> > > > double aGain, dGain;\n> > > > std::tie(shutterTime, aGain, dGain) =\n> > > > - calculateNewEv(context.activeState.agc.constraintMode,\n> > > > - context.activeState.agc.exposureMode,\n> > > > + calculateNewEv(frameContext.agc.constraintMode,\n> > > > + frameContext.agc.exposureMode,\n> > > > hist, effectiveExposureValue);\n> > > > \n> > > > LOG(RkISP1Agc, Debug)","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 1D680C3237\n\tfor ;\n\tMon, 17 Jun 2024 12:55:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3136765497;\n\tMon, 17 Jun 2024 14:55:18 +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 897906548B\n\tfor ;\n\tMon, 17 Jun 2024 14:55:16 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7654B9C1;\n\tMon, 17 Jun 2024 14:54:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TEqWqRTN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718628898;\n\tbh=QVPsPz2Kkq6KtAE4qx60TzLnXiLnMuXqgsIvq1hnMIg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TEqWqRTNfspRo5NkzMljUqwotpzO7ntMqQmMrpQhSRqmuOK1O9CcZdp0gcDS46Q8x\n\tYc7oz5dN4rNz1cj2/HobKks9+/b5LdP7YnhIbNuB5ztI+XFGmTq0tn4/zWb5Ihqu3G\n\t9/8o/RdgA5L0Hue+N1m2DMscNovizkDvFpOFy0dc=","Date":"Mon, 17 Jun 2024 15:54:54 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to\n\tcalculate new EV","Message-ID":"<20240617125454.GB10134@pendragon.ideasonboard.com>","References":"<20240616163910.5506-1-laurent.pinchart@ideasonboard.com>\n\t<20240616163910.5506-10-laurent.pinchart@ideasonboard.com>\n\t<171861655872.2248009.16546079695630293316@ping.linuxembedded.co.uk>\n\t<20240617093244.GJ30324@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}}]