[libcamera-devel,v4,02/19] libcamera: utils: Add alignUp and alignDown functions
diff mbox series

Message ID 20200720104736.19986-3-jacopo@jmondi.org
State Accepted
Headers show
Series
  • ipu3: rework pipe confiuguration
Related show

Commit Message

Jacopo Mondi July 20, 2020, 10:47 a.m. UTC 
Add to libcamera utils library two functions to round up or down a
value to an alignment and add a test in test/utils.cpp for the two
new functions.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/utils.h | 10 ++++++++++
 src/libcamera/utils.cpp            | 16 ++++++++++++++++
 test/utils.cpp                     | 11 +++++++++++
 3 files changed, 37 insertions(+)

Comments

Niklas Söderlund July 21, 2020, 11:46 a.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2020-07-20 12:47:19 +0200, Jacopo Mondi wrote:
> Add to libcamera utils library two functions to round up or down a
> value to an alignment and add a test in test/utils.cpp for the two
> new functions.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  include/libcamera/internal/utils.h | 10 ++++++++++
>  src/libcamera/utils.cpp            | 16 ++++++++++++++++
>  test/utils.cpp                     | 11 +++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
> index 8d026cc6c0fe..45cd6f120c51 100644
> --- a/include/libcamera/internal/utils.h
> +++ b/include/libcamera/internal/utils.h
> @@ -200,6 +200,16 @@ details::StringSplitter split(const std::string &str, const std::string &delim);
>  std::string libcameraBuildPath();
>  std::string libcameraSourcePath();
>  
> +constexpr unsigned int alignDown(unsigned int value, unsigned int alignment)
> +{
> +	return value / alignment * alignment;
> +}
> +
> +constexpr unsigned int alignUp(unsigned int value, unsigned int alignment)
> +{
> +	return (value + alignment - 1) / alignment * alignment;
> +}
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 0567328fe31b..615df46ac142 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -444,6 +444,22 @@ std::string libcameraSourcePath()
>  	return path + "/";
>  }
>  
> +/**
> + * \fn alignDown(unsigned int value, unsigned int alignment)
> + * \brief Align \a value down to \a alignment
> + * \param[in] value The value to align
> + * \param[in] alignment The alignment
> + * \return The value rounded down to the nearest multiple of \a alignment
> + */
> +
> +/**
> + * \fn alignUp(unsigned int value, unsigned int alignment)
> + * \brief Align \a value up to \a alignment
> + * \param[in] value The value to align
> + * \param[in] alignment The alignment
> + * \return The value rounded up to the nearest multiple of \a alignment
> + */
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/test/utils.cpp b/test/utils.cpp
> index f482e6a1d829..08f293898fd9 100644
> --- a/test/utils.cpp
> +++ b/test/utils.cpp
> @@ -166,6 +166,17 @@ protected:
>  			return TestFail;
>  		}
>  
> +		/* utils::alignUp() and utils::alignDown() tests. */
> +		if (utils::alignDown(6, 3) != 6 || utils::alignDown(7, 3) != 6) {
> +			cerr << "utils::alignDown test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (utils::alignUp(6, 3) != 6 || utils::alignUp(7, 3) != 9) {
> +			cerr << "utils::alignUp test failed" << endl;
> +			return TestFail;
> +		}
> +
>  		return TestPass;
>  	}
>  };
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham July 21, 2020, 11:59 a.m. UTC | #2 
Hi Jacopo,

On 20/07/2020 11:47, Jacopo Mondi wrote:
> Add to libcamera utils library two functions to round up or down a
> value to an alignment and add a test in test/utils.cpp for the two
> new functions.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Given that these introduce the two constexpr variants, I wonder if
 {constexpr , }Size::align{up,down}To()

would benefit from using these helpers to reduce duplication of the
calculation. I expect the compiler to appropriately inline the
constexpr's anyway...

I think Laurent's previous response was that these are small and can
just be separate, but that feels very un-dry now with 3 repeating
implementations of each of the up/down variants....

Still, my point is, I think I do see value in this being in utils ;-)

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

