[libcamera-devel,1/2] libcamera: base: utils: Add abs_diff() utility function

15052 diff mbox series

The abs_diff() function computes the absolute difference of two
elements. This may seem trivial at first, but can lead to unexpected
results when operating on unsigned operands. A common implementation
of the absolute difference of two unsigned int (used through the
libcamera code base) is

	std::abs(static_cast<int>(a - b))

but doesn't return the expected result when either a or b is larger than
UINT_MAX / 2 due to overflows. The abs_diff() function offers a safe
alternative.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/base/utils.h |  9 +++++++++
 src/libcamera/base/utils.cpp   | 17 +++++++++++++++++
 2 files changed, 26 insertions(+)

Quoting Laurent Pinchart (2021-12-06 15:26:43)
> The abs_diff() function computes the absolute difference of two
> elements. This may seem trivial at first, but can lead to unexpected
> results when operating on unsigned operands. A common implementation
> of the absolute difference of two unsigned int (used through the
> libcamera code base) is
> 
>         std::abs(static_cast<int>(a - b))
> 
> but doesn't return the expected result when either a or b is larger than
> UINT_MAX / 2 due to overflows. The abs_diff() function offers a safe
> alternative.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/base/utils.h |  9 +++++++++
>  src/libcamera/base/utils.cpp   | 17 +++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> index 3a803176693d..9ab18101cf27 100644
> --- a/include/libcamera/base/utils.h
> +++ b/include/libcamera/base/utils.h
> @@ -346,6 +346,15 @@ public:
>         }
>  };
>  
> +template<typename T>
> +decltype(auto) abs_diff(const T &a, const T &b)
> +{
> +       if (a < b)
> +               return b - a;
> +       else
> +               return a - b;
> +}
> +
>  } /* namespace utils */
>  
>  #ifndef __DOXYGEN__
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index 45b92b670837..8cf8a5b2c104 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -437,6 +437,23 @@ std::string toAscii(const std::string &str)
>   * \return True if \a Duration is a non-zero time value, False otherwise
>   */
>  
> +/**
> + * \fn abs_diff(const T& a, const T& b)
> + * \brief Calculates the absolute value of the difference between two elements
> + * \param[in] a The first element
> + * \param[in] b The second element
> + *
> + * This function calculates the absolute value of the difference between two
> + * elements of the same type, in such a way that a negative value will never
> + * occur during the calculation.
> + *
> + * This is inspired by the std::abs_diff() candidate proposed in N4318
> + * (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4318.pdf).

It's unfortunate that this proposal hasn't progressed.
I'm happy to see an implementation in our utils though.

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

> + *
> + * \return The absolute value of the difference of the two parameters \a a and
> + * \a b
> + */
> +
>  } /* namespace utils */
>  
>  #ifndef __DOXYGEN__
> -- 
> Regards,
> 
> Laurent Pinchart
>

Hi Laurent,

Thanks for the patch

On 12/6/21 8:56 PM, Laurent Pinchart wrote:
> The abs_diff() function computes the absolute difference of two
> elements. This may seem trivial at first, but can lead to unexpected
> results when operating on unsigned operands. A common implementation
> of the absolute difference of two unsigned int (used through the
> libcamera code base) is
>
> 	std::abs(static_cast<int>(a - b))
>
> but doesn't return the expected result when either a or b is larger than
> UINT_MAX / 2 due to overflows. The abs_diff() function offers a safe
> alternative.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   include/libcamera/base/utils.h |  9 +++++++++
>   src/libcamera/base/utils.cpp   | 17 +++++++++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> index 3a803176693d..9ab18101cf27 100644
> --- a/include/libcamera/base/utils.h
> +++ b/include/libcamera/base/utils.h
> @@ -346,6 +346,15 @@ public:
>   	}
>   };
>   
> +template<typename T>
> +decltype(auto) abs_diff(const T &a, const T &b)
> +{
> +	if (a < b)
> +		return b - a;
> +	else
> +		return a - b;
> +}
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index 45b92b670837..8cf8a5b2c104 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -437,6 +437,23 @@ std::string toAscii(const std::string &str)
>    * \return True if \a Duration is a non-zero time value, False otherwise
>    */
>   
> +/**
> + * \fn abs_diff(const T& a, const T& b)
> + * \brief Calculates the absolute value of the difference between two elements
> + * \param[in] a The first element
> + * \param[in] b The second element
> + *
> + * This function calculates the absolute value of the difference between two
> + * elements of the same type, in such a way that a negative value will never
> + * occur during the calculation.
> + *
> + * This is inspired by the std::abs_diff() candidate proposed in N4318
> + * (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4318.pdf).
> + *
> + * \return The absolute value of the difference of the two parameters \a a and
> + * \a b
> + */
> +
>   } /* namespace utils */
>   
>   #ifndef __DOXYGEN__