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

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 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 1324 matching lines @@
   if (last_stats_log_ms_ == -1 ||
       now_ms - last_stats_log_ms_ > kStatsLogIntervalMs) {
     last_stats_log_ms_ = now_ms;
     log_stats = true;
   }
 
   info->Clear();
   FillSenderStats(info, log_stats);
   FillReceiverStats(info, log_stats);
   FillSendAndReceiveCodecStats(info);
+  // TODO(holmer): We should either have rtt available as a metric on
+  // VideoSend/ReceiveStreams, or we should remove rtt from VideoSenderInfo.
   webrtc::Call::Stats stats = call_->GetStats();

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

What are these stats, which contain send/recv_bandwidth_bps that we don't use,
compared to WebRtcVideoSendStream::FillBitrateInfo bandwidth estimates?
FillBandwidthEstimationStats used to be the sum of both..

[holmer, 2017/05/30 14:44:28]

The only reason I have to call Call::GetStats() is because I need the RTT, since
we have to give it to info->senders. After this CL Call::GetStats() should be
called through WebRtcSession instead of going through WebRtcVideoChannel2.

[hbos, 2017/05/31 14:31:58]

Here call_->GetStats() is invoked from WebRtcVideoChannel2::GetStats which is
invoked from VideoChannel::GetStats doing InvokeOnWorker. That is,
call_->GetStats() is invoked on the worker thread.

RTCStatsCollector does GetCallStats() aka call_->GetStats() on the network
thread.

It seems the VideoChannel ensures call_ is used on the worker thread and
WebRtcSession ensures call_ is used on the network thread. Is both valid? Is
either wrong? Does it not matter?

If it doesn't matter then maybe we don't need GetCallStats_s and can call
GetCallStats directly on the StatsCollector case.

[stefan-webrtc, 2017/05/31 16:48:03]

Ideally we should probably call Call::GetStats on the worker thread. Changed in
an attempt to achieve this.

-  FillBandwidthEstimationStats(stats, info);
   if (stats.rtt_ms != -1) {
     for (size_t i = 0; i < info->senders.size(); ++i) {
       info->senders[i].rtt_ms = stats.rtt_ms;
     }
   }
 
   if (log_stats)
     LOG(LS_INFO) << stats.ToString(now_ms);
 
   return true;
@@ skipping 14 matching lines @@
                                             bool log_stats) {
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32_t, WebRtcVideoReceiveStream*>::iterator it =
            receive_streams_.begin();
        it != receive_streams_.end(); ++it) {
     video_media_info->receivers.push_back(
         it->second->GetVideoReceiverInfo(log_stats));
   }
 }
 
-void WebRtcVideoChannel2::FillBandwidthEstimationStats(
-    const webrtc::Call::Stats& stats,
-    VideoMediaInfo* video_media_info) {
-  BandwidthEstimationInfo bwe_info;
-  bwe_info.available_send_bandwidth = stats.send_bandwidth_bps;
-  bwe_info.available_recv_bandwidth = stats.recv_bandwidth_bps;
-  bwe_info.bucket_delay = stats.pacer_delay_ms;
-
-  // Get send stream bitrate stats.
+void WebRtcVideoChannel2::FillBitrateInfo(BandwidthEstimationInfo* bwe_info) {

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

nit: Do we still have a need for a bwe pointer or can we return
BandwidthEstimationInfo by value?

[holmer, 2017/05/30 14:44:28]

We could, but it'd require that we copy the BandwidthEstimationInfo just to fill
in the bitrate info. I could make that change, but I'm not sure it's better?

[hbos, 2017/05/31 14:31:58]

Acknowledged.

   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32_t, WebRtcVideoSendStream*>::iterator stream =
            send_streams_.begin();
        stream != send_streams_.end(); ++stream) {
-    stream->second->FillBandwidthEstimationInfo(&bwe_info);
+    stream->second->FillBitrateInfo(bwe_info);
   }
-  video_media_info->bw_estimations.push_back(bwe_info);
 }
 
 void WebRtcVideoChannel2::FillSendAndReceiveCodecStats(
     VideoMediaInfo* video_media_info) {
   for (const VideoCodec& codec : send_params_.codecs) {
     webrtc::RtpCodecParameters codec_params = codec.ToCodecParameters();
     video_media_info->send_codecs.insert(
         std::make_pair(codec_params.payload_type, std::move(codec_params)));
   }
   for (const VideoCodec& codec : recv_params_.codecs) {
@@ skipping 686 matching lines @@
     webrtc::VideoSendStream::StreamStats first_stream_stats =
         stats.substreams.begin()->second;
     info.fraction_lost =
         static_cast<float>(first_stream_stats.rtcp_stats.fraction_lost) /
         (1 << 8);
   }
 
   return info;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBandwidthEstimationInfo(
+void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBitrateInfo(
     BandwidthEstimationInfo* bwe_info) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   if (stream_ == NULL) {
     return;
   }
   webrtc::VideoSendStream::Stats stats = stream_->GetStats();
   for (std::map<uint32_t, webrtc::VideoSendStream::StreamStats>::iterator it =
            stats.substreams.begin();
        it != stats.substreams.end(); ++it) {
     bwe_info->transmit_bitrate += it->second.total_bitrate_bps;
@@ skipping 485 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             ulpfec_config.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket