ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor
diff mbox series

Message ID 20260621004115.3799303-1-laurent.pinchart@ideasonboard.com
State Rejected
Headers show
Series
  • ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor
Related show

Commit Message

Laurent Pinchart June 21, 2026, 12:41 a.m. UTC 
There's no need to inline the ValueNode::Accessor::get() function for
LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.

While at it, replace C++-style comments with C-style comments according
to the libcamera coding style.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
 src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
 2 files changed, 24 insertions(+), 26 deletions(-)


base-commit: bd266aa3405d3aea20b407b27e52482b02146030

Comments

Barnabás Pőcze June 22, 2026, 8:18 a.m. UTC | #1 
2026. 06. 21. 2:41 keltezéssel, Laurent Pinchart írta:
> There's no need to inline the ValueNode::Accessor::get() function for
> LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> 
> While at it, replace C++-style comments with C-style comments according
> to the libcamera coding style.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
>   src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
>   2 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> index f607d86c54c3..00569c2fd160 100644
> --- a/src/ipa/libipa/lsc_polynomial.cpp
> +++ b/src/ipa/libipa/lsc_polynomial.cpp
> @@ -77,5 +77,27 @@ namespace ipa {
>    * \param size The size of the reference image
>    */
>   
> -} // namespace ipa
> -} // namespace libcamera
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +template<>
> +std::optional<ipa::LscPolynomial>
> +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> +{
> +	std::optional<double> cx = obj["cx"].get<double>();
> +	std::optional<double> cy = obj["cy"].get<double>();
> +	std::optional<double> k0 = obj["k0"].get<double>();
> +	std::optional<double> k1 = obj["k1"].get<double>();
> +	std::optional<double> k2 = obj["k2"].get<double>();
> +	std::optional<double> k3 = obj["k3"].get<double>();
> +	std::optional<double> k4 = obj["k4"].get<double>();
> +
> +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> +		LOG(LscPolynomial, Error)
> +			<< "Polynomial is missing a parameter";
> +
> +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> +}
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> index d7d9ae42e360..d52ec86946b1 100644
> --- a/src/ipa/libipa/lsc_polynomial.h
> +++ b/src/ipa/libipa/lsc_polynomial.h
> @@ -80,28 +80,4 @@ private:
>   
>   } /* namespace ipa */
>   
> -#ifndef __DOXYGEN__
> -
> -template<>
> -struct ValueNode::Accessor<ipa::LscPolynomial> {
> -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> -	{
> -		std::optional<double> cx = obj["cx"].get<double>();
> -		std::optional<double> cy = obj["cy"].get<double>();
> -		std::optional<double> k0 = obj["k0"].get<double>();
> -		std::optional<double> k1 = obj["k1"].get<double>();
> -		std::optional<double> k2 = obj["k2"].get<double>();
> -		std::optional<double> k3 = obj["k3"].get<double>();
> -		std::optional<double> k4 = obj["k4"].get<double>();
> -
> -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> -			LOG(LscPolynomial, Error)
> -				<< "Polynomial is missing a parameter";
> -
> -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> -	}
> -};
> -
> -#endif
> -
>   } /* namespace libcamera */
> 
> base-commit: bd266aa3405d3aea20b407b27e52482b02146030
Kieran Bingham June 22, 2026, 8:44 a.m. UTC | #2 
Quoting Laurent Pinchart (2026-06-21 01:41:15)
> There's no need to inline the ValueNode::Accessor::get() function for
> LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> 
> While at it, replace C++-style comments with C-style comments according
> to the libcamera coding style.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
>  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
>  2 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> index f607d86c54c3..00569c2fd160 100644
> --- a/src/ipa/libipa/lsc_polynomial.cpp
> +++ b/src/ipa/libipa/lsc_polynomial.cpp
> @@ -77,5 +77,27 @@ namespace ipa {
>   * \param size The size of the reference image
>   */
>  
> -} // namespace ipa
> -} // namespace libcamera
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +template<>
> +std::optional<ipa::LscPolynomial>
> +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> +{
> +       std::optional<double> cx = obj["cx"].get<double>();
> +       std::optional<double> cy = obj["cy"].get<double>();
> +       std::optional<double> k0 = obj["k0"].get<double>();
> +       std::optional<double> k1 = obj["k1"].get<double>();
> +       std::optional<double> k2 = obj["k2"].get<double>();
> +       std::optional<double> k3 = obj["k3"].get<double>();
> +       std::optional<double> k4 = obj["k4"].get<double>();
> +
> +       if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> +               LOG(LscPolynomial, Error)
> +                       << "Polynomial is missing a parameter";
> +
> +       return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> +}
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> index d7d9ae42e360..d52ec86946b1 100644
> --- a/src/ipa/libipa/lsc_polynomial.h
> +++ b/src/ipa/libipa/lsc_polynomial.h
> @@ -80,28 +80,4 @@ private:
>  
>  } /* namespace ipa */
>  
> -#ifndef __DOXYGEN__
> -
> -template<>
> -struct ValueNode::Accessor<ipa::LscPolynomial> {
> -       std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> -       {
> -               std::optional<double> cx = obj["cx"].get<double>();
> -               std::optional<double> cy = obj["cy"].get<double>();
> -               std::optional<double> k0 = obj["k0"].get<double>();
> -               std::optional<double> k1 = obj["k1"].get<double>();
> -               std::optional<double> k2 = obj["k2"].get<double>();
> -               std::optional<double> k3 = obj["k3"].get<double>();
> -               std::optional<double> k4 = obj["k4"].get<double>();
> -
> -               if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> -                       LOG(LscPolynomial, Error)
> -                               << "Polynomial is missing a parameter";
> -
> -               return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> -       }
> -};
> -
> -#endif
> -
>  } /* namespace libcamera */
> 
> base-commit: bd266aa3405d3aea20b407b27e52482b02146030
> -- 
> Regards,
> 
> Laurent Pinchart
>
Jacopo Mondi June 22, 2026, 8:50 a.m. UTC | #3 
Hi

