[libcamera-devel] [PATCH] libcamera: ipu3: Fix compilation issues with gcc5

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Aug 03, 2020 at 04:18:37PM +0200, Jacopo Mondi wrote:
> GCC5 does not provide prototypes for the math library functions in
> the std:: namespace.
> 
> Remove the namespace specifier to fix build errors reported by
> that compiler version.
> 
> Fixes: 968ab9bad0ed ("libcamera: ipu3: imgu: Calculate ImgU pipe configuration")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> 
> Laurent, can you test with gcc and confirm this fixes the issue ?

It does fix the compilation issue, but... see below.

> ---
>  src/libcamera/pipeline/ipu3/imgu.cpp | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index b7593ceb3672..671a798e69d0 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -94,7 +94,7 @@ float findScaleFactor(float sf, const std::vector<float> &range,
>  	float bestDiff = std::numeric_limits<float>::max();
>  	unsigned int index = 0;
>  	for (unsigned int i = 0; i < range.size(); ++i) {
> -		float diff = std::abs(sf - range[i]);
> +		float diff = abs(sf - range[i]);

The abs() function in the global namespace is, according to
https://en.cppreference.com/w/c/numeric/math/abs,

	int abs( int n );

This will thus cast the argument to int, which is likely not what you
want. You need to use

	float fabsf( float arg );

instead (https://en.cppreference.com/w/c/numeric/math/fabs).

Same below.

The math functions in the std:: namespace don't suffer for this, as
multiple overloads are defined:

int       abs( int n );
long      abs( long n );
long long abs( long long n );

(https://en.cppreference.com/w/cpp/numeric/math/abs)

float       abs( float arg );
double      abs( double arg );
long double abs( long double arg );

(https://en.cppreference.com/w/cpp/numeric/math/fabs)

but they require <cmath>, not <math.h>.

It's up to you, but if you keep using math.h, you need fabsf() when the
argument and result are floats. Similarly, fmod() should become fmodf(),
otherwise you'll cast the argument to double and cast the result back to
float.

>  		if (diff < bestDiff) {
>  			bestDiff = diff;
>  			index = i;
> @@ -112,7 +112,7 @@ bool isSameRatio(const Size &in, const Size &out)
>  	float inRatio = static_cast<float>(in.width) / in.height;
>  	float outRatio = static_cast<float>(out.width) / out.height;
> 
> -	if (std::abs(inRatio - outRatio) > 0.1)
> +	if (abs(inRatio - outRatio) > 0.1)
>  		return false;
> 
>  	return true;
> @@ -136,7 +136,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
>  		while (ifHeight >= minIFHeight && ifHeight / bdsSF >= minBDSHeight) {
> 
>  			bdsHeight = ifHeight / bdsSF;
> -			if (std::fmod(bdsHeight, 1.0) == 0) {
> +			if (fmod(bdsHeight, 1.0) == 0) {
>  				unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
> 
>  				if (!(bdsIntHeight % BDS_ALIGN_H)) {
> @@ -152,7 +152,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
>  		while (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {
> 
>  			bdsHeight = ifHeight / bdsSF;
> -			if (std::fmod(bdsHeight, 1.0) == 0) {
> +			if (fmod(bdsHeight, 1.0) == 0) {
>  				unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
> 
>  				if (!(bdsIntHeight % BDS_ALIGN_H)) {
> @@ -176,7 +176,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
>  		while (ifHeight > minIFHeight && ifHeight / bdsSF >= minBDSHeight) {
> 
>  			bdsHeight = ifHeight / bdsSF;
> -			if (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {
> +			if (fmod(ifHeight, 1.0) == 0 && fmod(bdsHeight, 1.0) == 0) {
>  				unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
> 
>  				if (!(ifHeight % IF_ALIGN_H) &&
> @@ -199,7 +199,7 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, floa
>  	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
>  		float bdsWidth = static_cast<float>(iif.width) / sf;
> 
> -		if (std::fmod(bdsWidth, 1.0) == 0) {
> +		if (fmod(bdsWidth, 1.0) == 0) {
>  			unsigned int bdsIntWidth = static_cast<unsigned int>(bdsWidth);
>  			if (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)
>  				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
> @@ -212,7 +212,7 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, floa
>  	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
>  		float bdsWidth = static_cast<float>(iif.width) / sf;
> 
> -		if (std::fmod(bdsWidth, 1.0) == 0) {
> +		if (fmod(bdsWidth, 1.0) == 0) {
>  			unsigned int bdsIntWidth = static_cast<unsigned int>(bdsWidth);
>  			if (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth)
>  				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);

-- 
Regards,

Laurent Pinchart