Wire-up BWE feedback for audio receive streams.

webrtc/audio/audio_receive_stream.cc

Index: webrtc/audio/audio_receive_stream.cc
diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc
index dfad79f9d76420460267f2fdd0e1265a928b8786..4781d8f9c5aefdbb8dd434faf0edfb6be2b75a62 100644
--- a/webrtc/audio/audio_receive_stream.cc
+++ b/webrtc/audio/audio_receive_stream.cc
@@ -18,6 +18,7 @@
 #include "webrtc/audio/conversion.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/call/congestion_controller.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "webrtc/system_wrappers/include/tick_util.h"
 #include "webrtc/voice_engine/channel_proxy.h"
@@ -30,6 +31,21 @@
 #include "webrtc/voice_engine/voice_engine_impl.h"
 
 namespace webrtc {
+namespace {
+
+bool UseSendSideBwe(const webrtc::AudioReceiveStream::Config& config) {
+  if (!config.rtp.transport_cc) {
+    return false;
+  }
+  for (const auto& extension : config.rtp.extensions) {
+    if (extension.name == RtpExtension::kTransportSequenceNumber) {
+      return true;
+    }
+  }
+  return false;
+}
+}  // namespace
+
 std::string AudioReceiveStream::Config::Rtp::ToString() const {
   std::stringstream ss;
   ss << "{remote_ssrc: " << remote_ssrc;
@@ -65,16 +81,19 @@ std::string AudioReceiveStream::Config::ToString() const {
 
 namespace internal {
 AudioReceiveStream::AudioReceiveStream(
-    RemoteBitrateEstimator* remote_bitrate_estimator,
+    CongestionController* congestion_controller,
     const webrtc::AudioReceiveStream::Config& config,
     const rtc::scoped_refptr<webrtc::AudioState>& audio_state)
-    : remote_bitrate_estimator_(remote_bitrate_estimator),
+    : remote_bitrate_estimator_(

[the sun, 2016/01/08 10:29:35]

I'd prefer if you set remote_bitrate_estimator_ = nullptr in the .h and move
this initialization into the block starting at line 122 below. Then all CC/RBE
setup is in one place.

[stefan-webrtc, 2016/01/08 15:36:02]

I'll let you decide since it's in your code, but this means I'll have to drop
the const on remote_bitrate_estimator_.

+          config.combined_audio_video_bwe
+              ? congestion_controller->GetRemoteBitrateEstimator(
+                    UseSendSideBwe(config))
+              : nullptr),
       config_(config),
       audio_state_(audio_state),
       rtp_header_parser_(RtpHeaderParser::Create()) {
   LOG(LS_INFO) << "AudioReceiveStream: " << config_.ToString();
   RTC_DCHECK_NE(config_.voe_channel_id, -1);

[the sun, 2016/01/08 10:29:35]

RTC_DCHECK(congestion_controller);

[stefan-webrtc, 2016/01/08 15:36:02]

Done.

-  RTC_DCHECK(remote_bitrate_estimator_);
   RTC_DCHECK(audio_state_.get());
   RTC_DCHECK(rtp_header_parser_);
 
@@ -93,8 +112,6 @@ AudioReceiveStream::AudioReceiveStream(
           kRtpExtensionAbsoluteSendTime, extension.id);
       RTC_DCHECK(registered);
     } else if (extension.name == RtpExtension::kTransportSequenceNumber) {
-      // TODO(holmer): Need to do something here or in  DeliverRtp() to actually
-      //               handle audio packets with this header extension.
       bool registered = rtp_header_parser_->RegisterRtpHeaderExtension(
           kRtpExtensionTransportSequenceNumber, extension.id);
       RTC_DCHECK(registered);
@@ -102,11 +119,19 @@ AudioReceiveStream::AudioReceiveStream(
       RTC_NOTREACHED() << "Unsupported RTP extension.";
     }
   }
+  if (config.combined_audio_video_bwe && UseSendSideBwe(config)) {
+    channel_proxy_->SetCongestionControlObjects(

[the sun, 2016/01/08 10:29:36]

// Configure bandwidth estimation.
if (config.combined_audio_video_bwe) {
  if (UseSendSideBwe(config)) {
    remote_bitrate_estimator_ =
congestion_controller->GetRemoteBitrateEstimator(true);
    channel_proxy_->SetCongestionControlObjects(
        nullptr, nullptr, congestion_controller->packet_router());
  } else {
    remote_bitrate_estimator_ =
congestion_controller->GetRemoteBitrateEstimator(false);
  }
  RTC_DCHECK(remote_bitrate_estimator_);
 }

[stefan-webrtc, 2016/01/08 15:36:02]

Done.

+        nullptr, nullptr, congestion_controller->packet_router());
+  }
 }
 
 AudioReceiveStream::~AudioReceiveStream() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "~AudioReceiveStream: " << config_.ToString();
+  channel_proxy_->SetCongestionControlObjects(nullptr, nullptr, nullptr);
+  if (config_.combined_audio_video_bwe) {
+    remote_bitrate_estimator_->RemoveStream(config_.rtp.remote_ssrc);

[the sun, 2016/01/08 10:29:35]

It would be simpler to make the call conditional on remote_bitrate_estimator_.
Then the actual logic doesn't need to match/be repeated.

[stefan-webrtc, 2016/01/08 15:36:02]

Done.

+  }
 }
 
 void AudioReceiveStream::Start() {
@@ -141,10 +166,12 @@ bool AudioReceiveStream::DeliverRtp(const uint8_t* packet,
     return false;
   }
 
-  // Only forward if the parsed header has absolute sender time. RTP timestamps
-  // may have different rates for audio and video and shouldn't be mixed.
+  // Only forward if the parsed header has one of the headers necessary for
+  // bandwidth estimation. RTP timestamps has different rates for audio and
+  // video and shouldn't be mixed.
   if (config_.combined_audio_video_bwe &&

[the sun, 2016/01/08 10:29:35]

Ditto:
if (remote_bitrate_estimator_ &&
  ...

[stefan-webrtc, 2016/01/08 15:36:02]

Done.

-      header.extension.hasAbsoluteSendTime) {
+      (header.extension.hasAbsoluteSendTime ||
+       header.extension.hasTransportSequenceNumber)) {
     int64_t arrival_time_ms = TickTime::MillisecondTimestamp();
     if (packet_time.timestamp >= 0)
       arrival_time_ms = (packet_time.timestamp + 500) / 1000;