<div dir="ltr"><div dir="ltr">Hi David,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 16 Nov 2020 at 16:49, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</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">Introduce a function to fetch the AwbStatus (fetchAwbStatus), and call<br>
it unconditionally at the top of Prepare so that both Prepare and<br>
Process know thereafter that it's been done.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
src/ipa/raspberrypi/controller/rpi/agc.cpp | 37 ++++++++++++----------<br>
src/ipa/raspberrypi/controller/rpi/agc.hpp | 5 +--<br>
2 files changed, 23 insertions(+), 19 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
index ead28398..6de56def 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
@@ -5,6 +5,8 @@<br>
* agc.cpp - AGC/AEC control algorithm<br>
*/<br>
<br>
+#include <assert.h><br></blockquote><div><br></div><div>Perhaps you can use the ASSERT() macro defined in libcamera (log.h)? I think all code under libcamera is meant to use it instead of the standard one.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
#include <map><br>
<br>
#include "linux/bcm2835-isp.h"<br>
@@ -235,6 +237,8 @@ void Agc::Prepare(Metadata *image_metadata)<br>
int lock_count = lock_count_;<br>
lock_count_ = 0;<br>
status_.digital_gain = 1.0;<br>
+ fetchAwbStatus(image_metadata); // always fetch it so that Process knows it's been done<br>
+<br>
if (status_.total_exposure_value) {<br>
// Process has run, so we have meaningful values.<br>
DeviceStatus device_status;<br>
@@ -301,7 +305,7 @@ void Agc::Process(StatisticsPtr &stats, Metadata *image_metadata)<br>
// Some of the exposure has to be applied as digital gain, so work out<br>
// what that is. This function also tells us whether it's decided to<br>
// "desaturate" the image more quickly.<br>
- bool desaturate = applyDigitalGain(image_metadata, gain, target_Y);<br>
+ bool desaturate = applyDigitalGain(gain, target_Y);<br>
// The results have to be filtered so as not to change too rapidly.<br>
filterExposure(desaturate);<br>
// The last thing is to divide up the exposure value into a shutter time<br>
@@ -378,14 +382,19 @@ void Agc::fetchCurrentExposure(Metadata *image_metadata)<br>
current_.total_exposure_no_dg = current_.shutter * current_.analogue_gain;<br>
}<br>
<br>
-static double compute_initial_Y(bcm2835_isp_stats *stats, Metadata *image_metadata,<br>
+void Agc::fetchAwbStatus(Metadata *image_metadata)<br>
+{<br>
+ awb_.gain_r = 1.0; // in case not found in metadata<br>
+ awb_.gain_g = 1.0;<br>
+ awb_.gain_b = 1.0;<br>
+ if (image_metadata->Get("awb.status", awb_) != 0)<br>
+ LOG(RPiAgc, Warning) << "Agc: no AWB status found";<br>
+}<br>
+<br>
+static double compute_initial_Y(bcm2835_isp_stats *stats, AwbStatus const &awb,<br>
double weights[])<br>
{<br>
bcm2835_isp_stats_region *regions = stats->agc_stats;<br>
- struct AwbStatus awb;<br>
- awb.gain_r = awb.gain_g = awb.gain_b = 1.0; // in case no metadata<br>
- if (image_metadata->Get("awb.status", awb) != 0)<br>
- LOG(RPiAgc, Warning) << "Agc: no AWB status found";<br>
// Note how the calculation below means that equal weights give you<br>
// "average" metering (i.e. all pixels equally important).<br>
double R_sum = 0, G_sum = 0, B_sum = 0, pixel_sum = 0;<br>
@@ -437,7 +446,7 @@ void Agc::computeGain(bcm2835_isp_stats *statistics, Metadata *image_metadata,<br>
target_Y =<br>
config_.Y_target.Eval(config_.Y_target.Domain().Clip(lux.lux));<br>
target_Y = std::min(EV_GAIN_Y_TARGET_LIMIT, target_Y * ev_gain);<br>
- double initial_Y = compute_initial_Y(statistics, image_metadata,<br>
+ double initial_Y = compute_initial_Y(statistics, awb_,<br>
metering_mode_->weights);<br>
gain = std::min(10.0, target_Y / (initial_Y + .001));<br>
LOG(RPiAgc, Debug) << "Initially Y " << initial_Y << " target " << target_Y<br>
@@ -483,19 +492,13 @@ void Agc::computeTargetExposure(double gain)<br>
LOG(RPiAgc, Debug) << "Target total_exposure " << target_.total_exposure;<br>
}<br>
<br>
-bool Agc::applyDigitalGain(Metadata *image_metadata, double gain,<br>
- double target_Y)<br>
+bool Agc::applyDigitalGain(double gain, double target_Y)<br>
{<br>
- double dg = 1.0;<br>
+ double min_colour_gain = std::min({ awb_.gain_r, awb_.gain_g, awb_.gain_b, 1.0 });<br>
+ assert(min_colour_gain != 0.0);<br>
+ double dg = 1.0 / min_colour_gain;<br>
// I think this pipeline subtracts black level and rescales before we<br>
// get the stats, so no need to worry about it.<br>
- struct AwbStatus awb;<br>
- if (image_metadata->Get("awb.status", awb) == 0) {<br>
- double min_gain = std::min(awb.gain_r,<br>
- std::min(awb.gain_g, awb.gain_b));<br>
- dg *= std::max(1.0, 1.0 / min_gain);<br>
- } else<br>
- LOG(RPiAgc, Warning) << "Agc: no AWB status found";<br>
LOG(RPiAgc, Debug) << "after AWB, target dg " << dg << " gain " << gain<br>
<< " target_Y " << target_Y;<br>
// Finally, if we're trying to reduce exposure but the target_Y is<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
index 2442fc03..e7ac480f 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
@@ -83,11 +83,11 @@ private:<br>
AgcConfig config_;<br>
void housekeepConfig();<br>
void fetchCurrentExposure(Metadata *image_metadata);<br>
+ void fetchAwbStatus(Metadata *image_metadata);<br>
void computeGain(bcm2835_isp_stats *statistics, Metadata *image_metadata,<br>
double &gain, double &target_Y);<br>
void computeTargetExposure(double gain);<br>
- bool applyDigitalGain(Metadata *image_metadata, double gain,<br>
- double target_Y);<br>
+ bool applyDigitalGain(double gain, double target_Y);<br>
void filterExposure(bool desaturate);<br>
void divideUpExposure();<br>
void writeAndFinish(Metadata *image_metadata, bool desaturate);<br>
@@ -95,6 +95,7 @@ private:<br>
AgcExposureMode *exposure_mode_;<br>
AgcConstraintMode *constraint_mode_;<br>
uint64_t frame_count_;<br>
+ AwbStatus awb_;<br>
struct ExposureValues {<br>
ExposureValues() : shutter(0), analogue_gain(0),<br>
total_exposure(0), total_exposure_no_dg(0) {}<br></blockquote><div><br></div><div>Aparat from the minor:</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
2.20.1<br>
<br>
_______________________________________________<br>
libcamera-devel mailing list<br>
<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
<a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>