mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-16 15:20:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Add firing of OnRemoveTrack and OnRenegotationNeeded during rollback

Browse source 
Bug: chromium:980875
Change-Id: I71439cea4c79e4a8dae6488404b0c303a9c33a97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157581
Commit-Queue: Eldar Rello <elrello@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29563}
This commit is contained in:
Eldar Rello 2019-10-21 23:01:31 +03:00 committed by Commit Bot
parent 4b4713db32
commit ead0ec9a20
3 changed files with 153 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
pc/peer_connection.cc
View file

@ -2264,7 +2264,7 @@ void PeerConnection::SetLocalDescription(
// For SLD we support only explicit rollback.
if (desc->GetType() == SdpType::kRollback) {
if (IsUnifiedPlan()) {
RTCError error = Rollback();
RTCError error = Rollback(desc->GetType());
if (error.ok()) {
PostSetSessionDescriptionSuccess(observer);
} else {
@ -2654,12 +2654,12 @@ void PeerConnection::SetRemoteDescription(
if (configuration_.enable_implicit_rollback) {
if (desc->GetType() == SdpType::kOffer &&
signaling_state() == kHaveLocalOffer) {
Rollback();
Rollback(desc->GetType());
}
}
// Explicit rollback.
if (desc->GetType() == SdpType::kRollback) {
observer->OnSetRemoteDescriptionComplete(Rollback());
observer->OnSetRemoteDescriptionComplete(Rollback(desc->GetType()));
return;
}
} else if (desc->GetType() == SdpType::kRollback) {
@ -7610,7 +7610,7 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
return false;
}
RTCError PeerConnection::Rollback() {
RTCError PeerConnection::Rollback(SdpType sdp_type) {
auto state = signaling_state();
if (state != PeerConnectionInterface::kHaveLocalOffer &&
state != PeerConnectionInterface::kHaveRemoteOffer) {
@ -7630,6 +7630,10 @@ RTCError PeerConnection::Rollback() {
transport_controller_->RollbackTransportForMid(mid);
DestroyTransceiverChannel(transceiver);
if (signaling_state() == PeerConnectionInterface::kHaveRemoteOffer &&
transceiver->receiver()) {
Observer()->OnRemoveTrack(transceiver->receiver());
}
if (state.newly_created()) {
// Remove added transceivers with no added track.
if (transceiver->internal()->sender()->track()) {
@ -7654,6 +7658,15 @@ RTCError PeerConnection::Rollback() {
pending_local_description_.reset();
pending_remote_description_.reset();
ChangeSignalingState(PeerConnectionInterface::kStable);
// The assumption is that in case of implicit rollback UpdateNegotiationNeeded
// gets called in SetRemoteDescription.
if (sdp_type == SdpType::kRollback) {
UpdateNegotiationNeeded();
if (is_negotiation_needed_) {
Observer()->OnRenegotiationNeeded();
}
}
return RTCError::OK();
}

4
pc/peer_connection.h
View file

@ -1193,7 +1193,9 @@ class PeerConnection : public PeerConnectionInternal,
void UpdateNegotiationNeeded();
bool CheckIfNegotiationIsNeeded();
RTCError Rollback();
// | sdp_type | is the type of the SDP that caused the rollback.
RTCError Rollback(SdpType sdp_type);
sigslot::signal1<DataChannel*> SignalDataChannelCreated_
RTC_GUARDED_BY(signaling_thread());

146
pc/peer_connection_jsep_unittest.cc
View file

@ -1786,7 +1786,7 @@ TEST_F(PeerConnectionJsepTest, RollbackToStableStateAndClearRemoteOffer) {
EXPECT_EQ(callee->pc()->pending_remote_description(), nullptr);
}
TEST_F(PeerConnectionJsepTest, RollbackLocalOfferImplicitly) {
TEST_F(PeerConnectionJsepTest, RollbackImplicitly) {
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
config.enable_implicit_rollback = true;
@ -1796,9 +1796,48 @@ TEST_F(PeerConnectionJsepTest, RollbackLocalOfferImplicitly) {
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_EQ(callee->signaling_state(),
PeerConnectionInterface::kHaveRemoteOffer);
EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal());
EXPECT_FALSE(callee->observer()->negotiation_needed());
}
TEST_F(PeerConnectionJsepTest, AttemptToRollbackLocalOfferImplicitly) {
TEST_F(PeerConnectionJsepTest, RollbackImplicitlyNegotatiationNotNeeded) {
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
config.enable_implicit_rollback = true;
auto caller = CreatePeerConnection(config);
auto callee = CreatePeerConnection(config);
caller->AddAudioTrack("a");
callee->AddAudioTrack("b");
EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
callee->observer()->clear_negotiation_needed();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_EQ(callee->signaling_state(),
PeerConnectionInterface::kHaveRemoteOffer);
EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal());
// No negotiation needed as track got attached in the answer.
EXPECT_FALSE(callee->observer()->negotiation_needed());
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 0u);
}
TEST_F(PeerConnectionJsepTest, RollbackImplicitlyAndNegotiationNeeded) {
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
config.enable_implicit_rollback = true;
auto caller = CreatePeerConnection(config);
auto callee = CreatePeerConnection(config);
callee->AddAudioTrack("a");
EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
callee->observer()->clear_negotiation_needed();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_EQ(callee->signaling_state(),
PeerConnectionInterface::kHaveRemoteOffer);
EXPECT_FALSE(callee->observer()->negotiation_needed());
EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal());
EXPECT_TRUE(callee->observer()->negotiation_needed());
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 0u);
}
TEST_F(PeerConnectionJsepTest, AttemptToRollbackImplicitly) {
RTCConfiguration config;
config.sdp_semantics = SdpSemantics::kUnifiedPlan;
config.enable_implicit_rollback = true;
@ -1816,9 +1855,10 @@ TEST_F(PeerConnectionJsepTest, RollbackRemovesTransceiver) {
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_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{0});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 0u);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
}
TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) {
@ -1827,15 +1867,16 @@ TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) {
auto callee = CreatePeerConnection();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
callee->AddAudioTrack("a");
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
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});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
// 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});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
}
TEST_F(PeerConnectionJsepTest, RollbackRestoresMid) {
@ -1845,7 +1886,7 @@ TEST_F(PeerConnectionJsepTest, RollbackRestoresMid) {
callee->AddAudioTrack("a");
auto offer = callee->CreateOffer();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_NE(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
@ -1861,13 +1902,38 @@ TEST_F(PeerConnectionJsepTest, RollbackRestoresMidAndRemovesTransceiver) {
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_EQ(callee->pc()->GetTransceivers().size(), 2u);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
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)));
EXPECT_EQ(callee->observer()->remove_track_events_.size(),
callee->observer()->add_track_events_.size());
}
TEST_F(PeerConnectionJsepTest, RollbackHasNoEffectOnStableTransceivers) {
auto callee = CreatePeerConnection();
callee->AddVideoTrack("a");
auto caller = CreatePeerConnection();
caller->AddAudioTrack("b");
caller->AddVideoTrack("c");
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
// In stable don't add or remove anything.
callee->observer()->clear_negotiation_needed();
size_t transceiver_count = callee->pc()->GetTransceivers().size();
auto mid_0 = callee->pc()->GetTransceivers()[0]->mid();
auto mid_1 = callee->pc()->GetTransceivers()[1]->mid();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), transceiver_count);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), mid_0);
EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(), mid_1);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 0u);
EXPECT_FALSE(callee->observer()->negotiation_needed());
}
TEST_F(PeerConnectionJsepTest, ImplicitlyRollbackTransceiversWithSameMids) {
@ -1881,7 +1947,7 @@ TEST_F(PeerConnectionJsepTest, ImplicitlyRollbackTransceiversWithSameMids) {
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().size(), 2u);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(),
caller->pc()->GetTransceivers()[0]->mid());
@ -1902,7 +1968,7 @@ TEST_F(PeerConnectionJsepTest, RollbackToNegotiatedStableState) {
caller->AddVideoTrack("a");
callee->AddVideoTrack("b");
EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 2u);
auto audio_transport =
callee->pc()->GetTransceivers()[0]->sender()->dtls_transport();
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
@ -1920,14 +1986,68 @@ TEST_F(PeerConnectionJsepTest, RollbackToNegotiatedStableState) {
audio_transport); // Audio transport is still the same.
}
TEST_F(PeerConnectionJsepTest, RollbackLocalDirectionChange) {
auto caller = CreatePeerConnection();
caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
auto callee = CreatePeerConnection();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
callee->AddAudioTrack("a");
callee->pc()->GetTransceivers()[0]->SetDirection(
RtpTransceiverDirection::kSendOnly);
EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
auto audio_transport =
callee->pc()->GetTransceivers()[0]->receiver()->dtls_transport();
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->direction(),
RtpTransceiverDirection::kSendOnly);
// One way audio must remain working after rollback as local direction change
// comes in effect after completing full negotiation round.
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->receiver()->dtls_transport(),
audio_transport);
}
TEST_F(PeerConnectionJsepTest, RollbackRemoteDirectionChange) {
auto caller = CreatePeerConnection();
auto caller_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
auto callee = CreatePeerConnection();
callee->AddAudioTrack("a");
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
// In stable make remote audio receive only.
caller_transceiver->SetDirection(RtpTransceiverDirection::kRecvOnly);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
// The direction attribute is not modified by the offer.
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->direction(),
RtpTransceiverDirection::kSendRecv);
auto audio_transport =
callee->pc()->GetTransceivers()[0]->sender()->dtls_transport();
EXPECT_EQ(callee->observer()->add_track_events_.size(), 1u);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 1u);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->direction(),
RtpTransceiverDirection::kSendRecv);
// One way audio must remain working after rollback.
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
audio_transport);
EXPECT_EQ(callee->observer()->remove_track_events_.size(), 1u);
}
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());
callee->observer()->clear_negotiation_needed();
EXPECT_TRUE(callee->SetRemoteDescription(callee->CreateRollback()));
EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
EXPECT_TRUE(callee->observer()->negotiation_needed());
EXPECT_EQ(callee->pc()->GetTransceivers().size(), 2u);
EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(), absl::nullopt);
}