[libcamera-devel] [PATCH v2 1/2] ipa: rkisp1: Compute LSC algorithm parameter during configure
Jacopo Mondi
jacopo at jmondi.org
Fri Sep 30 11:23:16 CEST 2022
Can this be rebased on "ipa: rkisp1: Remove initialized_ flags from
algorithms" ?
For the patch
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
On Fri, Sep 30, 2022 at 04:22:23AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Hi Florian,
>
> Thank you for the patch.
>
> On Thu, Sep 29, 2022 at 11:12:44AM +0200, Florian Sylvestre via libcamera-devel wrote:
> > LSC gradient parameters where initially computed during prepare() phase.
>
> s/where/were/
>
> But actually, as the commit message describes the current situation,
>
> LSC gradient parameters are currently computed during the prepare() phase.
>
> > Because these parameters can be computed only one time and stays constant for
>
> s/stays/stay/
>
> > each frame after: move the computation to the configure() function.
>
> s/:/,/
>
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > ---
> > src/ipa/rkisp1/algorithms/lsc.cpp | 66 ++++++++++++++++++-------------
> > src/ipa/rkisp1/algorithms/lsc.h | 4 ++
> > 2 files changed, 42 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > index 44245caa..b1f39dfd 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > @@ -125,30 +125,15 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
> > int LensShadingCorrection::configure(IPAContext &context,
> > [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> > {
> > - context.configuration.lsc.enabled = initialized_;
> > - return 0;
> > -}
> > -
> > -/**
> > - * \copydoc libcamera::ipa::Algorithm::prepare
> > - */
> > -void LensShadingCorrection::prepare(IPAContext &context, const uint32_t frame,
> > - [[maybe_unused]] IPAFrameContext &frameContext,
> > - rkisp1_params_cfg *params)
> > -{
> > - if (frame > 0)
> > - return;
> > -
> > if (!initialized_)
> > - return;
> > + return EINVAL;
>
> This is a change of behaviour, as configure() previously returned 0 in
> that case, and the return value of configure() is checked by the caller.
>
> This being said, it looks like we could drop the initialized_ flag, here
> and in the other algorithms. It can only be false if init() returns an
> error, and in that case the IPA module initialization fails. I recall we
> introduced the initialized_ flag for a reason, but I can't locate any
> point in the history where it was actually useful. I'll submit a patch.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> >
> > - struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
> > const Size &size = context.configuration.sensor.size;
> > Size totalSize{};
> >
> > for (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {
> > - config.x_size_tbl[i] = xSize_[i] * size.width;
> > - config.y_size_tbl[i] = ySize_[i] * size.height;
> > + xSizes_[i] = xSize_[i] * size.width;
> > + ySizes_[i] = ySize_[i] * size.height;
> >
> > /*
> > * To prevent unexpected behavior of the ISP, the sum of x_size_tbl and
> > @@ -157,25 +142,50 @@ void LensShadingCorrection::prepare(IPAContext &context, const uint32_t frame,
> > * rounding-induced errors.
> > */
> > if (i == RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE - 1) {
> > - config.x_size_tbl[i] = size.width / 2 - totalSize.width;
> > - config.y_size_tbl[i] = size.height / 2 - totalSize.height;
> > + xSizes_[i] = size.width / 2 - totalSize.width;
> > + ySizes_[i] = size.height / 2 - totalSize.height;
> > }
> >
> > - totalSize.width += config.x_size_tbl[i];
> > - totalSize.height += config.y_size_tbl[i];
> > + totalSize.width += xSizes_[i];
> > + totalSize.height += ySizes_[i];
> >
> > - config.x_grad_tbl[i] = std::round(32768 / config.x_size_tbl[i]);
> > - config.y_grad_tbl[i] = std::round(32768 / config.y_size_tbl[i]);
> > + xGrad_[i] = std::round(32768 / xSizes_[i]);
> > + yGrad_[i] = std::round(32768 / ySizes_[i]);
> > }
> >
> > + context.configuration.lsc.enabled = true;
> > + return 0;
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::prepare
> > + */
> > +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,
> > + const uint32_t frame,
> > + [[maybe_unused]] IPAFrameContext &frameContext,
> > + rkisp1_params_cfg *params)
> > +{
> > + if (frame > 0)
> > + return;
> > +
> > + if (!initialized_)
> > + return;
> > +
> > + struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config;
> > +
> > + memcpy(config.x_grad_tbl, xGrad_, sizeof(config.x_grad_tbl));
> > + memcpy(config.y_grad_tbl, yGrad_, sizeof(config.y_grad_tbl));
> > + memcpy(config.x_size_tbl, xSizes_, sizeof(config.x_size_tbl));
> > + memcpy(config.y_size_tbl, ySizes_, sizeof(config.y_size_tbl));
> > +
> > std::copy(rData_.begin(), rData_.end(),
> > - ¶ms->others.lsc_config.r_data_tbl[0][0]);
> > + &config.r_data_tbl[0][0]);
> > std::copy(grData_.begin(), grData_.end(),
> > - ¶ms->others.lsc_config.gr_data_tbl[0][0]);
> > + &config.gr_data_tbl[0][0]);
> > std::copy(gbData_.begin(), gbData_.end(),
> > - ¶ms->others.lsc_config.gb_data_tbl[0][0]);
> > + &config.gb_data_tbl[0][0]);
> > std::copy(bData_.begin(), bData_.end(),
> > - ¶ms->others.lsc_config.b_data_tbl[0][0]);
> > + &config.b_data_tbl[0][0]);
> >
> > params->module_en_update |= RKISP1_CIF_ISP_MODULE_LSC;
> > params->module_ens |= RKISP1_CIF_ISP_MODULE_LSC;
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> > index da957d3e..b5cd5047 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.h
> > +++ b/src/ipa/rkisp1/algorithms/lsc.h
> > @@ -35,6 +35,10 @@ private:
> >
> > std::vector<double> xSize_;
> > std::vector<double> ySize_;
> > + uint16_t xGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
> > + uint16_t yGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
> > + uint16_t xSizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
> > + uint16_t ySizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
> > };
> >
> > } /* namespace ipa::rkisp1::algorithms */
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list