[libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Fill AGC and AWB metadata in algorithms
Naushir Patuck
naush at raspberrypi.com
Thu Oct 20 18:55:47 CEST 2022
On Thu, 20 Oct 2022 at 17:01, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:
> 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.
>
Ah, that all makes sense now, thanks for explaining!
Naush
> >
> > 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
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221020/99aec33c/attachment.htm>
More information about the libcamera-devel
mailing list