[RFC,v1,01/17] ipa: simple: Fix control presence sanity check

27180 diff mbox series

Check existence before the values are retrieved.

Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241
Fixes: fb8ad13dc3e3 ("libcamera: software_isp: Move exposure+gain to an algorithm module")
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/simple/soft_simple.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Quoting Barnabás Pőcze (2026-07-03 16:38:03)
> Check existence before the values are retrieved.
> 
> Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241
> Fixes: fb8ad13dc3e3 ("libcamera: software_isp: Move exposure+gain to an algorithm module")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/simple/soft_simple.cpp | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index 629e1a32de..2a41b5bac8 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -299,6 +299,13 @@ void IPASoftSimple::processStats(const uint32_t frame,
>                                  [[maybe_unused]] const uint32_t bufferId,
>                                  const ControlList &sensorControls)
>  {
> +       /* Sanity check */
> +       if (!sensorControls.contains(V4L2_CID_EXPOSURE) ||
> +           !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {
> +               LOG(IPASoft, Error) << "Control(s) missing";
> +               return;
> +       }
> +
>         IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>  
>         frameContext.sensor.exposure =

As the access to obtain the exposure and gain are right here, indeed
removing from below looks like it makes sense, but if we can't process -
then in fact I think this check/enforcement should move to the init or
configure functions and not run on every frame.

And looking at IPASoftSimple::init


        if (sensorControls.find(V4L2_CID_EXPOSURE) == sensorControls.end()) {
                LOG(IPASoft, Error) << "Don't have exposure control";
                return -EINVAL;
        }

        if (sensorControls.find(V4L2_CID_ANALOGUE_GAIN) == sensorControls.end()) {
                LOG(IPASoft, Error) << "Don't have gain control";
                return -EINVAL;
        }

I think we can just remove the sanity check rather than move it.

--
Kieran




> @@ -311,13 +318,6 @@ void IPASoftSimple::processStats(const uint32_t frame,
>                 algo->process(context_, frame, frameContext, stats_, metadata);
>         metadataReady.emit(frame, metadata);
>  
> -       /* Sanity check */
> -       if (!sensorControls.contains(V4L2_CID_EXPOSURE) ||
> -           !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {
> -               LOG(IPASoft, Error) << "Control(s) missing";
> -               return;
> -       }
> -
>         ControlList ctrls(sensorInfoMap_);
>  
>         auto &againNew = frameContext.sensor.gain;
> -- 
> 2.54.0
>