[libcamera-devel] [PATCH 3/3] ipa: ipu3: Add a IPAFrameContext queue

Umang Jain umang.jain at ideasonboard.com
Mon Apr 25 12:31:00 CEST 2022


Hello,

On 4/25/22 14:38, Umang Jain via libcamera-devel wrote:
> Hi Kieran,
>
> On 4/21/22 16:11, Kieran Bingham wrote:
>> Hi Umang,
>>
>> Quoting Umang Jain via libcamera-devel (2022-03-10 20:51:30)
>>> Having a single IPAFrameContext queue is limiting especially when
>>> we need to preserve per-frame controls coming in through
>>> libcamera::Request. Right now, we are not processing any controls
>>> on the IPA side (processControls()) but sooner or later we need to
>>> preserve the controls setting for the frames in the context in a
>>> retrievable fashion. Hence a std::deque is introduced to preserve
>>> the frame context of the incoming request's settings as soon as it is
>>> queued.
>>>
>>> Since IPAIPU3::processControls() is executed on
>>> IPU3CameraData::queuePendingRequests() code path, we need to store the
>>> incoming control setting in a separate IPAFrameContext and push that
>>> into the queue. The IPAFrameContext is then dropped when processing for
>>> that frame has been finished.
>> Great, having an identifiable (by frame number) context is going to make
>> things a lot easier I think.
>>
>>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>>> ---
>>>   src/ipa/ipu3/algorithms/agc.cpp          | 16 ++++-----
>>>   src/ipa/ipu3/algorithms/agc.h            |  4 +--
>>>   src/ipa/ipu3/algorithms/awb.cpp          | 17 +++++-----
>>>   src/ipa/ipu3/algorithms/tone_mapping.cpp | 14 ++++----
>>>   src/ipa/ipu3/ipa_context.cpp             | 42 
>>> ++++++++++++++++++++++++
>>>   src/ipa/ipu3/ipa_context.h               | 12 +++++++
>>>   src/ipa/ipu3/ipu3.cpp                    | 41 +++++++++++++++++++----
>>>   7 files changed, 115 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp 
>>> b/src/ipa/ipu3/algorithms/agc.cpp
>>> index cdc1327a..b24744f2 100644
>>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>>> @@ -87,7 +87,7 @@ int Agc::configure(IPAContext &context,
>>>                     [[maybe_unused]] const IPAConfigInfo &configInfo)
>>>   {
>>>          IPASessionConfiguration &configuration = 
>>> context.configuration;
>>> -       IPAFrameContext &frameContext = context.frameContext;
>>> +       IPAFrameContext &frameContext = context.getFrameContext(0);
>> I 'almost' want to call this context.frameContext(0); as an implicit
>> getter. But the 'get' does make it explicit.
>
>
> Yeah,  I like being explicit as in - get frame context of <frame number>
>
>>>            stride_ = configuration.grid.stride;
>>>   @@ -182,14 +182,14 @@ utils::Duration 
>>> Agc::filterExposure(utils::Duration exposureValue)
>>>    * \param[in] yGain The gain calculated based on the relative 
>>> luminance target
>>>    * \param[in] iqMeanGain The gain calculated based on the relative 
>>> luminance target
>>>    */
>>> -void Agc::computeExposure(IPAContext &context, double yGain,
>>> -                         double iqMeanGain)
>>> +void Agc::computeExposure(const uint32_t frame, IPAContext &context,
>>> +                         double yGain, double iqMeanGain)
>>>   {
>>>          const IPASessionConfiguration &configuration = 
>>> context.configuration;
>>> -       IPAFrameContext &frameContext = context.frameContext;
>>>          /* Get the effective exposure and gain applied on the 
>>> sensor. */
>>> -       uint32_t exposure = frameContext.sensor.exposure;
>>> -       double analogueGain = frameContext.sensor.gain;
>>> +       uint32_t exposure = context.prevFrameContext.sensor.exposure;
>>> +       double analogueGain = context.prevFrameContext.sensor.gain;
>>> +       IPAFrameContext &frameContext = context.getFrameContext(frame);
>> I would have almost expected to see
>>       IPAFrameContext &previous = context.getFrameContext(frame - 1);
>
>
> Why?
>
> As far as I understand, the frameContext here is the current frame's 
> context and we want to compute the exposure of current...
>
>>
>> But we might have occasions where that wasn't processed or not
>> available, so I guess having the most recent parameters referenced
>> directly probably makes sense, and also reduces having to look them up
>> each algorithm...
>>
>> That makes me think a name such as 'mostRecent' might be more
>> informative? I'm not sure... (There are many previous frames, but only
>> one mostRecent...?)
>>
>>
>> I was also going to argue/wonder why it wasn't a pointer to the existing
>> queue entry - but I suspect we might be freeing it when the frame is
>> completed, so a copy or move is likely easiest ... maybe that's an
>> optimisation later, and depends on how big it is if we are copying the
>> data.
>
>
> Yeah, optimization. I, for now, work with frame context's reference 
> throughout.
>
>>
>>>            /* Use the highest of the two gain estimates. */
>>>          double evGain = std::max(yGain, iqMeanGain);
>>> @@ -344,7 +344,7 @@ void Agc::process(const uint32_t frame, 
>>> IPAContext &context, const ipu3_uapi_sta
>>>          double yTarget = kRelativeLuminanceTarget;
>>>            for (unsigned int i = 0; i < 8; i++) {
>>> -               double yValue = estimateLuminance(context.frameContext,
>>> +               double yValue = 
>>> estimateLuminance(context.prevFrameContext,
>>> context.configuration.grid.bdsGrid,
>>>                                                    stats, yGain);
>>>                  double extraGain = std::min(10.0, yTarget / (yValue 
>>> + .001));
>>> @@ -357,7 +357,7 @@ void Agc::process(const uint32_t frame, 
>>> IPAContext &context, const ipu3_uapi_sta
>>>                          break;
>>>          }
>>>   -       computeExposure(context, yGain, iqMeanGain);
>>> +       computeExposure(frame, context, yGain, iqMeanGain);
>>>          frameCount_++;
>>>   }
>>>   diff --git a/src/ipa/ipu3/algorithms/agc.h 
>>> b/src/ipa/ipu3/algorithms/agc.h
>>> index e48506e5..f35de05f 100644
>>> --- a/src/ipa/ipu3/algorithms/agc.h
>>> +++ b/src/ipa/ipu3/algorithms/agc.h
>>> @@ -34,8 +34,8 @@ private:
>>>          double measureBrightness(const ipu3_uapi_stats_3a *stats,
>>>                                   const ipu3_uapi_grid_config &grid) 
>>> const;
>>>          utils::Duration filterExposure(utils::Duration 
>>> currentExposure);
>>> -       void computeExposure(IPAContext &context, double yGain,
>>> -                            double iqMeanGain);
>>> +       void computeExposure(const uint32_t frame, IPAContext &context,
>>> +                            double yGain, double iqMeanGain);
>>>          double estimateLuminance(IPAFrameContext &frameContext,
>>>                                   const ipu3_uapi_grid_config &grid,
>>>                                   const ipu3_uapi_stats_3a *stats,
>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp 
>>> b/src/ipa/ipu3/algorithms/awb.cpp
>>> index f1e753e6..00ac2984 100644
>>> --- a/src/ipa/ipu3/algorithms/awb.cpp
>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
>>> @@ -390,16 +390,17 @@ void Awb::calculateWBGains(const 
>>> ipu3_uapi_stats_3a *stats)
>>>   void Awb::process(const uint32_t frame, IPAContext &context, const 
>>> ipu3_uapi_stats_3a *stats)
>>>   {
>>>          calculateWBGains(stats);
>>> +       IPAFrameContext &frameContext = context.getFrameContext(frame);
>> This code's reason enough here to keep the explicit 'get' in
>> getFrameContext for me I think.
>>
>>>          /*
>>>           * Gains are only recalculated if enough zones were detected.
>>>           * The results are cached, so if no results were 
>>> calculated, we set the
>>>           * cached values from asyncResults_ here.
>>>           */
>>> -       context.frameContext.awb.gains.blue = asyncResults_.blueGain;
>>> -       context.frameContext.awb.gains.green = asyncResults_.greenGain;
>>> -       context.frameContext.awb.gains.red = asyncResults_.redGain;
>>> -       context.frameContext.awb.temperatureK = 
>>> asyncResults_.temperatureK;
>>> +       frameContext.awb.gains.blue = asyncResults_.blueGain;
>>> +       frameContext.awb.gains.green = asyncResults_.greenGain;
>>> +       frameContext.awb.gains.red = asyncResults_.redGain;
>>> +       frameContext.awb.temperatureK = asyncResults_.temperatureK;
>>>   }
>>>     constexpr uint16_t Awb::threshold(float value)
>>> @@ -450,10 +451,10 @@ void Awb::prepare([[maybe_unused]] const 
>>> uint32_t frame, IPAContext &context, ip
>>>          params->acc_param.bnr.opt_center_sqr.y_sqr_reset = 
>>> params->acc_param.bnr.opt_center.y_reset
>>>                                                          * 
>>> params->acc_param.bnr.opt_center.y_reset;
>>>          /* Convert to u3.13 fixed point values */
>>> -       params->acc_param.bnr.wb_gains.gr = 8192 * 
>>> context.frameContext.awb.gains.green;
>>> -       params->acc_param.bnr.wb_gains.r  = 8192 * 
>>> context.frameContext.awb.gains.red;
>>> -       params->acc_param.bnr.wb_gains.b  = 8192 * 
>>> context.frameContext.awb.gains.blue;
>>> -       params->acc_param.bnr.wb_gains.gb = 8192 * 
>>> context.frameContext.awb.gains.green;
>>> +       params->acc_param.bnr.wb_gains.gr = 8192 * 
>>> context.prevFrameContext.awb.gains.green;
>>> +       params->acc_param.bnr.wb_gains.r  = 8192 * 
>>> context.prevFrameContext.awb.gains.red;
>>> +       params->acc_param.bnr.wb_gains.b  = 8192 * 
>>> context.prevFrameContext.awb.gains.blue;
>>> +       params->acc_param.bnr.wb_gains.gb = 8192 * 
>>> context.prevFrameContext.awb.gains.green;
>>>            LOG(IPU3Awb, Debug) << "Color temperature estimated: " << 
>>> asyncResults_.temperatureK;
>>>   diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp 
>>> b/src/ipa/ipu3/algorithms/tone_mapping.cpp
>>> index bba5bc9a..f0fbaaf7 100644
>>> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
>>> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
>>> @@ -42,7 +42,7 @@ int ToneMapping::configure(IPAContext &context,
>>>                             [[maybe_unused]] const IPAConfigInfo 
>>> &configInfo)
>>>   {
>>>          /* Initialise tone mapping gamma value. */
>>> -       context.frameContext.toneMapping.gamma = 0.0;
>>> +       context.frameContextQueue.front().toneMapping.gamma = 0.0;
>> In another call you used getFrameContext(0). Which one is more
>> appropriate? Will it always be the same requirement? (to get the first,
>> or 0th?)
>>
>> Does the front of the queue represent frame 0, or frame 10 if there are
>> ten frames on the queue?
>
>
> It should be the 0th. FrameContexts queue should be cleared up once 
> stream has stopped.
>
>>
>> Perhaps a helper wrapper around the queue would let us declare methods
>> that are more descriptive? (Though I don't know which names are helpful
>> yet, so maybe it's overkill)
>>
>>     frameContextQueue.first() // Oldest?
>>     frameContextQueue.last() // Newest?
>>     frameContextQueue.mostRecent()
>>     frameContextQueue.mostRecentWithFilledStatistics() ?
>>     frameContextQueue.get(n)
>>     frameContextQueue.start(n)
>>     frameContextQueue.finish(n)
>>
>> (Or such, if they are useful)
>
>
> I'll look into it.
>
>>
>>>          return 0;
>>>   }
>>> @@ -62,7 +62,7 @@ void ToneMapping::prepare([[maybe_unused]] const 
>>> uint32_t frame,
>>>   {
>>>          /* Copy the calculated LUT into the parameters buffer. */
>>>          memcpy(params->acc_param.gamma.gc_lut.lut,
>>> - context.frameContext.toneMapping.gammaCorrection.lut,
>>> + context.prevFrameContext.toneMapping.gammaCorrection.lut,
>>>                 IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *
>>> sizeof(params->acc_param.gamma.gc_lut.lut[0]));
>>>   @@ -73,7 +73,7 @@ void ToneMapping::prepare([[maybe_unused]] const 
>>> uint32_t frame,
>>>     /**
>>>    * \brief Calculate the tone mapping look up table
>>> - * \param frame The frame number
>>> + te* \param frame The frame number
>> Oops?
>>
>
> :-/ Wondered why it didn't break the compilation and re-looked that 
> it's technically comment section
>
>>>    * \param context The shared IPA context
>>>    * \param stats The IPU3 statistics and ISP results
>>>    *
>>> @@ -83,6 +83,8 @@ void ToneMapping::prepare([[maybe_unused]] const 
>>> uint32_t frame,
>>>   void ToneMapping::process(const uint32_t frame, IPAContext &context,
>>>                            [[maybe_unused]] const ipu3_uapi_stats_3a 
>>> *stats)
>>>   {
>>> +       IPAFrameContext &frameContext = context.getFrameContext(frame);
>>> +
>> It does seem like we should do this lookup once before calling process()
>> on all algorithms perhaps.
>
>
> Ack!
>
>>
>>>          /*
>>>           * Hardcode gamma to 1.1 as a default for now.
>>>           *
>>> @@ -90,11 +92,11 @@ void ToneMapping::process(const uint32_t frame, 
>>> IPAContext &context,
>>>           */
>>>          gamma_ = 1.1;
>>>   -       if (context.frameContext.toneMapping.gamma == gamma_)
>>> +       if (frameContext.toneMapping.gamma == gamma_)
>>>                  return;
>>>            struct ipu3_uapi_gamma_corr_lut &lut =
>>> - context.frameContext.toneMapping.gammaCorrection;
>>> +               frameContext.toneMapping.gammaCorrection;
>>>            for (uint32_t i = 0; i < std::size(lut.lut); i++) {
>>>                  double j = static_cast<double>(i) / 
>>> (std::size(lut.lut) - 1);
>>> @@ -104,7 +106,7 @@ void ToneMapping::process(const uint32_t frame, 
>>> IPAContext &context,
>>>                  lut.lut[i] = gamma * 8191;
>>>          }
>>>   -       context.frameContext.toneMapping.gamma = gamma_;
>>> +       frameContext.toneMapping.gamma = gamma_;
>>>   }
>>>     } /* namespace ipa::ipu3::algorithms */
>>> diff --git a/src/ipa/ipu3/ipa_context.cpp 
>>> b/src/ipa/ipu3/ipa_context.cpp
>>> index 9c4ec936..41cbf717 100644
>>> --- a/src/ipa/ipu3/ipa_context.cpp
>>> +++ b/src/ipa/ipu3/ipa_context.cpp
>>> @@ -24,6 +24,48 @@ namespace libcamera::ipa::ipu3 {
>>>    * may also be updated in the start() operation.
>>>    */
>>>   +/**
>>> + * \brief Retrieve the context of a particular frame
>>> + * \param[in] frame Frame number
>>> + *
>>> + * Retrieve the frame context of the \a frame.
>>> + *
>>> + * \return The frame context of the given frame number or nullptr, 
>>> if not found
>> This doesn't match the implementation below.
>>
>>> + */
>>> +IPAFrameContext &IPAContext::getFrameContext(const uint32_t frame)
>>> +{
>>> +       auto iter = frameContextQueue.begin();
>>> +       while (iter != frameContextQueue.end()) {
>>> +               if (iter->frame == frame)
>>> +                       return *iter;
>>> +
>>> +               iter++;
>>> +       }
>>> +
>>> +       /*
>>> +        * \todo Handle the case where frame-context is not found here.
>>> +        * Should we be FATAL ?
>> We're in IPA here - so fatal could kill the IPA process without killing
>> or signalling the libcamera process. That's something we need to figure
>> out separately...
>
>
> Yes, hence seaparted out as \todo
>
>>
>> We might end up hitting this if we try to do something like
>>
>>     getFrameContext( frame - 1 );
>>
>> just after a frame was missed or not processed for some reason (like a
>> buffer underflow) which would mean we wouldn't have had stats on that
>> frame. So we need to work out how to handle that gracefully, and as
>> correctly as possible.
>
>
> and list down the cases in which this situation can occur. One is 
> frame drop, other ones that the frames cannot be processed are...?
>
> (will look into this as well)
>
>>
>>
>>> +        */
>>> +       return *iter; /* returns frameContextQueue.end() */
>>> +}
>>> +
>>> +/**
>>> + * \brief Construct a IPAFrameContext instance
>>> + */
>>> +IPAFrameContext::IPAFrameContext() = default;
>>> +
>>> +/**
>>> + * \brief Move constructor for IPAFrameContext
>>> + * \param[in] other The other IPAFrameContext
>>> + */
>>> +IPAFrameContext::IPAFrameContext(IPAFrameContext &&other) = default;
>>> +
>>> +/**
>>> + * \brief Move assignment operator for IPAFrameContext
>>> + * \param[in] other The other IPAFrameContext
>>> + */
>>> +IPAFrameContext &IPAFrameContext::operator=(IPAFrameContext 
>>> &&other) = default;
>>> +
>>>   /**
>>>    * \struct IPAFrameContext
>>>    * \brief Per-frame context for algorithms
>>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>>> index d993359a..eca346d6 100644
>>> --- a/src/ipa/ipu3/ipa_context.h
>>> +++ b/src/ipa/ipu3/ipa_context.h
>>> @@ -8,6 +8,8 @@
>>>     #pragma once
>>>   +#include <deque>
>>> +
>>>   #include <linux/intel-ipu3.h>
>>>     #include <libcamera/base/utils.h>
>>> @@ -39,6 +41,12 @@ struct IPASessionConfiguration {
>>>   };
>>>     struct IPAFrameContext {
>>> +       uint32_t frame;
>>> +
>>> +       IPAFrameContext();
>>> +       IPAFrameContext(IPAFrameContext &&other);
>>> +       IPAFrameContext &operator=(IPAFrameContext &&other);
>>> +
>>>          struct {
>>>                  uint32_t exposure;
>>>                  double gain;
>>> @@ -66,8 +74,12 @@ struct IPAFrameContext {
>>>   };
>>>     struct IPAContext {
>>> +       IPAFrameContext &getFrameContext(const uint32_t frame);
>>>          IPASessionConfiguration configuration;
>>>          IPAFrameContext frameContext;
>>> +
>>> +       std::deque<IPAFrameContext> frameContextQueue;
>>> +       IPAFrameContext prevFrameContext;
>>>   };
>>>     } /* namespace ipa::ipu3 */
>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>>> index 3d5c5706..f3128b0a 100644
>>> --- a/src/ipa/ipu3/ipu3.cpp
>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>> @@ -349,6 +349,8 @@ int IPAIPU3::start()
>>>    */
>>>   void IPAIPU3::stop()
>>>   {
>>> +       while (!context_.frameContextQueue.empty())
>>> +               context_.frameContextQueue.pop_front();
>> would deque::clear() be more distinct here?
>>
>
> Ah yeah
>
>>>   }
>>>     /**
>>> @@ -451,6 +453,14 @@ int IPAIPU3::configure(const IPAConfigInfo 
>>> &configInfo,
>>>           */
>>>          ctrls_ = configInfo.sensorControls;
>>>   +       /*
>>> +        * Insert a initial context into the queue to faciliate
>>> +        * algo->configure() below.
>>> +        */
>>> +       IPAFrameContext initContext;
>>> +       initContext.frame = 0;
>> I ... am not 100% sure about how to handle this.
>>
>> Shouldn't this be frameStarted(0). ... and what happens if you've added
>> this frameContext(0) here, and then the PH delivers a frameStarted(0)
>> event?
>>
>>> + context_.frameContextQueue.push_back(std::move(initContext));
>>> +
>>>          calculateBdsGrid(configInfo.bdsOutputSize);
>>>            /* Clean frameContext at each reconfiguration. */
>>> @@ -501,10 +511,25 @@ void IPAIPU3::unmapBuffers(const 
>>> std::vector<unsigned int> &ids)
>>>     void IPAIPU3::frameStarted([[maybe_unused]] const uint32_t frame)
>>>   {
>>> +       IPAFrameContext newContext;
>>> +       newContext.frame = frame;
>>> +
>>> + context_.frameContextQueue.push_back(std::move(newContext));
>>>   }
>>>     void IPAIPU3::frameCompleted([[maybe_unused]] const uint32_t frame)
>>>   {
>>> +       while (!context_.frameContextQueue.empty()) {
>>> +               auto &fc = context_.frameContextQueue.front();
>>> +               if (fc.frame < frame)
>>> + context_.frameContextQueue.pop_front();
>>> +
>>> +               /* Keep newer frames */
>>> +               if (fc.frame >= frame) {
>>> +                       context_.prevFrameContext = std::move(fc);
>>> +                       break;
>>> +               }
>>> +       }
>> I could see us making a dedicated templated queue class specialised to
>> the needs of keeping contexts around and remembering the most recent
>> completed data, which could then be used by other IPAs... But lets get
>> this working and right first and then we can refactor to something
>> generically re-usable I think.
>>
>>>   }
>>>     /**
>>> @@ -547,8 +572,9 @@ void IPAIPU3::processStatsBuffer(const uint32_t 
>>> frame, const int64_t frameTimest
>>>          const ipu3_uapi_stats_3a *stats =
>>>                  reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>>>   -       context_.frameContext.sensor.exposure = 
>>> sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>>> -       context_.frameContext.sensor.gain = 
>>> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>>> +       IPAFrameContext &curFrameContext = 
>>> context_.getFrameContext(frame);
>>> +       curFrameContext.sensor.exposure = 
>>> sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>>> +       curFrameContext.sensor.gain = 
>>> camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>>>            parseStatistics(frame, frameTimestamp, stats);
>>>   }
>>> @@ -623,11 +649,11 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>>          int64_t frameDuration = (vBlank + 
>>> sensorInfo_.outputSize.height) * lineDuration;
>>>          ctrls.set(controls::FrameDuration, frameDuration);
>>>   -       ctrls.set(controls::AnalogueGain, 
>>> context_.frameContext.sensor.gain);
>>> +       ctrls.set(controls::AnalogueGain, 
>>> context_.prevFrameContext.sensor.gain);
>> This doesn't sound right - when we set this metadata, it should be
>> reflecting the gain that was set for that frame.
>
>
> err, I can't remember the reason why I needed the prevFrameContext 
> here. I'll investigate it again.


Oh yeah, I remembered the case, it was due to the context.sensor.* 
values not getting updated / available for the current frame. But I have 
fixed that above but forgot to revert the change of use of 
prevFrameContext :-/

Using the curFrameContext values as you said, is fine (and validated by 
CTS right now). Sorry for the noise


>
>>
>> We really need a full sequence diagram of the operations from the app
>> (queueing a request) to the PH - to the IPA and when frames are
>> received over CSI2 and handled by the ISP...
>>
>> https://sequencediagram.org/ looks helpful for us to make that.
>
>
> Agreed, you already have an version, I will fork it to create the 
> interaction with frame context queue.
>
>>
>>>   - ctrls.set(controls::ColourTemperature, 
>>> context_.frameContext.awb.temperatureK);
>>> +       ctrls.set(controls::ColourTemperature, 
>>> context_.prevFrameContext.awb.temperatureK);
>>>   -       ctrls.set(controls::ExposureTime, 
>>> context_.frameContext.sensor.exposure * lineDuration);
>>> +       ctrls.set(controls::ExposureTime, 
>>> context_.prevFrameContext.sensor.exposure * lineDuration);
>>>            /*
>>>           * \todo The Metadata provides a path to getting extended data
>>> @@ -651,8 +677,9 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>>    */
>>>   void IPAIPU3::setControls(unsigned int frame)
>>>   {
>>> -       int32_t exposure = context_.frameContext.agc.exposure;
>>> -       int32_t gain = 
>>> camHelper_->gainCode(context_.frameContext.agc.gain);
>>> +       IPAFrameContext &context = context_.getFrameContext(frame);
>>> +       int32_t exposure = context.agc.exposure;
>>> +       int32_t gain = camHelper_->gainCode(context.agc.gain);
>>
>> We need to check and think about when these controls get set too. But -
>> I also think that's going to be part of the reworkign delayed
>> controls/perframe control support ... so maybe it's something to
>> consider on top anyway.
>>
>>
>>>            ControlList ctrls(ctrls_);
>>>          ctrls.set(V4L2_CID_EXPOSURE, exposure);
>>> -- 
>>> 2.31.0
>>>


More information about the libcamera-devel mailing list