[1/2] libcamera: converter: converter_dw100: Refactor dewarp param handling
diff mbox series

Message ID 20260617095857.667583-2-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Add cmNew to dewarp paremeters
Related show

Commit Message

Stefan Klug June 17, 2026, 9:58 a.m. UTC 
There is already a ConverterDW100Module::DewarpParams class while in
Dw100VertexMap the parameters are handled separately for no good reason.
Create a Dw100VertexMap::DewarpParams struct for that purpose and adjust
the code accordingly. This has the added benefit that the size checking
for the coefficient list can now be done in the tuning file loading code
where it belongs.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 .../internal/converter/converter_dw100.h      |  7 +-
 .../converter/converter_dw100_vertexmap.h     | 35 ++++++--
 src/libcamera/converter/converter_dw100.cpp   | 15 +++-
 .../converter/converter_dw100_vertexmap.cpp   | 82 ++++++++-----------
 4 files changed, 75 insertions(+), 64 deletions(-)

Comments

Barnabás Pőcze June 17, 2026, 12:39 p.m. UTC | #1 
2026. 06. 17. 11:58 keltezéssel, Stefan Klug írta:
> There is already a ConverterDW100Module::DewarpParams class while in
> Dw100VertexMap the parameters are handled separately for no good reason.
> Create a Dw100VertexMap::DewarpParams struct for that purpose and adjust
> the code accordingly. This has the added benefit that the size checking
> for the coefficient list can now be done in the tuning file loading code
> where it belongs.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>   .../internal/converter/converter_dw100.h      |  7 +-
>   .../converter/converter_dw100_vertexmap.h     | 35 ++++++--
>   src/libcamera/converter/converter_dw100.cpp   | 15 +++-
>   .../converter/converter_dw100_vertexmap.cpp   | 82 ++++++++-----------
>   4 files changed, 75 insertions(+), 64 deletions(-)
> 
> diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h
> index 1db1aadcb06b..003f5eb954e9 100644
> --- a/include/libcamera/internal/converter/converter_dw100.h
> +++ b/include/libcamera/internal/converter/converter_dw100.h
> @@ -69,18 +69,13 @@ private:
>   	int applyControls(const Stream *stream, const V4L2Request *request);
>   	void reinitRequest(V4L2Request *request);
> 
> -	struct DewarpParms {
> -		Matrix<double, 3, 3> cm;
> -		std::vector<double> coeffs;
> -	};
> -
>   	struct VertexMapInfo {
>   		Dw100VertexMap map;
>   		bool update;
>   	};
> 
>   	std::map<const Stream *, VertexMapInfo> vertexMaps_;
> -	std::optional<DewarpParms> dewarpParams_;
> +	std::optional<Dw100VertexMap::DewarpParams> dewarpParams_;
>   	unsigned int inputBufferCount_;
>   	V4L2M2MConverter converter_;
>   	Rectangle sensorCrop_;
> diff --git a/include/libcamera/internal/converter/converter_dw100_vertexmap.h b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> index 3bf80ea66dc7..787498c7fdb5 100644
> --- a/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> +++ b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> @@ -9,6 +9,7 @@
> 
>   #include <assert.h>
>   #include <cmath>
> +#include <optional>
>   #include <stdint.h>
>   #include <vector>
> 
> @@ -30,6 +31,33 @@ public:
>   		Crop = 1,
>   	};
> 
> +	struct DewarpParams {
> +		DewarpParams() : cm(Matrix<double, 3, 3>::identity()),
> +				 coefficients({})
> +		{
> +		}
> +
> +		Matrix<double, 3, 3> cm;
> +
> +		union {
> +			struct {
> +				double k1;
> +				double k2;
> +				double p1;
> +				double p2;
> +				double k3;
> +				double k4;
> +				double k5;
> +				double k6;
> +				double s1;
> +				double s2;
> +				double s3;
> +				double s4;
> +			} coefficient;
> +			std::array<double, 12> coefficients;
> +		};

I'm a bit worried about this part. As far as I am aware only the active member of
a union can technically be read (and there is the common initial sequence rule,
but I don't think it applies here, between a struct and and array). So I guess one
way to avoid this is to define a function for each. Or am I missing something that
makes this well-defined?


> +	};
> +
>   	void applyLimits();
>   	void setInputSize(const Size &size)
>   	{
> @@ -60,8 +88,7 @@ public:
>   	void setMode(const ScaleMode mode) { mode_ = mode; }
>   	ScaleMode mode() const { return mode_; }
> 
> -	int setDewarpParams(const Matrix<double, 3, 3> &cm, const Span<const double> &coeffs);
> -	bool dewarpParamsValid() { return dewarpParamsValid_; }
> +	void setDewarpParams(const DewarpParams &params) { dewarpParams_ = params; }
> 
>   	void setLensDewarpEnable(bool enable) { lensDewarpEnable_ = enable; }
>   	bool lensDewarpEnable() { return lensDewarpEnable_; }
> @@ -85,10 +112,8 @@ private:
>   	Point effectiveOffset_;
>   	Rectangle effectiveScalerCrop_;
> 
> -	Matrix<double, 3, 3> dewarpM_ = Matrix<double, 3, 3>::identity();
> -	std::array<double, 12> dewarpCoeffs_;
> +	std::optional<DewarpParams> dewarpParams_;
>   	bool lensDewarpEnable_ = true;
> -	bool dewarpParamsValid_ = false;
>   };
> 
>   } /* namespace libcamera */
> diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp
> index 44f7ec035e62..34f81c6b9fab 100644
> --- a/src/libcamera/converter/converter_dw100.cpp
> +++ b/src/libcamera/converter/converter_dw100.cpp
> @@ -15,6 +15,7 @@
>   #include <libcamera/stream.h>
> 
>   #include "libcamera/internal/converter.h"
> +#include "libcamera/internal/converter/converter_dw100_vertexmap.h"
>   #include "libcamera/internal/converter/converter_v4l2_m2m.h"
>   #include "libcamera/internal/media_device.h"
>   #include "libcamera/internal/v4l2_videodevice.h"
> @@ -99,7 +100,7 @@ ConverterDW100Module::createModule(DeviceEnumerator *enumerator)
>    */
>   int ConverterDW100Module::init(const ValueNode &params)
>   {
> -	DewarpParms dp;
> +	Dw100VertexMap::DewarpParams dp;
> 
>   	auto &cm = params["cm"];
>   	auto &coefficients = params["coefficients"];
> @@ -131,7 +132,15 @@ int ConverterDW100Module::init(const ValueNode &params)
>   		LOG(Converter, Error) << "Dewarp parameters 'coefficients' value is not a list";
>   		return -EINVAL;
>   	}
> -	dp.coeffs = std::move(*coeffs);
> +
> +	if (coeffs->size() != 4 && coeffs->size() != 5 &&
> +	    coeffs->size() != 8 && coeffs->size() != 12) {
> +		LOG(Converter, Error)
> +			<< "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
> +		return -EINVAL;
> +	}
> +
> +	std::copy(coeffs->begin(), coeffs->end(), &dp.coefficients[0]);
> 
>   	dewarpParams_ = dp;
> 
> @@ -163,7 +172,7 @@ int ConverterDW100Module::configure(const StreamConfiguration &inputCfg,
>   		vertexMap.setSensorCrop(sensorCrop_);
> 
>   		if (dewarpParams_)
> -			vertexMap.setDewarpParams(dewarpParams_->cm, dewarpParams_->coeffs);
> +			vertexMap.setDewarpParams(*dewarpParams_);
>   		info.update = true;
>   	}
> 
> diff --git a/src/libcamera/converter/converter_dw100_vertexmap.cpp b/src/libcamera/converter/converter_dw100_vertexmap.cpp
> index 6e238736c435..31d992546857 100644
> --- a/src/libcamera/converter/converter_dw100_vertexmap.cpp
> +++ b/src/libcamera/converter/converter_dw100_vertexmap.cpp
> @@ -227,6 +227,20 @@ int dw100VerticesForLength(const int length)
>    * into account within the possible limits.
>    */
> 
> +/**
> + * \struct Dw100VertexMap::DewarpParams
> + * \brief Structure combining all dewarp parameters
> + *
> + * \var Dw100VertexMap::DewarpParams::cm
> + * \brief The camera matrix
> + *
> + * \var Dw100VertexMap::DewarpParams::coefficient
> + * \brief Structure containing the lens dewarp coefficients
> +
> + * See https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further
> + * details on the model.
> + */
> +
>   /**
>    * \brief Apply limits on scale and offset
>    *
> @@ -549,7 +563,7 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
> 
>   			p = transformPoint(outputToSensor, p);
> 
> -			if (dewarpParamsValid_ && lensDewarpEnable_)
> +			if (lensDewarpEnable_)
>   				p = dewarpPoint(p);
> 
>   			p = transformPoint(sensorToInput, p);
> @@ -566,41 +580,13 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
> 
>   /**
>    * \brief Set the dewarp parameters
> - * \param cm The camera matrix
> - * \param coeffs The dewarp coefficients
> + * \param params The dewarp parameters
>    *
>    * Sets the dewarp parameters according to the commonly used dewarp model. See
>    * https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further details
>    * on the model. The parameter \a coeffs must either hold 4,5,8 or 12 values.
>    * They represent the parameters k1,k2,p1,p2[,k3[,k4,k5,k6[,s1,s2,s3,s4]]] in
>    * the model.
> - *
> - * \return A negative number on error, 0 otherwise
> - */
> -int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
> -				    const Span<const double> &coeffs)
> -{
> -	dewarpM_ = cm;
> -	dewarpCoeffs_.fill(0.0);
> -
> -	if (coeffs.size() != 4 && coeffs.size() != 5 &&
> -	    coeffs.size() != 8 && coeffs.size() != 12) {
> -		LOG(Converter, Error)
> -			<< "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
> -		dewarpParamsValid_ = false;
> -		return -EINVAL;
> -	}
> -	std::copy(coeffs.begin(), coeffs.end(), dewarpCoeffs_.begin());
> -
> -	dewarpParamsValid_ = true;
> -	return 0;
> -}
> -
> -/**
> - * \fn Dw100VertexMap::dewarpParamsValid()
> - * \brief Returns if the dewarp parameters are valid
> - *
> - * \return True if the dewarp parameters are valid, false otherwise
>    */
> 
>   /**
> @@ -627,32 +613,28 @@ int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
>    */
>   Vector2d Dw100VertexMap::dewarpPoint(const Vector2d &p)
>   {
> +	if (!dewarpParams_.has_value())
> +		return p;
> +
>   	double x, y;
>   	double xout, yout;
> -	double k1 = dewarpCoeffs_[0];
> -	double k2 = dewarpCoeffs_[1];
> -	double p1 = dewarpCoeffs_[2];
> -	double p2 = dewarpCoeffs_[3];
> -	double k3 = dewarpCoeffs_[4];
> -	double k4 = dewarpCoeffs_[5];
> -	double k5 = dewarpCoeffs_[6];
> -	double k6 = dewarpCoeffs_[7];
> -	double s1 = dewarpCoeffs_[8];
> -	double s2 = dewarpCoeffs_[9];
> -	double s3 = dewarpCoeffs_[10];
> -	double s4 = dewarpCoeffs_[11];
> +	auto &cm = dewarpParams_->cm;
> +	auto &c = dewarpParams_->coefficient;
> 
> -	y = (p.y() - dewarpM_[1][2]) / dewarpM_[1][1];
> -	x = (p.x() - dewarpM_[0][2] - y * dewarpM_[0][1]) / dewarpM_[0][0];
> +	y = (p.y() - cm[1][2]) / cm[1][1];
> +	x = (p.x() - cm[0][2] - y * cm[0][1]) / cm[0][0];
> 
>   	double r2 = x * x + y * y;
> -	double d = (1 + k1 * r2 + k2 * r2 * r2 + k3 * r2 * r2 * r2) /
> -		   (1 + k4 * r2 + k5 * r2 * r2 + k6 * r2 * r2 * r2);
> -	xout = x * d + 2 * p1 * x * y + p2 * (r2 + 2 * x * x) + s1 * r2 + s2 * r2 * r2;
> -	yout = y * d + 2 * p2 * x * y + p1 * (r2 + 2 * y * y) + s3 * r2 + s4 * r2 * r2;
> +	double r4 = r2 * r2;
> +	double r6 = r2 * r2 * r2;
> +	double d = (1 + c.k1 * r2 + c.k2 * r4 + c.k3 * r6) /
> +		   (1 + c.k4 * r2 + c.k5 * r4 + c.k6 * r6);
> 
> -	return { { xout * dewarpM_[0][0] + yout * dewarpM_[0][1] + dewarpM_[0][2],
> -		   yout * dewarpM_[1][1] + dewarpM_[1][2] } };
> +	xout = x * d + 2 * c.p1 * x * y + c.p2 * (r2 + 2 * x * x) + c.s1 * r2 + c.s2 * r4;
> +	yout = y * d + 2 * c.p2 * x * y + c.p1 * (r2 + 2 * y * y) + c.s3 * r2 + c.s4 * r4;
> +
> +	return { { xout * cm[0][0] + yout * cm[0][1] + cm[0][2],
> +		   yout * cm[1][1] + cm[1][2] } };
>   }
> 
>   } /* namespace libcamera */
> --
> 2.53.0
>
Stefan Klug June 17, 2026, 1:58 p.m. UTC | #2 
Hi Barnabás,

