From 64a79c71c479230a0d298bb264fd385035c2822e Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 10 May 2021 17:28:37 +0200 Subject: [PATCH] fix payload type collision handling for [35,65] BUG=webrtc:12747 Change-Id: I82a12b9d88197d478c13ec22d16f578ccd0b1e6c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218162 Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/master@{#33973} --- pc/peer_connection_signaling_unittest.cc | 76 ++++++++++++++++++++++++ pc/used_ids.h | 23 +++++-- 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/pc/peer_connection_signaling_unittest.cc b/pc/peer_connection_signaling_unittest.cc index ba7d813f1c..a4f05c14c8 100644 --- a/pc/peer_connection_signaling_unittest.cc +++ b/pc/peer_connection_signaling_unittest.cc @@ -11,6 +11,7 @@ // This file contains tests that check the PeerConnection's signaling state // machine, as well as tests that check basic, media-agnostic aspects of SDP. +#include #include #include @@ -955,6 +956,81 @@ TEST_P(PeerConnectionSignalingTest, ReceiveFlexFec) { EXPECT_TRUE(caller->SetLocalDescription(std::move(answer))); } +TEST_P(PeerConnectionSignalingTest, ReceiveFlexFecReoffer) { + auto caller = CreatePeerConnection(); + + std::string sdp = + "v=0\r\n" + "o=- 8403615332048243445 2 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=group:BUNDLE 0\r\n" + "m=video 9 UDP/TLS/RTP/SAVPF 102 35\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:9 IN IP4 0.0.0.0\r\n" + "a=ice-ufrag:IZeV\r\n" + "a=ice-pwd:uaZhQD4rYM/Tta2qWBT1Bbt4\r\n" + "a=ice-options:trickle\r\n" + "a=fingerprint:sha-256 " + "D8:6C:3D:FA:23:E2:2C:63:11:2D:D0:86:BE:C4:D0:65:F9:42:F7:1C:06:04:27:E6:" + "1C:2C:74:01:8D:50:67:23\r\n" + "a=setup:actpass\r\n" + "a=mid:0\r\n" + "a=sendrecv\r\n" + "a=msid:stream track\r\n" + "a=rtcp-mux\r\n" + "a=rtcp-rsize\r\n" + "a=rtpmap:102 VP8/90000\r\n" + "a=rtcp-fb:102 goog-remb\r\n" + "a=rtcp-fb:102 transport-cc\r\n" + "a=rtcp-fb:102 ccm fir\r\n" + "a=rtcp-fb:102 nack\r\n" + "a=rtcp-fb:102 nack pli\r\n" + "a=rtpmap:35 flexfec-03/90000\r\n" + "a=fmtp:35 repair-window=10000000\r\n" + "a=ssrc-group:FEC-FR 1224551896 1953032773\r\n" + "a=ssrc:1224551896 cname:/exJcmhSLpyu9FgV\r\n" + "a=ssrc:1953032773 cname:/exJcmhSLpyu9FgV\r\n"; + std::unique_ptr remote_description = + webrtc::CreateSessionDescription(SdpType::kOffer, sdp, nullptr); + + EXPECT_TRUE(caller->SetRemoteDescription(std::move(remote_description))); + + auto answer = caller->CreateAnswer(); + ASSERT_EQ(answer->description()->contents().size(), 1u); + ASSERT_NE( + answer->description()->contents()[0].media_description()->as_video(), + nullptr); + auto codecs = answer->description() + ->contents()[0] + .media_description() + ->as_video() + ->codecs(); + ASSERT_EQ(codecs.size(), 2u); + EXPECT_EQ(codecs[1].name, "flexfec-03"); + EXPECT_EQ(codecs[1].id, 35); + + EXPECT_TRUE(caller->SetLocalDescription(std::move(answer))); + + // This generates a collision for AV1 which needs to be remapped. + auto offer = caller->CreateOffer(RTCOfferAnswerOptions()); + auto offer_codecs = offer->description() + ->contents()[0] + .media_description() + ->as_video() + ->codecs(); + auto flexfec_it = std::find_if( + offer_codecs.begin(), offer_codecs.end(), + [](const cricket::Codec& codec) { return codec.name == "flexfec-03"; }); + ASSERT_EQ(flexfec_it->id, 35); + auto av1_it = std::find_if( + offer_codecs.begin(), offer_codecs.end(), + [](const cricket::Codec& codec) { return codec.name == "AV1X"; }); + if (av1_it != offer_codecs.end()) { + ASSERT_NE(av1_it->id, 35); + } +} + INSTANTIATE_TEST_SUITE_P(PeerConnectionSignalingTest, PeerConnectionSignalingTest, Values(SdpSemantics::kPlanB, diff --git a/pc/used_ids.h b/pc/used_ids.h index 78e64caa41..5960197344 100644 --- a/pc/used_ids.h +++ b/pc/used_ids.h @@ -60,7 +60,9 @@ class UsedIds { } protected: - bool IsIdUsed(int new_id) { return id_set_.find(new_id) != id_set_.end(); } + virtual bool IsIdUsed(int new_id) { + return id_set_.find(new_id) != id_set_.end(); + } const int min_allowed_id_; const int max_allowed_id_; @@ -92,11 +94,24 @@ class UsedIds { class UsedPayloadTypes : public UsedIds { public: UsedPayloadTypes() - : UsedIds(kDynamicPayloadTypeMin, kDynamicPayloadTypeMax) {} + : UsedIds(kFirstDynamicPayloadTypeLowerRange, + kLastDynamicPayloadTypeUpperRange) {} + + protected: + bool IsIdUsed(int new_id) override { + // Range marked for RTCP avoidance is "used". + if (new_id > kLastDynamicPayloadTypeLowerRange && + new_id < kFirstDynamicPayloadTypeUpperRange) + return true; + return UsedIds::IsIdUsed(new_id); + } private: - static const int kDynamicPayloadTypeMin = 96; - static const int kDynamicPayloadTypeMax = 127; + static const int kFirstDynamicPayloadTypeLowerRange = 35; + static const int kLastDynamicPayloadTypeLowerRange = 65; + + static const int kFirstDynamicPayloadTypeUpperRange = 96; + static const int kLastDynamicPayloadTypeUpperRange = 127; }; // Helper class used for finding duplicate RTP Header extension ids among