From 1c5808145e8b151800b0320b8a7316a09b706488 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 4 Jul 2022 09:41:24 +0000 Subject: [PATCH] Ignore RID that appears without an a=simulcast entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RID is defined for multiple usages in RFC 8851, but we only support usage with a=simulcast as specified in RFC 8853. Bug: chromium:1341043 Change-Id: Ie72074c5b394bdc41865938a86ec9c7629e1f5e0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267628 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#37417} --- pc/peer_connection_simulcast_unittest.cc | 21 +++++++++++++++++++++ pc/webrtc_sdp.cc | 7 +++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index e704aeba5a..b9681dca34 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -602,6 +602,27 @@ TEST_F(PeerConnectionSimulcastTests, SimulcastAudioRejected) { ElementsAre(Field("rid", &RtpEncodingParameters::rid, Eq("")))); } +// Check that modifying the offer to remove simulcast and at the same +// time leaving in a RID line does not cause an exception. +TEST_F(PeerConnectionSimulcastTests, SimulcastSldModificationRejected) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers({"1", "2", "3"}, true); + AddTransceiver(local.get(), layers); + auto offer = local->CreateOffer(); + std::string as_string; + EXPECT_TRUE(offer->ToString(&as_string)); + auto simulcast_marker = "a=rid:3 send\r\na=simulcast:send 1;2;3\r\n"; + auto pos = as_string.find(simulcast_marker); + EXPECT_NE(pos, std::string::npos); + as_string.erase(pos, strlen(simulcast_marker)); + SdpParseError parse_error; + auto modified_offer = + CreateSessionDescription(SdpType::kOffer, as_string, &parse_error); + EXPECT_TRUE(modified_offer); + EXPECT_TRUE(local->SetLocalDescription(std::move(modified_offer))); +} + #if RTC_METRICS_ENABLED // // Checks the logged metrics when simulcast is not used. diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 8e4552476b..c942689b8f 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -3282,7 +3282,6 @@ bool ParseContent(absl::string_view message, // If simulcast is specifed, split the rids into send and receive. // Rids that do not appear in simulcast attribute will be removed. - // If it is not specified, we assume that all rids are for send layers. std::vector send_rids; std::vector receive_rids; if (!simulcast.empty()) { @@ -3309,7 +3308,11 @@ bool ParseContent(absl::string_view message, media_desc->set_simulcast_description(simulcast); } else { - send_rids = rids; + // RID is specified in RFC 8851, which identifies a lot of usages. + // We only support RFC 8853 usage of RID, not anything else. + // Ignore all RID parameters when a=simulcast is missing. + // In particular do NOT do send_rids = rids; + RTC_LOG(LS_VERBOSE) << "Ignoring send_rids without simulcast"; } media_desc->set_receive_rids(receive_rids);