Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way delay.

Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time + render delay.

Make capture time in local timebase available for decoded VP9 video frames (propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=

Committed: https://crrev.com/5249599a9b69ad9c2d513210d694719f1011f977
Cr-Commit-Position: refs/heads/master@{#11901}

[åsapersson, 2016-02-12 15:59:15]

Description was changed from

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_ time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

to

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_ time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

[åsapersson, 2016-02-12 15:59:15]

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

[åsapersson, 2016-02-12 16:00:13]

Description was changed from

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_ time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

to

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

[stefan-webrtc, 2016-02-22 16:18:21]

lg!

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:178: frame.ntp_time_ms());
Maybe we should make sure this is positive before logging since ntp_time_ms() is
an estimate based on rtt. Or could you verify that the estimation makes sure it
doesn't happen?

[åsapersson, 2016-03-01 12:01:43]

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:178: frame.ntp_time_ms());
On 2016/02/22 16:18:21, stefan-webrtc (holmer) wrote:
> Maybe we should make sure this is positive before logging since ntp_time_ms()
is
> an estimate based on rtt. Or could you verify that the estimation makes sure
it
> doesn't happen?

RemoteNtpTimeEstimator::Estimate returns -1 on error (e.g. if having received
less than two RTCP SR reports).

So ntp_time_ms should be -1 if there is no estimate (or 0 if not set).

So maybe checking if frame.ntp_time_ms() > 0 should be enough?

[stefan-webrtc, 2016-03-01 12:38:21]

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:178: frame.ntp_time_ms());
On 2016/03/01 12:01:43, åsapersson wrote:
> On 2016/02/22 16:18:21, stefan-webrtc (holmer) wrote:
> > Maybe we should make sure this is positive before logging since
ntp_time_ms()
> is
> > an estimate based on rtt. Or could you verify that the estimation makes sure
> it
> > doesn't happen?
> 
> RemoteNtpTimeEstimator::Estimate returns -1 on error (e.g. if having received
> less than two RTCP SR reports).
> 
> So ntp_time_ms should be -1 if there is no estimate (or 0 if not set).
> 
> So maybe checking if frame.ntp_time_ms() > 0 should be enough?

I'm not sure that's enough as there are no guarantees that frame.ntp_time_ms() <
clock_->CurrentNtpInMilliseconds():

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...

Maybe we can make sure it is in that file though? I.e., add a line:
receiver_capture_ntp_ms = std::max(clock_->CurrentNtpInMilliseconds(),
receiver_capture_ntp_ms);

[åsapersson, 2016-03-01 13:46:42]

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
File webrtc/video/receive_statistics_proxy.cc (right):

https://codereview.webrtc.org/1688143003/diff/20001/webrtc/video/receive_stat...
webrtc/video/receive_statistics_proxy.cc:178: frame.ntp_time_ms());
On 2016/03/01 12:38:20, stefan-webrtc (holmer) wrote:
> On 2016/03/01 12:01:43, åsapersson wrote:
> > On 2016/02/22 16:18:21, stefan-webrtc (holmer) wrote:
> > > Maybe we should make sure this is positive before logging since
> ntp_time_ms()
> > is
> > > an estimate based on rtt. Or could you verify that the estimation makes
sure
> > it
> > > doesn't happen?
> > 
> > RemoteNtpTimeEstimator::Estimate returns -1 on error (e.g. if having
received
> > less than two RTCP SR reports).
> > 
> > So ntp_time_ms should be -1 if there is no estimate (or 0 if not set).
> > 
> > So maybe checking if frame.ntp_time_ms() > 0 should be enough?
> 
> I'm not sure that's enough as there are no guarantees that frame.ntp_time_ms()
<
> clock_->CurrentNtpInMilliseconds():
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...
> 
> Maybe we can make sure it is in that file though? I.e., add a line:
> receiver_capture_ntp_ms = std::max(clock_->CurrentNtpInMilliseconds(),
> receiver_capture_ntp_ms);

Ok. Added a check that the delay is positive before logging.

[stefan-webrtc, 2016-03-07 15:49:48]

lgtm

[åsapersson, 2016-03-08 08:14:00]

The CQ bit was checked by asapersson@webrtc.org

[commit-bot: I haz the power, 2016-03-08 08:14:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1688143003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1688143003/60001

[commit-bot: I haz the power, 2016-03-08 10:10:20]

Description was changed from

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

to

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

[commit-bot: I haz the power, 2016-03-08 10:10:21]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-08 10:10:32]

Description was changed from

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=
==========

to

==========
Update histogram "WebRTC.Video.OnewayDelayInMs" to use the estimated one-way
delay.
Previous logged delay was: network delay (rtt/2) + jitter delay + decode time +
render delay.

Make capture time in local timebase available for decoded VP9 video frames
(propagate ntp_time_ms from EncodedImage to decoded VideoFrame).

BUG=

Committed: https://crrev.com/5249599a9b69ad9c2d513210d694719f1011f977
Cr-Commit-Position: refs/heads/master@{#11901}
==========

[commit-bot: I haz the power, 2016-03-08 10:10:33]

Patchset 4 (id:??) landed as
https://crrev.com/5249599a9b69ad9c2d513210d694719f1011f977
Cr-Commit-Position: refs/heads/master@{#11901}

[åsapersson, 2016-04-15 10:06:34]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.webrtc.org/1893543003/ by asapersson@webrtc.org.

The reason for reverting is: The delay stats are high..