On Sun, Jun 21, 2026 at 03:41:15AM +0300, Laurent Pinchart wrote:
> There's no need to inline the ValueNode::Accessor::get() function for
> LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
>
> While at it, replace C++-style comments with C-style comments according
> to the libcamera coding style.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Can we hold on on (non bugfix) changes to this part considering it is
under rework?

> ---
>  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
>  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
>  2 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> index f607d86c54c3..00569c2fd160 100644
> --- a/src/ipa/libipa/lsc_polynomial.cpp
> +++ b/src/ipa/libipa/lsc_polynomial.cpp
> @@ -77,5 +77,27 @@ namespace ipa {
>   * \param size The size of the reference image
>   */
>
> -} // namespace ipa
> -} // namespace libcamera
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +template<>
> +std::optional<ipa::LscPolynomial>
> +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> +{
> +	std::optional<double> cx = obj["cx"].get<double>();
> +	std::optional<double> cy = obj["cy"].get<double>();
> +	std::optional<double> k0 = obj["k0"].get<double>();
> +	std::optional<double> k1 = obj["k1"].get<double>();
> +	std::optional<double> k2 = obj["k2"].get<double>();
> +	std::optional<double> k3 = obj["k3"].get<double>();
> +	std::optional<double> k4 = obj["k4"].get<double>();
> +
> +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> +		LOG(LscPolynomial, Error)
> +			<< "Polynomial is missing a parameter";
> +
> +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> +}
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> index d7d9ae42e360..d52ec86946b1 100644
> --- a/src/ipa/libipa/lsc_polynomial.h
> +++ b/src/ipa/libipa/lsc_polynomial.h
> @@ -80,28 +80,4 @@ private:
>
>  } /* namespace ipa */
>
> -#ifndef __DOXYGEN__
> -
> -template<>
> -struct ValueNode::Accessor<ipa::LscPolynomial> {
> -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> -	{
> -		std::optional<double> cx = obj["cx"].get<double>();
> -		std::optional<double> cy = obj["cy"].get<double>();
> -		std::optional<double> k0 = obj["k0"].get<double>();
> -		std::optional<double> k1 = obj["k1"].get<double>();
> -		std::optional<double> k2 = obj["k2"].get<double>();
> -		std::optional<double> k3 = obj["k3"].get<double>();
> -		std::optional<double> k4 = obj["k4"].get<double>();
> -
> -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> -			LOG(LscPolynomial, Error)
> -				<< "Polynomial is missing a parameter";
> -
> -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> -	}
> -};
> -
> -#endif
> -
>  } /* namespace libcamera */
>
> base-commit: bd266aa3405d3aea20b407b27e52482b02146030
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart June 22, 2026, 9:02 a.m. UTC | #4 
On Mon, Jun 22, 2026 at 10:50:09AM +0200, Jacopo Mondi wrote:
> On Sun, Jun 21, 2026 at 03:41:15AM +0300, Laurent Pinchart wrote:
> > There's no need to inline the ValueNode::Accessor::get() function for
> > LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> >
> > While at it, replace C++-style comments with C-style comments according
> > to the libcamera coding style.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Can we hold on on (non bugfix) changes to this part considering it is
> under rework?

