<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>This change doesn't affect the RPi platform, but I just wanted to give a quick comment.</div><div><br></div></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 19 Oct 2022 at 21:56, Laurent Pinchart via libcamera-devel <<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Fill the frame metadata in the AGC and AWB algorithm's prepare()<br>
function. This removes the need to fill metadata manually in the IPA<br>
module's processStatsBuffer() function.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
src/ipa/ipu3/algorithms/agc.cpp | 16 +++++++++++++++-<br>
src/ipa/ipu3/algorithms/awb.cpp | 10 ++++++++++<br>
src/ipa/ipu3/ipa_context.cpp | 3 +++<br>
src/ipa/ipu3/ipa_context.h | 1 +<br>
src/ipa/ipu3/ipu3.cpp | 13 +------------<br>
5 files changed, 30 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp<br>
index f4e559bf8b84..b5309bdbea25 100644<br>
--- a/src/ipa/ipu3/algorithms/agc.cpp<br>
+++ b/src/ipa/ipu3/algorithms/agc.cpp<br>
@@ -14,6 +14,7 @@<br>
#include <libcamera/base/log.h><br>
#include <libcamera/base/utils.h><br>
<br>
+#include <libcamera/control_ids.h><br>
#include <libcamera/ipa/core_ipa_interface.h><br>
<br>
#include "libipa/histogram.h"<br>
@@ -328,7 +329,7 @@ double Agc::estimateLuminance(IPAActiveState &activeState,<br>
void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,<br>
IPAFrameContext &frameContext,<br>
const ipu3_uapi_stats_3a *stats,<br>
- [[maybe_unused]] ControlList &metadata)<br>
+ ControlList &metadata)<br>
{<br>
/*<br>
* Estimate the gain needed to have the proportion of pixels in a given<br>
@@ -365,6 +366,19 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,<br>
<br>
computeExposure(context, frameContext, yGain, iqMeanGain);<br>
frameCount_++;<br>
+<br>
+ utils::Duration exposureTime = context.configuration.sensor.lineDuration<br>
+ * frameContext.sensor.exposure;<br>
+ metadata.set(controls::AnalogueGain, frameContext.sensor.gain);<br>
+ metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());<br></blockquote><div><br></div><div>Putting the metadata.set() calls into the algorithm source code like above would</div><div>mean that the metadata is populated with the values the AGC requested. This</div><div>could be wrong because:</div><div><br></div><div>1) If the sensor quantises the gain and/or shutter speed values from what AGC asked for.</div><div>2) Unless I am mistaken, this does not account for sensor delays.<br></div><div><br></div><div>I am assuming the metadata list here is what is returned back via the completed</div><div>request, is that correct? Of course, my understanding of this change could be completely</div><div>wrong, so please ignore the noise :-)</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ /* \todo Use VBlank value calculated from each frame exposure. */<br>
+ uint32_t vTotal = context.configuration.sensor.size.height<br>
+ + context.configuration.sensor.defVBlank;<br>
+ utils::Duration frameDuration = context.configuration.sensor.lineDuration<br>
+ * vTotal;<br>
+ metadata.set(controls::FrameDuration, frameDuration.get<std::micro>());<br>
+<br>
}<br>
<br>
REGISTER_IPA_ALGORITHM(Agc, "Agc")<br>
diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp<br>
index 6452b6a1acd2..dd7ebc07c534 100644<br>
--- a/src/ipa/ipu3/algorithms/awb.cpp<br>
+++ b/src/ipa/ipu3/algorithms/awb.cpp<br>
@@ -11,6 +11,8 @@<br>
<br>
#include <libcamera/base/log.h><br>
<br>
+#include <libcamera/control_ids.h><br>
+<br>
/**<br>
* \file awb.h<br>
*/<br>
@@ -403,6 +405,14 @@ void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,<br>
context.activeState.awb.gains.green = asyncResults_.greenGain;<br>
context.activeState.awb.gains.red = asyncResults_.redGain;<br>
context.activeState.awb.temperatureK = asyncResults_.temperatureK;<br>
+<br>
+ metadata.set(controls::AwbEnable, true);<br>
+ metadata.set(controls::ColourGains, {<br>
+ static_cast<float>(context.activeState.awb.gains.red),<br>
+ static_cast<float>(context.activeState.awb.gains.blue)<br>
+ });<br>
+ metadata.set(controls::ColourTemperature,<br>
+ context.activeState.awb.temperatureK);<br>
}<br>
<br>
constexpr uint16_t Awb::threshold(float value)<br>
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp<br>
index 68f017b04751..959f314f5452 100644<br>
--- a/src/ipa/ipu3/ipa_context.cpp<br>
+++ b/src/ipa/ipu3/ipa_context.cpp<br>
@@ -111,6 +111,9 @@ namespace libcamera::ipa::ipu3 {<br>
*<br>
* \var IPASessionConfiguration::sensor.defVBlank<br>
* \brief The default vblank value of the sensor<br>
+ *<br>
+ * \var IPASessionConfiguration::sensor.size<br>
+ * \brief Sensor output resolution<br>
*/<br>
<br>
/**<br>
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h<br>
index 36099353e9f2..e9a3863b1693 100644<br>
--- a/src/ipa/ipu3/ipa_context.h<br>
+++ b/src/ipa/ipu3/ipa_context.h<br>
@@ -41,6 +41,7 @@ struct IPASessionConfiguration {<br>
struct {<br>
int32_t defVBlank;<br>
utils::Duration lineDuration;<br>
+ Size size;<br>
} sensor;<br>
};<br>
<br>
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp<br>
index bc0f6007baca..a9a2b49ca95b 100644<br>
--- a/src/ipa/ipu3/ipu3.cpp<br>
+++ b/src/ipa/ipu3/ipu3.cpp<br>
@@ -502,6 +502,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,<br>
/* Initialise the sensor configuration. */<br>
context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength<br>
* 1.0s / sensorInfo_.pixelRate;<br>
+ context_.configuration.sensor.size = sensorInfo_.outputSize;<br>
<br>
/*<br>
* Compute the sensor V4L2 controls to be used by the algorithms and<br>
@@ -628,8 +629,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,<br>
frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();<br>
frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());<br>
<br>
- double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();<br>
- int32_t vBlank = context_.configuration.sensor.defVBlank;<br>
ControlList metadata(controls::controls);<br>
<br>
for (auto const &algo : algorithms())<br>
@@ -637,16 +636,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,<br>
<br>
setControls(frame);<br>
<br>
- /* \todo Use VBlank value calculated from each frame exposure. */<br>
- int64_t frameDuration = (vBlank + sensorInfo_.outputSize.height) * lineDuration;<br>
- metadata.set(controls::FrameDuration, frameDuration);<br>
-<br>
- metadata.set(controls::AnalogueGain, frameContext.sensor.gain);<br>
-<br>
- metadata.set(controls::ColourTemperature, context_.activeState.awb.temperatureK);<br>
-<br>
- metadata.set(controls::ExposureTime, frameContext.sensor.exposure * lineDuration);<br>
-<br>
/*<br>
* \todo The Metadata provides a path to getting extended data<br>
* out to the application. Further data such as a simplifed Histogram<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>