RTCOutboundRTPStreamStats added.

RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
Committed: https://crrev.com/69e9cb08285f6cbcab547c7a5e6aa668fa6f2d29
Cr-Commit-Position: refs/heads/master@{#14860}

[hbos, 2016-10-26 09:35:42]

Description was changed from

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

to

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

[hbos, 2016-10-26 09:39:19]

Description was changed from

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

to

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

[hbos, 2016-10-26 09:39:19]

hbos@webrtc.org changed reviewers:
+ deadbeef@webrtc.org, hta@webrtc.org

[hbos, 2016-10-26 10:05:41]

Patchset #1 (id:1) has been deleted

[hbos, 2016-10-26 10:25:36]

Description was changed from

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

to

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

[hbos, 2016-10-26 10:26:11]

Please take a look, hta and deadbeef.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:115: // video and are purposefully left
undefined for audio.
Is this true? Even for NACK?

FIR and SLI are documented as only valid for video.
PLI is said to be "packet loss indication" and is not documented as video-only,
but the reference link says it's "picture loss indication" and is about video
data.
NACK on the other hand has to do with RTP packets lost, is that not
video-specific?

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:131:
static_cast<double>(video_sender_info.preferred_bitrate);
Is this the correct value? There is also bandwidth estimation info[1], perhaps
this should be target_enc_bitrate?

[1]
https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/mediachann...

[hta-webrtc, 2016-10-27 12:34:51]

lgtm

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
File webrtc/stats/rtcstatsreport_unittest.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
webrtc/stats/rtcstatsreport_unittest.cc:63: EXPECT_EQ(report->timestamp_us(),
1337u);
Style nit: Since you're using 1337 in 4 different contexts, should you make it a
kReportTimestamp constant? Magical numbers are bad.

[Taylor Brandstetter, 2016-10-27 21:44:37]

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:115: // video and are purposefully left
undefined for audio.
On 2016/10/26 10:26:11, hbos wrote:
> Is this true? Even for NACK?
> 
> FIR and SLI are documented as only valid for video.
> PLI is said to be "packet loss indication" and is not documented as
video-only,
> but the reference link says it's "picture loss indication" and is about video
> data.
> NACK on the other hand has to do with RTP packets lost, is that not
> video-specific?

NACK seems relevant to audio, PLI doesn't. I'll make a stats PR to fix that.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:131:
static_cast<double>(video_sender_info.preferred_bitrate);
On 2016/10/26 10:26:11, hbos wrote:
> Is this the correct value? There is also bandwidth estimation info[1], perhaps
> this should be target_enc_bitrate?
> 
> [1]
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/mediachann...

I came to the same conclusion; it should be target_enc_bitrate.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
File webrtc/stats/rtcstatsreport_unittest.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
webrtc/stats/rtcstatsreport_unittest.cc:63: EXPECT_EQ(report->timestamp_us(),
1337u);
On 2016/10/27 12:34:51, hta-webrtc wrote:
> Style nit: Since you're using 1337 in 4 different contexts, should you make it
a
> kReportTimestamp constant? Magical numbers are bad.

It's not really a magical number though; it's only relevant to
RTCStatsReport::Create and the assertion immediately below it. So in my opinion
this is fine.

[hbos, 2016-10-28 09:42:19]

Patchset #2 (id:40001) has been deleted

[hbos, 2016-10-28 09:42:37]

PTAL deadbeef.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:115: // video and are purposefully left
undefined for audio.
On 2016/10/27 21:44:36, Taylor Brandstetter wrote:
> On 2016/10/26 10:26:11, hbos wrote:
> > Is this true? Even for NACK?
> > 
> > FIR and SLI are documented as only valid for video.
> > PLI is said to be "packet loss indication" and is not documented as
> video-only,
> > but the reference link says it's "picture loss indication" and is about
video
> > data.
> > NACK on the other hand has to do with RTP packets lost, is that not
> > video-specific?
> 
> NACK seems relevant to audio, PLI doesn't. I'll make a stats PR to fix that.

Acknowledged.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:131:
static_cast<double>(video_sender_info.preferred_bitrate);
On 2016/10/27 21:44:36, Taylor Brandstetter wrote:
> On 2016/10/26 10:26:11, hbos wrote:
> > Is this the correct value? There is also bandwidth estimation info[1],
perhaps
> > this should be target_enc_bitrate?
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/mediachann...
> 
> I came to the same conclusion; it should be target_enc_bitrate.

Done. Note that this means that all video-RTCOutboundRTPStreamStats gets the
same target_bitrate value.

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
File webrtc/stats/rtcstatsreport_unittest.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/stats/rtcstatsrepo...
webrtc/stats/rtcstatsreport_unittest.cc:63: EXPECT_EQ(report->timestamp_us(),
1337u);
On 2016/10/27 21:44:37, Taylor Brandstetter wrote:
> On 2016/10/27 12:34:51, hta-webrtc wrote:
> > Style nit: Since you're using 1337 in 4 different contexts, should you make
it
> a
> > kReportTimestamp constant? Magical numbers are bad.
> 
> It's not really a magical number though; it's only relevant to
> RTCStatsReport::Create and the assertion immediately below it. So in my
opinion
> this is fine.

Yeah it's a completely meaningless number. In the test where I used the same
number twice I updated the second number to 1338.

[hbos, 2016-10-28 10:20:13]

By the way the EXPECT_EQ of a hardcoded report was a good idea for how to do
testing.

[Taylor Brandstetter, 2016-10-28 17:31:34]

Sorry, it looks like I mixed up "target_enc_bitrate" with
"target_media_bitrate_bps".

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:131:
static_cast<double>(video_sender_info.preferred_bitrate);
On 2016/10/28 09:42:37, hbos wrote:
> On 2016/10/27 21:44:36, Taylor Brandstetter wrote:
> > On 2016/10/26 10:26:11, hbos wrote:
> > > Is this the correct value? There is also bandwidth estimation info[1],
> perhaps
> > > this should be target_enc_bitrate?
> > > 
> > > [1]
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/mediachann...
> > 
> > I came to the same conclusion; it should be target_enc_bitrate.
> 
> Done. Note that this means that all video-RTCOutboundRTPStreamStats gets the
> same target_bitrate value.

Wait, that doesn't seem right. I didn't realize target_enc_bitrate was combined
for all streams; what we really want are the individual
"target_media_bitrate_bps" values that go into this:
https://cs.chromium.org/chromium/src/third_party/webrtc/video_send_stream.h?d...

If there's no way to get that currently, guess we need another TODO. Sorry for
my confusion.

[hbos, 2016-10-31 09:21:01]

PTAL deadbeef

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2456463002/diff/20001/webrtc/api/rtcstatscollec...
webrtc/api/rtcstatscollector.cc:131:
static_cast<double>(video_sender_info.preferred_bitrate);
On 2016/10/28 17:31:34, Taylor Brandstetter wrote:
> On 2016/10/28 09:42:37, hbos wrote:
> > On 2016/10/27 21:44:36, Taylor Brandstetter wrote:
> > > On 2016/10/26 10:26:11, hbos wrote:
> > > > Is this the correct value? There is also bandwidth estimation info[1],
> > perhaps
> > > > this should be target_enc_bitrate?
> > > > 
> > > > [1]
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/mediachann...
> > > 
> > > I came to the same conclusion; it should be target_enc_bitrate.
> > 
> > Done. Note that this means that all video-RTCOutboundRTPStreamStats gets the
> > same target_bitrate value.
> 
> Wait, that doesn't seem right. I didn't realize target_enc_bitrate was
combined
> for all streams; what we really want are the individual
> "target_media_bitrate_bps" values that go into this:
>
https://cs.chromium.org/chromium/src/third_party/webrtc/video_send_stream.h?d...
> 
> If there's no way to get that currently, guess we need another TODO. Sorry for
> my confusion.

That will be easy to surface but I'll do it in a separate CL, making it a TODO
here.

[Taylor Brandstetter, 2016-10-31 18:18:46]

lgtm

[hbos, 2016-10-31 21:22:11]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2016-10-31 21:22:11]