If that hinders your work I don't mind.

> > ---
> >  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
> >  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
> >  2 files changed, 24 insertions(+), 26 deletions(-)
> >
> > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > index f607d86c54c3..00569c2fd160 100644
> > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > @@ -77,5 +77,27 @@ namespace ipa {
> >   * \param size The size of the reference image
> >   */
> >
> > -} // namespace ipa
> > -} // namespace libcamera
> > +} /* namespace ipa */
> > +
> > +#ifndef __DOXYGEN__
> > +template<>
> > +std::optional<ipa::LscPolynomial>
> > +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> > +{
> > +	std::optional<double> cx = obj["cx"].get<double>();
> > +	std::optional<double> cy = obj["cy"].get<double>();
> > +	std::optional<double> k0 = obj["k0"].get<double>();
> > +	std::optional<double> k1 = obj["k1"].get<double>();
> > +	std::optional<double> k2 = obj["k2"].get<double>();
> > +	std::optional<double> k3 = obj["k3"].get<double>();
> > +	std::optional<double> k4 = obj["k4"].get<double>();
> > +
> > +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > +		LOG(LscPolynomial, Error)
> > +			<< "Polynomial is missing a parameter";
> > +
> > +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > +}
> > +#endif /* __DOXYGEN__ */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > index d7d9ae42e360..d52ec86946b1 100644
> > --- a/src/ipa/libipa/lsc_polynomial.h
> > +++ b/src/ipa/libipa/lsc_polynomial.h
> > @@ -80,28 +80,4 @@ private:
> >
> >  } /* namespace ipa */
> >
> > -#ifndef __DOXYGEN__
> > -
> > -template<>
> > -struct ValueNode::Accessor<ipa::LscPolynomial> {
> > -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> > -	{
> > -		std::optional<double> cx = obj["cx"].get<double>();
> > -		std::optional<double> cy = obj["cy"].get<double>();
> > -		std::optional<double> k0 = obj["k0"].get<double>();
> > -		std::optional<double> k1 = obj["k1"].get<double>();
> > -		std::optional<double> k2 = obj["k2"].get<double>();
> > -		std::optional<double> k3 = obj["k3"].get<double>();
> > -		std::optional<double> k4 = obj["k4"].get<double>();
> > -
> > -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > -			LOG(LscPolynomial, Error)
> > -				<< "Polynomial is missing a parameter";
> > -
> > -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > -	}
> > -};
> > -
> > -#endif
> > -
> >  } /* namespace libcamera */
> >
> > base-commit: bd266aa3405d3aea20b407b27e52482b02146030
Jacopo Mondi June 22, 2026, 9:15 a.m. UTC | #5 
Hi Laurent

On Mon, Jun 22, 2026 at 12:02:35PM +0300, Laurent Pinchart wrote:
> On Mon, Jun 22, 2026 at 10:50:09AM +0200, Jacopo Mondi wrote:
> > On Sun, Jun 21, 2026 at 03:41:15AM +0300, Laurent Pinchart wrote:
> > > There's no need to inline the ValueNode::Accessor::get() function for
> > > LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> > >
> > > While at it, replace C++-style comments with C-style comments according
> > > to the libcamera coding style.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Can we hold on on (non bugfix) changes to this part considering it is
> > under rework?
>
> If that hinders your work I don't mind.
>

