[FFmpeg-devel] [PATCH 1/3] avformat/utils: function to get the formatted ntp time

[Michael Niedermayer]

On Thu, Apr 26, 2018 at 11:05:59AM +0000, Dixit, Vishwanath wrote:
> 
> 
> On 4/26/18 1:04 AM, Michael Niedermayer wrote:
> > On Tue, Apr 24, 2018 at 02:46:56PM +0530, vdixit at akamai.com wrote:
> >> From: Vishwanath Dixit <vdixit at akamai.com>
> >>
> >> This utility function creates 64-bit NTP time format as per the RFC
> >> 5905.
> >> A simple explaination of 64-bit NTP time format is here
> >> http://www.beaglesoft.com/Manual/page53.htm
> >> ---
> >>  libavformat/internal.h |  8 ++++++++
> >>  libavformat/utils.c    | 20 ++++++++++++++++++++
> >>  2 files changed, 28 insertions(+)
> >>
> >> diff --git a/libavformat/internal.h b/libavformat/internal.h
> >> index 3582682..e9f758f 100644
> >> --- a/libavformat/internal.h
> >> +++ b/libavformat/internal.h
> >> @@ -240,6 +240,14 @@ void ff_read_frame_flush(AVFormatContext *s);
> >>  uint64_t ff_ntp_time(void);
> >>  
> >>  /**
> >> + * Get the NTP time stamp formatted as per the RFC-5905.
> >> + *
> >> + * @param ntp_time NTP time in micro seconds (since NTP epoch)
> >> + * @return the formatted NTP time stamp
> >> + */
> >> +uint64_t ff_time_ntp_format(uint64_t ntp_time);
> >> +
> >> +/**
> >>   * Append the media-specific SDP fragment for the media stream c
> >>   * to the buffer buff.
> >>   *
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index c25eab4..b59d426 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> >> @@ -4637,6 +4637,26 @@ uint64_t ff_ntp_time(void)
> >>      return (av_gettime() / 1000) * 1000 + NTP_OFFSET_US;
> >>  }
> >>  
> >> +uint64_t ff_time_ntp_format(uint64_t ntp_time)
> >> +{
> >> +    uint64_t ntp_ts, frac_part;
> >> +    uint32_t sec, usec;
> >> +
> >> +    //current ntp time in seconds and micro seconds
> >
> >> +    sec = ntp_time / 1000000;
> >
> > This can be truncated
> Yes, but the truncated part is not getting discarded, as the following line is taking care of that. Please correct me if I have not understood what you have intended to say here.

ok, correcting you then :)
sec is 32bit
not all values of "ntp_time / 1000000;" fit in 32bit
there is no check for this it just produces a wrong result


> >
> >
> >> +    usec = ntp_time % 1000000;
> >> +
> >> +    //encoding in ntp timestamp format
> >> +    frac_part = usec * 0xFFFFFFFFULL;
> >> +    frac_part /= 1000000;
> >> +
> >> +    ntp_ts = (uint64_t) sec;
> >> +    ntp_ts <<= 32;
> >> +    ntp_ts |= frac_part;
> >> +
> >> +    return ntp_ts;
> >
> > this looks similar to what av_rescale does.
> Not really. This is a unique format as defined in RFC 5905. (please see this page for high level understanding of this format http://www.beaglesoft.com/Manual/page53.htm )

it still looks very similar to me. But lets keep your code. if you prefer.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180427/cd90cab2/attachment.sig>