Thanks for the review.

Quoting Barnabás Pőcze (2026-06-17 14:39:16)
> 2026. 06. 17. 11:58 keltezéssel, Stefan Klug írta:
> > There is already a ConverterDW100Module::DewarpParams class while in
> > Dw100VertexMap the parameters are handled separately for no good reason.
> > Create a Dw100VertexMap::DewarpParams struct for that purpose and adjust
> > the code accordingly. This has the added benefit that the size checking
> > for the coefficient list can now be done in the tuning file loading code
> > where it belongs.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >   .../internal/converter/converter_dw100.h      |  7 +-
> >   .../converter/converter_dw100_vertexmap.h     | 35 ++++++--
> >   src/libcamera/converter/converter_dw100.cpp   | 15 +++-
> >   .../converter/converter_dw100_vertexmap.cpp   | 82 ++++++++-----------
> >   4 files changed, 75 insertions(+), 64 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h
> > index 1db1aadcb06b..003f5eb954e9 100644
> > --- a/include/libcamera/internal/converter/converter_dw100.h
> > +++ b/include/libcamera/internal/converter/converter_dw100.h
> > @@ -69,18 +69,13 @@ private:
> >       int applyControls(const Stream *stream, const V4L2Request *request);
> >       void reinitRequest(V4L2Request *request);
> > 
> > -     struct DewarpParms {
> > -             Matrix<double, 3, 3> cm;
> > -             std::vector<double> coeffs;
> > -     };
> > -
> >       struct VertexMapInfo {
> >               Dw100VertexMap map;
> >               bool update;
> >       };
> > 
> >       std::map<const Stream *, VertexMapInfo> vertexMaps_;
> > -     std::optional<DewarpParms> dewarpParams_;
> > +     std::optional<Dw100VertexMap::DewarpParams> dewarpParams_;
> >       unsigned int inputBufferCount_;
> >       V4L2M2MConverter converter_;
> >       Rectangle sensorCrop_;
> > diff --git a/include/libcamera/internal/converter/converter_dw100_vertexmap.h b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> > index 3bf80ea66dc7..787498c7fdb5 100644
> > --- a/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> > +++ b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
> > @@ -9,6 +9,7 @@
> > 
> >   #include <assert.h>
> >   #include <cmath>
> > +#include <optional>
> >   #include <stdint.h>
> >   #include <vector>
> > 
> > @@ -30,6 +31,33 @@ public:
> >               Crop = 1,
> >       };
> > 
> > +     struct DewarpParams {
> > +             DewarpParams() : cm(Matrix<double, 3, 3>::identity()),
> > +                              coefficients({})
> > +             {
> > +             }
> > +
> > +             Matrix<double, 3, 3> cm;
> > +
> > +             union {
> > +                     struct {
> > +                             double k1;
> > +                             double k2;
> > +                             double p1;
> > +                             double p2;
> > +                             double k3;
> > +                             double k4;
> > +                             double k5;
> > +                             double k6;
> > +                             double s1;
> > +                             double s2;
> > +                             double s3;
> > +                             double s4;
> > +                     } coefficient;
> > +                     std::array<double, 12> coefficients;
> > +             };
> 
> I'm a bit worried about this part. As far as I am aware only the active member of
> a union can technically be read (and there is the common initial sequence rule,
> but I don't think it applies here, between a struct and and array). So I guess one
> way to avoid this is to define a function for each. Or am I missing something that
> makes this well-defined?

Thanks for pointing that out. I thought that is a common pattern.
According to https://stackoverflow.com/a/21763025 it actually seems to
be legal and the common initial sequence rule applies?

But nevertheless I changed the code to drop the union. I like that
version better. So I'll post a v2 soon.

Best regards,
Stefan

> 
> 
> > +     };
> > +
> >       void applyLimits();
> >       void setInputSize(const Size &size)
> >       {
> > @@ -60,8 +88,7 @@ public:
> >       void setMode(const ScaleMode mode) { mode_ = mode; }
> >       ScaleMode mode() const { return mode_; }
> > 
> > -     int setDewarpParams(const Matrix<double, 3, 3> &cm, const Span<const double> &coeffs);
> > -     bool dewarpParamsValid() { return dewarpParamsValid_; }
> > +     void setDewarpParams(const DewarpParams &params) { dewarpParams_ = params; }
> > 
> >       void setLensDewarpEnable(bool enable) { lensDewarpEnable_ = enable; }
> >       bool lensDewarpEnable() { return lensDewarpEnable_; }
> > @@ -85,10 +112,8 @@ private:
> >       Point effectiveOffset_;
> >       Rectangle effectiveScalerCrop_;
> > 
> > -     Matrix<double, 3, 3> dewarpM_ = Matrix<double, 3, 3>::identity();
> > -     std::array<double, 12> dewarpCoeffs_;
> > +     std::optional<DewarpParams> dewarpParams_;
> >       bool lensDewarpEnable_ = true;
> > -     bool dewarpParamsValid_ = false;
> >   };
> > 
> >   } /* namespace libcamera */
> > diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp
> > index 44f7ec035e62..34f81c6b9fab 100644
> > --- a/src/libcamera/converter/converter_dw100.cpp
> > +++ b/src/libcamera/converter/converter_dw100.cpp
> > @@ -15,6 +15,7 @@
> >   #include <libcamera/stream.h>
> > 
> >   #include "libcamera/internal/converter.h"
> > +#include "libcamera/internal/converter/converter_dw100_vertexmap.h"
> >   #include "libcamera/internal/converter/converter_v4l2_m2m.h"
> >   #include "libcamera/internal/media_device.h"
> >   #include "libcamera/internal/v4l2_videodevice.h"
> > @@ -99,7 +100,7 @@ ConverterDW100Module::createModule(DeviceEnumerator *enumerator)
> >    */
> >   int ConverterDW100Module::init(const ValueNode &params)
> >   {
> > -     DewarpParms dp;
> > +     Dw100VertexMap::DewarpParams dp;
> > 
> >       auto &cm = params["cm"];
> >       auto &coefficients = params["coefficients"];
> > @@ -131,7 +132,15 @@ int ConverterDW100Module::init(const ValueNode &params)
> >               LOG(Converter, Error) << "Dewarp parameters 'coefficients' value is not a list";
> >               return -EINVAL;
> >       }
> > -     dp.coeffs = std::move(*coeffs);
> > +
> > +     if (coeffs->size() != 4 && coeffs->size() != 5 &&
> > +         coeffs->size() != 8 && coeffs->size() != 12) {
> > +             LOG(Converter, Error)
> > +                     << "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
> > +             return -EINVAL;
> > +     }
> > +
> > +     std::copy(coeffs->begin(), coeffs->end(), &dp.coefficients[0]);
> > 
> >       dewarpParams_ = dp;
> > 
> > @@ -163,7 +172,7 @@ int ConverterDW100Module::configure(const StreamConfiguration &inputCfg,
> >               vertexMap.setSensorCrop(sensorCrop_);
> > 
> >               if (dewarpParams_)
> > -                     vertexMap.setDewarpParams(dewarpParams_->cm, dewarpParams_->coeffs);
> > +                     vertexMap.setDewarpParams(*dewarpParams_);
> >               info.update = true;
> >       }
> > 
> > diff --git a/src/libcamera/converter/converter_dw100_vertexmap.cpp b/src/libcamera/converter/converter_dw100_vertexmap.cpp
> > index 6e238736c435..31d992546857 100644
> > --- a/src/libcamera/converter/converter_dw100_vertexmap.cpp
> > +++ b/src/libcamera/converter/converter_dw100_vertexmap.cpp
> > @@ -227,6 +227,20 @@ int dw100VerticesForLength(const int length)
> >    * into account within the possible limits.
> >    */
> > 
> > +/**
> > + * \struct Dw100VertexMap::DewarpParams
> > + * \brief Structure combining all dewarp parameters
> > + *
> > + * \var Dw100VertexMap::DewarpParams::cm
> > + * \brief The camera matrix
> > + *
> > + * \var Dw100VertexMap::DewarpParams::coefficient
> > + * \brief Structure containing the lens dewarp coefficients
> > +
> > + * See https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further
> > + * details on the model.
> > + */
> > +
> >   /**
> >    * \brief Apply limits on scale and offset
> >    *
> > @@ -549,7 +563,7 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
> > 
> >                       p = transformPoint(outputToSensor, p);
> > 
> > -                     if (dewarpParamsValid_ && lensDewarpEnable_)
> > +                     if (lensDewarpEnable_)
> >                               p = dewarpPoint(p);
> > 
> >                       p = transformPoint(sensorToInput, p);
> > @@ -566,41 +580,13 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
> > 
> >   /**
> >    * \brief Set the dewarp parameters
> > - * \param cm The camera matrix
> > - * \param coeffs The dewarp coefficients
> > + * \param params The dewarp parameters
> >    *
> >    * Sets the dewarp parameters according to the commonly used dewarp model. See
> >    * https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further details
> >    * on the model. The parameter \a coeffs must either hold 4,5,8 or 12 values.
> >    * They represent the parameters k1,k2,p1,p2[,k3[,k4,k5,k6[,s1,s2,s3,s4]]] in
> >    * the model.
> > - *
> > - * \return A negative number on error, 0 otherwise
> > - */
> > -int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
> > -                                 const Span<const double> &coeffs)
> > -{
> > -     dewarpM_ = cm;
> > -     dewarpCoeffs_.fill(0.0);
> > -
> > -     if (coeffs.size() != 4 && coeffs.size() != 5 &&
> > -         coeffs.size() != 8 && coeffs.size() != 12) {
> > -             LOG(Converter, Error)
> > -                     << "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
> > -             dewarpParamsValid_ = false;
> > -             return -EINVAL;
> > -     }
> > -     std::copy(coeffs.begin(), coeffs.end(), dewarpCoeffs_.begin());
> > -
> > -     dewarpParamsValid_ = true;
> > -     return 0;
> > -}
> > -
> > -/**
> > - * \fn Dw100VertexMap::dewarpParamsValid()
> > - * \brief Returns if the dewarp parameters are valid
> > - *
> > - * \return True if the dewarp parameters are valid, false otherwise
> >    */
> > 
> >   /**
> > @@ -627,32 +613,28 @@ int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
> >    */
> >   Vector2d Dw100VertexMap::dewarpPoint(const Vector2d &p)
> >   {
> > +     if (!dewarpParams_.has_value())
> > +             return p;
> > +
> >       double x, y;
> >       double xout, yout;
> > -     double k1 = dewarpCoeffs_[0];
> > -     double k2 = dewarpCoeffs_[1];
> > -     double p1 = dewarpCoeffs_[2];
> > -     double p2 = dewarpCoeffs_[3];
> > -     double k3 = dewarpCoeffs_[4];
> > -     double k4 = dewarpCoeffs_[5];
> > -     double k5 = dewarpCoeffs_[6];
> > -     double k6 = dewarpCoeffs_[7];
> > -     double s1 = dewarpCoeffs_[8];
> > -     double s2 = dewarpCoeffs_[9];
> > -     double s3 = dewarpCoeffs_[10];
> > -     double s4 = dewarpCoeffs_[11];
> > +     auto &cm = dewarpParams_->cm;
> > +     auto &c = dewarpParams_->coefficient;
> > 
> > -     y = (p.y() - dewarpM_[1][2]) / dewarpM_[1][1];
> > -     x = (p.x() - dewarpM_[0][2] - y * dewarpM_[0][1]) / dewarpM_[0][0];
> > +     y = (p.y() - cm[1][2]) / cm[1][1];
> > +     x = (p.x() - cm[0][2] - y * cm[0][1]) / cm[0][0];
> > 
> >       double r2 = x * x + y * y;
> > -     double d = (1 + k1 * r2 + k2 * r2 * r2 + k3 * r2 * r2 * r2) /
> > -                (1 + k4 * r2 + k5 * r2 * r2 + k6 * r2 * r2 * r2);
> > -     xout = x * d + 2 * p1 * x * y + p2 * (r2 + 2 * x * x) + s1 * r2 + s2 * r2 * r2;
> > -     yout = y * d + 2 * p2 * x * y + p1 * (r2 + 2 * y * y) + s3 * r2 + s4 * r2 * r2;
> > +     double r4 = r2 * r2;
> > +     double r6 = r2 * r2 * r2;
> > +     double d = (1 + c.k1 * r2 + c.k2 * r4 + c.k3 * r6) /
> > +                (1 + c.k4 * r2 + c.k5 * r4 + c.k6 * r6);
> > 
> > -     return { { xout * dewarpM_[0][0] + yout * dewarpM_[0][1] + dewarpM_[0][2],
> > -                yout * dewarpM_[1][1] + dewarpM_[1][2] } };
> > +     xout = x * d + 2 * c.p1 * x * y + c.p2 * (r2 + 2 * x * x) + c.s1 * r2 + c.s2 * r4;
> > +     yout = y * d + 2 * c.p2 * x * y + c.p1 * (r2 + 2 * y * y) + c.s3 * r2 + c.s4 * r4;
> > +
> > +     return { { xout * cm[0][0] + yout * cm[0][1] + cm[0][2],
> > +                yout * cm[1][1] + cm[1][2] } };
> >   }
> > 
> >   } /* namespace libcamera */
> > --
> > 2.53.0
> > 
>
Barnabás Pőcze June 17, 2026, 2:26 p.m. UTC | #3 
2026. 06. 17. 15:58 keltezéssel, Stefan Klug írta:
> Hi Barnabás,
> 
> Thanks for the review.
> 
> Quoting Barnabás Pőcze (2026-06-17 14:39:16)
>> 2026. 06. 17. 11:58 keltezéssel, Stefan Klug írta:
>>> There is already a ConverterDW100Module::DewarpParams class while in
>>> Dw100VertexMap the parameters are handled separately for no good reason.
>>> Create a Dw100VertexMap::DewarpParams struct for that purpose and adjust
>>> the code accordingly. This has the added benefit that the size checking
>>> for the coefficient list can now be done in the tuning file loading code
>>> where it belongs.
>>>
>>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
>>> ---
>>>    .../internal/converter/converter_dw100.h      |  7 +-
>>>    .../converter/converter_dw100_vertexmap.h     | 35 ++++++--
>>>    src/libcamera/converter/converter_dw100.cpp   | 15 +++-
>>>    .../converter/converter_dw100_vertexmap.cpp   | 82 ++++++++-----------
>>>    4 files changed, 75 insertions(+), 64 deletions(-)
>>>
>>> diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h
>>> index 1db1aadcb06b..003f5eb954e9 100644
>>> --- a/include/libcamera/internal/converter/converter_dw100.h
>>> +++ b/include/libcamera/internal/converter/converter_dw100.h
>>> @@ -69,18 +69,13 @@ private:
>>>        int applyControls(const Stream *stream, const V4L2Request *request);
>>>        void reinitRequest(V4L2Request *request);
>>>
>>> -     struct DewarpParms {
>>> -             Matrix<double, 3, 3> cm;
>>> -             std::vector<double> coeffs;
>>> -     };
>>> -
>>>        struct VertexMapInfo {
>>>                Dw100VertexMap map;
>>>                bool update;
>>>        };
>>>
>>>        std::map<const Stream *, VertexMapInfo> vertexMaps_;
>>> -     std::optional<DewarpParms> dewarpParams_;
>>> +     std::optional<Dw100VertexMap::DewarpParams> dewarpParams_;
>>>        unsigned int inputBufferCount_;
>>>        V4L2M2MConverter converter_;
>>>        Rectangle sensorCrop_;
>>> diff --git a/include/libcamera/internal/converter/converter_dw100_vertexmap.h b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
>>> index 3bf80ea66dc7..787498c7fdb5 100644
>>> --- a/include/libcamera/internal/converter/converter_dw100_vertexmap.h
>>> +++ b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
>>> @@ -9,6 +9,7 @@
>>>
>>>    #include <assert.h>
>>>    #include <cmath>
>>> +#include <optional>
>>>    #include <stdint.h>
>>>    #include <vector>
>>>
>>> @@ -30,6 +31,33 @@ public:
>>>                Crop = 1,
>>>        };
>>>
>>> +     struct DewarpParams {
>>> +             DewarpParams() : cm(Matrix<double, 3, 3>::identity()),
>>> +                              coefficients({})
>>> +             {
>>> +             }
>>> +
>>> +             Matrix<double, 3, 3> cm;
>>> +
>>> +             union {
>>> +                     struct {
>>> +                             double k1;
>>> +                             double k2;
>>> +                             double p1;
>>> +                             double p2;
>>> +                             double k3;
>>> +                             double k4;
>>> +                             double k5;
>>> +                             double k6;
>>> +                             double s1;
>>> +                             double s2;
>>> +                             double s3;
>>> +                             double s4;
>>> +                     } coefficient;
>>> +                     std::array<double, 12> coefficients;
>>> +             };
>>
>> I'm a bit worried about this part. As far as I am aware only the active member of
>> a union can technically be read (and there is the common initial sequence rule,
>> but I don't think it applies here, between a struct and and array). So I guess one
>> way to avoid this is to define a function for each. Or am I missing something that
>> makes this well-defined?
> 
> Thanks for pointing that out. I thought that is a common pattern.
> According to https://stackoverflow.com/a/21763025 it actually seems to
> be legal and the common initial sequence rule applies?

Funny, because I have come across https://stackoverflow.com/a/36080743
which seems to come to a different conclusion. And I'm inclined to agree
based on my reading of the common initial sequence rules, but entirely sure.


> 
> But nevertheless I changed the code to drop the union. I like that
> version better. So I'll post a v2 soon.
> 
> Best regards,
> Stefan
> 
>>
>>
>>> +     };
>>> +
>>>        void applyLimits();
>>>        void setInputSize(const Size &size)
>>>        {
>>> @@ -60,8 +88,7 @@ public:
>>>        void setMode(const ScaleMode mode) { mode_ = mode; }
>>>        ScaleMode mode() const { return mode_; }
>>>
>>> -     int setDewarpParams(const Matrix<double, 3, 3> &cm, const Span<const double> &coeffs);
>>> -     bool dewarpParamsValid() { return dewarpParamsValid_; }
>>> +     void setDewarpParams(const DewarpParams &params) { dewarpParams_ = params; }
>>>
>>>        void setLensDewarpEnable(bool enable) { lensDewarpEnable_ = enable; }
>>>        bool lensDewarpEnable() { return lensDewarpEnable_; }
>>> @@ -85,10 +112,8 @@ private:
>>>        Point effectiveOffset_;
>>>        Rectangle effectiveScalerCrop_;
>>>
>>> -     Matrix<double, 3, 3> dewarpM_ = Matrix<double, 3, 3>::identity();
>>> -     std::array<double, 12> dewarpCoeffs_;
>>> +     std::optional<DewarpParams> dewarpParams_;
>>>        bool lensDewarpEnable_ = true;
>>> -     bool dewarpParamsValid_ = false;
>>>    };
>>>
>>>    } /* namespace libcamera */
>>> diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp
>>> index 44f7ec035e62..34f81c6b9fab 100644
>>> --- a/src/libcamera/converter/converter_dw100.cpp
>>> +++ b/src/libcamera/converter/converter_dw100.cpp
>>> @@ -15,6 +15,7 @@
>>>    #include <libcamera/stream.h>
>>>
>>>    #include "libcamera/internal/converter.h"
>>> +#include "libcamera/internal/converter/converter_dw100_vertexmap.h"
>>>    #include "libcamera/internal/converter/converter_v4l2_m2m.h"
>>>    #include "libcamera/internal/media_device.h"
>>>    #include "libcamera/internal/v4l2_videodevice.h"
>>> @@ -99,7 +100,7 @@ ConverterDW100Module::createModule(DeviceEnumerator *enumerator)
>>>     */
>>>    int ConverterDW100Module::init(const ValueNode &params)
>>>    {
>>> -     DewarpParms dp;
>>> +     Dw100VertexMap::DewarpParams dp;
>>>
>>>        auto &cm = params["cm"];
>>>        auto &coefficients = params["coefficients"];
>>> @@ -131,7 +132,15 @@ int ConverterDW100Module::init(const ValueNode &params)
>>>                LOG(Converter, Error) << "Dewarp parameters 'coefficients' value is not a list";
>>>                return -EINVAL;
>>>        }
>>> -     dp.coeffs = std::move(*coeffs);
>>> +
>>> +     if (coeffs->size() != 4 && coeffs->size() != 5 &&
>>> +         coeffs->size() != 8 && coeffs->size() != 12) {
>>> +             LOG(Converter, Error)
>>> +                     << "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     std::copy(coeffs->begin(), coeffs->end(), &dp.coefficients[0]);
>>>
>>>        dewarpParams_ = dp;
>>>
>>> @@ -163,7 +172,7 @@ int ConverterDW100Module::configure(const StreamConfiguration &inputCfg,
>>>                vertexMap.setSensorCrop(sensorCrop_);
>>>
>>>                if (dewarpParams_)
>>> -                     vertexMap.setDewarpParams(dewarpParams_->cm, dewarpParams_->coeffs);
>>> +                     vertexMap.setDewarpParams(*dewarpParams_);
>>>                info.update = true;
>>>        }
>>>
>>> diff --git a/src/libcamera/converter/converter_dw100_vertexmap.cpp b/src/libcamera/converter/converter_dw100_vertexmap.cpp
>>> index 6e238736c435..31d992546857 100644
>>> --- a/src/libcamera/converter/converter_dw100_vertexmap.cpp
>>> +++ b/src/libcamera/converter/converter_dw100_vertexmap.cpp
>>> @@ -227,6 +227,20 @@ int dw100VerticesForLength(const int length)
>>>     * into account within the possible limits.
>>>     */
>>>
>>> +/**
>>> + * \struct Dw100VertexMap::DewarpParams
>>> + * \brief Structure combining all dewarp parameters
>>> + *
>>> + * \var Dw100VertexMap::DewarpParams::cm
>>> + * \brief The camera matrix
>>> + *
>>> + * \var Dw100VertexMap::DewarpParams::coefficient
>>> + * \brief Structure containing the lens dewarp coefficients
>>> +
>>> + * See https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further
>>> + * details on the model.
>>> + */
>>> +
>>>    /**
>>>     * \brief Apply limits on scale and offset
>>>     *
>>> @@ -549,7 +563,7 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
>>>
>>>                        p = transformPoint(outputToSensor, p);
>>>
>>> -                     if (dewarpParamsValid_ && lensDewarpEnable_)
>>> +                     if (lensDewarpEnable_)
>>>                                p = dewarpPoint(p);
>>>
>>>                        p = transformPoint(sensorToInput, p);
>>> @@ -566,41 +580,13 @@ std::vector<uint32_t> Dw100VertexMap::getVertexMap()
>>>
>>>    /**
>>>     * \brief Set the dewarp parameters
>>> - * \param cm The camera matrix
>>> - * \param coeffs The dewarp coefficients
>>> + * \param params The dewarp parameters
>>>     *
>>>     * Sets the dewarp parameters according to the commonly used dewarp model. See
>>>     * https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further details
>>>     * on the model. The parameter \a coeffs must either hold 4,5,8 or 12 values.
>>>     * They represent the parameters k1,k2,p1,p2[,k3[,k4,k5,k6[,s1,s2,s3,s4]]] in
>>>     * the model.
>>> - *
>>> - * \return A negative number on error, 0 otherwise
>>> - */
>>> -int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
>>> -                                 const Span<const double> &coeffs)
>>> -{
>>> -     dewarpM_ = cm;
>>> -     dewarpCoeffs_.fill(0.0);
>>> -
>>> -     if (coeffs.size() != 4 && coeffs.size() != 5 &&
>>> -         coeffs.size() != 8 && coeffs.size() != 12) {
>>> -             LOG(Converter, Error)
>>> -                     << "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
>>> -             dewarpParamsValid_ = false;
>>> -             return -EINVAL;
>>> -     }
>>> -     std::copy(coeffs.begin(), coeffs.end(), dewarpCoeffs_.begin());
>>> -
>>> -     dewarpParamsValid_ = true;
>>> -     return 0;
>>> -}
>>> -
>>> -/**
>>> - * \fn Dw100VertexMap::dewarpParamsValid()
>>> - * \brief Returns if the dewarp parameters are valid
>>> - *
>>> - * \return True if the dewarp parameters are valid, false otherwise
>>>     */
>>>
>>>    /**
>>> @@ -627,32 +613,28 @@ int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
>>>     */
>>>    Vector2d Dw100VertexMap::dewarpPoint(const Vector2d &p)
>>>    {
>>> +     if (!dewarpParams_.has_value())
>>> +             return p;
>>> +
>>>        double x, y;
>>>        double xout, yout;
>>> -     double k1 = dewarpCoeffs_[0];
>>> -     double k2 = dewarpCoeffs_[1];
>>> -     double p1 = dewarpCoeffs_[2];
>>> -     double p2 = dewarpCoeffs_[3];
>>> -     double k3 = dewarpCoeffs_[4];
>>> -     double k4 = dewarpCoeffs_[5];
>>> -     double k5 = dewarpCoeffs_[6];
>>> -     double k6 = dewarpCoeffs_[7];
>>> -     double s1 = dewarpCoeffs_[8];
>>> -     double s2 = dewarpCoeffs_[9];
>>> -     double s3 = dewarpCoeffs_[10];
>>> -     double s4 = dewarpCoeffs_[11];
>>> +     auto &cm = dewarpParams_->cm;
>>> +     auto &c = dewarpParams_->coefficient;
>>>
>>> -     y = (p.y() - dewarpM_[1][2]) / dewarpM_[1][1];
>>> -     x = (p.x() - dewarpM_[0][2] - y * dewarpM_[0][1]) / dewarpM_[0][0];
>>> +     y = (p.y() - cm[1][2]) / cm[1][1];
>>> +     x = (p.x() - cm[0][2] - y * cm[0][1]) / cm[0][0];
>>>
>>>        double r2 = x * x + y * y;
>>> -     double d = (1 + k1 * r2 + k2 * r2 * r2 + k3 * r2 * r2 * r2) /
>>> -                (1 + k4 * r2 + k5 * r2 * r2 + k6 * r2 * r2 * r2);
>>> -     xout = x * d + 2 * p1 * x * y + p2 * (r2 + 2 * x * x) + s1 * r2 + s2 * r2 * r2;
>>> -     yout = y * d + 2 * p2 * x * y + p1 * (r2 + 2 * y * y) + s3 * r2 + s4 * r2 * r2;
>>> +     double r4 = r2 * r2;
>>> +     double r6 = r2 * r2 * r2;
>>> +     double d = (1 + c.k1 * r2 + c.k2 * r4 + c.k3 * r6) /
>>> +                (1 + c.k4 * r2 + c.k5 * r4 + c.k6 * r6);
>>>
>>> -     return { { xout * dewarpM_[0][0] + yout * dewarpM_[0][1] + dewarpM_[0][2],
>>> -                yout * dewarpM_[1][1] + dewarpM_[1][2] } };
>>> +     xout = x * d + 2 * c.p1 * x * y + c.p2 * (r2 + 2 * x * x) + c.s1 * r2 + c.s2 * r4;
>>> +     yout = y * d + 2 * c.p2 * x * y + c.p1 * (r2 + 2 * y * y) + c.s3 * r2 + c.s4 * r4;
>>> +
>>> +     return { { xout * cm[0][0] + yout * cm[0][1] + cm[0][2],
>>> +                yout * cm[1][1] + cm[1][2] } };
>>>    }
>>>
>>>    } /* namespace libcamera */
>>> --
>>> 2.53.0
>>>
>>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/converter/converter_dw100.h b/include/libcamera/internal/converter/converter_dw100.h
index 1db1aadcb06b..003f5eb954e9 100644
--- a/include/libcamera/internal/converter/converter_dw100.h
+++ b/include/libcamera/internal/converter/converter_dw100.h
@@ -69,18 +69,13 @@  private:
 	int applyControls(const Stream *stream, const V4L2Request *request);
 	void reinitRequest(V4L2Request *request);
 
