Only return stats for the most recent unsignaled audio stream.

Only return stats for the most recent unsignaled audio stream.

The track-level stats are currently implemented in terms of the stream-
level stats. Which is a problem if multiple unsignaled streams map to the
same track (see bug for more details). This CL fixes the problem
partially, but only returning stats for one of the unsignaled streams.
A better solution would be to return stats for both streams, but update
the track-level stats independently somehow. But that would require more
extensive changes, and it's not yet clear how we want to do it.

BUG=webrtc:8158
Review-Url: https://codereview.webrtc.org/3008373002
Cr-Commit-Position: refs/heads/master@{#19907}
Committed: https://webrtc.googlesource.com/src/+/4e2deab79cdcee734aede7482b1a28ed00f5292c

[Taylor Brandstetter, 2017-09-14 01:31:31]

Description was changed from

==========
Adding test for unsignaled SSRC changing and messing up track stats.

BUG=webrtc:8158
==========

to

==========
Only return stats for the most recent unsignaled audio stream.

The track-level stats are currently implemented in terms of the stream-
level stats. Which is a problem if multiple unsignaled streams map to the
same track (see bug for more details). This CL fixes the problem
partially, but only returning stats for one of the unsignaled streams.
A better solution would be to return stats for both streams, but update
the track-level stats independently somehow. But that would require more
extensive changes, and it's not yet clear how we want to do it.

BUG=webrtc:8158
==========

[Taylor Brandstetter, 2017-09-14 01:32:48]

deadbeef@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[Taylor Brandstetter, 2017-09-14 01:32:48]

PTAL. See the bug for more context. This fixes the issue partially; the
stats-consuming application will see counters reset to 0 when the new SSRC shows
up, but at least after that point they'll be updated correctly.

[Taylor Brandstetter, 2017-09-18 16:53:34]

Ping

[the sun, 2017-09-18 18:59:35]

lgtm

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine.cc:2283: --unsignaled_recv_ssrcs_.end(),
nit: I would've done it

auto end_it = --unsignaled_recv_ssrcs_.end();
if (std::find(unsignaled_recv_ssrcs.begin(), endit, ssrc) != end_it) {
  continue;
}

which is as many lines but easier to read

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/pc/peerconnection_...
File webrtc/pc/peerconnection_integrationtest.cc (right):

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/pc/peerconnection_...
webrtc/pc/peerconnection_integrationtest.cc:2019: // uint64_t
prev_samples_received = *track_stats[0]->total_samples_received;
remove commented-out line

[Taylor Brandstetter, 2017-09-20 00:19:46]

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvoiceengine.cc:2283: --unsignaled_recv_ssrcs_.end(),
On 2017/09/18 18:59:35, the sun wrote:
> nit: I would've done it
> 
> auto end_it = --unsignaled_recv_ssrcs_.end();
> if (std::find(unsignaled_recv_ssrcs.begin(), endit, ssrc) != end_it) {
>   continue;
> }
> 
> which is as many lines but easier to read

Can't quite do that since if the list is empty end() == begin() and decrementing
it is technically undefined behavior. But I'll do something similar.

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/pc/peerconnection_...
File webrtc/pc/peerconnection_integrationtest.cc (right):

https://codereview.webrtc.org/3008373002/diff/20001/webrtc/pc/peerconnection_...
webrtc/pc/peerconnection_integrationtest.cc:2019: // uint64_t
prev_samples_received = *track_stats[0]->total_samples_received;
On 2017/09/18 18:59:35, the sun wrote:
> remove commented-out line

Won't compile if I do (unused variable warning). I plan to uncomment the
relevant code here when the issue is fixed fully. See TODO below.

[Taylor Brandstetter, 2017-09-20 00:19:57]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2017-09-20 00:19:58]

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

[commit-bot: I haz the power, 2017-09-20 00:20:10]

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-09-20 00:30:37]

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

[commit-bot: I haz the power, 2017-09-20 00:30:38]

Try jobs failed on following builders:
  win_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_dbg/builds/22586)

[Taylor Brandstetter, 2017-09-20 20:27:43]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2017-09-20 20:27:44]

The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3008373002/#ps80001 (title:
"Increase test duration and weaken failure criteria.")

[commit-bot: I haz the power, 2017-09-20 20:27:49]

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-09-20 20:56:22]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1505939263887100,
"parent_rev": "feeb9bfe033daf9976819081a191f63d5d5f5346", "commit_rev":
"4e2deab79cdcee734aede7482b1a28ed00f5292c"}

[commit-bot: I haz the power, 2017-09-20 20:56:26]

Description was changed from

==========
Only return stats for the most recent unsignaled audio stream.

The track-level stats are currently implemented in terms of the stream-
level stats. Which is a problem if multiple unsignaled streams map to the
same track (see bug for more details). This CL fixes the problem
partially, but only returning stats for one of the unsignaled streams.
A better solution would be to return stats for both streams, but update
the track-level stats independently somehow. But that would require more
extensive changes, and it's not yet clear how we want to do it.

BUG=webrtc:8158
==========

to

==========
Only return stats for the most recent unsignaled audio stream.

The track-level stats are currently implemented in terms of the stream-
level stats. Which is a problem if multiple unsignaled streams map to the
same track (see bug for more details). This CL fixes the problem
partially, but only returning stats for one of the unsignaled streams.
A better solution would be to return stats for both streams, but update
the track-level stats independently somehow. But that would require more
extensive changes, and it's not yet clear how we want to do it.

BUG=webrtc:8158

Review-Url: https://codereview.webrtc.org/3008373002
Cr-Commit-Position: refs/heads/master@{#19907}
Committed:
https://webrtc.googlesource.com/src/+/4e2deab79cdcee734aede7482b1a28ed00f5292c
==========

[commit-bot: I haz the power, 2017-09-20 20:56:27]

Committed patchset #5 (id:80001) as
https://webrtc.googlesource.com/src/+/4e2deab79cdcee734aede7482b1a28ed00f5292c