Require negotiation to send transport cc feedback over RTCP.

BUG=4312

Review URL: https://codereview.webrtc.org/1452883002

Cr-Commit-Position: refs/heads/master@{#10735}

12 files changed, 99 insertions, 19 deletions

diff --git a/talk/media/base/codec.cc b/talk/media/base/codec.cc
index 5b747d1917..5fbae2c45e 100644
--- a/talk/media/base/codec.cc
+++ b/talk/media/base/codec.cc
@@ -334,6 +334,11 @@ bool HasRemb(const VideoCodec& codec) {
       FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
 }
 
+bool HasTransportCc(const VideoCodec& codec) {
+  return codec.HasFeedbackParam(
+      FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
+}
+
 bool CodecNamesEq(const std::string& name1, const std::string& name2) {
   return _stricmp(name1.c_str(), name2.c_str()) == 0;
 }
diff --git a/talk/media/base/codec.h b/talk/media/base/codec.h
index 3bb08e7c7a..afdf5fdad9 100644
--- a/talk/media/base/codec.h
+++ b/talk/media/base/codec.h
@@ -271,6 +271,7 @@ bool FindCodecById(const std::vector<Codec>& codecs,
 bool CodecNamesEq(const std::string& name1, const std::string& name2);
 bool HasNack(const VideoCodec& codec);
 bool HasRemb(const VideoCodec& codec);
+bool HasTransportCc(const VideoCodec& codec);
 
 }  // namespace cricket
 
diff --git a/talk/media/base/constants.cc b/talk/media/base/constants.cc
index 4063004968..2361be6f50 100644
--- a/talk/media/base/constants.cc
+++ b/talk/media/base/constants.cc
@@ -90,6 +90,7 @@ const int kPreferredUseInbandFec = 0;
 const char kRtcpFbParamNack[] = "nack";
 const char kRtcpFbNackParamPli[] = "pli";
 const char kRtcpFbParamRemb[] = "goog-remb";
+const char kRtcpFbParamTransportCc[] = "transport-cc";
 
 const char kRtcpFbParamCcm[] = "ccm";
 const char kRtcpFbCcmParamFir[] = "fir";
diff --git a/talk/media/base/constants.h b/talk/media/base/constants.h
index b6a9e5681f..706a7bdc87 100644
--- a/talk/media/base/constants.h
+++ b/talk/media/base/constants.h
@@ -107,6 +107,9 @@ extern const char kRtcpFbNackParamPli[];
 // rtcp-fb messages according to
 // http://tools.ietf.org/html/draft-alvestrand-rmcat-remb-00
 extern const char kRtcpFbParamRemb[];
+// rtcp-fb messages according to
+// https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01
+extern const char kRtcpFbParamTransportCc[];
 // ccm submessages according to RFC 5104
 extern const char kRtcpFbParamCcm[];
 extern const char kRtcpFbCcmParamFir[];
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 1612183c7f..2d648b73a3 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -166,6 +166,8 @@ void AddDefaultFeedbackParams(VideoCodec* codec) {
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
+  codec->AddFeedbackParam(
+      FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
 }
 
 static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type,
@@ -960,12 +962,15 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
     RTC_DCHECK(kv.second != nullptr);
     kv.second->SetCodec(supported_codecs.front());
   }
-  LOG(LS_INFO) << "SetNackAndRemb on all the receive streams because the send "
-                  "codec has changed.";
+  LOG(LS_INFO)
+      << "SetFeedbackOptions on all the receive streams because the send "
+         "codec has changed.";
   for (auto& kv : receive_streams_) {
     RTC_DCHECK(kv.second != nullptr);
-    kv.second->SetNackAndRemb(HasNack(supported_codecs.front().codec),
-                              HasRemb(supported_codecs.front().codec));
+    kv.second->SetFeedbackParameters(
+        HasNack(supported_codecs.front().codec),
+        HasRemb(supported_codecs.front().codec),
+        HasTransportCc(supported_codecs.front().codec));
   }
 
   // TODO(holmer): Changing the codec parameters shouldn't necessarily mean that
@@ -1215,6 +1220,8 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
   config.sync_group = sp.sync_label;
 
   config.rtp.remb = send_codec_ ? HasRemb(send_codec_->codec) : false;
+  config.rtp.transport_cc =
+      send_codec_ ? HasTransportCc(send_codec_->codec) : false;
 
   receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
       call_, sp, config, external_decoder_factory_, default_stream,
@@ -2465,20 +2472,28 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
   RecreateWebRtcStream();
 }
 
-void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
-    bool nack_enabled, bool remb_enabled) {
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
+    bool nack_enabled,
+    bool remb_enabled,
+    bool transport_cc_enabled) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
   if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