It makes the rebase a little more complicated, but if it's for this
single patch, I can cope with that.

I understand the libipa rework might take a bit to get in, so I
don't mind too much.

I can already move the ValueNode::Accessor::get() in the .cpp file in
v2 of libipa rework, or rebase if you prefer.

> > > ---
> > >  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
> > >  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
> > >  2 files changed, 24 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > > index f607d86c54c3..00569c2fd160 100644
> > > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > > @@ -77,5 +77,27 @@ namespace ipa {
> > >   * \param size The size of the reference image
> > >   */
> > >
> > > -} // namespace ipa
> > > -} // namespace libcamera
> > > +} /* namespace ipa */
> > > +
> > > +#ifndef __DOXYGEN__
> > > +template<>
> > > +std::optional<ipa::LscPolynomial>
> > > +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> > > +{
> > > +	std::optional<double> cx = obj["cx"].get<double>();
> > > +	std::optional<double> cy = obj["cy"].get<double>();
> > > +	std::optional<double> k0 = obj["k0"].get<double>();
> > > +	std::optional<double> k1 = obj["k1"].get<double>();
> > > +	std::optional<double> k2 = obj["k2"].get<double>();
> > > +	std::optional<double> k3 = obj["k3"].get<double>();
> > > +	std::optional<double> k4 = obj["k4"].get<double>();
> > > +
> > > +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > +		LOG(LscPolynomial, Error)
> > > +			<< "Polynomial is missing a parameter";
> > > +
> > > +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > +}
> > > +#endif /* __DOXYGEN__ */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > > index d7d9ae42e360..d52ec86946b1 100644
> > > --- a/src/ipa/libipa/lsc_polynomial.h
> > > +++ b/src/ipa/libipa/lsc_polynomial.h
> > > @@ -80,28 +80,4 @@ private:
> > >
> > >  } /* namespace ipa */
> > >
> > > -#ifndef __DOXYGEN__
> > > -
> > > -template<>
> > > -struct ValueNode::Accessor<ipa::LscPolynomial> {
> > > -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> > > -	{
> > > -		std::optional<double> cx = obj["cx"].get<double>();
> > > -		std::optional<double> cy = obj["cy"].get<double>();
> > > -		std::optional<double> k0 = obj["k0"].get<double>();
> > > -		std::optional<double> k1 = obj["k1"].get<double>();
> > > -		std::optional<double> k2 = obj["k2"].get<double>();
> > > -		std::optional<double> k3 = obj["k3"].get<double>();
> > > -		std::optional<double> k4 = obj["k4"].get<double>();
> > > -
> > > -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > -			LOG(LscPolynomial, Error)
> > > -				<< "Polynomial is missing a parameter";
> > > -
> > > -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > -	}
> > > -};
> > > -
> > > -#endif
> > > -
> > >  } /* namespace libcamera */
> > >
> > > base-commit: bd266aa3405d3aea20b407b27e52482b02146030
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart June 22, 2026, 9:18 a.m. UTC | #6 
On Mon, Jun 22, 2026 at 11:15:39AM +0200, Jacopo Mondi wrote:
> On Mon, Jun 22, 2026 at 12:02:35PM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 22, 2026 at 10:50:09AM +0200, Jacopo Mondi wrote:
> > > On Sun, Jun 21, 2026 at 03:41:15AM +0300, Laurent Pinchart wrote:
> > > > There's no need to inline the ValueNode::Accessor::get() function for
> > > > LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> > > >
> > > > While at it, replace C++-style comments with C-style comments according
> > > > to the libcamera coding style.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Can we hold on on (non bugfix) changes to this part considering it is
> > > under rework?
> >
> > If that hinders your work I don't mind.
> 
> It makes the rebase a little more complicated, but if it's for this
> single patch, I can cope with that.
> 
> I understand the libipa rework might take a bit to get in, so I
> don't mind too much.
> 
> I can already move the ValueNode::Accessor::get() in the .cpp file in
> v2 of libipa rework, or rebase if you prefer.

I don't have a preference, I can drop this patch.

