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