[PATCH v3 2/3] ipa: libipa: agc: Change luminance target to piecewise linear function
Paul Elder
paul.elder at ideasonboard.com
Wed May 29 09:42:38 CEST 2024
On Mon, May 20, 2024 at 02:12:26PM +0100, Dan Scally wrote:
>
> On 17/05/2024 12:47, Paul Elder wrote:
> > On Fri, May 17, 2024 at 12:29:16PM +0200, Dan Scally wrote:
> > > Hi Paul
> > >
> > > On 17/05/2024 10:08, Paul Elder wrote:
> > > > Change the relative luminance target from a scalar value to a piecewise
> > > > linear function that needs to be sampled by the estimate lux value.
> > > >
> > > > Also change the rkisp1 and ipu3 IPAs accordingly, as they use the libipa
> > > > agc. As they both don't yet have lux modules, hardcode them to a single
> > > > lux value for now.
> > > >
> > > > This affects the format of the tuning files, but as there aren't yet any
> > > > this shouldn't be an issue.
> > > >
> > > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > >
> > > Thanks for the patch, this looks good to me. We need to remember the same
> > > thing for the constraint target though I guess.
> > Yeah I just remembered that I had meant to squash that patch in with
> > this, but ig I can send it separately.
>
>
> Actually, one quick thought below...
>
> >
> >
> > Thanks,
> >
> > Paul
> >
> > >
> > > Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> > >
> > > > ---
> > > > No change in v3
> > > >
> > > > Changes in v2:
> > > > - s/FPoint/PointF/
> > > > - add warning when using default relative luminance target when loading
> > > > from the tuning file fails
> > > > ---
> > > > src/ipa/ipu3/algorithms/agc.cpp | 5 ++++-
> > > > src/ipa/libipa/agc_mean_luminance.cpp | 31 +++++++++++++++++++++------
> > > > src/ipa/libipa/agc_mean_luminance.h | 7 +++---
> > > > src/ipa/rkisp1/algorithms/agc.cpp | 5 ++++-
> > > > 4 files changed, 36 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > > > index c9b5548c4..984ed0874 100644
> > > > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > > > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > > > @@ -222,12 +222,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > > > double analogueGain = frameContext.sensor.gain;
> > > > utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> > > > + /* \todo Plumb in the lux value. Requires a lux algo + tuning */
> > > > + double lux = 400;
> > > > +
> > > > utils::Duration shutterTime;
> > > > double aGain, dGain;
> > > > std::tie(shutterTime, aGain, dGain) =
> > > > calculateNewEv(context.activeState.agc.constraintMode,
> > > > context.activeState.agc.exposureMode, hist,
> > > > - effectiveExposureValue);
> > > > + effectiveExposureValue, lux);
> > > > LOG(IPU3Agc, Debug)
> > > > << "Divided up shutter, analogue gain and digital gain are "
> > > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> > > > index 2bf84d05b..fe07777dc 100644
> > > > --- a/src/ipa/libipa/agc_mean_luminance.cpp
> > > > +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> > > > @@ -134,7 +134,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
> > > > */
> > > > AgcMeanLuminance::AgcMeanLuminance()
> > > > - : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
> > > > + : frameCount_(0), filteredExposure_(0s)
> > > > {
> > > > }
> > > > @@ -142,8 +142,16 @@ AgcMeanLuminance::~AgcMeanLuminance() = default;
> > > > void AgcMeanLuminance::parseRelativeLuminanceTarget(const YamlObject &tuningData)
> > > > {
> > > > - relativeLuminanceTarget_ =
> > > > - tuningData["relativeLuminanceTarget"].get<double>(kDefaultRelativeLuminanceTarget);
> > > > + int ret = relativeLuminanceTarget_.readYaml(tuningData["relativeLuminanceTarget"]);
> > > > + if (ret == 0)
> > > > + return;
> > > > +
> > > > + LOG(AgcMeanLuminance, Warning)
> > > > + << "Failed to load tuning parameter 'relativeLuminanceTarget', "
> > > > + << "using default [0, " << kDefaultRelativeLuminanceTarget << "]";
> > > > +
> > > > + std::vector<PointF> points = { { 0, kDefaultRelativeLuminanceTarget } };
> > > > + relativeLuminanceTarget_ = Pwl(points);
>
>
> Does this not need to have two points? The Pwl implementation looks like it
> expects an even number of points with a minimum of 2 - validation would fail
> in the readYaml() if that weren't met.
Indeed it does. I was getting mixed up with my matrix interpolator which
doesn't.
Thanks,
Paul
>
> > > > }
> > > > void AgcMeanLuminance::parseConstraint(const YamlObject &modeDict, int32_t id)
> > > > @@ -421,11 +429,18 @@ void AgcMeanLuminance::setLimits(utils::Duration minShutter,
> > > > /**
> > > > * \brief Estimate the initial gain needed to achieve a relative luminance
> > > > * target
> > > > + * \param[in] lux The lux value at which to sample the luminance target pwl
> > > > + *
> > > > + * To account for non-linearity caused by saturation, the value needs to be
> > > > + * estimated in an iterative process, as multiplying by a gain will not increase
> > > > + * the relative luminance by the same factor if some image regions are saturated
> > > > + *
> > > > * \return The calculated initial gain
> > > > */
> > > > -double AgcMeanLuminance::estimateInitialGain() const
> > > > +double AgcMeanLuminance::estimateInitialGain(double lux) const
> > > > {
> > > > - double yTarget = relativeLuminanceTarget_;
> > > > + double yTarget =
> > > > + relativeLuminanceTarget_.eval(relativeLuminanceTarget_.domain().clamp(lux));
> > > > double yGain = 1.0;
> > > > /*
> > > > @@ -520,6 +535,7 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
> > > > * the calculated gain
> > > > * \param[in] effectiveExposureValue The EV applied to the frame from which the
> > > > * statistics in use derive
> > > > + * \param[in] lux The lux value at which to sample the luminance target pwl
> > > > *
> > > > * Calculate a new exposure value to try to obtain the target. The calculated
> > > > * exposure value is filtered to prevent rapid changes from frame to frame, and
> > > > @@ -531,7 +547,8 @@ std::tuple<utils::Duration, double, double>
> > > > AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
> > > > uint32_t exposureModeIndex,
> > > > const Histogram &yHist,
> > > > - utils::Duration effectiveExposureValue)
> > > > + utils::Duration effectiveExposureValue,
> > > > + double lux)
> > > > {
> > > > /*
> > > > * The pipeline handler should validate that we have received an allowed
> > > > @@ -540,7 +557,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
> > > > std::shared_ptr<ExposureModeHelper> exposureModeHelper =
> > > > exposureModeHelpers_.at(exposureModeIndex);
> > > > - double gain = estimateInitialGain();
> > > > + double gain = estimateInitialGain(lux);
> > > > gain = constraintClampGain(constraintModeIndex, yHist, gain);
> > > > /*
> > > > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> > > > index 0a81c6d28..6ec2a0dc9 100644
> > > > --- a/src/ipa/libipa/agc_mean_luminance.h
> > > > +++ b/src/ipa/libipa/agc_mean_luminance.h
> > > > @@ -18,6 +18,7 @@
> > > > #include "exposure_mode_helper.h"
> > > > #include "histogram.h"
> > > > +#include "pwl.h"
> > > > namespace libcamera {
> > > > @@ -62,7 +63,7 @@ public:
> > > > std::tuple<utils::Duration, double, double>
> > > > calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex,
> > > > - const Histogram &yHist, utils::Duration effectiveExposureValue);
> > > > + const Histogram &yHist, utils::Duration effectiveExposureValue, double lux);
> > > > void resetFrameCount()
> > > > {
> > > > @@ -76,7 +77,7 @@ private:
> > > > void parseConstraint(const YamlObject &modeDict, int32_t id);
> > > > int parseConstraintModes(const YamlObject &tuningData);
> > > > int parseExposureModes(const YamlObject &tuningData);
> > > > - double estimateInitialGain() const;
> > > > + double estimateInitialGain(double lux) const;
> > > > double constraintClampGain(uint32_t constraintModeIndex,
> > > > const Histogram &hist,
> > > > double gain);
> > > > @@ -84,7 +85,7 @@ private:
> > > > uint64_t frameCount_;
> > > > utils::Duration filteredExposure_;
> > > > - double relativeLuminanceTarget_;
> > > > + Pwl relativeLuminanceTarget_;
> > > > std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
> > > > std::map<int32_t, std::shared_ptr<ExposureModeHelper>> exposureModeHelpers_;
> > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > > > index 4af397bdc..1c9872d02 100644
> > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > > > @@ -379,12 +379,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > > > double analogueGain = frameContext.sensor.gain;
> > > > utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> > > > + /* \todo Plumb in the lux value. Requires a lux algo + tuning */
> > > > + double lux = 400;
> > > > +
> > > > utils::Duration shutterTime;
> > > > double aGain, dGain;
> > > > std::tie(shutterTime, aGain, dGain) =
> > > > calculateNewEv(context.activeState.agc.constraintMode,
> > > > context.activeState.agc.exposureMode,
> > > > - hist, effectiveExposureValue);
> > > > + hist, effectiveExposureValue, lux);
> > > > LOG(RkISP1Agc, Debug)
> > > > << "Divided up shutter, analogue gain and digital gain are "
More information about the libcamera-devel
mailing list