[libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB metadata in algorithms
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Oct 20 18:01:19 CEST 2022
Quoting Naushir Patuck via libcamera-devel (2022-10-20 10:25:50)
> Hi Laurent,
>
> This change doesn't affect the RPi platform, but I just wanted to give a
> quick comment.
>
> On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart via libcamera-devel <
> libcamera-devel at lists.libcamera.org> wrote:
>
> > Fill the frame metadata in the AGC and AWB algorithm's prepare()
> > function. This removes the need to fill metadata manually in the IPA
> > module's processStatsBuffer() function.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-
> > src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++
> > src/ipa/ipu3/ipa_context.cpp | 3 +++
> > src/ipa/ipu3/ipa_context.h | 1 +
> > src/ipa/ipu3/ipu3.cpp | 13 +------------
> > 5 files changed, 30 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp
> > b/src/ipa/ipu3/algorithms/agc.cpp
> > index f4e559bf8b84..b5309bdbea25 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -14,6 +14,7 @@
> > #include <libcamera/base/log.h>
> > #include <libcamera/base/utils.h>
> >
> > +#include <libcamera/control_ids.h>
> > #include <libcamera/ipa/core_ipa_interface.h>
> >
> > #include "libipa/histogram.h"
> > @@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState
> > &activeState,
> > void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t
> > frame,
> > IPAFrameContext &frameContext,
> > const ipu3_uapi_stats_3a *stats,
> > - [[maybe_unused]] ControlList &metadata)
> > + ControlList &metadata)
> > {
> > /*
> > * Estimate the gain needed to have the proportion of pixels in a
> > given
> > @@ -365,6 +366,19 @@ void Agc::process(IPAContext &context,
> > [[maybe_unused]] const uint32_t frame,
> >
> > computeExposure(context, frameContext, yGain, iqMeanGain);
> > frameCount_++;
> > +
> > + utils::Duration exposureTime =
> > context.configuration.sensor.lineDuration
> > + * frameContext.sensor.exposure;
> > + metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > + metadata.set(controls::ExposureTime,
> > exposureTime.get<std::micro>());
> >
>
> Putting the metadata.set() calls into the algorithm source code like above
> would
> mean that the metadata is populated with the values the AGC requested. This
> could be wrong because:
The frame context stores multiple copies of the exposure and gain.
frameContext.sensor.{exposure,gain} are the values that are determined
to have been set according to delayed controls. These get set currently
in
void IPAIPU3::processStatsBuffer(const uint32_t frame,
[[maybe_unused]] const int64_t frameTimestamp,
const uint32_t bufferId, const ControlList &sensorControls)
It looks like we don't actually store what the algorithm specifically
wanted on that frame, so we're only storing what the agc 'currently'
would like (in the active state) and here we get from delayed controls
what was 'actually' set for the frame.
So I believe it's correct.
>
> 1) If the sensor quantises the gain and/or shutter speed values from what
> AGC asked for.
> 2) Unless I am mistaken, this does not account for sensor delays.
The results for the frame are passed from the pipeline handler to the
IPA from delayed controls. So it should all be handled.
> I am assuming the metadata list here is what is returned back via the
> completed
> request, is that correct? Of course, my understanding of this change could
> be completely
> wrong, so please ignore the noise :-)
Yes, this completed metadata is what the application should receive.
In this instance, it's a bit long winded, as the exposure/gains are
being sent from the pipeline handler, into the IPA, stored in the frame
context.sensor{} ... and then assigned to the metadata by the AGC
algorithm, which then returns the completed metadata back to the
pipeline handler for the request to be delivered to the application.
--
Kieran
>
> Regards,
> Naush
>
>
> > +
> > + /* \todo Use VBlank value calculated from each frame exposure. */
> > + uint32_t vTotal = context.configuration.sensor.size.height
> > + + context.configuration.sensor.defVBlank;
> > + utils::Duration frameDuration =
> > context.configuration.sensor.lineDuration
> > + * vTotal;
> > + metadata.set(controls::FrameDuration,
> > frameDuration.get<std::micro>());
> > +
> > }
> >
> > REGISTER_IPA_ALGORITHM(Agc, "Agc")
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp
> > b/src/ipa/ipu3/algorithms/awb.cpp
> > index 6452b6a1acd2..dd7ebc07c534 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -11,6 +11,8 @@
> >
> > #include <libcamera/base/log.h>
> >
> > +#include <libcamera/control_ids.h>
> > +
> > /**
> > * \file awb.h
> > */
> > @@ -403,6 +405,14 @@ void Awb::process(IPAContext &context,
> > [[maybe_unused]] const uint32_t frame,
> > context.activeState.awb.gains.green = asyncResults_.greenGain;
> > context.activeState.awb.gains.red = asyncResults_.redGain;
> > context.activeState.awb.temperatureK = asyncResults_.temperatureK;
> > +
> > + metadata.set(controls::AwbEnable, true);
> > + metadata.set(controls::ColourGains, {
> > +
> > static_cast<float>(context.activeState.awb.gains.red),
> > +
> > static_cast<float>(context.activeState.awb.gains.blue)
> > + });
> > + metadata.set(controls::ColourTemperature,
> > + context.activeState.awb.temperatureK);
> > }
> >
> > constexpr uint16_t Awb::threshold(float value)
> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > index 68f017b04751..959f314f5452 100644
> > --- a/src/ipa/ipu3/ipa_context.cpp
> > +++ b/src/ipa/ipu3/ipa_context.cpp
> > @@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {
> > *
> > * \var IPASessionConfiguration::sensor.defVBlank
> > * \brief The default vblank value of the sensor
> > + *
> > + * \var IPASessionConfiguration::sensor.size
> > + * \brief Sensor output resolution
> > */
> >
> > /**
> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > index 36099353e9f2..e9a3863b1693 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -41,6 +41,7 @@ struct IPASessionConfiguration {
> > struct {
> > int32_t defVBlank;
> > utils::Duration lineDuration;
> > + Size size;
> > } sensor;
> > };
> >
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index bc0f6007baca..a9a2b49ca95b 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> > /* Initialise the sensor configuration. */
> > context_.configuration.sensor.lineDuration =
> > sensorInfo_.minLineLength
> > * 1.0s /
> > sensorInfo_.pixelRate;
> > + context_.configuration.sensor.size = sensorInfo_.outputSize;
> >
> > /*
> > * Compute the sensor V4L2 controls to be used by the algorithms
> > and
> > @@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> > frameContext.sensor.exposure =
> > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > frameContext.sensor.gain =
> > camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> >
> > - double lineDuration =
> > context_.configuration.sensor.lineDuration.get<std::micro>();
> > - int32_t vBlank = context_.configuration.sensor.defVBlank;
> > ControlList metadata(controls::controls);
> >
> > for (auto const &algo : algorithms())
> > @@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
> >
> > setControls(frame);
> >
> > - /* \todo Use VBlank value calculated from each frame exposure. */
> > - int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) *
> > lineDuration;
> > - metadata.set(controls::FrameDuration, frameDuration);
> > -
> > - metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > -
> > - metadata.set(controls::ColourTemperature,
> > context_.activeState.awb.temperatureK);
> > -
> > - metadata.set(controls::ExposureTime, frameContext.sensor.exposure
> > * lineDuration);
> > -
> > /*
> > * \todo The Metadata provides a path to getting extended data
> > * out to the application. Further data such as a simplifed
> > Histogram
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> >
More information about the libcamera-devel
mailing list