> > > > ---
> > > >  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
> > > >  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
> > > >  2 files changed, 24 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > > > index f607d86c54c3..00569c2fd160 100644
> > > > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > > > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > > > @@ -77,5 +77,27 @@ namespace ipa {
> > > >   * \param size The size of the reference image
> > > >   */
> > > >
> > > > -} // namespace ipa
> > > > -} // namespace libcamera
> > > > +} /* namespace ipa */
> > > > +
> > > > +#ifndef __DOXYGEN__
> > > > +template<>
> > > > +std::optional<ipa::LscPolynomial>
> > > > +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> > > > +{
> > > > +	std::optional<double> cx = obj["cx"].get<double>();
> > > > +	std::optional<double> cy = obj["cy"].get<double>();
> > > > +	std::optional<double> k0 = obj["k0"].get<double>();
> > > > +	std::optional<double> k1 = obj["k1"].get<double>();
> > > > +	std::optional<double> k2 = obj["k2"].get<double>();
> > > > +	std::optional<double> k3 = obj["k3"].get<double>();
> > > > +	std::optional<double> k4 = obj["k4"].get<double>();
> > > > +
> > > > +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > > +		LOG(LscPolynomial, Error)
> > > > +			<< "Polynomial is missing a parameter";
> > > > +
> > > > +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > > +}
> > > > +#endif /* __DOXYGEN__ */
> > > > +
> > > > +} /* namespace libcamera */
> > > > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > > > index d7d9ae42e360..d52ec86946b1 100644
> > > > --- a/src/ipa/libipa/lsc_polynomial.h
> > > > +++ b/src/ipa/libipa/lsc_polynomial.h
> > > > @@ -80,28 +80,4 @@ private:
> > > >
> > > >  } /* namespace ipa */
> > > >
> > > > -#ifndef __DOXYGEN__
> > > > -
> > > > -template<>
> > > > -struct ValueNode::Accessor<ipa::LscPolynomial> {
> > > > -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> > > > -	{
> > > > -		std::optional<double> cx = obj["cx"].get<double>();
> > > > -		std::optional<double> cy = obj["cy"].get<double>();
> > > > -		std::optional<double> k0 = obj["k0"].get<double>();
> > > > -		std::optional<double> k1 = obj["k1"].get<double>();
> > > > -		std::optional<double> k2 = obj["k2"].get<double>();
> > > > -		std::optional<double> k3 = obj["k3"].get<double>();
> > > > -		std::optional<double> k4 = obj["k4"].get<double>();
> > > > -
> > > > -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > > -			LOG(LscPolynomial, Error)
> > > > -				<< "Polynomial is missing a parameter";
> > > > -
> > > > -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > > -	}
> > > > -};
> > > > -
> > > > -#endif
> > > > -
> > > >  } /* namespace libcamera */
> > > >
> > > > base-commit: bd266aa3405d3aea20b407b27e52482b02146030
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
Jacopo Mondi June 22, 2026, 2:26 p.m. UTC | #7 
Hi Laurent

On Mon, Jun 22, 2026 at 12:18:58PM +0300, Laurent Pinchart wrote:
> On Mon, Jun 22, 2026 at 11:15:39AM +0200, Jacopo Mondi wrote:
> > On Mon, Jun 22, 2026 at 12:02:35PM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 22, 2026 at 10:50:09AM +0200, Jacopo Mondi wrote:
> > > > On Sun, Jun 21, 2026 at 03:41:15AM +0300, Laurent Pinchart wrote:
> > > > > There's no need to inline the ValueNode::Accessor::get() function for
> > > > > LscPolynomial. Move it to from lsc_polynomial.h to lsc_polynomial.cpp.
> > > > >
> > > > > While at it, replace C++-style comments with C-style comments according
> > > > > to the libcamera coding style.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Can we hold on on (non bugfix) changes to this part considering it is
> > > > under rework?
> > >
> > > If that hinders your work I don't mind.
> >
> > It makes the rebase a little more complicated, but if it's for this
> > single patch, I can cope with that.
> >
> > I understand the libipa rework might take a bit to get in, so I
> > don't mind too much.
> >
> > I can already move the ValueNode::Accessor::get() in the .cpp file in
> > v2 of libipa rework, or rebase if you prefer.
>
> I don't have a preference, I can drop this patch.
>

