| Message ID | 20220305172849.1022923-4-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Quoting Umang Jain via libcamera-devel (2022-03-05 17:28:49) > Add a control-not-found error checking for the controls being > queried in IPAIPU3::updateSessionConfiguration(). > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/ipu3/ipu3.cpp | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 0df358ba..583d6b9a 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -180,16 +180,31 @@ private: > */ > void 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); I'm torn whether we should have a single list of mandatory controls that we check at init()/configure() or this which at least checks and obtains the control in one loop. But I note that this function is returning void, and so you are not returning an error if the controls are not found - So I think that's bad. It needs ot have the error propogated back up - ... so that means I think a single check in configure() (or if we can during init()) would be better. > + if (itVBlank == sensorControls.end()) { > + LOG(IPAIPU3, Error) << "Can't find vblank control"; > + return; > + } > > - 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; > + } > + > + int32_t minExposure = itExp->second.min().get<int32_t>(); > + int32_t maxExposure = itExp->second.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; > + } > > - 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>(); > + 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 > -- > 2.31.0 >
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 0df358ba..583d6b9a 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -180,16 +180,31 @@ private: */ void 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; + } - 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; + } + + int32_t minExposure = itExp->second.min().get<int32_t>(); + int32_t maxExposure = itExp->second.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; + } - 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>(); + 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
Add a control-not-found error checking for the controls being queried in IPAIPU3::updateSessionConfiguration(). Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/ipa/ipu3/ipu3.cpp | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)