admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove stopped transceivers at both local and remote SetDescription

Browse Source 
This should ensure that the correct number of senders and receivers
are shown.

Bug: webtc:11840
Change-Id: Id57f8f9b1ceb8900abb3f92bcae79e5f0341de15
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184606
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32158}
This commit is contained in:
Harald Alvestrand 2020-09-22 07:41:50 +00:00 committed by Commit Bot
parent dd7df5c989
commit 6f4de80ddd
5 changed files with 82 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
pc/peer_connection.cc
View File

@ -1988,13 +1988,7 @@ PeerConnection::GetTransceivers() const {
<< "GetTransceivers is only supported with Unified Plan SdpSemantics.";
std::vector<rtc::scoped_refptr<RtpTransceiverInterface>> all_transceivers;
for (const auto& transceiver : transceivers_) {
// Temporary fix: Do not show stopped transceivers.
// The long term fix is to remove them from transceivers_, but this
// turns out to cause issues with audio channel lifetimes.
// TODO(https://crbug.com/webrtc/11840): Fix issue.
if (!transceiver->stopped()) {
all_transceivers.push_back(transceiver);
}
all_transceivers.push_back(transceiver);
}
return all_transceivers;
}
@ -2534,6 +2528,47 @@ void PeerConnection::SetLocalDescription(
});
}
void PeerConnection::RemoveStoppedTransceivers() {
// 3.2.10.1: For each transceiver in the connection's set of transceivers
// run the following steps:
if (!IsUnifiedPlan())
return;
for (auto it = transceivers_.begin(); it != transceivers_.end();) {
const auto& transceiver = *it;
// 3.2.10.1.1: If transceiver is stopped, associated with an m= section
// and the associated m= section is rejected in
// connection.[[CurrentLocalDescription]] or
// connection.[[CurrentRemoteDescription]], remove the
// transceiver from the connection's set of transceivers.
if (!transceiver->stopped()) {
++it;
continue;
}
const ContentInfo* local_content =
FindMediaSectionForTransceiver(transceiver, local_description());
const ContentInfo* remote_content =
FindMediaSectionForTransceiver(transceiver, remote_description());
if ((local_content && local_content->rejected) ||
(remote_content && remote_content->rejected)) {
RTC_LOG(LS_INFO) << "Dissociating transceiver"
<< " since the media section is being recycled.";
(*it)->internal()->set_mid(absl::nullopt);
(*it)->internal()->set_mline_index(absl::nullopt);
it = transceivers_.erase(it);
continue;
}
if (!local_content && !remote_content) {
// TODO(bugs.webrtc.org/11973): Consider if this should be removed already
// See https://github.com/w3c/webrtc-pc/issues/2576
RTC_LOG(LS_INFO)
<< "Dropping stopped transceiver that was never associated";
it = transceivers_.erase(it);
continue;
}
++it;
}
}
void PeerConnection::DoSetLocalDescription(
std::unique_ptr<SessionDescriptionInterface> desc,
rtc::scoped_refptr<SetLocalDescriptionObserverInterface> observer) {
@ -2605,34 +2640,7 @@ void PeerConnection::DoSetLocalDescription(
RTC_DCHECK(local_description());
if (local_description()->GetType() == SdpType::kAnswer) {
// 3.2.10.1: For each transceiver in the connection's set of transceivers
// run the following steps:
if (IsUnifiedPlan()) {
for (auto it = transceivers_.begin(); it != transceivers_.end();) {
const auto& transceiver = *it;
// 3.2.10.1.1: If transceiver is stopped, associated with an m= section
// and the associated m= section is rejected in
// connection.[[CurrentLocalDescription]] or
// connection.[[CurrentRemoteDescription]], remove the
// transceiver from the connection's set of transceivers.
if (transceiver->stopped()) {
const ContentInfo* content =
FindMediaSectionForTransceiver(transceiver, local_description());
if (content && content->rejected) {
RTC_LOG(LS_INFO) << "Dissociating transceiver"
<< " since the media section is being recycled.";
(*it)->internal()->set_mid(absl::nullopt);
(*it)->internal()->set_mline_index(absl::nullopt);
it = transceivers_.erase(it);
} else {
++it;
}
} else {
++it;
}
}
}
RemoveStoppedTransceivers();
// TODO(deadbeef): We already had to hop to the network thread for
// MaybeStartGathering...
@ -3100,6 +3108,7 @@ void PeerConnection::DoSetRemoteDescription(
RTC_DCHECK(remote_description());
if (type == SdpType::kAnswer) {
RemoveStoppedTransceivers();
// TODO(deadbeef): We already had to hop to the network thread for
// MaybeStartGathering...
network_thread()->Invoke<void>(
@ -3765,23 +3774,17 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source,
RTC_DCHECK(IsUnifiedPlan());
// If this is an offer then the m= section might be recycled. If the m=
// section is being recycled (defined as: rejected in the current local or
// remote description and not rejected in new description), dissociate the
// currently associated RtpTransceiver by setting its mid property to null,
// and discard the mapping between the transceiver and its m= section index.
// remote description and not rejected in new description), the transceiver
// should have been removed by RemoveStoppedTransceivers().
if (IsMediaSectionBeingRecycled(type, content, old_local_content,
old_remote_content)) {
// We want to dissociate the transceiver that has the rejected mid.
const std::string& old_mid =
(old_local_content && old_local_content->rejected)
? old_local_content->name
: old_remote_content->name;
auto old_transceiver = GetAssociatedTransceiver(old_mid);
if (old_transceiver) {
RTC_LOG(LS_INFO) << "Dissociating transceiver for MID=" << old_mid
<< " since the media section is being recycled.";
old_transceiver->internal()->set_mid(absl::nullopt);
old_transceiver->internal()->set_mline_index(absl::nullopt);
}
// The transceiver should be disassociated in RemoveStoppedTransceivers()
RTC_DCHECK(!old_transceiver);
}
const MediaContentDescription* media_desc = content.media_description();
auto transceiver = GetAssociatedTransceiver(content.name);

3
pc/peer_connection.h
View File

@ -450,6 +450,9 @@ class PeerConnection : public PeerConnectionInternal,
std::unique_ptr<SessionDescriptionInterface> desc,
rtc::scoped_refptr<SetRemoteDescriptionObserverInterface> observer);
// Helper function to remove stopped transceivers.
void RemoveStoppedTransceivers() RTC_RUN_ON(signaling_thread());
void CreateAudioReceiver(MediaStreamInterface* stream,
const RtpSenderInfo& remote_sender_info)
RTC_RUN_ON(signaling_thread());

4
pc/peer_connection_interface_unittest.cc
View File

@ -2668,8 +2668,8 @@ TEST_P(PeerConnectionInterfaceTest, CloseAndTestStreamsAndStates) {
EXPECT_EQ(1u, pc_->local_streams()->count());
EXPECT_EQ(1u, pc_->remote_streams()->count());
} else {
// Verify that the RtpTransceivers are no longer returned.
EXPECT_EQ(0u, pc_->GetTransceivers().size());
// Verify that the RtpTransceivers are still returned.
EXPECT_EQ(2u, pc_->GetTransceivers().size());
}
auto audio_receiver = GetFirstReceiverOfType(cricket::MEDIA_TYPE_AUDIO);

15
pc/peer_connection_jsep_unittest.cc
View File

@ -354,6 +354,10 @@ TEST_F(PeerConnectionJsepTest, SetRemoteOfferDoesNotReuseStoppedTransceiver) {
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
auto transceivers = callee->pc()->GetTransceivers();
ASSERT_EQ(2u, transceivers.size());
// The stopped transceiver is removed in SetLocalDescription(answer)
ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
transceivers = callee->pc()->GetTransceivers();
ASSERT_EQ(1u, transceivers.size());
EXPECT_EQ(caller->pc()->GetTransceivers()[0]->mid(), transceivers[0]->mid());
EXPECT_FALSE(transceivers[0]->stopped());
@ -558,6 +562,9 @@ TEST_F(PeerConnectionJsepTest,
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
auto transceivers = callee->pc()->GetTransceivers();
EXPECT_EQ(1u, transceivers.size());
ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
transceivers = callee->pc()->GetTransceivers();
EXPECT_EQ(0u, transceivers.size());
}
@ -593,15 +600,15 @@ TEST_F(PeerConnectionJsepTest,
auto callee = CreatePeerConnection();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
std::string first_mid = *first_transceiver->mid();
ASSERT_EQ(1u, callee->pc()->GetTransceivers().size());
callee->pc()->GetTransceivers()[0]->StopInternal();
ASSERT_EQ(0u, callee->pc()->GetTransceivers().size());
ASSERT_EQ(1u, callee->pc()->GetTransceivers().size());
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
EXPECT_TRUE(first_transceiver->stopped());
// First transceivers aren't dissociated yet on caller side.
ASSERT_NE(absl::nullopt, first_transceiver->mid());
std::string first_mid = *first_transceiver->mid();
// First transceivers are dissociated on caller side.
ASSERT_EQ(absl::nullopt, first_transceiver->mid());
// They are disassociated on callee side.
ASSERT_EQ(0u, callee->pc()->GetTransceivers().size());

18
pc/peer_connection_rtp_unittest.cc
View File

@ -1555,6 +1555,24 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
}
TEST_F(PeerConnectionRtpTestUnifiedPlan,
StopAndNegotiateCausesTransceiverToDisappear) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
callee->pc()->GetTransceivers()[0]->StopStandard();
ASSERT_TRUE(callee->ExchangeOfferAnswerWith(caller.get()));
EXPECT_EQ(RtpTransceiverDirection::kStopped,
transceiver->current_direction());
EXPECT_EQ(0U, caller->pc()->GetTransceivers().size());
EXPECT_EQ(0U, callee->pc()->GetTransceivers().size());
EXPECT_EQ(0U, caller->pc()->GetSenders().size());
EXPECT_EQ(0U, callee->pc()->GetSenders().size());
EXPECT_EQ(0U, caller->pc()->GetReceivers().size());
EXPECT_EQ(0U, callee->pc()->GetReceivers().size());
}
// Test that AddTransceiver fails if trying to use unimplemented RTP encoding
// parameters with the send_encodings parameters.
TEST_F(PeerConnectionRtpTestUnifiedPlan,