<div dir="ltr"><div dir="ltr">Hi Jacopo,</div><div dir="ltr"><br></div><div dir="ltr">Thank you for your review feedback.<br><div><br></div></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 24 May 2021 at 12:58, Jacopo Mondi <<a href="mailto:jacopo@jmondi.org">jacopo@jmondi.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Mon, May 24, 2021 at 09:48:19AM +0100, Naushir Patuck wrote:<br>
> A new utils::Duration class is defined to represent a<br>
> std::chrono::duration type with double precision nanosecond timebase.<br>
> Using a double minimises the loss of precision when converting timebases.<br>
> This helper class may be used by IPAs to represent variables such as frame<br>
> durations and exposure times.<br>
><br>
> An operator << overload is define to help with displaying<br>
> utils::Duration value in stream objects. Currently, this will display<br>
> the duration value in microseconds.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> ---<br>
> include/libcamera/internal/utils.h | 42 ++++++++++++++++++++++++++++++<br>
> src/libcamera/utils.cpp | 41 +++++++++++++++++++++++++++++<br>
> 2 files changed, 83 insertions(+)<br>
><br>
> diff --git a/include/libcamera/internal/utils.h b/include/libcamera/internal/utils.h<br>
> index 83dada7cc16c..f536f2267d70 100644<br>
> --- a/include/libcamera/internal/utils.h<br>
> +++ b/include/libcamera/internal/utils.h<br>
> @@ -316,8 +316,50 @@ auto enumerate(T (&iterable)[N]) -> details::enumerate_adapter<T *><br>
> }<br>
> #endif<br>
><br>
> +class Duration : public std::chrono::duration<double, std::nano><br>
> +{<br>
> + using BaseDuration = std::chrono::duration<double, std::nano>;<br>
> +<br>
> +public:<br>
> + Duration() = default;<br>
> +<br>
> + template<typename Rep, typename Period><br>
> + constexpr Duration(const std::chrono::duration<Rep, Period> &d)<br>
> + : BaseDuration(d)<br>
> + {<br>
> + }<br>
> +<br>
> + template<typename Period><br>
> + double get() const<br>
> + {<br>
> + auto const c = std::chrono::duration_cast<std::chrono::duration<double, Period>>(*this);<br>
> + return c.count();<br>
> + }<br>
> +<br>
> + explicit constexpr operator bool() const<br>
> + {<br>
> + return *this != BaseDuration::zero();<br>
> + }<br>
> +};<br>
> +<br>
> } /* namespace utils */<br>
><br>
> +#ifndef __DOXYGEN__<br>
> +template<class CharT, class Traits><br>
> +static inline std::basic_ostream<CharT, Traits> &operator<<(std::basic_ostream<CharT, Traits> &os,<br>
> + const utils::Duration &d)<br>
> +{<br>
> + std::basic_ostringstream<CharT, Traits> s;<br>
> +<br>
> + s.flags(os.flags());<br>
> + s.imbue(os.getloc());<br>
> + s.setf(std::ios_base::fixed, std::ios_base::floatfield);<br>
> + s.precision(2);<br>
> + s << d.get<std::micro>() << "us";<br>
> + return os << s.str();<br>
> +}<br>
> +#endif<br>
> +<br>
> } /* namespace libcamera */<br>
><br>
> #endif /* __LIBCAMERA_INTERNAL_UTILS_H__ */<br>
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp<br>
> index 826144d3c837..480284c6c2c9 100644<br>
> --- a/src/libcamera/utils.cpp<br>
> +++ b/src/libcamera/utils.cpp<br>
> @@ -506,6 +506,47 @@ std::string libcameraSourcePath()<br>
> * loop, iterates over an indexed view of the \a iterable<br>
> */<br>
><br>
> +/**<br>
> + * \class Duration<br>
> + * \brief Helper class for std::chrono::duration types<br>
<br>
Helper class derived from std::chrono::duration that represents a time<br>
duration in nanoseconds with double precision<br></blockquote><div><br></div><div>Ack.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + */<br>
> +<br>
> +/**<br>
> + * \typedef Duration::BaseDuration<br>
> + * \brief Base struct for the \a Duration helper class<br>
> + */<br>
<br>
Isn't this private ? does it need documentation ? It's just a<br>
syntactic sugar used in the class definition, isn't it ?<br></blockquote><div><br></div><div>It is private/syntactic sugar - I was not sure what the rule was for</div><div>private members with regards to documentation. I can remove</div><div>it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
> +<br>
> +/**<br>
> + * \fn Duration::Duration(const std::chrono::duration<Rep, Period> &d)<br>
> + * \brief Copy constructor from a \a std::chrono::duration type<br>
> + * \param[in] d The std::chrono::duration object to convert from<br>
> + *<br>
> + * Constructs a \a Duration object from a \a std::chrono::duration object,<br>
> + * internally converting the representation to \a Duration::BaseDuration type<br>
> + */<br>
> +<br>
> +/**<br>
> + * \fn Duration::get<Period>()<br>
> + * \brief Retrieve the tick count, converted to the timebase provided by the<br>
> + * template argument Period of type \a std::ratio<br>
> + *<br>
> + * A typical usage example is given below:<br>
> + *<br>
> + * \code{.cpp}<br>
> + * utils::Duration d = 5s;<br>
> + * double d_in_ms = d.get<std::milli>();<br>
> + * \endcode<br>
> + *<br>
> + * \return The tick count of the Duration expressed in \a Period<br>
> + */<br>
> +<br>
> +/**<br>
> + * \fn Duration::operator bool()<br>
> + * \brief Boolean operator to test if a \a Duration holds a non-zero time value.<br>
> + *<br>
> + * \return True if \a Duration is a non-zero time value, False otherwise.<br>
> + */<br>
<br>
All minor, which if not other comments, can be changed with your ack<br>
when applying<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br></blockquote><div><br></div><div>Thank's for the tag. I will submit a v5 revision with the updates once you and</div><div>others have a chance to comment on all the other patches in the series.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Thanks<br>
j<br>
<br>
> +<br>
> } /* namespace utils */<br>
><br>
> } /* namespace libcamera */<br>
> --<br>
> 2.25.1<br>
><br>
</blockquote></div></div>