[{"id":24065,"web_url":"https://patchwork.libcamera.org/comment/24065/","msgid":"<431841f3-9199-06ed-15e3-0bfb633214c7@ideasonboard.com>","date":"2022-07-22T21:03:49","subject":"Re: [libcamera-devel] [RFC PATCH 06/12] ipa: libipa: Provide a\n\tcommon base for FrameContexts","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 7/21/22 17:43, Kieran Bingham via libcamera-devel wrote:\n> Provide a common IPAFrameContext as a base for IPA modules to inherit\n> from.\n>\n> This will allow having a common set of parameters for every FrameContext.\n>\n> We still have to use a templated instance of the FCQueue though to make\n> sure we allocate the correct types.\n>\n> Both the RKISP1 and the IPU3 create IPA specific FrameContext structures\n> which inherit from the IPAFrameContext.\n>\n> While adjusting interface to Algorithm::process() the FrameContext is\n> converted from a pointer to a reference.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nLooks good!\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n> ---\n>   src/ipa/ipu3/algorithms/af.cpp           |  3 ++-\n>   src/ipa/ipu3/algorithms/af.h             |  2 +-\n>   src/ipa/ipu3/algorithms/agc.cpp          |  9 ++++----\n>   src/ipa/ipu3/algorithms/agc.h            |  4 ++--\n>   src/ipa/ipu3/algorithms/awb.cpp          |  3 ++-\n>   src/ipa/ipu3/algorithms/awb.h            |  2 +-\n>   src/ipa/ipu3/algorithms/tone_mapping.cpp |  3 ++-\n>   src/ipa/ipu3/algorithms/tone_mapping.h   |  2 +-\n>   src/ipa/ipu3/ipa_context.cpp             | 26 +++++-------------------\n>   src/ipa/ipu3/ipa_context.h               |  5 ++---\n>   src/ipa/ipu3/ipu3.cpp                    |  6 +++---\n>   src/ipa/ipu3/module.h                    |  2 +-\n>   src/ipa/libipa/algorithm.h               |  2 +-\n>   src/ipa/libipa/fc_queue.cpp              | 21 +++++++++++++++++++\n>   src/ipa/libipa/fc_queue.h                |  6 ++++++\n>   src/ipa/rkisp1/algorithms/agc.cpp        |  2 +-\n>   src/ipa/rkisp1/algorithms/agc.h          |  2 +-\n>   src/ipa/rkisp1/algorithms/awb.cpp        |  2 +-\n>   src/ipa/rkisp1/algorithms/awb.h          |  2 +-\n>   src/ipa/rkisp1/ipa_context.h             |  4 +++-\n>   src/ipa/rkisp1/module.h                  |  2 +-\n>   src/ipa/rkisp1/rkisp1.cpp                |  5 ++++-\n>   22 files changed, 67 insertions(+), 48 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index d07521a090ac..fcbf8e557b10 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -420,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context)\n>    *\n>    * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing\n>    */\n> -void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Af::process(IPAContext &context,\n> +\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n>   \t\t const ipu3_uapi_stats_3a *stats)\n>   {\n>   \t/* Evaluate the AF buffer length */\n> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> index ccf015f3f8f5..7a5aeb1b6bf4 100644\n> --- a/src/ipa/ipu3/algorithms/af.h\n> +++ b/src/ipa/ipu3/algorithms/af.h\n> @@ -32,7 +32,7 @@ public:\n>   \n>   \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n>   \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t     const ipu3_uapi_stats_3a *stats) override;\n>   \n>   private:\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 5bc64ae52214..92ea6249798d 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -183,13 +183,13 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n>    * \\param[in] yGain The gain calculated based on the relative luminance target\n>    * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n>    */\n> -void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext,\n> +void Agc::computeExposure(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t\t  double yGain, double iqMeanGain)\n>   {\n>   \tconst IPASessionConfiguration &configuration = context.configuration;\n>   \t/* Get the effective exposure and gain applied on the sensor. */\n> -\tuint32_t exposure = frameContext->sensor.exposure;\n> -\tdouble analogueGain = frameContext->sensor.gain;\n> +\tuint32_t exposure = frameContext.sensor.exposure;\n> +\tdouble analogueGain = frameContext.sensor.gain;\n>   \n>   \t/* Use the highest of the two gain estimates. */\n>   \tdouble evGain = std::max(yGain, iqMeanGain);\n> @@ -323,7 +323,8 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n>    * Identify the current image brightness, and use that to estimate the optimal\n>    * new exposure and gain for the scene.\n>    */\n> -void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Agc::process(IPAContext &context,\n> +\t\t  IPU3FrameContext &frameContext,\n>   \t\t  const ipu3_uapi_stats_3a *stats)\n>   {\n>   \t/*\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 105ae0f2aac6..96585f48933f 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -28,14 +28,14 @@ public:\n>   \t~Agc() = default;\n>   \n>   \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t     const ipu3_uapi_stats_3a *stats) override;\n>   \n>   private:\n>   \tdouble measureBrightness(const ipu3_uapi_stats_3a *stats,\n>   \t\t\t\t const ipu3_uapi_grid_config &grid) const;\n>   \tutils::Duration filterExposure(utils::Duration currentExposure);\n> -\tvoid computeExposure(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid computeExposure(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t\t     double yGain, double iqMeanGain);\n>   \tdouble estimateLuminance(IPAActiveState &activeState,\n>   \t\t\t\t const ipu3_uapi_grid_config &grid,\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 704267222a31..45f8e5f29119 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -387,7 +387,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n>   /**\n>    * \\copydoc libcamera::ipa::Algorithm::process\n>    */\n> -void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Awb::process(IPAContext &context,\n> +\t\t  [[maybe_unused]] IPU3FrameContext &frameContext,\n>   \t\t  const ipu3_uapi_stats_3a *stats)\n>   {\n>   \tcalculateWBGains(stats);\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 0acd21480845..28e39620fd59 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -40,7 +40,7 @@ public:\n>   \n>   \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>   \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t     const ipu3_uapi_stats_3a *stats) override;\n>   \n>   private:\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index f86e79b24a67..977741c5a4d4 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -78,7 +78,8 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>    * The tone mapping look up table is generated as an inverse power curve from\n>    * our gamma setting.\n>    */\n> -void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void ToneMapping::process(IPAContext &context,\n> +\t\t\t  [[maybe_unused]] IPU3FrameContext &frameContext,\n>   \t\t\t  [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n>   {\n>   \t/*\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h\n> index d7d4800628f2..8cce38c742a6 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.h\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h\n> @@ -20,7 +20,7 @@ public:\n>   \n>   \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>   \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>   \t\t     const ipu3_uapi_stats_3a *stats) override;\n>   \n>   private:\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index dd139cb4c868..536d76ecd63b 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n>    * most recently computed by the IPA algorithms.\n>    */\n>   \n> -/**\n> - * \\struct IPAFrameContext\n> - * \\brief Context for a frame\n> - *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> - *\n> - * Fields in the frame context should reflect values and controls\n> - * associated with the specific frame as requested by the application, and\n> - * as configured by the hardware. Fields can be read by algorithms to\n> - * determine if they should update any specific action for this frame, and\n> - * finally to update the metadata control lists when the frame is fully\n> - * completed.\n> - */\n> -\n>   /**\n>    * \\struct IPAContext\n>    * \\brief Global IPA context data shared between all algorithms\n> @@ -181,16 +165,16 @@ namespace libcamera::ipa::ipu3 {\n>    */\n>   \n>   /**\n> - * \\var IPAFrameContext::frame\n> - * \\brief The frame number\n> + * \\struct IPU3FrameContext\n> + * \\copybrief libcamera::ipa::IPAFrameContext\n>    *\n> - * \\var IPAFrameContext::sensor\n> + * \\var IPU3FrameContext::sensor\n>    * \\brief Effective sensor values that were applied for the frame\n>    *\n> - * \\var IPAFrameContext::sensor.exposure\n> + * \\var IPU3FrameContext::sensor.exposure\n>    * \\brief Exposure time expressed as a number of lines\n>    *\n> - * \\var IPAFrameContext::sensor.gain\n> + * \\var IPU3FrameContext::sensor.gain\n>    * \\brief Analogue gain multiplier\n>    */\n>   \n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 193fc44a821a..c4aa4c3f4f6a 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -73,8 +73,7 @@ struct IPAActiveState {\n>   \t} toneMapping;\n>   };\n>   \n> -struct IPAFrameContext {\n> -\tuint32_t frame;\n> +struct IPU3FrameContext : public IPAFrameContext {\n>   \n>   \tstruct {\n>   \t\tuint32_t exposure;\n> @@ -86,7 +85,7 @@ struct IPAContext {\n>   \tIPASessionConfiguration configuration;\n>   \tIPAActiveState activeState;\n>   \n> -\tFCQueue<IPAFrameContext> frameContexts;\n> +\tFCQueue<IPU3FrameContext> frameContexts;\n>   };\n>   \n>   } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 55e82cd404f4..489e51bc6cf7 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -570,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>   \tconst ipu3_uapi_stats_3a *stats =\n>   \t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n>   \n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPU3FrameContext &frameContext = context_.frameContexts.get(frame);\n>   \n>   \tif (frameContext.frame != frame)\n>   \t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> @@ -583,7 +583,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>   \tControlList ctrls(controls::controls);\n>   \n>   \tfor (auto const &algo : algorithms_)\n> -\t\talgo->process(context_, &frameContext, stats);\n> +\t\talgo->process(context_, frameContext, stats);\n>   \n>   \tsetControls(frame);\n>   \n> @@ -619,7 +619,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>   void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n>   {\n>   \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> -\tIPAFrameContext &frameContext = context_.frameContexts.initialise(frame);\n> +\tIPU3FrameContext &frameContext = context_.frameContexts.initialise(frame);\n>   \n>   \t/* \\todo Implement queueRequest to each algorithm. */\n>   \t(void)frameContext;\n> diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h\n> index d94fc4594871..6d0d50f615d8 100644\n> --- a/src/ipa/ipu3/module.h\n> +++ b/src/ipa/ipu3/module.h\n> @@ -19,7 +19,7 @@ namespace libcamera {\n>   \n>   namespace ipa::ipu3 {\n>   \n> -using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo,\n> +using Module = ipa::Module<IPAContext, IPU3FrameContext, IPAConfigInfo,\n>   \t\t\t   ipu3_uapi_params, ipu3_uapi_stats_3a>;\n>   \n>   } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index 2a8871d831a3..adbcf69adb4f 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -41,7 +41,7 @@ public:\n>   \t}\n>   \n>   \tvirtual void process([[maybe_unused]] typename Module::Context &context,\n> -\t\t\t     [[maybe_unused]] typename Module::FrameContext *frameContext,\n> +\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n>   \t\t\t     [[maybe_unused]] const typename Module::Stats *stats)\n>   \t{\n>   \t}\n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index ecb47f287350..627c2fa3301e 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -55,6 +55,27 @@ namespace ipa {\n>    * the Request when it completes.\n>    */\n>   \n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Context for a frame\n> + *\n> + * The frame context stores data specific to a single frame processed by the\n> + * IPA. Each frame processed by the IPA has a context associated with it,\n> + * accessible through the Frame Context Queue.\n> + *\n> + * Fields in the frame context should reflect values and controls associated\n> + * with the specific frame as requested by the application, and as configured by\n> + * the hardware. Fields can be read by algorithms to determine if they should\n> + * update any specific action for this frame, and finally to update the metadata\n> + * control lists when the frame is fully completed.\n> + *\n> + * \\var IPAFrameContext::frame\n> + * \\brief The frame number\n> + *\n> + * \\var IPAFrameContext::error\n> + * \\brief The error flags associated with the frame\n> + */\n> +\n>   /**\n>    * \\fn FCQueue::clear()\n>    * \\brief Reinitialise all data on the queue\n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index 82ce36811926..6c7b8bf11ab2 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -13,6 +13,7 @@\n>   #include <libcamera/base/log.h>\n>   \n>   #include <libcamera/controls.h>\n> +#include <libcamera/request.h>\n>   \n>   namespace libcamera {\n>   \n> @@ -27,6 +28,11 @@ namespace ipa {\n>    */\n>   static constexpr uint32_t kMaxFrameContexts = 16;\n>   \n> +struct IPAFrameContext {\n> +\tunsigned int frame;\n> +\tRequest::ErrorFlags error;\n> +};\n> +\n>   template<typename FrameContext>\n>   class FCQueue : public std::array<FrameContext, kMaxFrameContexts>\n>   {\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 483e941fe427..40d27963ef68 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -281,7 +281,7 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n>    * new exposure and gain for the scene.\n>    */\n>   void Agc::process(IPAContext &context,\n> -\t\t  [[maybe_unused]] IPAFrameContext *frameContext,\n> +\t\t  [[maybe_unused]] RKISP1FrameContext &frameContext,\n>   \t\t  const rkisp1_stat_buffer *stats)\n>   {\n>   \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 22c02779d311..1a291bdf3636 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -29,7 +29,7 @@ public:\n>   \n>   \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n>   \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, RKISP1FrameContext &frameContext,\n>   \t\t     const rkisp1_stat_buffer *stats) override;\n>   \n>   private:\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 427aaeb1e955..3beafb73ca33 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -120,7 +120,7 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n>    * \\copydoc libcamera::ipa::Algorithm::process\n>    */\n>   void Awb::process([[maybe_unused]] IPAContext &context,\n> -\t\t  [[maybe_unused]] IPAFrameContext *frameCtx,\n> +\t\t  [[maybe_unused]] RKISP1FrameContext &frameCtx,\n>   \t\t  const rkisp1_stat_buffer *stats)\n>   {\n>   \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 7647842f6609..259b85eebdb4 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -23,7 +23,7 @@ public:\n>   \n>   \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n>   \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameCtx,\n> +\tvoid process(IPAContext &context, RKISP1FrameContext &frameCtx,\n>   \t\t     const rkisp1_stat_buffer *stats) override;\n>   \n>   private:\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 341fef2e68fe..a64dbc75fdd2 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -14,6 +14,8 @@\n>   \n>   #include <libcamera/geometry.h>\n>   \n> +#include <libipa/fc_queue.h>\n> +\n>   namespace libcamera {\n>   \n>   namespace ipa::rkisp1 {\n> @@ -64,7 +66,7 @@ struct IPAActiveState {\n>   \tunsigned int frameCount;\n>   };\n>   \n> -struct IPAFrameContext {\n> +struct RKISP1FrameContext : public IPAFrameContext {\n>   };\n>   \n>   struct IPAContext {\n> diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h\n> index 89f83208a75c..2568c624df0f 100644\n> --- a/src/ipa/rkisp1/module.h\n> +++ b/src/ipa/rkisp1/module.h\n> @@ -19,7 +19,7 @@ namespace libcamera {\n>   \n>   namespace ipa::rkisp1 {\n>   \n> -using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,\n> +using Module = ipa::Module<IPAContext, RKISP1FrameContext, IPACameraSensorInfo,\n>   \t\t\t   rkisp1_params_cfg, rkisp1_stat_buffer>;\n>   \n>   } /* namespace ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 9629ccbf4247..7481e67e70f6 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -301,8 +301,11 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>   \n>   \tunsigned int aeState = 0;\n>   \n> +\t/* \\todo Obtain the frame context to pass to process from the FCQueue */\n> +\tRKISP1FrameContext frameContext;\n> +\n>   \tfor (auto const &algo : algorithms())\n> -\t\talgo->process(context_, nullptr, stats);\n> +\t\talgo->process(context_, frameContext, stats);\n>   \n>   \tsetControls(frame);\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3B78ABE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Jul 2022 21:03:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9311A6330F;\n\tFri, 22 Jul 2022 23:03:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07078601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Jul 2022 23:03:56 +0200 (CEST)","from [IPV6:2401:4900:1f3f:a2d:c6dd:7a7c:3d3c:dbb9] (unknown\n\t[IPv6:2401:4900:1f3f:a2d:c6dd:7a7c:3d3c:dbb9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C23F66D5;\n\tFri, 22 Jul 2022 23:03:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658523837;\n\tbh=verOjPGGupRuZMHAhwu8Nx6tpArVCW30UYZ7q0zoW5w=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=rgSmGII/7YZylbWVLcq87lbbdE5310yh7ACEXmg3qT5o9CgDnLoSo3Ks+tiR/Twn+\n\t9Xx/ixWe8N876ChjCJq0DS+kHlMwrO9p2A1NBmKNIkOZpJ/lD6rGyK3wLMSmonb1Ht\n\tJfvDIVtidSorNuRPvfeob4nUK8NugDIm8MvMbebgmJdqCBeL/6m8zqxENGkKAXKVmE\n\t+2yziP1FBMbrQDGTN8/xh34vxtSfIbb+bHLDD2q+snwaUmDjHNo8buFBTWPF5BGrEF\n\tmbpoF8l+IrmljvOqCT649QC+c4tFzem7qSD3d5eDGTMus6BCDWGp1PRv/7gSqe54PS\n\t6HgUYvZ+e0ltg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658523835;\n\tbh=verOjPGGupRuZMHAhwu8Nx6tpArVCW30UYZ7q0zoW5w=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=JHW1opmOo7zCsknpZhIHXuyYrnBFb9e8/qVZc3sm4ImjZjpusQipdqbMjqmOHs2x0\n\tWGhawGSsG2qLVbJc2GhuVNg6RegJ8/WfjTsgIX7sn17jSV6yySKsLNTxWwjd1WKHkl\n\tF/LEHFfy3JCVbci6PnP/K9rdWu3B+dnP0ElrVNA0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JHW1opmO\"; dkim-atps=neutral","Message-ID":"<431841f3-9199-06ed-15e3-0bfb633214c7@ideasonboard.com>","Date":"Sat, 23 Jul 2022 02:33:49 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-7-kieran.bingham@ideasonboard.com>","In-Reply-To":"<20220721121310.1286862-7-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH 06/12] ipa: libipa: Provide a\n\tcommon base for FrameContexts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24108,"web_url":"https://patchwork.libcamera.org/comment/24108/","msgid":"<20220725153034.sbvpyav7m4qolfhd@uno.localdomain>","date":"2022-07-25T15:30:34","subject":"Re: [libcamera-devel] [RFC PATCH 06/12] ipa: libipa: Provide a\n\tcommon base for FrameContexts","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Thu, Jul 21, 2022 at 01:13:04PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Provide a common IPAFrameContext as a base for IPA modules to inherit\n> from.\n>\n> This will allow having a common set of parameters for every FrameContext.\n>\n> We still have to use a templated instance of the FCQueue though to make\n> sure we allocate the correct types.\n>\n> Both the RKISP1 and the IPU3 create IPA specific FrameContext structures\n> which inherit from the IPAFrameContext.\n>\n> While adjusting interface to Algorithm::process() the FrameContext is\n> converted from a pointer to a reference.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/af.cpp           |  3 ++-\n>  src/ipa/ipu3/algorithms/af.h             |  2 +-\n>  src/ipa/ipu3/algorithms/agc.cpp          |  9 ++++----\n>  src/ipa/ipu3/algorithms/agc.h            |  4 ++--\n>  src/ipa/ipu3/algorithms/awb.cpp          |  3 ++-\n>  src/ipa/ipu3/algorithms/awb.h            |  2 +-\n>  src/ipa/ipu3/algorithms/tone_mapping.cpp |  3 ++-\n>  src/ipa/ipu3/algorithms/tone_mapping.h   |  2 +-\n>  src/ipa/ipu3/ipa_context.cpp             | 26 +++++-------------------\n>  src/ipa/ipu3/ipa_context.h               |  5 ++---\n>  src/ipa/ipu3/ipu3.cpp                    |  6 +++---\n>  src/ipa/ipu3/module.h                    |  2 +-\n>  src/ipa/libipa/algorithm.h               |  2 +-\n>  src/ipa/libipa/fc_queue.cpp              | 21 +++++++++++++++++++\n>  src/ipa/libipa/fc_queue.h                |  6 ++++++\n>  src/ipa/rkisp1/algorithms/agc.cpp        |  2 +-\n>  src/ipa/rkisp1/algorithms/agc.h          |  2 +-\n>  src/ipa/rkisp1/algorithms/awb.cpp        |  2 +-\n>  src/ipa/rkisp1/algorithms/awb.h          |  2 +-\n>  src/ipa/rkisp1/ipa_context.h             |  4 +++-\n>  src/ipa/rkisp1/module.h                  |  2 +-\n>  src/ipa/rkisp1/rkisp1.cpp                |  5 ++++-\n>  22 files changed, 67 insertions(+), 48 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index d07521a090ac..fcbf8e557b10 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -420,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context)\n>   *\n>   * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing\n>   */\n> -void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Af::process(IPAContext &context,\n> +\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n>  \t\t const ipu3_uapi_stats_3a *stats)\n>  {\n>  \t/* Evaluate the AF buffer length */\n> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> index ccf015f3f8f5..7a5aeb1b6bf4 100644\n> --- a/src/ipa/ipu3/algorithms/af.h\n> +++ b/src/ipa/ipu3/algorithms/af.h\n> @@ -32,7 +32,7 @@ public:\n>\n>  \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n>  \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t     const ipu3_uapi_stats_3a *stats) override;\n>\n>  private:\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 5bc64ae52214..92ea6249798d 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -183,13 +183,13 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n>   * \\param[in] yGain The gain calculated based on the relative luminance target\n>   * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n>   */\n> -void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext,\n> +void Agc::computeExposure(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t\t  double yGain, double iqMeanGain)\n>  {\n>  \tconst IPASessionConfiguration &configuration = context.configuration;\n>  \t/* Get the effective exposure and gain applied on the sensor. */\n> -\tuint32_t exposure = frameContext->sensor.exposure;\n> -\tdouble analogueGain = frameContext->sensor.gain;\n> +\tuint32_t exposure = frameContext.sensor.exposure;\n> +\tdouble analogueGain = frameContext.sensor.gain;\n>\n>  \t/* Use the highest of the two gain estimates. */\n>  \tdouble evGain = std::max(yGain, iqMeanGain);\n> @@ -323,7 +323,8 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n>   * Identify the current image brightness, and use that to estimate the optimal\n>   * new exposure and gain for the scene.\n>   */\n> -void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Agc::process(IPAContext &context,\n> +\t\t  IPU3FrameContext &frameContext,\n>  \t\t  const ipu3_uapi_stats_3a *stats)\n>  {\n>  \t/*\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 105ae0f2aac6..96585f48933f 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -28,14 +28,14 @@ public:\n>  \t~Agc() = default;\n>\n>  \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t     const ipu3_uapi_stats_3a *stats) override;\n>\n>  private:\n>  \tdouble measureBrightness(const ipu3_uapi_stats_3a *stats,\n>  \t\t\t\t const ipu3_uapi_grid_config &grid) const;\n>  \tutils::Duration filterExposure(utils::Duration currentExposure);\n> -\tvoid computeExposure(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid computeExposure(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t\t     double yGain, double iqMeanGain);\n>  \tdouble estimateLuminance(IPAActiveState &activeState,\n>  \t\t\t\t const ipu3_uapi_grid_config &grid,\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 704267222a31..45f8e5f29119 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -387,7 +387,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n>  /**\n>   * \\copydoc libcamera::ipa::Algorithm::process\n>   */\n> -void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void Awb::process(IPAContext &context,\n> +\t\t  [[maybe_unused]] IPU3FrameContext &frameContext,\n>  \t\t  const ipu3_uapi_stats_3a *stats)\n>  {\n>  \tcalculateWBGains(stats);\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 0acd21480845..28e39620fd59 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -40,7 +40,7 @@ public:\n>\n>  \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>  \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t     const ipu3_uapi_stats_3a *stats) override;\n>\n>  private:\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index f86e79b24a67..977741c5a4d4 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -78,7 +78,8 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n>   * The tone mapping look up table is generated as an inverse power curve from\n>   * our gamma setting.\n>   */\n> -void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext,\n> +void ToneMapping::process(IPAContext &context,\n> +\t\t\t  [[maybe_unused]] IPU3FrameContext &frameContext,\n>  \t\t\t  [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n>  {\n>  \t/*\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h\n> index d7d4800628f2..8cce38c742a6 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.h\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h\n> @@ -20,7 +20,7 @@ public:\n>\n>  \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>  \tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n>  \t\t     const ipu3_uapi_stats_3a *stats) override;\n>\n>  private:\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index dd139cb4c868..536d76ecd63b 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 {\n>   * most recently computed by the IPA algorithms.\n>   */\n>\n> -/**\n> - * \\struct IPAFrameContext\n> - * \\brief Context for a frame\n> - *\n> - * The frame context stores data specific to a single frame processed by the\n> - * IPA. Each frame processed by the IPA has a context associated with it,\n> - * accessible through the IPAContext structure.\n> - *\n> - * Fields in the frame context should reflect values and controls\n> - * associated with the specific frame as requested by the application, and\n> - * as configured by the hardware. Fields can be read by algorithms to\n> - * determine if they should update any specific action for this frame, and\n> - * finally to update the metadata control lists when the frame is fully\n> - * completed.\n> - */\n> -\n>  /**\n>   * \\struct IPAContext\n>   * \\brief Global IPA context data shared between all algorithms\n> @@ -181,16 +165,16 @@ namespace libcamera::ipa::ipu3 {\n>   */\n>\n>  /**\n> - * \\var IPAFrameContext::frame\n> - * \\brief The frame number\n> + * \\struct IPU3FrameContext\n> + * \\copybrief libcamera::ipa::IPAFrameContext\n>   *\n> - * \\var IPAFrameContext::sensor\n> + * \\var IPU3FrameContext::sensor\n>   * \\brief Effective sensor values that were applied for the frame\n>   *\n> - * \\var IPAFrameContext::sensor.exposure\n> + * \\var IPU3FrameContext::sensor.exposure\n>   * \\brief Exposure time expressed as a number of lines\n>   *\n> - * \\var IPAFrameContext::sensor.gain\n> + * \\var IPU3FrameContext::sensor.gain\n>   * \\brief Analogue gain multiplier\n>   */\n>\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 193fc44a821a..c4aa4c3f4f6a 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -73,8 +73,7 @@ struct IPAActiveState {\n>  \t} toneMapping;\n>  };\n>\n> -struct IPAFrameContext {\n> -\tuint32_t frame;\n> +struct IPU3FrameContext : public IPAFrameContext {\n>\n>  \tstruct {\n>  \t\tuint32_t exposure;\n> @@ -86,7 +85,7 @@ struct IPAContext {\n>  \tIPASessionConfiguration configuration;\n>  \tIPAActiveState activeState;\n>\n> -\tFCQueue<IPAFrameContext> frameContexts;\n> +\tFCQueue<IPU3FrameContext> frameContexts;\n>  };\n>\n>  } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 55e82cd404f4..489e51bc6cf7 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -570,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>  \tconst ipu3_uapi_stats_3a *stats =\n>  \t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n>\n> -\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\tIPU3FrameContext &frameContext = context_.frameContexts.get(frame);\n>\n>  \tif (frameContext.frame != frame)\n>  \t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> @@ -583,7 +583,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>  \tControlList ctrls(controls::controls);\n>\n>  \tfor (auto const &algo : algorithms_)\n> -\t\talgo->process(context_, &frameContext, stats);\n> +\t\talgo->process(context_, frameContext, stats);\n>\n>  \tsetControls(frame);\n>\n> @@ -619,7 +619,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n>  void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n>  {\n>  \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> -\tIPAFrameContext &frameContext = context_.frameContexts.initialise(frame);\n> +\tIPU3FrameContext &frameContext = context_.frameContexts.initialise(frame);\n>\n>  \t/* \\todo Implement queueRequest to each algorithm. */\n>  \t(void)frameContext;\n> diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h\n> index d94fc4594871..6d0d50f615d8 100644\n> --- a/src/ipa/ipu3/module.h\n> +++ b/src/ipa/ipu3/module.h\n> @@ -19,7 +19,7 @@ namespace libcamera {\n>\n>  namespace ipa::ipu3 {\n>\n> -using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo,\n> +using Module = ipa::Module<IPAContext, IPU3FrameContext, IPAConfigInfo,\n>  \t\t\t   ipu3_uapi_params, ipu3_uapi_stats_3a>;\n>\n>  } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index 2a8871d831a3..adbcf69adb4f 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -41,7 +41,7 @@ public:\n>  \t}\n>\n>  \tvirtual void process([[maybe_unused]] typename Module::Context &context,\n> -\t\t\t     [[maybe_unused]] typename Module::FrameContext *frameContext,\n> +\t\t\t     [[maybe_unused]] typename Module::FrameContext &frameContext,\n>  \t\t\t     [[maybe_unused]] const typename Module::Stats *stats)\n>  \t{\n>  \t}\n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index ecb47f287350..627c2fa3301e 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -55,6 +55,27 @@ namespace ipa {\n>   * the Request when it completes.\n>   */\n>\n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Context for a frame\n> + *\n> + * The frame context stores data specific to a single frame processed by the\n> + * IPA. Each frame processed by the IPA has a context associated with it,\n> + * accessible through the Frame Context Queue.\n> + *\n> + * Fields in the frame context should reflect values and controls associated\n> + * with the specific frame as requested by the application, and as configured by\n> + * the hardware. Fields can be read by algorithms to determine if they should\n> + * update any specific action for this frame, and finally to update the metadata\n> + * control lists when the frame is fully completed.\n\nI think the documentation should be updated to state that this is the\nbase class which contains the minimal interface for the FCQueue to\noperate on it and that it is expected that each IPA extendes this with\nits own implementation.\n\n> + *\n> + * \\var IPAFrameContext::frame\n> + * \\brief The frame number\n> + *\n> + * \\var IPAFrameContext::error\n> + * \\brief The error flags associated with the frame\n> + */\n> +\n>  /**\n>   * \\fn FCQueue::clear()\n>   * \\brief Reinitialise all data on the queue\n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index 82ce36811926..6c7b8bf11ab2 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -13,6 +13,7 @@\n>  #include <libcamera/base/log.h>\n>\n>  #include <libcamera/controls.h>\n> +#include <libcamera/request.h>\n>\n>  namespace libcamera {\n>\n> @@ -27,6 +28,11 @@ namespace ipa {\n>   */\n>  static constexpr uint32_t kMaxFrameContexts = 16;\n>\n> +struct IPAFrameContext {\n> +\tunsigned int frame;\n> +\tRequest::ErrorFlags error;\n> +};\n> +\n>  template<typename FrameContext>\n>  class FCQueue : public std::array<FrameContext, kMaxFrameContexts>\n>  {\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 483e941fe427..40d27963ef68 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -281,7 +281,7 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n>   * new exposure and gain for the scene.\n>   */\n>  void Agc::process(IPAContext &context,\n> -\t\t  [[maybe_unused]] IPAFrameContext *frameContext,\n> +\t\t  [[maybe_unused]] RKISP1FrameContext &frameContext,\n>  \t\t  const rkisp1_stat_buffer *stats)\n>  {\n>  \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 22c02779d311..1a291bdf3636 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -29,7 +29,7 @@ public:\n>\n>  \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n>  \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameContext,\n> +\tvoid process(IPAContext &context, RKISP1FrameContext &frameContext,\n>  \t\t     const rkisp1_stat_buffer *stats) override;\n>\n>  private:\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 427aaeb1e955..3beafb73ca33 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -120,7 +120,7 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n>   * \\copydoc libcamera::ipa::Algorithm::process\n>   */\n>  void Awb::process([[maybe_unused]] IPAContext &context,\n> -\t\t  [[maybe_unused]] IPAFrameContext *frameCtx,\n> +\t\t  [[maybe_unused]] RKISP1FrameContext &frameCtx,\n>  \t\t  const rkisp1_stat_buffer *stats)\n>  {\n>  \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 7647842f6609..259b85eebdb4 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -23,7 +23,7 @@ public:\n>\n>  \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n>  \tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> -\tvoid process(IPAContext &context, IPAFrameContext *frameCtx,\n> +\tvoid process(IPAContext &context, RKISP1FrameContext &frameCtx,\n>  \t\t     const rkisp1_stat_buffer *stats) override;\n>\n>  private:\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 341fef2e68fe..a64dbc75fdd2 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -14,6 +14,8 @@\n>\n>  #include <libcamera/geometry.h>\n>\n> +#include <libipa/fc_queue.h>\n> +\n>  namespace libcamera {\n>\n>  namespace ipa::rkisp1 {\n> @@ -64,7 +66,7 @@ struct IPAActiveState {\n>  \tunsigned int frameCount;\n>  };\n>\n> -struct IPAFrameContext {\n> +struct RKISP1FrameContext : public IPAFrameContext {\n>  };\n>\n>  struct IPAContext {\n> diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h\n> index 89f83208a75c..2568c624df0f 100644\n> --- a/src/ipa/rkisp1/module.h\n> +++ b/src/ipa/rkisp1/module.h\n> @@ -19,7 +19,7 @@ namespace libcamera {\n>\n>  namespace ipa::rkisp1 {\n>\n> -using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,\n> +using Module = ipa::Module<IPAContext, RKISP1FrameContext, IPACameraSensorInfo,\n>  \t\t\t   rkisp1_params_cfg, rkisp1_stat_buffer>;\n>\n>  } /* namespace ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 9629ccbf4247..7481e67e70f6 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -301,8 +301,11 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n>\n>  \tunsigned int aeState = 0;\n>\n> +\t/* \\todo Obtain the frame context to pass to process from the FCQueue */\n> +\tRKISP1FrameContext frameContext;\n> +\n>  \tfor (auto const &algo : algorithms())\n> -\t\talgo->process(context_, nullptr, stats);\n> +\t\talgo->process(context_, frameContext, stats);\n>\n>  \tsetControls(frame);\n>\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 75D2BBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jul 2022 15:30:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFDEC63312;\n\tMon, 25 Jul 2022 17:30:37 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CFD2E6330A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jul 2022 17:30:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 51003240004;\n\tMon, 25 Jul 2022 15:30:36 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658763037;\n\tbh=FQobXmRxlMYe4f6KrWJJPcKZzlmvzPEwuyWJLDasDIU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=k44gy9sfLATIMQ3a14qL6KqrmSrrDY2TE2WEDEaQDS0n3G8+sYWsGhaMCdyhrcf9M\n\tfP68lrb5rUh0lyRj5mySBIgQQPUsisQ0k4m7JyUw+FRZNklARoBwQhFIj8dMCTR/dE\n\tGRuXsSIM9Z0xszCcQu+XOfGvoL3BGvTF4OkAFQTXdQl85vHSq8lqYig1mT3+SK+Sp2\n\t9mUSWrKfgjUM0Aaxc2CyasjqGxTjohYfvxZfuG725XYzY4dFYn2NwGIveixO8N19ko\n\tJyqPbPTwiIsJlty3qEO8r8PDKyXzT5KkBOvbJCNmRPXbkDWqFePalBgCiD1qWPGVYQ\n\tABHvXXgGrGxqg==","Date":"Mon, 25 Jul 2022 17:30:34 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20220725153034.sbvpyav7m4qolfhd@uno.localdomain>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-7-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220721121310.1286862-7-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 06/12] ipa: libipa: Provide a\n\tcommon base for FrameContexts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]