| Message ID | 20220805135312.47497-7-jacopo@jmondi.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch On 8/5/22 19:23, Jacopo Mondi via libcamera-devel wrote: > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> > > 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 > required by the FCQueue implementation. > > Make both the RKISP1 and the IPU3 define 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> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> 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 | 29 ++++-------------------- > src/ipa/ipu3/ipa_context.h | 8 ++----- > 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 | 5 ++++ > 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, 66 insertions(+), 54 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 9605c8a1106d..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,20 +165,17 @@ 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 > - * > - * \var IPAFrameContext::error > - * \brief The error flags for this frame context > */ > > } /* namespace libcamera::ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index dc5b7c30aaf9..c4aa4c3f4f6a 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -14,7 +14,6 @@ > > #include <libcamera/controls.h> > #include <libcamera/geometry.h> > -#include <libcamera/request.h> > > #include <libipa/fc_queue.h> > > @@ -74,22 +73,19 @@ struct IPAActiveState { > } toneMapping; > }; > > -struct IPAFrameContext { > - uint32_t frame; > +struct IPU3FrameContext : public IPAFrameContext { > > struct { > uint32_t exposure; > double gain; > } sensor; > - > - Request::Errors error; > }; > > 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 209a6b040f8f..24839fd1edee 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -569,7 +569,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"; > @@ -582,7 +582,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); > > @@ -618,7 +618,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 ccc659a63e3b..0fe3d772963a 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -49,7 +49,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 37ffca60b992..f0d0b3c7168d 100644 > --- a/src/ipa/libipa/fc_queue.cpp > +++ b/src/ipa/libipa/fc_queue.cpp > @@ -66,6 +66,27 @@ namespace ipa { > * the PFCError flag is automatically raised on the FrameContext. > */ > > +/** > + * \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 Clear the context queue > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h > index 023b52420fe7..73b7f25f8c20 100644 > --- a/src/ipa/libipa/fc_queue.h > +++ b/src/ipa/libipa/fc_queue.h > @@ -26,6 +26,11 @@ namespace ipa { > */ > static constexpr uint32_t kMaxFrameContexts = 16; > > +struct IPAFrameContext { > + unsigned int frame; > + Request::Errors error; > +}; > + > template<typename FrameContext> > class FCQueue : private 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 1c9818b7db2a..af7fe2740908 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -27,7 +27,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 667a8beb7689..5954034b8142 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -21,7 +21,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 a8daeca487ae..c42bcd73b314 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 { > @@ -78,7 +80,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 a64716f588a8..a2483f27cf52 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -329,8 +329,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); >
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 9605c8a1106d..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,20 +165,17 @@ 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 - * - * \var IPAFrameContext::error - * \brief The error flags for this frame context */ } /* namespace libcamera::ipa::ipu3 */ diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index dc5b7c30aaf9..c4aa4c3f4f6a 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -14,7 +14,6 @@ #include <libcamera/controls.h> #include <libcamera/geometry.h> -#include <libcamera/request.h> #include <libipa/fc_queue.h> @@ -74,22 +73,19 @@ struct IPAActiveState { } toneMapping; }; -struct IPAFrameContext { - uint32_t frame; +struct IPU3FrameContext : public IPAFrameContext { struct { uint32_t exposure; double gain; } sensor; - - Request::Errors error; }; 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 209a6b040f8f..24839fd1edee 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -569,7 +569,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"; @@ -582,7 +582,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); @@ -618,7 +618,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 ccc659a63e3b..0fe3d772963a 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -49,7 +49,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 37ffca60b992..f0d0b3c7168d 100644 --- a/src/ipa/libipa/fc_queue.cpp +++ b/src/ipa/libipa/fc_queue.cpp @@ -66,6 +66,27 @@ namespace ipa { * the PFCError flag is automatically raised on the FrameContext. */ +/** + * \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 Clear the context queue diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 023b52420fe7..73b7f25f8c20 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -26,6 +26,11 @@ namespace ipa { */ static constexpr uint32_t kMaxFrameContexts = 16; +struct IPAFrameContext { + unsigned int frame; + Request::Errors error; +}; + template<typename FrameContext> class FCQueue : private 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 1c9818b7db2a..af7fe2740908 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -27,7 +27,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 667a8beb7689..5954034b8142 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -21,7 +21,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 a8daeca487ae..c42bcd73b314 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 { @@ -78,7 +80,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 a64716f588a8..a2483f27cf52 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -329,8 +329,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);