[{"id":25015,"web_url":"https://patchwork.libcamera.org/comment/25015/","msgid":"<166368299360.3912877.5958289505265461429@Monstersaurus>","date":"2022-09-20T14:09:53","subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:37)\n> Inherit from the base FrameContext class in the IPU3 IPAFrameContext.\n> This allows dropping the frame member, which is now stored in the base\n> class.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/ipa_context.cpp | 24 ++++--------------------\n> src/ipa/ipu3/ipa_context.h | 7 ++++---\n> src/ipa/ipu3/ipu3.cpp | 5 +----\n> 3 files changed, 9 insertions(+), 27 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 13cdb835ef7f..9cfca0db3a0d 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n> * most recently computed by the IPA algorithms.\n> */\n> \n> -/**\n> - * \\struct IPAFrameContext\n> - * \\brief Context for a frame\n> - *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> - *\n> - * Fields in the frame context should reflect values and controls\n> - * associated with the specific frame as requested by the application, and\n> - * as configured by the hardware. Fields can be read by algorithms to\n> - * determine if they should update any specific action for this frame, and\n> - * finally to update the metadata control lists when the frame is fully\n> - * completed.\n> - */\n> -\n> /**\n> * \\struct IPAContext\n> * \\brief Global IPA context data shared between all algorithms\n> @@ -188,15 +172,15 @@ IPAFrameContext::IPAFrameContext() = default;\n> /**\n> * \\brief Construct a IPAFrameContext instance\n> */\n> -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> - : frame(id), frameControls(reqControls)\n> +IPAFrameContext::IPAFrameContext(const ControlList &reqControls)\n> + : frameControls(reqControls)\n> {\n> sensor = {};\n> }\n> \n> /**\n> - * \\var IPAFrameContext::frame\n> - * \\brief The frame number\n> + * \\struct IPAFrameContext\n> + * \\brief IPU3-specific FrameContext\n> *\n> * \\var IPAFrameContext::frameControls\n> * \\brief Controls sent in by the application while queuing the request\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 42e11141d3a1..e8fc42769075 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -17,6 +17,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> namespace libcamera {\n> \n> namespace ipa::ipu3 {\n> @@ -76,16 +78,15 @@ struct IPAActiveState {\n> } toneMapping;\n> };\n> \n> -struct IPAFrameContext {\n> +struct IPAFrameContext : public FrameContext {\n> IPAFrameContext();\n> - IPAFrameContext(uint32_t id, const ControlList &reqControls);\n> + IPAFrameContext(const ControlList &reqControls);\n> \n> struct {\n> uint32_t exposure;\n> double gain;\n> } sensor;\n> \n> - uint32_t frame;\n> ControlList frameControls;\n> };\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index e5a763fd2b08..b1b23fd8f927 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -607,9 +607,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> \n> IPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> \n> - if (frameContext.frame != frame)\n> - LOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> -\n\n(almost) unrelated change. Is this intentional?\n\nIt could be (but the frameContext.frame will still find the derived\nframe member right?), so an updated commit message, or split to a\nseparate patch and :\n\nReviewed-by: Kieran Bingham \n\n> frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> \n> @@ -654,7 +651,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> /* \\todo Start processing for 'frame' based on 'controls'. */\n> - context_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n> + context_.frameContexts[frame % kMaxFrameContexts] = { controls };\n> }\n> \n> /**\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 DFACEC0DA4\n\tfor ;\n\tTue, 20 Sep 2022 14:09:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 32D60621B2;\n\tTue, 20 Sep 2022 16:09:58 +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 D36D76218B\n\tfor ;\n\tTue, 20 Sep 2022 16:09:56 +0200 (CEST)","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 5764D6BE;\n\tTue, 20 Sep 2022 16:09:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663682998;\n\tbh=Je6le75/A5xWf7PB13ShHmSLBNgeNg7CQVu6BuJ/kfE=;\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=GPqrSX0QOkJS3LWoW9zLBp6uBUuPFsj8RhdlMvrdF7P4/2vO2ShgYhWcP2pPzIMO7\n\t5Hq8AFLUu+1ZOQlKm1YRfOuzrwyYnBm0lSE1UYhpX29XEadn+f4WRjRYBcUM4OCr1o\n\tLs0Rr1rbvqD4f5ue7XrCKvaUFOeRZIgR9QX4pozt1xx9ana0xPkMbvDLgQo2pr/wqQ\n\tonJaZ8lSpYqPPtwfotKsIkANLv9TxQxWM5DErEZTuCiebR/QZWTdMyrHBxPuS/xeCU\n\tfnvkMkQXAid4Sct4tAUlw/H7C+P1Ec3PZdqCum/GWFqDzjMnmneN+ZjR+FauFY+xqY\n\tJGdaaJm6TT9Vg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663682996;\n\tbh=Je6le75/A5xWf7PB13ShHmSLBNgeNg7CQVu6BuJ/kfE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=CbS3kIQljsMreDoxTi+eRvrro8WHdUtycmWPQzuQsC7VMZYNifJbhmudRAVW6OVUq\n\tkQPl8EHwsWRc5fX7rZ84cilBUhP24SzE6/5PqOTw6x/O08inVBdx5HDAgQJP4Ypi6r\n\ti3mB6vVCgvgw868ZOi8TDUts8tITXFK1yRQuyDA4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CbS3kIQl\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 20 Sep 2022 15:09:53 +0100","Message-ID":"<166368299360.3912877.5958289505265461429@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","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":25055,"web_url":"https://patchwork.libcamera.org/comment/25055/","msgid":"<20220921181030.oof6zaiwlej3dcwj@uno.localdomain>","date":"2022-09-21T18:10:30","subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Sep 08, 2022 at 04:41:37AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Inherit from the base FrameContext class in the IPU3 IPAFrameContext.\n> This allows dropping the frame member, which is now stored in the base\n> class.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/ipa/ipu3/ipa_context.cpp | 24 ++++--------------------\n> src/ipa/ipu3/ipa_context.h | 7 ++++---\n> src/ipa/ipu3/ipu3.cpp | 5 +----\n> 3 files changed, 9 insertions(+), 27 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 13cdb835ef7f..9cfca0db3a0d 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n> * most recently computed by the IPA algorithms.\n> */\n>\n> -/**\n> - * \\struct IPAFrameContext\n> - * \\brief Context for a frame\n> - *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> - *\n> - * Fields in the frame context should reflect values and controls\n> - * associated with the specific frame as requested by the application, and\n> - * as configured by the hardware. Fields can be read by algorithms to\n> - * determine if they should update any specific action for this frame, and\n> - * finally to update the metadata control lists when the frame is fully\n> - * completed.\n> - */\n> -\n> /**\n> * \\struct IPAContext\n> * \\brief Global IPA context data shared between all algorithms\n> @@ -188,15 +172,15 @@ IPAFrameContext::IPAFrameContext() = default;\n> /**\n> * \\brief Construct a IPAFrameContext instance\n> */\n> -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> -\t: frame(id), frameControls(reqControls)\n> +IPAFrameContext::IPAFrameContext(const ControlList &reqControls)\n> +\t: frameControls(reqControls)\n> {\n> \tsensor = {};\n> }\n>\n> /**\n> - * \\var IPAFrameContext::frame\n> - * \\brief The frame number\n> + * \\struct IPAFrameContext\n> + * \\brief IPU3-specific FrameContext\n> *\n> * \\var IPAFrameContext::frameControls\n> * \\brief Controls sent in by the application while queuing the request\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 42e11141d3a1..e8fc42769075 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -17,6 +17,8 @@\n> #include \n> #include \n>\n> +#include \n> +\n> namespace libcamera {\n>\n> namespace ipa::ipu3 {\n> @@ -76,16 +78,15 @@ struct IPAActiveState {\n> \t} toneMapping;\n> };\n>\n> -struct IPAFrameContext {\n> +struct IPAFrameContext : public FrameContext {\n\nThis could now be called IPU3FrameContext :)\n\n> \tIPAFrameContext();\n> -\tIPAFrameContext(uint32_t id, const ControlList &reqControls);\n> +\tIPAFrameContext(const ControlList &reqControls);\n>\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n>\n> -\tuint32_t frame;\n> \tControlList frameControls;\n> };\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index e5a763fd2b08..b1b23fd8f927 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -607,9 +607,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>\n> \tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n>\n> -\tif (frameContext.frame != frame)\n> -\t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> -\n> \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>\n> @@ -654,7 +651,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> -\tcontext_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n> +\tcontext_.frameContexts[frame % kMaxFrameContexts] = { controls };\n\nI understand this right that once moving to the FCQ the frame number\ninitialization and the check above will be there performed ?\n\nShould we allow then to have a constructor for the IPA-specific class\nthat accepts a frame number ?\n\nNits apart\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> }\n>\n> /**\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 3B11AC0DA4\n\tfor ;\n\tWed, 21 Sep 2022 18:10:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 949BA621EC;\n\tWed, 21 Sep 2022 20:10:34 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::224])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 15C3F621E9\n\tfor ;\n\tWed, 21 Sep 2022 20:10:33 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 69F39E0005;\n\tWed, 21 Sep 2022 18:10:32 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663783834;\n\tbh=wF6h+Ubi/I0qjpAQYmeFL5t3lXZ4RuUneDdQZZflpsw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=XseTa75NtyQufa3WmcLhGNMUfEWFR6GfwYrU9mstdU88yT+jpo2bdAdDZXN0CebpM\n\tn4AIJByW8m+zsGaCUgvDg3vf69owHi9JxfADSQ+jRH2vQ0CnTW9HT3n39T8LbpeoRn\n\tXhYXqbPcT/F3w4HJIyTN3HFMfG+3AD/z6jC5/Mbggj0FhyaU4R9FEznqEH7JwNI0eQ\n\t1uT6AjE3u3pegX1icXEB0DTMlBDkRxSYNjgHZizDV+o3CTuZyahUMbs34fpHqNyQg1\n\tcfOXb5dOUmEGu9oo1L7odNfelXjIPjavp8BcbMD8S31YRhsvSben2NsVrEbhR2qmeR\n\tsNx4/4j4H4JHg==","Date":"Wed, 21 Sep 2022 20:10:30 +0200","To":"Laurent Pinchart ","Message-ID":"<20220921181030.oof6zaiwlej3dcwj@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25089,"web_url":"https://patchwork.libcamera.org/comment/25089/","msgid":"","date":"2022-09-22T19:53:12","subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Sep 20, 2022 at 03:09:53PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:37)\n> > Inherit from the base FrameContext class in the IPU3 IPAFrameContext.\n> > This allows dropping the frame member, which is now stored in the base\n> > class.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/ipa_context.cpp | 24 ++++--------------------\n> > src/ipa/ipu3/ipa_context.h | 7 ++++---\n> > src/ipa/ipu3/ipu3.cpp | 5 +----\n> > 3 files changed, 9 insertions(+), 27 deletions(-)\n> > \n> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > index 13cdb835ef7f..9cfca0db3a0d 100644\n> > --- a/src/ipa/ipu3/ipa_context.cpp\n> > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n> > * most recently computed by the IPA algorithms.\n> > */\n> > \n> > -/**\n> > - * \\struct IPAFrameContext\n> > - * \\brief Context for a frame\n> > - *\n> > - * The frame context stores data specific to a single frame processed by the\n> > - * IPA. Each frame processed by the IPA has a context associated with it,\n> > - * accessible through the IPAContext structure.\n> > - *\n> > - * Fields in the frame context should reflect values and controls\n> > - * associated with the specific frame as requested by the application, and\n> > - * as configured by the hardware. Fields can be read by algorithms to\n> > - * determine if they should update any specific action for this frame, and\n> > - * finally to update the metadata control lists when the frame is fully\n> > - * completed.\n> > - */\n> > -\n> > /**\n> > * \\struct IPAContext\n> > * \\brief Global IPA context data shared between all algorithms\n> > @@ -188,15 +172,15 @@ IPAFrameContext::IPAFrameContext() = default;\n> > /**\n> > * \\brief Construct a IPAFrameContext instance\n> > */\n> > -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> > - : frame(id), frameControls(reqControls)\n> > +IPAFrameContext::IPAFrameContext(const ControlList &reqControls)\n> > + : frameControls(reqControls)\n> > {\n> > sensor = {};\n> > }\n> > \n> > /**\n> > - * \\var IPAFrameContext::frame\n> > - * \\brief The frame number\n> > + * \\struct IPAFrameContext\n> > + * \\brief IPU3-specific FrameContext\n> > *\n> > * \\var IPAFrameContext::frameControls\n> > * \\brief Controls sent in by the application while queuing the request\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 42e11141d3a1..e8fc42769075 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -17,6 +17,8 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +\n> > namespace libcamera {\n> > \n> > namespace ipa::ipu3 {\n> > @@ -76,16 +78,15 @@ struct IPAActiveState {\n> > } toneMapping;\n> > };\n> > \n> > -struct IPAFrameContext {\n> > +struct IPAFrameContext : public FrameContext {\n> > IPAFrameContext();\n> > - IPAFrameContext(uint32_t id, const ControlList &reqControls);\n> > + IPAFrameContext(const ControlList &reqControls);\n> > \n> > struct {\n> > uint32_t exposure;\n> > double gain;\n> > } sensor;\n> > \n> > - uint32_t frame;\n> > ControlList frameControls;\n> > };\n> > \n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index e5a763fd2b08..b1b23fd8f927 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -607,9 +607,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > \n> > IPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> > \n> > - if (frameContext.frame != frame)\n> > - LOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> > -\n> \n> (almost) unrelated change. Is this intentional?\n\nThe frame member of the base class is private, so this didn't compile\nanymore. I didn't think the check was useful, given the upcoming switch\nto FCQueue, so I dropped it. I'll mention that in the commit message.\n\n> It could be (but the frameContext.frame will still find the derived\n> frame member right?), so an updated commit message, or split to a\n> separate patch and :\n> \n> Reviewed-by: Kieran Bingham \n> \n> > frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > \n> > @@ -654,7 +651,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > /* \\todo Start processing for 'frame' based on 'controls'. */\n> > - context_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n> > + context_.frameContexts[frame % kMaxFrameContexts] = { controls };\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 C91B2C0DA4\n\tfor ;\n\tThu, 22 Sep 2022 19:53:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB03462215;\n\tThu, 22 Sep 2022 21:53:29 +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 08A0E6219A\n\tfor ;\n\tThu, 22 Sep 2022 21:53:29 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A9936BE;\n\tThu, 22 Sep 2022 21:53:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663876409;\n\tbh=IKyD+QQXnbVp3MwvolbUckN66CbjvV8kofcgh6d/azc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=t4CEeYlkcL9T/gFkeL9/G01KJGGo1B71rS/eH/rU4aQbC+quk7jtpJEuNh/hiJBDH\n\tRtnSTbUYDyvMAbN8A5wSYGqu90zOjNfTu5ef9zVVQu03tL8gpq02OeJ74FUEQYqKBI\n\t7yWMJETBKW9xJWHKAAoUGRyoWdqPI4V7ukybMOc59Vu2cYi3StkWGw4c5J6IG2qL49\n\tmB92xti1etCsdmzXlKsaGSXEyXF9HPL5BBZtsRoDvI6DtgJHTMWrpQkTo4AbMzag8v\n\tkaZ09E9wpJN8VnJLyGSTwf5EuMMZllWN3Ls/dzUw7HwB2BFt290dtFVbJenDKkoNko\n\t1fGBL6+un0BRg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663876408;\n\tbh=IKyD+QQXnbVp3MwvolbUckN66CbjvV8kofcgh6d/azc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uQ7A90aG/srANuueriJ54Wm7NV3RHhs5/cW5snNEM3lObS5iiJim1HLWm87uzFIU6\n\ttVmLBSEF2BDvzwmzfy5h2JxlxOckDO+2p2ua0ecLkTojHat0/9ZPEkUG5aJrS/1iNM\n\tX+W/yfdyGi5CrgaYkUyb5J0yxq2etUuujkMQUq+g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uQ7A90aG\"; dkim-atps=neutral","Date":"Thu, 22 Sep 2022 22:53:12 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>\n\t<166368299360.3912877.5958289505265461429@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166368299360.3912877.5958289505265461429@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25090,"web_url":"https://patchwork.libcamera.org/comment/25090/","msgid":"","date":"2022-09-22T19:54:54","subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Sep 21, 2022 at 08:10:30PM +0200, Jacopo Mondi wrote:\n> On Thu, Sep 08, 2022 at 04:41:37AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Inherit from the base FrameContext class in the IPU3 IPAFrameContext.\n> > This allows dropping the frame member, which is now stored in the base\n> > class.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/ipa/ipu3/ipa_context.cpp | 24 ++++--------------------\n> > src/ipa/ipu3/ipa_context.h | 7 ++++---\n> > src/ipa/ipu3/ipu3.cpp | 5 +----\n> > 3 files changed, 9 insertions(+), 27 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > index 13cdb835ef7f..9cfca0db3a0d 100644\n> > --- a/src/ipa/ipu3/ipa_context.cpp\n> > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n> > * most recently computed by the IPA algorithms.\n> > */\n> >\n> > -/**\n> > - * \\struct IPAFrameContext\n> > - * \\brief Context for a frame\n> > - *\n> > - * The frame context stores data specific to a single frame processed by the\n> > - * IPA. Each frame processed by the IPA has a context associated with it,\n> > - * accessible through the IPAContext structure.\n> > - *\n> > - * Fields in the frame context should reflect values and controls\n> > - * associated with the specific frame as requested by the application, and\n> > - * as configured by the hardware. Fields can be read by algorithms to\n> > - * determine if they should update any specific action for this frame, and\n> > - * finally to update the metadata control lists when the frame is fully\n> > - * completed.\n> > - */\n> > -\n> > /**\n> > * \\struct IPAContext\n> > * \\brief Global IPA context data shared between all algorithms\n> > @@ -188,15 +172,15 @@ IPAFrameContext::IPAFrameContext() = default;\n> > /**\n> > * \\brief Construct a IPAFrameContext instance\n> > */\n> > -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> > -\t: frame(id), frameControls(reqControls)\n> > +IPAFrameContext::IPAFrameContext(const ControlList &reqControls)\n> > +\t: frameControls(reqControls)\n> > {\n> > \tsensor = {};\n> > }\n> >\n> > /**\n> > - * \\var IPAFrameContext::frame\n> > - * \\brief The frame number\n> > + * \\struct IPAFrameContext\n> > + * \\brief IPU3-specific FrameContext\n> > *\n> > * \\var IPAFrameContext::frameControls\n> > * \\brief Controls sent in by the application while queuing the request\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 42e11141d3a1..e8fc42769075 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -17,6 +17,8 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > +\n> > namespace libcamera {\n> >\n> > namespace ipa::ipu3 {\n> > @@ -76,16 +78,15 @@ struct IPAActiveState {\n> > \t} toneMapping;\n> > };\n> >\n> > -struct IPAFrameContext {\n> > +struct IPAFrameContext : public FrameContext {\n> \n> This could now be called IPU3FrameContext :)\n\nThere's a subsequent patch in the series that handles renames :-)\n\n> > \tIPAFrameContext();\n> > -\tIPAFrameContext(uint32_t id, const ControlList &reqControls);\n> > +\tIPAFrameContext(const ControlList &reqControls);\n> >\n> > \tstruct {\n> > \t\tuint32_t exposure;\n> > \t\tdouble gain;\n> > \t} sensor;\n> >\n> > -\tuint32_t frame;\n> > \tControlList frameControls;\n> > };\n> >\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index e5a763fd2b08..b1b23fd8f927 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -607,9 +607,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> >\n> > \tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> >\n> > -\tif (frameContext.frame != frame)\n> > -\t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> > -\n> > \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> >\n> > @@ -654,7 +651,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> > -\tcontext_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n> > +\tcontext_.frameContexts[frame % kMaxFrameContexts] = { controls };\n> \n> I understand this right that once moving to the FCQ the frame number\n> initialization and the check above will be there performed ?\n\nThat's right.\n\n> Should we allow then to have a constructor for the IPA-specific class\n> that accepts a frame number ?\n\nWhat would you use that for ?\n\n> Nits apart\n> Reviewed-by: Jacopo Mondi \n> \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 2C917C0DA4\n\tfor ;\n\tThu, 22 Sep 2022 19:55:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC7236219A;\n\tThu, 22 Sep 2022 21:55:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 357CC6219A\n\tfor ;\n\tThu, 22 Sep 2022 21:55:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 822FB6BE;\n\tThu, 22 Sep 2022 21:55:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663876511;\n\tbh=+m+qCO/8Yt1O6RC6hxuZEPlwC+MTkITIcTxiilSIhRA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=lby3+fCOR0aeLIX7GLgAsK/WCGlGQkBSJKsKhLuQ2e7myxtWo1J/xK2gmFl91RvA2\n\tVnW8CEEVm0iiccVjsLmfGf6OW8aMYjQVO1dlBc6P3OgfNS6bAPK4wEWGPgG0/37F7I\n\tY1+Rj17JYnBCi3EaZVT2469Yu6lYuNKGO4tS+GwXuGJGc48aFc/GmWm2u3E83RZGQG\n\ts8Cm/zI/64vcX2XMlPrcKH/afa2kMpnhL3g3bofCmBOTuI8w5iZX7GIyeG9unQnIIH\n\tPBOsuiPAuGfUb9+79wKL7vXdHTqFFcxwam4x0ilMDKWL/DqYIByTEotENY5oCObPxa\n\tlye6fY4liRByw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663876509;\n\tbh=+m+qCO/8Yt1O6RC6hxuZEPlwC+MTkITIcTxiilSIhRA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UOnr9ZaDdDgOptIOkZj5uorYiOhsv7HTPKoHP7m8DAoNlSfjq+THMbGchPgoou3Tf\n\t0W53zuKDREy6SJBXG0YuciJjlOaVeAVYaI3EzF434+QuOioeOjdjXTw1I7qTQg/PUn\n\tXYjEX9QB0DEUxtNchhvJEfhnke73aXQn7nrWgN0o="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UOnr9ZaD\"; dkim-atps=neutral","Date":"Thu, 22 Sep 2022 22:54:54 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>\n\t<20220921181030.oof6zaiwlej3dcwj@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220921181030.oof6zaiwlej3dcwj@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25107,"web_url":"https://patchwork.libcamera.org/comment/25107/","msgid":"<20220923072332.qjhti3lkvjnyanor@uno.localdomain>","date":"2022-09-23T07:23:32","subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Sep 22, 2022 at 10:54:54PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Sep 21, 2022 at 08:10:30PM +0200, Jacopo Mondi wrote:\n> > On Thu, Sep 08, 2022 at 04:41:37AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > Inherit from the base FrameContext class in the IPU3 IPAFrameContext.\n> > > This allows dropping the frame member, which is now stored in the base\n> > > class.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/ipa/ipu3/ipa_context.cpp | 24 ++++--------------------\n> > > src/ipa/ipu3/ipa_context.h | 7 ++++---\n> > > src/ipa/ipu3/ipu3.cpp | 5 +----\n> > > 3 files changed, 9 insertions(+), 27 deletions(-)\n> > >\n> > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > > index 13cdb835ef7f..9cfca0db3a0d 100644\n> > > --- a/src/ipa/ipu3/ipa_context.cpp\n> > > +++ b/src/ipa/ipu3/ipa_context.cpp\n> > > @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n> > > * most recently computed by the IPA algorithms.\n> > > */\n> > >\n> > > -/**\n> > > - * \\struct IPAFrameContext\n> > > - * \\brief Context for a frame\n> > > - *\n> > > - * The frame context stores data specific to a single frame processed by the\n> > > - * IPA. Each frame processed by the IPA has a context associated with it,\n> > > - * accessible through the IPAContext structure.\n> > > - *\n> > > - * Fields in the frame context should reflect values and controls\n> > > - * associated with the specific frame as requested by the application, and\n> > > - * as configured by the hardware. Fields can be read by algorithms to\n> > > - * determine if they should update any specific action for this frame, and\n> > > - * finally to update the metadata control lists when the frame is fully\n> > > - * completed.\n> > > - */\n> > > -\n> > > /**\n> > > * \\struct IPAContext\n> > > * \\brief Global IPA context data shared between all algorithms\n> > > @@ -188,15 +172,15 @@ IPAFrameContext::IPAFrameContext() = default;\n> > > /**\n> > > * \\brief Construct a IPAFrameContext instance\n> > > */\n> > > -IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> > > -\t: frame(id), frameControls(reqControls)\n> > > +IPAFrameContext::IPAFrameContext(const ControlList &reqControls)\n> > > +\t: frameControls(reqControls)\n> > > {\n> > > \tsensor = {};\n> > > }\n> > >\n> > > /**\n> > > - * \\var IPAFrameContext::frame\n> > > - * \\brief The frame number\n> > > + * \\struct IPAFrameContext\n> > > + * \\brief IPU3-specific FrameContext\n> > > *\n> > > * \\var IPAFrameContext::frameControls\n> > > * \\brief Controls sent in by the application while queuing the request\n> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > index 42e11141d3a1..e8fc42769075 100644\n> > > --- a/src/ipa/ipu3/ipa_context.h\n> > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > @@ -17,6 +17,8 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > namespace libcamera {\n> > >\n> > > namespace ipa::ipu3 {\n> > > @@ -76,16 +78,15 @@ struct IPAActiveState {\n> > > \t} toneMapping;\n> > > };\n> > >\n> > > -struct IPAFrameContext {\n> > > +struct IPAFrameContext : public FrameContext {\n> >\n> > This could now be called IPU3FrameContext :)\n>\n> There's a subsequent patch in the series that handles renames :-)\n>\n> > > \tIPAFrameContext();\n> > > -\tIPAFrameContext(uint32_t id, const ControlList &reqControls);\n> > > +\tIPAFrameContext(const ControlList &reqControls);\n> > >\n> > > \tstruct {\n> > > \t\tuint32_t exposure;\n> > > \t\tdouble gain;\n> > > \t} sensor;\n> > >\n> > > -\tuint32_t frame;\n> > > \tControlList frameControls;\n> > > };\n> > >\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index e5a763fd2b08..b1b23fd8f927 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -607,9 +607,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > >\n> > > \tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> > >\n> > > -\tif (frameContext.frame != frame)\n> > > -\t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> > > -\n> > > \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > > \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > >\n> > > @@ -654,7 +651,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > > {\n> > > \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> > > -\tcontext_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };\n> > > +\tcontext_.frameContexts[frame % kMaxFrameContexts] = { controls };\n> >\n> > I understand this right that once moving to the FCQ the frame number\n> > initialization and the check above will be there performed ?\n>\n> That's right.\n>\n> > Should we allow then to have a constructor for the IPA-specific class\n> > that accepts a frame number ?\n>\n> What would you use that for ?\n\nNot advocating for it, I was pointing out it is there\n>\n> > > \tIPAFrameContext();\n> > > -\tIPAFrameContext(uint32_t id, const ControlList &reqControls);\n> > > +\tIPAFrameContext(const ControlList &reqControls);\n\nBut I realize now the line has a '-' sign in front. One day I'll learn\nhow to read patches\n\n\n>\n> > Nits apart\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > }\n> > >\n> > > /**\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 6D7CFC0DA4\n\tfor ;\n\tFri, 23 Sep 2022 07:23:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB24E621BC;\n\tFri, 23 Sep 2022 09:23:36 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::224])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1783621BC\n\tfor ;\n\tFri, 23 Sep 2022 09:23:34 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 580D5E0005;\n\tFri, 23 Sep 2022 07:23:34 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663917816;\n\tbh=KYO57QM9FZWZsxQitq/FjjWtcTB+6HNF+ni+WOmJpr8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dNEYs1uzPZV0EZ1uogihN7sIdIuYypaDpGLKBSq4Pyv8OI53ztjwrrfb0BCbM+BIZ\n\td09W1Fdh20w1oTVCFmptN4rVOCIlw7iTE1PD/cUyQbgkW857QqgedzTX+A6pmiQ2K8\n\t/ogYcaohh6Ub39uxJNgXAjbImMwb2M66ONEW3LkZg7+VfKKdT4LNTyZqBONQlypaMt\n\tvIDXfhzbw42BlWMwdku+a0e+EhKDtjwIuacWrIWYjxtGLIEDXPu+QBsXt8rcvZIFCR\n\tZGTLMmFZHegDTBtnik9AacRNbToGwf0YPG7UrDdGDre1q3zYs6Je8hgWL1ZRGfu9ta\n\tRQMdynvzLXJtg==","Date":"Fri, 23 Sep 2022 09:23:32 +0200","To":"Laurent Pinchart ","Message-ID":"<20220923072332.qjhti3lkvjnyanor@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-10-laurent.pinchart@ideasonboard.com>\n\t<20220921181030.oof6zaiwlej3dcwj@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 09/32] ipa: ipu3: Use base\n\tFrameContext class","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]