-	struct DewarpParms {
-		Matrix<double, 3, 3> cm;
-		std::vector<double> coeffs;
-	};
-
 	struct VertexMapInfo {
 		Dw100VertexMap map;
 		bool update;
 	};
 
 	std::map<const Stream *, VertexMapInfo> vertexMaps_;
-	std::optional<DewarpParms> dewarpParams_;
+	std::optional<Dw100VertexMap::DewarpParams> dewarpParams_;
 	unsigned int inputBufferCount_;
 	V4L2M2MConverter converter_;
 	Rectangle sensorCrop_;
diff --git a/include/libcamera/internal/converter/converter_dw100_vertexmap.h b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
index 3bf80ea66dc7..787498c7fdb5 100644
--- a/include/libcamera/internal/converter/converter_dw100_vertexmap.h
+++ b/include/libcamera/internal/converter/converter_dw100_vertexmap.h
@@ -9,6 +9,7 @@ 
 
 #include <assert.h>
 #include <cmath>
+#include <optional>
 #include <stdint.h>
 #include <vector>
 
@@ -30,6 +31,33 @@  public:
 		Crop = 1,
 	};
 
+	struct DewarpParams {
+		DewarpParams() : cm(Matrix<double, 3, 3>::identity()),
+				 coefficients({})
+		{
+		}
+
+		Matrix<double, 3, 3> cm;
+
+		union {
+			struct {
+				double k1;
+				double k2;
+				double p1;
+				double p2;
+				double k3;
+				double k4;
+				double k5;
+				double k6;
+				double s1;
+				double s2;
+				double s3;
+				double s4;
+			} coefficient;
+			std::array<double, 12> coefficients;
+		};
+	};
+
 	void applyLimits();
 	void setInputSize(const Size &size)
 	{
@@ -60,8 +88,7 @@  public:
 	void setMode(const ScaleMode mode) { mode_ = mode; }
 	ScaleMode mode() const { return mode_; }
 
-	int setDewarpParams(const Matrix<double, 3, 3> &cm, const Span<const double> &coeffs);
-	bool dewarpParamsValid() { return dewarpParamsValid_; }
+	void setDewarpParams(const DewarpParams &params) { dewarpParams_ = params; }
 
 	void setLensDewarpEnable(bool enable) { lensDewarpEnable_ = enable; }
 	bool lensDewarpEnable() { return lensDewarpEnable_; }
@@ -85,10 +112,8 @@  private:
 	Point effectiveOffset_;
 	Rectangle effectiveScalerCrop_;
 
-	Matrix<double, 3, 3> dewarpM_ = Matrix<double, 3, 3>::identity();
-	std::array<double, 12> dewarpCoeffs_;
+	std::optional<DewarpParams> dewarpParams_;
 	bool lensDewarpEnable_ = true;
-	bool dewarpParamsValid_ = false;
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/converter/converter_dw100.cpp b/src/libcamera/converter/converter_dw100.cpp
index 44f7ec035e62..34f81c6b9fab 100644
--- a/src/libcamera/converter/converter_dw100.cpp
+++ b/src/libcamera/converter/converter_dw100.cpp
@@ -15,6 +15,7 @@ 
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/converter.h"
+#include "libcamera/internal/converter/converter_dw100_vertexmap.h"
 #include "libcamera/internal/converter/converter_v4l2_m2m.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/v4l2_videodevice.h"
@@ -99,7 +100,7 @@  ConverterDW100Module::createModule(DeviceEnumerator *enumerator)
  */
 int ConverterDW100Module::init(const ValueNode &params)
 {
-	DewarpParms dp;
+	Dw100VertexMap::DewarpParams dp;
 
 	auto &cm = params["cm"];
 	auto &coefficients = params["coefficients"];
@@ -131,7 +132,15 @@  int ConverterDW100Module::init(const ValueNode &params)
 		LOG(Converter, Error) << "Dewarp parameters 'coefficients' value is not a list";
 		return -EINVAL;
 	}
-	dp.coeffs = std::move(*coeffs);
+
+	if (coeffs->size() != 4 && coeffs->size() != 5 &&
+	    coeffs->size() != 8 && coeffs->size() != 12) {
+		LOG(Converter, Error)
+			<< "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
+		return -EINVAL;
+	}
+
+	std::copy(coeffs->begin(), coeffs->end(), &dp.coefficients[0]);
 
 	dewarpParams_ = dp;
 
@@ -163,7 +172,7 @@  int ConverterDW100Module::configure(const StreamConfiguration &inputCfg,
 		vertexMap.setSensorCrop(sensorCrop_);
 
 		if (dewarpParams_)
-			vertexMap.setDewarpParams(dewarpParams_->cm, dewarpParams_->coeffs);
+			vertexMap.setDewarpParams(*dewarpParams_);
 		info.update = true;
 	}
 
diff --git a/src/libcamera/converter/converter_dw100_vertexmap.cpp b/src/libcamera/converter/converter_dw100_vertexmap.cpp
index 6e238736c435..31d992546857 100644
--- a/src/libcamera/converter/converter_dw100_vertexmap.cpp
+++ b/src/libcamera/converter/converter_dw100_vertexmap.cpp
@@ -227,6 +227,20 @@  int dw100VerticesForLength(const int length)
  * into account within the possible limits.
  */
 
+/**
+ * \struct Dw100VertexMap::DewarpParams
+ * \brief Structure combining all dewarp parameters
+ *
+ * \var Dw100VertexMap::DewarpParams::cm
+ * \brief The camera matrix
+ *
+ * \var Dw100VertexMap::DewarpParams::coefficient
+ * \brief Structure containing the lens dewarp coefficients
+
+ * See https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further
+ * details on the model.
+ */
+
 /**
  * \brief Apply limits on scale and offset
  *
@@ -549,7 +563,7 @@  std::vector<uint32_t> Dw100VertexMap::getVertexMap()
 
 			p = transformPoint(outputToSensor, p);
 
-			if (dewarpParamsValid_ && lensDewarpEnable_)
+			if (lensDewarpEnable_)
 				p = dewarpPoint(p);
 
 			p = transformPoint(sensorToInput, p);
@@ -566,41 +580,13 @@  std::vector<uint32_t> Dw100VertexMap::getVertexMap()
 
 /**
  * \brief Set the dewarp parameters
- * \param cm The camera matrix
- * \param coeffs The dewarp coefficients
+ * \param params The dewarp parameters
  *
  * Sets the dewarp parameters according to the commonly used dewarp model. See
  * https://docs.opencv.org/4.12.0/d9/d0c/group__calib3d.html for further details
  * on the model. The parameter \a coeffs must either hold 4,5,8 or 12 values.
  * They represent the parameters k1,k2,p1,p2[,k3[,k4,k5,k6[,s1,s2,s3,s4]]] in
  * the model.
- *
- * \return A negative number on error, 0 otherwise
- */
-int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
-				    const Span<const double> &coeffs)
-{
-	dewarpM_ = cm;
-	dewarpCoeffs_.fill(0.0);
-
-	if (coeffs.size() != 4 && coeffs.size() != 5 &&
-	    coeffs.size() != 8 && coeffs.size() != 12) {
-		LOG(Converter, Error)
-			<< "Dewarp 'coefficients' must have 4, 5, 8 or 12 values";
-		dewarpParamsValid_ = false;
-		return -EINVAL;
-	}
-	std::copy(coeffs.begin(), coeffs.end(), dewarpCoeffs_.begin());
-
-	dewarpParamsValid_ = true;
-	return 0;
-}
-
-/**
- * \fn Dw100VertexMap::dewarpParamsValid()
- * \brief Returns if the dewarp parameters are valid
- *
- * \return True if the dewarp parameters are valid, false otherwise
  */
 
 /**
@@ -627,32 +613,28 @@  int Dw100VertexMap::setDewarpParams(const Matrix<double, 3, 3> &cm,
  */
 Vector2d Dw100VertexMap::dewarpPoint(const Vector2d &p)
 {
+	if (!dewarpParams_.has_value())
+		return p;
+
 	double x, y;
 	double xout, yout;
-	double k1 = dewarpCoeffs_[0];
-	double k2 = dewarpCoeffs_[1];
-	double p1 = dewarpCoeffs_[2];
-	double p2 = dewarpCoeffs_[3];
-	double k3 = dewarpCoeffs_[4];
-	double k4 = dewarpCoeffs_[5];
-	double k5 = dewarpCoeffs_[6];
-	double k6 = dewarpCoeffs_[7];
-	double s1 = dewarpCoeffs_[8];
-	double s2 = dewarpCoeffs_[9];
-	double s3 = dewarpCoeffs_[10];
-	double s4 = dewarpCoeffs_[11];
+	auto &cm = dewarpParams_->cm;
+	auto &c = dewarpParams_->coefficient;
 
-	y = (p.y() - dewarpM_[1][2]) / dewarpM_[1][1];
-	x = (p.x() - dewarpM_[0][2] - y * dewarpM_[0][1]) / dewarpM_[0][0];
+	y = (p.y() - cm[1][2]) / cm[1][1];
+	x = (p.x() - cm[0][2] - y * cm[0][1]) / cm[0][0];
 
 	double r2 = x * x + y * y;
-	double d = (1 + k1 * r2 + k2 * r2 * r2 + k3 * r2 * r2 * r2) /
-		   (1 + k4 * r2 + k5 * r2 * r2 + k6 * r2 * r2 * r2);
-	xout = x * d + 2 * p1 * x * y + p2 * (r2 + 2 * x * x) + s1 * r2 + s2 * r2 * r2;
-	yout = y * d + 2 * p2 * x * y + p1 * (r2 + 2 * y * y) + s3 * r2 + s4 * r2 * r2;
+	double r4 = r2 * r2;
+	double r6 = r2 * r2 * r2;
+	double d = (1 + c.k1 * r2 + c.k2 * r4 + c.k3 * r6) /
+		   (1 + c.k4 * r2 + c.k5 * r4 + c.k6 * r6);
 
-	return { { xout * dewarpM_[0][0] + yout * dewarpM_[0][1] + dewarpM_[0][2],
-		   yout * dewarpM_[1][1] + dewarpM_[1][2] } };
+	xout = x * d + 2 * c.p1 * x * y + c.p2 * (r2 + 2 * x * x) + c.s1 * r2 + c.s2 * r4;
+	yout = y * d + 2 * c.p2 * x * y + c.p1 * (r2 + 2 * y * y) + c.s3 * r2 + c.s4 * r4;
+
+	return { { xout * cm[0][0] + yout * cm[0][1] + cm[0][2],
+		   yout * cm[1][1] + cm[1][2] } };
 }
 
 } /* namespace libcamera */