Wire up BWE stats through WebrtcSession so that they are filled in both for audio and video calls.

webrtc/pc/rtcstatscollector.cc

 /*
  *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 678 matching lines @@
 }
 
 void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
     int64_t timestamp_us) {
   RTC_DCHECK(network_thread_->IsCurrent());
   rtc::scoped_refptr<RTCStatsReport> report = RTCStatsReport::Create(
       timestamp_us);
 
   std::unique_ptr<SessionStats> session_stats =
       pc_->session()->GetStats(*channel_name_pairs_);
+  Call::Stats call_stats = pc_->session()->GetCallStats();

[Taylor Brandstetter, 2017/05/07 21:30:44]

This will cause a new invoke to the worker thread, which the new stats collector
implementation is meant to avoid. Maybe hbos has some thoughts on what to do.

[stefan-webrtc, 2017/05/08 07:12:56]

I guess we could merge it with WebRtcSession::GetStats()?

[Taylor Brandstetter, 2017/05/08 17:47:41]

WebRtcSession::GetStats invokes on the network thread, which is why it's called
in ProducePartialResultsOnNeworkThread. But GetCallStats invokes on the worker
thread, so ideally it should be called in a
"ProducePartialResultsOnWorkerThread", along with
[Voice|Video]Channel::GetStats.

Since we have a few situations where a single "RTCStats" object needs
information from multiple threads, it would make sense (to me) to gather and
store the "raw" stats on different threads, then only create the RTCStats
objects in a final pass once every thread has finished. hbos@, what do you
think?

[hbos, 2017/05/09 12:48:13]

Doing synchronous invokes from the network thread could also be a possible risk
of deadlock.

Getting "raw" stats on different threads and creating RTCStats objects in a
final pass sounds good to me. Though it could get complicated or require
refactoring, that might be good to do in a separate CL if we want to generalize
this technique?

You could simply fetch them as a "Prepare" step,
https://cs.chromium.org/chromium/src/third_party/webrtc/pc/rtcstatscollector....,
and pass them to ProducePartialResultsOnNetworkThread, but that doesn't get rid
of the extra thread jump.

   if (session_stats) {
     std::map<std::string, CertificateStatsPair> transport_cert_stats =
         PrepareTransportCertificateStats_n(*session_stats);
 
     ProduceCertificateStats_n(
         timestamp_us, transport_cert_stats, report.get());
     ProduceCodecStats_n(
         timestamp_us, *track_media_info_map_, report.get());
-    ProduceIceCandidateAndPairStats_n(
-        timestamp_us, *session_stats, track_media_info_map_->video_media_info(),
-        report.get());
+    ProduceIceCandidateAndPairStats_n(timestamp_us, *session_stats,
+                                      track_media_info_map_->video_media_info(),
+                                      call_stats, report.get());
     ProduceRTPStreamStats_n(
         timestamp_us, *session_stats, *track_media_info_map_, report.get());
     ProduceTransportStats_n(
         timestamp_us, *session_stats, transport_cert_stats, report.get());
   }
 
   AddPartialResults(report);
 }
 
 void RTCStatsCollector::AddPartialResults(
@@ skipping 108 matching lines @@
         DataStateToRTCDataChannelState(data_channel->state());
     data_channel_stats->messages_sent = data_channel->messages_sent();
     data_channel_stats->bytes_sent = data_channel->bytes_sent();
     data_channel_stats->messages_received = data_channel->messages_received();
     data_channel_stats->bytes_received = data_channel->bytes_received();
     report->AddStats(std::move(data_channel_stats));
   }
 }
 
 void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
-      int64_t timestamp_us, const SessionStats& session_stats,
-      const cricket::VideoMediaInfo* video_media_info,
-      RTCStatsReport* report) const {
+    int64_t timestamp_us,
+    const SessionStats& session_stats,
+    const cricket::VideoMediaInfo* video_media_info,
+    const Call::Stats& call_stats,
+    RTCStatsReport* report) const {
   RTC_DCHECK(network_thread_->IsCurrent());
   for (const auto& transport_stats : session_stats.transport_stats) {
     for (const auto& channel_stats : transport_stats.second.channel_stats) {
       std::string transport_id = RTCTransportStatsIDFromTransportChannel(
           transport_stats.second.transport_name, channel_stats.component);
       for (const cricket::ConnectionInfo& info :
            channel_stats.connection_infos) {
         std::unique_ptr<RTCIceCandidatePairStats> candidate_pair_stats(
             new RTCIceCandidatePairStats(
                 RTCIceCandidatePairStatsIDFromConnectionInfo(info),
@@ skipping 21 matching lines @@
         candidate_pair_stats->bytes_received =
             static_cast<uint64_t>(info.recv_total_bytes);
         candidate_pair_stats->total_round_trip_time =
             static_cast<double>(info.total_round_trip_time_ms) /
             rtc::kNumMillisecsPerSec;
         if (info.current_round_trip_time_ms) {
           candidate_pair_stats->current_round_trip_time =
               static_cast<double>(*info.current_round_trip_time_ms) /
               rtc::kNumMillisecsPerSec;
         }
-        if (info.best_connection && video_media_info &&
-            !video_media_info->bw_estimations.empty()) {
+        if (info.best_connection && video_media_info) {

[Taylor Brandstetter, 2017/05/07 21:30:44]

Does this still need "&& video_media_info"?

[stefan-webrtc, 2017/05/08 07:12:56]

No probably not.

           // The bandwidth estimations we have are for the selected candidate
           // pair ("info.best_connection").
-          RTC_DCHECK_EQ(video_media_info->bw_estimations.size(), 1);
-          RTC_DCHECK_GE(
-              video_media_info->bw_estimations[0].available_send_bandwidth, 0);
-          RTC_DCHECK_GE(
-              video_media_info->bw_estimations[0].available_recv_bandwidth, 0);
-          if (video_media_info->bw_estimations[0].available_send_bandwidth) {
+          RTC_DCHECK_GE(call_stats.send_bandwidth_bps, 0);
+          RTC_DCHECK_GE(call_stats.recv_bandwidth_bps, 0);
+          if (call_stats.send_bandwidth_bps > 0) {

[stefan-webrtc, 2017/05/05 15:41:54]

Not sure this check makes sense, but kept it to not change the behavior. Wdyt
hbos?

             candidate_pair_stats->available_outgoing_bitrate =
-                static_cast<double>(video_media_info->bw_estimations[0]
-                                        .available_send_bandwidth);
+                static_cast<double>(call_stats.send_bandwidth_bps);
           }
-          if (video_media_info->bw_estimations[0].available_recv_bandwidth) {
+          if (call_stats.recv_bandwidth_bps > 0) {
             candidate_pair_stats->available_incoming_bitrate =
-                static_cast<double>(video_media_info->bw_estimations[0]
-                                        .available_recv_bandwidth);
+                static_cast<double>(call_stats.recv_bandwidth_bps);
           }
         }
         candidate_pair_stats->requests_received =
             static_cast<uint64_t>(info.recv_ping_requests);
         candidate_pair_stats->requests_sent = static_cast<uint64_t>(
             info.sent_ping_requests_before_first_response);
         candidate_pair_stats->responses_received =
             static_cast<uint64_t>(info.recv_ping_responses);
         candidate_pair_stats->responses_sent =
             static_cast<uint64_t>(info.sent_ping_responses);
@@ skipping 335 matching lines @@
     const std::string& type) {
   return CandidateTypeToRTCIceCandidateType(type);
 }
 
 const char* DataStateToRTCDataChannelStateForTesting(
     DataChannelInterface::DataState state) {
   return DataStateToRTCDataChannelState(state);
 }
 
 }  // namespace webrtc