diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 9c4461d9ec..6a5f925732 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -3060,6 +3060,18 @@ static RTCError UpdateSimulcastLayerStatusInSender( return result; } +static bool SimulcastIsRejected( + const ContentInfo* local_content, + const MediaContentDescription& answer_media_desc) { + bool simulcast_offered = local_content && + local_content->media_description() && + local_content->media_description()->HasSimulcast(); + bool simulcast_answered = answer_media_desc.HasSimulcast(); + bool rids_supported = RtpExtension::FindHeaderExtensionByUri( + answer_media_desc.rtp_header_extensions(), RtpExtension::kRidUri); + return simulcast_offered && (!simulcast_answered || !rids_supported); +} + static RTCError DisableSimulcastInSender( rtc::scoped_refptr sender) { RTC_DCHECK(sender); @@ -3155,12 +3167,7 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, // Check if the offer indicated simulcast but the answer rejected it. // This can happen when simulcast is not supported on the remote party. - // This check can be simplified to comparing the number of send encodings, - // but that might break legacy implementation in which simulcast is not - // signaled in the remote description. - if (old_local_content && old_local_content->media_description() && - old_local_content->media_description()->HasSimulcast() && - !media_desc->HasSimulcast()) { + if (SimulcastIsRejected(old_local_content, *media_desc)) { RTCError error = DisableSimulcastInSender(transceiver->internal()->sender_internal()); if (!error.ok()) { diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 07e079a2a3..6fa4bae403 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -113,7 +113,7 @@ class PeerConnectionSimulcastTests : public testing::Test { const std::vector& answer_layers) { auto offer = local->CreateOfferAndSetAsLocal(); // Remove simulcast as the second peer connection won't support it. - auto removed_simulcast = RemoveSimulcast(offer.get()); + RemoveSimulcast(offer.get()); std::string err; EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &err)) << err; auto answer = remote->CreateAnswerAndSetAsLocal(); @@ -461,4 +461,38 @@ TEST_F(PeerConnectionSimulcastTests, EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); } +TEST_F(PeerConnectionSimulcastTests, NegotiationDoesNotHaveRidExtension) { + auto local = CreatePeerConnectionWrapper(); + auto remote = CreatePeerConnectionWrapper(); + auto layers = CreateLayers({"1", "2", "3"}, true); + auto expected_layers = CreateLayers({"1"}, true); + auto transceiver = AddTransceiver(local.get(), layers); + auto offer = local->CreateOfferAndSetAsLocal(); + // Remove simulcast as the second peer connection won't support it. + RemoveSimulcast(offer.get()); + std::string err; + EXPECT_TRUE(remote->SetRemoteDescription(std::move(offer), &err)) << err; + auto answer = remote->CreateAnswerAndSetAsLocal(); + // Setup the answer to look like a server response. + // Drop the RID header extension. + auto mcd_answer = answer->description()->contents()[0].media_description(); + auto& receive_layers = mcd_answer->simulcast_description().receive_layers(); + for (const SimulcastLayer& layer : layers) { + receive_layers.AddLayer(layer); + } + cricket::RtpHeaderExtensions extensions; + for (auto extension : mcd_answer->rtp_header_extensions()) { + if (extension.uri != RtpExtension::kRidUri) { + extensions.push_back(extension); + } + } + mcd_answer->set_rtp_header_extensions(extensions); + EXPECT_EQ(layers.size(), mcd_answer->simulcast_description() + .receive_layers() + .GetAllLayers() + .size()); + EXPECT_TRUE(local->SetRemoteDescription(std::move(answer), &err)) << err; + EXPECT_TRUE(ValidateTransceiverParameters(transceiver, expected_layers)); +} + } // namespace webrtc