[libcamera-devel,2/5] libcamera: Replace utils::clamp() with std::clamp()
diff mbox series

Message ID 20200822200037.20892-3-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Switch to C++17
Related show

Commit Message

Laurent Pinchart Aug. 22, 2020, 8 p.m. UTC 
Now that libcamera uses C++17, the C++ standard library provides
std::clamp(). Drop our custom utils::clamp() function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/utils.h   |  7 -------
 src/ipa/rkisp1/rkisp1.cpp            | 11 +++++------
 src/libcamera/pipeline/ipu3/imgu.cpp |  3 ++-
 src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++------
 src/libcamera/pipeline/vimc/vimc.cpp |  2 +-
 src/libcamera/utils.cpp              |  8 --------
 6 files changed, 14 insertions(+), 29 deletions(-)

Comments

Kieran Bingham Aug. 24, 2020, 9:54 a.m. UTC | #1 
Hi Laurent,

On 22/08/2020 21:00, Laurent Pinchart wrote:
> Now that libcamera uses C++17, the C++ standard library provides
> std::clamp(). Drop our custom utils::clamp() function.
> 

This one makes me happy. The less code we have to duplicate from
standard libraries the better ;-)


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

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

> ---
>  include/libcamera/internal/utils.h   |  7 -------
>  src/ipa/rkisp1/rkisp1.cpp            | 11 +++++------
>  src/libcamera/pipeline/ipu3/imgu.cpp |  3 ++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++------
>  src/libcamera/pipeline/vimc/vimc.cpp |  2 +-
>  src/libcamera/utils.cpp              |  8 --------
>  6 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
> index 45cd6f120c51..a1b644b0345b 100644
> --- a/include/libcamera/internal/utils.h
> +++ b/include/libcamera/internal/utils.h
> @@ -65,13 +65,6 @@ unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
>  	return count;
>  }
>  
> -/* C++11 doesn't provide std::clamp */
> -template <typename T>
> -const T& clamp(const T& v, const T& lo, const T& hi)
> -{
> -	return std::max(lo, std::min(v, hi));
> -}
> -
>  using clock = std::chrono::steady_clock;
>  using duration = std::chrono::steady_clock::duration;
>  using time_point = std::chrono::steady_clock::time_point;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 4bb1627342fd..3a1c50c4add0 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -24,7 +24,6 @@
>  #include <libipa/ipa_interface_wrapper.h>
>  
>  #include "libcamera/internal/log.h"
> -#include "libcamera/internal/utils.h"
>  
>  namespace libcamera {
>  
> @@ -234,13 +233,13 @@ void IPARkISP1::updateStatistics(unsigned int frame,
>  			double exposure;
>  
>  			exposure = factor * exposure_ * gain_ / minGain_;
> -			exposure_ = utils::clamp<uint64_t>((uint64_t)exposure,
> -							   minExposure_,
> -							   maxExposure_);
> +			exposure_ = std::clamp<uint64_t>((uint64_t)exposure,
> +							 minExposure_,
> +							 maxExposure_);
>  
>  			exposure = exposure / exposure_ * minGain_;
> -			gain_ = utils::clamp<uint64_t>((uint64_t)exposure,
> -						       minGain_, maxGain_);
> +			gain_ = std::clamp<uint64_t>((uint64_t)exposure,
> +						     minGain_, maxGain_);
>  
>  			setControls(frame + 1);
>  		}
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index eb829e096561..a4d74a62f69a 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -7,6 +7,7 @@
>  
>  #include "imgu.h"
>  
> +#include <algorithm>
>  #include <cmath>
>  #include <limits>
>  
> @@ -129,7 +130,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
>  	if (!isSameRatio(pipe->input, gdc)) {
>  		float estIFHeight = (iif.width * gdc.height) /
>  				    static_cast<float>(gdc.width);
> -		estIFHeight = utils::clamp<float>(estIFHeight, minIFHeight, iif.height);
> +		estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height);
>  		bool found = false;
>  
>  		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 019e50b8f444..2d881fe28f98 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -240,15 +240,15 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  			unsigned int limit;
>  			limit = utils::alignDown(cio2Configuration_.size.width - 1,
>  						 IMGU_OUTPUT_WIDTH_MARGIN);
> -			cfg->size.width = utils::clamp(cfg->size.width,
> -						       IMGU_OUTPUT_MIN_SIZE.width,
> -						       limit);
> +			cfg->size.width = std::clamp(cfg->size.width,
> +						     IMGU_OUTPUT_MIN_SIZE.width,
> +						     limit);
>  
>  			limit = utils::alignDown(cio2Configuration_.size.height - 1,
>  						 IMGU_OUTPUT_HEIGHT_MARGIN);
> -			cfg->size.height = utils::clamp(cfg->size.height,
> -							IMGU_OUTPUT_MIN_SIZE.height,
> -							limit);
> +			cfg->size.height = std::clamp(cfg->size.height,
> +						      IMGU_OUTPUT_MIN_SIZE.height,
> +						      limit);
>  
>  			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
>  					      IMGU_OUTPUT_HEIGHT_ALIGN);
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index cf244f11f242..7e237650b448 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -360,7 +360,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>  		}
>  
>  		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> -		controls.set(cid, utils::clamp(value, 0, 255));
> +		controls.set(cid, std::clamp(value, 0, 255));
>  	}
>  
>  	for (const auto &ctrl : controls)
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 615df46ac142..a5232902191e 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -146,14 +146,6 @@ std::string dirname(const std::string &path)
>   * \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
> - */
> -
>  /**
>   * \typedef clock
>   * \brief The libcamera clock (monotonic)
>
Niklas Söderlund Aug. 24, 2020, 9:11 p.m. UTC | #2 
Hi Laurent,

Thanks for your work.

On 2020-08-22 23:00:34 +0300, Laurent Pinchart wrote:
> Now that libcamera uses C++17, the C++ standard library provides
> std::clamp(). Drop our custom utils::clamp() function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/internal/utils.h   |  7 -------
>  src/ipa/rkisp1/rkisp1.cpp            | 11 +++++------
>  src/libcamera/pipeline/ipu3/imgu.cpp |  3 ++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++------
>  src/libcamera/pipeline/vimc/vimc.cpp |  2 +-
>  src/libcamera/utils.cpp              |  8 --------
>  6 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
> index 45cd6f120c51..a1b644b0345b 100644
> --- a/include/libcamera/internal/utils.h
> +++ b/include/libcamera/internal/utils.h
> @@ -65,13 +65,6 @@ unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
>  	return count;
>  }
>  
> -/* C++11 doesn't provide std::clamp */
> -template <typename T>
> -const T& clamp(const T& v, const T& lo, const T& hi)
> -{
> -	return std::max(lo, std::min(v, hi));
> -}
> -
>  using clock = std::chrono::steady_clock;
>  using duration = std::chrono::steady_clock::duration;
>  using time_point = std::chrono::steady_clock::time_point;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 4bb1627342fd..3a1c50c4add0 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -24,7 +24,6 @@
>  #include <libipa/ipa_interface_wrapper.h>
>  
>  #include "libcamera/internal/log.h"
> -#include "libcamera/internal/utils.h"
>  
>  namespace libcamera {
>  
> @@ -234,13 +233,13 @@ void IPARkISP1::updateStatistics(unsigned int frame,
>  			double exposure;
>  
>  			exposure = factor * exposure_ * gain_ / minGain_;
> -			exposure_ = utils::clamp<uint64_t>((uint64_t)exposure,
> -							   minExposure_,
> -							   maxExposure_);
> +			exposure_ = std::clamp<uint64_t>((uint64_t)exposure,
> +							 minExposure_,
> +							 maxExposure_);
>  
>  			exposure = exposure / exposure_ * minGain_;
> -			gain_ = utils::clamp<uint64_t>((uint64_t)exposure,
> -						       minGain_, maxGain_);
> +			gain_ = std::clamp<uint64_t>((uint64_t)exposure,
> +						     minGain_, maxGain_);
>  
>  			setControls(frame + 1);
>  		}
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index eb829e096561..a4d74a62f69a 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -7,6 +7,7 @@
>  
>  #include "imgu.h"
>  
> +#include <algorithm>
>  #include <cmath>
>  #include <limits>
>  
> @@ -129,7 +130,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
>  	if (!isSameRatio(pipe->input, gdc)) {
>  		float estIFHeight = (iif.width * gdc.height) /
>  				    static_cast<float>(gdc.width);
> -		estIFHeight = utils::clamp<float>(estIFHeight, minIFHeight, iif.height);
> +		estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height);
>  		bool found = false;
>  
>  		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 019e50b8f444..2d881fe28f98 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -240,15 +240,15 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  			unsigned int limit;
>  			limit = utils::alignDown(cio2Configuration_.size.width - 1,
>  						 IMGU_OUTPUT_WIDTH_MARGIN);
> -			cfg->size.width = utils::clamp(cfg->size.width,
> -						       IMGU_OUTPUT_MIN_SIZE.width,
> -						       limit);
> +			cfg->size.width = std::clamp(cfg->size.width,
> +						     IMGU_OUTPUT_MIN_SIZE.width,
> +						     limit);
>  
>  			limit = utils::alignDown(cio2Configuration_.size.height - 1,
>  						 IMGU_OUTPUT_HEIGHT_MARGIN);
> -			cfg->size.height = utils::clamp(cfg->size.height,
> -							IMGU_OUTPUT_MIN_SIZE.height,
> -							limit);
> +			cfg->size.height = std::clamp(cfg->size.height,
> +						      IMGU_OUTPUT_MIN_SIZE.height,
> +						      limit);
>  
>  			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
>  					      IMGU_OUTPUT_HEIGHT_ALIGN);
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index cf244f11f242..7e237650b448 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -360,7 +360,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>  		}
>  
>  		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> -		controls.set(cid, utils::clamp(value, 0, 255));
> +		controls.set(cid, std::clamp(value, 0, 255));
>  	}
>  
>  	for (const auto &ctrl : controls)
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 615df46ac142..a5232902191e 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -146,14 +146,6 @@ std::string dirname(const std::string &path)
>   * \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
> - */
> -
>  /**
>   * \typedef clock
>   * \brief The libcamera clock (monotonic)
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
index 45cd6f120c51..a1b644b0345b 100644
--- a/include/libcamera/internal/utils.h
+++ b/include/libcamera/internal/utils.h
@@ -65,13 +65,6 @@  unsigned int set_overlap(InputIt1 first1, InputIt1 last1,
 	return count;
 }
 
-/* C++11 doesn't provide std::clamp */
-template <typename T>
-const T& clamp(const T& v, const T& lo, const T& hi)
-{
-	return std::max(lo, std::min(v, hi));
-}
-
 using clock = std::chrono::steady_clock;
 using duration = std::chrono::steady_clock::duration;
 using time_point = std::chrono::steady_clock::time_point;
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 4bb1627342fd..3a1c50c4add0 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -24,7 +24,6 @@ 
 #include <libipa/ipa_interface_wrapper.h>
 
 #include "libcamera/internal/log.h"
