[libcamera-devel] [IPU3-IPA PATCH v3 3/6] ipu3: Set statistics with the effective AE AiqResults

[Kieran Bingham]

Quoting Han-Lin Chen (2021-11-11 10:49:05)
> Set the statistics with the latest AE AiqResults which has the same
> exposure time and analog gain. The patch reduces the AE hunting during
> the converging process.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>  aiq/aiq_input_parameters.cpp |  2 +-
>  ipu3.cpp                     | 75 ++++++++++++++++++++++++++++--------
>  2 files changed, 61 insertions(+), 16 deletions(-)
> 
> diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp
> index bc87b31..46553a6 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;
> diff --git a/ipu3.cpp b/ipu3.cpp
> index 8126e9d..9d07268 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);
>  
> @@ -83,7 +84,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 +283,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 +330,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);
>                 break;
>         }
>         case EventFillParams: {
> @@ -374,14 +380,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_);
> +       resultsHistory_.extendOne();
> +       aiq::AiqResults& latestResults = resultsHistory_.latest();
>  
> -       aic_.updateRuntimeParams(results_);
> +       aiq_.run2a(frame, aiqInputParams_, latestResults);
> +       aic_.updateRuntimeParams(latestResults);
>         aic_.run(params);
>  
> -       exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;
> -       gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;
> +       exposure_ = latestResults.ae()->exposures[0].sensor_exposure->coarse_integration_time;
> +       gain_ = latestResults.ae()->exposures[0].sensor_exposure->analog_gain_code_global;
> +
>         setControls(frame);
>  
>         IPU3Action op;
> @@ -392,7 +400,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 +412,45 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>          */
>  
>         ASSERT (frameTimestamp > 0);
> -       aiq_.setStatistics(frame, frameTimestamp, results_, stats);
> +
> +       /* Ae algorithm expects the statistics to be set with its corresponding
> +        * Ae result, i.e., the Ae result should match the exposure time and
> +        * 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().
> +        */
> +
> +       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& latestResults = resultsHistory_.latest();
> +       aiq::AiqResults& aeMatchedResults = resultsHistory_.searchBackward(pred, latestResults);
> +
> +       aiq::AiqResults combinedResults = latestResults;
> +       combinedResults.setAe(aeMatchedResults.ae());
> +
> +       /* Aiq library expects timestamp in microseconds */

That's a specific bug fix, that could/should have had it's own patch.

> +       aiq_.setStatistics(frame, (frameTimestamp / 1000), combinedResults, stats);

brackets aren't needed there either, but it does help improve
readability to me anyway.


>         /* 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 +467,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;

Hrm .. I think this is crucially needed too now that we've merged the 

f12efa673064 ipa: ipu3: Extend ipu3 ipa interface for sensor controls

So that should have been a separate patch too.


Anyway, patch splitting aside.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>  
>         queueFrameAction.emit(frame, op);
>  }
> -- 
> 2.34.0.rc1.387.gb447b232ab-goog
>