| Message ID | 20211026082142.69023-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi JM, On 10/26/21 1:51 PM, Jean-Michel Hautbois wrote: > In case the maximum exposure received from the sensor is very high, we > can have a very high shutter speed with a small analogue gain, and it > may result in very slow framerate. We are not really supporting it for > the moment, so clamp the shutter speed to an arbitrary value of 60ms. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Tested on nautilus and most of the CTS regressions were fixed (which were https://i.imgur.com/vv9Sjyh.png) Tested-by: Umang Jain <umang.jain@ideasonboard.com> I'll be trying to run low-light situations too, will keep you updated on the results. > --- > src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 6c151232..5927b5d3 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; > static constexpr double kMinAnalogueGain = 1.0; > static constexpr double kMaxAnalogueGain = 8.0; > > +/* Maximum shutter speed allowed */ > +static constexpr utils::Duration kMaxShutterSpeed = 60ms; > + > /* Histogram constants */ > static constexpr uint32_t knumHistogramBins = 256; > static constexpr double kEvGainTarget = 0.5; > @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /* \todo replace the exposure in lines storage with time based ones. */ > minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_; > - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_; > + maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, > + kMaxShutterSpeed / lineDuration_); > > minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
On 26/10/2021 10:52, Umang Jain wrote: > Hi JM, > > On 10/26/21 1:51 PM, Jean-Michel Hautbois wrote: >> In case the maximum exposure received from the sensor is very high, we >> can have a very high shutter speed with a small analogue gain, and it >> may result in very slow framerate. We are not really supporting it for >> the moment, so clamp the shutter speed to an arbitrary value of 60ms. >> >> Signed-off-by: Jean-Michel Hautbois >> <jeanmichel.hautbois@ideasonboard.com> > > > Tested on nautilus and most of the CTS regressions were fixed (which > were https://i.imgur.com/vv9Sjyh.png) Is "most" meaning "all the ones introduced by this new series" or is there still new ones ? > > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > > I'll be trying to run low-light situations too, will keep you updated on > the results. > >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp >> b/src/ipa/ipu3/algorithms/agc.cpp >> index 6c151232..5927b5d3 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; >> static constexpr double kMinAnalogueGain = 1.0; >> static constexpr double kMaxAnalogueGain = 8.0; >> +/* Maximum shutter speed allowed */ >> +static constexpr utils::Duration kMaxShutterSpeed = 60ms; >> + >> /* Histogram constants */ >> static constexpr uint32_t knumHistogramBins = 256; >> static constexpr double kEvGainTarget = 0.5; >> @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const >> IPAConfigInfo &configInfo) >> /* \todo replace the exposure in lines storage with time based >> ones. */ >> minExposureLines_ = context.configuration.agc.minShutterSpeed / >> lineDuration_; >> - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / >> lineDuration_; >> + maxExposureLines_ = >> std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, >> + kMaxShutterSpeed / lineDuration_); >> minAnalogueGain_ = >> std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); >> maxAnalogueGain_ = >> std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
Hi JM, On 10/26/21 2:24 PM, Jean-Michel Hautbois wrote: > > > On 26/10/2021 10:52, Umang Jain wrote: >> Hi JM, >> >> On 10/26/21 1:51 PM, Jean-Michel Hautbois wrote: >>> In case the maximum exposure received from the sensor is very high, we >>> can have a very high shutter speed with a small analogue gain, and it >>> may result in very slow framerate. We are not really supporting it for >>> the moment, so clamp the shutter speed to an arbitrary value of 60ms. >>> >>> Signed-off-by: Jean-Michel Hautbois >>> <jeanmichel.hautbois@ideasonboard.com> >> >> >> Tested on nautilus and most of the CTS regressions were fixed (which >> were https://i.imgur.com/vv9Sjyh.png) > > Is "most" meaning "all the ones introduced by this new series" or is > there still new ones ? Sorry, I mean the latter. All the regressions I saw with AGC fix series(merged), were fixed by this patch. > >> >> Tested-by: Umang Jain <umang.jain@ideasonboard.com> >> >> I'll be trying to run low-light situations too, will keep you updated >> on the results. >> >>> --- >>> src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp >>> b/src/ipa/ipu3/algorithms/agc.cpp >>> index 6c151232..5927b5d3 100644 >>> --- a/src/ipa/ipu3/algorithms/agc.cpp >>> +++ b/src/ipa/ipu3/algorithms/agc.cpp >>> @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; >>> static constexpr double kMinAnalogueGain = 1.0; >>> static constexpr double kMaxAnalogueGain = 8.0; >>> +/* Maximum shutter speed allowed */ >>> +static constexpr utils::Duration kMaxShutterSpeed = 60ms; >>> + >>> /* Histogram constants */ >>> static constexpr uint32_t knumHistogramBins = 256; >>> static constexpr double kEvGainTarget = 0.5; >>> @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const >>> IPAConfigInfo &configInfo) >>> /* \todo replace the exposure in lines storage with time based >>> ones. */ >>> minExposureLines_ = context.configuration.agc.minShutterSpeed >>> / lineDuration_; >>> - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / >>> lineDuration_; >>> + maxExposureLines_ = >>> std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, >>> + kMaxShutterSpeed / lineDuration_); >>> minAnalogueGain_ = >>> std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); >>> maxAnalogueGain_ = >>> std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
Hi Jean-Michel, Thank you for the patch. On Tue, Oct 26, 2021 at 10:21:42AM +0200, Jean-Michel Hautbois wrote: > In case the maximum exposure received from the sensor is very high, we > can have a very high shutter speed with a small analogue gain, and it > may result in very slow framerate. We are not really supporting it for > the moment, so clamp the shutter speed to an arbitrary value of 60ms. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 6c151232..5927b5d3 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; > static constexpr double kMinAnalogueGain = 1.0; > static constexpr double kMaxAnalogueGain = 8.0; > > +/* Maximum shutter speed allowed */ /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +static constexpr utils::Duration kMaxShutterSpeed = 60ms; > + > /* Histogram constants */ > static constexpr uint32_t knumHistogramBins = 256; > static constexpr double kEvGainTarget = 0.5; > @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /* \todo replace the exposure in lines storage with time based ones. */ > minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_; > - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_; > + maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, > + kMaxShutterSpeed / lineDuration_); > > minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
Quoting Laurent Pinchart (2021-10-26 10:21:57) > Hi Jean-Michel, > > Thank you for the patch. > > On Tue, Oct 26, 2021 at 10:21:42AM +0200, Jean-Michel Hautbois wrote: > > In case the maximum exposure received from the sensor is very high, we > > can have a very high shutter speed with a small analogue gain, and it > > may result in very slow framerate. We are not really supporting it for > > the moment, so clamp the shutter speed to an arbitrary value of 60ms. > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > > index 6c151232..5927b5d3 100644 > > --- a/src/ipa/ipu3/algorithms/agc.cpp > > +++ b/src/ipa/ipu3/algorithms/agc.cpp > > @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; > > static constexpr double kMinAnalogueGain = 1.0; > > static constexpr double kMaxAnalogueGain = 8.0; > > > > +/* Maximum shutter speed allowed */ > > /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */ > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > +static constexpr utils::Duration kMaxShutterSpeed = 60ms; > > + > > /* Histogram constants */ > > static constexpr uint32_t knumHistogramBins = 256; > > static constexpr double kEvGainTarget = 0.5; > > @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > > > /* \todo replace the exposure in lines storage with time based ones. */ > > minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_; > > - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_; > > + maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, > > + kMaxShutterSpeed / lineDuration_); > > > > minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > > maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain); > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 6c151232..5927b5d3 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -34,6 +34,9 @@ static constexpr uint32_t kFrameSkipCount = 6; static constexpr double kMinAnalogueGain = 1.0; static constexpr double kMaxAnalogueGain = 8.0; +/* Maximum shutter speed allowed */ +static constexpr utils::Duration kMaxShutterSpeed = 60ms; + /* Histogram constants */ static constexpr uint32_t knumHistogramBins = 256; static constexpr double kEvGainTarget = 0.5; @@ -54,7 +57,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) /* \todo replace the exposure in lines storage with time based ones. */ minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_; - maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_; + maxExposureLines_ = std::min(context.configuration.agc.maxShutterSpeed / lineDuration_, + kMaxShutterSpeed / lineDuration_); minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
In case the maximum exposure received from the sensor is very high, we can have a very high shutter speed with a small analogue gain, and it may result in very slow framerate. We are not really supporting it for the moment, so clamp the shutter speed to an arbitrary value of 60ms. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)