| Message ID | 20221019110434.17767-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart via libcamera-devel (2022-10-19 12:04:32) > 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@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 2 ++ > src/ipa/ipu3/algorithms/af.h | 2 +- > src/ipa/ipu3/algorithms/agc.cpp | 2 ++ > src/ipa/ipu3/algorithms/agc.h | 2 +- > src/ipa/ipu3/algorithms/awb.cpp | 1 + > 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 | 3 ++- > src/ipa/libipa/algorithm.cpp | 5 +++-- > src/ipa/libipa/algorithm.h | 1 + > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > src/ipa/rkisp1/algorithms/agc.h | 2 +- > src/ipa/rkisp1/algorithms/awb.cpp | 1 + > src/ipa/rkisp1/algorithms/awb.h | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 17 +++-------------- > 16 files changed, 26 insertions(+), 24 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 75632aa39d21..2d728871a63b 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -408,6 +408,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > * \param[in] context The IPA context > * \param[in] frame The frame context sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The statistics buffer of IPU3 > * > * Ideally, a clear image also has a relatively higher contrast. So, every > @@ -423,6 +424,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > */ > void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 89d37ac18615..ea7ca647df12 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -35,7 +35,7 @@ public: > IPAFrameContext &frameContext, > ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > 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 a1a3c38ffe84..44dd5c809e26 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -319,6 +319,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > * \param[in] context The shared IPA context > * \param[in] frame The current frame sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPU3 statistics and ISP results > * > * Identify the current image brightness, and use that to estimate the optimal > @@ -326,6 +327,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > */ > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 59b4b9843c2f..9106cbefe87a 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -29,7 +29,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index 0dbd7d4c374f..836eb0687181 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -389,6 +389,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > */ > void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 28e2d38a711c..c38c425c3654 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -43,7 +43,7 @@ public: > IPAFrameContext &frameContext, > ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > 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 eac3d4064443..6fcc03f3fcf7 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -78,6 +78,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > * \param[in] context The shared IPA context > * \param[in] frame The current frame sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPU3 statistics and ISP results > * > * The tone mapping look up table is generated as an inverse power curve from > @@ -85,6 +86,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > */ > void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > [[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 822e5168df82..ab2a1c000f2f 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -22,7 +22,7 @@ public: > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 891643e057b8..0ccc6bf5c8af 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -631,9 +631,10 @@ 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); It looks like ctrls() is what is currently used as metadata here. It gets emitted at the end of this function. Can we either use ctrls passed in to algo->process() or (preferred) rename ctrls to metadata here to ensure that /when/ we set metadata in the algorithms it will reach the pipeline handler? Probably keep the existing metadata additions in this function still - converting them can be later, but otherwise this is a partial implementation for IPU3. *with that* Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + ControlList metadata(controls::controls); > > for (auto const &algo : algorithms()) > - algo->process(context_, frame, frameContext, stats); > + algo->process(context_, frame, frameContext, metadata, stats); I would have had metadata as the last parameter as an 'output' (inputs, outputs) ... but I bet you've gone alphabetical. No point changing it though - that's too much work, and this is fine. > setControls(frame); > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > index 55abddab2b0a..a34c583d947e 100644 > --- a/src/ipa/libipa/algorithm.cpp > +++ b/src/ipa/libipa/algorithm.cpp > @@ -106,12 +106,13 @@ namespace ipa { > * \param[in] context The shared IPA context > * \param[in] frame The frame context sequence number > * \param[in] frameContext The current frame's context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPA statistics and ISP results > * > * 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..793247be8129 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -55,6 +55,7 @@ public: > virtual void process([[maybe_unused]] typename Module::Context &context, > [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] typename Module::FrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > [[maybe_unused]] const typename Module::Stats *stats) > { > } > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 04062a364e7f..9de11a2a3bca 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -288,7 +288,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * new exposure and gain for the scene. > */ > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) > + IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > + const rkisp1_stat_buffer *stats) > { > /* > * \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..99af09167331 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -30,7 +30,7 @@ public: > IPAFrameContext &frameContext, > rkisp1_params_cfg *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 3349948a3acf..9b97db7d5c5a 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -208,6 +208,7 @@ void Awb::queueRequest(IPAContext &context, > void Awb::process(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 d76b538288ec..3659b2d5405f 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -27,7 +27,7 @@ public: > IPAFrameContext &frameContext, > const ControlList &controls) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameCtx, > + IPAFrameContext &frameContext, ControlList &metadata, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 3f5c1a58695c..bef5211d5c34 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, metadata, stats); > > setControls(frame); > > - prepareMetadata(frame, aeState); > + metadataReady.emit(frame, metadata); That ties in much better. > } > > 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); Phew, that's good to see it all go. > - > - metadataReady.emit(frame, ctrls); > -} > - > } /* namespace ipa::rkisp1 */ > > /* > -- > Regards, > > Laurent Pinchart >
Hi Laurent On Wed, Oct 19, 2022 at 02:04:32PM +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@ideasonboard.com> Do we want metadata to be passed in as argument, or do we want metadata to be part of the frame context ? Also, a reference is nice, but we usually use pointers for modifiable arguments. Just pointing out for sake of consistency (I'm not even sure we actually enforce that anymore), I'm fine with a reference too.. > --- > src/ipa/ipu3/algorithms/af.cpp | 2 ++ > src/ipa/ipu3/algorithms/af.h | 2 +- > src/ipa/ipu3/algorithms/agc.cpp | 2 ++ > src/ipa/ipu3/algorithms/agc.h | 2 +- > src/ipa/ipu3/algorithms/awb.cpp | 1 + > 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 | 3 ++- > src/ipa/libipa/algorithm.cpp | 5 +++-- > src/ipa/libipa/algorithm.h | 1 + > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > src/ipa/rkisp1/algorithms/agc.h | 2 +- > src/ipa/rkisp1/algorithms/awb.cpp | 1 + > src/ipa/rkisp1/algorithms/awb.h | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 17 +++-------------- > 16 files changed, 26 insertions(+), 24 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 75632aa39d21..2d728871a63b 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -408,6 +408,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > * \param[in] context The IPA context > * \param[in] frame The frame context sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The statistics buffer of IPU3 > * > * Ideally, a clear image also has a relatively higher contrast. So, every > @@ -423,6 +424,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > */ > void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 89d37ac18615..ea7ca647df12 100644 > --- a/src/ipa/ipu3/algorithms/af.h > +++ b/src/ipa/ipu3/algorithms/af.h > @@ -35,7 +35,7 @@ public: > IPAFrameContext &frameContext, > ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > 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 a1a3c38ffe84..44dd5c809e26 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -319,6 +319,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > * \param[in] context The shared IPA context > * \param[in] frame The current frame sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPU3 statistics and ISP results > * > * Identify the current image brightness, and use that to estimate the optimal > @@ -326,6 +327,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > */ > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > const ipu3_uapi_stats_3a *stats) > { > /* > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 59b4b9843c2f..9106cbefe87a 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -29,7 +29,7 @@ public: > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index 0dbd7d4c374f..836eb0687181 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -389,6 +389,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > */ > void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 28e2d38a711c..c38c425c3654 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -43,7 +43,7 @@ public: > IPAFrameContext &frameContext, > ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > 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 eac3d4064443..6fcc03f3fcf7 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -78,6 +78,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > * \param[in] context The shared IPA context > * \param[in] frame The current frame sequence number > * \param[in] frameContext The current frame context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPU3 statistics and ISP results > * > * The tone mapping look up table is generated as an inverse power curve from > @@ -85,6 +86,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > */ > void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > [[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 822e5168df82..ab2a1c000f2f 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > @@ -22,7 +22,7 @@ public: > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, ipu3_uapi_params *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const ipu3_uapi_stats_3a *stats) override; > > private: > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 891643e057b8..0ccc6bf5c8af 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -631,9 +631,10 @@ 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, metadata, stats); > > setControls(frame); > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > index 55abddab2b0a..a34c583d947e 100644 > --- a/src/ipa/libipa/algorithm.cpp > +++ b/src/ipa/libipa/algorithm.cpp > @@ -106,12 +106,13 @@ namespace ipa { > * \param[in] context The shared IPA context > * \param[in] frame The frame context sequence number > * \param[in] frameContext The current frame's context > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > * \param[in] stats The IPA statistics and ISP results > * > * 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..793247be8129 100644 > --- a/src/ipa/libipa/algorithm.h > +++ b/src/ipa/libipa/algorithm.h > @@ -55,6 +55,7 @@ public: > virtual void process([[maybe_unused]] typename Module::Context &context, > [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] typename Module::FrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > [[maybe_unused]] const typename Module::Stats *stats) > { > } > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 04062a364e7f..9de11a2a3bca 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -288,7 +288,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > * new exposure and gain for the scene. > */ > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) > + IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > + const rkisp1_stat_buffer *stats) > { > /* > * \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..99af09167331 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -30,7 +30,7 @@ public: > IPAFrameContext &frameContext, > rkisp1_params_cfg *params) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, ControlList &metadata, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 3349948a3acf..9b97db7d5c5a 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -208,6 +208,7 @@ void Awb::queueRequest(IPAContext &context, > void Awb::process(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > IPAFrameContext &frameContext, > + [[maybe_unused]] ControlList &metadata, > 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 d76b538288ec..3659b2d5405f 100644 > --- a/src/ipa/rkisp1/algorithms/awb.h > +++ b/src/ipa/rkisp1/algorithms/awb.h > @@ -27,7 +27,7 @@ public: > IPAFrameContext &frameContext, > const ControlList &controls) override; > void process(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameCtx, > + IPAFrameContext &frameContext, ControlList &metadata, > const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 3f5c1a58695c..bef5211d5c34 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, metadata, stats); > > 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 >
Hi Kieran, On Wed, Oct 19, 2022 at 12:36:01PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart via libcamera-devel (2022-10-19 12:04:32) > > 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@ideasonboard.com> > > --- > > src/ipa/ipu3/algorithms/af.cpp | 2 ++ > > src/ipa/ipu3/algorithms/af.h | 2 +- > > src/ipa/ipu3/algorithms/agc.cpp | 2 ++ > > src/ipa/ipu3/algorithms/agc.h | 2 +- > > src/ipa/ipu3/algorithms/awb.cpp | 1 + > > 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 | 3 ++- > > src/ipa/libipa/algorithm.cpp | 5 +++-- > > src/ipa/libipa/algorithm.h | 1 + > > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > > src/ipa/rkisp1/algorithms/agc.h | 2 +- > > src/ipa/rkisp1/algorithms/awb.cpp | 1 + > > src/ipa/rkisp1/algorithms/awb.h | 2 +- > > src/ipa/rkisp1/rkisp1.cpp | 17 +++-------------- > > 16 files changed, 26 insertions(+), 24 deletions(-) > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > index 75632aa39d21..2d728871a63b 100644 > > --- a/src/ipa/ipu3/algorithms/af.cpp > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > @@ -408,6 +408,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > > * \param[in] context The IPA context > > * \param[in] frame The frame context sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The statistics buffer of IPU3 > > * > > * Ideally, a clear image also has a relatively higher contrast. So, every > > @@ -423,6 +424,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > > */ > > void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 89d37ac18615..ea7ca647df12 100644 > > --- a/src/ipa/ipu3/algorithms/af.h > > +++ b/src/ipa/ipu3/algorithms/af.h > > @@ -35,7 +35,7 @@ public: > > IPAFrameContext &frameContext, > > ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > 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 a1a3c38ffe84..44dd5c809e26 100644 > > --- a/src/ipa/ipu3/algorithms/agc.cpp > > +++ b/src/ipa/ipu3/algorithms/agc.cpp > > @@ -319,6 +319,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > > * \param[in] context The shared IPA context > > * \param[in] frame The current frame sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPU3 statistics and ISP results > > * > > * Identify the current image brightness, and use that to estimate the optimal > > @@ -326,6 +327,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > > */ > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) > > { > > /* > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > > index 59b4b9843c2f..9106cbefe87a 100644 > > --- a/src/ipa/ipu3/algorithms/agc.h > > +++ b/src/ipa/ipu3/algorithms/agc.h > > @@ -29,7 +29,7 @@ public: > > > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) override; > > > > private: > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > > index 0dbd7d4c374f..836eb0687181 100644 > > --- a/src/ipa/ipu3/algorithms/awb.cpp > > +++ b/src/ipa/ipu3/algorithms/awb.cpp > > @@ -389,6 +389,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > > */ > > void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 28e2d38a711c..c38c425c3654 100644 > > --- a/src/ipa/ipu3/algorithms/awb.h > > +++ b/src/ipa/ipu3/algorithms/awb.h > > @@ -43,7 +43,7 @@ public: > > IPAFrameContext &frameContext, > > ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > 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 eac3d4064443..6fcc03f3fcf7 100644 > > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > > @@ -78,6 +78,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > > * \param[in] context The shared IPA context > > * \param[in] frame The current frame sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPU3 statistics and ISP results > > * > > * The tone mapping look up table is generated as an inverse power curve from > > @@ -85,6 +86,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > > */ > > void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > [[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 822e5168df82..ab2a1c000f2f 100644 > > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > > @@ -22,7 +22,7 @@ public: > > void prepare(IPAContext &context, const uint32_t frame, > > IPAFrameContext &frameContext, ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) override; > > > > private: > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 891643e057b8..0ccc6bf5c8af 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -631,9 +631,10 @@ 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); > > It looks like ctrls() is what is currently used as metadata here. > > It gets emitted at the end of this function. > Can we either use ctrls passed in to algo->process() or (preferred) > rename ctrls to metadata here to ensure that /when/ we set metadata in > the algorithms it will reach the pipeline handler? Good catch. I'll do this in v2. > Probably keep the existing metadata additions in this function still - > converting them can be later, but otherwise this is a partial > implementation for IPU3. I'll add patches on top of the series to do so. > *with that* > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + ControlList metadata(controls::controls); > > > > for (auto const &algo : algorithms()) > > - algo->process(context_, frame, frameContext, stats); > > + algo->process(context_, frame, frameContext, metadata, stats); > > I would have had metadata as the last parameter as an 'output' > (inputs, outputs) ... but I bet you've gone alphabetical. > > No point changing it though - that's too much work, and this is fine. Too late, I've moved it already :-) I'm also toying with the idea of adding the metadata to the frame context, but that won't be done yet. > > setControls(frame); > > > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > > index 55abddab2b0a..a34c583d947e 100644 > > --- a/src/ipa/libipa/algorithm.cpp > > +++ b/src/ipa/libipa/algorithm.cpp > > @@ -106,12 +106,13 @@ namespace ipa { > > * \param[in] context The shared IPA context > > * \param[in] frame The frame context sequence number > > * \param[in] frameContext The current frame's context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPA statistics and ISP results > > * > > * 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..793247be8129 100644 > > --- a/src/ipa/libipa/algorithm.h > > +++ b/src/ipa/libipa/algorithm.h > > @@ -55,6 +55,7 @@ public: > > virtual void process([[maybe_unused]] typename Module::Context &context, > > [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] typename Module::FrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > [[maybe_unused]] const typename Module::Stats *stats) > > { > > } > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > index 04062a364e7f..9de11a2a3bca 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > @@ -288,7 +288,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > > * new exposure and gain for the scene. > > */ > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) > > + IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > + const rkisp1_stat_buffer *stats) > > { > > /* > > * \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..99af09167331 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.h > > +++ b/src/ipa/rkisp1/algorithms/agc.h > > @@ -30,7 +30,7 @@ public: > > IPAFrameContext &frameContext, > > rkisp1_params_cfg *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const rkisp1_stat_buffer *stats) override; > > > > private: > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > > index 3349948a3acf..9b97db7d5c5a 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.cpp > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > > @@ -208,6 +208,7 @@ void Awb::queueRequest(IPAContext &context, > > void Awb::process(IPAContext &context, > > [[maybe_unused]] const uint32_t frame, > > IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 d76b538288ec..3659b2d5405f 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.h > > +++ b/src/ipa/rkisp1/algorithms/awb.h > > @@ -27,7 +27,7 @@ public: > > IPAFrameContext &frameContext, > > const ControlList &controls) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameCtx, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const rkisp1_stat_buffer *stats) override; > > > > private: > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index 3f5c1a58695c..bef5211d5c34 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, metadata, stats); > > > > setControls(frame); > > > > - prepareMetadata(frame, aeState); > > + metadataReady.emit(frame, metadata); > > That ties in much better. > > > } > > > > 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); > > Phew, that's good to see it all go. > > > - > > - metadataReady.emit(frame, ctrls); > > -} > > - > > } /* namespace ipa::rkisp1 */ > > > > /*
Hi Jacopo, On Wed, Oct 19, 2022 at 01:49:12PM +0200, Jacopo Mondi wrote: > On Wed, Oct 19, 2022 at 02:04:32PM +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@ideasonboard.com> > > Do we want metadata to be passed in as argument, or do we want > metadata to be part of the frame context ? I've considered both, and haven't been able to decide which is best. Moving the metadata to the frame context is more complicated, as the FrameContext won't be default-constructible anymore. I started going in that direction, but it will require quite a lot of extra work. > Also, a reference is nice, but we usually use pointers for modifiable > arguments. Just pointing out for sake of consistency (I'm not even > sure we actually enforce that anymore), I'm fine with a reference > too.. The context and frame context are already passed by reference, so I've done the same. We could address all of those globally. > > --- > > src/ipa/ipu3/algorithms/af.cpp | 2 ++ > > src/ipa/ipu3/algorithms/af.h | 2 +- > > src/ipa/ipu3/algorithms/agc.cpp | 2 ++ > > src/ipa/ipu3/algorithms/agc.h | 2 +- > > src/ipa/ipu3/algorithms/awb.cpp | 1 + > > 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 | 3 ++- > > src/ipa/libipa/algorithm.cpp | 5 +++-- > > src/ipa/libipa/algorithm.h | 1 + > > src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- > > src/ipa/rkisp1/algorithms/agc.h | 2 +- > > src/ipa/rkisp1/algorithms/awb.cpp | 1 + > > src/ipa/rkisp1/algorithms/awb.h | 2 +- > > src/ipa/rkisp1/rkisp1.cpp | 17 +++-------------- > > 16 files changed, 26 insertions(+), 24 deletions(-) > > > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > > index 75632aa39d21..2d728871a63b 100644 > > --- a/src/ipa/ipu3/algorithms/af.cpp > > +++ b/src/ipa/ipu3/algorithms/af.cpp > > @@ -408,6 +408,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > > * \param[in] context The IPA context > > * \param[in] frame The frame context sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The statistics buffer of IPU3 > > * > > * Ideally, a clear image also has a relatively higher contrast. So, every > > @@ -423,6 +424,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) > > */ > > void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 89d37ac18615..ea7ca647df12 100644 > > --- a/src/ipa/ipu3/algorithms/af.h > > +++ b/src/ipa/ipu3/algorithms/af.h > > @@ -35,7 +35,7 @@ public: > > IPAFrameContext &frameContext, > > ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > 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 a1a3c38ffe84..44dd5c809e26 100644 > > --- a/src/ipa/ipu3/algorithms/agc.cpp > > +++ b/src/ipa/ipu3/algorithms/agc.cpp > > @@ -319,6 +319,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > > * \param[in] context The shared IPA context > > * \param[in] frame The current frame sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPU3 statistics and ISP results > > * > > * Identify the current image brightness, and use that to estimate the optimal > > @@ -326,6 +327,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > > */ > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) > > { > > /* > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > > index 59b4b9843c2f..9106cbefe87a 100644 > > --- a/src/ipa/ipu3/algorithms/agc.h > > +++ b/src/ipa/ipu3/algorithms/agc.h > > @@ -29,7 +29,7 @@ public: > > > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) override; > > > > private: > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > > index 0dbd7d4c374f..836eb0687181 100644 > > --- a/src/ipa/ipu3/algorithms/awb.cpp > > +++ b/src/ipa/ipu3/algorithms/awb.cpp > > @@ -389,6 +389,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) > > */ > > void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 28e2d38a711c..c38c425c3654 100644 > > --- a/src/ipa/ipu3/algorithms/awb.h > > +++ b/src/ipa/ipu3/algorithms/awb.h > > @@ -43,7 +43,7 @@ public: > > IPAFrameContext &frameContext, > > ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > 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 eac3d4064443..6fcc03f3fcf7 100644 > > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > > @@ -78,6 +78,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > > * \param[in] context The shared IPA context > > * \param[in] frame The current frame sequence number > > * \param[in] frameContext The current frame context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPU3 statistics and ISP results > > * > > * The tone mapping look up table is generated as an inverse power curve from > > @@ -85,6 +86,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, > > */ > > void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > [[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 822e5168df82..ab2a1c000f2f 100644 > > --- a/src/ipa/ipu3/algorithms/tone_mapping.h > > +++ b/src/ipa/ipu3/algorithms/tone_mapping.h > > @@ -22,7 +22,7 @@ public: > > void prepare(IPAContext &context, const uint32_t frame, > > IPAFrameContext &frameContext, ipu3_uapi_params *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const ipu3_uapi_stats_3a *stats) override; > > > > private: > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 891643e057b8..0ccc6bf5c8af 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -631,9 +631,10 @@ 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, metadata, stats); > > > > setControls(frame); > > > > diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp > > index 55abddab2b0a..a34c583d947e 100644 > > --- a/src/ipa/libipa/algorithm.cpp > > +++ b/src/ipa/libipa/algorithm.cpp > > @@ -106,12 +106,13 @@ namespace ipa { > > * \param[in] context The shared IPA context > > * \param[in] frame The frame context sequence number > > * \param[in] frameContext The current frame's context > > + * \param[out] metadata Metadata for the frame, to be filled by the algorithm > > * \param[in] stats The IPA statistics and ISP results > > * > > * 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..793247be8129 100644 > > --- a/src/ipa/libipa/algorithm.h > > +++ b/src/ipa/libipa/algorithm.h > > @@ -55,6 +55,7 @@ public: > > virtual void process([[maybe_unused]] typename Module::Context &context, > > [[maybe_unused]] const uint32_t frame, > > [[maybe_unused]] typename Module::FrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > [[maybe_unused]] const typename Module::Stats *stats) > > { > > } > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > index 04062a364e7f..9de11a2a3bca 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > @@ -288,7 +288,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const > > * new exposure and gain for the scene. > > */ > > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) > > + IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > + const rkisp1_stat_buffer *stats) > > { > > /* > > * \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..99af09167331 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.h > > +++ b/src/ipa/rkisp1/algorithms/agc.h > > @@ -30,7 +30,7 @@ public: > > IPAFrameContext &frameContext, > > rkisp1_params_cfg *params) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const rkisp1_stat_buffer *stats) override; > > > > private: > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > > index 3349948a3acf..9b97db7d5c5a 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.cpp > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > > @@ -208,6 +208,7 @@ void Awb::queueRequest(IPAContext &context, > > void Awb::process(IPAContext &context, > > [[maybe_unused]] const uint32_t frame, > > IPAFrameContext &frameContext, > > + [[maybe_unused]] ControlList &metadata, > > 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 d76b538288ec..3659b2d5405f 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.h > > +++ b/src/ipa/rkisp1/algorithms/awb.h > > @@ -27,7 +27,7 @@ public: > > IPAFrameContext &frameContext, > > const ControlList &controls) override; > > void process(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameCtx, > > + IPAFrameContext &frameContext, ControlList &metadata, > > const rkisp1_stat_buffer *stats) override; > > > > private: > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index 3f5c1a58695c..bef5211d5c34 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, metadata, stats); > > > > 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 */ > > > > /*
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index 75632aa39d21..2d728871a63b 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -408,6 +408,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) * \param[in] context The IPA context * \param[in] frame The frame context sequence number * \param[in] frameContext The current frame context + * \param[out] metadata Metadata for the frame, to be filled by the algorithm * \param[in] stats The statistics buffer of IPU3 * * Ideally, a clear image also has a relatively higher contrast. So, every @@ -423,6 +424,7 @@ bool Af::afIsOutOfFocus(IPAContext &context) */ void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, 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 89d37ac18615..ea7ca647df12 100644 --- a/src/ipa/ipu3/algorithms/af.h +++ b/src/ipa/ipu3/algorithms/af.h @@ -35,7 +35,7 @@ public: IPAFrameContext &frameContext, ipu3_uapi_params *params) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, + IPAFrameContext &frameContext, ControlList &metadata, 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 a1a3c38ffe84..44dd5c809e26 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -319,6 +319,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, * \param[in] context The shared IPA context * \param[in] frame The current frame sequence number * \param[in] frameContext The current frame context + * \param[out] metadata Metadata for the frame, to be filled by the algorithm * \param[in] stats The IPU3 statistics and ISP results * * Identify the current image brightness, and use that to estimate the optimal @@ -326,6 +327,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState, */ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, const ipu3_uapi_stats_3a *stats) { /* diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 59b4b9843c2f..9106cbefe87a 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -29,7 +29,7 @@ public: int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, + IPAFrameContext &frameContext, ControlList &metadata, const ipu3_uapi_stats_3a *stats) override; private: diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index 0dbd7d4c374f..836eb0687181 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -389,6 +389,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats) */ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, 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 28e2d38a711c..c38c425c3654 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -43,7 +43,7 @@ public: IPAFrameContext &frameContext, ipu3_uapi_params *params) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, + IPAFrameContext &frameContext, ControlList &metadata, 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 eac3d4064443..6fcc03f3fcf7 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp @@ -78,6 +78,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, * \param[in] context The shared IPA context * \param[in] frame The current frame sequence number * \param[in] frameContext The current frame context + * \param[out] metadata Metadata for the frame, to be filled by the algorithm * \param[in] stats The IPU3 statistics and ISP results * * The tone mapping look up table is generated as an inverse power curve from @@ -85,6 +86,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context, */ void ToneMapping::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, [[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 822e5168df82..ab2a1c000f2f 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.h +++ b/src/ipa/ipu3/algorithms/tone_mapping.h @@ -22,7 +22,7 @@ public: void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, ipu3_uapi_params *params) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, + IPAFrameContext &frameContext, ControlList &metadata, const ipu3_uapi_stats_3a *stats) override; private: diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 891643e057b8..0ccc6bf5c8af 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -631,9 +631,10 @@ 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, metadata, stats); setControls(frame); diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp index 55abddab2b0a..a34c583d947e 100644 --- a/src/ipa/libipa/algorithm.cpp +++ b/src/ipa/libipa/algorithm.cpp @@ -106,12 +106,13 @@ namespace ipa { * \param[in] context The shared IPA context * \param[in] frame The frame context sequence number * \param[in] frameContext The current frame's context + * \param[out] metadata Metadata for the frame, to be filled by the algorithm * \param[in] stats The IPA statistics and ISP results * * 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..793247be8129 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -55,6 +55,7 @@ public: virtual void process([[maybe_unused]] typename Module::Context &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] typename Module::FrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, [[maybe_unused]] const typename Module::Stats *stats) { } diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 04062a364e7f..9de11a2a3bca 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -288,7 +288,9 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const * new exposure and gain for the scene. */ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, - IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) + IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, + const rkisp1_stat_buffer *stats) { /* * \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..99af09167331 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -30,7 +30,7 @@ public: IPAFrameContext &frameContext, rkisp1_params_cfg *params) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, + IPAFrameContext &frameContext, ControlList &metadata, const rkisp1_stat_buffer *stats) override; private: diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 3349948a3acf..9b97db7d5c5a 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -208,6 +208,7 @@ void Awb::queueRequest(IPAContext &context, void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, IPAFrameContext &frameContext, + [[maybe_unused]] ControlList &metadata, 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 d76b538288ec..3659b2d5405f 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -27,7 +27,7 @@ public: IPAFrameContext &frameContext, const ControlList &controls) override; void process(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameCtx, + IPAFrameContext &frameContext, ControlList &metadata, const rkisp1_stat_buffer *stats) override; private: diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 3f5c1a58695c..bef5211d5c34 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, metadata, stats); 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 */ /*
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@ideasonboard.com> --- src/ipa/ipu3/algorithms/af.cpp | 2 ++ src/ipa/ipu3/algorithms/af.h | 2 +- src/ipa/ipu3/algorithms/agc.cpp | 2 ++ src/ipa/ipu3/algorithms/agc.h | 2 +- src/ipa/ipu3/algorithms/awb.cpp | 1 + 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 | 3 ++- src/ipa/libipa/algorithm.cpp | 5 +++-- src/ipa/libipa/algorithm.h | 1 + src/ipa/rkisp1/algorithms/agc.cpp | 4 +++- src/ipa/rkisp1/algorithms/agc.h | 2 +- src/ipa/rkisp1/algorithms/awb.cpp | 1 + src/ipa/rkisp1/algorithms/awb.h | 2 +- src/ipa/rkisp1/rkisp1.cpp | 17 +++-------------- 16 files changed, 26 insertions(+), 24 deletions(-)