@@ -435,14 +435,9 @@ void AgcChannel::switchMode(CameraMode const &cameraMode,
Duration fixedExposureTime = limitExposureTime(fixedExposureTime_);
if (fixedExposureTime && fixedAnalogueGain_) {
- /* We're going to reset the algorithm here with these fixed values. */
- fetchAwbStatus(metadata);
- double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 });
- ASSERT(minColourGain != 0.0);
-
/* This is the equivalent of computeTargetExposure and applyDigitalGain. */
target_.totalExposureNoDG = fixedExposureTime_ * fixedAnalogueGain_;
- target_.totalExposure = target_.totalExposureNoDG / minColourGain;
+ target_.totalExposure = target_.totalExposureNoDG;
/* Equivalent of filterExposure. This resets any "history". */
filtered_ = target_;
@@ -462,10 +457,10 @@ void AgcChannel::switchMode(CameraMode const &cameraMode,
*/
double ratio = lastSensitivity / cameraMode.sensitivity;
- target_.totalExposureNoDG *= ratio;
target_.totalExposure *= ratio;
- filtered_.totalExposureNoDG *= ratio;
+ target_.totalExposureNoDG = target_.totalExposure;
filtered_.totalExposure *= ratio;
+ filtered_.totalExposureNoDG = filtered_.totalExposure;
divideUpExposure();
} else {
@@ -716,8 +711,13 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,
}
/* Factor in the AWB correction if needed. */
- if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb)
- sum *= RGB<double>{ { awb.gainR, awb.gainR, awb.gainB } };
+ if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {
+ double minColourGain = std::min({ awb.gainR, awb.gainG, awb.gainB, 1.0 });
+ minColourGain = std::max(minColourGain, 1.0);
+ RGB<double> colourGains{ { awb.gainR, awb.gainR, awb.gainB } };
+ colourGains /= minColourGain;
+ sum *= colourGains;
+ }
double ySum = ipa::rec601LuminanceFromRGB(sum);
@@ -797,16 +797,8 @@ void AgcChannel::computeGain(StatisticsPtr &statistics, Metadata *imageMetadata,
void AgcChannel::computeTargetExposure(double gain)
{
if (status_.fixedExposureTime && status_.fixedAnalogueGain) {
- /*
- * When analogue gain and exposure time are both fixed, we need
- * to drive the total exposure so that we end up with a digital
- * gain of at least 1/minColourGain. Otherwise we'd desaturate
- * channels causing white to go cyan or magenta.
- */
- double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 });
- ASSERT(minColourGain != 0.0);
target_.totalExposure =
- status_.fixedExposureTime * status_.fixedAnalogueGain / minColourGain;
+ status_.fixedExposureTime * status_.fixedAnalogueGain;
} else {
/*
* The statistics reflect the image without digital gain, so the final
@@ -867,15 +859,8 @@ bool AgcChannel::applyChannelConstraints(const AgcChannelTotalExposures &channel
bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound)
{
- double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 });
- ASSERT(minColourGain != 0.0);
- double dg = 1.0 / minColourGain;
- /*
- * I think this pipeline subtracts black level and rescales before we
- * get the stats, so no need to worry about it.
- */
- LOG(RPiAgc, Debug) << "after AWB, target dg " << dg << " gain " << gain
- << " target_Y " << targetY;
+ double dg = 1.0;
+
/*
* Finally, if we're trying to reduce exposure but the target_Y is
* "close" to 1.0, then the gain computed for that constraint will be
@@ -521,10 +521,24 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr
pisp_wbg_config wbg;
pisp_fe_rgby_config rgby = {};
double dg = agcPrepareStatus ? agcPrepareStatus->digitalGain : 1.0;
+ double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 });
+ /* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */
+ double extraGain = 1.0 / std::max({ minColourGain, 0.1 });
- wbg.gain_r = clampField(dg * awbStatus->gainR, 14, 10);
- wbg.gain_g = clampField(dg * awbStatus->gainG, 14, 10);
- wbg.gain_b = clampField(dg * awbStatus->gainB, 14, 10);
+ /*
+ * Apply an extra gain of 1 / minColourGain so as not to apply < 1 gains to any
+ * channels (which would cause saturated pixels to go cyan or magenta).
+ * Doing this doesn't really apply more gain than necessary, because one of the
+ * channels is always getting the minimum gain possible. For this reason we also
+ * don't change the values that we report externally.
+ */
+ double gainR = awbStatus->gainR * extraGain;
+ double gainG = awbStatus->gainG * extraGain;
+ double gainB = awbStatus->gainB * extraGain;
+
+ wbg.gain_r = clampField(dg * gainR, 14, 10);
+ wbg.gain_g = clampField(dg * gainG, 14, 10);
+ wbg.gain_b = clampField(dg * gainB, 14, 10);
/*
* The YCbCr conversion block should contain the appropriate YCbCr
@@ -535,9 +549,9 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr
be_->GetYcbcr(csc);
/* The CSC coefficients already have the << 10 scaling applied. */
- rgby.gain_r = clampField(csc.coeffs[0] * awbStatus->gainR, 14);
- rgby.gain_g = clampField(csc.coeffs[1] * awbStatus->gainG, 14);
- rgby.gain_b = clampField(csc.coeffs[2] * awbStatus->gainB, 14);
+ rgby.gain_r = clampField(csc.coeffs[0] * gainR, 14);
+ rgby.gain_g = clampField(csc.coeffs[1] * gainG, 14);
+ rgby.gain_b = clampField(csc.coeffs[2] * gainB, 14);
LOG(IPARPI, Debug) << "Applying WB R: " << awbStatus->gainR << " B: "
<< awbStatus->gainB;
@@ -63,7 +63,8 @@ private:
bool validateIspControls();
void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);
- void applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls);
+ void applyDG(const struct AgcPrepareStatus *dgStatus,
+ const struct AwbStatus *awbStatus, ControlList &ctrls);
void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);
void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);
void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);
@@ -152,8 +153,7 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms,
applyCCM(ccmStatus, ctrls);
AgcPrepareStatus *dgStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
- if (dgStatus)
- applyDG(dgStatus, ctrls);
+ applyDG(dgStatus, awbStatus, ctrls);
AlscStatus *lsStatus = rpiMetadata.getLocked<AlscStatus>("alsc.status");
if (lsStatus)
@@ -329,10 +329,26 @@ void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
static_cast<int32_t>(awbStatus->gainB * 1000));
}
-void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls)
+void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus,
+ const struct AwbStatus *awbStatus, ControlList &ctrls)
{
+ double digitalGain = dgStatus ? dgStatus->digitalGain : 1.0;
+
+ if (awbStatus) {
+ /*
+ * We must apply sufficient extra digital gain to stop any of the channel gains being
+ * less than 1, which would cause saturation artifacts. Note that one of the colour
+ * channels is still getting the minimum possible gain, so it's not a "real" gain
+ * increase.
+ */
+ double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 });
+ /* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */
+ double extraGain = 1.0 / std::max({ minColourGain, 0.1 });
+ digitalGain *= extraGain;
+ }
+
ctrls.set(V4L2_CID_DIGITAL_GAIN,
- static_cast<int32_t>(dgStatus->digitalGain * 1000));
+ static_cast<int32_t>(digitalGain * 1000));
}
void IpaVc4::applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls)
Previously these were handled in the AGC/AEC exposure update calculations by explicitly driving a higher digital gain to "cancel out" any colour gains that were less than 1. Now we're ignoring this in the AGC and leaving it to the IPA code to normalise all the gains so that the smallest is 1. We don't regard this as a "real" increase because one of the colour channels (just not necessarily the green one) still gets the minimum gain possible. We do, however, update the statistics calculations so that they reflect any such digital gain increase, so that images are driven to the correct level. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/ipa/rpi/controller/rpi/agc_channel.cpp | 41 +++++++--------------- src/ipa/rpi/pisp/pisp.cpp | 26 ++++++++++---- src/ipa/rpi/vc4/vc4.cpp | 26 +++++++++++--- 3 files changed, 54 insertions(+), 39 deletions(-)