[libcamera-devel] [PATCH v2 3/6] ipu3: Set statistics with the effective AE AiqResults
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 2 14:58:16 CET 2021
Quoting Han-Lin Chen (2021-10-29 12:59:58)
> Set the statistics with the latest AE AiqResults which has the same
> exposure time and analog gain. The patch reduces the AE haunting during
s/haunting/hunting/
Although appropriate for Halloween ;-)
> the converging process.
>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
> aiq/aiq_input_parameters.cpp | 2 +-
> ipu3.cpp | 66 ++++++++++++++++++++++++++++++------
> 2 files changed, 56 insertions(+), 12 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..f463805 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_;
Is this still needed (results_)? Isn't it just stored in the history ring buffer?
> + 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);
> 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_);
Very happy to see the artifical run delay removed.
> 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_);
> +
Ah, ok so you are still using the results_ ... but is this introducing
unnecessary copies? Can't we just decide / create a new entry in the
resultsHistory_ (and later rename that to be 'results_') and directly
run the run2a call into that active entry?
> 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 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& aeMatchedResults = resultsHistory_.searchBackward(pred, results_);
This is an interesting implementation! (in a good way), though I wonder
if it's over complicated.
I thought we'd be able to do something simpler, because we know the
frame number index, so we should be able to associate that with the
resultsHistory_ and obtain the results from the frame that was queued
with the same 'frame' number...
Could you check the frame numbers of when the parameters are queued up,
against the 'frame' that comes in here, and see if it matches up with
your implementation as a suitable method to tie the results together?
> + aiq::AiqResults combinedResults = results_;
> + combinedResults.setAe(aeMatchedResults.ae());
Oh - that's interesting, so we take the latest calculated results, and
the ae() from the matched results..
> +
> + /* Aiq library expects timestamp in microseconds */
Perhaps std::chrono would help us here in the future. Doesn't have to be
now.
> + 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);
> }
> --
> 2.33.1.1089.g2158813163f-goog
>
More information about the libcamera-devel
mailing list