| Message ID | 20211020154607.180161-3-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Wed, Oct 20, 2021 at 05:45:56PM +0200, Jean-Michel Hautbois wrote: > The AGC frame context needs to be initialised correctly for the first > iteration. > > Set the gain and exposure appropriately to the current values known to > the IPA. This patch has changed a lot compared to v1, the commit message needs to be updated to explain what you're now doing. > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 8 ++++- > src/ipa/ipu3/ipa_context.h | 9 +++++ > src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++---- > 3 files changed, 73 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 970ec424..a001e349 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -60,7 +60,13 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > / configInfo.sensorInfo.pixelRate; > - maxExposureTime_ = kMaxExposure * lineDuration_; > + maxExposureTime_ = context.configuration.agc.maxShutterSpeed; > + > + /* Configure the default exposure and gain */ s/gain/gain./ > + context.frameContext.agc.gain = > + context.configuration.agc.minAnalogueGain; > + context.frameContext.agc.exposure = > + context.configuration.agc.minShutterSpeed / lineDuration_; > > return 0; > } > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 5bab684c..847c03fe 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -10,6 +10,8 @@ > > #include <linux/intel-ipu3.h> > > +#include <libcamera/base/utils.h> > + > #include <libcamera/geometry.h> > > namespace libcamera { > @@ -22,6 +24,13 @@ struct IPASessionConfiguration { > Size bdsOutputSize; > uint32_t stride; > } grid; > + > + struct { > + utils::Duration minShutterSpeed; > + utils::Duration maxShutterSpeed; > + double minAnalogueGain; > + double maxAnalogueGain; > + } agc; > }; > > struct IPAFrameContext { > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 388f1902..36ca83f5 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -97,6 +97,23 @@ > * \brief Number of cells on one line including the ImgU padding > */ > > +/** > + * \struct IPASessionConfiguration::agc > + * \brief AGC parameters configuration of the IPA > + * > + * \var IPASessionConfiguration::agc::minShutterSpeed > + * \brief Minimum shutter speed supported with the configured sensor > + * > + * \var IPASessionConfiguration::grid::maxShutterSpeed > + * \brief Maximum shutter speed supported with the configured sensor > + * > + * \var IPASessionConfiguration::grid::minAnalogueGain > + * \brief Minimum analogue gain supported with the configured sensor > + * > + * \var IPASessionConfiguration::grid::maxAnalogueGain > + * \brief Maximum analogue gain supported with the configured sensor > + */ > + > /** > * \struct IPAFrameContext::agc > * \brief Context for the Automatic Gain Control algorithm > @@ -158,6 +175,8 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(IPAIPU3) > > +using namespace std::literals::chrono_literals; > + > namespace ipa::ipu3 { > > class IPAIPU3 : public IPAIPU3Interface > @@ -182,6 +201,8 @@ private: > void updateControls(const IPACameraSensorInfo &sensorInfo, > const ControlInfoMap &sensorControls, > ControlInfoMap *ipaControls); > + void updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo, > + const ControlInfoMap &sensorControls); > void processControls(unsigned int frame, const ControlList &controls); > void fillParams(unsigned int frame, ipu3_uapi_params *params); > void parseStatistics(unsigned int frame, > @@ -216,6 +237,36 @@ private: > struct IPAContext context_; > }; > > +/* > + * Compute IPASessionConfiguration using the sensor information and the sensor > + * v4l2 controls. > + */ > +void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo, > + const ControlInfoMap &sensorControls) > +{ > + 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>(); > + > + utils::Duration lineDuration = sensorInfo.lineLength * 1.0s > + / sensorInfo.pixelRate; > + > + 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>(); > + > + /* > + * When the AGC computes the new exposure values for a frame, it needs > + * to know the limits for shutter speed and analogue gain. > + * As it depends on the sensor, update it with the controls. > + * > + * \todo take VBLANK into account for maximum shutter speed > + */ > + context_.configuration.agc.minShutterSpeed = minExposure * lineDuration; > + context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration; > + context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); > + context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); > +} > > /* > * Compute camera controls using the sensor information and the sensor > @@ -436,15 +487,18 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, > > calculateBdsGrid(configInfo.bdsOutputSize); > > + /* Update the camera controls using the new sensor settings. */ > + updateControls(sensorInfo_, ctrls_, ipaControls); > + > + /* Update the IPASessionConfiguration using the sensor settings */ s/settings/settings./ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + updateSessionConfiguration(sensorInfo_, ctrls_); > + > for (auto const &algo : algorithms_) { > int ret = algo->configure(context_, configInfo); > if (ret) > return ret; > } > > - /* Update the camera controls using the new sensor settings. */ > - updateControls(sensorInfo_, ctrls_, ipaControls); > - > return 0; > } > > @@ -543,10 +597,6 @@ void IPAIPU3::parseStatistics(unsigned int frame, > { > ControlList ctrls(controls::controls); > > - /* \todo These fields should not be written by the IPAIPU3 layer */ > - context_.frameContext.agc.gain = camHelper_->gain(gain_); > - context_.frameContext.agc.exposure = exposure_; > - > for (auto const &algo : algorithms_) > algo->process(context_, stats); >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 970ec424..a001e349 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -60,7 +60,13 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s / configInfo.sensorInfo.pixelRate; - maxExposureTime_ = kMaxExposure * lineDuration_; + maxExposureTime_ = context.configuration.agc.maxShutterSpeed; + + /* Configure the default exposure and gain */ + context.frameContext.agc.gain = + context.configuration.agc.minAnalogueGain; + context.frameContext.agc.exposure = + context.configuration.agc.minShutterSpeed / lineDuration_; return 0; } diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 5bab684c..847c03fe 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -10,6 +10,8 @@ #include <linux/intel-ipu3.h> +#include <libcamera/base/utils.h> + #include <libcamera/geometry.h> namespace libcamera { @@ -22,6 +24,13 @@ struct IPASessionConfiguration { Size bdsOutputSize; uint32_t stride; } grid; + + struct { + utils::Duration minShutterSpeed; + utils::Duration maxShutterSpeed; + double minAnalogueGain; + double maxAnalogueGain; + } agc; }; struct IPAFrameContext { diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 388f1902..36ca83f5 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -97,6 +97,23 @@ * \brief Number of cells on one line including the ImgU padding */ +/** + * \struct IPASessionConfiguration::agc + * \brief AGC parameters configuration of the IPA + * + * \var IPASessionConfiguration::agc::minShutterSpeed + * \brief Minimum shutter speed supported with the configured sensor + * + * \var IPASessionConfiguration::grid::maxShutterSpeed + * \brief Maximum shutter speed supported with the configured sensor + * + * \var IPASessionConfiguration::grid::minAnalogueGain + * \brief Minimum analogue gain supported with the configured sensor + * + * \var IPASessionConfiguration::grid::maxAnalogueGain + * \brief Maximum analogue gain supported with the configured sensor + */ + /** * \struct IPAFrameContext::agc * \brief Context for the Automatic Gain Control algorithm @@ -158,6 +175,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPAIPU3) +using namespace std::literals::chrono_literals; + namespace ipa::ipu3 { class IPAIPU3 : public IPAIPU3Interface @@ -182,6 +201,8 @@ private: void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); + void updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls); void processControls(unsigned int frame, const ControlList &controls); void fillParams(unsigned int frame, ipu3_uapi_params *params); void parseStatistics(unsigned int frame, @@ -216,6 +237,36 @@ private: struct IPAContext context_; }; +/* + * Compute IPASessionConfiguration using the sensor information and the sensor + * v4l2 controls. + */ +void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo, + const ControlInfoMap &sensorControls) +{ + 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>(); + + utils::Duration lineDuration = sensorInfo.lineLength * 1.0s + / sensorInfo.pixelRate; + + 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>(); + + /* + * When the AGC computes the new exposure values for a frame, it needs + * to know the limits for shutter speed and analogue gain. + * As it depends on the sensor, update it with the controls. + * + * \todo take VBLANK into account for maximum shutter speed + */ + context_.configuration.agc.minShutterSpeed = minExposure * lineDuration; + context_.configuration.agc.maxShutterSpeed = maxExposure * lineDuration; + context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); + context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); +} /* * Compute camera controls using the sensor information and the sensor @@ -436,15 +487,18 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, calculateBdsGrid(configInfo.bdsOutputSize); + /* Update the camera controls using the new sensor settings. */ + updateControls(sensorInfo_, ctrls_, ipaControls); + + /* Update the IPASessionConfiguration using the sensor settings */ + updateSessionConfiguration(sensorInfo_, ctrls_); + for (auto const &algo : algorithms_) { int ret = algo->configure(context_, configInfo); if (ret) return ret; } - /* Update the camera controls using the new sensor settings. */ - updateControls(sensorInfo_, ctrls_, ipaControls); - return 0; } @@ -543,10 +597,6 @@ void IPAIPU3::parseStatistics(unsigned int frame, { ControlList ctrls(controls::controls); - /* \todo These fields should not be written by the IPAIPU3 layer */ - context_.frameContext.agc.gain = camHelper_->gain(gain_); - context_.frameContext.agc.exposure = exposure_; - for (auto const &algo : algorithms_) algo->process(context_, stats);