admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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}
This commit is contained in:
Philipp Hancke 2023-01-17 15:13:59 +01:00 committed by WebRTC LUCI CQ
parent 68564bb59c
commit 815522782a
2 changed files with 63 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
pc/webrtc_sdp.cc
View File

@ -1890,7 +1890,7 @@ void AddPacketizationLine(const T& codec, std::string* message) {
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 @@ void AddRtcpFbLines(const T& codec, std::string* message) {
}
}
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 @@ void BuildRtpmap(const MediaContentDescription* media_desc,
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 @@ void BuildRtpmap(const MediaContentDescription* media_desc,
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;

32
pc/webrtc_sdp_unittest.cc
View File

@ -3296,6 +3296,38 @@ TEST_F(WebRtcSdpTest, DeserializeSerializeRtcpFbWildcard) {
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);