| Message ID | 20220818094410.1671-3-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55) > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> > > Move the existing frame context structure to the IPAActiveState. > This structure should store the most up to date results for the IPA > which may be shared to other algorithms that operate on the data. > Laurent queried this patch in v2. I suggested we add the following to this commit message: """ All existing algorithm state is moved to the ActiveState and the algorithms should be updated individually to use the FrameContext directly where appropriate. """ > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/ipa/rkisp1/algorithms/agc.cpp | 20 ++++++++-------- > src/ipa/rkisp1/algorithms/awb.cpp | 34 ++++++++++++++-------------- > src/ipa/rkisp1/algorithms/blc.cpp | 2 +- > src/ipa/rkisp1/algorithms/cproc.cpp | 4 ++-- > src/ipa/rkisp1/algorithms/dpcc.cpp | 2 +- > src/ipa/rkisp1/algorithms/filter.cpp | 4 ++-- > src/ipa/rkisp1/algorithms/gsl.cpp | 2 +- > src/ipa/rkisp1/algorithms/lsc.cpp | 2 +- > src/ipa/rkisp1/ipa_context.h | 7 ++++-- > src/ipa/rkisp1/rkisp1.cpp | 12 +++++----- > 10 files changed, 46 insertions(+), 43 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index a1bb7d972926..483e941fe427 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -73,8 +73,8 @@ Agc::Agc() > int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > { > /* Configure the default exposure and gain. */ > - context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > - context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > + context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > + context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > > /* > * According to the RkISP1 documentation: > @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; > context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; > > - /* \todo Use actual frame index by populating it in the frameContext. */ > + /* \todo Use actual frame index by populating it in the activeState. */ > frameCount_ = 0; > return 0; > } > @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > > /** > * \brief Estimate the new exposure and gain values > - * \param[inout] frameContext The shared IPA frame Context > + * \param[inout] context The shared IPA Context > * \param[in] yGain The gain calculated on the current brightness level > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > */ > void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > { > IPASessionConfiguration &configuration = context.configuration; > - IPAFrameContext &frameContext = context.frameContext; > + IPAActiveState &activeState = context.activeState; > > /* Get the effective exposure and gain applied on the sensor. */ > - uint32_t exposure = frameContext.sensor.exposure; > - double analogueGain = frameContext.sensor.gain; > + uint32_t exposure = activeState.sensor.exposure; > + double analogueGain = activeState.sensor.gain; > > /* Use the highest of the two gain estimates. */ > double evGain = std::max(yGain, iqMeanGain); > @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > << stepGain; > > /* Update the estimated exposure and gain. */ > - frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration; > - frameContext.agc.gain = stepGain; > + activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; > + activeState.agc.gain = stepGain; > } > > /** > @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context, > */ > void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params) > { > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > /* Configure the measurement window. */ > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 9f00364d12b1..427aaeb1e955 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) > int Awb::configure(IPAContext &context, > const IPACameraSensorInfo &configInfo) > { > - context.frameContext.awb.gains.red = 1.0; > - context.frameContext.awb.gains.blue = 1.0; > - context.frameContext.awb.gains.green = 1.0; > + context.activeState.awb.gains.red = 1.0; > + context.activeState.awb.gains.blue = 1.0; > + context.activeState.awb.gains.green = 1.0; > > /* > * Define the measurement window for AWB as a centered rectangle > @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > */ > void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > { > - params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green; > - params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue; > - params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red; > - params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green; > + params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green; > + params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue; > + params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red; > + params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green; > > /* Update the gains. */ > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; > > /* If we already have configured the gains and window, return. */ > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > /* Configure the gains to apply. */ > @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context, > { > const rkisp1_cif_isp_stat *params = &stats->params; > const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; > - IPAFrameContext &frameContext = context.frameContext; > + IPAActiveState &activeState = context.activeState; > > /* Get the YCbCr mean values */ > double yMean = awb->awb_mean[0].mean_y_or_g; > @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context, > > /* Filter the values to avoid oscillations. */ > double speed = 0.2; > - redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red; > - blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue; > + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; > + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; > > /* > * Gain values are unsigned integer value, range 0 to 4 with 8 bit > * fractional part. > */ > - frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > - frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > + activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > + activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > /* Hardcode the green gain to 1.0. */ > - frameContext.awb.gains.green = 1.0; > + activeState.awb.gains.green = 1.0; > > - frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > + activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > > - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red > - << " and for blue: " << context.frameContext.awb.gains.blue; > + LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red > + << " and for blue: " << context.activeState.awb.gains.blue; > } > > REGISTER_IPA_ALGORITHM(Awb, "Awb") > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp > index a58569fa2dc2..4d55a2d529cb 100644 > --- a/src/ipa/rkisp1/algorithms/blc.cpp > +++ b/src/ipa/rkisp1/algorithms/blc.cpp > @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, > void BlackLevelCorrection::prepare(IPAContext &context, > rkisp1_params_cfg *params) > { > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > if (!tuningParameters_) > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp > index bca5ab6907d6..a3b778d1c908 100644 > --- a/src/ipa/rkisp1/algorithms/cproc.cpp > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp > @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > const ControlList &controls) > { > - auto &cproc = context.frameContext.cproc; > + auto &cproc = context.activeState.cproc; > > const auto &brightness = controls.get(controls::Brightness); > if (brightness) { > @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > void ColorProcessing::prepare(IPAContext &context, > rkisp1_params_cfg *params) > { > - auto &cproc = context.frameContext.cproc; > + auto &cproc = context.activeState.cproc; > > /* Check if the algorithm configuration has been updated. */ > if (!cproc.updateParams) > diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp > index 69bc651eaf08..93d37b1dae44 100644 > --- a/src/ipa/rkisp1/algorithms/dpcc.cpp > +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp > @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context, > void DefectPixelClusterCorrection::prepare(IPAContext &context, > rkisp1_params_cfg *params) > { > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > if (!initialized_) > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp > index 8ca10fd1ee9d..bc7fc1f32f34 100644 > --- a/src/ipa/rkisp1/algorithms/filter.cpp > +++ b/src/ipa/rkisp1/algorithms/filter.cpp > @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > const ControlList &controls) > { > - auto &filter = context.frameContext.filter; > + auto &filter = context.activeState.filter; > > const auto &sharpness = controls.get(controls::Sharpness); > if (sharpness) { > @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context, > */ > void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params) > { > - auto &filter = context.frameContext.filter; > + auto &filter = context.activeState.filter; > > /* Check if the algorithm configuration has been updated. */ > if (!filter.updateParams) > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp > index 2fd1a23d3a9b..dd9974627cd2 100644 > --- a/src/ipa/rkisp1/algorithms/gsl.cpp > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp > @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, > void GammaSensorLinearization::prepare(IPAContext &context, > rkisp1_params_cfg *params) > { > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > if (!initialized_) > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > index 05c8c0dab5c8..4ed467086199 100644 > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > void LensShadingCorrection::prepare(IPAContext &context, > rkisp1_params_cfg *params) > { > - if (context.frameContext.frameCount > 0) > + if (context.activeState.frameCount > 0) > return; > > if (!initialized_) > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 2bdb6a81d7c9..a8daeca487ae 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -41,7 +41,7 @@ struct IPASessionConfiguration { > } hw; > }; > > -struct IPAFrameContext { > +struct IPAActiveState { > struct { > uint32_t exposure; > double gain; > @@ -78,9 +78,12 @@ struct IPAFrameContext { > unsigned int frameCount; > }; > > +struct IPAFrameContext { > +}; > + > struct IPAContext { > IPASessionConfiguration configuration; > - IPAFrameContext frameContext; > + IPAActiveState activeState; > }; > > } /* namespace ipa::rkisp1 */ > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 17d42d38eb45..88e6a2b23eed 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, > context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); > context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); > > - context_.frameContext.frameCount = 0; > + context_.activeState.frameCount = 0; > > for (auto const &algo : algorithms()) { > int ret = algo->configure(context_, info); > @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) > algo->prepare(context_, params); > > paramsBufferReady.emit(frame); > - context_.frameContext.frameCount++; > + context_.activeState.frameCount++; > } > > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, > @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > reinterpret_cast<rkisp1_stat_buffer *>( > mappedBuffers_.at(bufferId).planes()[0].data()); > > - context_.frameContext.sensor.exposure = > + context_.activeState.sensor.exposure = > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>(); > - context_.frameContext.sensor.gain = > + context_.activeState.sensor.gain = > camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>()); > > unsigned int aeState = 0; > @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > void IPARkISP1::setControls(unsigned int frame) > { > - uint32_t exposure = context_.frameContext.agc.exposure; > - uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain); > + uint32_t exposure = context_.activeState.agc.exposure; > + uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain); > > ControlList ctrls(ctrls_); > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure)); > -- > 2.37.2 >
On Thu, Aug 18, 2022 at 05:07:27PM +0100, Kieran Bingham via libcamera-devel wrote: > Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55) > > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> > > > > Move the existing frame context structure to the IPAActiveState. > > This structure should store the most up to date results for the IPA > > which may be shared to other algorithms that operate on the data. > > Laurent queried this patch in v2. > > I suggested we add the following to this commit message: > > """ > All existing algorithm state is moved to the ActiveState and the > algorithms should be updated individually to use the FrameContext > directly where appropriate. > """ I'd like to make it clear (partly to check that my understanding is correct :-)) that this commit is about moving the existing frame context aside to make space for the new one. How about -------- The RkISP1 IPA module creates a single instance of its IPAFrameContext structure, effectively using it more as an active state than a per-frame context. To prepare for the introduction of a real per-frame context, move all the members of the IPAFrameContext structure to a new IPAActiveState structure. The IPAFrameContext becomes effectively unused at runtime, but can't be removed yet as it is still required as a template argument to the Module and Algorithm classes. The new IPAActiveState structure is not foreseen to stay, its members should move to either the new per-frame context that will be introduced, or to the individual algorithm classes. -------- > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/ipa/rkisp1/algorithms/agc.cpp | 20 ++++++++-------- > > src/ipa/rkisp1/algorithms/awb.cpp | 34 ++++++++++++++-------------- > > src/ipa/rkisp1/algorithms/blc.cpp | 2 +- > > src/ipa/rkisp1/algorithms/cproc.cpp | 4 ++-- > > src/ipa/rkisp1/algorithms/dpcc.cpp | 2 +- > > src/ipa/rkisp1/algorithms/filter.cpp | 4 ++-- > > src/ipa/rkisp1/algorithms/gsl.cpp | 2 +- > > src/ipa/rkisp1/algorithms/lsc.cpp | 2 +- > > src/ipa/rkisp1/ipa_context.h | 7 ++++-- > > src/ipa/rkisp1/rkisp1.cpp | 12 +++++----- > > 10 files changed, 46 insertions(+), 43 deletions(-) > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > index a1bb7d972926..483e941fe427 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > @@ -73,8 +73,8 @@ Agc::Agc() > > int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > { > > /* Configure the default exposure and gain. */ > > - context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > > - context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > > + context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > > + context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > > > > /* > > * According to the RkISP1 documentation: > > @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; > > context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; > > > > - /* \todo Use actual frame index by populating it in the frameContext. */ > > + /* \todo Use actual frame index by populating it in the activeState. */ That will go later in the series, but we could still keep it accurate in the meantime by writing /* * \todo Use the upcoming per-frame context API that will provide a * frame index */ > > frameCount_ = 0; > > return 0; > > } > > @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > > > > /** > > * \brief Estimate the new exposure and gain values > > - * \param[inout] frameContext The shared IPA frame Context > > + * \param[inout] context The shared IPA Context And add to the commit message While at it, fix a typo in the documentation of the Agc::computeExposure() function that incorrectly refers to the frame context instead of the global context. > > * \param[in] yGain The gain calculated on the current brightness level > > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > > */ > > void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > > { > > IPASessionConfiguration &configuration = context.configuration; > > - IPAFrameContext &frameContext = context.frameContext; > > + IPAActiveState &activeState = context.activeState; > > > > /* Get the effective exposure and gain applied on the sensor. */ > > - uint32_t exposure = frameContext.sensor.exposure; > > - double analogueGain = frameContext.sensor.gain; > > + uint32_t exposure = activeState.sensor.exposure; > > + double analogueGain = activeState.sensor.gain; > > > > /* Use the highest of the two gain estimates. */ > > double evGain = std::max(yGain, iqMeanGain); > > @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > > << stepGain; > > > > /* Update the estimated exposure and gain. */ > > - frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration; > > - frameContext.agc.gain = stepGain; > > + activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; > > + activeState.agc.gain = stepGain; > > } > > > > /** > > @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context, > > */ > > void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params) > > { > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > /* Configure the measurement window. */ > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > > index 9f00364d12b1..427aaeb1e955 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.cpp > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > > @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) > > int Awb::configure(IPAContext &context, > > const IPACameraSensorInfo &configInfo) > > { > > - context.frameContext.awb.gains.red = 1.0; > > - context.frameContext.awb.gains.blue = 1.0; > > - context.frameContext.awb.gains.green = 1.0; > > + context.activeState.awb.gains.red = 1.0; > > + context.activeState.awb.gains.blue = 1.0; > > + context.activeState.awb.gains.green = 1.0; > > > > /* > > * Define the measurement window for AWB as a centered rectangle > > @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > > */ > > void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > > { > > - params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green; > > - params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue; > > - params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red; > > - params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green; > > + params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green; > > + params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue; > > + params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red; > > + params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green; > > > > /* Update the gains. */ > > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; > > > > /* If we already have configured the gains and window, return. */ > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > /* Configure the gains to apply. */ > > @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context, > > { > > const rkisp1_cif_isp_stat *params = &stats->params; > > const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; > > - IPAFrameContext &frameContext = context.frameContext; > > + IPAActiveState &activeState = context.activeState; > > > > /* Get the YCbCr mean values */ > > double yMean = awb->awb_mean[0].mean_y_or_g; > > @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context, > > > > /* Filter the values to avoid oscillations. */ > > double speed = 0.2; > > - redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red; > > - blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue; > > + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; > > + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; > > > > /* > > * Gain values are unsigned integer value, range 0 to 4 with 8 bit > > * fractional part. > > */ > > - frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > > - frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > > + activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > > + activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > > /* Hardcode the green gain to 1.0. */ > > - frameContext.awb.gains.green = 1.0; > > + activeState.awb.gains.green = 1.0; > > > > - frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > > + activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > > > > - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red > > - << " and for blue: " << context.frameContext.awb.gains.blue; > > + LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red > > + << " and for blue: " << context.activeState.awb.gains.blue; > > } > > > > REGISTER_IPA_ALGORITHM(Awb, "Awb") > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp > > index a58569fa2dc2..4d55a2d529cb 100644 > > --- a/src/ipa/rkisp1/algorithms/blc.cpp > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp > > @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, > > void BlackLevelCorrection::prepare(IPAContext &context, > > rkisp1_params_cfg *params) > > { > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > if (!tuningParameters_) > > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp > > index bca5ab6907d6..a3b778d1c908 100644 > > --- a/src/ipa/rkisp1/algorithms/cproc.cpp > > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp > > @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > > [[maybe_unused]] const uint32_t frame, > > const ControlList &controls) > > { > > - auto &cproc = context.frameContext.cproc; > > + auto &cproc = context.activeState.cproc; > > > > const auto &brightness = controls.get(controls::Brightness); > > if (brightness) { > > @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > > void ColorProcessing::prepare(IPAContext &context, > > rkisp1_params_cfg *params) > > { > > - auto &cproc = context.frameContext.cproc; > > + auto &cproc = context.activeState.cproc; > > > > /* Check if the algorithm configuration has been updated. */ > > if (!cproc.updateParams) > > diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp > > index 69bc651eaf08..93d37b1dae44 100644 > > --- a/src/ipa/rkisp1/algorithms/dpcc.cpp > > +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp > > @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context, > > void DefectPixelClusterCorrection::prepare(IPAContext &context, > > rkisp1_params_cfg *params) > > { > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > if (!initialized_) > > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp > > index 8ca10fd1ee9d..bc7fc1f32f34 100644 > > --- a/src/ipa/rkisp1/algorithms/filter.cpp > > +++ b/src/ipa/rkisp1/algorithms/filter.cpp > > @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context, > > [[maybe_unused]] const uint32_t frame, > > const ControlList &controls) > > { > > - auto &filter = context.frameContext.filter; > > + auto &filter = context.activeState.filter; > > > > const auto &sharpness = controls.get(controls::Sharpness); > > if (sharpness) { > > @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context, > > */ > > void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params) > > { > > - auto &filter = context.frameContext.filter; > > + auto &filter = context.activeState.filter; > > > > /* Check if the algorithm configuration has been updated. */ > > if (!filter.updateParams) > > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp > > index 2fd1a23d3a9b..dd9974627cd2 100644 > > --- a/src/ipa/rkisp1/algorithms/gsl.cpp > > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp > > @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, > > void GammaSensorLinearization::prepare(IPAContext &context, > > rkisp1_params_cfg *params) > > { > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > if (!initialized_) > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > > index 05c8c0dab5c8..4ed467086199 100644 > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > > @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > > void LensShadingCorrection::prepare(IPAContext &context, > > rkisp1_params_cfg *params) > > { > > - if (context.frameContext.frameCount > 0) > > + if (context.activeState.frameCount > 0) > > return; > > > > if (!initialized_) > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index 2bdb6a81d7c9..a8daeca487ae 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -41,7 +41,7 @@ struct IPASessionConfiguration { > > } hw; > > }; > > > > -struct IPAFrameContext { > > +struct IPAActiveState { > > struct { > > uint32_t exposure; > > double gain; > > @@ -78,9 +78,12 @@ struct IPAFrameContext { > > unsigned int frameCount; > > }; > > > > +struct IPAFrameContext { > > +}; > > + > > struct IPAContext { > > IPASessionConfiguration configuration; > > - IPAFrameContext frameContext; > > + IPAActiveState activeState; > > }; The documentation in ipa_context.cpp needs to be updated too. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This patch could go the very front of the series. > > > > } /* namespace ipa::rkisp1 */ > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index 17d42d38eb45..88e6a2b23eed 100644 > > --- a/src/ipa/rkisp1/rkisp1.cpp > > +++ b/src/ipa/rkisp1/rkisp1.cpp > > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, > > context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); > > context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); > > > > - context_.frameContext.frameCount = 0; > > + context_.activeState.frameCount = 0; > > > > for (auto const &algo : algorithms()) { > > int ret = algo->configure(context_, info); > > @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) > > algo->prepare(context_, params); > > > > paramsBufferReady.emit(frame); > > - context_.frameContext.frameCount++; > > + context_.activeState.frameCount++; > > } > > > > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, > > @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > reinterpret_cast<rkisp1_stat_buffer *>( > > mappedBuffers_.at(bufferId).planes()[0].data()); > > > > - context_.frameContext.sensor.exposure = > > + context_.activeState.sensor.exposure = > > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>(); > > - context_.frameContext.sensor.gain = > > + context_.activeState.sensor.gain = > > camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>()); > > > > unsigned int aeState = 0; > > @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > > > void IPARkISP1::setControls(unsigned int frame) > > { > > - uint32_t exposure = context_.frameContext.agc.exposure; > > - uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain); > > + uint32_t exposure = context_.activeState.agc.exposure; > > + uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain); > > > > ControlList ctrls(ctrls_); > > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
Quoting Laurent Pinchart (2022-08-24 01:37:02) > On Thu, Aug 18, 2022 at 05:07:27PM +0100, Kieran Bingham via libcamera-devel wrote: > > Quoting Jacopo Mondi via libcamera-devel (2022-08-18 10:43:55) > > > From: Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> > > > > > > Move the existing frame context structure to the IPAActiveState. > > > This structure should store the most up to date results for the IPA > > > which may be shared to other algorithms that operate on the data. > > > > Laurent queried this patch in v2. > > > > I suggested we add the following to this commit message: > > > > """ > > All existing algorithm state is moved to the ActiveState and the > > algorithms should be updated individually to use the FrameContext > > directly where appropriate. > > """ > > I'd like to make it clear (partly to check that my understanding is > correct :-)) that this commit is about moving the existing frame context > aside to make space for the new one. How about > > -------- > The RkISP1 IPA module creates a single instance of its IPAFrameContext > structure, effectively using it more as an active state than a per-frame > context. To prepare for the introduction of a real per-frame context, > move all the members of the IPAFrameContext structure to a new > IPAActiveState structure. The IPAFrameContext becomes effectively > unused at runtime, but can't be removed yet as it is still required as a > template argument to the Module and Algorithm classes. > > The new IPAActiveState structure is not foreseen to stay, its members > should move to either the new per-frame context that will be introduced, > or to the individual algorithm classes. Not foreseen might be too strong. I keep saying, I believe there may be good cause for it ActiveState to exist. But until we get progress to actually implement something real (blocked effectively by this series) I can't quantify exactly what. My expectations are that things like Modes (Is AWB enabled or in manual) are going to be in a shared IPAActiveState - as these may need to be retrieved or queried (or adjusted in the case of disabling a mode so that AF can operate) based on an active state, not a frame context. Otherwise, the first paragraph is fine with me. > -------- > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/ipa/rkisp1/algorithms/agc.cpp | 20 ++++++++-------- > > > src/ipa/rkisp1/algorithms/awb.cpp | 34 ++++++++++++++-------------- > > > src/ipa/rkisp1/algorithms/blc.cpp | 2 +- > > > src/ipa/rkisp1/algorithms/cproc.cpp | 4 ++-- > > > src/ipa/rkisp1/algorithms/dpcc.cpp | 2 +- > > > src/ipa/rkisp1/algorithms/filter.cpp | 4 ++-- > > > src/ipa/rkisp1/algorithms/gsl.cpp | 2 +- > > > src/ipa/rkisp1/algorithms/lsc.cpp | 2 +- > > > src/ipa/rkisp1/ipa_context.h | 7 ++++-- > > > src/ipa/rkisp1/rkisp1.cpp | 12 +++++----- > > > 10 files changed, 46 insertions(+), 43 deletions(-) > > > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > > index a1bb7d972926..483e941fe427 100644 > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > > @@ -73,8 +73,8 @@ Agc::Agc() > > > int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > > { > > > /* Configure the default exposure and gain. */ > > > - context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > > > - context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > > > + context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); > > > + context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; > > > > > > /* > > > * According to the RkISP1 documentation: > > > @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > > context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; > > > context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; > > > > > > - /* \todo Use actual frame index by populating it in the frameContext. */ > > > + /* \todo Use actual frame index by populating it in the activeState. */ > > That will go later in the series, but we could still keep it accurate in > the meantime by writing > > /* > * \todo Use the upcoming per-frame context API that will provide a > * frame index > */ > > > > frameCount_ = 0; > > > return 0; > > > } > > > @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) > > > > > > /** > > > * \brief Estimate the new exposure and gain values > > > - * \param[inout] frameContext The shared IPA frame Context > > > + * \param[inout] context The shared IPA Context > > And add to the commit message > > While at it, fix a typo in the documentation of the > Agc::computeExposure() function that incorrectly refers to the frame > context instead of the global context. > > > > * \param[in] yGain The gain calculated on the current brightness level > > > * \param[in] iqMeanGain The gain calculated based on the relative luminance target > > > */ > > > void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > > > { > > > IPASessionConfiguration &configuration = context.configuration; > > > - IPAFrameContext &frameContext = context.frameContext; > > > + IPAActiveState &activeState = context.activeState; > > > > > > /* Get the effective exposure and gain applied on the sensor. */ > > > - uint32_t exposure = frameContext.sensor.exposure; > > > - double analogueGain = frameContext.sensor.gain; > > > + uint32_t exposure = activeState.sensor.exposure; > > > + double analogueGain = activeState.sensor.gain; > > > > > > /* Use the highest of the two gain estimates. */ > > > double evGain = std::max(yGain, iqMeanGain); > > > @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) > > > << stepGain; > > > > > > /* Update the estimated exposure and gain. */ > > > - frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration; > > > - frameContext.agc.gain = stepGain; > > > + activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; > > > + activeState.agc.gain = stepGain; > > > } > > > > > > /** > > > @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context, > > > */ > > > void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params) > > > { > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > /* Configure the measurement window. */ > > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > > > index 9f00364d12b1..427aaeb1e955 100644 > > > --- a/src/ipa/rkisp1/algorithms/awb.cpp > > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > > > @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) > > > int Awb::configure(IPAContext &context, > > > const IPACameraSensorInfo &configInfo) > > > { > > > - context.frameContext.awb.gains.red = 1.0; > > > - context.frameContext.awb.gains.blue = 1.0; > > > - context.frameContext.awb.gains.green = 1.0; > > > + context.activeState.awb.gains.red = 1.0; > > > + context.activeState.awb.gains.blue = 1.0; > > > + context.activeState.awb.gains.green = 1.0; > > > > > > /* > > > * Define the measurement window for AWB as a centered rectangle > > > @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > > > */ > > > void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) > > > { > > > - params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green; > > > - params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue; > > > - params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red; > > > - params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green; > > > + params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green; > > > + params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue; > > > + params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red; > > > + params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green; > > > > > > /* Update the gains. */ > > > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; > > > > > > /* If we already have configured the gains and window, return. */ > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > /* Configure the gains to apply. */ > > > @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context, > > > { > > > const rkisp1_cif_isp_stat *params = &stats->params; > > > const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; > > > - IPAFrameContext &frameContext = context.frameContext; > > > + IPAActiveState &activeState = context.activeState; > > > > > > /* Get the YCbCr mean values */ > > > double yMean = awb->awb_mean[0].mean_y_or_g; > > > @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context, > > > > > > /* Filter the values to avoid oscillations. */ > > > double speed = 0.2; > > > - redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red; > > > - blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue; > > > + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; > > > + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; > > > > > > /* > > > * Gain values are unsigned integer value, range 0 to 4 with 8 bit > > > * fractional part. > > > */ > > > - frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > > > - frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > > > + activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); > > > + activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > > > /* Hardcode the green gain to 1.0. */ > > > - frameContext.awb.gains.green = 1.0; > > > + activeState.awb.gains.green = 1.0; > > > > > > - frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > > > + activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > > > > > > - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red > > > - << " and for blue: " << context.frameContext.awb.gains.blue; > > > + LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red > > > + << " and for blue: " << context.activeState.awb.gains.blue; > > > } > > > > > > REGISTER_IPA_ALGORITHM(Awb, "Awb") > > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp > > > index a58569fa2dc2..4d55a2d529cb 100644 > > > --- a/src/ipa/rkisp1/algorithms/blc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp > > > @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, > > > void BlackLevelCorrection::prepare(IPAContext &context, > > > rkisp1_params_cfg *params) > > > { > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > if (!tuningParameters_) > > > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp > > > index bca5ab6907d6..a3b778d1c908 100644 > > > --- a/src/ipa/rkisp1/algorithms/cproc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp > > > @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > > > [[maybe_unused]] const uint32_t frame, > > > const ControlList &controls) > > > { > > > - auto &cproc = context.frameContext.cproc; > > > + auto &cproc = context.activeState.cproc; > > > > > > const auto &brightness = controls.get(controls::Brightness); > > > if (brightness) { > > > @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context, > > > void ColorProcessing::prepare(IPAContext &context, > > > rkisp1_params_cfg *params) > > > { > > > - auto &cproc = context.frameContext.cproc; > > > + auto &cproc = context.activeState.cproc; > > > > > > /* Check if the algorithm configuration has been updated. */ > > > if (!cproc.updateParams) > > > diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp > > > index 69bc651eaf08..93d37b1dae44 100644 > > > --- a/src/ipa/rkisp1/algorithms/dpcc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp > > > @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context, > > > void DefectPixelClusterCorrection::prepare(IPAContext &context, > > > rkisp1_params_cfg *params) > > > { > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > if (!initialized_) > > > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp > > > index 8ca10fd1ee9d..bc7fc1f32f34 100644 > > > --- a/src/ipa/rkisp1/algorithms/filter.cpp > > > +++ b/src/ipa/rkisp1/algorithms/filter.cpp > > > @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context, > > > [[maybe_unused]] const uint32_t frame, > > > const ControlList &controls) > > > { > > > - auto &filter = context.frameContext.filter; > > > + auto &filter = context.activeState.filter; > > > > > > const auto &sharpness = controls.get(controls::Sharpness); > > > if (sharpness) { > > > @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context, > > > */ > > > void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params) > > > { > > > - auto &filter = context.frameContext.filter; > > > + auto &filter = context.activeState.filter; > > > > > > /* Check if the algorithm configuration has been updated. */ > > > if (!filter.updateParams) > > > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp > > > index 2fd1a23d3a9b..dd9974627cd2 100644 > > > --- a/src/ipa/rkisp1/algorithms/gsl.cpp > > > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp > > > @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, > > > void GammaSensorLinearization::prepare(IPAContext &context, > > > rkisp1_params_cfg *params) > > > { > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > if (!initialized_) > > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > > > index 05c8c0dab5c8..4ed467086199 100644 > > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > > > @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > > > void LensShadingCorrection::prepare(IPAContext &context, > > > rkisp1_params_cfg *params) > > > { > > > - if (context.frameContext.frameCount > 0) > > > + if (context.activeState.frameCount > 0) > > > return; > > > > > > if (!initialized_) > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > > index 2bdb6a81d7c9..a8daeca487ae 100644 > > > --- a/src/ipa/rkisp1/ipa_context.h > > > +++ b/src/ipa/rkisp1/ipa_context.h > > > @@ -41,7 +41,7 @@ struct IPASessionConfiguration { > > > } hw; > > > }; > > > > > > -struct IPAFrameContext { > > > +struct IPAActiveState { > > > struct { > > > uint32_t exposure; > > > double gain; > > > @@ -78,9 +78,12 @@ struct IPAFrameContext { > > > unsigned int frameCount; > > > }; > > > > > > +struct IPAFrameContext { > > > +}; > > > + > > > struct IPAContext { > > > IPASessionConfiguration configuration; > > > - IPAFrameContext frameContext; > > > + IPAActiveState activeState; > > > }; > > The documentation in ipa_context.cpp needs to be updated too. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > This patch could go the very front of the series. > > > > > > > } /* namespace ipa::rkisp1 */ > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > > index 17d42d38eb45..88e6a2b23eed 100644 > > > --- a/src/ipa/rkisp1/rkisp1.cpp > > > +++ b/src/ipa/rkisp1/rkisp1.cpp > > > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, > > > context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); > > > context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); > > > > > > - context_.frameContext.frameCount = 0; > > > + context_.activeState.frameCount = 0; > > > > > > for (auto const &algo : algorithms()) { > > > int ret = algo->configure(context_, info); > > > @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) > > > algo->prepare(context_, params); > > > > > > paramsBufferReady.emit(frame); > > > - context_.frameContext.frameCount++; > > > + context_.activeState.frameCount++; > > > } > > > > > > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, > > > @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > > reinterpret_cast<rkisp1_stat_buffer *>( > > > mappedBuffers_.at(bufferId).planes()[0].data()); > > > > > > - context_.frameContext.sensor.exposure = > > > + context_.activeState.sensor.exposure = > > > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>(); > > > - context_.frameContext.sensor.gain = > > > + context_.activeState.sensor.gain = > > > camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>()); > > > > > > unsigned int aeState = 0; > > > @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId > > > > > > void IPARkISP1::setControls(unsigned int frame) > > > { > > > - uint32_t exposure = context_.frameContext.agc.exposure; > > > - uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain); > > > + uint32_t exposure = context_.activeState.agc.exposure; > > > + uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain); > > > > > > ControlList ctrls(ctrls_); > > > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure)); > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index a1bb7d972926..483e941fe427 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -73,8 +73,8 @@ Agc::Agc() int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { /* Configure the default exposure and gain. */ - context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); - context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration; + context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); + context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; /* * According to the RkISP1 documentation: @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; - /* \todo Use actual frame index by populating it in the frameContext. */ + /* \todo Use actual frame index by populating it in the activeState. */ frameCount_ = 0; return 0; } @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue) /** * \brief Estimate the new exposure and gain values - * \param[inout] frameContext The shared IPA frame Context + * \param[inout] context The shared IPA Context * \param[in] yGain The gain calculated on the current brightness level * \param[in] iqMeanGain The gain calculated based on the relative luminance target */ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) { IPASessionConfiguration &configuration = context.configuration; - IPAFrameContext &frameContext = context.frameContext; + IPAActiveState &activeState = context.activeState; /* Get the effective exposure and gain applied on the sensor. */ - uint32_t exposure = frameContext.sensor.exposure; - double analogueGain = frameContext.sensor.gain; + uint32_t exposure = activeState.sensor.exposure; + double analogueGain = activeState.sensor.gain; /* Use the highest of the two gain estimates. */ double evGain = std::max(yGain, iqMeanGain); @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain) << stepGain; /* Update the estimated exposure and gain. */ - frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration; - frameContext.agc.gain = stepGain; + activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; + activeState.agc.gain = stepGain; } /** @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context, */ void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; /* Configure the measurement window. */ diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 9f00364d12b1..427aaeb1e955 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) int Awb::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { - context.frameContext.awb.gains.red = 1.0; - context.frameContext.awb.gains.blue = 1.0; - context.frameContext.awb.gains.green = 1.0; + context.activeState.awb.gains.red = 1.0; + context.activeState.awb.gains.blue = 1.0; + context.activeState.awb.gains.green = 1.0; /* * Define the measurement window for AWB as a centered rectangle @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) */ void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params) { - params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green; - params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue; - params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red; - params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green; + params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green; + params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue; + params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red; + params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green; /* Update the gains. */ params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; /* If we already have configured the gains and window, return. */ - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; /* Configure the gains to apply. */ @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context, { const rkisp1_cif_isp_stat *params = &stats->params; const rkisp1_cif_isp_awb_stat *awb = ¶ms->awb; - IPAFrameContext &frameContext = context.frameContext; + IPAActiveState &activeState = context.activeState; /* Get the YCbCr mean values */ double yMean = awb->awb_mean[0].mean_y_or_g; @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context, /* Filter the values to avoid oscillations. */ double speed = 0.2; - redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red; - blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue; + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; /* * Gain values are unsigned integer value, range 0 to 4 with 8 bit * fractional part. */ - frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); - frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); + activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256); + activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); /* Hardcode the green gain to 1.0. */ - frameContext.awb.gains.green = 1.0; + activeState.awb.gains.green = 1.0; - frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); + activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red - << " and for blue: " << context.frameContext.awb.gains.blue; + LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red + << " and for blue: " << context.activeState.awb.gains.blue; } REGISTER_IPA_ALGORITHM(Awb, "Awb") diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp index a58569fa2dc2..4d55a2d529cb 100644 --- a/src/ipa/rkisp1/algorithms/blc.cpp +++ b/src/ipa/rkisp1/algorithms/blc.cpp @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, void BlackLevelCorrection::prepare(IPAContext &context, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!tuningParameters_) diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp index bca5ab6907d6..a3b778d1c908 100644 --- a/src/ipa/rkisp1/algorithms/cproc.cpp +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -40,7 +40,7 @@ void ColorProcessing::queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame, const ControlList &controls) { - auto &cproc = context.frameContext.cproc; + auto &cproc = context.activeState.cproc; const auto &brightness = controls.get(controls::Brightness); if (brightness) { @@ -73,7 +73,7 @@ void ColorProcessing::queueRequest(IPAContext &context, void ColorProcessing::prepare(IPAContext &context, rkisp1_params_cfg *params) { - auto &cproc = context.frameContext.cproc; + auto &cproc = context.activeState.cproc; /* Check if the algorithm configuration has been updated. */ if (!cproc.updateParams) diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp index 69bc651eaf08..93d37b1dae44 100644 --- a/src/ipa/rkisp1/algorithms/dpcc.cpp +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp @@ -234,7 +234,7 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context, void DefectPixelClusterCorrection::prepare(IPAContext &context, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp index 8ca10fd1ee9d..bc7fc1f32f34 100644 --- a/src/ipa/rkisp1/algorithms/filter.cpp +++ b/src/ipa/rkisp1/algorithms/filter.cpp @@ -46,7 +46,7 @@ void Filter::queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame, const ControlList &controls) { - auto &filter = context.frameContext.filter; + auto &filter = context.activeState.filter; const auto &sharpness = controls.get(controls::Sharpness); if (sharpness) { @@ -87,7 +87,7 @@ void Filter::queueRequest(IPAContext &context, */ void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params) { - auto &filter = context.frameContext.filter; + auto &filter = context.activeState.filter; /* Check if the algorithm configuration has been updated. */ if (!filter.updateParams) diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp index 2fd1a23d3a9b..dd9974627cd2 100644 --- a/src/ipa/rkisp1/algorithms/gsl.cpp +++ b/src/ipa/rkisp1/algorithms/gsl.cpp @@ -121,7 +121,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, void GammaSensorLinearization::prepare(IPAContext &context, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 05c8c0dab5c8..4ed467086199 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -125,7 +125,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, void LensShadingCorrection::prepare(IPAContext &context, rkisp1_params_cfg *params) { - if (context.frameContext.frameCount > 0) + if (context.activeState.frameCount > 0) return; if (!initialized_) diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 2bdb6a81d7c9..a8daeca487ae 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -41,7 +41,7 @@ struct IPASessionConfiguration { } hw; }; -struct IPAFrameContext { +struct IPAActiveState { struct { uint32_t exposure; double gain; @@ -78,9 +78,12 @@ struct IPAFrameContext { unsigned int frameCount; }; +struct IPAFrameContext { +}; + struct IPAContext { IPASessionConfiguration configuration; - IPAFrameContext frameContext; + IPAActiveState activeState; }; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 17d42d38eb45..88e6a2b23eed 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); - context_.frameContext.frameCount = 0; + context_.activeState.frameCount = 0; for (auto const &algo : algorithms()) { int ret = algo->configure(context_, info); @@ -306,7 +306,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) algo->prepare(context_, params); paramsBufferReady.emit(frame); - context_.frameContext.frameCount++; + context_.activeState.frameCount++; } void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, @@ -316,9 +316,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId reinterpret_cast<rkisp1_stat_buffer *>( mappedBuffers_.at(bufferId).planes()[0].data()); - context_.frameContext.sensor.exposure = + context_.activeState.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>(); - context_.frameContext.sensor.gain = + context_.activeState.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>()); unsigned int aeState = 0; @@ -333,8 +333,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId void IPARkISP1::setControls(unsigned int frame) { - uint32_t exposure = context_.frameContext.agc.exposure; - uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain); + uint32_t exposure = context_.activeState.agc.exposure; + uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain); ControlList ctrls(ctrls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));