| Message ID | 20260703153819.1088752-2-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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 >
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 = @@ -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;
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(-)