commit 815522782a92e168b80edc760b2e53e4d0e4ea0d
author Philipp Hancke <phancke@microsoft.com> Tue Jan 17 14:13:59 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jan 30 10:20:50 2023
tree 76159a771d29a9bbacfe2509f0ef76997e6491c6
parent 68564bb59cf3792ea9009be0e124e6c36d4488dc

sdp: add rtcp-fb:* lines for common feedback

which potentially allows switching to that pattern in the future.
Video FEC mechanisms (ulpfec, flexfec-03, RED) that currently
do not have any feedback parameters but will still be considered "common" and feedback may be sent for them.

For audio this causes rtcp-feedback to be sent for G711 and G722 if negotiated.

BUG=webrtc:14802

Change-Id: I54852d39e176f918d4b36462526ceb40617b8fbe
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290702
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39224}

pc/webrtc_sdp.cc

diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 3b34746..7a63ae4 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -1890,7 +1890,7 @@
 
 template <class T>
 void AddRtcpFbLines(const T& codec, std::string* message) {
-  for (const cricket::FeedbackParam& param : codec.feedback_params.params()) {
+  for (const auto& param : codec.feedback_params.params()) {
     rtc::StringBuilder os;
     WriteRtcpFbHeader(codec.id, &os);
     os << " " << param.id();
@@ -1901,6 +1901,30 @@
   }
 }
 
+template <class T>
+void AddWildcardRtcpFbLines(const std::vector<T>& codecs,
+                            std::string* message) {
+  if (codecs.empty()) {
+    return;
+  }
+  for (const auto& param : codecs[0].feedback_params.params()) {
+    bool is_common_feedback = absl::c_all_of(codecs, [&param](const T& codec) {
+      // FEC mechanisms like RED, ulpfec and flexfec have empty feedback.
+      return codec.feedback_params.params().empty() ||
+             codec.feedback_params.Has(param);
+    });
+    if (is_common_feedback) {
+      rtc::StringBuilder os;
+      WriteRtcpFbHeader(kWildcardPayloadType, &os);
+      os << " " << param.id();
+      if (!param.param().empty()) {
+        os << " " << param.param();
+      }
+      AddLine(os.str(), message);
+    }
+  }
+}
+
 bool GetMinValue(const std::vector<int>& values, int* value) {
   if (values.empty()) {
     return false;
@@ -1944,6 +1968,9 @@
       AddRtcpFbLines(codec, message);
       AddFmtpLine(codec, message);
     }
+    // rtcp-fb:* is added in addition to the per-codec feedback to allow
+    // downstream users time to upgrade their parsers.
+    AddWildcardRtcpFbLines(media_desc->as_video()->codecs(), message);
   } else if (media_type == cricket::MEDIA_TYPE_AUDIO) {
     std::vector<int> ptimes;
     std::vector<int> maxptimes;
@@ -1975,6 +2002,9 @@
         maxptimes.push_back(maxptime);
       }
     }
+    // rtcp-fb:* is added in addition to the per-codec feedback to allow
+    // downstream users time to upgrade their parsers.
+    AddWildcardRtcpFbLines(media_desc->as_audio()->codecs(), message);
     // Populate the maxptime attribute with the smallest maxptime of all codecs
     // under the same m-line.
     int min_maxptime = INT_MAX;

pc/webrtc_sdp_unittest.cc

diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 9f1cfc9..3e5e9a0 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -3296,6 +3296,38 @@
   TestSerialize(jdesc_output);
 }
 
+TEST_F(WebRtcSdpTest, SerializeRtcpFbWildcard) {
+  std::string sdp =
+      "v=0\r\n"
+      "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+      "s=-\r\n"
+      "t=0 0\r\n"
+      "m=video 3457 RTP/SAVPF 100 101 102 103\r\n"
+      "a=rtpmap:100 VP8/90000\r\n"
+      "a=rtcp-fb:100 nack\r\n"
+      "a=rtcp-fb:100 nack pli\r\n"
+      "a=rtpmap:101 rtx/90000\r\n"
+      "a=fmtp:101 apt=100\r\n"
+      "a=rtpmap:102 VP9/90000\r\n"
+      "a=rtcp-fb:102 nack\r\n"
+      "a=rtpmap:103 rtx/90000\r\n"
+      "a=fmtp:103 apt=102\r\n";
+  JsepSessionDescription jdesc(kDummyType);
+  SdpParseError error;
+  EXPECT_TRUE(webrtc::SdpDeserialize(sdp, &jdesc, &error));
+
+  std::string message = webrtc::SdpSerialize(jdesc);
+  // NACK is a common parameter so serialized with wildcard in addition to
+  // per-codec entries.
+  EXPECT_NE(message.find("\r\na=rtcp-fb:100 nack\r\n"), std::string::npos);
+  EXPECT_NE(message.find("\r\na=rtcp-fb:102 nack\r\n"), std::string::npos);
+  EXPECT_NE(message.find("\r\na=rtcp-fb:* nack\r\n"), std::string::npos);
+  // PLI is not a common parameter so no wildcard.
+  EXPECT_NE(message.find("\r\na=rtcp-fb:100 nack pli\r\n"), std::string::npos);
+  EXPECT_EQ(message.find("\r\na=rtcp-fb:102 nack pli\r\n"), std::string::npos);
+  EXPECT_EQ(message.find("\r\na=rtcp-fb:* nack pli\r\n"), std::string::npos);
+}
+
 TEST_F(WebRtcSdpTest, DeserializeVideoFmtp) {
   JsepSessionDescription jdesc_output(kDummyType);