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

14419 diff mbox series

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.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
---
 aiq/aiq_results.cpp | 85 ++++++++++++++++++++++++++++++++++++++++-----
 aiq/aiq_results.h   | 38 +++++++++++++++-----
 2 files changed, 107 insertions(+), 16 deletions(-)

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@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
>

Hi Kieran,
I'm sorry for the late reply.

On Tue, Nov 2, 2021 at 9:37 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> 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
Sounds great to me.
>
> > Signed-off-by: Han-Lin Chen <hanlinchen@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?
I think AiqResult doesn't have a "move" semantics ;-).
The move constructor/assignment would be suppressed automatically when
the copy constructor is defined, but it's good to make it more
explicit :/
>
>
> >         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
> >

Quoting Hanlin Chen (2021-11-10 13:02:17)
> Hi Kieran,
> I'm sorry for the late reply.

Don't worry - it's all asynchronous ;-)

> 
> On Tue, Nov 2, 2021 at 9:37 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
> >
> > 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
> Sounds great to me.
> >
> > > Signed-off-by: Han-Lin Chen <hanlinchen@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?
> I think AiqResult doesn't have a "move" semantics ;-).
> The move constructor/assignment would be suppressed automatically when
> the copy constructor is defined, but it's good to make it more
> explicit :/

Ah yes I recall there are implicit rules that delete things. I can never
remember them ;-) All I have in my head is the rule of 3/5/x where if
you define one you should define or delete all of them ...



> >
> >
> > >         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
> > >