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

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 65833873f68c0e1a6f76edf67cba5fede33405d6..95b34acef0f94bf33087bc9ee955ee389dab635d 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -1342,8 +1342,9 @@ bool WebRtcVideoChannel2::GetStats(VideoMediaInfo* info) {
   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;
@@ -1378,22 +1379,13 @@ void WebRtcVideoChannel2::FillReceiverStats(VideoMediaInfo* video_media_info,
   }
 }
 
-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(
@@ -2100,7 +2092,7 @@ VideoSenderInfo WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo(
   return info;
 }
 
-void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBandwidthEstimationInfo(
+void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBitrateInfo(
     BandwidthEstimationInfo* bwe_info) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   if (stream_ == NULL) {