diff --git a/api/jsep.cc b/api/jsep.cc index ddb39b6181..01f5720563 100644 --- a/api/jsep.cc +++ b/api/jsep.cc @@ -41,7 +41,6 @@ void SetSessionDescriptionObserver::OnFailure(const std::string& error) { const char SessionDescriptionInterface::kOffer[] = "offer"; const char SessionDescriptionInterface::kPrAnswer[] = "pranswer"; const char SessionDescriptionInterface::kAnswer[] = "answer"; -const char SessionDescriptionInterface::kRollback[] = "rollback"; const char* SdpTypeToString(SdpType type) { switch (type) { @@ -51,8 +50,6 @@ const char* SdpTypeToString(SdpType type) { return SessionDescriptionInterface::kPrAnswer; case SdpType::kAnswer: return SessionDescriptionInterface::kAnswer; - case SdpType::kRollback: - return SessionDescriptionInterface::kRollback; } return ""; } @@ -64,8 +61,6 @@ absl::optional SdpTypeFromString(const std::string& type_str) { return SdpType::kPrAnswer; } else if (type_str == SessionDescriptionInterface::kAnswer) { return SdpType::kAnswer; - } else if (type_str == SessionDescriptionInterface::kRollback) { - return SdpType::kRollback; } else { return absl::nullopt; } diff --git a/api/jsep.h b/api/jsep.h index 3f7f12a45d..6da782748d 100644 --- a/api/jsep.h +++ b/api/jsep.h @@ -103,11 +103,8 @@ enum class SdpType { kOffer, // Description must be treated as an SDP offer. kPrAnswer, // Description must be treated as an SDP answer, but not a final // answer. - kAnswer, // Description must be treated as an SDP final answer, and the - // offer-answer exchange must be considered complete after - // receiving this. - kRollback // Resets any pending offers and sets signaling state back to - // stable. + kAnswer // Description must be treated as an SDP final answer, and the offer- + // answer exchange must be considered complete after receiving this. }; // Returns the string form of the given SDP type. String forms are defined in @@ -131,7 +128,6 @@ class RTC_EXPORT SessionDescriptionInterface { static const char kOffer[]; static const char kPrAnswer[]; static const char kAnswer[]; - static const char kRollback[]; virtual ~SessionDescriptionInterface() {} diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index f526c37f42..a4176418a2 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -659,9 +659,6 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // logs with TURN server logs. It will not be added if it's an empty string. std::string turn_logging_id; - // Added to be able to control rollout of this feature. - bool enable_implicit_rollback = false; - // // Don't forget to update operator== if adding something. // diff --git a/pc/jsep_session_description.cc b/pc/jsep_session_description.cc index 7f30b50d97..cc544dc5e1 100644 --- a/pc/jsep_session_description.cc +++ b/pc/jsep_session_description.cc @@ -152,10 +152,8 @@ std::unique_ptr CreateSessionDescription( const std::string& sdp, SdpParseError* error_out) { auto jsep_desc = std::make_unique(type); - if (type != SdpType::kRollback) { - if (!SdpDeserialize(sdp, jsep_desc.get(), error_out)) { - return nullptr; - } + if (!SdpDeserialize(sdp, jsep_desc.get(), error_out)) { + return nullptr; } return std::move(jsep_desc); } diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index d83b16e9b3..52ae53c4f7 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -471,16 +471,6 @@ void JsepTransportController::SetMediaTransportSettings( use_datagram_transport_for_data_channels_receive_only; } -void JsepTransportController::RollbackTransportForMid(const std::string& mid) { - if (!network_thread_->IsCurrent()) { - network_thread_->Invoke(RTC_FROM_HERE, - [=] { RollbackTransportForMid(mid); }); - return; - } - RemoveTransportForMid(mid); - MaybeDestroyJsepTransport(mid); -} - std::unique_ptr JsepTransportController::CreateIceTransport(const std::string transport_name, bool rtcp) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index bcaeed539f..a46a71efbb 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -239,10 +239,6 @@ class JsepTransportController : public sigslot::has_slots<> { bool use_datagram_transport_for_data_channels, bool use_datagram_transport_for_data_channels_receive_only); - // TODO(elrello): For now the rollback only removes mid to transport mapping - // and deletes unused transport, but doesn't consider anything more complex. - void RollbackTransportForMid(const std::string& mid); - // If media transport is present enabled and supported, // when this method is called, it creates a media transport and generates its // offer. The new offer is then returned, and the created media transport will diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index c0e1831916..c2723e7f8e 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -782,7 +782,6 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( absl::optional crypto_options; bool offer_extmap_allow_mixed; std::string turn_logging_id; - bool enable_implicit_rollback; }; static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this), "Did you add something to RTCConfiguration and forget to " @@ -848,8 +847,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( o.use_datagram_transport_for_data_channels_receive_only && crypto_options == o.crypto_options && offer_extmap_allow_mixed == o.offer_extmap_allow_mixed && - turn_logging_id == o.turn_logging_id && - enable_implicit_rollback == o.enable_implicit_rollback; + turn_logging_id == o.turn_logging_id; } bool PeerConnectionInterface::RTCConfiguration::operator!=( @@ -2259,23 +2257,6 @@ void PeerConnection::SetLocalDescription( return; } - // For SLD we support only explicit rollback. - if (desc->GetType() == SdpType::kRollback) { - if (IsUnifiedPlan()) { - RTCError error = Rollback(); - if (error.ok()) { - PostSetSessionDescriptionSuccess(observer); - } else { - PostSetSessionDescriptionFailure(observer, std::move(error)); - } - } else { - PostSetSessionDescriptionFailure( - observer, RTCError(RTCErrorType::UNSUPPORTED_OPERATION, - "Rollback not supported in Plan B")); - } - return; - } - RTCError error = ValidateSessionDescription(desc.get(), cricket::CS_LOCAL); if (!error.ok()) { std::string error_message = GetSetDescriptionErrorMessage( @@ -2648,24 +2629,7 @@ void PeerConnection::SetRemoteDescription( RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error_message))); return; } - if (IsUnifiedPlan()) { - if (configuration_.enable_implicit_rollback) { - if (desc->GetType() == SdpType::kOffer && - signaling_state() == kHaveLocalOffer) { - Rollback(); - } - } - // Explicit rollback. - if (desc->GetType() == SdpType::kRollback) { - observer->OnSetRemoteDescriptionComplete(Rollback()); - return; - } - } else if (desc->GetType() == SdpType::kRollback) { - observer->OnSetRemoteDescriptionComplete( - RTCError(RTCErrorType::UNSUPPORTED_OPERATION, - "Rollback not supported in Plan B")); - return; - } + if (desc->GetType() == SdpType::kOffer) { // Report to UMA the format of the received offer. ReportSdpFormatReceived(*desc); @@ -3418,12 +3382,8 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, transceiver = CreateAndAddTransceiver(sender, receiver); transceiver->internal()->set_direction( RtpTransceiverDirection::kRecvOnly); - if (type == SdpType::kOffer) { - transceiver_stable_states_by_transceivers_[transceiver] = - TransceiverStableState(RtpTransceiverDirection::kRecvOnly, - absl::nullopt, absl::nullopt, true); - } } + // Check if the offer indicated simulcast but the answer rejected it. // This can happen when simulcast is not supported on the remote party. if (SimulcastIsRejected(old_local_content, *media_desc)) { @@ -3456,20 +3416,6 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, return std::move(error); } } - if (type == SdpType::kOffer) { - // Make sure we don't overwrite existing stable states and that the - // state is really going to change when adding new record to the map. - auto it = transceiver_stable_states_by_transceivers_.find(transceiver); - if (it == transceiver_stable_states_by_transceivers_.end() && - (transceiver->internal()->mid() != content.name || - transceiver->internal()->mline_index() != mline_index)) { - transceiver_stable_states_by_transceivers_[transceiver] = - TransceiverStableState(transceiver->internal()->direction(), - transceiver->internal()->mid(), - transceiver->internal()->mline_index(), false); - } - } - // Associate the found or created RtpTransceiver with the m= section by // setting the value of the RtpTransceiver's mid property to the MID of the m= // section, and establish a mapping between the transceiver and the index of @@ -5891,7 +5837,6 @@ RTCError PeerConnection::UpdateSessionState( } else { RTC_DCHECK(type == SdpType::kAnswer); ChangeSignalingState(PeerConnectionInterface::kStable); - transceiver_stable_states_by_transceivers_.clear(); } // Update internal objects according to the session description's media @@ -7605,51 +7550,4 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() { return false; } -RTCError PeerConnection::Rollback() { - auto state = signaling_state(); - if (state != PeerConnectionInterface::kHaveLocalOffer && - state != PeerConnectionInterface::kHaveRemoteOffer) { - return RTCError(RTCErrorType::INVALID_STATE, - "Called in wrong signalingState: " + - GetSignalingStateString(signaling_state())); - } - RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK(IsUnifiedPlan()); - - for (auto&& transceivers_stable_state_pair : - transceiver_stable_states_by_transceivers_) { - auto transceiver = transceivers_stable_state_pair.first; - auto state = transceivers_stable_state_pair.second; - RTC_DCHECK(transceiver->internal()->mid().has_value()); - std::string mid = transceiver->internal()->mid().value(); - transport_controller_->RollbackTransportForMid(mid); - DestroyTransceiverChannel(transceiver); - - if (state.newly_created()) { - // Remove added transceivers with no added track. - if (transceiver->internal()->sender()->track()) { - transceiver->internal()->set_created_by_addtrack(true); - } else { - int remaining_transceiver_count = 0; - for (auto&& t : transceivers_) { - if (t != transceiver) { - transceivers_[remaining_transceiver_count++] = t; - } - } - transceivers_.resize(remaining_transceiver_count); - } - } - transceiver->internal()->sender_internal()->set_transport(nullptr); - transceiver->internal()->receiver_internal()->set_transport(nullptr); - transceiver->internal()->set_direction(state.direction()); - transceiver->internal()->set_mid(state.mid()); - transceiver->internal()->set_mline_index(state.mline_index()); - } - transceiver_stable_states_by_transceivers_.clear(); - pending_local_description_.reset(); - pending_remote_description_.reset(); - ChangeSignalingState(PeerConnectionInterface::kStable); - return RTCError::OK(); -} - } // namespace webrtc diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 393a1ddd91..c783ae9e21 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -391,34 +391,6 @@ class PeerConnection : public PeerConnectionInternal, FieldTrialFlag receive_only; }; - // Captures partial state to be used for rollback. Applicable only in - // Unified Plan. - class TransceiverStableState { - public: - TransceiverStableState() {} - TransceiverStableState(RtpTransceiverDirection direction, - absl::optional mid, - absl::optional mline_index, - bool newly_created) - : direction_(direction), - mid_(mid), - mline_index_(mline_index), - newly_created_(newly_created) {} - RtpTransceiverDirection direction() const { return direction_; } - absl::optional mid() const { return mid_; } - absl::optional mline_index() const { return mline_index_; } - bool newly_created() const { return newly_created_; } - - private: - RtpTransceiverDirection direction_ = RtpTransceiverDirection::kRecvOnly; - absl::optional mid_; - absl::optional mline_index_; - // Indicates that the transceiver was created as part of applying a - // description to track potential need for removing transceiver during - // rollback. - bool newly_created_ = false; - }; - // Implements MessageHandler. void OnMessage(rtc::Message* msg) override; @@ -1193,7 +1165,6 @@ class PeerConnection : public PeerConnectionInternal, void UpdateNegotiationNeeded(); bool CheckIfNegotiationIsNeeded(); - RTCError Rollback(); sigslot::signal1 SignalDataChannelCreated_ RTC_GUARDED_BY(signaling_thread()); @@ -1315,11 +1286,7 @@ class PeerConnection : public PeerConnectionInternal, RTC_GUARDED_BY(signaling_thread()); // A pointer is passed to senders_ rtc::scoped_refptr stats_collector_ RTC_GUARDED_BY(signaling_thread()); - // Holds changes made to transceivers during applying descriptors for - // potential rollback. Gets cleared once signaling state goes to stable. - std::map>, - TransceiverStableState> - transceiver_stable_states_by_transceivers_; + std::vector< rtc::scoped_refptr>> transceivers_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index b06091b3d9..3a0ef0f9be 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -230,7 +230,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, // will set the whole offer/answer exchange in motion. Just need to wait for // the signaling state to reach "stable". void CreateAndSetAndSignalOffer() { - auto offer = CreateOfferAndWait(); + auto offer = CreateOffer(); ASSERT_NE(nullptr, offer); EXPECT_TRUE(SetLocalDescriptionAndSendSdpMessage(std::move(offer))); } @@ -302,13 +302,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, return ice_candidate_pair_change_history_; } - // Every PeerConnection signaling state in order that has been seen by the - // observer. - std::vector - peer_connection_signaling_state_history() const { - return peer_connection_signaling_state_history_; - } - void AddAudioVideoTracks() { AddAudioTrack(); AddVideoTrack(); @@ -584,14 +577,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, network_manager()->set_mdns_responder(std::move(mdns_responder)); } - // Returns null on failure. - std::unique_ptr CreateOfferAndWait() { - rtc::scoped_refptr observer( - new rtc::RefCountedObject()); - pc()->CreateOffer(observer, offer_answer_options_); - return WaitForDescriptionFromObserver(observer); - } - private: explicit PeerConnectionWrapper(const std::string& debug_name) : debug_name_(debug_name) {} @@ -746,6 +731,14 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, ResetRtpReceiverObservers(); } + // Returns null on failure. + std::unique_ptr CreateOffer() { + rtc::scoped_refptr observer( + new rtc::RefCountedObject()); + pc()->CreateOffer(observer, offer_answer_options_); + return WaitForDescriptionFromObserver(observer); + } + // Returns null on failure. std::unique_ptr CreateAnswer() { rtc::scoped_refptr observer( @@ -901,7 +894,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, void OnSignalingChange( webrtc::PeerConnectionInterface::SignalingState new_state) override { EXPECT_EQ(pc()->signaling_state(), new_state); - peer_connection_signaling_state_history_.push_back(new_state); } void OnAddTrack(rtc::scoped_refptr receiver, const std::vector>& @@ -1045,8 +1037,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, ice_gathering_state_history_; std::vector ice_candidate_pair_change_history_; - std::vector - peer_connection_signaling_state_history_; + webrtc::FakeRtcEventLogFactory* event_log_factory_; rtc::AsyncInvoker invoker_; @@ -6000,61 +5991,6 @@ TEST_P(PeerConnectionIntegrationTest, OnIceCandidateError) { caller()->error_event().host_candidate.find(":")); } -TEST_F(PeerConnectionIntegrationTestUnifiedPlan, - AudioKeepsFlowingAfterImplicitRollback) { - PeerConnectionInterface::RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config)); - ConnectFakeSignaling(); - caller()->AddAudioTrack(); - callee()->AddAudioTrack(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalAudio(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); - SetSignalIceCandidates(false); // Workaround candidate outrace sdp. - caller()->AddVideoTrack(); - callee()->AddVideoTrack(); - rtc::scoped_refptr observer( - new rtc::RefCountedObject()); - callee()->pc()->SetLocalDescription(observer, - callee()->CreateOfferAndWait().release()); - EXPECT_TRUE_WAIT(observer->called(), kDefaultTimeout); - caller()->CreateAndSetAndSignalOffer(); // Implicit rollback. - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); -} - -TEST_F(PeerConnectionIntegrationTestUnifiedPlan, - ImplicitRollbackVisitsStableState) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - - ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config)); - - rtc::scoped_refptr sld_observer( - new rtc::RefCountedObject()); - callee()->pc()->SetLocalDescription(sld_observer, - callee()->CreateOfferAndWait().release()); - EXPECT_TRUE_WAIT(sld_observer->called(), kDefaultTimeout); - EXPECT_EQ(sld_observer->error(), ""); - - rtc::scoped_refptr srd_observer( - new rtc::RefCountedObject()); - callee()->pc()->SetRemoteDescription( - srd_observer, caller()->CreateOfferAndWait().release()); - EXPECT_TRUE_WAIT(srd_observer->called(), kDefaultTimeout); - EXPECT_EQ(srd_observer->error(), ""); - - EXPECT_THAT(callee()->peer_connection_signaling_state_history(), - ElementsAre(PeerConnectionInterface::kHaveLocalOffer, - PeerConnectionInterface::kStable, - PeerConnectionInterface::kHaveRemoteOffer)); -} - INSTANTIATE_TEST_SUITE_P(PeerConnectionIntegrationTest, PeerConnectionIntegrationTest, Values(SdpSemantics::kPlanB, diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc index 514374bbff..1fe8d074f5 100644 --- a/pc/peer_connection_jsep_unittest.cc +++ b/pc/peer_connection_jsep_unittest.cc @@ -1727,220 +1727,4 @@ TEST_F(PeerConnectionJsepTest, SetLocalDescriptionFailsMissingMid) { error); } -TEST_F(PeerConnectionJsepTest, RollbackSupportedInUnifiedPlan) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - auto caller = CreatePeerConnection(config); - auto callee = CreatePeerConnection(config); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->SetLocalDescription(caller->CreateRollback())); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->SetRemoteDescription(caller->CreateRollback())); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->SetRemoteDescription(callee->CreateOffer())); -} - -TEST_F(PeerConnectionJsepTest, RollbackNotSupportedInPlanB) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kPlanB; - config.enable_implicit_rollback = true; - auto caller = CreatePeerConnection(config); - auto callee = CreatePeerConnection(config); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_FALSE(caller->SetLocalDescription(caller->CreateRollback())); - EXPECT_FALSE(caller->SetRemoteDescription(caller->CreateRollback())); - EXPECT_FALSE(caller->SetRemoteDescription(callee->CreateOffer())); -} - -TEST_F(PeerConnectionJsepTest, RollbackFailsInStableState) { - auto caller = CreatePeerConnection(); - EXPECT_FALSE(caller->SetLocalDescription(caller->CreateRollback())); - EXPECT_FALSE(caller->SetRemoteDescription(caller->CreateRollback())); -} - -TEST_F(PeerConnectionJsepTest, RollbackToStableStateAndClearLocalOffer) { - auto caller = CreatePeerConnection(); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->SetLocalDescription(caller->CreateRollback())); - EXPECT_EQ(caller->signaling_state(), PeerConnectionInterface::kStable); - EXPECT_EQ(caller->pc()->pending_local_description(), nullptr); - - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(caller->signaling_state(), PeerConnectionInterface::kStable); - EXPECT_EQ(caller->pc()->pending_local_description(), nullptr); -} - -TEST_F(PeerConnectionJsepTest, RollbackToStableStateAndClearRemoteOffer) { - auto caller = CreatePeerConnection(); - auto callee = CreatePeerConnection(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(callee->signaling_state(), PeerConnectionInterface::kStable); - EXPECT_EQ(callee->pc()->pending_remote_description(), nullptr); - - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_TRUE(callee->SetLocalDescription(caller->CreateRollback())); - EXPECT_EQ(callee->signaling_state(), PeerConnectionInterface::kStable); - EXPECT_EQ(callee->pc()->pending_remote_description(), nullptr); -} - -TEST_F(PeerConnectionJsepTest, RollbackLocalOfferImplicitly) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - auto caller = CreatePeerConnection(config); - auto callee = CreatePeerConnection(config); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->signaling_state(), - PeerConnectionInterface::kHaveRemoteOffer); -} - -TEST_F(PeerConnectionJsepTest, AttemptToRollbackLocalOfferImplicitly) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - auto caller = CreatePeerConnection(config); - auto callee = CreatePeerConnection(config); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - EXPECT_FALSE(callee->SetRemoteDescription( - CreateSessionDescription(SdpType::kOffer, "invalid sdp"))); - EXPECT_EQ(callee->signaling_state(), - PeerConnectionInterface::kHaveLocalOffer); -} - -TEST_F(PeerConnectionJsepTest, RollbackRemovesTransceiver) { - auto caller = CreatePeerConnection(); - caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto callee = CreatePeerConnection(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{0}); -} - -TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) { - auto caller = CreatePeerConnection(); - caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto callee = CreatePeerConnection(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - callee->AddAudioTrack("a"); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - // Transceiver can't be removed as track was added to it. - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); - // Mid got cleared to make it reusable. - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt); - // Transceiver should be counted as addTrack-created after rollback. - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); -} - -TEST_F(PeerConnectionJsepTest, RollbackRestoresMid) { - auto caller = CreatePeerConnection(); - caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto callee = CreatePeerConnection(); - callee->AddAudioTrack("a"); - auto offer = callee->CreateOffer(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); - EXPECT_NE(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt); - EXPECT_TRUE(callee->SetLocalDescription(std::move(offer))); -} - -TEST_F(PeerConnectionJsepTest, RollbackRestoresMidAndRemovesTransceiver) { - auto callee = CreatePeerConnection(); - callee->AddVideoTrack("a"); - auto offer = callee->CreateOffer(); - auto caller = CreatePeerConnection(); - caller->AddAudioTrack("b"); - caller->AddVideoTrack("c"); - auto mid = callee->pc()->GetTransceivers()[0]->mid(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2}); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1}); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), mid); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->media_type(), - cricket::MEDIA_TYPE_VIDEO); - EXPECT_TRUE(callee->SetLocalDescription(std::move(offer))); -} - -TEST_F(PeerConnectionJsepTest, ImplicitlyRollbackTransceiversWithSameMids) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.enable_implicit_rollback = true; - auto caller = CreatePeerConnection(config); - caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); - auto callee = CreatePeerConnection(config); - callee->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - auto initial_mid = callee->pc()->GetTransceivers()[0]->mid(); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2}); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt); - EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(), - caller->pc()->GetTransceivers()[0]->mid()); - EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal()); // Go to stable. - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - EXPECT_NE(callee->pc()->GetTransceivers()[0]->mid(), initial_mid); -} - -TEST_F(PeerConnectionJsepTest, RollbackToNegotiatedStableState) { - RTCConfiguration config; - config.sdp_semantics = SdpSemantics::kUnifiedPlan; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; - auto caller = CreatePeerConnection(config); - caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto callee = CreatePeerConnection(config); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal()); - caller->AddVideoTrack("a"); - callee->AddVideoTrack("b"); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2}); - auto audio_transport = - callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(), - callee->pc()->GetTransceivers()[1]->sender()->dtls_transport()); - EXPECT_NE(callee->pc()->GetTransceivers()[1]->sender()->dtls_transport(), - nullptr); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback())); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(), - audio_transport); // Audio must remain working after rollback. - EXPECT_EQ(callee->pc()->GetTransceivers()[1]->sender()->dtls_transport(), - nullptr); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(), - audio_transport); // Audio transport is still the same. -} - -TEST_F(PeerConnectionJsepTest, RollbackAfterMultipleSLD) { - auto callee = CreatePeerConnection(); - callee->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - callee->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); - EXPECT_TRUE(callee->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(callee->SetRemoteDescription(callee->CreateRollback())); - EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2}); - EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt); - EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(), absl::nullopt); -} - -TEST_F(PeerConnectionJsepTest, NoRollbackNeeded) { - auto caller = CreatePeerConnection(); - caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto callee = CreatePeerConnection(); - callee->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(caller->CreateOfferAndSetAsLocal()); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); - EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer())); -} - } // namespace webrtc diff --git a/pc/peer_connection_wrapper.cc b/pc/peer_connection_wrapper.cc index 7c0b3391d0..b4b07823f7 100644 --- a/pc/peer_connection_wrapper.cc +++ b/pc/peer_connection_wrapper.cc @@ -125,11 +125,6 @@ PeerConnectionWrapper::CreateAnswerAndSetAsLocal( return answer; } -std::unique_ptr -PeerConnectionWrapper::CreateRollback() { - return CreateSessionDescription(SdpType::kRollback, ""); -} - std::unique_ptr PeerConnectionWrapper::CreateSdp( rtc::FunctionView fn, std::string* error_out) { diff --git a/pc/peer_connection_wrapper.h b/pc/peer_connection_wrapper.h index 4d2bc284a7..fafee24b6f 100644 --- a/pc/peer_connection_wrapper.h +++ b/pc/peer_connection_wrapper.h @@ -87,7 +87,6 @@ class PeerConnectionWrapper { const PeerConnectionInterface::RTCOfferAnswerOptions& options); // Calls CreateAnswerAndSetAsLocal with default options. std::unique_ptr CreateAnswerAndSetAsLocal(); - std::unique_ptr CreateRollback(); // Calls the underlying PeerConnection's SetLocalDescription method with the // given session description and waits for the success/failure response. diff --git a/pc/sdp_utils.cc b/pc/sdp_utils.cc index f5385a6529..5bfdaa4bcb 100644 --- a/pc/sdp_utils.cc +++ b/pc/sdp_utils.cc @@ -29,10 +29,8 @@ std::unique_ptr CloneSessionDescriptionAsType( SdpType type) { RTC_DCHECK(sdesc); auto clone = std::make_unique(type); - if (sdesc->description()) { - clone->Initialize(sdesc->description()->Clone(), sdesc->session_id(), - sdesc->session_version()); - } + clone->Initialize(sdesc->description()->Clone(), sdesc->session_id(), + sdesc->session_version()); // As of writing, our version of GCC does not allow returning a unique_ptr of // a subclass as a unique_ptr of a base class. To get around this, we need to // std::move the return value.