-      config_.rtp.remb == remb_enabled) {
-    LOG(LS_INFO) << "Ignoring call to SetNackAndRemb because parameters are "
-                    "unchanged; nack=" << nack_enabled
-                 << ", remb=" << remb_enabled;
+      config_.rtp.remb == remb_enabled &&
+      config_.rtp.transport_cc == transport_cc_enabled) {
+    LOG(LS_INFO)
+        << "Ignoring call to SetFeedbackParameters because parameters are "
+           "unchanged; nack="
+        << nack_enabled << ", remb=" << remb_enabled
+        << ", transport_cc=" << transport_cc_enabled;
     return;
   }
   config_.rtp.remb = remb_enabled;
   config_.rtp.nack.rtp_history_ms = nack_history_ms;
-  LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetNackAndRemb; nack="
-               << nack_enabled << ", remb=" << remb_enabled;
+  config_.rtp.transport_cc = transport_cc_enabled;
+  LOG(LS_INFO)
+      << "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
+      << nack_enabled << ", remb=" << remb_enabled
+      << ", transport_cc=" << transport_cc_enabled;
   RecreateWebRtcStream();
 }
 
diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h
index 24033a1603..85e79281f8 100644
--- a/talk/media/webrtc/webrtcvideoengine2.h
+++ b/talk/media/webrtc/webrtcvideoengine2.h
@@ -400,7 +400,9 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     const std::vector<uint32_t>& GetSsrcs() const;
 
     void SetLocalSsrc(uint32_t local_ssrc);
-    void SetNackAndRemb(bool nack_enabled, bool remb_enabled);
+    void SetFeedbackParameters(bool nack_enabled,
+                               bool remb_enabled,
+                               bool transport_cc_enabled);
     void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
     void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);
 
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index 87a0ac81a9..832c4d0623 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -75,6 +75,8 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) {
   EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
       cricket::kRtcpFbParamRemb, cricket::kParamValueEmpty)));
   EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
+      cricket::kRtcpFbParamTransportCc, cricket::kParamValueEmpty)));
+  EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
       cricket::kRtcpFbParamCcm, cricket::kRtcpFbCcmParamFir)));
 }
 
@@ -1451,6 +1453,11 @@ TEST_F(WebRtcVideoChannel2Test, RembIsEnabledByDefault) {
   EXPECT_TRUE(stream->GetConfig().rtp.remb);
 }
 
