Don't use transport-cc if RFC8888 feedback is negotiated.

Bug: webrtc:378698658
Change-Id: I06536445d32577b7b4d24ae7ca529d9b270b34d0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368863
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43435}

media/base/codec.cc

@@ -68,6 +68,15 @@ void FeedbackParams::Add(const FeedbackParam& param) {
   RTC_CHECK(!HasDuplicateEntries());
 }
 
+bool FeedbackParams::Remove(const FeedbackParam& param) {
+  if (!Has(param)) {
+    return false;
+  }
+  params_.erase(std::remove(params_.begin(), params_.end(), param),
+                params_.end());
+  return true;
+}
+
 void FeedbackParams::Intersect(const FeedbackParams& from) {
   std::vector<FeedbackParam>::iterator iter_to = params_.begin();
   while (iter_to != params_.end()) {

media/base/codec.h

@@ -56,6 +56,7 @@ class FeedbackParams {
 
   bool Has(const FeedbackParam& param) const;
   void Add(const FeedbackParam& param);
+  bool Remove(const FeedbackParam& param);
 
   void Intersect(const FeedbackParams& from);
 

pc/congestion_control_integrationtest.cc

@@ -25,6 +25,7 @@ namespace webrtc {
 
 using testing::Eq;
 using testing::HasSubstr;
+using testing::Not;
 
 class PeerConnectionCongestionControlTest
     : public PeerConnectionIntegrationBaseTest {
@@ -65,6 +66,9 @@ TEST_F(PeerConnectionCongestionControlTest, ReceiveOfferSetsCcfbFlag) {
   for (const auto& content : parsed_contents) {
     EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb());
   }
+  // Check that the answer does not contain transport-cc
+  std::string answer_str = absl::StrCat(*caller()->pc()->remote_description());
+  EXPECT_THAT(answer_str, Not(HasSubstr("transport-cc")));
 }
 
 TEST_F(PeerConnectionCongestionControlTest, CcfbGetsUsed) {
@@ -82,6 +86,9 @@ TEST_F(PeerConnectionCongestionControlTest, CcfbGetsUsed) {
   auto pc_internal = caller()->pc_internal();
   EXPECT_TRUE_WAIT(pc_internal->FeedbackAccordingToRfc8888CountForTesting() > 0,
                    kDefaultTimeout);
+  // There should be no transport-cc generated.
+  EXPECT_THAT(pc_internal->FeedbackAccordingToTransportCcCountForTesting(),
+              Eq(0));
 }
 
 TEST_F(PeerConnectionCongestionControlTest, TransportCcGetsUsed) {

pc/media_session.cc

@@ -1542,6 +1542,22 @@ MediaSessionDescriptionFactory::CreateAnswerOrError(
   StreamParamsVec current_streams =
       GetCurrentStreamParams(current_active_contents);
 
+  // Decide what congestion control feedback format we're using.
+  bool has_ack_ccfb = false;
+  if (transport_desc_factory_->trials().IsEnabled(
+          "WebRTC-RFC8888CongestionControlFeedback")) {
+    for (const auto& content : offer->contents()) {
+      if (content.media_description()->rtcp_fb_ack_ccfb()) {
+        has_ack_ccfb = true;
+      } else if (has_ack_ccfb) {
+        RTC_LOG(LS_ERROR)
+            << "Inconsistent rtcp_fb_ack_ccfb marking, ignoring all";
+        has_ack_ccfb = false;
+        break;
+      }
+    }
+  }
+
   // Get list of all possible codecs that respects existing payload type
   // mappings and uses a single payload type space.
   //
@@ -1601,9 +1617,17 @@ MediaSessionDescriptionFactory::CreateAnswerOrError(
         msection_index < current_description->contents().size()) {
       current_content = &current_description->contents()[msection_index];
     }
+    // Don't offer the transport-cc header extension if "ack ccfb" is in use.
+    auto header_extensions_in = media_description_options.header_extensions;
+    if (has_ack_ccfb) {
+      for (auto& option : header_extensions_in) {
+        if (option.uri == webrtc::RtpExtension::kTransportSequenceNumberUri) {
+          option.direction = RtpTransceiverDirection::kStopped;
+        }
+      }
+    }
     RtpHeaderExtensions header_extensions = RtpHeaderExtensionsFromCapabilities(
-        UnstoppedRtpHeaderExtensionCapabilities(
+        UnstoppedRtpHeaderExtensionCapabilities(header_extensions_in));
-            media_description_options.header_extensions));
     RTCError error;
     switch (media_description_options.type) {
       case MEDIA_TYPE_AUDIO:
@@ -2220,7 +2244,16 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer(
   }
   AssignCodecIdsAndLinkRed(pt_suggester_, media_description_options.mid,
                            codecs_to_include);
-
+  // RFC 8888 support. Only answer with "ack ccfb" if offer has it and
+  // experiment is enabled.
+  if (offer_content_description->rtcp_fb_ack_ccfb()) {
+    answer_content->set_rtcp_fb_ack_ccfb(
+        transport_desc_factory_->trials().IsEnabled(
+            "WebRTC-RFC8888CongestionControlFeedback"));
+    for (auto& codec : codecs_to_include) {
+      codec.feedback_params.Remove(FeedbackParam(kRtcpFbParamTransportCc));
+    }
+  }
   if (!SetCodecsInAnswer(offer_content_description, codecs_to_include,
                          media_description_options, session_options,
                          ssrc_generator(), current_streams,
@@ -2229,15 +2262,6 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer(
     LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
                          "Failed to set codecs in answer");
   }
-  // RFC 8888 support. Only answer with "ack ccfb" if offer has it and
-  // experiment is enabled.
-  // TODO: https://issues.webrtc.org/42225697 - disable transport-cc
-  // when ccfb is negotiated.
-  if (offer_content_description->rtcp_fb_ack_ccfb()) {
-    answer_content->set_rtcp_fb_ack_ccfb(
-        transport_desc_factory_->trials().IsEnabled(
-            "WebRTC-RFC8888CongestionControlFeedback"));
-  }
   if (!CreateMediaContentAnswer(
           offer_content_description, media_description_options, session_options,
           filtered_rtp_header_extensions(header_extensions), ssrc_generator(),