| Message ID | 20220517191833.333122-3-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang On Tue, May 17, 2022 at 09:18:32PM +0200, Umang Jain via libcamera-devel wrote: > Currently we have a single structure of IPAFrameContext but > subsequently, we shall have a ring buffer (or similar) container > to keep IPAFrameContext structures for each frame. > > It would be a hassle to query out the frame context required for > process() (since they will reside in a ring buffer) by the IPA > for each process. Hence, prepare the process() libipa template to > accept a particular IPAFrameContext early on. > > As for this patch, we shall pass in the pointer as nullptr, so > that the changes compile and keep working as-is. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/ipa/ipu3/algorithms/af.cpp | 4 +++- > src/ipa/ipu3/algorithms/af.h | 3 ++- > src/ipa/ipu3/algorithms/agc.cpp | 4 +++- > src/ipa/ipu3/algorithms/agc.h | 3 ++- > src/ipa/ipu3/algorithms/algorithm.h | 4 +++- > src/ipa/ipu3/algorithms/awb.cpp | 3 ++- > src/ipa/ipu3/algorithms/awb.h | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++- > src/ipa/ipu3/ipu3.cpp | 2 +- > src/ipa/libipa/algorithm.cpp | 1 + > src/ipa/libipa/algorithm.h | 4 +++- > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > src/ipa/rkisp1/algorithms/agc.h | 3 ++- > src/ipa/rkisp1/algorithms/algorithm.h | 4 +++- > src/ipa/rkisp1/algorithms/awb.cpp | 4 +++- > src/ipa/rkisp1/algorithms/awb.h | 3 ++- > src/ipa/rkisp1/rkisp1.cpp | 2 +- > 18 files changed, 40 insertions(+), 17 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 8a5a6b1a..d07521a0 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -406,6 +406,7 @@ bool Af::afIsOutOfFocus(IPAContext context) > /** > * \brief Determine the max contrast image and lens position. > * \param[in] context The IPA context. > + * \param[in] frameContext The current frame context > * \param[in] stats The statistics buffer of IPU3. > * > * Ideally, a clear image also has a relatively higher contrast. So, every > @@ -419,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > */ > -void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > +void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) > { > /* Evaluate the AF buffer length */ > uint32_t afRawBufferLen = context.configuration.af.afGrid.width * > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > index b85cf941..ccf015f3 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -32,7 +32,8 @@ public: > > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > void afCoarseScan(IPAContext &context); > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index fdeec09d..383a8deb 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -318,12 +318,14 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > /** > * \brief Process IPU3 statistics, and run AGC operations > * \param[in] context The shared IPA context > + * \param[in] frameContext The current frame context > * \param[in] stats The IPU3 statistics and ISP results > * > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > +void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) > { > /* > * Estimate the gain needed to have the proportion of pixels in a given > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 31420841..219a1a96 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -28,7 +28,8 @@ public: > ~Agc() = default; > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > double measureBrightness(const ipu3_uapi_stats_3a *stats, > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h > index d2eecc78..234b2bd7 100644 > --- a/src/ipa/ipu3/algorithms/algorithm.h > +++ b/src/ipa/ipu3/algorithms/algorithm.h > @@ -17,7 +17,9 @@ namespace libcamera { > > namespace ipa::ipu3 { > > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, > + IPAConfigInfo, ipu3_uapi_params, > + ipu3_uapi_stats_3a>; > > } /* namespace ipa::ipu3 */ > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index ab6924eb..5c232d92 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, const ipu3_uapi_stats_3a *stats) > +void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 ab4b0a33..9a50a985 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -40,7 +40,8 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > /* \todo Make these structs available to all the ISPs ? */ > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp > index 7c78d0d9..f86e79b2 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -72,12 +72,13 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > /** > * \brief Calculate the tone mapping look up table > * \param context The shared IPA context > + * \param frameContext The current frame context > * \param stats The IPU3 statistics and ISP results > * > * The tone mapping look up table is generated as an inverse power curve from > * our gamma setting. > */ > -void ToneMapping::process(IPAContext &context, > +void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 b727ab1e..d7d48006 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -20,7 +20,8 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > double gamma_; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 3b4fc911..16e5028f 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -576,7 +576,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > ControlList ctrls(controls::controls); > > for (auto const &algo : algorithms_) > - algo->process(context_, stats); > + algo->process(context_, nullptr, stats); > > setControls(frame); > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > index 398d5372..cce2ed62 100644 > --- a/src/ipa/libipa/algorithm.cpp > +++ b/src/ipa/libipa/algorithm.cpp > @@ -64,6 +64,7 @@ namespace ipa { > * \fn Algorithm::process() > * \brief Process ISP statistics, and run algorithm operations > * \param[in] context The shared IPA context > + * \param[in] frameContext The current frame's context > * \param[in] stats The IPA statistics and ISP results > * > * This function is called while camera is running for every frame processed by > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h > index 766aee5d..032a05b5 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -10,7 +10,8 @@ namespace libcamera { > > namespace ipa { > > -template<typename Context, typename Config, typename Params, typename Stats> > +template<typename Context, typename FrameContext, typename Config, > + typename Params, typename Stats> > class Algorithm > { > public: > @@ -28,6 +29,7 @@ public: > } > > virtual void process([[maybe_unused]] Context &context, > + [[maybe_unused]] FrameContext *frameContext, > [[maybe_unused]] const Stats *stats) > { > } > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 5f4c3f93..b5a184d9 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -280,7 +280,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) > +void Agc::process(IPAContext &context, > + [[maybe_unused]] IPAFrameContext *frameContext, > + const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP); > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index ce1adf27..22c02779 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -29,7 +29,8 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const rkisp1_stat_buffer *stats) override; > > private: > void computeExposure(IPAContext &Context, double yGain, double iqMeanGain); > diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h > index d46c3188..68e3a44e 100644 > --- a/src/ipa/rkisp1/algorithms/algorithm.h > +++ b/src/ipa/rkisp1/algorithms/algorithm.h > @@ -19,7 +19,9 @@ namespace libcamera { > > namespace ipa::rkisp1 { > > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, > + IPACameraSensorInfo, rkisp1_params_cfg, > + rkisp1_stat_buffer>; > > } /* namespace ipa::rkisp1 */ > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index be4585c6..88441382 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -119,7 +119,9 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > /** > * \copydoc libcamera::ipa::Algorithm::process > */ > -void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats) > +void Awb::process([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] IPAFrameContext *frameCtx, > + const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h > index 11946643..7647842f 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -23,7 +23,8 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameCtx, > + const rkisp1_stat_buffer *stats) override; > > private: > uint32_t estimateCCT(double red, double green, double blue); > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index ef1f0d56..c818a6d7 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -272,7 +272,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > unsigned int aeState = 0; > > for (auto const &algo : algorithms_) > - algo->process(context_, stats); > + algo->process(context_, nullptr, stats); > > setControls(frame); > > -- > 2.31.0 >
Hi Umang, On 18/05/2022 09:47, Jacopo Mondi via libcamera-devel wrote: > Hi Umang > > On Tue, May 17, 2022 at 09:18:32PM +0200, Umang Jain via libcamera-devel wrote: >> Currently we have a single structure of IPAFrameContext but >> subsequently, we shall have a ring buffer (or similar) container >> to keep IPAFrameContext structures for each frame. >> >> It would be a hassle to query out the frame context required for >> process() (since they will reside in a ring buffer) by the IPA >> for each process. Hence, prepare the process() libipa template to >> accept a particular IPAFrameContext early on. >> >> As for this patch, we shall pass in the pointer as nullptr, so >> that the changes compile and keep working as-is. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > >> --- >> src/ipa/ipu3/algorithms/af.cpp | 4 +++- >> src/ipa/ipu3/algorithms/af.h | 3 ++- >> src/ipa/ipu3/algorithms/agc.cpp | 4 +++- >> src/ipa/ipu3/algorithms/agc.h | 3 ++- >> src/ipa/ipu3/algorithms/algorithm.h | 4 +++- >> src/ipa/ipu3/algorithms/awb.cpp | 3 ++- >> src/ipa/ipu3/algorithms/awb.h | 3 ++- >> src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- >> src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++- >> src/ipa/ipu3/ipu3.cpp | 2 +- >> src/ipa/libipa/algorithm.cpp | 1 + >> src/ipa/libipa/algorithm.h | 4 +++- >> src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- >> src/ipa/rkisp1/algorithms/agc.h | 3 ++- >> src/ipa/rkisp1/algorithms/algorithm.h | 4 +++- >> src/ipa/rkisp1/algorithms/awb.cpp | 4 +++- >> src/ipa/rkisp1/algorithms/awb.h | 3 ++- >> src/ipa/rkisp1/rkisp1.cpp | 2 +- >> 18 files changed, 40 insertions(+), 17 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp >> index 8a5a6b1a..d07521a0 100644 >> --- a/src/ipa/ipu3/algorithms/af.cpp >> +++ b/src/ipa/ipu3/algorithms/af.cpp >> @@ -406,6 +406,7 @@ bool Af::afIsOutOfFocus(IPAContext context) >> /** >> * \brief Determine the max contrast image and lens position. >> * \param[in] context The IPA context. >> + * \param[in] frameContext The current frame context >> * \param[in] stats The statistics buffer of IPU3. >> * >> * Ideally, a clear image also has a relatively higher contrast. So, every >> @@ -419,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) >> * >> * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing >> */ >> -void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) >> +void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) >> { >> /* Evaluate the AF buffer length */ >> uint32_t afRawBufferLen = context.configuration.af.afGrid.width * >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h >> index b85cf941..ccf015f3 100644 >> --- a/src/ipa/ipu3/algorithms/af.h >> +++ b/src/ipa/ipu3/algorithms/af.h >> @@ -32,7 +32,8 @@ public: >> >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> void afCoarseScan(IPAContext &context); >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index fdeec09d..383a8deb 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -318,12 +318,14 @@ double Agc::estimateLuminance(IPAActiveState &activeState, >> /** >> * \brief Process IPU3 statistics, and run AGC operations >> * \param[in] context The shared IPA context >> + * \param[in] frameContext The current frame context >> * \param[in] stats The IPU3 statistics and ISP results >> * >> * Identify the current image brightness, and use that to estimate the optimal >> * new exposure and gain for the scene. >> */ >> -void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) >> +void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) >> { >> /* >> * Estimate the gain needed to have the proportion of pixels in a given >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index 31420841..219a1a96 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -28,7 +28,8 @@ public: >> ~Agc() = default; >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> double measureBrightness(const ipu3_uapi_stats_3a *stats, >> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h >> index d2eecc78..234b2bd7 100644 >> --- a/src/ipa/ipu3/algorithms/algorithm.h >> +++ b/src/ipa/ipu3/algorithms/algorithm.h >> @@ -17,7 +17,9 @@ namespace libcamera { >> >> namespace ipa::ipu3 { >> >> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, >> + IPAConfigInfo, ipu3_uapi_params, >> + ipu3_uapi_stats_3a>; >> >> } /* namespace ipa::ipu3 */ >> >> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp >> index ab6924eb..5c232d92 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, const ipu3_uapi_stats_3a *stats) >> +void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 ab4b0a33..9a50a985 100644 >> --- a/src/ipa/ipu3/algorithms/awb.h >> +++ b/src/ipa/ipu3/algorithms/awb.h >> @@ -40,7 +40,8 @@ public: >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> /* \todo Make these structs available to all the ISPs ? */ >> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp >> index 7c78d0d9..f86e79b2 100644 >> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp >> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp >> @@ -72,12 +72,13 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, >> /** >> * \brief Calculate the tone mapping look up table >> * \param context The shared IPA context >> + * \param frameContext The current frame context >> * \param stats The IPU3 statistics and ISP results >> * >> * The tone mapping look up table is generated as an inverse power curve from >> * our gamma setting. >> */ >> -void ToneMapping::process(IPAContext &context, >> +void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 b727ab1e..d7d48006 100644 >> --- a/src/ipa/ipu3/algorithms/tone_mapping.h >> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h >> @@ -20,7 +20,8 @@ public: >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> double gamma_; >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >> index 3b4fc911..16e5028f 100644 >> --- a/src/ipa/ipu3/ipu3.cpp >> +++ b/src/ipa/ipu3/ipu3.cpp >> @@ -576,7 +576,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, >> ControlList ctrls(controls::controls); >> >> for (auto const &algo : algorithms_) >> - algo->process(context_, stats); >> + algo->process(context_, nullptr, stats); >> >> setControls(frame); >> >> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp >> index 398d5372..cce2ed62 100644 >> --- a/src/ipa/libipa/algorithm.cpp >> +++ b/src/ipa/libipa/algorithm.cpp >> @@ -64,6 +64,7 @@ namespace ipa { >> * \fn Algorithm::process() >> * \brief Process ISP statistics, and run algorithm operations >> * \param[in] context The shared IPA context >> + * \param[in] frameContext The current frame's context >> * \param[in] stats The IPA statistics and ISP results >> * >> * This function is called while camera is running for every frame processed by >> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h >> index 766aee5d..032a05b5 100644 >> --- a/src/ipa/libipa/algorithm.h >> +++ b/src/ipa/libipa/algorithm.h >> @@ -10,7 +10,8 @@ namespace libcamera { >> >> namespace ipa { >> >> -template<typename Context, typename Config, typename Params, typename Stats> >> +template<typename Context, typename FrameContext, typename Config, >> + typename Params, typename Stats> I am wondering if these namings could be confusing. Maybe Context should be called State and then, you would get the FrameContext of this State passed down to the algorithm ? Nevertheless, Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> class Algorithm >> { >> public: >> @@ -28,6 +29,7 @@ public: >> } >> >> virtual void process([[maybe_unused]] Context &context, >> + [[maybe_unused]] FrameContext *frameContext, >> [[maybe_unused]] const Stats *stats) >> { >> } >> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp >> index 5f4c3f93..b5a184d9 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.cpp >> +++ b/src/ipa/rkisp1/algorithms/agc.cpp >> @@ -280,7 +280,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const >> * Identify the current image brightness, and use that to estimate the optimal >> * new exposure and gain for the scene. >> */ >> -void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) >> +void Agc::process(IPAContext &context, >> + [[maybe_unused]] IPAFrameContext *frameContext, >> + const rkisp1_stat_buffer *stats) >> { >> const rkisp1_cif_isp_stat *params = &stats->params; >> ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP); >> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h >> index ce1adf27..22c02779 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.h >> +++ b/src/ipa/rkisp1/algorithms/agc.h >> @@ -29,7 +29,8 @@ public: >> >> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; >> void prepare(IPAContext &context, rkisp1_params_cfg *params) override; >> - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const rkisp1_stat_buffer *stats) override; >> >> private: >> void computeExposure(IPAContext &Context, double yGain, double iqMeanGain); >> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h >> index d46c3188..68e3a44e 100644 >> --- a/src/ipa/rkisp1/algorithms/algorithm.h >> +++ b/src/ipa/rkisp1/algorithms/algorithm.h >> @@ -19,7 +19,9 @@ namespace libcamera { >> >> namespace ipa::rkisp1 { >> >> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, >> + IPACameraSensorInfo, rkisp1_params_cfg, >> + rkisp1_stat_buffer>; >> >> } /* namespace ipa::rkisp1 */ >> >> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp >> index be4585c6..88441382 100644 >> --- a/src/ipa/rkisp1/algorithms/awb.cpp >> +++ b/src/ipa/rkisp1/algorithms/awb.cpp >> @@ -119,7 +119,9 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) >> /** >> * \copydoc libcamera::ipa::Algorithm::process >> */ >> -void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats) >> +void Awb::process([[maybe_unused]] IPAContext &context, >> + [[maybe_unused]] IPAFrameContext *frameCtx, >> + const rkisp1_stat_buffer *stats) >> { >> const rkisp1_cif_isp_stat *params = &stats->params; >> const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; >> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h >> index 11946643..7647842f 100644 >> --- a/src/ipa/rkisp1/algorithms/awb.h >> +++ b/src/ipa/rkisp1/algorithms/awb.h >> @@ -23,7 +23,8 @@ public: >> >> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; >> void prepare(IPAContext &context, rkisp1_params_cfg *params) override; >> - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameCtx, >> + const rkisp1_stat_buffer *stats) override; >> >> private: >> uint32_t estimateCCT(double red, double green, double blue); >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp >> index ef1f0d56..c818a6d7 100644 >> --- a/src/ipa/rkisp1/rkisp1.cpp >> +++ b/src/ipa/rkisp1/rkisp1.cpp >> @@ -272,7 +272,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId >> unsigned int aeState = 0; >> >> for (auto const &algo : algorithms_) >> - algo->process(context_, stats); >> + algo->process(context_, nullptr, stats); >> >> setControls(frame); >> >> -- >> 2.31.0 >>
Quoting Umang Jain via libcamera-devel (2022-05-17 20:18:32) > Currently we have a single structure of IPAFrameContext but > subsequently, we shall have a ring buffer (or similar) container > to keep IPAFrameContext structures for each frame. > > It would be a hassle to query out the frame context required for > process() (since they will reside in a ring buffer) by the IPA > for each process. Hence, prepare the process() libipa template to > accept a particular IPAFrameContext early on. > > As for this patch, we shall pass in the pointer as nullptr, so > that the changes compile and keep working as-is. > Great, yes this simplifies getting this done for IPU3. I believe the FrameContext will become an inherent design requirement for the IPA and algorithm class when libipa is used, so I would expect that once IPU3 is done, we should add the same to the RKISP, and then convert this paramter from a pointer to a reference to enforce that any new implementation should implement a ring buffer for the contexts from the start. (but with two existing implementations, that's fine). As JM says, now we have context and frameContext, but I'm not sure how to make that clearer yet. But that could also be updated on top. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 4 +++- > src/ipa/ipu3/algorithms/af.h | 3 ++- > src/ipa/ipu3/algorithms/agc.cpp | 4 +++- > src/ipa/ipu3/algorithms/agc.h | 3 ++- > src/ipa/ipu3/algorithms/algorithm.h | 4 +++- > src/ipa/ipu3/algorithms/awb.cpp | 3 ++- > src/ipa/ipu3/algorithms/awb.h | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- > src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++- > src/ipa/ipu3/ipu3.cpp | 2 +- > src/ipa/libipa/algorithm.cpp | 1 + > src/ipa/libipa/algorithm.h | 4 +++- > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > src/ipa/rkisp1/algorithms/agc.h | 3 ++- > src/ipa/rkisp1/algorithms/algorithm.h | 4 +++- > src/ipa/rkisp1/algorithms/awb.cpp | 4 +++- > src/ipa/rkisp1/algorithms/awb.h | 3 ++- > src/ipa/rkisp1/rkisp1.cpp | 2 +- > 18 files changed, 40 insertions(+), 17 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 8a5a6b1a..d07521a0 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -406,6 +406,7 @@ bool Af::afIsOutOfFocus(IPAContext context) > /** > * \brief Determine the max contrast image and lens position. > * \param[in] context The IPA context. > + * \param[in] frameContext The current frame context > * \param[in] stats The statistics buffer of IPU3. > * > * Ideally, a clear image also has a relatively higher contrast. So, every > @@ -419,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > */ > -void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > +void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) > { > /* Evaluate the AF buffer length */ > uint32_t afRawBufferLen = context.configuration.af.afGrid.width * > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h > index b85cf941..ccf015f3 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -32,7 +32,8 @@ public: > > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > void afCoarseScan(IPAContext &context); > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index fdeec09d..383a8deb 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -318,12 +318,14 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > /** > * \brief Process IPU3 statistics, and run AGC operations > * \param[in] context The shared IPA context > + * \param[in] frameContext The current frame context > * \param[in] stats The IPU3 statistics and ISP results > * > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > +void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) > { > /* > * Estimate the gain needed to have the proportion of pixels in a given > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 31420841..219a1a96 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -28,7 +28,8 @@ public: > ~Agc() = default; > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > double measureBrightness(const ipu3_uapi_stats_3a *stats, > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h > index d2eecc78..234b2bd7 100644 > --- a/src/ipa/ipu3/algorithms/algorithm.h > +++ b/src/ipa/ipu3/algorithms/algorithm.h > @@ -17,7 +17,9 @@ namespace libcamera { > > namespace ipa::ipu3 { > > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, > + IPAConfigInfo, ipu3_uapi_params, > + ipu3_uapi_stats_3a>; > > } /* namespace ipa::ipu3 */ > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index ab6924eb..5c232d92 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, const ipu3_uapi_stats_3a *stats) > +void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 ab4b0a33..9a50a985 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -40,7 +40,8 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > /* \todo Make these structs available to all the ISPs ? */ > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp > index 7c78d0d9..f86e79b2 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -72,12 +72,13 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > /** > * \brief Calculate the tone mapping look up table > * \param context The shared IPA context > + * \param frameContext The current frame context > * \param stats The IPU3 statistics and ISP results > * > * The tone mapping look up table is generated as an inverse power curve from > * our gamma setting. > */ > -void ToneMapping::process(IPAContext &context, > +void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 b727ab1e..d7d48006 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -20,7 +20,8 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void prepare(IPAContext &context, ipu3_uapi_params *params) override; > - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const ipu3_uapi_stats_3a *stats) override; > > private: > double gamma_; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 3b4fc911..16e5028f 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -576,7 +576,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, > ControlList ctrls(controls::controls); > > for (auto const &algo : algorithms_) > - algo->process(context_, stats); > + algo->process(context_, nullptr, stats); > > setControls(frame); > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > index 398d5372..cce2ed62 100644 > --- a/src/ipa/libipa/algorithm.cpp > +++ b/src/ipa/libipa/algorithm.cpp > @@ -64,6 +64,7 @@ namespace ipa { > * \fn Algorithm::process() > * \brief Process ISP statistics, and run algorithm operations > * \param[in] context The shared IPA context > + * \param[in] frameContext The current frame's context > * \param[in] stats The IPA statistics and ISP results > * > * This function is called while camera is running for every frame processed by > diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h > index 766aee5d..032a05b5 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -10,7 +10,8 @@ namespace libcamera { > > namespace ipa { > > -template<typename Context, typename Config, typename Params, typename Stats> > +template<typename Context, typename FrameContext, typename Config, > + typename Params, typename Stats> > class Algorithm > { > public: > @@ -28,6 +29,7 @@ public: > } > > virtual void process([[maybe_unused]] Context &context, > + [[maybe_unused]] FrameContext *frameContext, > [[maybe_unused]] const Stats *stats) > { > } > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 5f4c3f93..b5a184d9 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -280,7 +280,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * Identify the current image brightness, and use that to estimate the optimal > * new exposure and gain for the scene. > */ > -void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) > +void Agc::process(IPAContext &context, > + [[maybe_unused]] IPAFrameContext *frameContext, > + const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP); > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index ce1adf27..22c02779 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -29,7 +29,8 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameContext, > + const rkisp1_stat_buffer *stats) override; > > private: > void computeExposure(IPAContext &Context, double yGain, double iqMeanGain); > diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h > index d46c3188..68e3a44e 100644 > --- a/src/ipa/rkisp1/algorithms/algorithm.h > +++ b/src/ipa/rkisp1/algorithms/algorithm.h > @@ -19,7 +19,9 @@ namespace libcamera { > > namespace ipa::rkisp1 { > > -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; > +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, > + IPACameraSensorInfo, rkisp1_params_cfg, > + rkisp1_stat_buffer>; > > } /* namespace ipa::rkisp1 */ > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index be4585c6..88441382 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -119,7 +119,9 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > /** > * \copydoc libcamera::ipa::Algorithm::process > */ > -void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats) > +void Awb::process([[maybe_unused]] IPAContext &context, > + [[maybe_unused]] IPAFrameContext *frameCtx, > + const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; > diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h > index 11946643..7647842f 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -23,7 +23,8 @@ public: > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; > + void process(IPAContext &context, IPAFrameContext *frameCtx, > + const rkisp1_stat_buffer *stats) override; > > private: > uint32_t estimateCCT(double red, double green, double blue); > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index ef1f0d56..c818a6d7 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -272,7 +272,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > unsigned int aeState = 0; > > for (auto const &algo : algorithms_) > - algo->process(context_, stats); > + algo->process(context_, nullptr, stats); > > setControls(frame); > > -- > 2.31.0 >
Hi Kieran, On 5/18/22 12:35, Kieran Bingham wrote: > Quoting Umang Jain via libcamera-devel (2022-05-17 20:18:32) >> Currently we have a single structure of IPAFrameContext but >> subsequently, we shall have a ring buffer (or similar) container >> to keep IPAFrameContext structures for each frame. >> >> It would be a hassle to query out the frame context required for >> process() (since they will reside in a ring buffer) by the IPA >> for each process. Hence, prepare the process() libipa template to >> accept a particular IPAFrameContext early on. >> >> As for this patch, we shall pass in the pointer as nullptr, so >> that the changes compile and keep working as-is. >> > Great, yes this simplifies getting this done for IPU3. > > I believe the FrameContext will become an inherent design requirement > for the IPA and algorithm class when libipa is used, so I would expect > that once IPU3 is done, we should add the same to the RKISP, and then > convert this paramter from a pointer to a reference to enforce that any > new implementation should implement a ring buffer for the contexts from > the start. (but with two existing implementations, that's fine). > > As JM says, now we have context and frameContext, but I'm not sure how > to make that clearer yet. But that could also be updated on top. If we want to bikeshed the names of various context structures, a good time will be after we land this series but before I start implementing all this stuff for RkISP. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/af.cpp | 4 +++- >> src/ipa/ipu3/algorithms/af.h | 3 ++- >> src/ipa/ipu3/algorithms/agc.cpp | 4 +++- >> src/ipa/ipu3/algorithms/agc.h | 3 ++- >> src/ipa/ipu3/algorithms/algorithm.h | 4 +++- >> src/ipa/ipu3/algorithms/awb.cpp | 3 ++- >> src/ipa/ipu3/algorithms/awb.h | 3 ++- >> src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- >> src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++- >> src/ipa/ipu3/ipu3.cpp | 2 +- >> src/ipa/libipa/algorithm.cpp | 1 + >> src/ipa/libipa/algorithm.h | 4 +++- >> src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- >> src/ipa/rkisp1/algorithms/agc.h | 3 ++- >> src/ipa/rkisp1/algorithms/algorithm.h | 4 +++- >> src/ipa/rkisp1/algorithms/awb.cpp | 4 +++- >> src/ipa/rkisp1/algorithms/awb.h | 3 ++- >> src/ipa/rkisp1/rkisp1.cpp | 2 +- >> 18 files changed, 40 insertions(+), 17 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp >> index 8a5a6b1a..d07521a0 100644 >> --- a/src/ipa/ipu3/algorithms/af.cpp >> +++ b/src/ipa/ipu3/algorithms/af.cpp >> @@ -406,6 +406,7 @@ bool Af::afIsOutOfFocus(IPAContext context) >> /** >> * \brief Determine the max contrast image and lens position. >> * \param[in] context The IPA context. >> + * \param[in] frameContext The current frame context >> * \param[in] stats The statistics buffer of IPU3. >> * >> * Ideally, a clear image also has a relatively higher contrast. So, every >> @@ -419,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) >> * >> * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing >> */ >> -void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) >> +void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) >> { >> /* Evaluate the AF buffer length */ >> uint32_t afRawBufferLen = context.configuration.af.afGrid.width * >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h >> index b85cf941..ccf015f3 100644 >> --- a/src/ipa/ipu3/algorithms/af.h >> +++ b/src/ipa/ipu3/algorithms/af.h >> @@ -32,7 +32,8 @@ public: >> >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> void afCoarseScan(IPAContext &context); >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index fdeec09d..383a8deb 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -318,12 +318,14 @@ double Agc::estimateLuminance(IPAActiveState &activeState, >> /** >> * \brief Process IPU3 statistics, and run AGC operations >> * \param[in] context The shared IPA context >> + * \param[in] frameContext The current frame context >> * \param[in] stats The IPU3 statistics and ISP results >> * >> * Identify the current image brightness, and use that to estimate the optimal >> * new exposure and gain for the scene. >> */ >> -void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) >> +void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) >> { >> /* >> * Estimate the gain needed to have the proportion of pixels in a given >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index 31420841..219a1a96 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -28,7 +28,8 @@ public: >> ~Agc() = default; >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> double measureBrightness(const ipu3_uapi_stats_3a *stats, >> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h >> index d2eecc78..234b2bd7 100644 >> --- a/src/ipa/ipu3/algorithms/algorithm.h >> +++ b/src/ipa/ipu3/algorithms/algorithm.h >> @@ -17,7 +17,9 @@ namespace libcamera { >> >> namespace ipa::ipu3 { >> >> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, >> + IPAConfigInfo, ipu3_uapi_params, >> + ipu3_uapi_stats_3a>; >> >> } /* namespace ipa::ipu3 */ >> >> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp >> index ab6924eb..5c232d92 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, const ipu3_uapi_stats_3a *stats) >> +void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 ab4b0a33..9a50a985 100644 >> --- a/src/ipa/ipu3/algorithms/awb.h >> +++ b/src/ipa/ipu3/algorithms/awb.h >> @@ -40,7 +40,8 @@ public: >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> /* \todo Make these structs available to all the ISPs ? */ >> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp >> index 7c78d0d9..f86e79b2 100644 >> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp >> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp >> @@ -72,12 +72,13 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, >> /** >> * \brief Calculate the tone mapping look up table >> * \param context The shared IPA context >> + * \param frameContext The current frame context >> * \param stats The IPU3 statistics and ISP results >> * >> * The tone mapping look up table is generated as an inverse power curve from >> * our gamma setting. >> */ >> -void ToneMapping::process(IPAContext &context, >> +void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 b727ab1e..d7d48006 100644 >> --- a/src/ipa/ipu3/algorithms/tone_mapping.h >> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h >> @@ -20,7 +20,8 @@ public: >> >> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; >> void prepare(IPAContext &context, ipu3_uapi_params *params) override; >> - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const ipu3_uapi_stats_3a *stats) override; >> >> private: >> double gamma_; >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp >> index 3b4fc911..16e5028f 100644 >> --- a/src/ipa/ipu3/ipu3.cpp >> +++ b/src/ipa/ipu3/ipu3.cpp >> @@ -576,7 +576,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, >> ControlList ctrls(controls::controls); >> >> for (auto const &algo : algorithms_) >> - algo->process(context_, stats); >> + algo->process(context_, nullptr, stats); >> >> setControls(frame); >> >> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp >> index 398d5372..cce2ed62 100644 >> --- a/src/ipa/libipa/algorithm.cpp >> +++ b/src/ipa/libipa/algorithm.cpp >> @@ -64,6 +64,7 @@ namespace ipa { >> * \fn Algorithm::process() >> * \brief Process ISP statistics, and run algorithm operations >> * \param[in] context The shared IPA context >> + * \param[in] frameContext The current frame's context >> * \param[in] stats The IPA statistics and ISP results >> * >> * This function is called while camera is running for every frame processed by >> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h >> index 766aee5d..032a05b5 100644 >> --- a/src/ipa/libipa/algorithm.h >> +++ b/src/ipa/libipa/algorithm.h >> @@ -10,7 +10,8 @@ namespace libcamera { >> >> namespace ipa { >> >> -template<typename Context, typename Config, typename Params, typename Stats> >> +template<typename Context, typename FrameContext, typename Config, >> + typename Params, typename Stats> >> class Algorithm >> { >> public: >> @@ -28,6 +29,7 @@ public: >> } >> >> virtual void process([[maybe_unused]] Context &context, >> + [[maybe_unused]] FrameContext *frameContext, >> [[maybe_unused]] const Stats *stats) >> { >> } >> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp >> index 5f4c3f93..b5a184d9 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.cpp >> +++ b/src/ipa/rkisp1/algorithms/agc.cpp >> @@ -280,7 +280,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const >> * Identify the current image brightness, and use that to estimate the optimal >> * new exposure and gain for the scene. >> */ >> -void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) >> +void Agc::process(IPAContext &context, >> + [[maybe_unused]] IPAFrameContext *frameContext, >> + const rkisp1_stat_buffer *stats) >> { >> const rkisp1_cif_isp_stat *params = &stats->params; >> ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP); >> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h >> index ce1adf27..22c02779 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.h >> +++ b/src/ipa/rkisp1/algorithms/agc.h >> @@ -29,7 +29,8 @@ public: >> >> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; >> void prepare(IPAContext &context, rkisp1_params_cfg *params) override; >> - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameContext, >> + const rkisp1_stat_buffer *stats) override; >> >> private: >> void computeExposure(IPAContext &Context, double yGain, double iqMeanGain); >> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h >> index d46c3188..68e3a44e 100644 >> --- a/src/ipa/rkisp1/algorithms/algorithm.h >> +++ b/src/ipa/rkisp1/algorithms/algorithm.h >> @@ -19,7 +19,9 @@ namespace libcamera { >> >> namespace ipa::rkisp1 { >> >> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; >> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, >> + IPACameraSensorInfo, rkisp1_params_cfg, >> + rkisp1_stat_buffer>; >> >> } /* namespace ipa::rkisp1 */ >> >> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp >> index be4585c6..88441382 100644 >> --- a/src/ipa/rkisp1/algorithms/awb.cpp >> +++ b/src/ipa/rkisp1/algorithms/awb.cpp >> @@ -119,7 +119,9 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) >> /** >> * \copydoc libcamera::ipa::Algorithm::process >> */ >> -void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats) >> +void Awb::process([[maybe_unused]] IPAContext &context, >> + [[maybe_unused]] IPAFrameContext *frameCtx, >> + const rkisp1_stat_buffer *stats) >> { >> const rkisp1_cif_isp_stat *params = &stats->params; >> const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; >> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h >> index 11946643..7647842f 100644 >> --- a/src/ipa/rkisp1/algorithms/awb.h >> +++ b/src/ipa/rkisp1/algorithms/awb.h >> @@ -23,7 +23,8 @@ public: >> >> int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; >> void prepare(IPAContext &context, rkisp1_params_cfg *params) override; >> - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; >> + void process(IPAContext &context, IPAFrameContext *frameCtx, >> + const rkisp1_stat_buffer *stats) override; >> >> private: >> uint32_t estimateCCT(double red, double green, double blue); >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp >> index ef1f0d56..c818a6d7 100644 >> --- a/src/ipa/rkisp1/rkisp1.cpp >> +++ b/src/ipa/rkisp1/rkisp1.cpp >> @@ -272,7 +272,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId >> unsigned int aeState = 0; >> >> for (auto const &algo : algorithms_) >> - algo->process(context_, stats); >> + algo->process(context_, nullptr, stats); >> >> setControls(frame); >> >> -- >> 2.31.0 >>
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index 8a5a6b1a..d07521a0 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -406,6 +406,7 @@ bool Af::afIsOutOfFocus(IPAContext context) /** * \brief Determine the max contrast image and lens position. * \param[in] context The IPA context. + * \param[in] frameContext The current frame context * \param[in] stats The statistics buffer of IPU3. * * Ideally, a clear image also has a relatively higher contrast. So, every @@ -419,7 +420,8 @@ bool Af::afIsOutOfFocus(IPAContext context) * * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing */ -void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) +void Af::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) { /* Evaluate the AF buffer length */ uint32_t afRawBufferLen = context.configuration.af.afGrid.width * diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h index b85cf941..ccf015f3 100644 --- a/src/ipa/ipu3/algorithms/af.h +++ b/src/ipa/ipu3/algorithms/af.h @@ -32,7 +32,8 @@ public: void prepare(IPAContext &context, ipu3_uapi_params *params) override; int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; + void process(IPAContext &context, IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) override; private: void afCoarseScan(IPAContext &context); diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index fdeec09d..383a8deb 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -318,12 +318,14 @@ double Agc::estimateLuminance(IPAActiveState &activeState, /** * \brief Process IPU3 statistics, and run AGC operations * \param[in] context The shared IPA context + * \param[in] frameContext The current frame context * \param[in] stats The IPU3 statistics and ISP results * * Identify the current image brightness, and use that to estimate the optimal * new exposure and gain for the scene. */ -void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) +void Agc::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) { /* * Estimate the gain needed to have the proportion of pixels in a given diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 31420841..219a1a96 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -28,7 +28,8 @@ public: ~Agc() = default; int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; + void process(IPAContext &context, IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) override; private: double measureBrightness(const ipu3_uapi_stats_3a *stats, diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h index d2eecc78..234b2bd7 100644 --- a/src/ipa/ipu3/algorithms/algorithm.h +++ b/src/ipa/ipu3/algorithms/algorithm.h @@ -17,7 +17,9 @@ namespace libcamera { namespace ipa::ipu3 { -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>; +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, + IPAConfigInfo, ipu3_uapi_params, + ipu3_uapi_stats_3a>; } /* namespace ipa::ipu3 */ diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index ab6924eb..5c232d92 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, const ipu3_uapi_stats_3a *stats) +void Awb::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 ab4b0a33..9a50a985 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -40,7 +40,8 @@ public: int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void prepare(IPAContext &context, ipu3_uapi_params *params) override; - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; + void process(IPAContext &context, IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) override; private: /* \todo Make these structs available to all the ISPs ? */ diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp index 7c78d0d9..f86e79b2 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp @@ -72,12 +72,13 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, /** * \brief Calculate the tone mapping look up table * \param context The shared IPA context + * \param frameContext The current frame context * \param stats The IPU3 statistics and ISP results * * The tone mapping look up table is generated as an inverse power curve from * our gamma setting. */ -void ToneMapping::process(IPAContext &context, +void ToneMapping::process(IPAContext &context, [[maybe_unused]] IPAFrameContext *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 b727ab1e..d7d48006 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.h +++ b/src/ipa/ipu3/algorithms/tone_mapping.h @@ -20,7 +20,8 @@ public: int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void prepare(IPAContext &context, ipu3_uapi_params *params) override; - void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; + void process(IPAContext &context, IPAFrameContext *frameContext, + const ipu3_uapi_stats_3a *stats) override; private: double gamma_; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 3b4fc911..16e5028f 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -576,7 +576,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, ControlList ctrls(controls::controls); for (auto const &algo : algorithms_) - algo->process(context_, stats); + algo->process(context_, nullptr, stats); setControls(frame); diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp index 398d5372..cce2ed62 100644 --- a/src/ipa/libipa/algorithm.cpp +++ b/src/ipa/libipa/algorithm.cpp @@ -64,6 +64,7 @@ namespace ipa { * \fn Algorithm::process() * \brief Process ISP statistics, and run algorithm operations * \param[in] context The shared IPA context + * \param[in] frameContext The current frame's context * \param[in] stats The IPA statistics and ISP results * * This function is called while camera is running for every frame processed by diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h index 766aee5d..032a05b5 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -10,7 +10,8 @@ namespace libcamera { namespace ipa { -template<typename Context, typename Config, typename Params, typename Stats> +template<typename Context, typename FrameContext, typename Config, + typename Params, typename Stats> class Algorithm { public: @@ -28,6 +29,7 @@ public: } virtual void process([[maybe_unused]] Context &context, + [[maybe_unused]] FrameContext *frameContext, [[maybe_unused]] const Stats *stats) { } diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 5f4c3f93..b5a184d9 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -280,7 +280,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const * Identify the current image brightness, and use that to estimate the optimal * new exposure and gain for the scene. */ -void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) +void Agc::process(IPAContext &context, + [[maybe_unused]] IPAFrameContext *frameContext, + const rkisp1_stat_buffer *stats) { const rkisp1_cif_isp_stat *params = &stats->params; ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP); diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index ce1adf27..22c02779 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -29,7 +29,8 @@ public: int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; + void process(IPAContext &context, IPAFrameContext *frameContext, + const rkisp1_stat_buffer *stats) override; private: void computeExposure(IPAContext &Context, double yGain, double iqMeanGain); diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h index d46c3188..68e3a44e 100644 --- a/src/ipa/rkisp1/algorithms/algorithm.h +++ b/src/ipa/rkisp1/algorithms/algorithm.h @@ -19,7 +19,9 @@ namespace libcamera { namespace ipa::rkisp1 { -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>; +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext, + IPACameraSensorInfo, rkisp1_params_cfg, + rkisp1_stat_buffer>; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index be4585c6..88441382 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -119,7 +119,9 @@ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) /** * \copydoc libcamera::ipa::Algorithm::process */ -void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer *stats) +void Awb::process([[maybe_unused]] IPAContext &context, + [[maybe_unused]] IPAFrameContext *frameCtx, + const rkisp1_stat_buffer *stats) { const rkisp1_cif_isp_stat *params = &stats->params; const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h index 11946643..7647842f 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -23,7 +23,8 @@ public: int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; - void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; + void process(IPAContext &context, IPAFrameContext *frameCtx, + const rkisp1_stat_buffer *stats) override; private: uint32_t estimateCCT(double red, double green, double blue); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index ef1f0d56..c818a6d7 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -272,7 +272,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId unsigned int aeState = 0; for (auto const &algo : algorithms_) - algo->process(context_, stats); + algo->process(context_, nullptr, stats); setControls(frame);
Currently we have a single structure of IPAFrameContext but subsequently, we shall have a ring buffer (or similar) container to keep IPAFrameContext structures for each frame. It would be a hassle to query out the frame context required for process() (since they will reside in a ring buffer) by the IPA for each process. Hence, prepare the process() libipa template to accept a particular IPAFrameContext early on. As for this patch, we shall pass in the pointer as nullptr, so that the changes compile and keep working as-is. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/ipa/ipu3/algorithms/af.cpp | 4 +++- src/ipa/ipu3/algorithms/af.h | 3 ++- src/ipa/ipu3/algorithms/agc.cpp | 4 +++- src/ipa/ipu3/algorithms/agc.h | 3 ++- src/ipa/ipu3/algorithms/algorithm.h | 4 +++- src/ipa/ipu3/algorithms/awb.cpp | 3 ++- src/ipa/ipu3/algorithms/awb.h | 3 ++- src/ipa/ipu3/algorithms/tone_mapping.cpp | 3 ++- src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++- src/ipa/ipu3/ipu3.cpp | 2 +- src/ipa/libipa/algorithm.cpp | 1 + src/ipa/libipa/algorithm.h | 4 +++- src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- src/ipa/rkisp1/algorithms/agc.h | 3 ++- src/ipa/rkisp1/algorithms/algorithm.h | 4 +++- src/ipa/rkisp1/algorithms/awb.cpp | 4 +++- src/ipa/rkisp1/algorithms/awb.h | 3 ++- src/ipa/rkisp1/rkisp1.cpp | 2 +- 18 files changed, 40 insertions(+), 17 deletions(-)