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

Allow transceivers to be not found in more cases.

Browse Source 
This fixes the case where a media section is rejected in an answer,
something that is done by SFUs, but not possible using transceiver.stop().

Bug: chromium:1134686
Change-Id: Ia33579070093ab70c4191710fd1dcb3ca377befd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187349
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32363}
This commit is contained in:
Harald Alvestrand 2020-10-09 08:13:30 +00:00 committed by Commit Bot
parent 38e9b06151
commit 09bd9ba1f3
2 changed files with 66 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
pc/peer_connection_rtp_unittest.cc
View File

@ -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,

16
pc/sdp_offer_answer.cc
View File

@ -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);