[libcamera-devel] [PATCH v2 2/6] ipu3: Add a class AiqResultsRingBuffer to reserve AiqResults history

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Nov 2 14:37:19 CET 2021


Quoting Han-Lin Chen (2021-10-29 12:59:57)
> The AIQ algorithm expects the statstistics comes with the effective AiqResults
> 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.

Excellent, I'm pleased to see this development.

This patch does a few things which should be explictily mentioned (or
broken in to independent patches)

 - Make the parameters to setXXX() functions const.
 - Implement copy constructors
 - Implement a RingBuffer to maintain AiqResults History

> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>  aiq/aiq_results.cpp | 85 ++++++++++++++++++++++++++++++++++++++++-----
>  aiq/aiq_results.h   | 38 +++++++++++++++-----
>  2 files changed, 107 insertions(+), 16 deletions(-)
> 
> diff --git a/aiq/aiq_results.cpp b/aiq/aiq_results.cpp
> index f727f36..deda4be 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
>          *   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,48 @@ 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::Push(const AiqResults& result)
> +{
> +       operator[](end_) = result;
> +       end_ = (end_ + 1) % bufferSize;
> +       size_ = std::min(size_ + 1, bufferSize);
> +       if (size_ == bufferSize) {
> +               start_ = end_;
> +       }
> +}
> +
> +AiqResults& AiqResultsRingBuffer::searchBackward(
> +               const std::function<bool(AiqResults&)> pred, AiqResults& def)
> +{
> +       if (size_ == 0)
> +               return def;
> +
> +       unsigned int i = (end_ - 1) % bufferSize;
> +       while (i != start_) {
> +               if (pred(operator[](i))) {
> +                       return operator[](i);
> +               }
> +               i = --i % bufferSize;
> +       }
> +
> +       return def;
> +}
> +
>  } /* namespace ipa::ipu3::aiq */
>  
>  } /* namespace libcamera */
> diff --git a/aiq/aiq_results.h b/aiq/aiq_results.h
> index ae19a6c..7005a47 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,8 @@ class AiqResults
>  {
>  public:
>         AiqResults();
> +       AiqResults(const AiqResults &other);
> +       AiqResults &operator=(const AiqResults &other);

Should the move constructor/assingment operators be deleted or
implemented?


>         const ia_aiq_ae_results *ae() { return &ae_; }
>         ia_aiq_af_results *af() { return &af_; }
> @@ -46,14 +50,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 +113,24 @@ 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 Push(const AiqResults& result);
> +       unsigned int size() { return size_; }
> +       AiqResults& searchBackward(const std::function<bool(AiqResults&)> pred,
> +                                                                                                                AiqResults& def);
> +
> +private:
> +       unsigned int start_ = 0;
> +       unsigned int end_ = 0;
> +       unsigned int size_ = 0;
> +};
> +
>  } /* namespace libcamera::ipa::ipu3::aiq */
>  
>  #endif /* IPA_IPU3_AIQ_RESULTS_H */
> -- 
> 2.33.1.1089.g2158813163f-goog
>


More information about the libcamera-devel mailing list