[{"id":31936,"web_url":"https://patchwork.libcamera.org/comment/31936/","msgid":"<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>","date":"2024-10-28T10:27:02","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 16/10/2024 18:03, Jacopo Mondi wrote:\n> Pass to the FCQueue the algorithm's active state to use the most\n> recent state of IPA algorithms to initialize a FrameContext.\n>\n> Modify all IPA modules that use libipa to pass a const ActiveState\n> reference to the FCQueue function and make their IPAActiveState\n> implementation derive a base ActiveState structure.\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/ipa/ipu3/ipa_context.h | 2 +-\n> src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n> src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n> src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n> src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n> src/ipa/rkisp1/ipa_context.h | 5 +++--\n> src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n> src/ipa/simple/ipa_context.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 9 ++++++---\n> 9 files changed, 48 insertions(+), 25 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index c85d1e34ea85..526af2ac0b06 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n> \t} sensor;\n> };\n> \n> -struct IPAActiveState {\n> +struct IPAActiveState : public ActiveState {\n> \tstruct {\n> \t\tuint32_t focus;\n> \t\tdouble maxVariance;\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 10a8c86d8e64..84c443a009fd 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> \t */\n> \tparams->use = {};\n> \n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tfor (auto const &algo : algorithms())\n> \t\talgo->prepare(context_, frame, frameContext, params);\n> @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> \tconst ipu3_uapi_stats_3a *stats =\n> \t\treinterpret_cast(mem.data());\n> \n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\nI get why it needs to be done, but I don't particularly like that we then have to pass activeState \nto .get()...do we know what kinds of situations result in .get() calls before .alloc() calls for a \nFrameContext?\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> @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> */\n> void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tfor (auto const &algo : algorithms())\n> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index fa2454fd5706..56c7c75a8f48 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -42,6 +42,7 @@ namespace ipa {\n> * \\fn FrameContext::init()\n> * \\brief Initialize a frame context\n> * \\param[in] frameNum The frame number to assign to this FrameContext\n> + * \\param[in] activeState The IPA current active state\n> *\n> * This function initializes a frame context by assigning it a frame number.\n> * The single IPA modules are expected to override this function to initialize\n> @@ -117,9 +118,10 @@ namespace ipa {\n> */\n> \n> /**\n> - * \\fn FCQueue::alloc(uint32_t frame)\n> + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n> * \\brief Allocate and return a FrameContext for the \\a frame\n> * \\param[in] frame The frame context sequence number\n> + * \\param[in] activeState The IPA current active state\n> *\n> * The first call to obtain a FrameContext from the FCQueue should be handled\n> * through this function. The FrameContext will be initialised, if not\n> @@ -135,12 +137,14 @@ namespace ipa {\n> */\n> \n> /**\n> - * \\fn FCQueue::get(uint32_t frame)\n> + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n> * \\brief Obtain the FrameContext for the \\a frame\n> * \\param[in] frame The frame context sequence number\n> + * \\param[in] activeState The IPA current active state\n> *\n> * If the FrameContext is not correctly initialised for the \\a frame, it will be\n> - * initialised.\n> + * initialised using the most current state of IPA algorithm contained in\n> + * \\a activeState.\n> *\n> * \\return A reference to the FrameContext for sequence \\a frame\n> */\n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index bfcce5a81356..48842e54cd35 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -21,9 +21,16 @@ namespace ipa {\n> template\n> class FCQueue;\n> \n> +struct ActiveState {\n> +};\n> +\n> struct FrameContext {\n> +public:\n> +\tvirtual ~FrameContext() = default;\n> +\nIs this addition related?\n> protected:\n> -\tvirtual void init(const uint32_t frameNum)\n> +\tvirtual void init(const uint32_t frameNum,\n> +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n> \t{\n> \t\tframe = frameNum;\n> \t\tinitialised = true;\n> @@ -52,7 +59,7 @@ public:\n> \t\t}\n> \t}\n> \n> -\tFrameContext &alloc(const uint32_t frame)\n> +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n> \t{\n> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> \n> @@ -71,12 +78,12 @@ public:\n> \t\t\tLOG(FCQueue, Warning)\n> \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n> \t\telse\n> -\t\t\tframeContext.init(frame);\n> +\t\t\tframeContext.init(frame, activeState);\n> \n> \t\treturn frameContext;\n> \t}\n> \n> -\tFrameContext &get(uint32_t frame)\n> +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n> \t{\n> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> \n> @@ -103,7 +110,7 @@ public:\n> \t\t\t * Make sure the FrameContext gets initialised if get()\n> \t\t\t * is called before alloc() by the IPA for frame#0.\n> \t\t\t */\n> -\t\t\tframeContext.init(frame);\n> +\t\t\tframeContext.init(frame, activeState);\n> \n> \t\t\treturn frameContext;\n> \t\t}\n> @@ -123,7 +130,7 @@ public:\n> \t\tLOG(FCQueue, Warning)\n> \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n> \n> -\t\tframeContext.init(frame);\n> +\t\tframeContext.init(frame, activeState);\n> \n> \t\treturn frameContext;\n> \t}\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 4e4fe5f4ae96..2dad42b3154f 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n> * \\brief Analogue gain multiplier\n> */\n> \n> -void IPAFrameContext::init(const uint32_t frameNum)\n> +void IPAFrameContext::init(const uint32_t frameNum,\n> +\t\t\t const ActiveState &activeState)\n> {\n> -\tFrameContext::init(frameNum);\n> +\tFrameContext::init(frameNum, activeState);\n> }\n> \n> /**\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 51e04bc30627..f708b32111af 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n> \tuint32_t paramFormat;\n> };\n> \n> -struct IPAActiveState {\n> +struct IPAActiveState : public ActiveState {\n> \tstruct {\n> \t\tstruct {\n> \t\t\tuint32_t exposure;\n> @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n> \t\tMatrix ccm;\n> \t} ccm;\n> \n> -\tvoid init(const uint32_t frame) override;\n> +\tvoid init(const uint32_t frame,\n> +\t\t const ActiveState &activeState) override;\n> };\n> \n> struct IPAContext {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 9e161cabdea4..7c1cefc1c7fa 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \n> void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tfor (auto const &a : algorithms()) {\n> \t\tAlgorithm *algo = static_cast(a.get());\n> @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> \n> void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tRkISP1Params params(context_.configuration.paramFormat,\n> \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n> @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t\t const ControlList &sensorControls)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \t/*\n> \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n> \t * internal sensor delays and other timing parameters into account.\n> \t */\n> \n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \tuint32_t exposure = frameContext.agc.exposure;\n> \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n> \n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 3519f20f6415..956f4fb71abf 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n> \t} agc;\n> };\n> \n> -struct IPAActiveState {\n> +struct IPAActiveState : public ActiveState {\n> \tstruct {\n> \t\tuint8_t level;\n> \t} blc;\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index b28c7039f7bd..71b31d728117 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n> \n> void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tfor (auto const &algo : algorithms())\n> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n> \n> void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \tfor (auto const &algo : algorithms())\n> \t\talgo->prepare(context_, frame, frameContext, params_);\n> \tsetIspParams.emit();\n> @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n> \t\t\t\t const ControlList &sensorControls)\n> {\n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> +\t\t\t\t\t\t\t\t context_.activeState);\n> \n> \tframeContext.sensor.exposure =\n> \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 DDA76C3220\n\tfor ;\n\tMon, 28 Oct 2024 10:27:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92C1C6539F;\n\tMon, 28 Oct 2024 11:27:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42A0F618C0\n\tfor ;\n\tMon, 28 Oct 2024 11:27:05 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 84DA510C4\n\tfor ;\n\tMon, 28 Oct 2024 11:27:03 +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=\"b0zvyqKg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730111223;\n\tbh=EKkRAxVl6xnKSMJDVE+89N9VanDZ+xUAqiM/YZHCMx8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=b0zvyqKgCI6w8pZj7M2I320GP7gB6Jndxfh38AjHyuePCvZzFjf+eXj6iXBFYmqAz\n\t1CATF1zw5/UFGEockV9AUW3qsKusGbkMz6H1aR+q6j0mFhR6BwJ2TFkI+PsFu8DBZv\n\tHNCDQbNxco99N+FOr9r4VKnmWA6/NyT7THILz15E=","Message-ID":"<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>","Date":"Mon, 28 Oct 2024 10:27:02 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","To":"libcamera-devel@lists.libcamera.org","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":31939,"web_url":"https://patchwork.libcamera.org/comment/31939/","msgid":"","date":"2024-10-28T10:56:45","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Mon, Oct 28, 2024 at 10:27:02AM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 16/10/2024 18:03, Jacopo Mondi wrote:\n> > Pass to the FCQueue the algorithm's active state to use the most\n> > recent state of IPA algorithms to initialize a FrameContext.\n> >\n> > Modify all IPA modules that use libipa to pass a const ActiveState\n> > reference to the FCQueue function and make their IPAActiveState\n> > implementation derive a base ActiveState structure.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/ipa/ipu3/ipa_context.h | 2 +-\n> > src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n> > src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n> > src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n> > src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n> > src/ipa/rkisp1/ipa_context.h | 5 +++--\n> > src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n> > src/ipa/simple/ipa_context.h | 2 +-\n> > src/ipa/simple/soft_simple.cpp | 9 ++++++---\n> > 9 files changed, 48 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index c85d1e34ea85..526af2ac0b06 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n> > \t} sensor;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tuint32_t focus;\n> > \t\tdouble maxVariance;\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index 10a8c86d8e64..84c443a009fd 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > \t */\n> > \tparams->use = {};\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->prepare(context_, frame, frameContext, params);\n> > @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > \tconst ipu3_uapi_stats_3a *stats =\n> > \t\treinterpret_cast(mem.data());\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> I get why it needs to be done, but I don't particularly like that we then\n> have to pass activeState to .get()...do we know what kinds of situations\n\nBecause it's more letters to type or for other reasons ?\n\n> result in .get() calls before .alloc() calls for a FrameContext?\n\nI'll explain my use case, which is detailed in 4/4.\n\nCurrently the ph/ipa interactions are clocked by a user submitting a\nRequest. A Request gets queued, it alloc() a frame context and\nit goes through algo->queueRequest(). AGC::queueRequest() initializes\nFrameContext.agc.meteringMode using the active state.\n\nNow, we move towards a model where the IPA is instead clocked by\nframes being produced by the sensor and thus we can have Request\nunderruns if an app doesn't queue requests fast enough to keep it up\nwith the sensor's frame rate. In this case we'll go through\nprocessStats() without going through IPA::queueRequest. In this case\nwe get() a frame context, and if we get it before it has been\nallocated, we should make sure it can be processed by algo->process()\nwithout causing issues as the one that 4/4 fixes.\n\nI presume however, that even if we get through an alloc() making sure\nthat the initialization made by algorithms using the active state are\nhandled in IPAFrameContext::init() we would probably get more robust\ncode, making sure the algorithms' implementation do not rely on too\nmany internal state-keeping and instead work on what's implemented in\nIPAFrameContext.\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> > @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > */\n> > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > index fa2454fd5706..56c7c75a8f48 100644\n> > --- a/src/ipa/libipa/fc_queue.cpp\n> > +++ b/src/ipa/libipa/fc_queue.cpp\n> > @@ -42,6 +42,7 @@ namespace ipa {\n> > * \\fn FrameContext::init()\n> > * \\brief Initialize a frame context\n> > * \\param[in] frameNum The frame number to assign to this FrameContext\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * This function initializes a frame context by assigning it a frame number.\n> > * The single IPA modules are expected to override this function to initialize\n> > @@ -117,9 +118,10 @@ namespace ipa {\n> > */\n> > /**\n> > - * \\fn FCQueue::alloc(uint32_t frame)\n> > + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n> > * \\brief Allocate and return a FrameContext for the \\a frame\n> > * \\param[in] frame The frame context sequence number\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * The first call to obtain a FrameContext from the FCQueue should be handled\n> > * through this function. The FrameContext will be initialised, if not\n> > @@ -135,12 +137,14 @@ namespace ipa {\n> > */\n> > /**\n> > - * \\fn FCQueue::get(uint32_t frame)\n> > + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n> > * \\brief Obtain the FrameContext for the \\a frame\n> > * \\param[in] frame The frame context sequence number\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * If the FrameContext is not correctly initialised for the \\a frame, it will be\n> > - * initialised.\n> > + * initialised using the most current state of IPA algorithm contained in\n> > + * \\a activeState.\n> > *\n> > * \\return A reference to the FrameContext for sequence \\a frame\n> > */\n> > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > index bfcce5a81356..48842e54cd35 100644\n> > --- a/src/ipa/libipa/fc_queue.h\n> > +++ b/src/ipa/libipa/fc_queue.h\n> > @@ -21,9 +21,16 @@ namespace ipa {\n> > template\n> > class FCQueue;\n> > +struct ActiveState {\n> > +};\n> > +\n> > struct FrameContext {\n> > +public:\n> > +\tvirtual ~FrameContext() = default;\n> > +\n> Is this addition related?\n> > protected:\n> > -\tvirtual void init(const uint32_t frameNum)\n> > +\tvirtual void init(const uint32_t frameNum,\n> > +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n> > \t{\n> > \t\tframe = frameNum;\n> > \t\tinitialised = true;\n> > @@ -52,7 +59,7 @@ public:\n> > \t\t}\n> > \t}\n> > -\tFrameContext &alloc(const uint32_t frame)\n> > +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n> > \t{\n> > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > @@ -71,12 +78,12 @@ public:\n> > \t\t\tLOG(FCQueue, Warning)\n> > \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n> > \t\telse\n> > -\t\t\tframeContext.init(frame);\n> > +\t\t\tframeContext.init(frame, activeState);\n> > \t\treturn frameContext;\n> > \t}\n> > -\tFrameContext &get(uint32_t frame)\n> > +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n> > \t{\n> > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > @@ -103,7 +110,7 @@ public:\n> > \t\t\t * Make sure the FrameContext gets initialised if get()\n> > \t\t\t * is called before alloc() by the IPA for frame#0.\n> > \t\t\t */\n> > -\t\t\tframeContext.init(frame);\n> > +\t\t\tframeContext.init(frame, activeState);\n> > \t\t\treturn frameContext;\n> > \t\t}\n> > @@ -123,7 +130,7 @@ public:\n> > \t\tLOG(FCQueue, Warning)\n> > \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n> > -\t\tframeContext.init(frame);\n> > +\t\tframeContext.init(frame, activeState);\n> > \t\treturn frameContext;\n> > \t}\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 4e4fe5f4ae96..2dad42b3154f 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n> > * \\brief Analogue gain multiplier\n> > */\n> > -void IPAFrameContext::init(const uint32_t frameNum)\n> > +void IPAFrameContext::init(const uint32_t frameNum,\n> > +\t\t\t const ActiveState &activeState)\n> > {\n> > -\tFrameContext::init(frameNum);\n> > +\tFrameContext::init(frameNum, activeState);\n> > }\n> > /**\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 51e04bc30627..f708b32111af 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n> > \tuint32_t paramFormat;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tstruct {\n> > \t\t\tuint32_t exposure;\n> > @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n> > \t\tMatrix ccm;\n> > \t} ccm;\n> > -\tvoid init(const uint32_t frame) override;\n> > +\tvoid init(const uint32_t frame,\n> > +\t\t const ActiveState &activeState) override;\n> > };\n> > struct IPAContext {\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 9e161cabdea4..7c1cefc1c7fa 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> > void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &a : algorithms()) {\n> > \t\tAlgorithm *algo = static_cast(a.get());\n> > @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tRkISP1Params params(context_.configuration.paramFormat,\n> > \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n> > @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> > \t\t\t\t const ControlList &sensorControls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \t/*\n> > \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n> > \t * internal sensor delays and other timing parameters into account.\n> > \t */\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tuint32_t exposure = frameContext.agc.exposure;\n> > \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index 3519f20f6415..956f4fb71abf 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n> > \t} agc;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tuint8_t level;\n> > \t} blc;\n> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > index b28c7039f7bd..71b31d728117 100644\n> > --- a/src/ipa/simple/soft_simple.cpp\n> > +++ b/src/ipa/simple/soft_simple.cpp\n> > @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n> > void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n> > void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->prepare(context_, frame, frameContext, params_);\n> > \tsetIspParams.emit();\n> > @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> > \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n> > \t\t\t\t const ControlList &sensorControls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tframeContext.sensor.exposure =\n> > \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 58EBAC3220\n\tfor ;\n\tMon, 28 Oct 2024 10:56:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A5376539E;\n\tMon, 28 Oct 2024 11:56:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A047660360\n\tfor ;\n\tMon, 28 Oct 2024 11:56:52 +0100 (CET)","from ideasonboard.com (mob-5-90-59-111.net.vodafone.it\n\t[5.90.59.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DC40910C4;\n\tMon, 28 Oct 2024 11:56:49 +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=\"hh8263kv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730113010;\n\tbh=WIOdB5//1ZUUbu0Jie0OkIhhTWfqQ0oyqaCTlKHYn0w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hh8263kvMf9AQN8WzjdJqyqvaL6qpywY6PcaQJqldZMQTNxhVpMREGPlOTJV0CYka\n\tsRYUZTmr6GSggse3DK/9LdH4FhtStjRuBF0EIpsoq4n0WR03JuZ80DcydE931f65aW\n\tpgCwBVrXl5DlFAzK9Phc0GmMMk0TdlgZ1trZNAEM=","Date":"Mon, 28 Oct 2024 11:56:45 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","Message-ID":"","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>\n\t<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<3f4ae231-d750-458b-9d2c-67a60d968263@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":31940,"web_url":"https://patchwork.libcamera.org/comment/31940/","msgid":"","date":"2024-10-28T11:00:10","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi again, sorry missed one comment (sorry but not having empty lines\nbetween the quoted text and your reply makes it quite terse to read\nfor me)\n\nOn Mon, Oct 28, 2024 at 10:27:02AM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 16/10/2024 18:03, Jacopo Mondi wrote:\n> > Pass to the FCQueue the algorithm's active state to use the most\n> > recent state of IPA algorithms to initialize a FrameContext.\n> >\n> > Modify all IPA modules that use libipa to pass a const ActiveState\n> > reference to the FCQueue function and make their IPAActiveState\n> > implementation derive a base ActiveState structure.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/ipa/ipu3/ipa_context.h | 2 +-\n> > src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n> > src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n> > src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n> > src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n> > src/ipa/rkisp1/ipa_context.h | 5 +++--\n> > src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n> > src/ipa/simple/ipa_context.h | 2 +-\n> > src/ipa/simple/soft_simple.cpp | 9 ++++++---\n> > 9 files changed, 48 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index c85d1e34ea85..526af2ac0b06 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n> > \t} sensor;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tuint32_t focus;\n> > \t\tdouble maxVariance;\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index 10a8c86d8e64..84c443a009fd 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > \t */\n> > \tparams->use = {};\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->prepare(context_, frame, frameContext, params);\n> > @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > \tconst ipu3_uapi_stats_3a *stats =\n> > \t\treinterpret_cast(mem.data());\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> I get why it needs to be done, but I don't particularly like that we then\n> have to pass activeState to .get()...do we know what kinds of situations\n> result in .get() calls before .alloc() calls for a FrameContext?\n> > \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n> > \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> > @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > */\n> > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > index fa2454fd5706..56c7c75a8f48 100644\n> > --- a/src/ipa/libipa/fc_queue.cpp\n> > +++ b/src/ipa/libipa/fc_queue.cpp\n> > @@ -42,6 +42,7 @@ namespace ipa {\n> > * \\fn FrameContext::init()\n> > * \\brief Initialize a frame context\n> > * \\param[in] frameNum The frame number to assign to this FrameContext\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * This function initializes a frame context by assigning it a frame number.\n> > * The single IPA modules are expected to override this function to initialize\n> > @@ -117,9 +118,10 @@ namespace ipa {\n> > */\n> > /**\n> > - * \\fn FCQueue::alloc(uint32_t frame)\n> > + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n> > * \\brief Allocate and return a FrameContext for the \\a frame\n> > * \\param[in] frame The frame context sequence number\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * The first call to obtain a FrameContext from the FCQueue should be handled\n> > * through this function. The FrameContext will be initialised, if not\n> > @@ -135,12 +137,14 @@ namespace ipa {\n> > */\n> > /**\n> > - * \\fn FCQueue::get(uint32_t frame)\n> > + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n> > * \\brief Obtain the FrameContext for the \\a frame\n> > * \\param[in] frame The frame context sequence number\n> > + * \\param[in] activeState The IPA current active state\n> > *\n> > * If the FrameContext is not correctly initialised for the \\a frame, it will be\n> > - * initialised.\n> > + * initialised using the most current state of IPA algorithm contained in\n> > + * \\a activeState.\n> > *\n> > * \\return A reference to the FrameContext for sequence \\a frame\n> > */\n> > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > index bfcce5a81356..48842e54cd35 100644\n> > --- a/src/ipa/libipa/fc_queue.h\n> > +++ b/src/ipa/libipa/fc_queue.h\n> > @@ -21,9 +21,16 @@ namespace ipa {\n> > template\n> > class FCQueue;\n> > +struct ActiveState {\n> > +};\n> > +\n> > struct FrameContext {\n> > +public:\n> > +\tvirtual ~FrameContext() = default;\n> > +\n> Is this addition related?\n\nNot in this patch, but probably in 1/4 where I make the FrameContext\nclass derivable by introducing a virtual method.\n\nThanks for spotting.\n j\n\n> > protected:\n> > -\tvirtual void init(const uint32_t frameNum)\n> > +\tvirtual void init(const uint32_t frameNum,\n> > +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n> > \t{\n> > \t\tframe = frameNum;\n> > \t\tinitialised = true;\n> > @@ -52,7 +59,7 @@ public:\n> > \t\t}\n> > \t}\n> > -\tFrameContext &alloc(const uint32_t frame)\n> > +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n> > \t{\n> > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > @@ -71,12 +78,12 @@ public:\n> > \t\t\tLOG(FCQueue, Warning)\n> > \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n> > \t\telse\n> > -\t\t\tframeContext.init(frame);\n> > +\t\t\tframeContext.init(frame, activeState);\n> > \t\treturn frameContext;\n> > \t}\n> > -\tFrameContext &get(uint32_t frame)\n> > +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n> > \t{\n> > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > @@ -103,7 +110,7 @@ public:\n> > \t\t\t * Make sure the FrameContext gets initialised if get()\n> > \t\t\t * is called before alloc() by the IPA for frame#0.\n> > \t\t\t */\n> > -\t\t\tframeContext.init(frame);\n> > +\t\t\tframeContext.init(frame, activeState);\n> > \t\t\treturn frameContext;\n> > \t\t}\n> > @@ -123,7 +130,7 @@ public:\n> > \t\tLOG(FCQueue, Warning)\n> > \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n> > -\t\tframeContext.init(frame);\n> > +\t\tframeContext.init(frame, activeState);\n> > \t\treturn frameContext;\n> > \t}\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 4e4fe5f4ae96..2dad42b3154f 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n> > * \\brief Analogue gain multiplier\n> > */\n> > -void IPAFrameContext::init(const uint32_t frameNum)\n> > +void IPAFrameContext::init(const uint32_t frameNum,\n> > +\t\t\t const ActiveState &activeState)\n> > {\n> > -\tFrameContext::init(frameNum);\n> > +\tFrameContext::init(frameNum, activeState);\n> > }\n> > /**\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 51e04bc30627..f708b32111af 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n> > \tuint32_t paramFormat;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tstruct {\n> > \t\t\tuint32_t exposure;\n> > @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n> > \t\tMatrix ccm;\n> > \t} ccm;\n> > -\tvoid init(const uint32_t frame) override;\n> > +\tvoid init(const uint32_t frame,\n> > +\t\t const ActiveState &activeState) override;\n> > };\n> > struct IPAContext {\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 9e161cabdea4..7c1cefc1c7fa 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> > void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &a : algorithms()) {\n> > \t\tAlgorithm *algo = static_cast(a.get());\n> > @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tRkISP1Params params(context_.configuration.paramFormat,\n> > \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n> > @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> > \t\t\t\t const ControlList &sensorControls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \t/*\n> > \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n> > \t * internal sensor delays and other timing parameters into account.\n> > \t */\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tuint32_t exposure = frameContext.agc.exposure;\n> > \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index 3519f20f6415..956f4fb71abf 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n> > \t} agc;\n> > };\n> > -struct IPAActiveState {\n> > +struct IPAActiveState : public ActiveState {\n> > \tstruct {\n> > \t\tuint8_t level;\n> > \t} blc;\n> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > index b28c7039f7bd..71b31d728117 100644\n> > --- a/src/ipa/simple/soft_simple.cpp\n> > +++ b/src/ipa/simple/soft_simple.cpp\n> > @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n> > void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n> > void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tfor (auto const &algo : algorithms())\n> > \t\talgo->prepare(context_, frame, frameContext, params_);\n> > \tsetIspParams.emit();\n> > @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> > \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n> > \t\t\t\t const ControlList &sensorControls)\n> > {\n> > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > +\t\t\t\t\t\t\t\t context_.activeState);\n> > \tframeContext.sensor.exposure =\n> > \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 AB2F2BD78E\n\tfor ;\n\tMon, 28 Oct 2024 11:00:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B84B6539E;\n\tMon, 28 Oct 2024 12:00:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DA0060367\n\tfor ;\n\tMon, 28 Oct 2024 12:00:22 +0100 (CET)","from ideasonboard.com (mob-5-90-59-111.net.vodafone.it\n\t[5.90.59.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFCC31A7D;\n\tMon, 28 Oct 2024 12:00:16 +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=\"R2CV7OMh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730113219;\n\tbh=O2ZMcQvGtt8Jjxg6apv1/p/xHtvLd7Pt9OwwtuKwuEw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=R2CV7OMhiZOHTW2nFi0vsySwW+0sk3q9LoQBtAuZw4R37vG3xLjjPO+ZyW44DdFGf\n\teC7XYYZQko7b/DMBdhXEu3j7bphNEcAWHQcatYHC3uiTIO9MbZG99PTFY3o/M/GFYI\n\tNa05aOBfAMnvr4GMDrkTEqWSulOGNeJprvNmzRQ0=","Date":"Mon, 28 Oct 2024 12:00:10 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","Message-ID":"","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>\n\t<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<3f4ae231-d750-458b-9d2c-67a60d968263@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":31955,"web_url":"https://patchwork.libcamera.org/comment/31955/","msgid":"<8c713e46-63b8-41ed-acab-7a313c426b31@ideasonboard.com>","date":"2024-10-29T09:51:03","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 28/10/2024 10:56, Jacopo Mondi wrote:\n> Hi Dan\n>\n> On Mon, Oct 28, 2024 at 10:27:02AM +0000, Dan Scally wrote:\n>> Hi Jacopo\n>>\n>> On 16/10/2024 18:03, Jacopo Mondi wrote:\n>>> Pass to the FCQueue the algorithm's active state to use the most\n>>> recent state of IPA algorithms to initialize a FrameContext.\n>>>\n>>> Modify all IPA modules that use libipa to pass a const ActiveState\n>>> reference to the FCQueue function and make their IPAActiveState\n>>> implementation derive a base ActiveState structure.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/ipa/ipu3/ipa_context.h | 2 +-\n>>> src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n>>> src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n>>> src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n>>> src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n>>> src/ipa/rkisp1/ipa_context.h | 5 +++--\n>>> src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n>>> src/ipa/simple/ipa_context.h | 2 +-\n>>> src/ipa/simple/soft_simple.cpp | 9 ++++++---\n>>> 9 files changed, 48 insertions(+), 25 deletions(-)\n>>>\n>>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n>>> index c85d1e34ea85..526af2ac0b06 100644\n>>> --- a/src/ipa/ipu3/ipa_context.h\n>>> +++ b/src/ipa/ipu3/ipa_context.h\n>>> @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n>>> \t} sensor;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tuint32_t focus;\n>>> \t\tdouble maxVariance;\n>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>>> index 10a8c86d8e64..84c443a009fd 100644\n>>> --- a/src/ipa/ipu3/ipu3.cpp\n>>> +++ b/src/ipa/ipu3/ipu3.cpp\n>>> @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> \t */\n>>> \tparams->use = {};\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->prepare(context_, frame, frameContext, params);\n>>> @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>> \tconst ipu3_uapi_stats_3a *stats =\n>>> \t\treinterpret_cast(mem.data());\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>> I get why it needs to be done, but I don't particularly like that we then\n>> have to pass activeState to .get()...do we know what kinds of situations\n> Because it's more letters to type or for other reasons ?\n\n\nMore because it feels like we're stacking workarounds on top of a problem that it would be better to \navoid if possible, though it may not be reasonably possible, in which case c'est la vie.\n\n>\n>> result in .get() calls before .alloc() calls for a FrameContext?\n> I'll explain my use case, which is detailed in 4/4.\n>\n> Currently the ph/ipa interactions are clocked by a user submitting a\n> Request. A Request gets queued, it alloc() a frame context and\n> it goes through algo->queueRequest(). AGC::queueRequest() initializes\n> FrameContext.agc.meteringMode using the active state.\n>\n> Now, we move towards a model where the IPA is instead clocked by\n> frames being produced by the sensor and thus we can have Request\n> underruns if an app doesn't queue requests fast enough to keep it up\n> with the sensor's frame rate. In this case we'll go through\n> processStats() without going through IPA::queueRequest. In this case\n> we get() a frame context, and if we get it before it has been\n> allocated, we should make sure it can be processed by algo->process()\n> without causing issues as the one that 4/4 fixes.\n\nYeah ok. Do we get those Request underruns in the current implementation? I lean towards the idea \nthat perhaps alloc()ing the FrameContext in queueRequest() is the wrong approach when the IPA is \nclocked by the sensor, and perhaps we ought to switch to indexing the FCQueue using the sensor frame \nnumber...but I imagine that's a large bit of work.\n\n>\n> I presume however, that even if we get through an alloc() making sure\n> that the initialization made by algorithms using the active state are\n> handled in IPAFrameContext::init() we would probably get more robust\n> code, making sure the algorithms' implementation do not rely on too\n> many internal state-keeping and instead work on what's implemented in\n> IPAFrameContext.\nProbably true.\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>>> @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>> */\n>>> void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n>>> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n>>> index fa2454fd5706..56c7c75a8f48 100644\n>>> --- a/src/ipa/libipa/fc_queue.cpp\n>>> +++ b/src/ipa/libipa/fc_queue.cpp\n>>> @@ -42,6 +42,7 @@ namespace ipa {\n>>> * \\fn FrameContext::init()\n>>> * \\brief Initialize a frame context\n>>> * \\param[in] frameNum The frame number to assign to this FrameContext\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * This function initializes a frame context by assigning it a frame number.\n>>> * The single IPA modules are expected to override this function to initialize\n>>> @@ -117,9 +118,10 @@ namespace ipa {\n>>> */\n>>> /**\n>>> - * \\fn FCQueue::alloc(uint32_t frame)\n>>> + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n>>> * \\brief Allocate and return a FrameContext for the \\a frame\n>>> * \\param[in] frame The frame context sequence number\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * The first call to obtain a FrameContext from the FCQueue should be handled\n>>> * through this function. The FrameContext will be initialised, if not\n>>> @@ -135,12 +137,14 @@ namespace ipa {\n>>> */\n>>> /**\n>>> - * \\fn FCQueue::get(uint32_t frame)\n>>> + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n>>> * \\brief Obtain the FrameContext for the \\a frame\n>>> * \\param[in] frame The frame context sequence number\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * If the FrameContext is not correctly initialised for the \\a frame, it will be\n>>> - * initialised.\n>>> + * initialised using the most current state of IPA algorithm contained in\n>>> + * \\a activeState.\n>>> *\n>>> * \\return A reference to the FrameContext for sequence \\a frame\n>>> */\n>>> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n>>> index bfcce5a81356..48842e54cd35 100644\n>>> --- a/src/ipa/libipa/fc_queue.h\n>>> +++ b/src/ipa/libipa/fc_queue.h\n>>> @@ -21,9 +21,16 @@ namespace ipa {\n>>> template\n>>> class FCQueue;\n>>> +struct ActiveState {\n>>> +};\n>>> +\n>>> struct FrameContext {\n>>> +public:\n>>> +\tvirtual ~FrameContext() = default;\n>>> +\n>> Is this addition related?\n>>> protected:\n>>> -\tvirtual void init(const uint32_t frameNum)\n>>> +\tvirtual void init(const uint32_t frameNum,\n>>> +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n>>> \t{\n>>> \t\tframe = frameNum;\n>>> \t\tinitialised = true;\n>>> @@ -52,7 +59,7 @@ public:\n>>> \t\t}\n>>> \t}\n>>> -\tFrameContext &alloc(const uint32_t frame)\n>>> +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n>>> \t{\n>>> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n>>> @@ -71,12 +78,12 @@ public:\n>>> \t\t\tLOG(FCQueue, Warning)\n>>> \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n>>> \t\telse\n>>> -\t\t\tframeContext.init(frame);\n>>> +\t\t\tframeContext.init(frame, activeState);\n>>> \t\treturn frameContext;\n>>> \t}\n>>> -\tFrameContext &get(uint32_t frame)\n>>> +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n>>> \t{\n>>> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n>>> @@ -103,7 +110,7 @@ public:\n>>> \t\t\t * Make sure the FrameContext gets initialised if get()\n>>> \t\t\t * is called before alloc() by the IPA for frame#0.\n>>> \t\t\t */\n>>> -\t\t\tframeContext.init(frame);\n>>> +\t\t\tframeContext.init(frame, activeState);\n>>> \t\t\treturn frameContext;\n>>> \t\t}\n>>> @@ -123,7 +130,7 @@ public:\n>>> \t\tLOG(FCQueue, Warning)\n>>> \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n>>> -\t\tframeContext.init(frame);\n>>> +\t\tframeContext.init(frame, activeState);\n>>> \t\treturn frameContext;\n>>> \t}\n>>> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n>>> index 4e4fe5f4ae96..2dad42b3154f 100644\n>>> --- a/src/ipa/rkisp1/ipa_context.cpp\n>>> +++ b/src/ipa/rkisp1/ipa_context.cpp\n>>> @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n>>> * \\brief Analogue gain multiplier\n>>> */\n>>> -void IPAFrameContext::init(const uint32_t frameNum)\n>>> +void IPAFrameContext::init(const uint32_t frameNum,\n>>> +\t\t\t const ActiveState &activeState)\n>>> {\n>>> -\tFrameContext::init(frameNum);\n>>> +\tFrameContext::init(frameNum, activeState);\n>>> }\n>>> /**\n>>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>>> index 51e04bc30627..f708b32111af 100644\n>>> --- a/src/ipa/rkisp1/ipa_context.h\n>>> +++ b/src/ipa/rkisp1/ipa_context.h\n>>> @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n>>> \tuint32_t paramFormat;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tstruct {\n>>> \t\t\tuint32_t exposure;\n>>> @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n>>> \t\tMatrix ccm;\n>>> \t} ccm;\n>>> -\tvoid init(const uint32_t frame) override;\n>>> +\tvoid init(const uint32_t frame,\n>>> +\t\t const ActiveState &activeState) override;\n>>> };\n>>> struct IPAContext {\n>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>> index 9e161cabdea4..7c1cefc1c7fa 100644\n>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>> @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>>> void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &a : algorithms()) {\n>>> \t\tAlgorithm *algo = static_cast(a.get());\n>>> @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tRkISP1Params params(context_.configuration.paramFormat,\n>>> \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n>>> @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n>>> \t\t\t\t const ControlList &sensorControls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \t/*\n>>> \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n>>> @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n>>> \t * internal sensor delays and other timing parameters into account.\n>>> \t */\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tuint32_t exposure = frameContext.agc.exposure;\n>>> \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n>>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>>> index 3519f20f6415..956f4fb71abf 100644\n>>> --- a/src/ipa/simple/ipa_context.h\n>>> +++ b/src/ipa/simple/ipa_context.h\n>>> @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n>>> \t} agc;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tuint8_t level;\n>>> \t} blc;\n>>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n>>> index b28c7039f7bd..71b31d728117 100644\n>>> --- a/src/ipa/simple/soft_simple.cpp\n>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>> @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n>>> void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n>>> @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n>>> void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->prepare(context_, frame, frameContext, params_);\n>>> \tsetIspParams.emit();\n>>> @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>>> \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n>>> \t\t\t\t const ControlList &sensorControls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tframeContext.sensor.exposure =\n>>> \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 8A0B8C3220\n\tfor ;\n\tTue, 29 Oct 2024 09:51:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 85CED6539E;\n\tTue, 29 Oct 2024 10:51:08 +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 D576D60366\n\tfor ;\n\tTue, 29 Oct 2024 10:51:06 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 46C98AF;\n\tTue, 29 Oct 2024 10:51:04 +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=\"BQ0GX6/f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730195464;\n\tbh=7DeRBCfqhVfZLu4fJRbabGeCQrYl/RKCHg6Uur4w604=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=BQ0GX6/fPLQo0e/XE8De8W6rXBYtMh/g6P+cvKalsntcqWCJXOYk9WQZHWNghFJ+u\n\t2dOXmnyjP2zAX8p2q2AY90tTYkL3l9/i1JHSy6/cdKvuRB824NNTuGMQucjL+5N0qr\n\tl5+C1lmV9AqdnxY4/4MAc/h5lAJwNYR8Ukwx3aZc=","Message-ID":"<8c713e46-63b8-41ed-acab-7a313c426b31@ideasonboard.com>","Date":"Tue, 29 Oct 2024 09:51:03 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>\n\t<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":31956,"web_url":"https://patchwork.libcamera.org/comment/31956/","msgid":"<282fcf62-9541-4266-b001-5740d4308cac@ideasonboard.com>","date":"2024-10-29T09:51:40","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"On 28/10/2024 11:00, Jacopo Mondi wrote:\n> Hi again, sorry missed one comment (sorry but not having empty lines\n> between the quoted text and your reply makes it quite terse to read\n> for me)\n\n\nOh sorry - I'll try to remember to add them.\n\n\n> On Mon, Oct 28, 2024 at 10:27:02AM +0000, Dan Scally wrote:\n>> Hi Jacopo\n>>\n>> On 16/10/2024 18:03, Jacopo Mondi wrote:\n>>> Pass to the FCQueue the algorithm's active state to use the most\n>>> recent state of IPA algorithms to initialize a FrameContext.\n>>>\n>>> Modify all IPA modules that use libipa to pass a const ActiveState\n>>> reference to the FCQueue function and make their IPAActiveState\n>>> implementation derive a base ActiveState structure.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/ipa/ipu3/ipa_context.h | 2 +-\n>>> src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n>>> src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n>>> src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n>>> src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n>>> src/ipa/rkisp1/ipa_context.h | 5 +++--\n>>> src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n>>> src/ipa/simple/ipa_context.h | 2 +-\n>>> src/ipa/simple/soft_simple.cpp | 9 ++++++---\n>>> 9 files changed, 48 insertions(+), 25 deletions(-)\n>>>\n>>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n>>> index c85d1e34ea85..526af2ac0b06 100644\n>>> --- a/src/ipa/ipu3/ipa_context.h\n>>> +++ b/src/ipa/ipu3/ipa_context.h\n>>> @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n>>> \t} sensor;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tuint32_t focus;\n>>> \t\tdouble maxVariance;\n>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>>> index 10a8c86d8e64..84c443a009fd 100644\n>>> --- a/src/ipa/ipu3/ipu3.cpp\n>>> +++ b/src/ipa/ipu3/ipu3.cpp\n>>> @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> \t */\n>>> \tparams->use = {};\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->prepare(context_, frame, frameContext, params);\n>>> @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>> \tconst ipu3_uapi_stats_3a *stats =\n>>> \t\treinterpret_cast(mem.data());\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>> I get why it needs to be done, but I don't particularly like that we then\n>> have to pass activeState to .get()...do we know what kinds of situations\n>> result in .get() calls before .alloc() calls for a FrameContext?\n>>> \tframeContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get();\n>>> \tframeContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>>> @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>>> */\n>>> void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n>>> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n>>> index fa2454fd5706..56c7c75a8f48 100644\n>>> --- a/src/ipa/libipa/fc_queue.cpp\n>>> +++ b/src/ipa/libipa/fc_queue.cpp\n>>> @@ -42,6 +42,7 @@ namespace ipa {\n>>> * \\fn FrameContext::init()\n>>> * \\brief Initialize a frame context\n>>> * \\param[in] frameNum The frame number to assign to this FrameContext\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * This function initializes a frame context by assigning it a frame number.\n>>> * The single IPA modules are expected to override this function to initialize\n>>> @@ -117,9 +118,10 @@ namespace ipa {\n>>> */\n>>> /**\n>>> - * \\fn FCQueue::alloc(uint32_t frame)\n>>> + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n>>> * \\brief Allocate and return a FrameContext for the \\a frame\n>>> * \\param[in] frame The frame context sequence number\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * The first call to obtain a FrameContext from the FCQueue should be handled\n>>> * through this function. The FrameContext will be initialised, if not\n>>> @@ -135,12 +137,14 @@ namespace ipa {\n>>> */\n>>> /**\n>>> - * \\fn FCQueue::get(uint32_t frame)\n>>> + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n>>> * \\brief Obtain the FrameContext for the \\a frame\n>>> * \\param[in] frame The frame context sequence number\n>>> + * \\param[in] activeState The IPA current active state\n>>> *\n>>> * If the FrameContext is not correctly initialised for the \\a frame, it will be\n>>> - * initialised.\n>>> + * initialised using the most current state of IPA algorithm contained in\n>>> + * \\a activeState.\n>>> *\n>>> * \\return A reference to the FrameContext for sequence \\a frame\n>>> */\n>>> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n>>> index bfcce5a81356..48842e54cd35 100644\n>>> --- a/src/ipa/libipa/fc_queue.h\n>>> +++ b/src/ipa/libipa/fc_queue.h\n>>> @@ -21,9 +21,16 @@ namespace ipa {\n>>> template\n>>> class FCQueue;\n>>> +struct ActiveState {\n>>> +};\n>>> +\n>>> struct FrameContext {\n>>> +public:\n>>> +\tvirtual ~FrameContext() = default;\n>>> +\n>> Is this addition related?\n> Not in this patch, but probably in 1/4 where I make the FrameContext\n> class derivable by introducing a virtual method.\n>\n> Thanks for spotting.\n> j\n>\n>>> protected:\n>>> -\tvirtual void init(const uint32_t frameNum)\n>>> +\tvirtual void init(const uint32_t frameNum,\n>>> +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n>>> \t{\n>>> \t\tframe = frameNum;\n>>> \t\tinitialised = true;\n>>> @@ -52,7 +59,7 @@ public:\n>>> \t\t}\n>>> \t}\n>>> -\tFrameContext &alloc(const uint32_t frame)\n>>> +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n>>> \t{\n>>> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n>>> @@ -71,12 +78,12 @@ public:\n>>> \t\t\tLOG(FCQueue, Warning)\n>>> \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n>>> \t\telse\n>>> -\t\t\tframeContext.init(frame);\n>>> +\t\t\tframeContext.init(frame, activeState);\n>>> \t\treturn frameContext;\n>>> \t}\n>>> -\tFrameContext &get(uint32_t frame)\n>>> +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n>>> \t{\n>>> \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n>>> @@ -103,7 +110,7 @@ public:\n>>> \t\t\t * Make sure the FrameContext gets initialised if get()\n>>> \t\t\t * is called before alloc() by the IPA for frame#0.\n>>> \t\t\t */\n>>> -\t\t\tframeContext.init(frame);\n>>> +\t\t\tframeContext.init(frame, activeState);\n>>> \t\t\treturn frameContext;\n>>> \t\t}\n>>> @@ -123,7 +130,7 @@ public:\n>>> \t\tLOG(FCQueue, Warning)\n>>> \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n>>> -\t\tframeContext.init(frame);\n>>> +\t\tframeContext.init(frame, activeState);\n>>> \t\treturn frameContext;\n>>> \t}\n>>> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n>>> index 4e4fe5f4ae96..2dad42b3154f 100644\n>>> --- a/src/ipa/rkisp1/ipa_context.cpp\n>>> +++ b/src/ipa/rkisp1/ipa_context.cpp\n>>> @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n>>> * \\brief Analogue gain multiplier\n>>> */\n>>> -void IPAFrameContext::init(const uint32_t frameNum)\n>>> +void IPAFrameContext::init(const uint32_t frameNum,\n>>> +\t\t\t const ActiveState &activeState)\n>>> {\n>>> -\tFrameContext::init(frameNum);\n>>> +\tFrameContext::init(frameNum, activeState);\n>>> }\n>>> /**\n>>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>>> index 51e04bc30627..f708b32111af 100644\n>>> --- a/src/ipa/rkisp1/ipa_context.h\n>>> +++ b/src/ipa/rkisp1/ipa_context.h\n>>> @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n>>> \tuint32_t paramFormat;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tstruct {\n>>> \t\t\tuint32_t exposure;\n>>> @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n>>> \t\tMatrix ccm;\n>>> \t} ccm;\n>>> -\tvoid init(const uint32_t frame) override;\n>>> +\tvoid init(const uint32_t frame,\n>>> +\t\t const ActiveState &activeState) override;\n>>> };\n>>> struct IPAContext {\n>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>> index 9e161cabdea4..7c1cefc1c7fa 100644\n>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>> @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>>> void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &a : algorithms()) {\n>>> \t\tAlgorithm *algo = static_cast(a.get());\n>>> @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tRkISP1Params params(context_.configuration.paramFormat,\n>>> \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n>>> @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n>>> void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n>>> \t\t\t\t const ControlList &sensorControls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \t/*\n>>> \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n>>> @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n>>> \t * internal sensor delays and other timing parameters into account.\n>>> \t */\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tuint32_t exposure = frameContext.agc.exposure;\n>>> \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n>>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>>> index 3519f20f6415..956f4fb71abf 100644\n>>> --- a/src/ipa/simple/ipa_context.h\n>>> +++ b/src/ipa/simple/ipa_context.h\n>>> @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n>>> \t} agc;\n>>> };\n>>> -struct IPAActiveState {\n>>> +struct IPAActiveState : public ActiveState {\n>>> \tstruct {\n>>> \t\tuint8_t level;\n>>> \t} blc;\n>>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n>>> index b28c7039f7bd..71b31d728117 100644\n>>> --- a/src/ipa/simple/soft_simple.cpp\n>>> +++ b/src/ipa/simple/soft_simple.cpp\n>>> @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n>>> void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->queueRequest(context_, frame, frameContext, controls);\n>>> @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n>>> void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tfor (auto const &algo : algorithms())\n>>> \t\talgo->prepare(context_, frame, frameContext, params_);\n>>> \tsetIspParams.emit();\n>>> @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>>> \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n>>> \t\t\t\t const ControlList &sensorControls)\n>>> {\n>>> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>>> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n>>> +\t\t\t\t\t\t\t\t context_.activeState);\n>>> \tframeContext.sensor.exposure =\n>>> \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 0AD6AC3220\n\tfor ;\n\tTue, 29 Oct 2024 09:51:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE1C7653A0;\n\tTue, 29 Oct 2024 10:51:46 +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 2906360366\n\tfor ;\n\tTue, 29 Oct 2024 10:51:44 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C203AF;\n\tTue, 29 Oct 2024 10:51:41 +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=\"EjtiOW25\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730195501;\n\tbh=/DmngILer9coHwKDl06dsy2gScfiMkK2LW8LW9HPxsg=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=EjtiOW25OxOCS8NCIiG6sJJpX+035OUTlpnVedV+wXNTzW0Fup98EUZXCfeVy5vke\n\t01MLcrjzmXNsKNg0CzClywPqqyMdrcDVWKvvCTL0BzUG+sWUCBdpYF3zg1NYnmg8rZ\n\tu4RLbUBRNEM88T1HrAcy9JY5BVWK/q8ECYvoV8e4=","Message-ID":"<282fcf62-9541-4266-b001-5740d4308cac@ideasonboard.com>","Date":"Tue, 29 Oct 2024 09:51:40 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>\n\t<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":31959,"web_url":"https://patchwork.libcamera.org/comment/31959/","msgid":"<22rxpdr7bx5bf7lqz5sjvqzcq5fpuyucyv3fvsphgoybb6bscr@vkegk4s3xad5>","date":"2024-10-29T10:51:37","subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Tue, Oct 29, 2024 at 09:51:03AM +0000, Dan Scally wrote:\n> Hi Jacopo\n>\n> On 28/10/2024 10:56, Jacopo Mondi wrote:\n> > Hi Dan\n> >\n> > On Mon, Oct 28, 2024 at 10:27:02AM +0000, Dan Scally wrote:\n> > > Hi Jacopo\n> > >\n> > > On 16/10/2024 18:03, Jacopo Mondi wrote:\n> > > > Pass to the FCQueue the algorithm's active state to use the most\n> > > > recent state of IPA algorithms to initialize a FrameContext.\n> > > >\n> > > > Modify all IPA modules that use libipa to pass a const ActiveState\n> > > > reference to the FCQueue function and make their IPAActiveState\n> > > > implementation derive a base ActiveState structure.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/ipa/ipu3/ipa_context.h | 2 +-\n> > > > src/ipa/ipu3/ipu3.cpp | 9 ++++++---\n> > > > src/ipa/libipa/fc_queue.cpp | 10 +++++++---\n> > > > src/ipa/libipa/fc_queue.h | 19 +++++++++++++------\n> > > > src/ipa/rkisp1/ipa_context.cpp | 5 +++--\n> > > > src/ipa/rkisp1/ipa_context.h | 5 +++--\n> > > > src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++----\n> > > > src/ipa/simple/ipa_context.h | 2 +-\n> > > > src/ipa/simple/soft_simple.cpp | 9 ++++++---\n> > > > 9 files changed, 48 insertions(+), 25 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > > index c85d1e34ea85..526af2ac0b06 100644\n> > > > --- a/src/ipa/ipu3/ipa_context.h\n> > > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > > @@ -46,7 +46,7 @@ struct IPASessionConfiguration {\n> > > > \t} sensor;\n> > > > };\n> > > > -struct IPAActiveState {\n> > > > +struct IPAActiveState : public ActiveState {\n> > > > \tstruct {\n> > > > \t\tuint32_t focus;\n> > > > \t\tdouble maxVariance;\n> > > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > > index 10a8c86d8e64..84c443a009fd 100644\n> > > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > > @@ -561,7 +561,8 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > > > \t */\n> > > > \tparams->use = {};\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tfor (auto const &algo : algorithms())\n> > > > \t\talgo->prepare(context_, frame, frameContext, params);\n> > > > @@ -594,7 +595,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > > \tconst ipu3_uapi_stats_3a *stats =\n> > > > \t\treinterpret_cast(mem.data());\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > I get why it needs to be done, but I don't particularly like that we then\n> > > have to pass activeState to .get()...do we know what kinds of situations\n> > Because it's more letters to type or for other reasons ?\n>\n>\n> More because it feels like we're stacking workarounds on top of a problem\n> that it would be better to avoid if possible, though it may not be\n> reasonably possible, in which case c'est la vie.\n>\n> >\n> > > result in .get() calls before .alloc() calls for a FrameContext?\n> > I'll explain my use case, which is detailed in 4/4.\n> >\n> > Currently the ph/ipa interactions are clocked by a user submitting a\n> > Request. A Request gets queued, it alloc() a frame context and\n> > it goes through algo->queueRequest(). AGC::queueRequest() initializes\n> > FrameContext.agc.meteringMode using the active state.\n> >\n> > Now, we move towards a model where the IPA is instead clocked by\n> > frames being produced by the sensor and thus we can have Request\n> > underruns if an app doesn't queue requests fast enough to keep it up\n> > with the sensor's frame rate. In this case we'll go through\n> > processStats() without going through IPA::queueRequest. In this case\n> > we get() a frame context, and if we get it before it has been\n> > allocated, we should make sure it can be processed by algo->process()\n> > without causing issues as the one that 4/4 fixes.\n>\n> Yeah ok. Do we get those Request underruns in the current implementation? I\n\nNope, because we clock the IPA with user provided Requests, so they\ncan't underrun :)\n\n> lean towards the idea that perhaps alloc()ing the FrameContext in\n> queueRequest() is the wrong approach when the IPA is clocked by the sensor,\n> and perhaps we ought to switch to indexing the FCQueue using the sensor\n> frame number...but I imagine that's a large bit of work.\n>\n\nthat's indeed where we want to go, all the ph/ipa/fcq interface\nshould be modeled around the HW frame sequence number not the user\nrequest number.\n\nAnd yes, I presume we want to rework the IPA/FCQ interface for\nalloc/get to better handle this, maybe involving algos as Stefan\nsuggested for the frame context initialisation.\n\nI can have a look at how that would look like and I'm open to\nsuggestions ;)\n\n> >\n> > I presume however, that even if we get through an alloc() making sure\n> > that the initialization made by algorithms using the active state are\n> > handled in IPAFrameContext::init() we would probably get more robust\n> > code, making sure the algorithms' implementation do not rely on too\n> > many internal state-keeping and instead work on what's implemented in\n> > IPAFrameContext.\n> Probably true.\n\nRight now I'm debated between two models.\n\nIn one, where algos are more seen like pluggable modules, the\ninitialization of FrameContext is done by the FCQ using the active\nstate, as done in this patch, to make sure the FrameContext has all the\ninformation required to be processed by all algorithms\nimplementations. This would make it easier to swap algos but might\nlead to a more bloated (and costly) initialization in the FCQ\nimplementation.\n\nOn the other side, if we let algos initialize FrameContext when we\nget/alloc it (using the ActiveState if the want to or defaults), the\nalgo implementation is self contained as there will be assumption made\nin process()/prepare() on the initialization done with the algorithm\nimplementation itself.\n\nI was more leaning towards the former, but maybe the latter is easier\nand more self-contained. Opinions ?\n\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> > > > @@ -627,7 +629,8 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> > > > */\n> > > > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tfor (auto const &algo : algorithms())\n> > > > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > > > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > > > index fa2454fd5706..56c7c75a8f48 100644\n> > > > --- a/src/ipa/libipa/fc_queue.cpp\n> > > > +++ b/src/ipa/libipa/fc_queue.cpp\n> > > > @@ -42,6 +42,7 @@ namespace ipa {\n> > > > * \\fn FrameContext::init()\n> > > > * \\brief Initialize a frame context\n> > > > * \\param[in] frameNum The frame number to assign to this FrameContext\n> > > > + * \\param[in] activeState The IPA current active state\n> > > > *\n> > > > * This function initializes a frame context by assigning it a frame number.\n> > > > * The single IPA modules are expected to override this function to initialize\n> > > > @@ -117,9 +118,10 @@ namespace ipa {\n> > > > */\n> > > > /**\n> > > > - * \\fn FCQueue::alloc(uint32_t frame)\n> > > > + * \\fn FCQueue::alloc(uint32_t frame, const ActiveState &activeState)\n> > > > * \\brief Allocate and return a FrameContext for the \\a frame\n> > > > * \\param[in] frame The frame context sequence number\n> > > > + * \\param[in] activeState The IPA current active state\n> > > > *\n> > > > * The first call to obtain a FrameContext from the FCQueue should be handled\n> > > > * through this function. The FrameContext will be initialised, if not\n> > > > @@ -135,12 +137,14 @@ namespace ipa {\n> > > > */\n> > > > /**\n> > > > - * \\fn FCQueue::get(uint32_t frame)\n> > > > + * \\fn FCQueue::get(uint32_t frame, const ActiveState &activeState)\n> > > > * \\brief Obtain the FrameContext for the \\a frame\n> > > > * \\param[in] frame The frame context sequence number\n> > > > + * \\param[in] activeState The IPA current active state\n> > > > *\n> > > > * If the FrameContext is not correctly initialised for the \\a frame, it will be\n> > > > - * initialised.\n> > > > + * initialised using the most current state of IPA algorithm contained in\n> > > > + * \\a activeState.\n> > > > *\n> > > > * \\return A reference to the FrameContext for sequence \\a frame\n> > > > */\n> > > > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > > > index bfcce5a81356..48842e54cd35 100644\n> > > > --- a/src/ipa/libipa/fc_queue.h\n> > > > +++ b/src/ipa/libipa/fc_queue.h\n> > > > @@ -21,9 +21,16 @@ namespace ipa {\n> > > > template\n> > > > class FCQueue;\n> > > > +struct ActiveState {\n> > > > +};\n> > > > +\n> > > > struct FrameContext {\n> > > > +public:\n> > > > +\tvirtual ~FrameContext() = default;\n> > > > +\n> > > Is this addition related?\n> > > > protected:\n> > > > -\tvirtual void init(const uint32_t frameNum)\n> > > > +\tvirtual void init(const uint32_t frameNum,\n> > > > +\t\t\t [[maybe_unused]] const ActiveState &activeState)\n> > > > \t{\n> > > > \t\tframe = frameNum;\n> > > > \t\tinitialised = true;\n> > > > @@ -52,7 +59,7 @@ public:\n> > > > \t\t}\n> > > > \t}\n> > > > -\tFrameContext &alloc(const uint32_t frame)\n> > > > +\tFrameContext &alloc(const uint32_t frame, const ActiveState &activeState)\n> > > > \t{\n> > > > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > > > @@ -71,12 +78,12 @@ public:\n> > > > \t\t\tLOG(FCQueue, Warning)\n> > > > \t\t\t\t<< \"Frame \" << frame << \" already initialised\";\n> > > > \t\telse\n> > > > -\t\t\tframeContext.init(frame);\n> > > > +\t\t\tframeContext.init(frame, activeState);\n> > > > \t\treturn frameContext;\n> > > > \t}\n> > > > -\tFrameContext &get(uint32_t frame)\n> > > > +\tFrameContext &get(uint32_t frame, const ActiveState &activeState)\n> > > > \t{\n> > > > \t\tFrameContext &frameContext = contexts_[frame % contexts_.size()];\n> > > > @@ -103,7 +110,7 @@ public:\n> > > > \t\t\t * Make sure the FrameContext gets initialised if get()\n> > > > \t\t\t * is called before alloc() by the IPA for frame#0.\n> > > > \t\t\t */\n> > > > -\t\t\tframeContext.init(frame);\n> > > > +\t\t\tframeContext.init(frame, activeState);\n> > > > \t\t\treturn frameContext;\n> > > > \t\t}\n> > > > @@ -123,7 +130,7 @@ public:\n> > > > \t\tLOG(FCQueue, Warning)\n> > > > \t\t\t<< \"Obtained an uninitialised FrameContext for \" << frame;\n> > > > -\t\tframeContext.init(frame);\n> > > > +\t\tframeContext.init(frame, activeState);\n> > > > \t\treturn frameContext;\n> > > > \t}\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > > index 4e4fe5f4ae96..2dad42b3154f 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > > @@ -417,9 +417,10 @@ namespace libcamera::ipa::rkisp1 {\n> > > > * \\brief Analogue gain multiplier\n> > > > */\n> > > > -void IPAFrameContext::init(const uint32_t frameNum)\n> > > > +void IPAFrameContext::init(const uint32_t frameNum,\n> > > > +\t\t\t const ActiveState &activeState)\n> > > > {\n> > > > -\tFrameContext::init(frameNum);\n> > > > +\tFrameContext::init(frameNum, activeState);\n> > > > }\n> > > > /**\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > index 51e04bc30627..f708b32111af 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > @@ -64,7 +64,7 @@ struct IPASessionConfiguration {\n> > > > \tuint32_t paramFormat;\n> > > > };\n> > > > -struct IPAActiveState {\n> > > > +struct IPAActiveState : public ActiveState {\n> > > > \tstruct {\n> > > > \t\tstruct {\n> > > > \t\t\tuint32_t exposure;\n> > > > @@ -178,7 +178,8 @@ struct IPAFrameContext : public FrameContext {\n> > > > \t\tMatrix ccm;\n> > > > \t} ccm;\n> > > > -\tvoid init(const uint32_t frame) override;\n> > > > +\tvoid init(const uint32_t frame,\n> > > > +\t\t const ActiveState &activeState) override;\n> > > > };\n> > > > struct IPAContext {\n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index 9e161cabdea4..7c1cefc1c7fa 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -325,7 +325,8 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> > > > void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tfor (auto const &a : algorithms()) {\n> > > > \t\tAlgorithm *algo = static_cast(a.get());\n> > > > @@ -337,7 +338,8 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> > > > void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tRkISP1Params params(context_.configuration.paramFormat,\n> > > > \t\t\t mappedBuffers_.at(bufferId).planes()[0]);\n> > > > @@ -351,7 +353,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> > > > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> > > > \t\t\t\t const ControlList &sensorControls)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \t/*\n> > > > \t * In raw capture mode, the ISP is bypassed and no statistics buffer is\n> > > > @@ -447,7 +450,8 @@ void IPARkISP1::setControls(unsigned int frame)\n> > > > \t * internal sensor delays and other timing parameters into account.\n> > > > \t */\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tuint32_t exposure = frameContext.agc.exposure;\n> > > > \tuint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);\n> > > > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > > > index 3519f20f6415..956f4fb71abf 100644\n> > > > --- a/src/ipa/simple/ipa_context.h\n> > > > +++ b/src/ipa/simple/ipa_context.h\n> > > > @@ -24,7 +24,7 @@ struct IPASessionConfiguration {\n> > > > \t} agc;\n> > > > };\n> > > > -struct IPAActiveState {\n> > > > +struct IPAActiveState : public ActiveState {\n> > > > \tstruct {\n> > > > \t\tuint8_t level;\n> > > > \t} blc;\n> > > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > > > index b28c7039f7bd..71b31d728117 100644\n> > > > --- a/src/ipa/simple/soft_simple.cpp\n> > > > +++ b/src/ipa/simple/soft_simple.cpp\n> > > > @@ -249,7 +249,8 @@ void IPASoftSimple::stop()\n> > > > void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tfor (auto const &algo : algorithms())\n> > > > \t\talgo->queueRequest(context_, frame, frameContext, controls);\n> > > > @@ -257,7 +258,8 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro\n> > > > void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tfor (auto const &algo : algorithms())\n> > > > \t\talgo->prepare(context_, frame, frameContext, params_);\n> > > > \tsetIspParams.emit();\n> > > > @@ -267,7 +269,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> > > > \t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n> > > > \t\t\t\t const ControlList &sensorControls)\n> > > > {\n> > > > -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> > > > +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame,\n> > > > +\t\t\t\t\t\t\t\t context_.activeState);\n> > > > \tframeContext.sensor.exposure =\n> > > > \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();","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 3630BC3220\n\tfor ;\n\tTue, 29 Oct 2024 10:51:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 520E06539E;\n\tTue, 29 Oct 2024 11:51:44 +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 74E8260366\n\tfor ;\n\tTue, 29 Oct 2024 11:51:42 +0100 (CET)","from ideasonboard.com (mob-5-90-50-182.net.vodafone.it\n\t[5.90.50.182])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 843514D4;\n\tTue, 29 Oct 2024 11:51:39 +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=\"GwUOEZpF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730199100;\n\tbh=Yym818bO5U0FKsTd17hdsx99m/76eGVSEApc0WTymFQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GwUOEZpFfFadEMyI+fod5ugIHC6Qn5oSZkPp7SvXZECTN1uhvmHWEb4+0o0WopJ39\n\tLlxKx7hL47uTOJxWSMELQpTOmirA/VJKe2z43RMK/dQfsSIyztL3+uMgjthncKMJZ/\n\tUIT1ijiDMxHKfjb0VjjYTBGYYFYHud8nN8tDl2Yw=","Date":"Tue, 29 Oct 2024 11:51:37 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/4] libipa: FCQueue: Initialize FrameContext with\n\tactiveState","Message-ID":"<22rxpdr7bx5bf7lqz5sjvqzcq5fpuyucyv3fvsphgoybb6bscr@vkegk4s3xad5>","References":"<20241016170348.715993-1-jacopo.mondi@ideasonboard.com>\n\t<20241016170348.715993-4-jacopo.mondi@ideasonboard.com>\n\t<3f4ae231-d750-458b-9d2c-67a60d968263@ideasonboard.com>\n\t\n\t<8c713e46-63b8-41ed-acab-7a313c426b31@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<8c713e46-63b8-41ed-acab-7a313c426b31@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\" "}}]