Message ID | 20250117143410.20363-4-dan.scally@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Dan, Thank you for the patch. On Fri, Jan 17, 2025 at 02:34:10PM +0000, Daniel Scally wrote: > Add controls for AeFlickerMode and AeFlickerPeriod to the > AgcMeanLuminance class. The controls passed to an algorithm's > queueRequest() are forwarded to AgcMeanLuminance through a new > function, and the values then passed to the ExposureModeHelper to be > used in calculating the new exposure time. > > Take the opportunity to call the new parseControls() function in each > of the derived class' queueRequest() function. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 3 +- > src/ipa/libipa/agc_mean_luminance.cpp | 47 ++++++++++++++++++++++++++- > src/ipa/libipa/agc_mean_luminance.h | 5 +++ > src/ipa/mali-c55/algorithms/agc.cpp | 2 ++ > src/ipa/rkisp1/algorithms/agc.cpp | 2 ++ > 5 files changed, 57 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 383b046c..b993aaa7 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -129,8 +129,9 @@ int Agc::configure(IPAContext &context, > void Agc::queueRequest([[maybe_unused]] typename Module::Context &context, > [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] typename Module::FrameContext &frameContext, > - [[maybe_unused]] const ControlList &controls) > + const ControlList &controls) > { > + parseControls(controls); > } > > Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats, > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index b5e6afe3..4acf7b55 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -136,6 +136,10 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; > AgcMeanLuminance::AgcMeanLuminance() > : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0) > { > + controls_[&controls::AeFlickerMode] = ControlInfo(static_cast<int>(controls::FlickerOff), > + static_cast<int>(controls::FlickerManual), > + static_cast<int>(controls::FlickerOff)); > + controls_[&controls::AeFlickerPeriod] = ControlInfo(100, 1000000); > } > > AgcMeanLuminance::~AgcMeanLuminance() = default; > @@ -479,6 +483,39 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, > return gain; > } > > +/** > + * \brief Parse the controls passed to an algorithm for the ones we need > + * \param[in] controls the ControlList passed to an algorithm by the IPA > + * > + * This function must be called by a derived class in its queueRequest() > + * function so that we can extract the controls needed by this base class. > + */ > +void AgcMeanLuminance::parseControls(const ControlList &controls) > +{ > + const auto &flickerMode = controls.get(controls::AeFlickerMode); > + if (flickerMode) { > + switch (*flickerMode) { > + case controls::AeFlickerModeEnum::FlickerOff: > + case controls::AeFlickerModeEnum::FlickerManual: > + flickerMode_ = static_cast<controls::AeFlickerModeEnum>(*flickerMode); > + break; > + default: > + LOG(AgcMeanLuminance, Error) > + << "Flicker mode " << *flickerMode << " is not supported"; > + break; > + } > + } > + > + const auto &flickerPeriod = controls.get(controls::AeFlickerPeriod); > + if (flickerPeriod) { > + /* > + * If at some future point we support automatic flicker > + * mitigation then this will need revision. > + */ > + flickerPeriod_ = *flickerPeriod * 1.0us; > + } > +} > + > /** > * \brief Apply a filter on the exposure value to limit the speed of changes > * \param[in] exposureValue The target exposure from the AGC algorithm > @@ -560,7 +597,15 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, > newExposureValue = filterExposure(newExposureValue); > > frameCount_++; > - return exposureModeHelper->splitExposure(newExposureValue, 0us); > + > + utils::Duration flickerPeriod; > + > + if (flickerMode_ == controls::AeFlickerModeEnum::FlickerManual) > + flickerPeriod = flickerPeriod_; > + else > + flickerPeriod = 0us; > + > + return exposureModeHelper->splitExposure(newExposureValue, flickerPeriod); > } > > /** > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h > index c41391cb..b9b36687 100644 > --- a/src/ipa/libipa/agc_mean_luminance.h > +++ b/src/ipa/libipa/agc_mean_luminance.h > @@ -14,6 +14,7 @@ > > #include <libcamera/base/utils.h> > > +#include <libcamera/control_ids.h> > #include <libcamera/controls.h> > > #include "libcamera/internal/yaml_parser.h" > @@ -71,6 +72,8 @@ public: > frameCount_ = 0; > } > > + void parseControls(const ControlList &controls); > + > private: > virtual double estimateLuminance(const double gain) const = 0; > > @@ -87,6 +90,8 @@ private: > uint64_t frameCount_; > utils::Duration filteredExposure_; > double relativeLuminanceTarget_; > + utils::Duration flickerPeriod_; > + controls::AeFlickerModeEnum flickerMode_; These can probably be replaced by a std::optional<utils::Duration> flickerPeriod_; too. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > std::map<int32_t, std::vector<AgcConstraint>> constraintModes_; > std::map<int32_t, std::shared_ptr<ExposureModeHelper>> exposureModeHelpers_; > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp > index 70667db3..6a80c44f 100644 > --- a/src/ipa/mali-c55/algorithms/agc.cpp > +++ b/src/ipa/mali-c55/algorithms/agc.cpp > @@ -238,6 +238,8 @@ void Agc::queueRequest(IPAContext &context, const uint32_t frame, > << "Digital gain set to " << agc.manual.ispGain > << " on request sequence " << frame; > } > + > + parseControls(controls); > } > > size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 40e5a8f4..7b1763a2 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -275,6 +275,8 @@ void Agc::queueRequest(IPAContext &context, > agc.maxFrameDuration = maxFrameDuration; > } > frameContext.agc.maxFrameDuration = agc.maxFrameDuration; > + > + parseControls(controls); > } > > /**
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 383b046c..b993aaa7 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -129,8 +129,9 @@ int Agc::configure(IPAContext &context, void Agc::queueRequest([[maybe_unused]] typename Module::Context &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] typename Module::FrameContext &frameContext, - [[maybe_unused]] const ControlList &controls) + const ControlList &controls) { + parseControls(controls); } Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats, diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index b5e6afe3..4acf7b55 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -136,6 +136,10 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; AgcMeanLuminance::AgcMeanLuminance() : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0) { + controls_[&controls::AeFlickerMode] = ControlInfo(static_cast<int>(controls::FlickerOff), + static_cast<int>(controls::FlickerManual), + static_cast<int>(controls::FlickerOff)); + controls_[&controls::AeFlickerPeriod] = ControlInfo(100, 1000000); } AgcMeanLuminance::~AgcMeanLuminance() = default; @@ -479,6 +483,39 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, return gain; } +/** + * \brief Parse the controls passed to an algorithm for the ones we need + * \param[in] controls the ControlList passed to an algorithm by the IPA + * + * This function must be called by a derived class in its queueRequest() + * function so that we can extract the controls needed by this base class. + */ +void AgcMeanLuminance::parseControls(const ControlList &controls) +{ + const auto &flickerMode = controls.get(controls::AeFlickerMode); + if (flickerMode) { + switch (*flickerMode) { + case controls::AeFlickerModeEnum::FlickerOff: + case controls::AeFlickerModeEnum::FlickerManual: + flickerMode_ = static_cast<controls::AeFlickerModeEnum>(*flickerMode); + break; + default: + LOG(AgcMeanLuminance, Error) + << "Flicker mode " << *flickerMode << " is not supported"; + break; + } + } + + const auto &flickerPeriod = controls.get(controls::AeFlickerPeriod); + if (flickerPeriod) { + /* + * If at some future point we support automatic flicker + * mitigation then this will need revision. + */ + flickerPeriod_ = *flickerPeriod * 1.0us; + } +} + /** * \brief Apply a filter on the exposure value to limit the speed of changes * \param[in] exposureValue The target exposure from the AGC algorithm @@ -560,7 +597,15 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, newExposureValue = filterExposure(newExposureValue); frameCount_++; - return exposureModeHelper->splitExposure(newExposureValue, 0us); + + utils::Duration flickerPeriod; + + if (flickerMode_ == controls::AeFlickerModeEnum::FlickerManual) + flickerPeriod = flickerPeriod_; + else + flickerPeriod = 0us; + + return exposureModeHelper->splitExposure(newExposureValue, flickerPeriod); } /** diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h index c41391cb..b9b36687 100644 --- a/src/ipa/libipa/agc_mean_luminance.h +++ b/src/ipa/libipa/agc_mean_luminance.h @@ -14,6 +14,7 @@ #include <libcamera/base/utils.h> +#include <libcamera/control_ids.h> #include <libcamera/controls.h> #include "libcamera/internal/yaml_parser.h" @@ -71,6 +72,8 @@ public: frameCount_ = 0; } + void parseControls(const ControlList &controls); + private: virtual double estimateLuminance(const double gain) const = 0; @@ -87,6 +90,8 @@ private: uint64_t frameCount_; utils::Duration filteredExposure_; double relativeLuminanceTarget_; + utils::Duration flickerPeriod_; + controls::AeFlickerModeEnum flickerMode_; std::map<int32_t, std::vector<AgcConstraint>> constraintModes_; std::map<int32_t, std::shared_ptr<ExposureModeHelper>> exposureModeHelpers_; diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp index 70667db3..6a80c44f 100644 --- a/src/ipa/mali-c55/algorithms/agc.cpp +++ b/src/ipa/mali-c55/algorithms/agc.cpp @@ -238,6 +238,8 @@ void Agc::queueRequest(IPAContext &context, const uint32_t frame, << "Digital gain set to " << agc.manual.ispGain << " on request sequence " << frame; } + + parseControls(controls); } size_t Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 40e5a8f4..7b1763a2 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -275,6 +275,8 @@ void Agc::queueRequest(IPAContext &context, agc.maxFrameDuration = maxFrameDuration; } frameContext.agc.maxFrameDuration = agc.maxFrameDuration; + + parseControls(controls); } /**
Add controls for AeFlickerMode and AeFlickerPeriod to the AgcMeanLuminance class. The controls passed to an algorithm's queueRequest() are forwarded to AgcMeanLuminance through a new function, and the values then passed to the ExposureModeHelper to be used in calculating the new exposure time. Take the opportunity to call the new parseControls() function in each of the derived class' queueRequest() function. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 3 +- src/ipa/libipa/agc_mean_luminance.cpp | 47 ++++++++++++++++++++++++++- src/ipa/libipa/agc_mean_luminance.h | 5 +++ src/ipa/mali-c55/algorithms/agc.cpp | 2 ++ src/ipa/rkisp1/algorithms/agc.cpp | 2 ++ 5 files changed, 57 insertions(+), 2 deletions(-)