[libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper macros for std::chrono::duration

[Naushir Patuck]

Hi Laurent,

On Wed, 19 May 2021 at 15:24, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:
> > A new RPiController::Duration typedef is defined to represent a
> > std::chrono::duration type with double precision nanosecond timebase
> > that will be used throughout. This minimises the loss of precision when
> > converting timebases. A function for returning the duration count in any
> > timebase is also provided.
> >
> > An operator << overload is define to help with displaying
> > RPiController::Duration value in stream objects.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 src/ipa/raspberrypi/controller/duration.hpp
> >
> > diff --git a/src/ipa/raspberrypi/controller/duration.hpp
> b/src/ipa/raspberrypi/controller/duration.hpp
> > new file mode 100644
> > index 000000000000..98aa3d78f52f
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/controller/duration.hpp
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > + *
> > + * duration.hpp - Helper macros for std::chrono::duration handling.
> > + */
> > +#pragma once
> > +
> > +#include <chrono>
> > +#include <iomanip>
> > +#include <iostream>
> > +
> > +namespace RPiController {
> > +
> > +using Duration = std::chrono::duration<double, std::nano>;
> > +
> > +// Helper to convert and return the count of any std::chrono::duration
> type to
> > +// another timebase.  Use a double rep type to try and preserve
> precision.
> > +template <typename P, typename T>
> > +static constexpr double DurationValue(T const &d)
> > +{
> > +     return std::chrono::duration_cast<std::chrono::duration<double,
> P>>(d).count();
> > +};
> > +
> > +static inline std::ostream &operator<<(std::ostream &os, const Duration
> &duration)
> > +{
> > +        std::streamsize ss = os.precision();
> > +        os << std::fixed << std::setprecision(2) <<
> DurationValue<std::micro>(duration) << " us";
> > +        os << std::setprecision(ss) << std::defaultfloat;
>
> This will reset to std::defaultfloat, while the stream may already be in
> std::fixed.
>

Good point, I should save and reapply like I do with precision.


>
> > +        return os;
> > +}
>
> Would it be too much yak-shaving to ask for the implementation to be
> closer to
> https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and
> moved to utils.h ? It could be useful more widely in libcamera than just
> in the RPi IPA.
>

Yes, I could look into that.  Did you mean move the whole
implementation into utils.h, or
only the operator <<() bit?  Actually, I would like to make an impactful
change in the next
revision of the series, and replace Duration with a class derived from a
std::chrono::duration
object.  This is a bit neater in terms of encapsulation, and allows me to
implement an
operator bool() method that cleans up the code a bit more.    Functionally,
everything else
remains identical.  Did you want me to put that into utils.h?

Sorry David, your review for patch 1/4 may need redoing with :-(

Regards,
Naush


> > +
> > +} // namespace RPiController
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210519/04a240be/attachment-0001.htm>