[libcamera-devel] [PATCH v3 05/16] ipa: ipu3: Update camera controls in configure()
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Fri Oct 15 09:26:10 CEST 2021
Hi Jacopo,
On 11/10/2021 17:11, Jacopo Mondi wrote:
> When a new CameraConfiguration is applied to the Camera the IPA is
> configured as well, using the newly applied sensor configuration and its
> updated V4L2 controls.
>
> Also update the Camera controls at IPA::configure() time by re-computing
> the controls::ExposureTime and controls::FrameDurationLimits limits and
> update the controls on the pipeline handler side after having configured
> the IPA.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/ipa/ipu3.mojom | 3 +-
> src/ipa/ipu3/ipu3.cpp | 82 +++++++++++++++++++---------
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +-
> 3 files changed, 60 insertions(+), 29 deletions(-)
>
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index 2045ce909a88..2f254ed4193e 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -45,7 +45,8 @@ interface IPAIPU3Interface {
> start() => (int32 ret);
> stop();
>
> - configure(IPAConfigInfo configInfo) => (int32 ret);
> + configure(IPAConfigInfo configInfo)
> + => (int32 ret, libcamera.ControlInfoMap ipaControls);
>
> mapBuffers(array<libcamera.IPABuffer> buffers);
> unmapBuffers(array<uint32> ids);
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 6d9bbf391cb2..388f1902bcab 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -171,13 +171,17 @@ public:
> int start() override;
> void stop() override {}
>
> - int configure(const IPAConfigInfo &configInfo) override;
> + int configure(const IPAConfigInfo &configInfo,
> + ControlInfoMap *ipaControls) override;
>
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> void unmapBuffers(const std::vector<unsigned int> &ids) override;
> void processEvent(const IPU3Event &event) override;
>
> private:
> + void updateControls(const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> + ControlInfoMap *ipaControls);
> void processControls(unsigned int frame, const ControlList &controls);
> void fillParams(unsigned int frame, ipu3_uapi_params *params);
> void parseStatistics(unsigned int frame,
> @@ -212,36 +216,30 @@ private:
> struct IPAContext context_;
> };
>
> -/**
> - * Initialize the IPA module and its controls.
> +
> +/*
> + * Compute camera controls using the sensor information and the sensor
> + * v4l2 controls.
> *
> - * This function receives the camera sensor information from the pipeline
> - * handler, computes the limits of the controls it handles and returns
> - * them in the \a ipaControls output parameter.
> + * Some of the camera controls are computed by the pipeline handler, some others
> + * by the IPA module which is in charge of handling, for example, the exposure
> + * time and the frame duration.
> + *
> + * This function computes:
> + * - controls::ExposureTime
> + * - controls::FrameDurationLimits
> */
> -int IPAIPU3::init(const IPASettings &settings,
> - const IPACameraSensorInfo &sensorInfo,
> - const ControlInfoMap &sensorControls,
> - ControlInfoMap *ipaControls)
> +void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> + ControlInfoMap *ipaControls)
> {
> - camHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);
> - if (camHelper_ == nullptr) {
> - LOG(IPAIPU3, Error)
> - << "Failed to create camera sensor helper for "
> - << settings.sensorModel;
> - return -ENODEV;
> - }
> -
> - /* Initialize Controls. */
> ControlInfoMap::Map controls{};
>
> /*
> - * Compute exposure time limits.
> - *
> - * Initialize the control using the line length and pixel rate of the
> - * current configuration converted to microseconds. Use the
> - * V4L2_CID_EXPOSURE control to get exposure min, max and default and
> - * convert it from lines to microseconds.
> + * Compute exposure time limits by using line length and pixel rate
> + * converted to microseconds. Use the V4L2_CID_EXPOSURE control to get
> + * exposure min, max and default and convert it from lines to
> + * microseconds.
> */
> double lineDuration = sensorInfo.lineLength / (sensorInfo.pixelRate / 1e6);
> const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> @@ -279,6 +277,27 @@ int IPAIPU3::init(const IPASettings &settings,
> frameDurations[2]);
>
> *ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> +}
> +
> +/**
> + * Initialize the IPA module and its controls.
> + *
> + * This function receives the camera sensor information from the pipeline
> + * handler, computes the limits of the controls it handles and returns
> + * them in the \a ipaControls output parameter.
> + */
> +int IPAIPU3::init(const IPASettings &settings,
> + const IPACameraSensorInfo &sensorInfo,
> + const ControlInfoMap &sensorControls,
> + ControlInfoMap *ipaControls)
> +{
> + camHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);
> + if (camHelper_ == nullptr) {
> + LOG(IPAIPU3, Error)
> + << "Failed to create camera sensor helper for "
> + << settings.sensorModel;
> + return -ENODEV;
> + }
>
> /* Construct our Algorithms */
> algorithms_.push_back(std::make_unique<algorithms::Agc>());
> @@ -286,6 +305,9 @@ int IPAIPU3::init(const IPASettings &settings,
> algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
> algorithms_.push_back(std::make_unique<algorithms::ToneMapping>());
>
> + /* Initialize controls. */
> + updateControls(sensorInfo, sensorControls, ipaControls);
> +
Thanks, I had a patch almost doing the same, I did not notice you
already did that... :-).
> return 0;
> }
>
> @@ -365,7 +387,8 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)
> << (int)bdsGrid.height << " << " << (int)bdsGrid.block_height_log2 << ")";
> }
>
> -int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> +int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> + ControlInfoMap *ipaControls)
> {
> if (configInfo.sensorControls.empty()) {
> LOG(IPAIPU3, Error) << "No sensor controls provided";
> @@ -374,6 +397,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>
> sensorInfo_ = configInfo.sensorInfo;
>
> + /*
> + * Compute the sensor V4L2 controls to be used by the algorithms and
> + * to be set on the sensor.
> + */
> ctrls_ = configInfo.sensorControls;
>
> const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> @@ -415,6 +442,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
> return ret;
> }
>
> + /* Update the camera controls using the new sensor settings. */
> + updateControls(sensorInfo_, ctrls_, ipaControls);
> +
I would have put that call before the algorithm->configure() calls, as
we may (wait for it) need to pass the updated values to some of them
(AGC?) through the IPAContext.
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> return 0;
> }
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 1033153360b8..e0b61deb9b47 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -659,14 +659,14 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> configInfo.bdsOutputSize = config->imguConfig().bds;
> configInfo.iif = config->imguConfig().iif;
>
> - ret = data->ipa_->configure(configInfo);
> + ret = data->ipa_->configure(configInfo, &data->ipaControls_);
> if (ret) {
> LOG(IPU3, Error) << "Failed to configure IPA: "
> << strerror(-ret);
> return ret;
> }
>
> - return 0;
> + return updateControls(data);
> }
>
> int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
>
More information about the libcamera-devel
mailing list