[libcamera-devel] [PATCH v3 3/3] ipa: ipu3: Ensure controls exists in updateSessionConfiguration()
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Mar 25 10:08:54 CET 2022
Hi Umang,
Quoting Umang Jain via libcamera-devel (2022-03-24 14:20:35)
> Add a control-not-found error checking for the controls being
> queried in IPAIPU3::updateSessionConfiguration(). If the control
> is not found, progagate the error back to the caller. This requires
> a change in the function signature of private member function
> IPAIPU3::updateSessionConfiguration().
In RPi IPA this is handled with a dedicated function that can check a
list of controls, which helps make it extendable and keep the list of
required controls centralised.
Look at
IPARPi::validateSensorControls()
IPARPi::validateIspControls()
IPARPi::validateLensControls()
A loop or dedicated validate function might make sense, but I think this
is also valid. At least this implementation means once the ctrl is
found, it's used without having to search again later.
So either way:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/ipa/ipu3/ipu3.cpp | 44 +++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 3717d893..66346728 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -149,7 +149,7 @@ private:
> void updateControls(const IPACameraSensorInfo &sensorInfo,
> const ControlInfoMap &sensorControls,
> ControlInfoMap *ipaControls);
> - void updateSessionConfiguration(const ControlInfoMap &sensorControls);
> + int updateSessionConfiguration(const ControlInfoMap &sensorControls);
> void processControls(unsigned int frame, const ControlList &controls);
> void fillParams(unsigned int frame, ipu3_uapi_params *params);
> void parseStatistics(unsigned int frame,
> @@ -180,18 +180,33 @@ private:
> * \brief Compute IPASessionConfiguration using the sensor information and the
> * sensor V4L2 controls
> */
> -void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
> +int IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
> {
> - const ControlInfo vBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
> - context_.configuration.sensor.defVBlank = vBlank.def().get<int32_t>();
> + const auto itVBlank = sensorControls.find(V4L2_CID_VBLANK);
> + if (itVBlank == sensorControls.end()) {
> + LOG(IPAIPU3, Error) << "Can't find vblank control";
> + return -EINVAL;
> + }
>
> - const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> - int32_t minExposure = v4l2Exposure.min().get<int32_t>();
> - int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
> + context_.configuration.sensor.defVBlank = itVBlank->second.def().get<int32_t>();
> +
> + const auto itExp = sensorControls.find(V4L2_CID_EXPOSURE);
> + if (itExp == sensorControls.end()) {
> + LOG(IPAIPU3, Error) << "Can't find exposure control";
> + return -EINVAL;
> + }
> +
> + int32_t minExposure = itExp->second.min().get<int32_t>();
> + int32_t maxExposure = itExp->second.max().get<int32_t>();
>
> - const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> - int32_t minGain = v4l2Gain.min().get<int32_t>();
> - int32_t maxGain = v4l2Gain.max().get<int32_t>();
> + const auto itGain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN);
> + if (itGain == sensorControls.end()) {
> + LOG(IPAIPU3, Error) << "Can't find analogue gain control";
> + return -EINVAL;
> + }
> +
> + int32_t minGain = itGain->second.min().get<int32_t>();
> + int32_t maxGain = itGain->second.max().get<int32_t>();
>
> /*
> * When the AGC computes the new exposure values for a frame, it needs
> @@ -204,6 +219,8 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
> context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> +
> + return 0;
> }
>
> /**
> @@ -437,10 +454,13 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> updateControls(sensorInfo_, sensorCtrls_, ipaControls);
>
> /* Update the IPASessionConfiguration using the sensor settings. */
> - updateSessionConfiguration(sensorCtrls_);
> + int ret = updateSessionConfiguration(sensorCtrls_);
> + if (ret < 0)
> + return ret;
> +
>
> for (auto const &algo : algorithms_) {
> - int ret = algo->configure(context_, configInfo);
> + ret = algo->configure(context_, configInfo);
> if (ret)
> return ret;
> }
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list