Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 -> estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 -> estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867

Committed: https://crrev.com/3cc47ebd2d0efc6a48ddb5b142fa9ea9c1ae4435
Cr-Commit-Position: refs/heads/master@{#14454}

[åsapersson, 2016-09-20 13:23:12]

Description was changed from

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867
==========

to

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867
==========

[åsapersson, 2016-09-20 13:23:12]

asapersson@webrtc.org changed reviewers:
+ mflodman@webrtc.org, stefan@webrtc.org

[stefan-webrtc, 2016-09-28 09:18:05]

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/rtp_to_ntp.cc (right):

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/rtp_to_ntp.cc:113: if (rtcp_timestamp_new <
rtcp_timestamp_old) {
Remove {}

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/rtp_to_ntp.cc:114: return false;
Should we add a warning?

[åsapersson, 2016-09-29 07:25:16]

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/rtp_to_ntp.cc (right):

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/rtp_to_ntp.cc:113: if (rtcp_timestamp_new <
rtcp_timestamp_old) {
On 2016/09/28 09:18:05, stefan-webrtc (holmer) wrote:
> Remove {}

Done.

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/rtp_to_ntp.cc:114: return false;
On 2016/09/28 09:18:05, stefan-webrtc (holmer) wrote:
> Should we add a warning?

Maybe UpdateRtcpList should be updated to have this check and then add a log
there. Add this as a first CL?

[stefan-webrtc, 2016-09-30 08:40:46]

lgtm

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/rtp_to_ntp.cc (right):

https://codereview.webrtc.org/2354843003/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/rtp_to_ntp.cc:114: return false;
On 2016/09/29 07:25:16, åsapersson wrote:
> On 2016/09/28 09:18:05, stefan-webrtc (holmer) wrote:
> > Should we add a warning?
> 
> Maybe UpdateRtcpList should be updated to have this check and then add a log
> there. Add this as a first CL?

Ok

[åsapersson, 2016-09-30 08:59:23]

The CQ bit was checked by asapersson@webrtc.org

[commit-bot: I haz the power, 2016-09-30 08:59:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-30 10:16:21]

Description was changed from

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867
==========

to

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867
==========

[commit-bot: I haz the power, 2016-09-30 10:16:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-30 10:16:28]

Description was changed from

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867
==========

to

==========
Add sanity check for decreasing RTP timestamp in RtpToNtpMs.

The capture time for a frame (capture_ms) is set later (in
ViEEncoder::IncomingCapturedFrame) than the timestamp.
Could potentially cause the RTP timestamp in consecutive RTCP SR to decrease.
Example:
// Frame1 46371: timestamp:2732, capture_ms:46373, rtcp SR ms: 46423 ->
estimated current RTP timestamp:2732+(46423-46373)*90 = 7232
// Frame2 46404: timestamp:5702, capture_ms:46412, rtcp SR ms: 46428 ->
estimated current RTP timestamp:5702+(46428-46412)*90 = 7142
// Diff:  33 ms:          33 ms,            39 ms,              5 ms

BUG=b/31154867

Committed: https://crrev.com/3cc47ebd2d0efc6a48ddb5b142fa9ea9c1ae4435
Cr-Commit-Position: refs/heads/master@{#14454}
==========

[commit-bot: I haz the power, 2016-09-30 10:16:29]

Patchset 2 (id:??) landed as
https://crrev.com/3cc47ebd2d0efc6a48ddb5b142fa9ea9c1ae4435
Cr-Commit-Position: refs/heads/master@{#14454}