> ---
>  include/libcamera/internal/utils.h | 10 ++++++++++
>  src/libcamera/utils.cpp            | 16 ++++++++++++++++
>  test/utils.cpp                     | 11 +++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
> index 8d026cc6c0fe..45cd6f120c51 100644
> --- a/include/libcamera/internal/utils.h
> +++ b/include/libcamera/internal/utils.h
> @@ -200,6 +200,16 @@ details::StringSplitter split(const std::string &str, const std::string &delim);
>  std::string libcameraBuildPath();
>  std::string libcameraSourcePath();
>  
> +constexpr unsigned int alignDown(unsigned int value, unsigned int alignment)
> +{
> +	return value / alignment * alignment;
> +}
> +
> +constexpr unsigned int alignUp(unsigned int value, unsigned int alignment)
> +{
> +	return (value + alignment - 1) / alignment * alignment;
> +}
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 0567328fe31b..615df46ac142 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -444,6 +444,22 @@ std::string libcameraSourcePath()
>  	return path + "/";
>  }
>  
> +/**
> + * \fn alignDown(unsigned int value, unsigned int alignment)
> + * \brief Align \a value down to \a alignment
> + * \param[in] value The value to align
> + * \param[in] alignment The alignment
> + * \return The value rounded down to the nearest multiple of \a alignment
> + */
> +
> +/**
> + * \fn alignUp(unsigned int value, unsigned int alignment)
> + * \brief Align \a value up to \a alignment
> + * \param[in] value The value to align
> + * \param[in] alignment The alignment
> + * \return The value rounded up to the nearest multiple of \a alignment
> + */
> +
>  } /* namespace utils */
>  
>  } /* namespace libcamera */
> diff --git a/test/utils.cpp b/test/utils.cpp
> index f482e6a1d829..08f293898fd9 100644
> --- a/test/utils.cpp
> +++ b/test/utils.cpp
> @@ -166,6 +166,17 @@ protected:
>  			return TestFail;
>  		}
>  
> +		/* utils::alignUp() and utils::alignDown() tests. */
> +		if (utils::alignDown(6, 3) != 6 || utils::alignDown(7, 3) != 6) {
> +			cerr << "utils::alignDown test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		if (utils::alignUp(6, 3) != 6 || utils::alignUp(7, 3) != 9) {
> +			cerr << "utils::alignUp test failed" << endl;
> +			return TestFail;
> +		}
> +
>  		return TestPass;
>  	}
>  };
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h
index 8d026cc6c0fe..45cd6f120c51 100644
--- a/include/libcamera/internal/utils.h
+++ b/include/libcamera/internal/utils.h
@@ -200,6 +200,16 @@  details::StringSplitter split(const std::string &str, const std::string &delim);
 std::string libcameraBuildPath();
 std::string libcameraSourcePath();
 
+constexpr unsigned int alignDown(unsigned int value, unsigned int alignment)
+{
+	return value / alignment * alignment;
+}
+
+constexpr unsigned int alignUp(unsigned int value, unsigned int alignment)
+{
+	return (value + alignment - 1) / alignment * alignment;
+}
+
 } /* namespace utils */
 
 } /* namespace libcamera */
diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index 0567328fe31b..615df46ac142 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -444,6 +444,22 @@  std::string libcameraSourcePath()
 	return path + "/";
 }
 
+/**
+ * \fn alignDown(unsigned int value, unsigned int alignment)
+ * \brief Align \a value down to \a alignment
+ * \param[in] value The value to align
+ * \param[in] alignment The alignment
+ * \return The value rounded down to the nearest multiple of \a alignment
+ */
+
+/**
+ * \fn alignUp(unsigned int value, unsigned int alignment)
+ * \brief Align \a value up to \a alignment
+ * \param[in] value The value to align
+ * \param[in] alignment The alignment
+ * \return The value rounded up to the nearest multiple of \a alignment
+ */
+
 } /* namespace utils */
 
 } /* namespace libcamera */
diff --git a/test/utils.cpp b/test/utils.cpp
index f482e6a1d829..08f293898fd9 100644
--- a/test/utils.cpp
+++ b/test/utils.cpp
@@ -166,6 +166,17 @@  protected:
 			return TestFail;
 		}
 
+		/* utils::alignUp() and utils::alignDown() tests. */
+		if (utils::alignDown(6, 3) != 6 || utils::alignDown(7, 3) != 6) {
+			cerr << "utils::alignDown test failed" << endl;
+			return TestFail;
+		}
+
+		if (utils::alignUp(6, 3) != 6 || utils::alignUp(7, 3) != 9) {
+			cerr << "utils::alignUp test failed" << endl;
+			return TestFail;
+		}
+
 		return TestPass;
 	}
 };