Introduce helper class NtpTime

Introducing helper class NtpTime
Seconds and fractions parts of the ntp time presented with two values, but used as one.
This helper structure can make that use more clear.

[danilchap, 2015-11-11 11:04:00]

Description was changed from

==========
Introduce helper class NtpTime
seconds and fractions part of the ntp time are used together.
This helper structure should make that use more clear.
==========

to

==========
Introducing helper class NtpTime
Seconds and fractions parts of the ntp time presented with two values, but used
as one.
This helper structure can make that use more clear.
==========

[danilchap, 2015-11-11 11:04:00]

danilchap@webrtc.org changed reviewers:
+ asapersson@webrtc.org, sprang@webrtc.org

[sprang_webrtc, 2015-11-12 15:34:48]

Looks good. Would we have use of this outside of the rtp module?

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/ntp_time.h (right):

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:38: void clear() { seconds_ =
fractions_ = 0; }
Assign separately

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:41: uint32_t MidNtp() const { return
(seconds_ << 16) | (fractions_ >> 16); }
I think this one deserves a comment :)

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:44: explicit operator bool() const {
return seconds_ != 0 || fractions_ != 0; }
When would this be used? Are we implicitly saying time 0 is not a valid number?
I think testing for equality with NtpTime(0, 0) would be more clear in that
case, or the use of rtc::Otional for instance.

[danilchap, 2015-11-12 19:05:31]

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/ntp_time.h (right):

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:38: void clear() { seconds_ =
fractions_ = 0; }
On 2015/11/12 15:34:48, språng wrote:
> Assign separately

Done.

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:41: uint32_t MidNtp() const { return
(seconds_ << 16) | (fractions_ >> 16); }
On 2015/11/12 15:34:48, språng wrote:
> I think this one deserves a comment :)
(bad excuse)RTCPUtility::MidNtp I took this from don't have one :)

this one seems RTP specific (mentioned in rfc3611, section 4.5 dlrr report
block, LRR and DLRR). Used to calculate delays (so do not use full scale and
full precision of the ntp time)

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:44: explicit operator bool() const {
return seconds_ != 0 || fractions_ != 0; }
On 2015/11/12 15:34:48, språng wrote:
> When would this be used? Are we implicitly saying time 0 is not a valid
number?
> I think testing for equality with NtpTime(0, 0) would be more clear in that
> case, or the use of rtc::Otional for instance.

To make it more explicit conversion to bool replaced with function 'valid'.

This kind of test used in RtcpReciver to check if NtpTime was ever received. 
According to NTP standard value 0/0 considered invalid.
RFC1305, Data format section:  "There will exist an 200-picosecond interval,
henceforth ignored, every 136 years when the 64-bit field will be zero and thus
considered invalid."

[sprang_webrtc, 2015-11-16 09:55:25]

lgtm with nit

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/ntp_time.h (right):

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:41: uint32_t MidNtp() const { return
(seconds_ << 16) | (fractions_ >> 16); }
On 2015/11/12 19:05:31, danilchap wrote:
> On 2015/11/12 15:34:48, språng wrote:
> > I think this one deserves a comment :)
> (bad excuse)RTCPUtility::MidNtp I took this from don't have one :)
> 
> this one seems RTP specific (mentioned in rfc3611, section 4.5 dlrr report
> block, LRR and DLRR). Used to calculate delays (so do not use full scale and
> full precision of the ntp time)

Ah, so great that we get one now!

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:44: explicit operator bool() const {
return seconds_ != 0 || fractions_ != 0; }
On 2015/11/12 19:05:31, danilchap wrote:
> On 2015/11/12 15:34:48, språng wrote:
> > When would this be used? Are we implicitly saying time 0 is not a valid
> number?
> > I think testing for equality with NtpTime(0, 0) would be more clear in that
> > case, or the use of rtc::Otional for instance.
> 
> To make it more explicit conversion to bool replaced with function 'valid'.
> 
> This kind of test used in RtcpReciver to check if NtpTime was ever received. 
> According to NTP standard value 0/0 considered invalid.
> RFC1305, Data format section:  "There will exist an 200-picosecond interval,
> henceforth ignored, every 136 years when the 64-bit field will be zero and
thus
> considered invalid."

Gotcha. A comment to that affect would be great. :)

[danilchap, 2015-11-16 11:01:33]

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/ntp_time.h (right):

https://codereview.webrtc.org/1435833003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/ntp_time.h:44: explicit operator bool() const {
return seconds_ != 0 || fractions_ != 0; }
On 2015/11/16 09:55:25, språng wrote:
> On 2015/11/12 19:05:31, danilchap wrote:
> > On 2015/11/12 15:34:48, språng wrote:
> > > When would this be used? Are we implicitly saying time 0 is not a valid
> > number?
> > > I think testing for equality with NtpTime(0, 0) would be more clear in
that
> > > case, or the use of rtc::Otional for instance.
> > 
> > To make it more explicit conversion to bool replaced with function 'valid'.
> > 
> > This kind of test used in RtcpReciver to check if NtpTime was ever received.

> > According to NTP standard value 0/0 considered invalid.
> > RFC1305, Data format section:  "There will exist an 200-picosecond interval,
> > henceforth ignored, every 136 years when the 64-bit field will be zero and
> thus
> > considered invalid."
> 
> Gotcha. A comment to that affect would be great. :)

Done.