| Message ID | 20220207135937.221226-4-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi JM, Quoting Jean-Michel Hautbois (2022-02-07 13:59:37) > Instead of having a local cached value for line duration, store it in > the IPASessionConfiguration::agc structure. Looking at my previous comments on this patch in https://patchwork.libcamera.org/patch/14778/, I was suggesting that I believed this line duration should be in the sensor specific structure, not the AGC specific structure. I'm sorry my comment then was not clear, perhaps I should have made it more direct. Is the lineDuration specific to the AGC algorithm in any way that I have not seen or understood? In rkisp1/ipa_context.h, you have: struct IPASessionConfiguration { ... struct { utils::Duration lineDuration; } sensor; ... }; > While at it, configure the default analogue gain and shutter speed to > controlled fixed values. > > The latter is set to be 10ms as it will in most cases be close to the > one needed, making the AGC faster to converge. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++++---------- > src/ipa/ipu3/algorithms/agc.h | 3 +-- > src/ipa/ipu3/ipa_context.cpp | 3 +++ > src/ipa/ipu3/ipa_context.h | 1 + > 4 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index a929085c..137c36cf 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10; > static constexpr double kRelativeLuminanceTarget = 0.16; > > Agc::Agc() > - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s), > + : frameCount_(0), minShutterSpeed_(0s), > maxShutterSpeed_(0s), filteredExposure_(0s) > { > } > @@ -85,11 +85,14 @@ Agc::Agc() > */ > int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > { > - stride_ = context.configuration.grid.stride; > + IPASessionConfiguration &configuration = context.configuration; > + IPAFrameContext &frameContext = context.frameContext; > + > + stride_ = configuration.grid.stride; > > /* \todo use the IPAContext to provide the limits */ > - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > - / configInfo.sensorInfo.pixelRate; > + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s > + / configInfo.sensorInfo.pixelRate; And in RKISP1 this is calculated during the IPA configure, before it calls configure on the algorithms. Should we do that here to align them? > > minShutterSpeed_ = context.configuration.agc.minShutterSpeed; > maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed, > @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain); > > /* Configure the default exposure and gain. */ > - context.frameContext.agc.gain = minAnalogueGain_; > - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_; > + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain); > + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration; > > return 0; > } > @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > * \param[in] yGain The gain calculated based on the relative luminance target > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > */ > -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > +void Agc::computeExposure(IPAContext &context, double yGain, > double iqMeanGain) > { > + const IPASessionConfiguration &configuration = context.configuration; > + IPAFrameContext &frameContext = context.frameContext; > /* Get the effective exposure and gain applied on the sensor. */ > uint32_t exposure = frameContext.sensor.exposure; > double analogueGain = frameContext.sensor.gain; > @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > /* extracted from Rpi::Agc::computeTargetExposure */ > > /* Calculate the shutter time in seconds */ > - utils::Duration currentShutter = exposure * lineDuration_; > + utils::Duration currentShutter = exposure * configuration.agc.lineDuration; > > /* > * Update the exposure value for the next computation using the values > @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > << stepGain; > > /* Update the estimated exposure and gain. */ > - frameContext.agc.exposure = shutterTime / lineDuration_; > + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration; > frameContext.agc.gain = stepGain; > } > > @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > break; > } > > - computeExposure(context.frameContext, yGain, iqMeanGain); > + computeExposure(context, yGain, iqMeanGain); > frameCount_++; > } > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 84bfe045..ad705605 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -34,7 +34,7 @@ private: > double measureBrightness(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid) const; > utils::Duration filterExposure(utils::Duration currentExposure); > - void computeExposure(IPAFrameContext &frameContext, double yGain, > + void computeExposure(IPAContext &context, double yGain, > double iqMeanGain); > double estimateLuminance(IPAFrameContext &frameContext, > const ipu3_uapi_grid_config &grid, > @@ -43,7 +43,6 @@ private: > > uint64_t frameCount_; > > - utils::Duration lineDuration_; > utils::Duration minShutterSpeed_; > utils::Duration maxShutterSpeed_; > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index 86794ac1..ace9c66f 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 { > * > * \var IPASessionConfiguration::agc.maxAnalogueGain > * \brief Maximum analogue gain supported with the configured sensor > + * > + * \var IPASessionConfiguration::agc.lineDuration > + * \brief Line duration in microseconds > */ > > /** > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index c6dc0814..7696fd14 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -30,6 +30,7 @@ struct IPASessionConfiguration { > utils::Duration maxShutterSpeed; > double minAnalogueGain; > double maxAnalogueGain; > + utils::Duration lineDuration; > } agc; > }; > > -- > 2.32.0 >
Hi Kieran, On 07/02/2022 16:34, Kieran Bingham wrote: > Hi JM, > > Quoting Jean-Michel Hautbois (2022-02-07 13:59:37) >> Instead of having a local cached value for line duration, store it in >> the IPASessionConfiguration::agc structure. > > Looking at my previous comments on this patch in > https://patchwork.libcamera.org/patch/14778/, I was suggesting that I > believed this line duration should be in the sensor specific structure, > not the AGC specific structure. > > I'm sorry my comment then was not clear, perhaps I should have made it > more direct. > > Is the lineDuration specific to the AGC algorithm in any way that I have > not seen or understood? > > In rkisp1/ipa_context.h, you have: > > struct IPASessionConfiguration { > ... > struct { > utils::Duration lineDuration; > } sensor; > ... > }; > > >> While at it, configure the default analogue gain and shutter speed to >> controlled fixed values. >> >> The latter is set to be 10ms as it will in most cases be close to the >> one needed, making the AGC faster to converge. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++++---------- >> src/ipa/ipu3/algorithms/agc.h | 3 +-- >> src/ipa/ipu3/ipa_context.cpp | 3 +++ >> src/ipa/ipu3/ipa_context.h | 1 + >> 4 files changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index a929085c..137c36cf 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10; >> static constexpr double kRelativeLuminanceTarget = 0.16; >> >> Agc::Agc() >> - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s), >> + : frameCount_(0), minShutterSpeed_(0s), >> maxShutterSpeed_(0s), filteredExposure_(0s) >> { >> } >> @@ -85,11 +85,14 @@ Agc::Agc() >> */ >> int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) >> { >> - stride_ = context.configuration.grid.stride; >> + IPASessionConfiguration &configuration = context.configuration; >> + IPAFrameContext &frameContext = context.frameContext; >> + >> + stride_ = configuration.grid.stride; >> >> /* \todo use the IPAContext to provide the limits */ >> - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s >> - / configInfo.sensorInfo.pixelRate; >> + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s >> + / configInfo.sensorInfo.pixelRate; > > And in RKISP1 this is calculated during the IPA configure, before it > calls configure on the algorithms. Should we do that here to align them? > I noticed that lineDuration_ is cached and used in updateControls() which is called in configure()... and in init() (so, before it is set ?)... We might have an issue there ? > >> >> minShutterSpeed_ = context.configuration.agc.minShutterSpeed; >> maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed, >> @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) >> maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain); >> >> /* Configure the default exposure and gain. */ >> - context.frameContext.agc.gain = minAnalogueGain_; >> - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_; >> + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain); >> + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration; >> >> return 0; >> } >> @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) >> * \param[in] yGain The gain calculated based on the relative luminance target >> * \param[in] iqMeanGain The gain calculated based on the relative luminance target >> */ >> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, >> +void Agc::computeExposure(IPAContext &context, double yGain, >> double iqMeanGain) >> { >> + const IPASessionConfiguration &configuration = context.configuration; >> + IPAFrameContext &frameContext = context.frameContext; >> /* Get the effective exposure and gain applied on the sensor. */ >> uint32_t exposure = frameContext.sensor.exposure; >> double analogueGain = frameContext.sensor.gain; >> @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, >> /* extracted from Rpi::Agc::computeTargetExposure */ >> >> /* Calculate the shutter time in seconds */ >> - utils::Duration currentShutter = exposure * lineDuration_; >> + utils::Duration currentShutter = exposure * configuration.agc.lineDuration; >> >> /* >> * Update the exposure value for the next computation using the values >> @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, >> << stepGain; >> >> /* Update the estimated exposure and gain. */ >> - frameContext.agc.exposure = shutterTime / lineDuration_; >> + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration; >> frameContext.agc.gain = stepGain; >> } >> >> @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) >> break; >> } >> >> - computeExposure(context.frameContext, yGain, iqMeanGain); >> + computeExposure(context, yGain, iqMeanGain); >> frameCount_++; >> } >> >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index 84bfe045..ad705605 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -34,7 +34,7 @@ private: >> double measureBrightness(const ipu3_uapi_stats_3a *stats, >> const ipu3_uapi_grid_config &grid) const; >> utils::Duration filterExposure(utils::Duration currentExposure); >> - void computeExposure(IPAFrameContext &frameContext, double yGain, >> + void computeExposure(IPAContext &context, double yGain, >> double iqMeanGain); >> double estimateLuminance(IPAFrameContext &frameContext, >> const ipu3_uapi_grid_config &grid, >> @@ -43,7 +43,6 @@ private: >> >> uint64_t frameCount_; >> >> - utils::Duration lineDuration_; >> utils::Duration minShutterSpeed_; >> utils::Duration maxShutterSpeed_; >> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp >> index 86794ac1..ace9c66f 100644 >> --- a/src/ipa/ipu3/ipa_context.cpp >> +++ b/src/ipa/ipu3/ipa_context.cpp >> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 { >> * >> * \var IPASessionConfiguration::agc.maxAnalogueGain >> * \brief Maximum analogue gain supported with the configured sensor >> + * >> + * \var IPASessionConfiguration::agc.lineDuration >> + * \brief Line duration in microseconds >> */ >> >> /** >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h >> index c6dc0814..7696fd14 100644 >> --- a/src/ipa/ipu3/ipa_context.h >> +++ b/src/ipa/ipu3/ipa_context.h >> @@ -30,6 +30,7 @@ struct IPASessionConfiguration { >> utils::Duration maxShutterSpeed; >> double minAnalogueGain; >> double maxAnalogueGain; >> + utils::Duration lineDuration; >> } agc; >> }; >> >> -- >> 2.32.0 >>
Quoting Jean-Michel Hautbois (2022-02-08 17:09:59) > Hi Kieran, > > On 07/02/2022 16:34, Kieran Bingham wrote: > > Hi JM, > > > > Quoting Jean-Michel Hautbois (2022-02-07 13:59:37) > >> Instead of having a local cached value for line duration, store it in > >> the IPASessionConfiguration::agc structure. > > > > Looking at my previous comments on this patch in > > https://patchwork.libcamera.org/patch/14778/, I was suggesting that I > > believed this line duration should be in the sensor specific structure, > > not the AGC specific structure. > > > > I'm sorry my comment then was not clear, perhaps I should have made it > > more direct. > > > > Is the lineDuration specific to the AGC algorithm in any way that I have > > not seen or understood? > > > > In rkisp1/ipa_context.h, you have: > > > > struct IPASessionConfiguration { > > ... > > struct { > > utils::Duration lineDuration; > > } sensor; > > ... > > }; > > > > > >> While at it, configure the default analogue gain and shutter speed to > >> controlled fixed values. > >> > >> The latter is set to be 10ms as it will in most cases be close to the > >> one needed, making the AGC faster to converge. > >> > >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >> --- > >> src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++++---------- > >> src/ipa/ipu3/algorithms/agc.h | 3 +-- > >> src/ipa/ipu3/ipa_context.cpp | 3 +++ > >> src/ipa/ipu3/ipa_context.h | 1 + > >> 4 files changed, 20 insertions(+), 12 deletions(-) > >> > >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > >> index a929085c..137c36cf 100644 > >> --- a/src/ipa/ipu3/algorithms/agc.cpp > >> +++ b/src/ipa/ipu3/algorithms/agc.cpp > >> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10; > >> static constexpr double kRelativeLuminanceTarget = 0.16; > >> > >> Agc::Agc() > >> - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s), > >> + : frameCount_(0), minShutterSpeed_(0s), > >> maxShutterSpeed_(0s), filteredExposure_(0s) > >> { > >> } > >> @@ -85,11 +85,14 @@ Agc::Agc() > >> */ > >> int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > >> { > >> - stride_ = context.configuration.grid.stride; > >> + IPASessionConfiguration &configuration = context.configuration; > >> + IPAFrameContext &frameContext = context.frameContext; > >> + > >> + stride_ = configuration.grid.stride; > >> > >> /* \todo use the IPAContext to provide the limits */ > >> - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > >> - / configInfo.sensorInfo.pixelRate; > >> + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s > >> + / configInfo.sensorInfo.pixelRate; > > > > And in RKISP1 this is calculated during the IPA configure, before it > > calls configure on the algorithms. Should we do that here to align them? > > > > I noticed that lineDuration_ is cached and used in updateControls() > which is called in configure()... and in init() (so, before it is set ?)... > We might have an issue there ? Ok - so we have two instances of lineDuration_ ... so they should be moved to a single one in the sensor specific structure ... It should be initialised during init() before it's used, and updated in configure() before any algorithms are called? Is that right? > >> > >> minShutterSpeed_ = context.configuration.agc.minShutterSpeed; > >> maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed, > >> @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > >> maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain); > >> > >> /* Configure the default exposure and gain. */ > >> - context.frameContext.agc.gain = minAnalogueGain_; > >> - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_; > >> + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain); > >> + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration; > >> > >> return 0; > >> } > >> @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > >> * \param[in] yGain The gain calculated based on the relative luminance target > >> * \param[in] iqMeanGain The gain calculated based on the relative luminance target > >> */ > >> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > >> +void Agc::computeExposure(IPAContext &context, double yGain, > >> double iqMeanGain) > >> { > >> + const IPASessionConfiguration &configuration = context.configuration; > >> + IPAFrameContext &frameContext = context.frameContext; > >> /* Get the effective exposure and gain applied on the sensor. */ > >> uint32_t exposure = frameContext.sensor.exposure; > >> double analogueGain = frameContext.sensor.gain; > >> @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > >> /* extracted from Rpi::Agc::computeTargetExposure */ > >> > >> /* Calculate the shutter time in seconds */ > >> - utils::Duration currentShutter = exposure * lineDuration_; > >> + utils::Duration currentShutter = exposure * configuration.agc.lineDuration; > >> > >> /* > >> * Update the exposure value for the next computation using the values > >> @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, > >> << stepGain; > >> > >> /* Update the estimated exposure and gain. */ > >> - frameContext.agc.exposure = shutterTime / lineDuration_; > >> + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration; > >> frameContext.agc.gain = stepGain; > >> } > >> > >> @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > >> break; > >> } > >> > >> - computeExposure(context.frameContext, yGain, iqMeanGain); > >> + computeExposure(context, yGain, iqMeanGain); > >> frameCount_++; > >> } > >> > >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > >> index 84bfe045..ad705605 100644 > >> --- a/src/ipa/ipu3/algorithms/agc.h > >> +++ b/src/ipa/ipu3/algorithms/agc.h > >> @@ -34,7 +34,7 @@ private: > >> double measureBrightness(const ipu3_uapi_stats_3a *stats, > >> const ipu3_uapi_grid_config &grid) const; > >> utils::Duration filterExposure(utils::Duration currentExposure); > >> - void computeExposure(IPAFrameContext &frameContext, double yGain, > >> + void computeExposure(IPAContext &context, double yGain, > >> double iqMeanGain); > >> double estimateLuminance(IPAFrameContext &frameContext, > >> const ipu3_uapi_grid_config &grid, > >> @@ -43,7 +43,6 @@ private: > >> > >> uint64_t frameCount_; > >> > >> - utils::Duration lineDuration_; > >> utils::Duration minShutterSpeed_; > >> utils::Duration maxShutterSpeed_; > >> > >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > >> index 86794ac1..ace9c66f 100644 > >> --- a/src/ipa/ipu3/ipa_context.cpp > >> +++ b/src/ipa/ipu3/ipa_context.cpp > >> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 { > >> * > >> * \var IPASessionConfiguration::agc.maxAnalogueGain > >> * \brief Maximum analogue gain supported with the configured sensor > >> + * > >> + * \var IPASessionConfiguration::agc.lineDuration > >> + * \brief Line duration in microseconds > >> */ > >> > >> /** > >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > >> index c6dc0814..7696fd14 100644 > >> --- a/src/ipa/ipu3/ipa_context.h > >> +++ b/src/ipa/ipu3/ipa_context.h > >> @@ -30,6 +30,7 @@ struct IPASessionConfiguration { > >> utils::Duration maxShutterSpeed; > >> double minAnalogueGain; > >> double maxAnalogueGain; > >> + utils::Duration lineDuration; > >> } agc; > >> }; > >> > >> -- > >> 2.32.0 > >>
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index a929085c..137c36cf 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10; static constexpr double kRelativeLuminanceTarget = 0.16; Agc::Agc() - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s), + : frameCount_(0), minShutterSpeed_(0s), maxShutterSpeed_(0s), filteredExposure_(0s) { } @@ -85,11 +85,14 @@ Agc::Agc() */ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) { - stride_ = context.configuration.grid.stride; + IPASessionConfiguration &configuration = context.configuration; + IPAFrameContext &frameContext = context.frameContext; + + stride_ = configuration.grid.stride; /* \todo use the IPAContext to provide the limits */ - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s - / configInfo.sensorInfo.pixelRate; + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s + / configInfo.sensorInfo.pixelRate; minShutterSpeed_ = context.configuration.agc.minShutterSpeed; maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed, @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain); /* Configure the default exposure and gain. */ - context.frameContext.agc.gain = minAnalogueGain_; - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_; + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain); + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration; return 0; } @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) * \param[in] yGain The gain calculated based on the relative luminance target * \param[in] iqMeanGain The gain calculated based on the relative luminance target */ -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, +void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) { + const IPASessionConfiguration &configuration = context.configuration; + IPAFrameContext &frameContext = context.frameContext; /* Get the effective exposure and gain applied on the sensor. */ uint32_t exposure = frameContext.sensor.exposure; double analogueGain = frameContext.sensor.gain; @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, /* extracted from Rpi::Agc::computeTargetExposure */ /* Calculate the shutter time in seconds */ - utils::Duration currentShutter = exposure * lineDuration_; + utils::Duration currentShutter = exposure * configuration.agc.lineDuration; /* * Update the exposure value for the next computation using the values @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain, << stepGain; /* Update the estimated exposure and gain. */ - frameContext.agc.exposure = shutterTime / lineDuration_; + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration; frameContext.agc.gain = stepGain; } @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) break; } - computeExposure(context.frameContext, yGain, iqMeanGain); + computeExposure(context, yGain, iqMeanGain); frameCount_++; } diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 84bfe045..ad705605 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -34,7 +34,7 @@ private: double measureBrightness(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) const; utils::Duration filterExposure(utils::Duration currentExposure); - void computeExposure(IPAFrameContext &frameContext, double yGain, + void computeExposure(IPAContext &context, double yGain, double iqMeanGain); double estimateLuminance(IPAFrameContext &frameContext, const ipu3_uapi_grid_config &grid, @@ -43,7 +43,6 @@ private: uint64_t frameCount_; - utils::Duration lineDuration_; utils::Duration minShutterSpeed_; utils::Duration maxShutterSpeed_; diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index 86794ac1..ace9c66f 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 { * * \var IPASessionConfiguration::agc.maxAnalogueGain * \brief Maximum analogue gain supported with the configured sensor + * + * \var IPASessionConfiguration::agc.lineDuration + * \brief Line duration in microseconds */ /** diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index c6dc0814..7696fd14 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -30,6 +30,7 @@ struct IPASessionConfiguration { utils::Duration maxShutterSpeed; double minAnalogueGain; double maxAnalogueGain; + utils::Duration lineDuration; } agc; };
Instead of having a local cached value for line duration, store it in the IPASessionConfiguration::agc structure. While at it, configure the default analogue gain and shutter speed to controlled fixed values. The latter is set to be 10ms as it will in most cases be close to the one needed, making the AGC faster to converge. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++++---------- src/ipa/ipu3/algorithms/agc.h | 3 +-- src/ipa/ipu3/ipa_context.cpp | 3 +++ src/ipa/ipu3/ipa_context.h | 1 + 4 files changed, 20 insertions(+), 12 deletions(-)