+TEST_F(WebRtcVideoChannel2Test, TransportCcIsEnabledByDefault) {
+  FakeVideoReceiveStream* stream = AddRecvStream();
+  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+}
+
 TEST_F(WebRtcVideoChannel2Test, RembCanBeEnabledAndDisabled) {
   FakeVideoReceiveStream* stream = AddRecvStream();
   EXPECT_TRUE(stream->GetConfig().rtp.remb);
@@ -1471,6 +1478,27 @@ TEST_F(WebRtcVideoChannel2Test, RembCanBeEnabledAndDisabled) {
   EXPECT_TRUE(stream->GetConfig().rtp.remb);
 }
 
+TEST_F(WebRtcVideoChannel2Test, TransportCcCanBeEnabledAndDisabled) {
+  FakeVideoReceiveStream* stream = AddRecvStream();
+  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+
+  // Verify that transport cc feedback is turned off when send(!) codecs without
+  // transport cc feedback are set.
+  cricket::VideoSendParameters parameters;
+  parameters.codecs.push_back(kVp8Codec);
+  EXPECT_TRUE(parameters.codecs[0].feedback_params.params().empty());
+  EXPECT_TRUE(channel_->SetSendParameters(parameters));
+  stream = fake_call_->GetVideoReceiveStreams()[0];
+  EXPECT_FALSE(stream->GetConfig().rtp.transport_cc);
+
+  // Verify that transport cc feedback is turned on when setting default codecs
+  // since the default codecs have transport cc feedback enabled.
+  parameters.codecs = engine_.codecs();
+  EXPECT_TRUE(channel_->SetSendParameters(parameters));
+  stream = fake_call_->GetVideoReceiveStreams()[0];
+  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+}
+
 TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) {
   VerifyCodecHasDefaultFeedbackParams(default_codec_);
 
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index e2450d9c7d..e541d370d8 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -1533,9 +1533,8 @@ TEST_F(EndToEndTest, AssignsTransportSequenceNumbers) {
   tester.RunTest();
 }
 
-TEST_F(EndToEndTest, ReceivesTransportFeedback) {
+void TransportFeedbackTest(bool feedback_enabled) {
   static const int kExtensionId = 5;
-
   class TransportFeedbackObserver : public test::DirectTransport {
    public:
     TransportFeedbackObserver(Call* receiver_call, rtc::Event* done_event)
@@ -1563,12 +1562,16 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
 
   class TransportFeedbackTester : public MultiStreamTest {
    public:
-    TransportFeedbackTester() : done_(false, false) {}
+    explicit TransportFeedbackTester(bool feedback_enabled)
+        : feedback_enabled_(feedback_enabled), done_(false, false) {}
     virtual ~TransportFeedbackTester() {}
 
    protected:
     void Wait() override {
-      EXPECT_TRUE(done_.Wait(CallTest::kDefaultTimeoutMs));
+      const int64_t kDisabledFeedbackTimeoutMs = 5000;
+      EXPECT_EQ(feedback_enabled_, done_.Wait(feedback_enabled_
+                                  ? test::CallTest::kDefaultTimeoutMs
+                                  : kDisabledFeedbackTimeoutMs));
     }
 
     void UpdateSendConfig(
@@ -1585,6 +1588,7 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
         VideoReceiveStream::Config* receive_config) override {
       receive_config->rtp.extensions.push_back(
           RtpExtension(RtpExtension::kTransportSequenceNumber, kExtensionId));
+      receive_config->rtp.transport_cc = feedback_enabled_;
     }
 
     test::DirectTransport* CreateReceiveTransport(
@@ -1593,10 +1597,20 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
     }
 
    private:
+    const bool feedback_enabled_;
     rtc::Event done_;
-  } tester;
+  } tester(feedback_enabled);
   tester.RunTest();
 }
+
+TEST_F(EndToEndTest, ReceivesTransportFeedback) {
+  TransportFeedbackTest(true);
+}
+
+TEST_F(EndToEndTest, TransportFeedbackNotConfigured) {
+  TransportFeedbackTest(false);
+}
+
 TEST_F(EndToEndTest, ObserversEncodedFrames) {
   class EncodedFrameTestObserver : public EncodedFrameObserver {
    public:
diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc
index 6450e0290d..c9b8e11122 100644
--- a/webrtc/video/rampup_tests.cc
+++ b/webrtc/video/rampup_tests.cc
@@ -124,16 +124,20 @@ void RampUpTester::ModifyConfigs(
   send_config->rtp.extensions.clear();
 
   bool remb;
+  bool transport_cc;
   if (extension_type_ == RtpExtension::kAbsSendTime) {
     remb = true;
+    transport_cc = false;
     send_config->rtp.extensions.push_back(
         RtpExtension(extension_type_.c_str(), kAbsSendTimeExtensionId));
   } else if (extension_type_ == RtpExtension::kTransportSequenceNumber) {
     remb = false;
+    transport_cc = true;
     send_config->rtp.extensions.push_back(RtpExtension(
         extension_type_.c_str(), kTransportSequenceNumberExtensionId));
   } else {
     remb = true;
+    transport_cc = false;
     send_config->rtp.extensions.push_back(RtpExtension(
         extension_type_.c_str(), kTransmissionTimeOffsetExtensionId));
   }
@@ -153,6 +157,7 @@ void RampUpTester::ModifyConfigs(
   size_t i = 0;
   for (VideoReceiveStream::Config& recv_config : *receive_configs) {
     recv_config.rtp.remb = remb;
+    recv_config.rtp.transport_cc = transport_cc;
     recv_config.rtp.extensions = send_config->rtp.extensions;
 
     recv_config.rtp.remote_ssrc = ssrcs_[i];
diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc
index ebceb5642b..a46fe1bd7c 100644
--- a/webrtc/video/video_quality_test.cc
+++ b/webrtc/video/video_quality_test.cc
@@ -828,6 +828,7 @@ void VideoQualityTest::SetupCommon(Transport* send_transport,
     receive_configs_[i].rtp.rtx[kSendRtxPayloadType].ssrc = kSendRtxSsrcs[i];
     receive_configs_[i].rtp.rtx[kSendRtxPayloadType].payload_type =
         kSendRtxPayloadType;
+    receive_configs_[i].rtp.transport_cc = params_.common.send_side_bwe;
   }
 }
 
diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc
index 287f5355dc..683e89aa97 100644
--- a/webrtc/video/video_receive_stream.cc
+++ b/webrtc/video/video_receive_stream.cc
@@ -81,6 +81,7 @@ std::string VideoReceiveStream::Config::Rtp::ToString() const {
      << (rtcp_xr.receiver_reference_time_report ? "on" : "off");
   ss << '}';
   ss << ", remb: " << (remb ? "on" : "off");
+  ss << ", transport_cc: " << (transport_cc ? "on" : "off");
   ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}';
   ss << ", fec: " << fec.ToString();
   ss << ", rtx: {";
@@ -153,7 +154,8 @@ VideoReceiveStream::VideoReceiveStream(
       call_stats_(call_stats) {
   LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
 
-  bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions);
+  bool send_side_bwe =
+      config.rtp.transport_cc && UseSendSideBwe(config_.rtp.extensions);
 
   RemoteBitrateEstimator* bitrate_estimator =
       congestion_controller_->GetRemoteBitrateEstimator(send_side_bwe);
diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h
index 275162ca1c..6be2c5a86e 100644
--- a/webrtc/video_receive_stream.h
+++ b/webrtc/video_receive_stream.h
@@ -113,6 +113,9 @@ class VideoReceiveStream : public ReceiveStream {
       // See draft-alvestrand-rmcat-remb for information.
       bool remb = false;
 
+      // See draft-holmer-rmcat-transport-wide-cc-extensions for details.
+      bool transport_cc = false;
+
       // See NackConfig for description.
       NackConfig nack;