I've taken you change in v2 with the following resulting diff

--- a/src/ipa/libipa/lsc_polynomial.cpp
+++ b/src/ipa/libipa/lsc_polynomial.cpp
@@ -18,6 +18,27 @@

 namespace libcamera {

+#ifndef __DOXYGEN__
+template<>
+std::optional<ipa::Polynomial>
+ValueNode::Accessor<ipa::Polynomial>::get(const ValueNode &obj) const
+{
+       std::optional<double> cx = obj["cx"].get<double>();
+       std::optional<double> cy = obj["cy"].get<double>();
+       std::optional<double> k0 = obj["k0"].get<double>();
+       std::optional<double> k1 = obj["k1"].get<double>();
+       std::optional<double> k2 = obj["k2"].get<double>();
+       std::optional<double> k3 = obj["k3"].get<double>();
+       std::optional<double> k4 = obj["k4"].get<double>();
+
+       if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
+               LOG(LscPolynomial, Error)
+                       << "Polynomial is missing a parameter";
+
+       return ipa::Polynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
+}
+#endif /* __DOXYGEN__ */
+
 LOG_DEFINE_CATEGORY(LscPolynomial)

 namespace ipa {
diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
index 72cca591f260..e7533bccacd0 100644
--- a/src/ipa/libipa/lsc_polynomial.h
+++ b/src/ipa/libipa/lsc_polynomial.h
@@ -135,28 +135,4 @@ private:

 } /* namespace ipa */

-#ifndef __DOXYGEN__
-
-template<>
-struct ValueNode::Accessor<ipa::Polynomial> {
-       std::optional<ipa::Polynomial> get(const ValueNode &obj) const
-       {
-               std::optional<double> cx = obj["cx"].get<double>();
-               std::optional<double> cy = obj["cy"].get<double>();
-               std::optional<double> k0 = obj["k0"].get<double>();
-               std::optional<double> k1 = obj["k1"].get<double>();
-               std::optional<double> k2 = obj["k2"].get<double>();
-               std::optional<double> k3 = obj["k3"].get<double>();
-               std::optional<double> k4 = obj["k4"].get<double>();
-
-               if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
-                       LOG(LscPolynomial, Error)
-                               << "Polynomial is missing a parameter";
-
-               return ipa::Polynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
-       }
-};
-
-#endif
-
 } /* namespace libcamera */

