[libcamera-devel] [PATCH v2 1/3] ipa: Pass metadata to Algorithm::process() function
Jacopo Mondi
jacopo at jmondi.org
Thu Oct 20 09:28:14 CEST 2022
Hi Laurent
On Wed, Oct 19, 2022 at 11:56:12PM +0300, Laurent Pinchart via libcamera-devel wrote:
> Extend the Algorithm::process() function with a metadata control list,
> to be filled by individual algorithms with frame metadata. Update the
> rkisp1 and ipu3 IPA modules accordingly, and drop the dead code in the
> IPARkISP1::prepareMetadata() function while at it.
>
> This only creates the infrastructure, filling metadata in individual
> algorithms will be handled separately.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> ---
> Changes since v1:
>
> - Pass metadata as last argument to process()
> - Add missing documentation update in src/ipa/rkisp1/algorithms/agc.cpp
> - Correctly send metadata to pipeline handler in IPU3 IPA module
> ---
> 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/awb.cpp | 3 ++-
> src/ipa/ipu3/algorithms/awb.h | 3 ++-
> src/ipa/ipu3/algorithms/tone_mapping.cpp | 4 +++-
> src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++-
> src/ipa/ipu3/ipu3.cpp | 14 +++++++-------
> src/ipa/libipa/algorithm.cpp | 5 +++--
> src/ipa/libipa/algorithm.h | 3 ++-
> src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-
> src/ipa/rkisp1/algorithms/agc.h | 3 ++-
> src/ipa/rkisp1/algorithms/awb.cpp | 3 ++-
> src/ipa/rkisp1/algorithms/awb.h | 5 +++--
> src/ipa/rkisp1/rkisp1.cpp | 17 +++--------------
> 16 files changed, 44 insertions(+), 37 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 75632aa39d21..5a0452a5719b 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -409,6 +409,7 @@ bool Af::afIsOutOfFocus(IPAContext &context)
> * \param[in] frame The frame context sequence number
> * \param[in] frameContext The current frame context
> * \param[in] stats The statistics buffer of IPU3
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> *
> * Ideally, a clear image also has a relatively higher contrast. So, every
> * image for each focus step should be tested to find an optimal focus step.
> @@ -423,7 +424,8 @@ bool Af::afIsOutOfFocus(IPAContext &context)
> */
> void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> [[maybe_unused]] IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats)
> + const ipu3_uapi_stats_3a *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> /* 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 89d37ac18615..c6168e30fb2e 100644
> --- a/src/ipa/ipu3/algorithms/af.h
> +++ b/src/ipa/ipu3/algorithms/af.h
> @@ -36,7 +36,8 @@ public:
> ipu3_uapi_params *params) override;
> void process(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats) override;
> + const ipu3_uapi_stats_3a *stats,
> + ControlList &metadata) override;
>
> private:
> void afCoarseScan(IPAContext &context);
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index a1a3c38ffe84..f4e559bf8b84 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -320,13 +320,15 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
> * \param[in] frame The current frame sequence number
> * \param[in] frameContext The current frame context
> * \param[in] stats The IPU3 statistics and ISP results
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> *
> * 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]] const uint32_t frame,
> IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats)
> + const ipu3_uapi_stats_3a *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> /*
> * 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 59b4b9843c2f..9d6e3ff1a687 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -30,7 +30,8 @@ public:
> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> void process(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats) override;
> + const ipu3_uapi_stats_3a *stats,
> + ControlList &metadata) override;
>
> private:
> double measureBrightness(const ipu3_uapi_stats_3a *stats,
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index 0dbd7d4c374f..6452b6a1acd2 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -389,7 +389,8 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> */
> void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> [[maybe_unused]] IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats)
> + const ipu3_uapi_stats_3a *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> calculateWBGains(stats);
>
> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> index 28e2d38a711c..7a70854e8999 100644
> --- a/src/ipa/ipu3/algorithms/awb.h
> +++ b/src/ipa/ipu3/algorithms/awb.h
> @@ -44,7 +44,8 @@ public:
> ipu3_uapi_params *params) override;
> void process(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats) override;
> + const ipu3_uapi_stats_3a *stats,
> + ControlList &metadata) 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 eac3d4064443..a169894cb628 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -79,13 +79,15 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,
> * \param[in] frame The current frame sequence number
> * \param[in] frameContext The current frame context
> * \param[in] stats The IPU3 statistics and ISP results
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> *
> * The tone mapping look up table is generated as an inverse power curve from
> * our gamma setting.
> */
> void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> [[maybe_unused]] IPAFrameContext &frameContext,
> - [[maybe_unused]] const ipu3_uapi_stats_3a *stats)
> + [[maybe_unused]] const ipu3_uapi_stats_3a *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> /*
> * Hardcode gamma to 1.1 as a default for now.
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h
> index 822e5168df82..5ae35da55e87 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.h
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h
> @@ -23,7 +23,8 @@ public:
> IPAFrameContext &frameContext, ipu3_uapi_params *params) override;
> void process(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> - const ipu3_uapi_stats_3a *stats) override;
> + const ipu3_uapi_stats_3a *stats,
> + ControlList &metadata) override;
>
> private:
> double gamma_;
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 891643e057b8..bc0f6007baca 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -630,22 +630,22 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
>
> double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
> int32_t vBlank = context_.configuration.sensor.defVBlank;
> - ControlList ctrls(controls::controls);
> + ControlList metadata(controls::controls);
>
> for (auto const &algo : algorithms())
> - algo->process(context_, frame, frameContext, stats);
> + algo->process(context_, frame, frameContext, stats, metadata);
>
> setControls(frame);
>
> /* \todo Use VBlank value calculated from each frame exposure. */
> int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;
> - ctrls.set(controls::FrameDuration, frameDuration);
> + metadata.set(controls::FrameDuration, frameDuration);
>
> - ctrls.set(controls::AnalogueGain, frameContext.sensor.gain);
> + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
>
> - ctrls.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);
> + metadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);
>
> - ctrls.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);
> + metadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);
>
> /*
> * \todo The Metadata provides a path to getting extended data
> @@ -655,7 +655,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> * likely want to avoid putting platform specific metadata in.
> */
>
> - metadataReady.emit(frame, ctrls);
> + metadataReady.emit(frame, metadata);
> }
>
> /**
> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> index 55abddab2b0a..71f583840a5a 100644
> --- a/src/ipa/libipa/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -107,11 +107,12 @@ namespace ipa {
> * \param[in] frame The frame context sequence number
> * \param[in] frameContext The current frame's context
> * \param[in] stats The IPA statistics and ISP results
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> *
> * This function is called while camera is running for every frame processed by
> * the ISP, to process statistics generated from that frame by the ISP.
> - * Algorithms shall use this data to run calculations and update their state
> - * accordingly.
> + * Algorithms shall use this data to run calculations, update their state
> + * accordingly, and fill the frame metadata.
> *
> * Processing shall not take an undue amount of time, and any extended or
> * computationally expensive calculations or operations must be handled
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> index d8601f9ccaff..38b3231c5d0e 100644
> --- a/src/ipa/libipa/algorithm.h
> +++ b/src/ipa/libipa/algorithm.h
> @@ -55,7 +55,8 @@ public:
> virtual void process([[maybe_unused]] typename Module::Context &context,
> [[maybe_unused]] const uint32_t frame,
> [[maybe_unused]] typename Module::FrameContext &frameContext,
> - [[maybe_unused]] const typename Module::Stats *stats)
> + [[maybe_unused]] const typename Module::Stats *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> }
> };
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 04062a364e7f..37cd10f9b302 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -283,12 +283,14 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const
> * \param[in] frame The frame context sequence number
> * \param[in] frameContext The current frame context
> * \param[in] stats The RKISP1 statistics and ISP results
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> *
> * 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]] const uint32_t frame,
> - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats)
> + IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> /*
> * \todo Verify that the exposure and gain applied by the sensor for
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 9ad5c32fd6f6..da4d2d4e8359 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -31,7 +31,8 @@ public:
> rkisp1_params_cfg *params) override;
> void process(IPAContext &context, const uint32_t frame,
> IPAFrameContext &frameContext,
> - const rkisp1_stat_buffer *stats) override;
> + const rkisp1_stat_buffer *stats,
> + ControlList &metadata) override;
>
> private:
> void computeExposure(IPAContext &Context, IPAFrameContext &frameContext,
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 3349948a3acf..f597ded800c2 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -208,7 +208,8 @@ void Awb::queueRequest(IPAContext &context,
> void Awb::process(IPAContext &context,
> [[maybe_unused]] const uint32_t frame,
> IPAFrameContext &frameContext,
> - const rkisp1_stat_buffer *stats)
> + const rkisp1_stat_buffer *stats,
> + [[maybe_unused]] ControlList &metadata)
> {
> 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 d76b538288ec..f5633b1171a0 100644
> --- a/src/ipa/rkisp1/algorithms/awb.h
> +++ b/src/ipa/rkisp1/algorithms/awb.h
> @@ -27,8 +27,9 @@ public:
> IPAFrameContext &frameContext,
> const ControlList &controls) override;
> void process(IPAContext &context, const uint32_t frame,
> - IPAFrameContext &frameCtx,
> - const rkisp1_stat_buffer *stats) override;
> + IPAFrameContext &frameContext,
> + const rkisp1_stat_buffer *stats,
> + ControlList &metadata) 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 ba3c547e02f6..494d8c891509 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -69,7 +69,6 @@ protected:
>
> private:
> void setControls(unsigned int frame);
> - void prepareMetadata(unsigned int frame, unsigned int aeState);
>
> std::map<unsigned int, FrameBuffer> buffers_;
> std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
> @@ -338,14 +337,14 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
> frameContext.sensor.gain =
> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>
> - unsigned int aeState = 0;
> + ControlList metadata(controls::controls);
>
> for (auto const &algo : algorithms())
> - algo->process(context_, frame, frameContext, stats);
> + algo->process(context_, frame, frameContext, stats, metadata);
>
> setControls(frame);
>
> - prepareMetadata(frame, aeState);
> + metadataReady.emit(frame, metadata);
> }
>
> void IPARkISP1::setControls(unsigned int frame)
> @@ -366,16 +365,6 @@ void IPARkISP1::setControls(unsigned int frame)
> setSensorControls.emit(frame, ctrls);
> }
>
> -void IPARkISP1::prepareMetadata(unsigned int frame, unsigned int aeState)
> -{
> - ControlList ctrls(controls::controls);
> -
> - if (aeState)
> - ctrls.set(controls::AeLocked, aeState == 2);
> -
> - metadataReady.emit(frame, ctrls);
> -}
> -
> } /* namespace ipa::rkisp1 */
>
> /*
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list