| Message ID | 20220721121310.1286862-7-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, On 7/21/22 17:43, Kieran Bingham via libcamera-devel wrote: > Provide a common IPAFrameContext as a base for IPA modules to inherit > from. > > This will allow having a common set of parameters for every FrameContext. > > We still have to use a templated instance of the FCQueue though to make > sure we allocate the correct types. > > Both the RKISP1 and the IPU3 create IPA specific FrameContext structures > which inherit from the IPAFrameContext. > > While adjusting interface to Algorithm::process() the FrameContext is > converted from a pointer to a reference. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Looks good! Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 3 ++- > src/ipa/ipu3/algorithms/af.h | 2 +- > src/ipa/ipu3/algorithms/agc.cpp | 9 ++++---- > src/ipa/ipu3/algorithms/agc.h | 4 ++-- > src/ipa/ipu3/algorithms/awb.cpp | 3 ++- > src/ipa/ipu3/algorithms/awb.h | 2 +- > src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.h | 2 +- > src/ipa/ipu3/ipa_context.cpp | 26 +++++------------------- > src/ipa/ipu3/ipa_context.h | 5 ++--- > src/ipa/ipu3/ipu3.cpp | 6 +++--- > src/ipa/ipu3/module.h | 2 +- > src/ipa/libipa/algorithm.h | 2 +- > src/ipa/libipa/fc_queue.cpp | 21 +++++++++++++++++++ > src/ipa/libipa/fc_queue.h | 6 ++++++ > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > src/ipa/rkisp1/algorithms/agc.h | 2 +- > src/ipa/rkisp1/algorithms/awb.cpp | 2 +- > src/ipa/rkisp1/algorithms/awb.h | 2 +- > src/ipa/rkisp1/ipa_context.h | 4 +++- > src/ipa/rkisp1/module.h | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 5 ++++- > 22 files changed, 67 insertions(+), 48 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index d07521a090ac..fcbf8e557b10 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -420,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > */ > -void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Af::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > /* Evaluate the AF buffer length */ > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > index ccf015f3f8f5..7a5aeb1b6bf4 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -32,7 +32,7 @@ public: > > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5bc64ae52214..92ea6249798d 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -183,13 +183,13 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > * \param[in] yGain The gain calculated based on the relative luminance target > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > */ > -void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext, > +void Agc::computeExposure(IPAContext &context, IPU3FrameContext &frameContext, > double yGain, double iqMeanGain) > { > const IPASessionConfiguration &configuration = context.configuration; > /* Get the effective exposure and gain applied on the sensor. */ > - uint32_t exposure = frameContext->sensor.exposure; > - double analogueGain = frameContext->sensor.gain; > + uint32_t exposure = frameContext.sensor.exposure; > + double analogueGain = frameContext.sensor.gain; > > /* Use the highest of the two gain estimates. */ > double evGain = std::max(yGain, iqMeanGain); > @@ -323,7 +323,8 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Agc::process(IPAContext &context, > + IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 105ae0f2aac6..96585f48933f 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -28,14 +28,14 @@ public: > ~Agc() = default; > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > double measureBrightness(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid) const; > utils::Duration filterExposure(utils::Duration currentExposure); > - void computeExposure(IPAContext &context, IPAFrameContext *frameContext, > + void computeExposure(IPAContext &context, IPU3FrameContext &frameContext, > double yGain, double iqMeanGain); > double estimateLuminance(IPAActiveState &activeState, > const ipu3_uapi_grid_config &grid, > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index 704267222a31..45f8e5f29119 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -387,7 +387,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > /** > * \copydoc libcamera::ipa::Algorithm::process > */ > -void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Awb::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > calculateWBGains(stats); > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h > index 0acd21480845..28e39620fd59 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -40,7 +40,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp > index f86e79b24a67..977741c5a4d4 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -78,7 +78,8 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > * The tone mapping look up table is generated as an inverse power curve from > * our gamma setting. > */ > -void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void ToneMapping::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > [[maybe_unused]] const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h > index d7d4800628f2..8cce38c742a6 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -20,7 +20,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index dd139cb4c868..536d76ecd63b 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 { > * most recently computed by the IPA algorithms. > */ > > -/** > - * \struct IPAFrameContext > - * \brief Context for a frame > - * > - * The frame context stores data specific to a single frame processed by the > - * IPA. Each frame processed by the IPA has a context associated with it, > - * accessible through the IPAContext structure. > - * > - * Fields in the frame context should reflect values and controls > - * associated with the specific frame as requested by the application, and > - * as configured by the hardware. Fields can be read by algorithms to > - * determine if they should update any specific action for this frame, and > - * finally to update the metadata control lists when the frame is fully > - * completed. > - */ > - > /** > * \struct IPAContext > * \brief Global IPA context data shared between all algorithms > @@ -181,16 +165,16 @@ namespace libcamera::ipa::ipu3 { > */ > > /** > - * \var IPAFrameContext::frame > - * \brief The frame number > + * \struct IPU3FrameContext > + * \copybrief libcamera::ipa::IPAFrameContext > * > - * \var IPAFrameContext::sensor > + * \var IPU3FrameContext::sensor > * \brief Effective sensor values that were applied for the frame > * > - * \var IPAFrameContext::sensor.exposure > + * \var IPU3FrameContext::sensor.exposure > * \brief Exposure time expressed as a number of lines > * > - * \var IPAFrameContext::sensor.gain > + * \var IPU3FrameContext::sensor.gain > * \brief Analogue gain multiplier > */ > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 193fc44a821a..c4aa4c3f4f6a 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -73,8 +73,7 @@ struct IPAActiveState { > } toneMapping; > }; > > -struct IPAFrameContext { > - uint32_t frame; > +struct IPU3FrameContext : public IPAFrameContext { > > struct { > uint32_t exposure; > @@ -86,7 +85,7 @@ struct IPAContext { > IPASessionConfiguration configuration; > IPAActiveState activeState; > > - FCQueue<IPAFrameContext> frameContexts; > + FCQueue<IPU3FrameContext> frameContexts; > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 55e82cd404f4..489e51bc6cf7 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -570,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > const ipu3_uapi_stats_3a *stats = > reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data()); > > - IPAFrameContext &frameContext = context_.frameContexts.get(frame); > + IPU3FrameContext &frameContext = context_.frameContexts.get(frame); > > if (frameContext.frame != frame) > LOG(IPAIPU3, Warning) << "Frame " << frame << " does not match its frame context"; > @@ -583,7 +583,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > ControlList ctrls(controls::controls); > > for (auto const &algo : algorithms_) > - algo->process(context_, &frameContext, stats); > + algo->process(context_, frameContext, stats); > > setControls(frame); > > @@ -619,7 +619,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) > { > /* \todo Start processing for 'frame' based on 'controls'. */ > - IPAFrameContext &frameContext = context_.frameContexts.initialise(frame); > + IPU3FrameContext &frameContext = context_.frameContexts.initialise(frame); > > /* \todo Implement queueRequest to each algorithm. */ > (void)frameContext; > diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h > index d94fc4594871..6d0d50f615d8 100644 > --- a/src/ipa/ipu3/module.h > +++ b/src/ipa/ipu3/module.h > @@ -19,7 +19,7 @@ namespace libcamera { > > namespace ipa::ipu3 { > > -using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo, > +using Module = ipa::Module<IPAContext, IPU3FrameContext, IPAConfigInfo, > ipu3_uapi_params, ipu3_uapi_stats_3a>; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h > index 2a8871d831a3..adbcf69adb4f 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -41,7 +41,7 @@ public: > } > > virtual void process([[maybe_unused]] typename Module::Context &context, > - [[maybe_unused]] typename Module::FrameContext *frameContext, > + [[maybe_unused]] typename Module::FrameContext &frameContext, > [[maybe_unused]] const typename Module::Stats *stats) > { > } > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp > index ecb47f287350..627c2fa3301e 100644 > --- a/src/ipa/libipa/fc_queue.cpp > +++ b/src/ipa/libipa/fc_queue.cpp > @@ -55,6 +55,27 @@ namespace ipa { > * the Request when it completes. > */ > > +/** > + * \struct IPAFrameContext > + * \brief Context for a frame > + * > + * The frame context stores data specific to a single frame processed by the > + * IPA. Each frame processed by the IPA has a context associated with it, > + * accessible through the Frame Context Queue. > + * > + * Fields in the frame context should reflect values and controls associated > + * with the specific frame as requested by the application, and as configured by > + * the hardware. Fields can be read by algorithms to determine if they should > + * update any specific action for this frame, and finally to update the metadata > + * control lists when the frame is fully completed. > + * > + * \var IPAFrameContext::frame > + * \brief The frame number > + * > + * \var IPAFrameContext::error > + * \brief The error flags associated with the frame > + */ > + > /** > * \fn FCQueue::clear() > * \brief Reinitialise all data on the queue > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h > index 82ce36811926..6c7b8bf11ab2 100644 > --- a/src/ipa/libipa/fc_queue.h > +++ b/src/ipa/libipa/fc_queue.h > @@ -13,6 +13,7 @@ > #include <libcamera/base/log.h> > > #include <libcamera/controls.h> > +#include <libcamera/request.h> > > namespace libcamera { > > @@ -27,6 +28,11 @@ namespace ipa { > */ > static constexpr uint32_t kMaxFrameContexts = 16; > > +struct IPAFrameContext { > + unsigned int frame; > + Request::ErrorFlags error; > +}; > + > template<typename FrameContext> > class FCQueue : public std::array<FrameContext, kMaxFrameContexts> > { > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 483e941fe427..40d27963ef68 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -281,7 +281,7 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * new exposure and gain for the scene. > */ > void Agc::process(IPAContext &context, > - [[maybe_unused]] IPAFrameContext *frameContext, > + [[maybe_unused]] RKISP1FrameContext &frameContext, > const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index 22c02779d311..1a291bdf3636 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -29,7 +29,7 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, RKISP1FrameContext &frameContext, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 427aaeb1e955..3beafb73ca33 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -120,7 +120,7 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > * \copydoc libcamera::ipa::Algorithm::process > */ > void Awb::process([[maybe_unused]] IPAContext &context, > - [[maybe_unused]] IPAFrameContext *frameCtx, > + [[maybe_unused]] RKISP1FrameContext &frameCtx, > const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h > index 7647842f6609..259b85eebdb4 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -23,7 +23,7 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, IPAFrameContext *frameCtx, > + void process(IPAContext &context, RKISP1FrameContext &frameCtx, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 341fef2e68fe..a64dbc75fdd2 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -14,6 +14,8 @@ > > #include <libcamera/geometry.h> > > +#include <libipa/fc_queue.h> > + > namespace libcamera { > > namespace ipa::rkisp1 { > @@ -64,7 +66,7 @@ struct IPAActiveState { > unsigned int frameCount; > }; > > -struct IPAFrameContext { > +struct RKISP1FrameContext : public IPAFrameContext { > }; > > struct IPAContext { > diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h > index 89f83208a75c..2568c624df0f 100644 > --- a/src/ipa/rkisp1/module.h > +++ b/src/ipa/rkisp1/module.h > @@ -19,7 +19,7 @@ namespace libcamera { > > namespace ipa::rkisp1 { > > -using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, > +using Module = ipa::Module<IPAContext, RKISP1FrameContext, IPACameraSensorInfo, > rkisp1_params_cfg, rkisp1_stat_buffer>; > > } /* namespace ipa::rkisp1 */ > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 9629ccbf4247..7481e67e70f6 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -301,8 +301,11 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > unsigned int aeState = 0; > > + /* \todo Obtain the frame context to pass to process from the FCQueue */ > + RKISP1FrameContext frameContext; > + > for (auto const &algo : algorithms()) > - algo->process(context_, nullptr, stats); > + algo->process(context_, frameContext, stats); > > setControls(frame); >
Hi Kieran On Thu, Jul 21, 2022 at 01:13:04PM +0100, Kieran Bingham via libcamera-devel wrote: > Provide a common IPAFrameContext as a base for IPA modules to inherit > from. > > This will allow having a common set of parameters for every FrameContext. > > We still have to use a templated instance of the FCQueue though to make > sure we allocate the correct types. > > Both the RKISP1 and the IPU3 create IPA specific FrameContext structures > which inherit from the IPAFrameContext. > > While adjusting interface to Algorithm::process() the FrameContext is > converted from a pointer to a reference. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 3 ++- > src/ipa/ipu3/algorithms/af.h | 2 +- > src/ipa/ipu3/algorithms/agc.cpp | 9 ++++---- > src/ipa/ipu3/algorithms/agc.h | 4 ++-- > src/ipa/ipu3/algorithms/awb.cpp | 3 ++- > src/ipa/ipu3/algorithms/awb.h | 2 +- > src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.h | 2 +- > src/ipa/ipu3/ipa_context.cpp | 26 +++++------------------- > src/ipa/ipu3/ipa_context.h | 5 ++--- > src/ipa/ipu3/ipu3.cpp | 6 +++--- > src/ipa/ipu3/module.h | 2 +- > src/ipa/libipa/algorithm.h | 2 +- > src/ipa/libipa/fc_queue.cpp | 21 +++++++++++++++++++ > src/ipa/libipa/fc_queue.h | 6 ++++++ > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > src/ipa/rkisp1/algorithms/agc.h | 2 +- > src/ipa/rkisp1/algorithms/awb.cpp | 2 +- > src/ipa/rkisp1/algorithms/awb.h | 2 +- > src/ipa/rkisp1/ipa_context.h | 4 +++- > src/ipa/rkisp1/module.h | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 5 ++++- > 22 files changed, 67 insertions(+), 48 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index d07521a090ac..fcbf8e557b10 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -420,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > */ > -void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Af::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > /* Evaluate the AF buffer length */ > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > index ccf015f3f8f5..7a5aeb1b6bf4 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -32,7 +32,7 @@ public: > > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5bc64ae52214..92ea6249798d 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -183,13 +183,13 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > * \param[in] yGain The gain calculated based on the relative luminance target > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > */ > -void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext, > +void Agc::computeExposure(IPAContext &context, IPU3FrameContext &frameContext, > double yGain, double iqMeanGain) > { > const IPASessionConfiguration &configuration = context.configuration; > /* Get the effective exposure and gain applied on the sensor. */ > - uint32_t exposure = frameContext->sensor.exposure; > - double analogueGain = frameContext->sensor.gain; > + uint32_t exposure = frameContext.sensor.exposure; > + double analogueGain = frameContext.sensor.gain; > > /* Use the highest of the two gain estimates. */ > double evGain = std::max(yGain, iqMeanGain); > @@ -323,7 +323,8 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Agc::process(IPAContext &context, > + IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 105ae0f2aac6..96585f48933f 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -28,14 +28,14 @@ public: > ~Agc() = default; > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > double measureBrightness(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid) const; > utils::Duration filterExposure(utils::Duration currentExposure); > - void computeExposure(IPAContext &context, IPAFrameContext *frameContext, > + void computeExposure(IPAContext &context, IPU3FrameContext &frameContext, > double yGain, double iqMeanGain); > double estimateLuminance(IPAActiveState &activeState, > const ipu3_uapi_grid_config &grid, > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index 704267222a31..45f8e5f29119 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -387,7 +387,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > /** > * \copydoc libcamera::ipa::Algorithm::process > */ > -void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void Awb::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) > { > calculateWBGains(stats); > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h > index 0acd21480845..28e39620fd59 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -40,7 +40,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp > index f86e79b24a67..977741c5a4d4 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -78,7 +78,8 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > * The tone mapping look up table is generated as an inverse power curve from > * our gamma setting. > */ > -void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > +void ToneMapping::process(IPAContext &context, > + [[maybe_unused]] IPU3FrameContext &frameContext, > [[maybe_unused]] const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h > index d7d4800628f2..8cce38c742a6 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -20,7 +20,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, IPU3FrameContext &frameContext, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index dd139cb4c868..536d76ecd63b 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 { > * most recently computed by the IPA algorithms. > */ > > -/** > - * \struct IPAFrameContext > - * \brief Context for a frame > - * > - * The frame context stores data specific to a single frame processed by the > - * IPA. Each frame processed by the IPA has a context associated with it, > - * accessible through the IPAContext structure. > - * > - * Fields in the frame context should reflect values and controls > - * associated with the specific frame as requested by the application, and > - * as configured by the hardware. Fields can be read by algorithms to > - * determine if they should update any specific action for this frame, and > - * finally to update the metadata control lists when the frame is fully > - * completed. > - */ > - > /** > * \struct IPAContext > * \brief Global IPA context data shared between all algorithms > @@ -181,16 +165,16 @@ namespace libcamera::ipa::ipu3 { > */ > > /** > - * \var IPAFrameContext::frame > - * \brief The frame number > + * \struct IPU3FrameContext > + * \copybrief libcamera::ipa::IPAFrameContext > * > - * \var IPAFrameContext::sensor > + * \var IPU3FrameContext::sensor > * \brief Effective sensor values that were applied for the frame > * > - * \var IPAFrameContext::sensor.exposure > + * \var IPU3FrameContext::sensor.exposure > * \brief Exposure time expressed as a number of lines > * > - * \var IPAFrameContext::sensor.gain > + * \var IPU3FrameContext::sensor.gain > * \brief Analogue gain multiplier > */ > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 193fc44a821a..c4aa4c3f4f6a 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -73,8 +73,7 @@ struct IPAActiveState { > } toneMapping; > }; > > -struct IPAFrameContext { > - uint32_t frame; > +struct IPU3FrameContext : public IPAFrameContext { > > struct { > uint32_t exposure; > @@ -86,7 +85,7 @@ struct IPAContext { > IPASessionConfiguration configuration; > IPAActiveState activeState; > > - FCQueue<IPAFrameContext> frameContexts; > + FCQueue<IPU3FrameContext> frameContexts; > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 55e82cd404f4..489e51bc6cf7 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -570,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > const ipu3_uapi_stats_3a *stats = > reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data()); > > - IPAFrameContext &frameContext = context_.frameContexts.get(frame); > + IPU3FrameContext &frameContext = context_.frameContexts.get(frame); > > if (frameContext.frame != frame) > LOG(IPAIPU3, Warning) << "Frame " << frame << " does not match its frame context"; > @@ -583,7 +583,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > ControlList ctrls(controls::controls); > > for (auto const &algo : algorithms_) > - algo->process(context_, &frameContext, stats); > + algo->process(context_, frameContext, stats); > > setControls(frame); > > @@ -619,7 +619,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) > { > /* \todo Start processing for 'frame' based on 'controls'. */ > - IPAFrameContext &frameContext = context_.frameContexts.initialise(frame); > + IPU3FrameContext &frameContext = context_.frameContexts.initialise(frame); > > /* \todo Implement queueRequest to each algorithm. */ > (void)frameContext; > diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h > index d94fc4594871..6d0d50f615d8 100644 > --- a/src/ipa/ipu3/module.h > +++ b/src/ipa/ipu3/module.h > @@ -19,7 +19,7 @@ namespace libcamera { > > namespace ipa::ipu3 { > > -using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo, > +using Module = ipa::Module<IPAContext, IPU3FrameContext, IPAConfigInfo, > ipu3_uapi_params, ipu3_uapi_stats_3a>; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h > index 2a8871d831a3..adbcf69adb4f 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -41,7 +41,7 @@ public: > } > > virtual void process([[maybe_unused]] typename Module::Context &context, > - [[maybe_unused]] typename Module::FrameContext *frameContext, > + [[maybe_unused]] typename Module::FrameContext &frameContext, > [[maybe_unused]] const typename Module::Stats *stats) > { > } > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp > index ecb47f287350..627c2fa3301e 100644 > --- a/src/ipa/libipa/fc_queue.cpp > +++ b/src/ipa/libipa/fc_queue.cpp > @@ -55,6 +55,27 @@ namespace ipa { > * the Request when it completes. > */ > > +/** > + * \struct IPAFrameContext > + * \brief Context for a frame > + * > + * The frame context stores data specific to a single frame processed by the > + * IPA. Each frame processed by the IPA has a context associated with it, > + * accessible through the Frame Context Queue. > + * > + * Fields in the frame context should reflect values and controls associated > + * with the specific frame as requested by the application, and as configured by > + * the hardware. Fields can be read by algorithms to determine if they should > + * update any specific action for this frame, and finally to update the metadata > + * control lists when the frame is fully completed. I think the documentation should be updated to state that this is the base class which contains the minimal interface for the FCQueue to operate on it and that it is expected that each IPA extendes this with its own implementation. > + * > + * \var IPAFrameContext::frame > + * \brief The frame number > + * > + * \var IPAFrameContext::error > + * \brief The error flags associated with the frame > + */ > + > /** > * \fn FCQueue::clear() > * \brief Reinitialise all data on the queue > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h > index 82ce36811926..6c7b8bf11ab2 100644 > --- a/src/ipa/libipa/fc_queue.h > +++ b/src/ipa/libipa/fc_queue.h > @@ -13,6 +13,7 @@ > #include <libcamera/base/log.h> > > #include <libcamera/controls.h> > +#include <libcamera/request.h> > > namespace libcamera { > > @@ -27,6 +28,11 @@ namespace ipa { > */ > static constexpr uint32_t kMaxFrameContexts = 16; > > +struct IPAFrameContext { > + unsigned int frame; > + Request::ErrorFlags error; > +}; > + > template<typename FrameContext> > class FCQueue : public std::array<FrameContext, kMaxFrameContexts> > { > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 483e941fe427..40d27963ef68 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -281,7 +281,7 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * new exposure and gain for the scene. > */ > void Agc::process(IPAContext &context, > - [[maybe_unused]] IPAFrameContext *frameContext, > + [[maybe_unused]] RKISP1FrameContext &frameContext, > const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index 22c02779d311..1a291bdf3636 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -29,7 +29,7 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, IPAFrameContext *frameContext, > + void process(IPAContext &context, RKISP1FrameContext &frameContext, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 427aaeb1e955..3beafb73ca33 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -120,7 +120,7 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > * \copydoc libcamera::ipa::Algorithm::process > */ > void Awb::process([[maybe_unused]] IPAContext &context, > - [[maybe_unused]] IPAFrameContext *frameCtx, > + [[maybe_unused]] RKISP1FrameContext &frameCtx, > const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h > index 7647842f6609..259b85eebdb4 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -23,7 +23,7 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, IPAFrameContext *frameCtx, > + void process(IPAContext &context, RKISP1FrameContext &frameCtx, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 341fef2e68fe..a64dbc75fdd2 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -14,6 +14,8 @@ > > #include <libcamera/geometry.h> > > +#include <libipa/fc_queue.h> > + > namespace libcamera { > > namespace ipa::rkisp1 { > @@ -64,7 +66,7 @@ struct IPAActiveState { > unsigned int frameCount; > }; > > -struct IPAFrameContext { > +struct RKISP1FrameContext : public IPAFrameContext { > }; > > struct IPAContext { > diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h > index 89f83208a75c..2568c624df0f 100644 > --- a/src/ipa/rkisp1/module.h > +++ b/src/ipa/rkisp1/module.h > @@ -19,7 +19,7 @@ namespace libcamera { > > namespace ipa::rkisp1 { > > -using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, > +using Module = ipa::Module<IPAContext, RKISP1FrameContext, IPACameraSensorInfo, > rkisp1_params_cfg, rkisp1_stat_buffer>; > > } /* namespace ipa::rkisp1 */ > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 9629ccbf4247..7481e67e70f6 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -301,8 +301,11 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > unsigned int aeState = 0; > > + /* \todo Obtain the frame context to pass to process from the FCQueue */ > + RKISP1FrameContext frameContext; > + > for (auto const &algo : algorithms()) > - algo->process(context_, nullptr, stats); > + algo->process(context_, frameContext, stats); > > setControls(frame); > > -- > 2.34.1 >
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index d07521a090ac..fcbf8e557b10 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -420,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) * * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing */ -void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, +void Af::process(IPAContext &context, + [[maybe_unused]] IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) { /* Evaluate the AF buffer length */ diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h index ccf015f3f8f5..7a5aeb1b6bf4 100644 --- a/src/ipa/ipu3/algorithms/af.h +++ b/src/ipa/ipu3/algorithms/af.h @@ -32,7 +32,7 @@ public: void prepare(IPAContext &context, ipu3_uapi_params *params) override; int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; - void process(IPAContext &context, IPAFrameContext *frameContext, + void process(IPAContext &context, IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) override; private: diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 5bc64ae52214..92ea6249798d 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -183,13 +183,13 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) * \param[in] yGain The gain calculated based on the relative luminance target * \param[in] iqMeanGain The gain calculated based on the relative luminance target */ -void Agc::computeExposure(IPAContext &context, IPAFrameContext *frameContext, +void Agc::computeExposure(IPAContext &context, IPU3FrameContext &frameContext, double yGain, double iqMeanGain) { const IPASessionConfiguration &configuration = context.configuration; /* Get the effective exposure and gain applied on the sensor. */ - uint32_t exposure = frameContext->sensor.exposure; - double analogueGain = frameContext->sensor.gain; + uint32_t exposure = frameContext.sensor.exposure; + double analogueGain = frameContext.sensor.gain; /* Use the highest of the two gain estimates. */ double evGain = std::max(yGain, iqMeanGain); @@ -323,7 +323,8 @@ double Agc::estimateLuminance(IPAActiveState &activeState, * Identify the current image brightness, and use that to estimate the optimal * new exposure and gain for the scene. */ -void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, +void Agc::process(IPAContext &context, + IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) { /* diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 105ae0f2aac6..96585f48933f 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -28,14 +28,14 @@ public: ~Agc() = default; int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; - void process(IPAContext &context, IPAFrameContext *frameContext, + void process(IPAContext &context, IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) override; private: double measureBrightness(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) const; utils::Duration filterExposure(utils::Duration currentExposure); - void computeExposure(IPAContext &context, IPAFrameContext *frameContext, + void computeExposure(IPAContext &context, IPU3FrameContext &frameContext, double yGain, double iqMeanGain); double estimateLuminance(IPAActiveState &activeState, const ipu3_uapi_grid_config &grid, diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index 704267222a31..45f8e5f29119 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -387,7 +387,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) /** * \copydoc libcamera::ipa::Algorithm::process */ -void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, +void Awb::process(IPAContext &context, + [[maybe_unused]] IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) { calculateWBGains(stats); diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index 0acd21480845..28e39620fd59 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -40,7 +40,7 @@ public: int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void prepare(IPAContext &context, ipu3_uapi_params *params) override; - void process(IPAContext &context, IPAFrameContext *frameContext, + void process(IPAContext &context, IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) override; private: diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp index f86e79b24a67..977741c5a4d4 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp @@ -78,7 +78,8 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, * The tone mapping look up table is generated as an inverse power curve from * our gamma setting. */ -void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, +void ToneMapping::process(IPAContext &context, + [[maybe_unused]] IPU3FrameContext &frameContext, [[maybe_unused]] const ipu3_uapi_stats_3a *stats) { /* diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h index d7d4800628f2..8cce38c742a6 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.h +++ b/src/ipa/ipu3/algorithms/tone_mapping.h @@ -20,7 +20,7 @@ public: int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void prepare(IPAContext &context, ipu3_uapi_params *params) override; - void process(IPAContext &context, IPAFrameContext *frameContext, + void process(IPAContext &context, IPU3FrameContext &frameContext, const ipu3_uapi_stats_3a *stats) override; private: diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index dd139cb4c868..536d76ecd63b 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -35,22 +35,6 @@ namespace libcamera::ipa::ipu3 { * most recently computed by the IPA algorithms. */ -/** - * \struct IPAFrameContext - * \brief Context for a frame - * - * The frame context stores data specific to a single frame processed by the - * IPA. Each frame processed by the IPA has a context associated with it, - * accessible through the IPAContext structure. - * - * Fields in the frame context should reflect values and controls - * associated with the specific frame as requested by the application, and - * as configured by the hardware. Fields can be read by algorithms to - * determine if they should update any specific action for this frame, and - * finally to update the metadata control lists when the frame is fully - * completed. - */ - /** * \struct IPAContext * \brief Global IPA context data shared between all algorithms @@ -181,16 +165,16 @@ namespace libcamera::ipa::ipu3 { */ /** - * \var IPAFrameContext::frame - * \brief The frame number + * \struct IPU3FrameContext + * \copybrief libcamera::ipa::IPAFrameContext * - * \var IPAFrameContext::sensor + * \var IPU3FrameContext::sensor * \brief Effective sensor values that were applied for the frame * - * \var IPAFrameContext::sensor.exposure + * \var IPU3FrameContext::sensor.exposure * \brief Exposure time expressed as a number of lines * - * \var IPAFrameContext::sensor.gain + * \var IPU3FrameContext::sensor.gain * \brief Analogue gain multiplier */ diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 193fc44a821a..c4aa4c3f4f6a 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -73,8 +73,7 @@ struct IPAActiveState { } toneMapping; }; -struct IPAFrameContext { - uint32_t frame; +struct IPU3FrameContext : public IPAFrameContext { struct { uint32_t exposure; @@ -86,7 +85,7 @@ struct IPAContext { IPASessionConfiguration configuration; IPAActiveState activeState; - FCQueue<IPAFrameContext> frameContexts; + FCQueue<IPU3FrameContext> frameContexts; }; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 55e82cd404f4..489e51bc6cf7 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -570,7 +570,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, const ipu3_uapi_stats_3a *stats = reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data()); - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPU3FrameContext &frameContext = context_.frameContexts.get(frame); if (frameContext.frame != frame) LOG(IPAIPU3, Warning) << "Frame " << frame << " does not match its frame context"; @@ -583,7 +583,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, ControlList ctrls(controls::controls); for (auto const &algo : algorithms_) - algo->process(context_, &frameContext, stats); + algo->process(context_, frameContext, stats); setControls(frame); @@ -619,7 +619,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) { /* \todo Start processing for 'frame' based on 'controls'. */ - IPAFrameContext &frameContext = context_.frameContexts.initialise(frame); + IPU3FrameContext &frameContext = context_.frameContexts.initialise(frame); /* \todo Implement queueRequest to each algorithm. */ (void)frameContext; diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h index d94fc4594871..6d0d50f615d8 100644 --- a/src/ipa/ipu3/module.h +++ b/src/ipa/ipu3/module.h @@ -19,7 +19,7 @@ namespace libcamera { namespace ipa::ipu3 { -using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo, +using Module = ipa::Module<IPAContext, IPU3FrameContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h index 2a8871d831a3..adbcf69adb4f 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -41,7 +41,7 @@ public: } virtual void process([[maybe_unused]] typename Module::Context &context, - [[maybe_unused]] typename Module::FrameContext *frameContext, + [[maybe_unused]] typename Module::FrameContext &frameContext, [[maybe_unused]] const typename Module::Stats *stats) { } diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp index ecb47f287350..627c2fa3301e 100644 --- a/src/ipa/libipa/fc_queue.cpp +++ b/src/ipa/libipa/fc_queue.cpp @@ -55,6 +55,27 @@ namespace ipa { * the Request when it completes. */ +/** + * \struct IPAFrameContext + * \brief Context for a frame + * + * The frame context stores data specific to a single frame processed by the + * IPA. Each frame processed by the IPA has a context associated with it, + * accessible through the Frame Context Queue. + * + * Fields in the frame context should reflect values and controls associated + * with the specific frame as requested by the application, and as configured by + * the hardware. Fields can be read by algorithms to determine if they should + * update any specific action for this frame, and finally to update the metadata + * control lists when the frame is fully completed. + * + * \var IPAFrameContext::frame + * \brief The frame number + * + * \var IPAFrameContext::error + * \brief The error flags associated with the frame + */ + /** * \fn FCQueue::clear() * \brief Reinitialise all data on the queue diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 82ce36811926..6c7b8bf11ab2 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -13,6 +13,7 @@ #include <libcamera/base/log.h> #include <libcamera/controls.h> +#include <libcamera/request.h> namespace libcamera { @@ -27,6 +28,11 @@ namespace ipa { */ static constexpr uint32_t kMaxFrameContexts = 16; +struct IPAFrameContext { + unsigned int frame; + Request::ErrorFlags error; +}; + template<typename FrameContext> class FCQueue : public std::array<FrameContext, kMaxFrameContexts> { diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 483e941fe427..40d27963ef68 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -281,7 +281,7 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const * new exposure and gain for the scene. */ void Agc::process(IPAContext &context, - [[maybe_unused]] IPAFrameContext *frameContext, + [[maybe_unused]] RKISP1FrameContext &frameContext, const rkisp1_stat_buffer *stats) { const rkisp1_cif_isp_stat *params = &stats->params; diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index 22c02779d311..1a291bdf3636 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -29,7 +29,7 @@ public: int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; - void process(IPAContext &context, IPAFrameContext *frameContext, + void process(IPAContext &context, RKISP1FrameContext &frameContext, const rkisp1_stat_buffer *stats) override; private: diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 427aaeb1e955..3beafb73ca33 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -120,7 +120,7 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) * \copydoc libcamera::ipa::Algorithm::process */ void Awb::process([[maybe_unused]] IPAContext &context, - [[maybe_unused]] IPAFrameContext *frameCtx, + [[maybe_unused]] RKISP1FrameContext &frameCtx, const rkisp1_stat_buffer *stats) { const rkisp1_cif_isp_stat *params = &stats->params; diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h index 7647842f6609..259b85eebdb4 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -23,7 +23,7 @@ public: int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; - void process(IPAContext &context, IPAFrameContext *frameCtx, + void process(IPAContext &context, RKISP1FrameContext &frameCtx, const rkisp1_stat_buffer *stats) override; private: diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 341fef2e68fe..a64dbc75fdd2 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -14,6 +14,8 @@ #include <libcamera/geometry.h> +#include <libipa/fc_queue.h> + namespace libcamera { namespace ipa::rkisp1 { @@ -64,7 +66,7 @@ struct IPAActiveState { unsigned int frameCount; }; -struct IPAFrameContext { +struct RKISP1FrameContext : public IPAFrameContext { }; struct IPAContext { diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h index 89f83208a75c..2568c624df0f 100644 --- a/src/ipa/rkisp1/module.h +++ b/src/ipa/rkisp1/module.h @@ -19,7 +19,7 @@ namespace libcamera { namespace ipa::rkisp1 { -using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, +using Module = ipa::Module<IPAContext, RKISP1FrameContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9629ccbf4247..7481e67e70f6 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -301,8 +301,11 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId unsigned int aeState = 0; + /* \todo Obtain the frame context to pass to process from the FCQueue */ + RKISP1FrameContext frameContext; + for (auto const &algo : algorithms()) - algo->process(context_, nullptr, stats); + algo->process(context_, frameContext, stats); setControls(frame);
Provide a common IPAFrameContext as a base for IPA modules to inherit from. This will allow having a common set of parameters for every FrameContext. We still have to use a templated instance of the FCQueue though to make sure we allocate the correct types. Both the RKISP1 and the IPU3 create IPA specific FrameContext structures which inherit from the IPAFrameContext. While adjusting interface to Algorithm::process() the FrameContext is converted from a pointer to a reference. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/ipa/ipu3/algorithms/af.cpp | 3 ++- src/ipa/ipu3/algorithms/af.h | 2 +- src/ipa/ipu3/algorithms/agc.cpp | 9 ++++---- src/ipa/ipu3/algorithms/agc.h | 4 ++-- src/ipa/ipu3/algorithms/awb.cpp | 3 ++- src/ipa/ipu3/algorithms/awb.h | 2 +- src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- src/ipa/ipu3/algorithms/tone_mapping.h | 2 +- src/ipa/ipu3/ipa_context.cpp | 26 +++++------------------- src/ipa/ipu3/ipa_context.h | 5 ++--- src/ipa/ipu3/ipu3.cpp | 6 +++--- src/ipa/ipu3/module.h | 2 +- src/ipa/libipa/algorithm.h | 2 +- src/ipa/libipa/fc_queue.cpp | 21 +++++++++++++++++++ src/ipa/libipa/fc_queue.h | 6 ++++++ src/ipa/rkisp1/algorithms/agc.cpp | 2 +- src/ipa/rkisp1/algorithms/agc.h | 2 +- src/ipa/rkisp1/algorithms/awb.cpp | 2 +- src/ipa/rkisp1/algorithms/awb.h | 2 +- src/ipa/rkisp1/ipa_context.h | 4 +++- src/ipa/rkisp1/module.h | 2 +- src/ipa/rkisp1/rkisp1.cpp | 5 ++++- 22 files changed, 67 insertions(+), 48 deletions(-)