| Message ID | 20200714104212.48683-4-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Jul 14, 2020 at 12:41:55PM +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. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/internal/utils.h | 3 +++ > src/libcamera/utils.cpp | 22 ++++++++++++++++++++++ > test/utils.cpp | 18 ++++++++++++++++++ > 3 files changed, 43 insertions(+) > > diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h > index 8d026cc6c0fe..25eb24ec2d16 100644 > --- a/include/libcamera/internal/utils.h > +++ b/include/libcamera/internal/utils.h > @@ -200,6 +200,9 @@ details::StringSplitter split(const std::string &str, const std::string &delim); > std::string libcameraBuildPath(); > std::string libcameraSourcePath(); > > +unsigned int alignUp(unsigned int value, unsigned int alignment); > +unsigned int alignDown(unsigned int value, unsigned int alignment); I would inline those two functions, to let the compiler optimize the code. For instance it's common to have a constexpr alignment value, which can often translate to more efficient code. I would also mark the functions as constexpr. As constexpr implies inline, constexpr unsigned int alignUp(unsigned int value, unsigned int alignment) { return (value + alignment - 1) / alignment * alignment; } constexpr unsigned int alignDown(unsigned int value, unsigned int alignment) { return value / alignment * alignment; } > + > } /* namespace utils */ > > } /* namespace libcamera */ > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index 0567328fe31b..52fb0bb171d8 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -444,6 +444,28 @@ std::string libcameraSourcePath() > return path + "/"; > } > > +/** > + * \brief Align \a value to \a alignment Maybe s/value to/value up to/ ? Same for alignDown(). > + * \param[in] value The value to align > + * \param[in] alignment The alignment > + * \return The value rounded up to the nearest multiple of \a alignment > + */ > +unsigned int alignUp(unsigned int value, unsigned int alignment) > +{ > + return (value + alignment - 1) / alignment * alignment; > +} > + > +/** > + * \brief Align \a value 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 > + */ > +unsigned int alignDown(unsigned int value, unsigned int alignment) > +{ > + return value / alignment * alignment; > +} > + > } /* namespace utils */ > > } /* namespace libcamera */ > diff --git a/test/utils.cpp b/test/utils.cpp > index f482e6a1d829..02054f46b00e 100644 > --- a/test/utils.cpp > +++ b/test/utils.cpp > @@ -7,6 +7,7 @@ > > #include <iostream> > #include <map> > +#include <random> > #include <sstream> > #include <string> > #include <vector> > @@ -166,6 +167,23 @@ protected: > return TestFail; > } > > + /* utils::alignUp() and utils::alignDown() tests. */ > + random_device random; > + unsigned int val = random(); Usage of random number in tests makes it difficult to reproduce failures. Additionally, the random alignment, could be 0, leading to a potential division by 0. I think you can just hardcode values : if (utils::alignUp(6, 3) != 6 || utils::alignUp(7, 3) != 9) { cerr << "utils::alignUp test failed" << endl; return TestFail; } if (utils::alignDown(6, 3) != 6 || utils::alignDown(7, 3) != 6) { cerr << "utils::alignDown test failed" << endl; return TestFail; } Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + unsigned int align = random() % (val / 10); > + unsigned int roundUp = (val + align - 1) / align; > + unsigned int roundDown = val / align; > + > + if (utils::alignUp(val, align) != align * roundUp) { > + cerr << "utils::alignUp test failed" << endl; > + return TestFail; > + } > + > + if (utils::alignDown(val, align) != align * roundDown) { > + cerr << "utils::alignDown test failed" << endl; > + return TestFail; > + } > + > return TestPass; > } > };
diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h index 8d026cc6c0fe..25eb24ec2d16 100644 --- a/include/libcamera/internal/utils.h +++ b/include/libcamera/internal/utils.h @@ -200,6 +200,9 @@ details::StringSplitter split(const std::string &str, const std::string &delim); std::string libcameraBuildPath(); std::string libcameraSourcePath(); +unsigned int alignUp(unsigned int value, unsigned int alignment); +unsigned int alignDown(unsigned int value, unsigned int alignment); + } /* namespace utils */ } /* namespace libcamera */ diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp index 0567328fe31b..52fb0bb171d8 100644 --- a/src/libcamera/utils.cpp +++ b/src/libcamera/utils.cpp @@ -444,6 +444,28 @@ std::string libcameraSourcePath() return path + "/"; } +/** + * \brief Align \a value 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 + */ +unsigned int alignUp(unsigned int value, unsigned int alignment) +{ + return (value + alignment - 1) / alignment * alignment; +} + +/** + * \brief Align \a value 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 + */ +unsigned int alignDown(unsigned int value, unsigned int alignment) +{ + return value / alignment * alignment; +} + } /* namespace utils */ } /* namespace libcamera */ diff --git a/test/utils.cpp b/test/utils.cpp index f482e6a1d829..02054f46b00e 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -7,6 +7,7 @@ #include <iostream> #include <map> +#include <random> #include <sstream> #include <string> #include <vector> @@ -166,6 +167,23 @@ protected: return TestFail; } + /* utils::alignUp() and utils::alignDown() tests. */ + random_device random; + unsigned int val = random(); + unsigned int align = random() % (val / 10); + unsigned int roundUp = (val + align - 1) / align; + unsigned int roundDown = val / align; + + if (utils::alignUp(val, align) != align * roundUp) { + cerr << "utils::alignUp test failed" << endl; + return TestFail; + } + + if (utils::alignDown(val, align) != align * roundDown) { + cerr << "utils::alignDown test failed" << endl; + return TestFail; + } + return TestPass; } };
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. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/internal/utils.h | 3 +++ src/libcamera/utils.cpp | 22 ++++++++++++++++++++++ test/utils.cpp | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+)