diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc index eeb44d82a2..4d6da66943 100644 --- a/pc/peer_connection_rtp_unittest.cc +++ b/pc/peer_connection_rtp_unittest.cc @@ -164,6 +164,28 @@ class PeerConnectionRtpTestUnifiedPlan : public PeerConnectionRtpBaseTest { protected: PeerConnectionRtpTestUnifiedPlan() : PeerConnectionRtpBaseTest(SdpSemantics::kUnifiedPlan) {} + + // Helper to emulate an SFU that rejects an offered media section + // in answer. + bool ExchangeOfferAnswerWhereRemoteStopsTransceiver( + PeerConnectionWrapper* caller, + PeerConnectionWrapper* callee, + size_t mid_to_stop) { + auto offer = caller->CreateOffer(); + caller->SetLocalDescription(CloneSessionDescription(offer.get())); + callee->SetRemoteDescription(std::move(offer)); + EXPECT_LT(mid_to_stop, callee->pc()->GetTransceivers().size()); + // Must use StopInternal in order to do instant reject. + callee->pc()->GetTransceivers()[mid_to_stop]->StopInternal(); + auto answer = callee->CreateAnswer(); + EXPECT_TRUE(answer); + bool set_local_answer = + callee->SetLocalDescription(CloneSessionDescription(answer.get())); + EXPECT_TRUE(set_local_answer); + bool set_remote_answer = caller->SetRemoteDescription(std::move(answer)); + EXPECT_TRUE(set_remote_answer); + return set_remote_answer; + } }; // These tests cover |webrtc::PeerConnectionObserver| callbacks firing upon @@ -1573,6 +1595,42 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, EXPECT_EQ(0U, callee->pc()->GetReceivers().size()); } +TEST_F(PeerConnectionRtpTestUnifiedPlan, + SetLocalDescriptionWorksAfterRepeatedAddRemove) { + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + auto video_track = caller->CreateVideoTrack("v"); + auto track = caller->CreateAudioTrack("a"); + caller->AddTransceiver(video_track); + auto transceiver = caller->AddTransceiver(track); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->pc()->RemoveTrack(transceiver->sender()); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->AddTrack(track); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->pc()->RemoveTrack(transceiver->sender()); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); +} + +// This is a repro of Chromium bug https://crbug.com/1134686 +TEST_F(PeerConnectionRtpTestUnifiedPlan, + SetLocalDescriptionWorksAfterRepeatedAddRemoveWithRemoteReject) { + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + auto video_track = caller->CreateVideoTrack("v"); + auto track = caller->CreateAudioTrack("a"); + caller->AddTransceiver(video_track); + auto transceiver = caller->AddTransceiver(track); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->pc()->RemoveTrack(transceiver->sender()); + ExchangeOfferAnswerWhereRemoteStopsTransceiver(caller.get(), callee.get(), 1); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->AddTrack(track); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + caller->pc()->RemoveTrack(transceiver->sender()); + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); +} + // Test that AddTransceiver fails if trying to use unimplemented RTP encoding // parameters with the send_encodings parameters. TEST_F(PeerConnectionRtpTestUnifiedPlan, diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 5dc5571d55..c6432a6f34 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -2871,17 +2871,16 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiversAndDataChannels( old_remote_content = &old_remote_description->description()->contents()[i]; } - // In the case where an m-section has completed its rejection, - // and is not being reused, we do not expect a transceiver. - if (old_local_content && old_local_content->rejected && - old_remote_content && old_remote_content->rejected && - new_content.rejected) { - continue; - } auto transceiver_or_error = AssociateTransceiver(source, new_session.GetType(), i, new_content, old_local_content, old_remote_content); if (!transceiver_or_error.ok()) { + // In the case where a transceiver is rejected locally, we don't + // expect to find a transceiver, but might find it in the case + // where state is still "stopping", not "stopped". + if (new_content.rejected) { + continue; + } return transceiver_or_error.MoveError(); } auto transceiver = transceiver_or_error.MoveValue(); @@ -2946,8 +2945,9 @@ SdpOfferAnswerHandler::AssociateTransceiver( transceiver = transceivers().FindByMLineIndex(mline_index); } if (!transceiver) { + // This may happen normally when media sections are rejected. LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "Unknown transceiver"); + "Transceiver not found based on m-line index"); } } else { RTC_DCHECK_EQ(source, cricket::CS_REMOTE);