[libcamera-devel] libcamera: utils: Add clamp()

Message ID 20190713113946.25744-1-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • [libcamera-devel] libcamera: utils: Add clamp()
Related show

Commit Message

Niklas Söderlund July 13, 2019, 11:39 a.m. UTC
C++11 does not support std::clamp(), add a custom implementation in
utils.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/include/utils.h | 7 +++++++
 src/libcamera/utils.cpp       | 9 +++++++++
 2 files changed, 16 insertions(+)

Comments

Laurent Pinchart July 13, 2019, 11:53 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Sat, Jul 13, 2019 at 08:39:46PM +0900, Niklas Söderlund wrote:
> C++11 does not support std::clamp(), add a custom implementation in
> utils.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/include/utils.h | 7 +++++++
>  src/libcamera/utils.cpp       | 9 +++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> index 97bd470a45b050ef..beb2ce3ff7da9352 100644
> --- a/src/libcamera/include/utils.h
> +++ b/src/libcamera/include/utils.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_UTILS_H__
>  #define __LIBCAMERA_UTILS_H__
>  
> +#include <algorithm>
>  #include <memory>
>  
>  #define ARRAY_SIZE(a)	(sizeof(a) / sizeof(a[0]))
> @@ -45,6 +46,12 @@ unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
>  	return count;
>  }
>  
> +/* C++11 doesn't provide std::clamp */
> +template <typename T>
> +T clamp(const T& v, const T& lo, const T& hi) {

The { should go to the next line.

The function should return a const T&. Ideally it should even be a
constexpr, but std::max and std::min are only constexpr starting in
C++14, so you would have to reimplement that, and it's likely not worth
it.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

with this fixed.

> +	return std::max(lo, std::min(v, hi));
> +}
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index ef365366f29b426e..b0ca3565ee5a0d83 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -85,6 +85,15 @@ char *secure_getenv(const char *name)
>   * \return The number of elements in the intersection of the two ranges
>   */
>  
> +/**
> + * \fn libcamera::utils::clamp(const T& v, const T& lo, const T& hi)
> + * \param[in] v The value to clamp
> + * \param[in] lo The lower boundary to clamp v to
> + * \param[in] hi The higher boundary to clamp v to
> + *
> + * \return lo if v is less than lo, hi if v is greater than hi, otherwise v
> + */
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
Niklas Söderlund July 13, 2019, 1:38 p.m. UTC | #2
Pushed with Laurent's tag.

On 2019-07-13 14:53:19 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sat, Jul 13, 2019 at 08:39:46PM +0900, Niklas Söderlund wrote:
> > C++11 does not support std::clamp(), add a custom implementation in
> > utils.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/include/utils.h | 7 +++++++
> >  src/libcamera/utils.cpp       | 9 +++++++++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> > index 97bd470a45b050ef..beb2ce3ff7da9352 100644
> > --- a/src/libcamera/include/utils.h
> > +++ b/src/libcamera/include/utils.h
> > @@ -7,6 +7,7 @@
> >  #ifndef __LIBCAMERA_UTILS_H__
> >  #define __LIBCAMERA_UTILS_H__
> >  
> > +#include <algorithm>
> >  #include <memory>
> >  
> >  #define ARRAY_SIZE(a)	(sizeof(a) / sizeof(a[0]))
> > @@ -45,6 +46,12 @@ unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
> >  	return count;
> >  }
> >  
> > +/* C++11 doesn't provide std::clamp */
> > +template <typename T>
> > +T clamp(const T& v, const T& lo, const T& hi) {
> 
> The { should go to the next line.
> 
> The function should return a const T&. Ideally it should even be a
> constexpr, but std::max and std::min are only constexpr starting in
> C++14, so you would have to reimplement that, and it's likely not worth
> it.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> with this fixed.
> 
> > +	return std::max(lo, std::min(v, hi));
> > +}
> > +
> >  } /* namespace utils */
> >  
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > index ef365366f29b426e..b0ca3565ee5a0d83 100644
> > --- a/src/libcamera/utils.cpp
> > +++ b/src/libcamera/utils.cpp
> > @@ -85,6 +85,15 @@ char *secure_getenv(const char *name)
> >   * \return The number of elements in the intersection of the two ranges
> >   */
> >  
> > +/**
> > + * \fn libcamera::utils::clamp(const T& v, const T& lo, const T& hi)
> > + * \param[in] v The value to clamp
> > + * \param[in] lo The lower boundary to clamp v to
> > + * \param[in] hi The higher boundary to clamp v to
> > + *
> > + * \return lo if v is less than lo, hi if v is greater than hi, otherwise v
> > + */
> > +
> >  } /* namespace utils */
> >  
> >  } /* namespace libcamera */
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch

diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
index 97bd470a45b050ef..beb2ce3ff7da9352 100644
--- a/src/libcamera/include/utils.h
+++ b/src/libcamera/include/utils.h
@@ -7,6 +7,7 @@ 
 #ifndef __LIBCAMERA_UTILS_H__
 #define __LIBCAMERA_UTILS_H__
 
+#include <algorithm>
 #include <memory>
 
 #define ARRAY_SIZE(a)	(sizeof(a) / sizeof(a[0]))
@@ -45,6 +46,12 @@  unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
 	return count;
 }
 
+/* C++11 doesn't provide std::clamp */
+template <typename T>
+T clamp(const T& v, const T& lo, const T& hi) {
+	return std::max(lo, std::min(v, hi));
+}
+
 } /* namespace utils */
 
 } /* namespace libcamera */
diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index ef365366f29b426e..b0ca3565ee5a0d83 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -85,6 +85,15 @@  char *secure_getenv(const char *name)
  * \return The number of elements in the intersection of the two ranges
  */
 
+/**
+ * \fn libcamera::utils::clamp(const T& v, const T& lo, const T& hi)
+ * \param[in] v The value to clamp
+ * \param[in] lo The lower boundary to clamp v to
+ * \param[in] hi The higher boundary to clamp v to
+ *
+ * \return lo if v is less than lo, hi if v is greater than hi, otherwise v
+ */
+
 } /* namespace utils */
 
 } /* namespace libcamera */