Message ID | 20220927023642.12341-22-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Laurent, On Tue, Sep 27, 2022 at 05:36:30AM +0300, Laurent Pinchart via libcamera-devel wrote: > Rework the algorithm's usage of the active state and frame context to > store data in the right place. > > The active state stores two distinct categories of information: > > - The consolidated value of all algorithm controls. Requests passed to > the queueRequest() function store values for controls that the > application wants to modify for that particular frame, and the > queueRequest() function updates the active state with those values. > The active state thus contains a consolidated view of the value of all > controls handled by the algorithm. > > - The value of parameters computed by the algorithm when running in auto > mode. Algorithms running in auto mode compute new parameters every > time statistics buffers are received (either synchronously, or > possibly in a background thread). The latest computed value of those > parameters is stored in the active state in the process() function. > > The frame context also stores two categories of information: > > - The value of the controls to be applied to the frame. These values are > typically set in the queueRequest() function, from the consolidated > control values stored in the active state. The frame context thus > stores values for all controls related to the algorithm, not limited > to the controls specified in the corresponding request, but > consolidated from all requests that have been queued so far. > > For controls that can be specified manually or computed by the > algorithm depending on the operation mode (such as the colour gains), > the control value will be stored in the frame context in > queueRequest() only when operating in manual mode. When operating in > auto mode, the values are computed by the algorithm and stored in the > frame context in prepare(), just before being stored in the ISP > parameters buffer. > > The queueRequest() function can also store ancillary data in the frame > context, such as flags to indicate if (and what) control values have > changed compared to the previous request. > > - Status information computed by the algorithm for a frame. For > instance, the colour temperature estimated by the algorithm from ISP > statistics calculated on a frame is stored in the frame context for > that frame in the process() function. > > The active state and frame context thus both contain identical members > for most control values, but store values that have a different meaning. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/awb.cpp | 75 +++++++++++++++++++------------ > src/ipa/rkisp1/ipa_context.cpp | 51 +++++++++++++++++---- > src/ipa/rkisp1/ipa_context.h | 25 +++++++++-- > 3 files changed, 110 insertions(+), 41 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 78398927e462..e491cf7507e0 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -36,9 +36,12 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) > int Awb::configure(IPAContext &context, > const IPACameraSensorInfo &configInfo) > { > - context.activeState.awb.gains.red = 1.0; > - context.activeState.awb.gains.blue = 1.0; > - context.activeState.awb.gains.green = 1.0; > + context.activeState.awb.gains.manual.red = 1.0; > + context.activeState.awb.gains.manual.blue = 1.0; > + context.activeState.awb.gains.manual.green = 1.0; > + context.activeState.awb.gains.automatic.red = 1.0; > + context.activeState.awb.gains.automatic.blue = 1.0; > + context.activeState.awb.gains.automatic.green = 1.0; > context.activeState.awb.autoEnabled = true; > > /* > @@ -75,13 +78,22 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > * \copydoc libcamera::ipa::Algorithm::prepare > */ > void Awb::prepare(IPAContext &context, const uint32_t frame, > - [[maybe_unused]] IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) > + IPAFrameContext &frameContext, rkisp1_params_cfg *params) > { > - 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; > + /* > + * This is the latest time we can read the active state. This is the > + * most up-to-date automatic values we can read. > + */ > + if (frameContext.awb.autoEnabled) { > + frameContext.awb.gains.red = context.activeState.awb.gains.automatic.red; > + frameContext.awb.gains.green = context.activeState.awb.gains.automatic.green; > + frameContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue; > + } > + > + params->others.awb_gain_config.gain_green_b = 256 * frameContext.awb.gains.green; > + params->others.awb_gain_config.gain_blue = 256 * frameContext.awb.gains.blue; > + params->others.awb_gain_config.gain_red = 256 * frameContext.awb.gains.red; > + params->others.awb_gain_config.gain_green_r = 256 * frameContext.awb.gains.green; > > /* Update the gains. */ > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; > @@ -127,7 +139,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, > */ > void Awb::queueRequest(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > - [[maybe_unused]] IPAFrameContext &frameContext, > + IPAFrameContext &frameContext, > const ControlList &controls) > { > auto &awb = context.activeState.awb; > @@ -142,21 +154,29 @@ void Awb::queueRequest(IPAContext &context, > > const auto &colourGains = controls.get(controls::ColourGains); > if (colourGains && !awb.autoEnabled) { > - awb.gains.red = (*colourGains)[0]; > - awb.gains.blue = (*colourGains)[1]; > + awb.gains.manual.red = (*colourGains)[0]; > + awb.gains.manual.blue = (*colourGains)[1]; > > LOG(RkISP1Awb, Debug) > - << "Set colour gains to red: " << awb.gains.red > - << ", blue: " << awb.gains.blue; > + << "Set colour gains to red: " << awb.gains.manual.red > + << ", blue: " << awb.gains.manual.blue; > + } > + > + frameContext.awb.autoEnabled = awb.autoEnabled; > + > + if (!awb.autoEnabled) { > + frameContext.awb.gains.red = awb.gains.manual.red; > + frameContext.awb.gains.green = 1.0; > + frameContext.awb.gains.blue = awb.gains.manual.blue; Do I read it wrong or if (!awb.autoEnabled && !colourGains) we here initialize frameContext.awb.gains with an un-initialized awb.gains.manual ? > } > > /** > * \copydoc libcamera::ipa::Algorithm::process > */ > -void Awb::process([[maybe_unused]] IPAContext &context, > +void Awb::process(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > - [[maybe_unused]] IPAFrameContext &frameCtx, > + IPAFrameContext &frameContext, > const rkisp1_stat_buffer *stats) > { > const rkisp1_cif_isp_stat *params = &stats->params; > @@ -187,30 +207,27 @@ void Awb::process([[maybe_unused]] IPAContext &context, > double greenMean = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean; > double blueMean = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean; > > + frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > + > /* Estimate the red and blue gains to apply in a grey world. */ > double redGain = greenMean / (redMean + 1); > double blueGain = greenMean / (blueMean + 1); > > /* Filter the values to avoid oscillations. */ > double speed = 0.2; > - redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; > - blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; > + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red; > + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue; > > /* > * Gain values are unsigned integer value, range 0 to 4 with 8 bit > - * fractional part. > + * fractional part. Hardcode the green gain to 1.0. > */ > - if (activeState.awb.autoEnabled) { > - 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. */ > - activeState.awb.gains.green = 1.0; > + activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256); > + activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); > + activeState.awb.gains.automatic.green = 1.0; > > - activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); > - > - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red > - << " and for blue: " << context.activeState.awb.gains.blue; > + LOG(RkISP1Awb, Debug) << "Gain found for red: " << activeState.awb.gains.automatic.red > + << " and for blue: " << activeState.awb.gains.automatic.blue; > } > > REGISTER_IPA_ALGORITHM(Awb, "Awb") > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp > index c7d5b1b6ec5a..ba80a0707ef7 100644 > --- a/src/ipa/rkisp1/ipa_context.cpp > +++ b/src/ipa/rkisp1/ipa_context.cpp > @@ -120,17 +120,29 @@ namespace libcamera::ipa::rkisp1 { > * \struct IPAActiveState::awb.gains > * \brief White balance gains > * > - * \var IPAActiveState::awb.gains.red > - * \brief White balance gain for R channel > + * \struct IPAActiveState::awb.gains.manual > + * \brief Manual white balance gains (set through requests) > * > - * \var IPAActiveState::awb.gains.green > - * \brief White balance gain for G channel > + * \var IPAActiveState::awb.gains.manual.red > + * \brief Manual white balance gain for R channel > * > - * \var IPAActiveState::awb.gains.blue > - * \brief White balance gain for B channel > + * \var IPAActiveState::awb.gains.manual.green > + * \brief Manual white balance gain for G channel > * > - * \var IPAActiveState::awb.temperatureK > - * \brief Estimated color temperature > + * \var IPAActiveState::awb.gains.manual.blue > + * \brief Manual white balance gain for B channel > + * > + * \struct IPAActiveState::awb.gains.automatic > + * \brief Automatic white balance gains (computed by the algorithm) > + * > + * \var IPAActiveState::awb.gains.automatic.red > + * \brief Automatic white balance gain for R channel > + * > + * \var IPAActiveState::awb.gains.automatic.green > + * \brief Automatic white balance gain for G channel > + * > + * \var IPAActiveState::awb.gains.automatic.blue > + * \brief Automatic white balance gain for B channel > * > * \var IPAActiveState::awb.autoEnabled > * \brief Whether the Auto White Balance algorithm is enabled > @@ -201,6 +213,29 @@ namespace libcamera::ipa::rkisp1 { > * The gain should be adapted to the sensor specific gain code before applying. > */ > > +/** > + * \var IPAFrameContext::awb > + * \brief Automatic White Balance parameters for this frame > + * > + * \struct IPAFrameContext::awb.gains > + * \brief White balance gains > + * > + * \var IPAFrameContext::awb.gains.red > + * \brief White balance gain for R channel > + * > + * \var IPAFrameContext::awb.gains.green > + * \brief White balance gain for G channel > + * > + * \var IPAFrameContext::awb.gains.blue > + * \brief White balance gain for B channel > + * > + * \var IPAFrameContext::awb.temperatureK > + * \brief Estimated color temperature > + * > + * \var IPAFrameContext::awb.autoEnabled > + * \brief Whether the Auto White Balance algorithm is enabled > + */ > + > /** > * \var IPAFrameContext::sensor > * \brief Sensor configuration that used been used for this frame > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index a4d134e700b5..f23ebb2c9004 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -57,12 +57,18 @@ struct IPAActiveState { > > struct { > struct { > - double red; > - double green; > - double blue; > + struct { > + double red; > + double green; > + double blue; > + } manual; > + struct { > + double red; > + double green; > + double blue; > + } automatic; > } gains; > > - double temperatureK; > bool autoEnabled; > } awb; > > @@ -91,6 +97,17 @@ struct IPAFrameContext : public FrameContext { > double gain; > } agc; > > + struct { > + struct { > + double red; > + double green; > + double blue; > + } gains; > + > + double temperatureK; > + bool autoEnabled; > + } awb; > + > struct { > uint32_t exposure; > double gain; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 78398927e462..e491cf7507e0 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -36,9 +36,12 @@ LOG_DEFINE_CATEGORY(RkISP1Awb) int Awb::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { - context.activeState.awb.gains.red = 1.0; - context.activeState.awb.gains.blue = 1.0; - context.activeState.awb.gains.green = 1.0; + context.activeState.awb.gains.manual.red = 1.0; + context.activeState.awb.gains.manual.blue = 1.0; + context.activeState.awb.gains.manual.green = 1.0; + context.activeState.awb.gains.automatic.red = 1.0; + context.activeState.awb.gains.automatic.blue = 1.0; + context.activeState.awb.gains.automatic.green = 1.0; context.activeState.awb.autoEnabled = true; /* @@ -75,13 +78,22 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) * \copydoc libcamera::ipa::Algorithm::prepare */ void Awb::prepare(IPAContext &context, const uint32_t frame, - [[maybe_unused]] IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + IPAFrameContext &frameContext, rkisp1_params_cfg *params) { - 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; + /* + * This is the latest time we can read the active state. This is the + * most up-to-date automatic values we can read. + */ + if (frameContext.awb.autoEnabled) { + frameContext.awb.gains.red = context.activeState.awb.gains.automatic.red; + frameContext.awb.gains.green = context.activeState.awb.gains.automatic.green; + frameContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue; + } + + params->others.awb_gain_config.gain_green_b = 256 * frameContext.awb.gains.green; + params->others.awb_gain_config.gain_blue = 256 * frameContext.awb.gains.blue; + params->others.awb_gain_config.gain_red = 256 * frameContext.awb.gains.red; + params->others.awb_gain_config.gain_green_r = 256 * frameContext.awb.gains.green; /* Update the gains. */ params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; @@ -127,7 +139,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, */ void Awb::queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame, - [[maybe_unused]] IPAFrameContext &frameContext, + IPAFrameContext &frameContext, const ControlList &controls) { auto &awb = context.activeState.awb; @@ -142,21 +154,29 @@ void Awb::queueRequest(IPAContext &context, const auto &colourGains = controls.get(controls::ColourGains); if (colourGains && !awb.autoEnabled) { - awb.gains.red = (*colourGains)[0]; - awb.gains.blue = (*colourGains)[1]; + awb.gains.manual.red = (*colourGains)[0]; + awb.gains.manual.blue = (*colourGains)[1]; LOG(RkISP1Awb, Debug) - << "Set colour gains to red: " << awb.gains.red - << ", blue: " << awb.gains.blue; + << "Set colour gains to red: " << awb.gains.manual.red + << ", blue: " << awb.gains.manual.blue; + } + + frameContext.awb.autoEnabled = awb.autoEnabled; + + if (!awb.autoEnabled) { + frameContext.awb.gains.red = awb.gains.manual.red; + frameContext.awb.gains.green = 1.0; + frameContext.awb.gains.blue = awb.gains.manual.blue; } } /** * \copydoc libcamera::ipa::Algorithm::process */ -void Awb::process([[maybe_unused]] IPAContext &context, +void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, - [[maybe_unused]] IPAFrameContext &frameCtx, + IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats) { const rkisp1_cif_isp_stat *params = &stats->params; @@ -187,30 +207,27 @@ void Awb::process([[maybe_unused]] IPAContext &context, double greenMean = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean; double blueMean = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean; + frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); + /* Estimate the red and blue gains to apply in a grey world. */ double redGain = greenMean / (redMean + 1); double blueGain = greenMean / (blueMean + 1); /* Filter the values to avoid oscillations. */ double speed = 0.2; - redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red; - blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue; + redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red; + blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue; /* * Gain values are unsigned integer value, range 0 to 4 with 8 bit - * fractional part. + * fractional part. Hardcode the green gain to 1.0. */ - if (activeState.awb.autoEnabled) { - 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. */ - activeState.awb.gains.green = 1.0; + activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256); + activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256); + activeState.awb.gains.automatic.green = 1.0; - activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean); - - LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red - << " and for blue: " << context.activeState.awb.gains.blue; + LOG(RkISP1Awb, Debug) << "Gain found for red: " << activeState.awb.gains.automatic.red + << " and for blue: " << activeState.awb.gains.automatic.blue; } REGISTER_IPA_ALGORITHM(Awb, "Awb") diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index c7d5b1b6ec5a..ba80a0707ef7 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -120,17 +120,29 @@ namespace libcamera::ipa::rkisp1 { * \struct IPAActiveState::awb.gains * \brief White balance gains * - * \var IPAActiveState::awb.gains.red - * \brief White balance gain for R channel + * \struct IPAActiveState::awb.gains.manual + * \brief Manual white balance gains (set through requests) * - * \var IPAActiveState::awb.gains.green - * \brief White balance gain for G channel + * \var IPAActiveState::awb.gains.manual.red + * \brief Manual white balance gain for R channel * - * \var IPAActiveState::awb.gains.blue - * \brief White balance gain for B channel + * \var IPAActiveState::awb.gains.manual.green + * \brief Manual white balance gain for G channel * - * \var IPAActiveState::awb.temperatureK - * \brief Estimated color temperature + * \var IPAActiveState::awb.gains.manual.blue + * \brief Manual white balance gain for B channel + * + * \struct IPAActiveState::awb.gains.automatic + * \brief Automatic white balance gains (computed by the algorithm) + * + * \var IPAActiveState::awb.gains.automatic.red + * \brief Automatic white balance gain for R channel + * + * \var IPAActiveState::awb.gains.automatic.green + * \brief Automatic white balance gain for G channel + * + * \var IPAActiveState::awb.gains.automatic.blue + * \brief Automatic white balance gain for B channel * * \var IPAActiveState::awb.autoEnabled * \brief Whether the Auto White Balance algorithm is enabled @@ -201,6 +213,29 @@ namespace libcamera::ipa::rkisp1 { * The gain should be adapted to the sensor specific gain code before applying. */ +/** + * \var IPAFrameContext::awb + * \brief Automatic White Balance parameters for this frame + * + * \struct IPAFrameContext::awb.gains + * \brief White balance gains + * + * \var IPAFrameContext::awb.gains.red + * \brief White balance gain for R channel + * + * \var IPAFrameContext::awb.gains.green + * \brief White balance gain for G channel + * + * \var IPAFrameContext::awb.gains.blue + * \brief White balance gain for B channel + * + * \var IPAFrameContext::awb.temperatureK + * \brief Estimated color temperature + * + * \var IPAFrameContext::awb.autoEnabled + * \brief Whether the Auto White Balance algorithm is enabled + */ + /** * \var IPAFrameContext::sensor * \brief Sensor configuration that used been used for this frame diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index a4d134e700b5..f23ebb2c9004 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -57,12 +57,18 @@ struct IPAActiveState { struct { struct { - double red; - double green; - double blue; + struct { + double red; + double green; + double blue; + } manual; + struct { + double red; + double green; + double blue; + } automatic; } gains; - double temperatureK; bool autoEnabled; } awb; @@ -91,6 +97,17 @@ struct IPAFrameContext : public FrameContext { double gain; } agc; + struct { + struct { + double red; + double green; + double blue; + } gains; + + double temperatureK; + bool autoEnabled; + } awb; + struct { uint32_t exposure; double gain;