> > > > > ---
> > > > >  src/ipa/libipa/lsc_polynomial.cpp | 26 ++++++++++++++++++++++++--
> > > > >  src/ipa/libipa/lsc_polynomial.h   | 24 ------------------------
> > > > >  2 files changed, 24 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > > > > index f607d86c54c3..00569c2fd160 100644
> > > > > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > > > > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > > > > @@ -77,5 +77,27 @@ namespace ipa {
> > > > >   * \param size The size of the reference image
> > > > >   */
> > > > >
> > > > > -} // namespace ipa
> > > > > -} // namespace libcamera
> > > > > +} /* namespace ipa */
> > > > > +
> > > > > +#ifndef __DOXYGEN__
> > > > > +template<>
> > > > > +std::optional<ipa::LscPolynomial>
> > > > > +ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
> > > > > +{
> > > > > +	std::optional<double> cx = obj["cx"].get<double>();
> > > > > +	std::optional<double> cy = obj["cy"].get<double>();
> > > > > +	std::optional<double> k0 = obj["k0"].get<double>();
> > > > > +	std::optional<double> k1 = obj["k1"].get<double>();
> > > > > +	std::optional<double> k2 = obj["k2"].get<double>();
> > > > > +	std::optional<double> k3 = obj["k3"].get<double>();
> > > > > +	std::optional<double> k4 = obj["k4"].get<double>();
> > > > > +
> > > > > +	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > > > +		LOG(LscPolynomial, Error)
> > > > > +			<< "Polynomial is missing a parameter";
> > > > > +
> > > > > +	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > > > +}
> > > > > +#endif /* __DOXYGEN__ */
> > > > > +
> > > > > +} /* namespace libcamera */
> > > > > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > > > > index d7d9ae42e360..d52ec86946b1 100644
> > > > > --- a/src/ipa/libipa/lsc_polynomial.h
> > > > > +++ b/src/ipa/libipa/lsc_polynomial.h
> > > > > @@ -80,28 +80,4 @@ private:
> > > > >
> > > > >  } /* namespace ipa */
> > > > >
> > > > > -#ifndef __DOXYGEN__
> > > > > -
> > > > > -template<>
> > > > > -struct ValueNode::Accessor<ipa::LscPolynomial> {
> > > > > -	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
> > > > > -	{
> > > > > -		std::optional<double> cx = obj["cx"].get<double>();
> > > > > -		std::optional<double> cy = obj["cy"].get<double>();
> > > > > -		std::optional<double> k0 = obj["k0"].get<double>();
> > > > > -		std::optional<double> k1 = obj["k1"].get<double>();
> > > > > -		std::optional<double> k2 = obj["k2"].get<double>();
> > > > > -		std::optional<double> k3 = obj["k3"].get<double>();
> > > > > -		std::optional<double> k4 = obj["k4"].get<double>();
> > > > > -
> > > > > -		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
> > > > > -			LOG(LscPolynomial, Error)
> > > > > -				<< "Polynomial is missing a parameter";
> > > > > -
> > > > > -		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
> > > > > -	}
> > > > > -};
> > > > > -
> > > > > -#endif
> > > > > -
> > > > >  } /* namespace libcamera */
> > > > >
> > > > > base-commit: bd266aa3405d3aea20b407b27e52482b02146030
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
index f607d86c54c3..00569c2fd160 100644
--- a/src/ipa/libipa/lsc_polynomial.cpp
+++ b/src/ipa/libipa/lsc_polynomial.cpp
@@ -77,5 +77,27 @@  namespace ipa {
  * \param size The size of the reference image
  */
 
-} // namespace ipa
-} // namespace libcamera
+} /* namespace ipa */
+
+#ifndef __DOXYGEN__
+template<>
+std::optional<ipa::LscPolynomial>
+ValueNode::Accessor<ipa::LscPolynomial>::get(const ValueNode &obj) const
+{
+	std::optional<double> cx = obj["cx"].get<double>();
+	std::optional<double> cy = obj["cy"].get<double>();
+	std::optional<double> k0 = obj["k0"].get<double>();
+	std::optional<double> k1 = obj["k1"].get<double>();
+	std::optional<double> k2 = obj["k2"].get<double>();
+	std::optional<double> k3 = obj["k3"].get<double>();
+	std::optional<double> k4 = obj["k4"].get<double>();
+
+	if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
+		LOG(LscPolynomial, Error)
+			<< "Polynomial is missing a parameter";
+
+	return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
+}
+#endif /* __DOXYGEN__ */
+
+} /* namespace libcamera */
diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
index d7d9ae42e360..d52ec86946b1 100644
--- a/src/ipa/libipa/lsc_polynomial.h
+++ b/src/ipa/libipa/lsc_polynomial.h
@@ -80,28 +80,4 @@  private:
 
 } /* namespace ipa */
 
-#ifndef __DOXYGEN__
-
-template<>
-struct ValueNode::Accessor<ipa::LscPolynomial> {
-	std::optional<ipa::LscPolynomial> get(const ValueNode &obj) const
-	{
-		std::optional<double> cx = obj["cx"].get<double>();
-		std::optional<double> cy = obj["cy"].get<double>();
-		std::optional<double> k0 = obj["k0"].get<double>();
-		std::optional<double> k1 = obj["k1"].get<double>();
-		std::optional<double> k2 = obj["k2"].get<double>();
-		std::optional<double> k3 = obj["k3"].get<double>();
-		std::optional<double> k4 = obj["k4"].get<double>();
-
-		if (!(cx && cy && k0 && k1 && k2 && k3 && k4))
-			LOG(LscPolynomial, Error)
-				<< "Polynomial is missing a parameter";
-
-		return ipa::LscPolynomial(*cx, *cy, *k0, *k1, *k2, *k3, *k4);
-	}
-};
-
-#endif
-
 } /* namespace libcamera */