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

[Dixit, Vishwanath]

On 4/27/18 5:15 AM, Michael Niedermayer wrote:
> 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
Yes, you are right. But, I can use only the LS 32bits of the result in the following code. It is a limitation of 64-bit NTP time format and the issue is going to occur in 2036. Not sure how to handle it here. At most, I can print a warning message when the result gets truncated. Could you please suggest any alternative option?
>
>
>>>
>>>
>>>> +    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.
Okay, let's keep the code, otherwise I will not be able to get the expected functionality using av_rescale function.
>
> [...]
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel