[libcamera-devel] [PATCH 3/6] ipu3: Set statistics with the effective AE AiqResults
Hanlin Chen
hanlinchen at chromium.org
Fri Oct 29 13:55:45 CEST 2021
On Fri, Oct 29, 2021 at 2:34 AM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hi Han-Lin,
>
> Thank you for the patch.
>
> On 10/28/21 3:33 PM, Han-Lin Chen wrote:
> > Set the statistics with the latest AE AiqResults which has the same
> > exposure time and analog gain. The patch reduces the AE haunting during
> > the converging process.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.com>
> > ---
> > aiq/aiq_input_parameters.cpp | 4 +--
> > ipu3.cpp | 66 ++++++++++++++++++++++++++++++------
> > 2 files changed, 57 insertions(+), 13 deletions(-)
> >
> > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp
> > index bc87b31..5dd2f6c 100644
> > --- a/aiq/aiq_input_parameters.cpp
> > +++ b/aiq/aiq_input_parameters.cpp
> > @@ -130,7 +130,7 @@ AiqInputParameters &AiqInputParameters::operator=(const AiqInputParameters &othe
> >
> > void AiqInputParameters::setAeAwbAfDefaults()
> > {
> > - /*Ae Params */
> > + /* Ae Params */
> > aeInputParams.num_exposures = NUM_EXPOSURES;
> > aeInputParams.frame_use = ia_aiq_frame_use_preview;
> > aeInputParams.flash_mode = ia_aiq_flash_mode_off;
> > @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults()
> > ia_aiq_af_range_normal,
> > ia_aiq_af_metering_mode_auto,
> > ia_aiq_flash_mode_off,
> > - NULL, NULL, false
> > + &focusRect, &manualFocusParams, false
>
>
> Should this be moved to focus patch one instead? I find this odd to be
> present in this patch, since it's about AE results?
Yes, you're right ;-).
>
> > };
> >
> > /* GBCE Params */
> > diff --git a/ipu3.cpp b/ipu3.cpp
> > index 8126e9d..b7de901 100644
> > --- a/ipu3.cpp
> > +++ b/ipu3.cpp
> > @@ -59,7 +59,8 @@ private:
> > void fillParams(unsigned int frame, ipu3_uapi_params *params);
> > void parseStatistics(unsigned int frame,
> > int64_t frameTimestamp,
> > - const ipu3_uapi_stats_3a *stats);
> > + const ipu3_uapi_stats_3a *stats,
> > + const ControlList& sensorCtrls);
> >
> > void setControls(unsigned int frame);
> >
> > @@ -84,6 +85,7 @@ private:
> > /* Temporary storage until we have a FrameContext object / struct */
> > aiq::AiqInputParameters aiqInputParams_;
> > aiq::AiqResults results_;
> > + aiq::AiqResultsRingBuffer resultsHistory_;
> >
> > BinaryData aiqb_;
> > BinaryData nvm_;
> > @@ -282,6 +284,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> > /* Upate the camera controls using the new sensor settings. */
> > updateControls(sensorInfo_, ctrls_, ipaControls);
> >
> > + resultsHistory_.Reset();
> > +
> > return 0;
> > }
> >
> > @@ -327,7 +331,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> > const ipu3_uapi_stats_3a *stats =
> > reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
> >
> > - parseStatistics(event.frame, event.frameTimestamp, stats);
> > + parseStatistics(event.frame,
> > + event.frameTimestamp,
> > + stats,
> > + event.sensorControls);
>
>
> indentation
>
> > break;
> > }
> > case EventFillParams: {
> > @@ -374,14 +381,16 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> > */
> >
> > /* Run algorithms into/using this context structure */
> > - if (frame % 10 == 0)
> > - aiq_.run2a(frame, aiqInputParams_, results_);
> > + aiq_.run2a(frame, aiqInputParams_, results_);
> >
> > aic_.updateRuntimeParams(results_);
> > aic_.run(params);
> >
> > exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;
> > gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;
> > +
> > + resultsHistory_.Push(results_);
>
>
> Possibly usage of ringbuffer to highlight how it's used, what it
> provides (a high level documentation on it's 'need', the 'when' and
> 'why' of previous algo-runs results lookup) would be good to understand
> the context I believe. All this can be a separate patch.
>
> > +
> > setControls(frame);
> >
> > IPU3Action op;
> > @@ -392,7 +401,8 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> >
> > void IPAIPU3::parseStatistics(unsigned int frame,
> > int64_t frameTimestamp,
> > - const ipu3_uapi_stats_3a *stats)
> > + const ipu3_uapi_stats_3a *stats,
> > + const ControlList& sensorCtrls)
> > {
> > ControlList ctrls(controls::controls);
> >
> > @@ -403,10 +413,43 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> > */
> >
> > ASSERT (frameTimestamp > 0);
> > - aiq_.setStatistics(frame, frameTimestamp, results_, stats);
> > +
> > + /* Ae algorithm expects the statistics to be set with it's corresponding Ae
> > + * result, i.e., the Ae result should match the exposure time/analog gain
> > + * with the the effective sensor controls of the statistics.
> > + * Search the required Ae result in the result history and combine it with
> > + * the latest result as the input to AIQ::setStatistics().
> > + */
>
>
> Is there a bit more descriptive documentation I can refer to? Maybe in
> the ia_aiq documentation to understand 'when' and 'how' the aiq results
> are meant to be re-used? I think I wrote my last comment a bit hastily
Sadly, there are no documents about it. I collect the restriction from
Intel's HAL implementation and experiments.
>
> > +
> > + int32_t effectiveExpo = 0;
> > + int32_t effectiveGain = 0;
> > + ControlValue ctrlValue;
> > +
> > + ctrlValue = sensorCtrls.get(V4L2_CID_EXPOSURE);
> > + if (!ctrlValue.isNone())
> > + effectiveExpo = ctrlValue.get<int32_t>();
> > +
> > + ctrlValue = sensorCtrls.get(V4L2_CID_ANALOGUE_GAIN);
> > + if (!ctrlValue.isNone())
> > + effectiveGain = ctrlValue.get<int32_t>();
> > +
> > + auto pred = [effectiveExpo, effectiveGain] (aiq::AiqResults& result) {
> > + ia_aiq_exposure_sensor_parameters* sensorExposure =
> > + result.ae()->exposures[0].sensor_exposure;
> > +
> > + return (effectiveExpo == sensorExposure->coarse_integration_time ||
> > + effectiveGain == sensorExposure->analog_gain_code_global);
> > + };
> > + aiq::AiqResults& aeMatchedResults = resultsHistory_.searchBackward(pred, results_);
> > + aiq::AiqResults combinedResults = results_;
> > + combinedResults.setAe(aeMatchedResults.ae());
> > +
> > + /* Aiq library expects timestamp in microseconds */
> > + aiq_.setStatistics(frame, (frameTimestamp / 1000), combinedResults, stats);
> >
> > /* Set frame durations from exposure results */
> > - ia_aiq_exposure_sensor_parameters *sensorExposure = results_.ae()->exposures->sensor_exposure;
> > + ia_aiq_exposure_sensor_parameters *sensorExposure = combinedResults.ae()->exposures->sensor_exposure;
> > int64_t frameDuration = (sensorExposure->line_length_pixels * sensorExposure->frame_length_lines) /
> > (sensorInfo_.pixelRate / 1e6);
> > ctrls.set(controls::FrameDuration, frameDuration);
> > @@ -423,10 +466,11 @@ void IPAIPU3::setControls(unsigned int frame)
> > IPU3Action op;
> > op.op = ActionSetSensorControls;
> >
> > - ControlList ctrls(ctrls_);
> > - ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> > - ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> > - op.controls = ctrls;
> > + ControlList sensorCtrls(ctrls_);
> > + sensorCtrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
> > + sensorCtrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> > +
> > + op.sensorControls = sensorCtrls;
> >
> > queueFrameAction.emit(frame, op);
> > }
More information about the libcamera-devel
mailing list