| Message ID | 20260803131435.153927-3-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > The check is useless because `DelayedControls::get()` always populates > the result even if the value is "none", so the presence check will > always succeed. I'm not sure I get the meaning right but the presence of the controls is already checked in IPASoftSimple::init. Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > And furthermore the check also has the issue of being later than the > actual access. > > Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/simple/soft_simple.cpp | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index 629e1a32de..e99df1986f 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -311,13 +311,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;
Hi 2026. 08. 04. 11:41 keltezéssel, Milan Zamazal írta: > Hi Barnabás, > > Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > >> The check is useless because `DelayedControls::get()` always populates >> the result even if the value is "none", so the presence check will >> always succeed. > > I'm not sure I get the meaning right but the presence of the controls is > already checked in IPASoftSimple::init. That only checks if the sensor supports these controls. This check is for the control list that contains the controls that were in effect for the given frame. See the linked issue and https://patchwork.libcamera.org/patch/27180/#39737 for more info. > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > >> And furthermore the check also has the issue of being later than the >> actual access. >> >> Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241 >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/simple/soft_simple.cpp | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp >> index 629e1a32de..e99df1986f 100644 >> --- a/src/ipa/simple/soft_simple.cpp >> +++ b/src/ipa/simple/soft_simple.cpp >> @@ -311,13 +311,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; >
Hi Barnabás On Mon, Aug 03, 2026 at 03:13:47PM +0200, Barnabás Pőcze wrote: > The check is useless because `DelayedControls::get()` always populates > the result even if the value is "none", so the presence check will > always succeed. And furthermore the check also has the issue of being > later than the actual access. > > Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > src/ipa/simple/soft_simple.cpp | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index 629e1a32de..e99df1986f 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -311,13 +311,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.55.0 >
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 629e1a32de..e99df1986f 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -311,13 +311,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;
The check is useless because `DelayedControls::get()` always populates the result even if the value is "none", so the presence check will always succeed. And furthermore the check also has the issue of being later than the actual access. Link: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/241 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/simple/soft_simple.cpp | 7 ------- 1 file changed, 7 deletions(-)