[libcamera-devel] [PATCH v4 03/32] ipa: libipa: Pass a reference instead of pointer to Algorithm::process()
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Sep 20 15:25:20 CEST 2022
Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:31)
> Frame contexts will become the core component of IPA modules, always
> available to functions of the algorithms. To indicate and prepare for
> this, turn the frame context pointer passed to Algorithm::process() into
> a reference.
Excellent.
> The RkISP1 IPA module doesn't use frame contexts yet, so pass a dummy
> context for now.
>
> While at it, drop an unneeded [[maybe_unused]] from Agc::process().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/af.cpp | 2 +-
> src/ipa/ipu3/algorithms/af.h | 2 +-
> src/ipa/ipu3/algorithms/agc.cpp | 8 ++++----
> src/ipa/ipu3/algorithms/agc.h | 4 ++--
> src/ipa/ipu3/algorithms/awb.cpp | 2 +-
> src/ipa/ipu3/algorithms/awb.h | 2 +-
> src/ipa/ipu3/algorithms/tone_mapping.cpp | 2 +-
> src/ipa/ipu3/algorithms/tone_mapping.h | 2 +-
> src/ipa/ipu3/ipu3.cpp | 2 +-
> src/ipa/libipa/algorithm.h | 2 +-
> src/ipa/rkisp1/algorithms/agc.cpp | 3 ++-
> src/ipa/rkisp1/algorithms/agc.h | 2 +-
> src/ipa/rkisp1/algorithms/awb.cpp | 2 +-
> src/ipa/rkisp1/algorithms/awb.h | 2 +-
> src/ipa/rkisp1/rkisp1.cpp | 5 ++++-
> 15 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 9127c24f1287..5ab64bf88313 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -417,7 +417,7 @@ 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]] IPAFrameContext &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 9b93594898bb..29117e8bdd0d 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, IPAFrameContext &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 ed4809d98007..f8ca29640a44 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, IPAFrameContext &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,7 @@ 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, IPAFrameContext &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..876fbbb6b585 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, IPAFrameContext &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, IPAFrameContext &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 b658ee546b87..128f5d9d5351 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -387,7 +387,7 @@ 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]] 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 0acd21480845..ebbb2d58be7e 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, IPAFrameContext &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 c21647e8c51b..24faba9be2b2 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -78,7 +78,7 @@ 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]] 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 d7d4800628f2..5be62f6ee9bd 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, IPAFrameContext &frameContext,
> const ipu3_uapi_stats_3a *stats) override;
>
> private:
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index e3c1e8160759..f0c92507533c 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -616,7 +616,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);
>
> 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/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 708c36ed1af8..7642796d69a3 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -275,13 +275,14 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const
> /**
> * \brief Process RkISP1 statistics, and run AGC operations
> * \param[in] context The shared IPA context
> + * \param[in] frame The frame context sequence number
I don't think this is in yet ... Perhaps this is for a later patch.
With that,
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> * \param[in] stats The RKISP1 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,
> - [[maybe_unused]] IPAFrameContext *frameContext,
> + [[maybe_unused]] IPAFrameContext &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..6a5723ecbcf8 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, IPAFrameContext &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 b12df21de4aa..2bc5d3aa16ab 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -152,7 +152,7 @@ void Awb::queueRequest(IPAContext &context,
> * \copydoc libcamera::ipa::Algorithm::process
> */
> void Awb::process([[maybe_unused]] IPAContext &context,
> - [[maybe_unused]] IPAFrameContext *frameCtx,
> + [[maybe_unused]] IPAFrameContext &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 4917267bb99d..fc221acea617 100644
> --- a/src/ipa/rkisp1/algorithms/awb.h
> +++ b/src/ipa/rkisp1/algorithms/awb.h
> @@ -23,7 +23,7 @@ public:
> void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> void queueRequest(IPAContext &context, const uint32_t frame,
> const ControlList &controls) override;
> - void process(IPAContext &context, IPAFrameContext *frameCtx,
> + void process(IPAContext &context, IPAFrameContext &frameCtx,
> const rkisp1_stat_buffer *stats) override;
>
> private:
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 3ff1eb48f605..3bc6d4dd2784 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -326,8 +326,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 */
> + IPAFrameContext frameContext;
> +
> for (auto const &algo : algorithms())
> - algo->process(context_, nullptr, stats);
> + algo->process(context_, frameContext, stats);
>
> setControls(frame);
>
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list