| Message ID | 20231110104520.384229-1-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 2905eabad9ffa3a83c498d5a4a98a110fc0317af |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for this fix. On Fri, 10 Nov 2023 at 10:45, David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > The recent change where time-filtering is done before sorting out the > digital gain means that the target exposure without digital gain is no > longer set, breaking the 'AeLocked' calculation. > > We can use the regular (full) target exposure instead. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Fixes: 84b6327789fc ("ipa: rpi: agc: Filter exposures before dealing with digital gain") Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/rpi/agc_channel.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp > index 8d374b53..8116c6c1 100644 > --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp > +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp > @@ -977,7 +977,7 @@ void AgcChannel::divideUpExposure() > void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) > { > status_.totalExposureValue = filtered_.totalExposure; > - status_.targetExposureValue = desaturate ? 0s : target_.totalExposureNoDG; > + status_.targetExposureValue = desaturate ? 0s : target_.totalExposure; > status_.shutterTime = filtered_.shutter; > status_.analogueGain = filtered_.analogueGain; > /* > -- > 2.39.2 >
Quoting David Plowman via libcamera-devel (2023-11-10 10:45:20) > The recent change where time-filtering is done before sorting out the > digital gain means that the target exposure without digital gain is no > longer set, breaking the 'AeLocked' calculation. > > We can use the regular (full) target exposure instead. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Fixes: 84b6327789fc ("ipa: rpi: agc: Filter exposures before dealing with digital gain") > --- > src/ipa/rpi/controller/rpi/agc_channel.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp > index 8d374b53..8116c6c1 100644 > --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp > +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp > @@ -977,7 +977,7 @@ void AgcChannel::divideUpExposure() > void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) > { > status_.totalExposureValue = filtered_.totalExposure; > - status_.targetExposureValue = desaturate ? 0s : target_.totalExposureNoDG; > + status_.targetExposureValue = desaturate ? 0s : target_.totalExposure; It's hard for me to follow this back at the moment, but target_.totalExposureNoDG is used during process() before we get to writeAndFinish(). Are there other places that need updating too? or are those ok as this is more of an ordering thing? Anyway, this fixes a reported bug so: Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > status_.shutterTime = filtered_.shutter; > status_.analogueGain = filtered_.analogueGain; > /* > -- > 2.39.2 >
diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index 8d374b53..8116c6c1 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -977,7 +977,7 @@ void AgcChannel::divideUpExposure() void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) { status_.totalExposureValue = filtered_.totalExposure; - status_.targetExposureValue = desaturate ? 0s : target_.totalExposureNoDG; + status_.targetExposureValue = desaturate ? 0s : target_.totalExposure; status_.shutterTime = filtered_.shutter; status_.analogueGain = filtered_.analogueGain; /*
The recent change where time-filtering is done before sorting out the digital gain means that the target exposure without digital gain is no longer set, breaking the 'AeLocked' calculation. We can use the regular (full) target exposure instead. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Fixes: 84b6327789fc ("ipa: rpi: agc: Filter exposures before dealing with digital gain") --- src/ipa/rpi/controller/rpi/agc_channel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)