[{"id":36743,"web_url":"https://patchwork.libcamera.org/comment/36743/","msgid":"","date":"2025-11-06T18:44:57","subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Fri, Oct 24, 2025 at 10:50:47AM +0200, Stefan Klug wrote:\n> For per frame control we want to tick the IPA by the sensor frame\n> sequence instead of the request frame sequence. This has the side effect\n> that the IPA must be able to cope with situations where a frame context\n> is required for a frame that was not queued before (computeParams is\n> called without a corresponding request) or processStats is called for an\n> unexpected sequence number (because a scratch buffer was used on kernel\n> side)\n>\n> Prepare for that by allowing the frame context to be initialized on demand.\n>\n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 2 ++\n> src/ipa/rkisp1/ipa_context.h | 2 ++\n> src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> 3 files changed, 10 insertions(+)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 399fb51be414..27109478c340 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -172,6 +172,8 @@ void Awb::queueRequest(IPAContext &context,\n> \tawbAlgo_->handleControls(controls);\n>\n> \tframeContext.awb.autoEnabled = awb.autoEnabled;\n> +\tframeContext.awb.gains = awb.automatic.gains;\n> +\tframeContext.awb.temperatureK = awb.automatic.temperatureK;\n\nunrelated ?\n\n>\n> \tif (awb.autoEnabled)\n> \t\treturn;\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index f85a130d9c23..185951fb8286 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -214,6 +214,8 @@ struct IPAFrameContext : public FrameContext {\n> \t\tdouble strength;\n> \t\tdouble gain;\n> \t} wdr;\n> +\n> +\tbool initialised;\n\nAs already pointed out, the base struct already has an initialised\nmember, an indication that something could maybe be improved.\n\nthe here introduced initializeFrameContext()\nis called from 4 places:\n\n- start() [where you should call alloc() imho; with valid controls]\n- queueRequest() [which calls alloc(); with valid controls]\n- computeParams() [which calls get(); without controls]\n- processStats() [which calls get(); without controls)\n\nIf I'm not mistaken\n\n- start() will always be called before queueRequest and the frame\n context won't be initialized, but controls need to be queued to\n algorithms\n- queueRequest() will be called with valid controls, but the frame\n context could already be initialized by the FCQ\n- computeParams/processStats won't have valid controls but the frame\n context might already be initialised. Here I'm not sure if we need\n to even try to call algo->queueRequest() because there won't be\n controls associated with this frame context.\n\nAll in all, I'm wondering if initializeFrameContext() is not actually about only\nqueueing controls to the algos, and we can even skip calling it from\ncomputeParams/processStats for which the correct initialisation of the frame\ncontext is already performed by the FCQueue.\n\n\n> };\n>\n> struct IPAContext {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 01b30c947a0a..23d80bc43c5d 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -343,6 +343,10 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> \t\t\t\t IPAFrameContext &frameContext,\n> \t\t\t\t const ControlList &controls)\n> {\n> +\tif (frameContext.initialised)\n> +\t\treturn;\n> +\n> +\tframeContext.initialised = true;\n> \tfor (auto const &a : algorithms()) {\n> \t\tAlgorithm *algo = static_cast(a.get());\n> \t\tif (algo->disabled_)\n> @@ -354,6 +358,7 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)\n> {\n> \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tinitializeFrameContext(frame, frameContext, {});\n>\n> \t/*\n> \t * \\todo: This needs discussion. In raw mode, computeParams is\n> @@ -383,6 +388,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t const ControlList &sensorControls)\n> {\n> \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tinitializeFrameContext(frame, frameContext, {});\n>\n> \t/*\n> \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> --\n> 2.48.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 0EEBDC3241\n\tfor ;\n\tThu, 6 Nov 2025 18:45:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0391D60A85;\n\tThu, 6 Nov 2025 19:45:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D5C6606E6\n\tfor ;\n\tThu, 6 Nov 2025 19:45:01 +0100 (CET)","from ideasonboard.com (mob-5-90-141-71.net.vodafone.it\n\t[5.90.141.71])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A77FA6AF;\n\tThu, 6 Nov 2025 19:43:05 +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=\"DutNjWpz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762454585;\n\tbh=drEznRnU4PQfHX5SjiSMeHRD1HR4+QLxvy9Nw1K/2fs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DutNjWpzzmEonPg3wAKNewDEFb2+K1PGDtouvTjlhtxQZQeTTPp5hfVPeDO1Jl5l4\n\tMb2bPGsDrIWmWDhm9HkNWGgjtZkfyP7Sm/TNn/diVSnnUajiqUZ5pQ1jQUzadMyHz1\n\tfndz+n9u2aYGsvdA6vntmKRp/GqvlCinWDVxiDgE=","Date":"Thu, 6 Nov 2025 19:44:57 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","Message-ID":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-24-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20251024085130.995967-24-stefan.klug@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":36744,"web_url":"https://patchwork.libcamera.org/comment/36744/","msgid":"","date":"2025-11-07T08:02:48","subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi again Stefan\n\nOn Thu, Nov 06, 2025 at 07:44:57PM +0100, Jacopo Mondi wrote:\n> Hi Stefan\n>\n> On Fri, Oct 24, 2025 at 10:50:47AM +0200, Stefan Klug wrote:\n> > For per frame control we want to tick the IPA by the sensor frame\n> > sequence instead of the request frame sequence. This has the side effect\n> > that the IPA must be able to cope with situations where a frame context\n> > is required for a frame that was not queued before (computeParams is\n> > called without a corresponding request) or processStats is called for an\n> > unexpected sequence number (because a scratch buffer was used on kernel\n> > side)\n> >\n> > Prepare for that by allowing the frame context to be initialized on demand.\n> >\n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++\n> > src/ipa/rkisp1/ipa_context.h | 2 ++\n> > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > 3 files changed, 10 insertions(+)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index 399fb51be414..27109478c340 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -172,6 +172,8 @@ void Awb::queueRequest(IPAContext &context,\n> > \tawbAlgo_->handleControls(controls);\n> >\n> > \tframeContext.awb.autoEnabled = awb.autoEnabled;\n> > +\tframeContext.awb.gains = awb.automatic.gains;\n> > +\tframeContext.awb.temperatureK = awb.automatic.temperatureK;\n>\n> unrelated ?\n>\n> >\n> > \tif (awb.autoEnabled)\n> > \t\treturn;\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index f85a130d9c23..185951fb8286 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -214,6 +214,8 @@ struct IPAFrameContext : public FrameContext {\n> > \t\tdouble strength;\n> > \t\tdouble gain;\n> > \t} wdr;\n> > +\n> > +\tbool initialised;\n>\n> As already pointed out, the base struct already has an initialised\n> member, an indication that something could maybe be improved.\n>\n> the here introduced initializeFrameContext()\n> is called from 4 places:\n>\n> - start() [where you should call alloc() imho; with valid controls]\n> - queueRequest() [which calls alloc(); with valid controls]\n> - computeParams() [which calls get(); without controls]\n> - processStats() [which calls get(); without controls)\n>\n> If I'm not mistaken\n>\n> - start() will always be called before queueRequest and the frame\n> context won't be initialized, but controls need to be queued to\n> algorithms\n> - queueRequest() will be called with valid controls, but the frame\n> context could already be initialized by the FCQ\n> - computeParams/processStats won't have valid controls but the frame\n> context might already be initialised. Here I'm not sure if we need\n\nthis should be \"might have not been initialised\" as we're dealing with\nthe case where prepare/process for frame X are called before the\ncorresponding request X has been queued.\n\n> to even try to call algo->queueRequest() because there won't be\n> controls associated with this frame context.\n>\n> All in all, I'm wondering if initializeFrameContext() is not actually about only\n> queueing controls to the algos, and we can even skip calling it from\n> computeParams/processStats for which the correct initialisation of the frame\n> context is already performed by the FCQueue.\n\nI slept over this a bit.. I think we should slightly re-work the FCQ\nto make this a bit nicer.\n\nI think the first thing to do would be to propagate to IPAs the\n\"error\" conditions from FCQueue::alloc() and ::get().\n\nThis would help you detect if an unexpected sequence of events has\nhappened and act accordingly.\n\nLet's leave out start() for a bit (I would like to clarify, here on in\nthe other patch if start() should call alloc() or not, but that's\nunrelated). Let's only consider queueRequest()/process()/prepare().\n\nIn normal operating conditions, with user queueing requests fast\nenough to sustain the sensor frame rate, as we now clock the IPA with\nframes from the sensor, and we have no guarantee that requests will be\nqueued fast enough to sustain this.\n\n\n- IPA::queueRequest(Request:X)\n - FCQueue::alloc(X):\n - frame context already initialized\n - This means that prepare/process have already been called\n - The Request is late and the corresponding frame has already\n been produced\n - We need to accummulate the Request's controls in the list of\n controls to apply to the \"next\" algo->queueRequest() loop\n - frame context not initialized\n - Normal operating conditions\n - Call algo->queueRequest(request->controls())\n\n- computeParams(Frame:X)\n - FCQeueue::get(X)\n - frame context already initialized\n - Normal operating conditions\n - call algos->prepare()\n - frame context not initialized\n - User request underrun\n - if any pending controls for late requests are present we\n should handle them: call algo->queueRequest() to provide\n them the pending controls we have accumulated\n\n- processStats(Frame:X)\n - same as computeParams(X)\n\nThis is what I think it is implemented in this series ? Do you agree\nor I missed anything ?\n\nIf that's the case, wouldn't it be nicer if we could do\n\n IPA::queueRequest(request) {\n auto { fc, err } =\n context_.frameContexts.alloc(request->sequece);\n if (err == REQUEST_UNDERRUN) {\n /* Accumulate controls in the list of pending controls\n * because the request is late */\n pendingControls.merge(request->controls);\n }\n\n queueRequestToAlgo(request->controls);\n }\n\n\n IPA::computeParams(frame) {\n auto { fc, err } = context_.frameContexts.get(frame);\n if (err == REQUEST_UNDERRUN) {\n queueRequestToAlgo(pendingControls);\n }\n\n for (algo : algos) {\n algo->prepare();\n }\n\n setSensorControls();\n }\n\n IPA::processStats(frame, sensorControls) {\n auto { fc, err } = context_.frameContexts.get(frame);\n if (err == REQUEST_UNDERRUN) {\n queueRequestToAlgo(pendingControls);\n }\n\n updateSensorControls(sensorControls);\n\n for (algo : algos) {\n algo->process();\n }\n\n metadataReady.emit();\n }\n\nThe only error condition from FCQueue is then a request underrun\n(apart from the Fatal 'overwrite' error), which corresponds to the two\nconditions now demoted to Debug messages in alloc() and get()\n\nin case of underruns we handle the list of pending controls, either by\naccummulating them in queueRequest() or by sending them to algos in\ncomputeParams and processStats.\n\nDoes this match the mental model you have implemented in this series\nor have I missed some parts ?\n\nOne part it is not totally clear to me in this series is if\ncomputeParams/processStats for a frame will always be called in\nsequence or not, and if not, if there's anything to do to address this\ncondition.\n\nThanks\n j\n\n>\n> > };\n> >\n> > struct IPAContext {\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 01b30c947a0a..23d80bc43c5d 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -343,6 +343,10 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > \t\t\t\t IPAFrameContext &frameContext,\n> > \t\t\t\t const ControlList &controls)\n> > {\n> > +\tif (frameContext.initialised)\n> > +\t\treturn;\n> > +\n> > +\tframeContext.initialised = true;\n> > \tfor (auto const &a : algorithms()) {\n> > \t\tAlgorithm *algo = static_cast(a.get());\n> > \t\tif (algo->disabled_)\n> > @@ -354,6 +358,7 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)\n> > {\n> > \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tinitializeFrameContext(frame, frameContext, {});\n> >\n> > \t/*\n> > \t * \\todo: This needs discussion. In raw mode, computeParams is\n> > @@ -383,6 +388,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,\n> > \t\t\t const ControlList &sensorControls)\n> > {\n> > \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tinitializeFrameContext(frame, frameContext, {});\n> >\n> > \t/*\n> > \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > --\n> > 2.48.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 E6DD8BDE4C\n\tfor ;\n\tFri, 7 Nov 2025 08:02:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E471606D5;\n\tFri, 7 Nov 2025 09:02:54 +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 5FC84606D5\n\tfor ;\n\tFri, 7 Nov 2025 09:02:52 +0100 (CET)","from ideasonboard.com (mob-5-90-142-135.net.vodafone.it\n\t[5.90.142.135])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4B68216D7;\n\tFri, 7 Nov 2025 09:00:56 +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=\"oVgNl+tJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762502456;\n\tbh=uqufBkbYjZ0LtnHgjQP8MqjxONsDFGfWZs3rEVZBgys=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oVgNl+tJ85uQfreb4KwQC655v05msIuX/ZnFjTtjqvbfomAqbJ6oAhV+ZLbkexQJB\n\tqRDEcfwJKQoEFob+/4Elxef55FiPTiP8c7rVsudxGB5aY39gPIMnKpJ5VyqdI4kT4e\n\ta2H/vdvZihIyc21pcDvEgcYmZXlI1w85UvevlepA=","Date":"Fri, 7 Nov 2025 09:02:48 +0100","From":"Jacopo Mondi ","To":"Jacopo Mondi ","Cc":"Stefan Klug , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","Message-ID":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-24-stefan.klug@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\" "}},{"id":36749,"web_url":"https://patchwork.libcamera.org/comment/36749/","msgid":"<176251207553.509848.11267537510627837019@localhost>","date":"2025-11-07T10:41:15","subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nThanks for looking deep into this.\n\nQuoting Jacopo Mondi (2025-11-07 09:02:48)\n> Hi again Stefan\n> \n> On Thu, Nov 06, 2025 at 07:44:57PM +0100, Jacopo Mondi wrote:\n> > Hi Stefan\n> >\n> > On Fri, Oct 24, 2025 at 10:50:47AM +0200, Stefan Klug wrote:\n> > > For per frame control we want to tick the IPA by the sensor frame\n> > > sequence instead of the request frame sequence. This has the side effect\n> > > that the IPA must be able to cope with situations where a frame context\n> > > is required for a frame that was not queued before (computeParams is\n> > > called without a corresponding request) or processStats is called for an\n> > > unexpected sequence number (because a scratch buffer was used on kernel\n> > > side)\n> > >\n> > > Prepare for that by allowing the frame context to be initialized on demand.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++\n> > > src/ipa/rkisp1/ipa_context.h | 2 ++\n> > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > 3 files changed, 10 insertions(+)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index 399fb51be414..27109478c340 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -172,6 +172,8 @@ void Awb::queueRequest(IPAContext &context,\n> > > awbAlgo_->handleControls(controls);\n> > >\n> > > frameContext.awb.autoEnabled = awb.autoEnabled;\n> > > + frameContext.awb.gains = awb.automatic.gains;\n> > > + frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> >\n> > unrelated ?\n\nOoops :-)\n\n> >\n> > >\n> > > if (awb.autoEnabled)\n> > > return;\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index f85a130d9c23..185951fb8286 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -214,6 +214,8 @@ struct IPAFrameContext : public FrameContext {\n> > > double strength;\n> > > double gain;\n> > > } wdr;\n> > > +\n> > > + bool initialised;\n> >\n> > As already pointed out, the base struct already has an initialised\n> > member, an indication that something could maybe be improved.\n> >\n> > the here introduced initializeFrameContext()\n> > is called from 4 places:\n> >\n> > - start() [where you should call alloc() imho; with valid controls]\n> > - queueRequest() [which calls alloc(); with valid controls]\n> > - computeParams() [which calls get(); without controls]\n> > - processStats() [which calls get(); without controls)\n> >\n> > If I'm not mistaken\n> >\n> > - start() will always be called before queueRequest and the frame\n> > context won't be initialized, but controls need to be queued to\n> > algorithms\n> > - queueRequest() will be called with valid controls, but the frame\n> > context could already be initialized by the FCQ\n> > - computeParams/processStats won't have valid controls but the frame\n> > context might already be initialised. Here I'm not sure if we need\n> \n> this should be \"might have not been initialised\" as we're dealing with\n> the case where prepare/process for frame X are called before the\n> corresponding request X has been queued.\n> \n> > to even try to call algo->queueRequest() because there won't be\n> > controls associated with this frame context.\n> >\n> > All in all, I'm wondering if initializeFrameContext() is not actually about only\n> > queueing controls to the algos, and we can even skip calling it from\n> > computeParams/processStats for which the correct initialisation of the frame\n> > context is already performed by the FCQueue.\n\nI think I miss something here. I don't think we can skip calling\ninitializeFrameContext() for computeParams/processStats because it\nensures that algo->queueRequest() is called for the FC. I think\nalgo->queueRequest() should be renamed to algo->initializeRequest()\nwhich is the actual intent. The FCQueue only zeroes the frame context\nbut actual initialization happens in algo->queueRequest().\n\n> \n> I slept over this a bit.. I think we should slightly re-work the FCQ\n> to make this a bit nicer.\n\nI completely agree. I'm not happy with the design of the FCQueue. I\ntried to apply modifications in baby steps to ensure I didn't miss\nanything in the big picture. But there is still room for further\nimprovements.\n\n> \n> I think the first thing to do would be to propagate to IPAs the\n> \"error\" conditions from FCQueue::alloc() and ::get().\n> \n> This would help you detect if an unexpected sequence of events has\n> happened and act accordingly.\n> \n> Let's leave out start() for a bit (I would like to clarify, here on in\n> the other patch if start() should call alloc() or not, but that's\n> unrelated). Let's only consider queueRequest()/process()/prepare().\n> \n> In normal operating conditions, with user queueing requests fast\n> enough to sustain the sensor frame rate, as we now clock the IPA with\n> frames from the sensor, and we have no guarantee that requests will be\n> queued fast enough to sustain this.\n> \n> \n> - IPA::queueRequest(Request:X)\n> - FCQueue::alloc(X):\n> - frame context already initialized\n> - This means that prepare/process have already been called\n> - The Request is late and the corresponding frame has already\n> been produced\n> - We need to accummulate the Request's controls in the list of\n> controls to apply to the \"next\" algo->queueRequest() loop\n> - frame context not initialized\n> - Normal operating conditions\n> - Call algo->queueRequest(request->controls())\n> \n> - computeParams(Frame:X)\n> - FCQeueue::get(X)\n> - frame context already initialized\n> - Normal operating conditions\n> - call algos->prepare()\n> - frame context not initialized\n> - User request underrun\n> - if any pending controls for late requests are present we\n> should handle them: call algo->queueRequest() to provide\n> them the pending controls we have accumulated\n> \n> - processStats(Frame:X)\n> - same as computeParams(X)\n> \n> This is what I think it is implemented in this series ? Do you agree\n> or I missed anything ?\n\nI think that's it, yes.\n\n> \n> If that's the case, wouldn't it be nicer if we could do\n> \n> IPA::queueRequest(request) {\n> auto { fc, err } =\n> context_.frameContexts.alloc(request->sequece);\n\nThis is a tiny bit misleading as it is not the request sequence, but\nsensor frame and controls.\n\n> if (err == REQUEST_UNDERRUN) {\n> /* Accumulate controls in the list of pending controls\n> * because the request is late */\n> pendingControls.merge(request->controls);\n> }\n> \n> queueRequestToAlgo(request->controls);\n> }\n> \n> \n> IPA::computeParams(frame) {\n> auto { fc, err } = context_.frameContexts.get(frame);\n> if (err == REQUEST_UNDERRUN) {\n> queueRequestToAlgo(pendingControls);\n> }\n> \n> for (algo : algos) {\n> algo->prepare();\n> }\n> \n> setSensorControls();\n> }\n> \n> IPA::processStats(frame, sensorControls) {\n> auto { fc, err } = context_.frameContexts.get(frame);\n> if (err == REQUEST_UNDERRUN) {\n> queueRequestToAlgo(pendingControls);\n> }\n> \n> updateSensorControls(sensorControls);\n> \n> for (algo : algos) {\n> algo->process();\n> }\n> \n> metadataReady.emit();\n> }\n> \n> The only error condition from FCQueue is then a request underrun\n> (apart from the Fatal 'overwrite' error), which corresponds to the two\n> conditions now demoted to Debug messages in alloc() and get()\n> \n> in case of underruns we handle the list of pending controls, either by\n> accummulating them in queueRequest() or by sending them to algos in\n> computeParams and processStats.\n> \n> Does this match the mental model you have implemented in this series\n> or have I missed some parts ?\n\nI think you got that all right. I'm not sure if I like that style better\nas it adds more control logic into computeParams()/processStats() and\nassumes that the FCQueue has knowledge of the expected order of actions.\nBut I agree that you could get rid of the outer initialized member in\nthat case.\n\nI'd like the FCQueue to be as simple as possible. Continuing on your\nthoughts what about removing the FCQueue::alloc all together?\n\nIf we replace alloc+get by a single get():\n\nauto { fc, did_allocate } = context_.frameContexts.get(frame);\nwe could move all the debug messages out of the FCQueue into the IPA.\n\nWe can then pass the did_allocate to initializeFrameContext() which\nwould be close to your proposal or stick to the FC.initialized member\nwhich I somehow still like better. But that is a technical nuance.\n\n> \n> One part it is not totally clear to me in this series is if\n> computeParams/processStats for a frame will always be called in\n> sequence or not, and if not, if there's anything to do to address this\n> condition.\n\nI wouldn't make much guarantees here. But I can't come up with a\ncondition where processStats is called without a prior computeParams. A\ncomputeParams() on a frame after processStats was called on that frame\nshould never happen.\n\nBest regards,\nStefan\n\n> \n> Thanks\n> j\n> \n> >\n> > > };\n> > >\n> > > struct IPAContext {\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index 01b30c947a0a..23d80bc43c5d 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -343,6 +343,10 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > IPAFrameContext &frameContext,\n> > > const ControlList &controls)\n> > > {\n> > > + if (frameContext.initialised)\n> > > + return;\n> > > +\n> > > + frameContext.initialised = true;\n> > > for (auto const &a : algorithms()) {\n> > > Algorithm *algo = static_cast(a.get());\n> > > if (algo->disabled_)\n> > > @@ -354,6 +358,7 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)\n> > > {\n> > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > + initializeFrameContext(frame, frameContext, {});\n> > >\n> > > /*\n> > > * \\todo: This needs discussion. In raw mode, computeParams is\n> > > @@ -383,6 +388,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,\n> > > const ControlList &sensorControls)\n> > > {\n> > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > + initializeFrameContext(frame, frameContext, {});\n> > >\n> > > /*\n> > > * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > > --\n> > > 2.48.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 3C5C1C3241\n\tfor ;\n\tFri, 7 Nov 2025 10:41:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36D5E60A8B;\n\tFri, 7 Nov 2025 11:41:19 +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 1CDF5606D5\n\tfor ;\n\tFri, 7 Nov 2025 11:41:18 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:8809:eccd:216c:b9a0])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 287A3F89;\n\tFri, 7 Nov 2025 11:39:22 +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=\"uH5rsDhF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762511962;\n\tbh=jooVEUcgiqH9uURIE5s32oSkuLz6bikIcgM3kIokJXE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uH5rsDhF9ALVgq95VQDQUyxHkP/+oP/HaNa7HTzboSEtVAZxrzDO8xG2UeYe+TCaV\n\tpW3QpXd7idnMyyNtfrsSbN3JnIWQnnDM3WiXPgZqS3HDFJXcW/thm2p7uiSDsubRVz\n\tUnySKG6GK6ZNQcJFfwnOGgMqe0dOumPDJ8FBaIxc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-24-stefan.klug@ideasonboard.com>\n\t\n\t","Subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","To":"Jacopo Mondi ","Date":"Fri, 07 Nov 2025 11:41:15 +0100","Message-ID":"<176251207553.509848.11267537510627837019@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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":36755,"web_url":"https://patchwork.libcamera.org/comment/36755/","msgid":"","date":"2025-11-08T10:56:53","subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Fri, Nov 07, 2025 at 11:41:15AM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Thanks for looking deep into this.\n>\n> Quoting Jacopo Mondi (2025-11-07 09:02:48)\n> > Hi again Stefan\n> >\n> > On Thu, Nov 06, 2025 at 07:44:57PM +0100, Jacopo Mondi wrote:\n> > > Hi Stefan\n> > >\n> > > On Fri, Oct 24, 2025 at 10:50:47AM +0200, Stefan Klug wrote:\n> > > > For per frame control we want to tick the IPA by the sensor frame\n> > > > sequence instead of the request frame sequence. This has the side effect\n> > > > that the IPA must be able to cope with situations where a frame context\n> > > > is required for a frame that was not queued before (computeParams is\n> > > > called without a corresponding request) or processStats is called for an\n> > > > unexpected sequence number (because a scratch buffer was used on kernel\n> > > > side)\n> > > >\n> > > > Prepare for that by allowing the frame context to be initialized on demand.\n> > > >\n> > > > Signed-off-by: Stefan Klug \n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++\n> > > > src/ipa/rkisp1/ipa_context.h | 2 ++\n> > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > 3 files changed, 10 insertions(+)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > index 399fb51be414..27109478c340 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > @@ -172,6 +172,8 @@ void Awb::queueRequest(IPAContext &context,\n> > > > awbAlgo_->handleControls(controls);\n> > > >\n> > > > frameContext.awb.autoEnabled = awb.autoEnabled;\n> > > > + frameContext.awb.gains = awb.automatic.gains;\n> > > > + frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> > >\n> > > unrelated ?\n>\n> Ooops :-)\n>\n> > >\n> > > >\n> > > > if (awb.autoEnabled)\n> > > > return;\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > index f85a130d9c23..185951fb8286 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > @@ -214,6 +214,8 @@ struct IPAFrameContext : public FrameContext {\n> > > > double strength;\n> > > > double gain;\n> > > > } wdr;\n> > > > +\n> > > > + bool initialised;\n> > >\n> > > As already pointed out, the base struct already has an initialised\n> > > member, an indication that something could maybe be improved.\n> > >\n> > > the here introduced initializeFrameContext()\n> > > is called from 4 places:\n> > >\n> > > - start() [where you should call alloc() imho; with valid controls]\n> > > - queueRequest() [which calls alloc(); with valid controls]\n> > > - computeParams() [which calls get(); without controls]\n> > > - processStats() [which calls get(); without controls)\n> > >\n> > > If I'm not mistaken\n> > >\n> > > - start() will always be called before queueRequest and the frame\n> > > context won't be initialized, but controls need to be queued to\n> > > algorithms\n> > > - queueRequest() will be called with valid controls, but the frame\n> > > context could already be initialized by the FCQ\n> > > - computeParams/processStats won't have valid controls but the frame\n> > > context might already be initialised. Here I'm not sure if we need\n> >\n> > this should be \"might have not been initialised\" as we're dealing with\n> > the case where prepare/process for frame X are called before the\n> > corresponding request X has been queued.\n> >\n> > > to even try to call algo->queueRequest() because there won't be\n> > > controls associated with this frame context.\n> > >\n> > > All in all, I'm wondering if initializeFrameContext() is not actually about only\n> > > queueing controls to the algos, and we can even skip calling it from\n> > > computeParams/processStats for which the correct initialisation of the frame\n> > > context is already performed by the FCQueue.\n>\n> I think I miss something here. I don't think we can skip calling\n> initializeFrameContext() for computeParams/processStats because it\n> ensures that algo->queueRequest() is called for the FC. I think\n\nRight, I think I realized that in my last reply\n\n> algo->queueRequest() should be renamed to algo->initializeRequest()\n> which is the actual intent. The FCQueue only zeroes the frame context\n> but actual initialization happens in algo->queueRequest().\n>\n> >\n> > I slept over this a bit.. I think we should slightly re-work the FCQ\n> > to make this a bit nicer.\n>\n> I completely agree. I'm not happy with the design of the FCQueue. I\n> tried to apply modifications in baby steps to ensure I didn't miss\n> anything in the big picture. But there is still room for further\n> improvements.\n>\n> >\n> > I think the first thing to do would be to propagate to IPAs the\n> > \"error\" conditions from FCQueue::alloc() and ::get().\n> >\n> > This would help you detect if an unexpected sequence of events has\n> > happened and act accordingly.\n> >\n> > Let's leave out start() for a bit (I would like to clarify, here on in\n> > the other patch if start() should call alloc() or not, but that's\n> > unrelated). Let's only consider queueRequest()/process()/prepare().\n> >\n> > In normal operating conditions, with user queueing requests fast\n> > enough to sustain the sensor frame rate, as we now clock the IPA with\n> > frames from the sensor, and we have no guarantee that requests will be\n> > queued fast enough to sustain this.\n> >\n> >\n> > - IPA::queueRequest(Request:X)\n> > - FCQueue::alloc(X):\n> > - frame context already initialized\n> > - This means that prepare/process have already been called\n> > - The Request is late and the corresponding frame has already\n> > been produced\n> > - We need to accummulate the Request's controls in the list of\n> > controls to apply to the \"next\" algo->queueRequest() loop\n> > - frame context not initialized\n> > - Normal operating conditions\n> > - Call algo->queueRequest(request->controls())\n> >\n> > - computeParams(Frame:X)\n> > - FCQeueue::get(X)\n> > - frame context already initialized\n> > - Normal operating conditions\n> > - call algos->prepare()\n> > - frame context not initialized\n> > - User request underrun\n> > - if any pending controls for late requests are present we\n> > should handle them: call algo->queueRequest() to provide\n> > them the pending controls we have accumulated\n> >\n> > - processStats(Frame:X)\n> > - same as computeParams(X)\n> >\n> > This is what I think it is implemented in this series ? Do you agree\n> > or I missed anything ?\n>\n> I think that's it, yes.\n>\n> >\n> > If that's the case, wouldn't it be nicer if we could do\n> >\n> > IPA::queueRequest(request) {\n> > auto { fc, err } =\n> > context_.frameContexts.alloc(request->sequece);\n>\n> This is a tiny bit misleading as it is not the request sequence, but\n> sensor frame and controls.\n>\n\nThat's something else that I think we should better clarify.\n\nParams and stats have the frame sequence as it comes from the kernel\nas identifier.\n\nRequest has a sequence number that is unrelated to the frame sequence\nnumber.\n\nI have not absorbed patch 27 yet, but I presume that's where you\nassociated a request with the most up to date frame sequence ?\n\nif that's not the case, how can requests be associated with frame\nnumbers as the two sequences are not related in any way ?\n\n\n> > if (err == REQUEST_UNDERRUN) {\n> > /* Accumulate controls in the list of pending controls\n> > * because the request is late */\n> > pendingControls.merge(request->controls);\n> > }\n> >\n> > queueRequestToAlgo(request->controls);\n> > }\n> >\n> >\n> > IPA::computeParams(frame) {\n> > auto { fc, err } = context_.frameContexts.get(frame);\n> > if (err == REQUEST_UNDERRUN) {\n> > queueRequestToAlgo(pendingControls);\n> > }\n> >\n> > for (algo : algos) {\n> > algo->prepare();\n> > }\n> >\n> > setSensorControls();\n> > }\n> >\n> > IPA::processStats(frame, sensorControls) {\n> > auto { fc, err } = context_.frameContexts.get(frame);\n> > if (err == REQUEST_UNDERRUN) {\n> > queueRequestToAlgo(pendingControls);\n> > }\n> >\n> > updateSensorControls(sensorControls);\n> >\n> > for (algo : algos) {\n> > algo->process();\n> > }\n> >\n> > metadataReady.emit();\n> > }\n> >\n> > The only error condition from FCQueue is then a request underrun\n> > (apart from the Fatal 'overwrite' error), which corresponds to the two\n> > conditions now demoted to Debug messages in alloc() and get()\n> >\n> > in case of underruns we handle the list of pending controls, either by\n> > accummulating them in queueRequest() or by sending them to algos in\n> > computeParams and processStats.\n> >\n> > Does this match the mental model you have implemented in this series\n> > or have I missed some parts ?\n>\n> I think you got that all right. I'm not sure if I like that style better\n> as it adds more control logic into computeParams()/processStats() and\n> assumes that the FCQueue has knowledge of the expected order of actions.\n\nMy main goal here is to push as much of the design in the common\nlibipa part and make the IPA module as easy to implement as possible.\n\nThe amount of logic implemented in\nIPARkISP1::initializeFrameContext() in this series is quite dense\n\n\tif (frameContext.initialised) {\n\t\tif (!controls.empty()) {\n\t\t}\n\t\treturn;\n\t}\n\n\tconst ControlList *controls2 = &controls;\n\tif (!controlsToApply_.empty()) {\n\t}\n\n\tframeContext.initialised = true;\n\tfor (auto const &a : algorithms()) {\n\t\tAlgorithm *algo = static_cast(a.get());\n\t\tif (algo->disabled_)\n\t\t\tcontinue;\n\t\talgo->queueRequest(context_, frame, frameContext, *controls2);\n\t}\n\n\tcontrolsToApply_.clear();\n\nAnd will have to be verbatim replicated in each IPA (I guess).\n\nI would be curious to check if the above proposed model which\nrevolves around the concept of queuing controls to the algos with a\nsingle queueRequestToAlgo() function that has to be called by\nprepare()/process() only in case of FCQueue error would result in less\nlogic to replicate in other IPAs.\n\n\n> But I agree that you could get rid of the outer initialized member in\n> that case.\n>\n> I'd like the FCQueue to be as simple as possible. Continuing on your\n\nehehe I'm fine paying the price of a more complex FCQueue if the logic\nin the single IPAs is reduced :)\n\n> thoughts what about removing the FCQueue::alloc all together?\n>\n> If we replace alloc+get by a single get():\n>\n> auto { fc, did_allocate } = context_.frameContexts.get(frame);\n> we could move all the debug messages out of the FCQueue into the IPA.\n\nI considered this as well.\n\nHowever alloc() and get() have different semantics in my opinion,\nmostly around the idea that alloc() should be called before get() in\n\"normal\" operations.\n\nAll our PFC designs revolves around the idea that there is a normal\noperating condition where requests are queued fast enough to sustain\nthe frame rate. In case this doesn't happen we have to handle an\n\"underrun\" as an error condition. Now, we can re-discuss this, but\nseeing that in case of underrun the IPA has to take counter-measures\nto either to accumulate controls for late requests or feed the algos\nwith any pending controls for early frames [*] makes me think as these\ntwo as \"error\" conditions.\n\n[*] I quite like the idea of collapsing the FCQueue error to a single\nunderrun condition. In the end a late request at queueRequest() time\nmeans we had a computeParams() for a frame for which the application\nhas not supplied a Request (yet). And the error paths are\neffectively the ones described above:\n - late request: accumulate the controls in a pendingControls_ list\n - early computeParams(): queue any pendingControls_ to algos to\n maintain their status up-to-date with users' request\n\nIn the end, if we have an early computeParams() we'll always have a late\nqueueRequest() and we can only mitigate this by pushing the missed\ncontrols to algos as soon as we can. There might be an argument here\nif a late queueRequest() should push pendingControls_ (before merging\nits own controls there)\n\n>\n> We can then pass the did_allocate to initializeFrameContext() which\n> would be close to your proposal or stick to the FC.initialized member\n> which I somehow still like better. But that is a technical nuance.\n>\n> >\n> > One part it is not totally clear to me in this series is if\n> > computeParams/processStats for a frame will always be called in\n> > sequence or not, and if not, if there's anything to do to address this\n> > condition.\n>\n> I wouldn't make much guarantees here. But I can't come up with a\n> condition where processStats is called without a prior computeParams. A\n\nIf a pipeline is designed correctly, this shouldn't happen, right ?\n\nWe can even assert if we detect this condition and check if it ever\nhappens at runtime ?\n\n> computeParams() on a frame after processStats was called on that frame\n> should never happen.\n>\n> Best regards,\n> Stefan\n>\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > > };\n> > > >\n> > > > struct IPAContext {\n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index 01b30c947a0a..23d80bc43c5d 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -343,6 +343,10 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > > IPAFrameContext &frameContext,\n> > > > const ControlList &controls)\n> > > > {\n> > > > + if (frameContext.initialised)\n> > > > + return;\n> > > > +\n> > > > + frameContext.initialised = true;\n> > > > for (auto const &a : algorithms()) {\n> > > > Algorithm *algo = static_cast(a.get());\n> > > > if (algo->disabled_)\n> > > > @@ -354,6 +358,7 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > > void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)\n> > > > {\n> > > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > + initializeFrameContext(frame, frameContext, {});\n> > > >\n> > > > /*\n> > > > * \\todo: This needs discussion. In raw mode, computeParams is\n> > > > @@ -383,6 +388,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,\n> > > > const ControlList &sensorControls)\n> > > > {\n> > > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > + initializeFrameContext(frame, frameContext, {});\n> > > >\n> > > > /*\n> > > > * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > > > --\n> > > > 2.48.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 B7B2BC3241\n\tfor ;\n\tSat, 8 Nov 2025 10:56:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE49560A8B;\n\tSat, 8 Nov 2025 11:56:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FC9460856\n\tfor ;\n\tSat, 8 Nov 2025 11:56:56 +0100 (CET)","from ideasonboard.com (mob-5-90-142-135.net.vodafone.it\n\t[5.90.142.135])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B981A55A;\n\tSat, 8 Nov 2025 11:54:59 +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=\"aB1twyGI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762599300;\n\tbh=vT163NzKaYYNab7/fqiUS2RMXTGaNoyXzd1SGOmbJDI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aB1twyGIJhmTQshidCHkOlq5K+Hf1lZzFWbNdT/+AG1g8beb4wYrrsTKpPZKEJYJB\n\t0ro6sbBoDK8YAvsVHyPHZgDWVKR1XdpQ+Jk1L1kOr6LUbPAO7a9iEzoxsr9DskIc/p\n\tZfOD93TmrJULLA/7djKCxBe4Sd7oUVDWHC290f+4=","Date":"Sat, 8 Nov 2025 11:56:53 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","Message-ID":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-24-stefan.klug@ideasonboard.com>\n\t\n\t\n\t<176251207553.509848.11267537510627837019@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<176251207553.509848.11267537510627837019@localhost>","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":36758,"web_url":"https://patchwork.libcamera.org/comment/36758/","msgid":"<176276830696.16094.16417454371395161146@localhost>","date":"2025-11-10T09:51:46","subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nQuoting Jacopo Mondi (2025-11-08 11:56:53)\n> Hi Stefan\n> \n> On Fri, Nov 07, 2025 at 11:41:15AM +0100, Stefan Klug wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for looking deep into this.\n> >\n> > Quoting Jacopo Mondi (2025-11-07 09:02:48)\n> > > Hi again Stefan\n> > >\n> > > On Thu, Nov 06, 2025 at 07:44:57PM +0100, Jacopo Mondi wrote:\n> > > > Hi Stefan\n> > > >\n> > > > On Fri, Oct 24, 2025 at 10:50:47AM +0200, Stefan Klug wrote:\n> > > > > For per frame control we want to tick the IPA by the sensor frame\n> > > > > sequence instead of the request frame sequence. This has the side effect\n> > > > > that the IPA must be able to cope with situations where a frame context\n> > > > > is required for a frame that was not queued before (computeParams is\n> > > > > called without a corresponding request) or processStats is called for an\n> > > > > unexpected sequence number (because a scratch buffer was used on kernel\n> > > > > side)\n> > > > >\n> > > > > Prepare for that by allowing the frame context to be initialized on demand.\n> > > > >\n> > > > > Signed-off-by: Stefan Klug \n> > > > > ---\n> > > > > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++\n> > > > > src/ipa/rkisp1/ipa_context.h | 2 ++\n> > > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > > 3 files changed, 10 insertions(+)\n> > > > >\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > > index 399fb51be414..27109478c340 100644\n> > > > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > > > @@ -172,6 +172,8 @@ void Awb::queueRequest(IPAContext &context,\n> > > > > awbAlgo_->handleControls(controls);\n> > > > >\n> > > > > frameContext.awb.autoEnabled = awb.autoEnabled;\n> > > > > + frameContext.awb.gains = awb.automatic.gains;\n> > > > > + frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> > > >\n> > > > unrelated ?\n> >\n> > Ooops :-)\n> >\n> > > >\n> > > > >\n> > > > > if (awb.autoEnabled)\n> > > > > return;\n> > > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > > index f85a130d9c23..185951fb8286 100644\n> > > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > > @@ -214,6 +214,8 @@ struct IPAFrameContext : public FrameContext {\n> > > > > double strength;\n> > > > > double gain;\n> > > > > } wdr;\n> > > > > +\n> > > > > + bool initialised;\n> > > >\n> > > > As already pointed out, the base struct already has an initialised\n> > > > member, an indication that something could maybe be improved.\n> > > >\n> > > > the here introduced initializeFrameContext()\n> > > > is called from 4 places:\n> > > >\n> > > > - start() [where you should call alloc() imho; with valid controls]\n> > > > - queueRequest() [which calls alloc(); with valid controls]\n> > > > - computeParams() [which calls get(); without controls]\n> > > > - processStats() [which calls get(); without controls)\n> > > >\n> > > > If I'm not mistaken\n> > > >\n> > > > - start() will always be called before queueRequest and the frame\n> > > > context won't be initialized, but controls need to be queued to\n> > > > algorithms\n> > > > - queueRequest() will be called with valid controls, but the frame\n> > > > context could already be initialized by the FCQ\n> > > > - computeParams/processStats won't have valid controls but the frame\n> > > > context might already be initialised. Here I'm not sure if we need\n> > >\n> > > this should be \"might have not been initialised\" as we're dealing with\n> > > the case where prepare/process for frame X are called before the\n> > > corresponding request X has been queued.\n> > >\n> > > > to even try to call algo->queueRequest() because there won't be\n> > > > controls associated with this frame context.\n> > > >\n> > > > All in all, I'm wondering if initializeFrameContext() is not actually about only\n> > > > queueing controls to the algos, and we can even skip calling it from\n> > > > computeParams/processStats for which the correct initialisation of the frame\n> > > > context is already performed by the FCQueue.\n> >\n> > I think I miss something here. I don't think we can skip calling\n> > initializeFrameContext() for computeParams/processStats because it\n> > ensures that algo->queueRequest() is called for the FC. I think\n> \n> Right, I think I realized that in my last reply\n> \n> > algo->queueRequest() should be renamed to algo->initializeRequest()\n> > which is the actual intent. The FCQueue only zeroes the frame context\n> > but actual initialization happens in algo->queueRequest().\n> >\n> > >\n> > > I slept over this a bit.. I think we should slightly re-work the FCQ\n> > > to make this a bit nicer.\n> >\n> > I completely agree. I'm not happy with the design of the FCQueue. I\n> > tried to apply modifications in baby steps to ensure I didn't miss\n> > anything in the big picture. But there is still room for further\n> > improvements.\n> >\n> > >\n> > > I think the first thing to do would be to propagate to IPAs the\n> > > \"error\" conditions from FCQueue::alloc() and ::get().\n> > >\n> > > This would help you detect if an unexpected sequence of events has\n> > > happened and act accordingly.\n> > >\n> > > Let's leave out start() for a bit (I would like to clarify, here on in\n> > > the other patch if start() should call alloc() or not, but that's\n> > > unrelated). Let's only consider queueRequest()/process()/prepare().\n> > >\n> > > In normal operating conditions, with user queueing requests fast\n> > > enough to sustain the sensor frame rate, as we now clock the IPA with\n> > > frames from the sensor, and we have no guarantee that requests will be\n> > > queued fast enough to sustain this.\n> > >\n> > >\n> > > - IPA::queueRequest(Request:X)\n> > > - FCQueue::alloc(X):\n> > > - frame context already initialized\n> > > - This means that prepare/process have already been called\n> > > - The Request is late and the corresponding frame has already\n> > > been produced\n> > > - We need to accummulate the Request's controls in the list of\n> > > controls to apply to the \"next\" algo->queueRequest() loop\n> > > - frame context not initialized\n> > > - Normal operating conditions\n> > > - Call algo->queueRequest(request->controls())\n> > >\n> > > - computeParams(Frame:X)\n> > > - FCQeueue::get(X)\n> > > - frame context already initialized\n> > > - Normal operating conditions\n> > > - call algos->prepare()\n> > > - frame context not initialized\n> > > - User request underrun\n> > > - if any pending controls for late requests are present we\n> > > should handle them: call algo->queueRequest() to provide\n> > > them the pending controls we have accumulated\n> > >\n> > > - processStats(Frame:X)\n> > > - same as computeParams(X)\n> > >\n> > > This is what I think it is implemented in this series ? Do you agree\n> > > or I missed anything ?\n> >\n> > I think that's it, yes.\n> >\n> > >\n> > > If that's the case, wouldn't it be nicer if we could do\n> > >\n> > > IPA::queueRequest(request) {\n> > > auto { fc, err } =\n> > > context_.frameContexts.alloc(request->sequece);\n> >\n> > This is a tiny bit misleading as it is not the request sequence, but\n> > sensor frame and controls.\n> >\n> \n> That's something else that I think we should better clarify.\n> \n> Params and stats have the frame sequence as it comes from the kernel\n> as identifier.\n> \n> Request has a sequence number that is unrelated to the frame sequence\n> number.\n> \n> I have not absorbed patch 27 yet, but I presume that's where you\n> associated a request with the most up to date frame sequence ?\n> \n> if that's not the case, how can requests be associated with frame\n> numbers as the two sequences are not related in any way ?\n\nYes, that is done in that patch. For the happy path we can also assume\nthat request->sequence == frame sequence. For me the important aspect is\nthat the IPA has no notion of a request (aside from that function beeing\ncalled queueRequest and probably should be renamed to\ninitializeFrameContext). So the IPA handles only sensor sequence numbers\nand controls.\n\n> \n> \n> > > if (err == REQUEST_UNDERRUN) {\n> > > /* Accumulate controls in the list of pending controls\n> > > * because the request is late */\n> > > pendingControls.merge(request->controls);\n> > > }\n> > >\n> > > queueRequestToAlgo(request->controls);\n> > > }\n> > >\n> > >\n> > > IPA::computeParams(frame) {\n> > > auto { fc, err } = context_.frameContexts.get(frame);\n> > > if (err == REQUEST_UNDERRUN) {\n> > > queueRequestToAlgo(pendingControls);\n> > > }\n> > >\n> > > for (algo : algos) {\n> > > algo->prepare();\n> > > }\n> > >\n> > > setSensorControls();\n> > > }\n> > >\n> > > IPA::processStats(frame, sensorControls) {\n> > > auto { fc, err } = context_.frameContexts.get(frame);\n> > > if (err == REQUEST_UNDERRUN) {\n> > > queueRequestToAlgo(pendingControls);\n> > > }\n> > >\n> > > updateSensorControls(sensorControls);\n> > >\n> > > for (algo : algos) {\n> > > algo->process();\n> > > }\n> > >\n> > > metadataReady.emit();\n> > > }\n> > >\n> > > The only error condition from FCQueue is then a request underrun\n> > > (apart from the Fatal 'overwrite' error), which corresponds to the two\n> > > conditions now demoted to Debug messages in alloc() and get()\n> > >\n> > > in case of underruns we handle the list of pending controls, either by\n> > > accummulating them in queueRequest() or by sending them to algos in\n> > > computeParams and processStats.\n> > >\n> > > Does this match the mental model you have implemented in this series\n> > > or have I missed some parts ?\n> >\n> > I think you got that all right. I'm not sure if I like that style better\n> > as it adds more control logic into computeParams()/processStats() and\n> > assumes that the FCQueue has knowledge of the expected order of actions.\n> \n> My main goal here is to push as much of the design in the common\n> libipa part and make the IPA module as easy to implement as possible.\n> \n> The amount of logic implemented in\n> IPARkISP1::initializeFrameContext() in this series is quite dense\n> \n> if (frameContext.initialised) {\n> if (!controls.empty()) {\n> }\n> return;\n> }\n> \n> const ControlList *controls2 = &controls;\n> if (!controlsToApply_.empty()) {\n> }\n> \n> frameContext.initialised = true;\n> for (auto const &a : algorithms()) {\n> Algorithm *algo = static_cast(a.get());\n> if (algo->disabled_)\n> continue;\n> algo->queueRequest(context_, frame, frameContext, *controls2);\n> }\n> \n> controlsToApply_.clear();\n> \n> And will have to be verbatim replicated in each IPA (I guess).\n> \n> I would be curious to check if the above proposed model which\n> revolves around the concept of queuing controls to the algos with a\n> single queueRequestToAlgo() function that has to be called by\n> prepare()/process() only in case of FCQueue error would result in less\n> logic to replicate in other IPAs.\n\nOk, I see. Given that target it makes absolute sense to keep logic\ninside the FCQueue.\n\n> \n> \n> > But I agree that you could get rid of the outer initialized member in\n> > that case.\n> >\n> > I'd like the FCQueue to be as simple as possible. Continuing on your\n> \n> ehehe I'm fine paying the price of a more complex FCQueue if the logic\n> in the single IPAs is reduced :)\n\nThen why not going one step further? If we allow the FCQueue to call\nqueueRequestToAlgo() we could move all the pending controls handling\ninto the FCQueue. Then we could drop the repetitive error handling from\nthe IPA implementations. Only aspect I don't like too much in that case\nis that the initialized-state handling of the frame context is then\ninside the FCQueue, so you can't use a frame context alone (as is\ncurrently done on start()). That is maybe not that big of a deal but\nrequires the FCQueue to accept alloc beeing called 2 times for frame 0.\n\nSomething like:\n\nIPA::IPA() {\n\tcontext_.frameContexts.setQueueCallback(IPA::queueRequestToAlgo)\n}\n\nIPA::start(controls) {\n auto fc = context_.frameContexts.alloc(0, controls);\n ...\n}\n\nIPA::queueRequest(frame, controls) {\n /* Note that controls is passed into FCQueue */\n auto fc = context_.frameContexts.alloc(frame, controls);\n}\n \nIPA::computeParams(frame) {\n auto fc = context_.frameContexts.get(frame);\n \n for (algo : algos) {\n algo->prepare();\n }\n \n setSensorControls();\n}\n \nIPA::processStats(frame, sensorControls) {\n auto fc = context_.frameContexts.get(frame);\n \n for (algo : algos) {\n algo->process();\n }\n \n metadataReady.emit();\n}\n\nWhat do you think?\n\nBest regards,\nStefan\n\n> \n> > thoughts what about removing the FCQueue::alloc all together?\n> >\n> > If we replace alloc+get by a single get():\n> >\n> > auto { fc, did_allocate } = context_.frameContexts.get(frame);\n> > we could move all the debug messages out of the FCQueue into the IPA.\n> \n> I considered this as well.\n> \n> However alloc() and get() have different semantics in my opinion,\n> mostly around the idea that alloc() should be called before get() in\n> \"normal\" operations.\n> \n> All our PFC designs revolves around the idea that there is a normal\n> operating condition where requests are queued fast enough to sustain\n> the frame rate. In case this doesn't happen we have to handle an\n> \"underrun\" as an error condition. Now, we can re-discuss this, but\n> seeing that in case of underrun the IPA has to take counter-measures\n> to either to accumulate controls for late requests or feed the algos\n> with any pending controls for early frames [*] makes me think as these\n> two as \"error\" conditions.\n> \n> [*] I quite like the idea of collapsing the FCQueue error to a single\n> underrun condition. In the end a late request at queueRequest() time\n> means we had a computeParams() for a frame for which the application\n> has not supplied a Request (yet). And the error paths are\n> effectively the ones described above:\n> - late request: accumulate the controls in a pendingControls_ list\n> - early computeParams(): queue any pendingControls_ to algos to\n> maintain their status up-to-date with users' request\n> \n> In the end, if we have an early computeParams() we'll always have a late\n> queueRequest() and we can only mitigate this by pushing the missed\n> controls to algos as soon as we can. There might be an argument here\n> if a late queueRequest() should push pendingControls_ (before merging\n> its own controls there)\n> \n> >\n> > We can then pass the did_allocate to initializeFrameContext() which\n> > would be close to your proposal or stick to the FC.initialized member\n> > which I somehow still like better. But that is a technical nuance.\n> >\n> > >\n> > > One part it is not totally clear to me in this series is if\n> > > computeParams/processStats for a frame will always be called in\n> > > sequence or not, and if not, if there's anything to do to address this\n> > > condition.\n> >\n> > I wouldn't make much guarantees here. But I can't come up with a\n> > condition where processStats is called without a prior computeParams. A\n> \n> If a pipeline is designed correctly, this shouldn't happen, right ?\n> \n> We can even assert if we detect this condition and check if it ever\n> happens at runtime ?\n> \n> > computeParams() on a frame after processStats was called on that frame\n> > should never happen.\n> >\n> > Best regards,\n> > Stefan\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > >\n> > > > > };\n> > > > >\n> > > > > struct IPAContext {\n> > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > index 01b30c947a0a..23d80bc43c5d 100644\n> > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > @@ -343,6 +343,10 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > > > IPAFrameContext &frameContext,\n> > > > > const ControlList &controls)\n> > > > > {\n> > > > > + if (frameContext.initialised)\n> > > > > + return;\n> > > > > +\n> > > > > + frameContext.initialised = true;\n> > > > > for (auto const &a : algorithms()) {\n> > > > > Algorithm *algo = static_cast(a.get());\n> > > > > if (algo->disabled_)\n> > > > > @@ -354,6 +358,7 @@ void IPARkISP1::initializeFrameContext(const uint32_t frame,\n> > > > > void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)\n> > > > > {\n> > > > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > > + initializeFrameContext(frame, frameContext, {});\n> > > > >\n> > > > > /*\n> > > > > * \\todo: This needs discussion. In raw mode, computeParams is\n> > > > > @@ -383,6 +388,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,\n> > > > > const ControlList &sensorControls)\n> > > > > {\n> > > > > IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > > + initializeFrameContext(frame, frameContext, {});\n> > > > >\n> > > > > /*\n> > > > > * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > > > > --\n> > > > > 2.48.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 961BBC3263\n\tfor ;\n\tMon, 10 Nov 2025 09:51:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 587DC60A8B;\n\tMon, 10 Nov 2025 10:51:51 +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 9300D6069A\n\tfor ;\n\tMon, 10 Nov 2025 10:51:49 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:26aa:8669:decb:f45c])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7A6FEC7B;\n\tMon, 10 Nov 2025 10:49:51 +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=\"syqcymLa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762768191;\n\tbh=qM3rC0cVjtFGZ6JyJ1hMGkR3FigbOoR7FHM4fXJEHn0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=syqcymLaFZvWJkzUvq1hPmkJRBVl9JuBNtuaz4q+52MPzStb8GWmTPa3o44ozsgg4\n\tsaazaiWo03E8bExIk6+ER3bmTVWjD9p4bftZATfxcrTrUGzwmB2qnh23TRzbFuljdB\n\td4OesNnGMbLnZqUBab7b1wEpyPV77oGFgLGCK4yY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-24-stefan.klug@ideasonboard.com>\n\t\n\t\n\t<176251207553.509848.11267537510627837019@localhost>\n\t","Subject":"Re: [PATCH v1 23/35] ipa: rkisp1: Lazy initialise frame context","From":"Stefan Klug ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","To":"Jacopo Mondi ","Date":"Mon, 10 Nov 2025 10:51:46 +0100","Message-ID":"<176276830696.16094.16417454371395161146@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" "}}]