Add periodic logging of video stats.

Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats, VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

BUG=

Committed: https://crrev.com/2e5cfcd6c27861fb1839f12bb2268d01d6f5a85e
Cr-Commit-Position: refs/heads/master@{#13727}

[åsapersson, 2016-07-08 11:40:51]

Description was changed from

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 8 seconds).

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

to

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 8 seconds).

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

[åsapersson, 2016-07-08 11:40:52]

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

[pbos-webrtc, 2016-07-10 16:58:50]

pbos@webrtc.org changed reviewers:
+ pbos@webrtc.org

[pbos-webrtc, 2016-07-10 16:58:51]

drive-by comments

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/call/call.cc
File webrtc/call/call.cc (right):

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/call/call.cc#newco...
webrtc/call/call.cc:207: ss << "[send_bw_bps:" << send_bandwidth_bps << "],";
Pref. removing []s because they look like arrays of values, and not key-values.

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:115: ss << "[tot_bps:" << total_bitrate_bps
<< "],";
Remove []s unless this format is from somewhere. Also can we have total_bps or
do these strings come from somewhere?

Same with w: and h: etc, these would be more readable as decode_fps etc. instead
of being shortened.

[stefan-webrtc, 2016-08-01 11:14:09]

A few nits, nothing major.

lgtm

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1364: rtc::TimeDiff(now_ms,
last_stats_log_ms_) > kStatsLogIntervalMs) {
rtc::TimeDiff seems a bit pointless to me as all it does is to do:
return later - earlier;

I'd prefer:
now_ms - last_stats_log_ms_

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:2142: rtc::TimeDiff(now_ms,
last_stats_log_ms_) > kStatsLogIntervalMs) {
Same here

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:114: ss << "VideoReceiveStream stats:" <<
time_ms << ", {ssrc:" << ssrc << ", ";
I'd prefer having a space after each ":", same in the other ToString methods.

[åsapersson, 2016-08-01 15:30:57]

Patchset #5 (id:80001) has been deleted

[åsapersson, 2016-08-01 15:50:52]

Patchset #5 (id:100001) has been deleted

[åsapersson, 2016-08-01 15:55:22]

Also removed logging for rtx streams, ptal.

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/call/call.cc
File webrtc/call/call.cc (right):

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/call/call.cc#newco...
webrtc/call/call.cc:207: ss << "[send_bw_bps:" << send_bandwidth_bps << "],";
On 2016/07/10 16:58:50, pbos-webrtc wrote:
> Pref. removing []s because they look like arrays of values, and not
key-values.

Done.

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/2133073002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:115: ss << "[tot_bps:" << total_bitrate_bps
<< "],";
On 2016/07/10 16:58:51, pbos-webrtc wrote:
> Remove []s unless this format is from somewhere. Also can we have total_bps or
> do these strings come from somewhere?
> 
> Same with w: and h: etc, these would be more readable as decode_fps etc.
instead
> of being shortened.

Done.

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1364: rtc::TimeDiff(now_ms,
last_stats_log_ms_) > kStatsLogIntervalMs) {
On 2016/08/01 11:14:09, stefan-webrtc (holmer) wrote:
> rtc::TimeDiff seems a bit pointless to me as all it does is to do:
> return later - earlier;
> 
> I'd prefer:
> now_ms - last_stats_log_ms_

Done.

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:2142: rtc::TimeDiff(now_ms,
last_stats_log_ms_) > kStatsLogIntervalMs) {
On 2016/08/01 11:14:09, stefan-webrtc (holmer) wrote:
> Same here

Done.

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/2133073002/diff/60001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:114: ss << "VideoReceiveStream stats:" <<
time_ms << ", {ssrc:" << ssrc << ", ";
On 2016/08/01 11:14:09, stefan-webrtc (holmer) wrote:
> I'd prefer having a space after each ":", same in the other ToString methods.

Done.

[mflodman, 2016-08-04 13:37:09]

LG, minor comments.

https://codereview.webrtc.org/2133073002/diff/160001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2133073002/diff/160001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.cc:1352: FillSenderStats(info);
We could keep track of time just here and add a "bool log" (or similar) as input
argument to all Fill*Stats(). That would ensure we log all streams at the same
time based on the same timer.

https://codereview.webrtc.org/2133073002/diff/160001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.cc:1352: FillSenderStats(info);
And related, should we just enable this as is or give a heads up to users and
add this as an option for now?

[åsapersson, 2016-08-04 14:12:18]

Description was changed from

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 8 seconds).

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

to

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

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

[åsapersson, 2016-08-04 14:40:40]

Patchset #8 (id:180001) has been deleted

[åsapersson, 2016-08-04 14:54:18]

https://codereview.webrtc.org/2133073002/diff/160001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2133073002/diff/160001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.cc:1352: FillSenderStats(info);
On 2016/08/04 13:37:09, mflodman wrote:
> We could keep track of time just here and add a "bool log" (or similar) as
input
> argument to all Fill*Stats(). That would ensure we log all streams at the same
> time based on the same timer.

Done.

[åsapersson, 2016-08-08 12:46:30]

Patchset #9 (id:220001) has been deleted

[mflodman, 2016-08-11 12:38:25]

lgtm

[åsapersson, 2016-08-11 12:46:15]

The CQ bit was checked by asapersson@webrtc.org

[åsapersson, 2016-08-11 12:46:16]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2133073002/#ps240001 (title:
"")

[commit-bot: I haz the power, 2016-08-11 12:46:19]

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-08-11 14:46:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 14:46:45]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[åsapersson, 2016-08-11 15:35:11]

The CQ bit was checked by asapersson@webrtc.org

[commit-bot: I haz the power, 2016-08-11 15:35:24]

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-08-11 15:41:22]

Description was changed from

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

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

to

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

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

[commit-bot: I haz the power, 2016-08-11 15:41:24]

Committed patchset #9 (id:240001)

[commit-bot: I haz the power, 2016-08-11 15:41:29]

Description was changed from

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

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

to

==========
Add periodic logging of video stats.

Add ToString method to: Call::Stats, VideoSendStream::Stats,
VideoReceiveStream::Stats and log stats periodically (every 10 seconds).

BUG=

Committed: https://crrev.com/2e5cfcd6c27861fb1839f12bb2268d01d6f5a85e
Cr-Commit-Position: refs/heads/master@{#13727}
==========

[commit-bot: I haz the power, 2016-08-11 15:41:30]

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