[libcamera-devel] [PATCH v4 3/3] ipa: ipu3: Ensure controls exists in before they are queried
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Mar 25 12:48:57 CET 2022
Quoting Umang Jain via libcamera-devel (2022-03-25 10:34:10)
> Add a validation check for sensor controls validateSensorControls()
> before they are queried in IPAIPU3::updateSessionConfiguration().
> Fail the IPAIPU3::configure() if the required sensor controls are
> not found.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/ipa/ipu3/ipu3.cpp | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 3717d893..50b52d8b 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -155,6 +155,7 @@ private:
> void parseStatistics(unsigned int frame,
> int64_t frameTimestamp,
> const ipu3_uapi_stats_3a *stats);
> + bool validateSensorControls();
>
> void setControls(unsigned int frame);
> void calculateBdsGrid(const Size &bdsOutputSize);
> @@ -268,6 +269,28 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> *ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> }
>
> +/**
> + * \brief Validate that the sensor controls mandatory for the IPA exists
> + */
> +bool IPAIPU3::validateSensorControls()
> +{
> + static const uint32_t ctrls[] = {
> + V4L2_CID_ANALOGUE_GAIN,
> + V4L2_CID_EXPOSURE,
> + V4L2_CID_VBLANK,
> + };
> +
> + for (auto c : ctrls) {
> + if (sensorCtrls_.find(c) == sensorCtrls_.end()) {
> + LOG(IPAIPU3, Error) << "Unable to find sensor control "
> + << utils::hex(c);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> /**
> * \brief Initialize the IPA module and its controls
> *
> @@ -433,6 +456,11 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> /* Clean frameContext at each reconfiguration. */
> context_.frameContext = {};
>
> + if (!validateSensorControls()) {
> + LOG(IPAIPU3, Error) << "Sensor control validation failed.";
> + return -EINVAL;
> + }
> +
I think this is fine, but I would have probably put it directly after
the assignment of
sensorCtrls_ = configInfo.sensorControls;
in IPAIPU3::configure().
Or better yet, if IPAIPU3::validateSensorControls() takes a reference to
a control list, we can validate the configInfo.sensorControls; before we
assign it? That way we're logically validating the input before we
accept it.
Either way though - it's not a big deal, so:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> /* Update the camera controls using the new sensor settings. */
> updateControls(sensorInfo_, sensorCtrls_, ipaControls);
>
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list