[libcamera-devel] [IPU3-IPA PATCH v3 2/6] ipu3: Add a class AiqResultsRingBuffer to reserve AiqResults history
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Nov 17 18:41:17 CET 2021
Quoting Han-Lin Chen (2021-11-11 10:49:04)
> The AIQ algorithm expects the statstistics comes with the effective AiqResults
s/statstistics/statistics/
Could be fixed while applying if there's nothing else.
> applied on the sensor, which may not always be the latest AiqResults,
> since pipeline handler may delay setting the controls based on SOF.
> Add a class to reserve the history of the AiqResults generated for previous
> frames, so IPA can have a chance to look for the suitable one backwards.
>
> In details, the patch adds following to AiqResult Class.
> - Make the parameters to setXXX() functions const.
> - Implement copy constructors
> - Implement a RingBuffer to maintain AiqResults History
I wish those were separate patches, but we can keep it as is.
Only a question below, but I think I can say:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
> aiq/aiq_results.cpp | 90 +++++++++++++++++++++++++++++++++++++++++----
> aiq/aiq_results.h | 43 ++++++++++++++++++----
> 2 files changed, 117 insertions(+), 16 deletions(-)
>
> diff --git a/aiq/aiq_results.cpp b/aiq/aiq_results.cpp
> index f727f36..befcf7a 100644
> --- a/aiq/aiq_results.cpp
> +++ b/aiq/aiq_results.cpp
> @@ -63,7 +63,34 @@ AiqResults::AiqResults()
> sa_.channel_b = channelB_.data();
> }
>
> -void AiqResults::setAe(ia_aiq_ae_results *ae)
> +AiqResults::AiqResults(const AiqResults &other)
> + :AiqResults()
> +{
> + setAe(&other.ae_);
> + setAf(&other.af_);
> + setAfBracket(&other.afBracket_);
> + setAwb(&other.awb_);
> + setGbce(&other.gbce_);
> + setDetectedSceneMode(other.detectedSceneMode_);
> + setPa(&other.pa_);
> + setSa(&other.sa_);
> +}
> +
> +AiqResults& AiqResults::operator=(const AiqResults &other)
> +{
> + setAe(&other.ae_);
> + setAf(&other.af_);
> + setAfBracket(&other.afBracket_);
> + setAwb(&other.awb_);
> + setGbce(&other.gbce_);
> + setDetectedSceneMode(other.detectedSceneMode_);
> + setPa(&other.pa_);
> + setSa(&other.sa_);
> +
> + return *this;
> +}
> +
> +void AiqResults::setAe(const ia_aiq_ae_results *ae)
> {
> /* Todo: Potentially Requires copying
Have you checked through the set*'s to make sure they can be copied
successfully? I know this was an area that found lots of magic pointers
that were somewhere inside the AIQ library space...
> * ia_aiq_aperture_control *aperture_control;
> @@ -121,7 +148,7 @@ void AiqResults::setAe(ia_aiq_ae_results *ae)
> }
> }
>
> -void AiqResults::setAf(ia_aiq_af_results *af)
> +void AiqResults::setAf(const ia_aiq_af_results *af)
> {
> ASSERT(af);
>
> @@ -133,7 +160,7 @@ void AiqResults::setAf(ia_aiq_af_results *af)
> af_.final_lens_position_reached = af->final_lens_position_reached;
> }
>
> -void AiqResults::setAfBracket(ia_aiq_af_bracket_results *afBracket)
> +void AiqResults::setAfBracket(const ia_aiq_af_bracket_results *afBracket)
> {
> ASSERT(afBracket);
>
> @@ -145,7 +172,7 @@ void AiqResults::setAfBracket(ia_aiq_af_bracket_results *afBracket)
> afBracket_.lens_positions_bracketing = afBracket->lens_positions_bracketing;
> }
>
> -void AiqResults::setAwb(ia_aiq_awb_results *awb)
> +void AiqResults::setAwb(const ia_aiq_awb_results *awb)
> {
> ASSERT(awb);
>
> @@ -157,7 +184,7 @@ void AiqResults::setAwb(ia_aiq_awb_results *awb)
> awb_.distance_from_convergence = awb->distance_from_convergence;
> }
>
> -void AiqResults::setGbce(ia_aiq_gbce_results *gbce)
> +void AiqResults::setGbce(const ia_aiq_gbce_results *gbce)
> {
> ASSERT(gbce);
>
> @@ -181,12 +208,12 @@ void AiqResults::setGbce(ia_aiq_gbce_results *gbce)
> }
> }
>
> -void AiqResults::setDetectedSceneMode(ia_aiq_scene_mode dsm)
> +void AiqResults::setDetectedSceneMode(const ia_aiq_scene_mode dsm)
> {
> detectedSceneMode_ = dsm;
> }
>
> -void AiqResults::setPa(ia_aiq_pa_results *pa)
> +void AiqResults::setPa(const ia_aiq_pa_results *pa)
> {
> ASSERT(pa);
>
> @@ -234,7 +261,7 @@ void AiqResults::setPa(ia_aiq_pa_results *pa)
> pa_.brightness_level = pa->brightness_level;
> }
>
> -void AiqResults::setSa(ia_aiq_sa_results *sa)
> +void AiqResults::setSa(const ia_aiq_sa_results *sa)
> {
> ASSERT(sa && sa->channel_r && sa->channel_gr &&
> sa->channel_gb && sa->channel_b);
> @@ -275,6 +302,53 @@ void AiqResults::setSa(ia_aiq_sa_results *sa)
> sa_.frame_params = sa->frame_params;
> }
>
> +AiqResults& AiqResultsRingBuffer::operator[](unsigned int index)
> +{
> + return std::array<AiqResults, bufferSize>::operator[](index % bufferSize);
> +}
> +
> +const AiqResults& AiqResultsRingBuffer::operator[](unsigned int index) const
> +{
> + return std::array<AiqResults, bufferSize>::operator[](index % bufferSize);
> +}
> +
> +void AiqResultsRingBuffer::reset()
> +{
> + start_ = end_ = size_ = 0;
> +}
> +
> +void AiqResultsRingBuffer::extendOne()
> +{
> + end_ = (end_ + 1) % bufferSize;
> + size_ = std::min(size_ + 1, bufferSize);
> + if (size_ == bufferSize) {
> + start_ = end_;
> + }
> +}
> +
> +AiqResults& AiqResultsRingBuffer::latest()
> +{
> + unsigned int i = (end_ + bufferSize - 1) % bufferSize;
> + return operator[](i);
> +}
> +
> +AiqResults& AiqResultsRingBuffer::searchBackward(
> + const std::function<bool(AiqResults&)> pred, AiqResults& def)
> +{
> + if (size_ == 0)
> + return def;
> +
> + unsigned int i = (end_ + bufferSize - 1) % bufferSize;
> + while (i != start_) {
> + if (pred(operator[](i))) {
> + return operator[](i);
> + }
> + i = (i + bufferSize - 1) % bufferSize;
> + }
> +
> + return def;
> +}
> +
> } /* namespace ipa::ipu3::aiq */
>
> } /* namespace libcamera */
> diff --git a/aiq/aiq_results.h b/aiq/aiq_results.h
> index ae19a6c..8c95852 100644
> --- a/aiq/aiq_results.h
> +++ b/aiq/aiq_results.h
> @@ -8,6 +8,8 @@
> * of the aiq result structures.
> */
>
> +#include <array>
> +#include <functional>
> #include <vector>
>
> #include <ia_imaging/ia_aiq.h>
> @@ -37,6 +39,10 @@ class AiqResults
> {
> public:
> AiqResults();
> + AiqResults(const AiqResults &other);
> + AiqResults &operator=(const AiqResults &other);
> + AiqResults(const AiqResults &&other) = delete;
> + AiqResults &operator=(const AiqResults &&other) = delete;
>
> const ia_aiq_ae_results *ae() { return &ae_; }
> ia_aiq_af_results *af() { return &af_; }
> @@ -46,14 +52,14 @@ public:
> const ia_aiq_pa_results *pa() { return &pa_; }
> const ia_aiq_sa_results *sa() { return &sa_; }
>
> - void setAe(ia_aiq_ae_results *ae);
> - void setAf(ia_aiq_af_results *af);
> - void setAfBracket(ia_aiq_af_bracket_results *afBracket);
> - void setAwb(ia_aiq_awb_results *awb);
> - void setGbce(ia_aiq_gbce_results *gbce);
> - void setDetectedSceneMode(ia_aiq_scene_mode dsm);
> - void setPa(ia_aiq_pa_results *pa);
> - void setSa(ia_aiq_sa_results *sa);
> + void setAe(const ia_aiq_ae_results *ae);
> + void setAf(const ia_aiq_af_results *af);
> + void setAfBracket(const ia_aiq_af_bracket_results *afBracket);
> + void setAwb(const ia_aiq_awb_results *awb);
> + void setGbce(const ia_aiq_gbce_results *gbce);
> + void setDetectedSceneMode(const ia_aiq_scene_mode dsm);
> + void setPa(const ia_aiq_pa_results *pa);
> + void setSa(const ia_aiq_sa_results *sa);
>
> private:
> ia_aiq_ae_results ae_;
> @@ -109,6 +115,27 @@ private:
> std::vector<float> channelGb_;
> };
>
> +static constexpr unsigned int bufferSize = 16;
> +class AiqResultsRingBuffer: public std::array<AiqResults, bufferSize>
> +{
> +public:
> + AiqResults &operator[](unsigned int index);
> + const AiqResults &operator[](unsigned int index) const;
> + void reset();
> + void extendOne();
> + AiqResults& latest();
> + unsigned int size() { return size_; }
> + AiqResults& searchBackward(const std::function<bool(AiqResults&)> pred,
> + AiqResults& def);
> +
> +private:
> + void incrementSize();
> +
> + unsigned int start_ = 0;
> + unsigned int end_ = 0;
> + unsigned int size_ = 0;
> +};
> +
> } /* namespace libcamera::ipa::ipu3::aiq */
>
> #endif /* IPA_IPU3_AIQ_RESULTS_H */
> --
> 2.34.0.rc1.387.gb447b232ab-goog
>
More information about the libcamera-devel
mailing list