RTCMediaStreamTrackStats.framesSent collected by RTCStatsCollector.

RTCMediaStreamTrackStats.framesSent collected by RTCStatsCollector.

BUG=webrtc:6757, chromium:659137, chromium:627816
Review-Url: https://codereview.webrtc.org/2606033002
Cr-Commit-Position: refs/heads/master@{#16188}
Committed: https://chromium.googlesource.com/external/webrtc/+/fefe076789f9294aaeb0ecc4221ad5e5970492e9

[hbos, 2016-12-29 14:17:36]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 14:17:40]

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

[hbos, 2016-12-29 14:20:56]

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

[hbos, 2016-12-29 14:20:57]

Please take a look, hta and deadbeef.

I'm thinking that if "RTCMediaStreamTrackStats.framesPerSecond = framerate_sent
= encode_frame_rate"[1] then it makes sense for
"RTCMediaStreamTrackStats.framesPerSecond = frames_encoded" given no member
named "sent".

[1]
https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvi...

[hbos, 2016-12-29 14:21:49]

I meant "RTCMediaStreamTrackStats.framesSent = frames_encoded"

[commit-bot: I haz the power, 2016-12-29 15:31:31]

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

[commit-bot: I haz the power, 2016-12-29 15:31:32]

Dry run: This issue passed the CQ dry run.

[hbos, 2016-12-29 15:34:59]

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector.cc
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector....
webrtc/api/rtcstatscollector.cc:455: video_track_stats->frames_sent =
sender_info->frames_encoded;
This is the same as RTCOutboundRTPStreamStats.framesEncoded added in
https://codereview.webrtc.org/2588373005/diff/20001/webrtc/api/rtcstatscollec...
!

[hta-webrtc, 2017-01-02 16:00:12]

lgtm

once you add the right TODO.

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector.cc
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector....
webrtc/api/rtcstatscollector.cc:455: video_track_stats->frames_sent =
sender_info->frames_encoded;
On 2016/12/29 15:34:59, hbos wrote:
> This is the same as RTCOutboundRTPStreamStats.framesEncoded added in
>
https://codereview.webrtc.org/2588373005/diff/20001/webrtc/api/rtcstatscollec...
> !

The reason why the stats have two variables is that we might drop some frames
due to congestion control after encoding them.
Add a TODO that says "will reduce this by frames dropped due to congestion
control when available".

[hbos, 2017-01-03 09:13:56]

On 2017/01/02 16:00:12, hta-webrtc wrote:
> lgtm
> 
> once you add the right TODO.

What about the issue of a track switching SSRCs and this stat coming from SSRC?

[Taylor Brandstetter, 2017-01-04 23:44:37]

On 2017/01/03 09:13:56, hbos wrote:
> On 2017/01/02 16:00:12, hta-webrtc wrote:
> > lgtm
> > 
> > once you add the right TODO.
> 
> What about the issue of a track switching SSRCs and this stat coming from
SSRC?

Do you mean if there's an SSRC collision? It looks like those are currently
handled transparently, and won't affect the stats.

https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/sour...

[Taylor Brandstetter, 2017-01-04 23:45:18]

lgtm, though I agree about the TODO.

[hbos, 2017-01-19 10:39:58]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 10:40:05]

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

[hbos, 2017-01-19 10:41:16]

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector.cc
File webrtc/api/rtcstatscollector.cc (right):

https://codereview.webrtc.org/2606033002/diff/1/webrtc/api/rtcstatscollector....
webrtc/api/rtcstatscollector.cc:455: video_track_stats->frames_sent =
sender_info->frames_encoded;
On 2017/01/02 16:00:12, hta-webrtc wrote:
> On 2016/12/29 15:34:59, hbos wrote:
> > This is the same as RTCOutboundRTPStreamStats.framesEncoded added in
> >
>
https://codereview.webrtc.org/2588373005/diff/20001/webrtc/api/rtcstatscollec...
> > !
> 
> The reason why the stats have two variables is that we might drop some frames
> due to congestion control after encoding them.
> Add a TODO that says "will reduce this by frames dropped due to congestion
> control when available".

Done.

[commit-bot: I haz the power, 2017-01-19 12:27:23]

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

[commit-bot: I haz the power, 2017-01-19 12:27:24]

Dry run: This issue passed the CQ dry run.

[hbos, 2017-01-20 12:03:58]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2017-01-20 12:03:59]

The patchset sent to the CQ was uploaded after l-g-t-m from hta@webrtc.org,
deadbeef@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2606033002/#ps60001 (title:
"Rebase with master")

[commit-bot: I haz the power, 2017-01-20 12:04:07]

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, 2017-01-20 14:14:25]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484913838875810,
"parent_rev": "2d4d653e1fd25ade9ae768058b1225e6b430bc76", "commit_rev":
"fefe076789f9294aaeb0ecc4221ad5e5970492e9"}

[commit-bot: I haz the power, 2017-01-20 14:14:30]

Description was changed from

==========
RTCMediaStreamTrackStats.framesSent collected by RTCStatsCollector.

BUG=webrtc:6757, chromium:659137, chromium:627816
==========

to

==========
RTCMediaStreamTrackStats.framesSent collected by RTCStatsCollector.

BUG=webrtc:6757, chromium:659137, chromium:627816

Review-Url: https://codereview.webrtc.org/2606033002
Cr-Commit-Position: refs/heads/master@{#16188}
Committed:
https://chromium.googlesource.com/external/webrtc/+/fefe076789f9294aaeb0ecc42...
==========

[commit-bot: I haz the power, 2017-01-20 14:14:31]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/fefe076789f9294aaeb0ecc42...