The patchset sent to the CQ was uploaded after l-g-t-m from hta@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2456463002/#ps80001 (title:
"Rebase and TODO for target_bitrate")

[commit-bot: I haz the power, 2016-10-31 21:22:23]

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-10-31 21:48:30]

Description was changed from

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

to

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

[commit-bot: I haz the power, 2016-10-31 21:48:31]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-10-31 21:48:47]

Description was changed from

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854
==========

to

==========
RTCOutboundRTPStreamStats[1] added.

This also adds RTCRTPStreamStats[2] which it derives from. Not all stats
are supported in this CL, this must be addressed before closing the
issue.

RTCStatsReport also gets a timestamp and ToString.

[1] https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
[2] https://w3c.github.io/webrtc-stats/#streamstats-dict*

BUG=chromium:627816, chromium:657856, chromium:657854

Committed: https://crrev.com/69e9cb08285f6cbcab547c7a5e6aa668fa6f2d29
Cr-Commit-Position: refs/heads/master@{#14860}
==========

[commit-bot: I haz the power, 2016-10-31 21:48:48]

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

[perkj_webrtc, 2016-11-01 06:20:36]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.webrtc.org/2465193002/ by perkj@webrtc.org.

The reason for reverting is: Breaks Chrome FYI

Ihttps://build.chromium.org/p/chromium.webrtc.fyi/builders/Win%20Builder/builds/7610/steps/compile/logs/stdio.

[perkj_webrtc, 2016-11-01 06:52:15]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.webrtc.org/2465223002/ by perkj@webrtc.org.

The reason for reverting is: Breaks Chrome FYI. 
peerconnection_unittest calls RTCStatsReport::Create without  parameters..