-#include "libcamera/internal/utils.h"
 
 namespace libcamera {
 
@@ -234,13 +233,13 @@  void IPARkISP1::updateStatistics(unsigned int frame,
 			double exposure;
 
 			exposure = factor * exposure_ * gain_ / minGain_;
-			exposure_ = utils::clamp<uint64_t>((uint64_t)exposure,
-							   minExposure_,
-							   maxExposure_);
+			exposure_ = std::clamp<uint64_t>((uint64_t)exposure,
+							 minExposure_,
+							 maxExposure_);
 
 			exposure = exposure / exposure_ * minGain_;
-			gain_ = utils::clamp<uint64_t>((uint64_t)exposure,
-						       minGain_, maxGain_);
+			gain_ = std::clamp<uint64_t>((uint64_t)exposure,
+						     minGain_, maxGain_);
 
 			setControls(frame + 1);
 		}
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index eb829e096561..a4d74a62f69a 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -7,6 +7,7 @@ 
 
 #include "imgu.h"
 
+#include <algorithm>
 #include <cmath>
 #include <limits>
 
@@ -129,7 +130,7 @@  void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
 	if (!isSameRatio(pipe->input, gdc)) {
 		float estIFHeight = (iif.width * gdc.height) /
 				    static_cast<float>(gdc.width);
-		estIFHeight = utils::clamp<float>(estIFHeight, minIFHeight, iif.height);
+		estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height);
 		bool found = false;
 
 		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 019e50b8f444..2d881fe28f98 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -240,15 +240,15 @@  CameraConfiguration::Status IPU3CameraConfiguration::validate()
 			unsigned int limit;
 			limit = utils::alignDown(cio2Configuration_.size.width - 1,
 						 IMGU_OUTPUT_WIDTH_MARGIN);
-			cfg->size.width = utils::clamp(cfg->size.width,
-						       IMGU_OUTPUT_MIN_SIZE.width,
-						       limit);
+			cfg->size.width = std::clamp(cfg->size.width,
+						     IMGU_OUTPUT_MIN_SIZE.width,
+						     limit);
 
 			limit = utils::alignDown(cio2Configuration_.size.height - 1,
 						 IMGU_OUTPUT_HEIGHT_MARGIN);
-			cfg->size.height = utils::clamp(cfg->size.height,
-							IMGU_OUTPUT_MIN_SIZE.height,
-							limit);
+			cfg->size.height = std::clamp(cfg->size.height,
+						      IMGU_OUTPUT_MIN_SIZE.height,
+						      limit);
 
 			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
 					      IMGU_OUTPUT_HEIGHT_ALIGN);
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index cf244f11f242..7e237650b448 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -360,7 +360,7 @@  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 		}
 
 		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
-		controls.set(cid, utils::clamp(value, 0, 255));
+		controls.set(cid, std::clamp(value, 0, 255));
 	}
 
 	for (const auto &ctrl : controls)
diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index 615df46ac142..a5232902191e 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -146,14 +146,6 @@  std::string dirname(const std::string &path)
  * \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
- */
-
 /**
  * \typedef clock
  * \brief The libcamera clock (monotonic)