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

Report an error when trying to set complex Plan B SDP on Unified Plan

Browse Source 
This changes the PeerConnection when in Unified Plan mode to reject
SDP applied with SetLocalDescription or SetRemoteDescription if the
SDP has multiple "Plan B tracks" (a=ssrc lines) in a media section.
The error is to inform developers that the given SDP will not be
interpreted as they might expect.

Bug: None
Change-Id: I7a0e11282fbf63dac06038cd22a66683517a87d0
Reviewed-on: https://webrtc-review.googlesource.com/68764
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22829}
This commit is contained in:
Steve Anton 2018-04-09 14:10:01 -07:00 committed by Commit Bot
parent d5601329aa
commit ba42e99f18
4 changed files with 85 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
pc/peerconnection.cc
View File

@ -2194,6 +2194,11 @@ void PeerConnection::SetRemoteDescription(
return;
}
if (desc->GetType() == SdpType::kOffer) {
// Report to UMA the format of the received offer.
ReportSdpFormatReceived(*desc);
}
RTCError error = ValidateSessionDescription(desc.get(), cricket::CS_REMOTE);
if (!error.ok()) {
std::string error_message = GetSetDescriptionErrorMessage(
@ -2225,11 +2230,6 @@ void PeerConnection::SetRemoteDescription(
}
RTC_DCHECK(remote_description());
if (type == SdpType::kOffer) {
// Report to UMA the format of the received offer.
ReportSdpFormatReceived(*remote_description());
}
if (type == SdpType::kAnswer) {
// TODO(deadbeef): We already had to hop to the network thread for
// MaybeStartGathering...
@ -5751,6 +5751,25 @@ RTCError PeerConnection::ValidateSessionDescription(
}
}
if (IsUnifiedPlan()) {
// Ensure that each audio and video media section has at most one
// "StreamParams". This will return an error if receiving a session
// description from a "Plan B" endpoint which adds multiple tracks of the
// same type. With Unified Plan, there can only be at most one track per
// media section.
for (const ContentInfo& content : sdesc->description()->contents()) {
const MediaContentDescription& desc = *content.description;
if ((desc.type() == cricket::MEDIA_TYPE_AUDIO ||
desc.type() == cricket::MEDIA_TYPE_VIDEO) &&
desc.streams().size() > 1u) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"Media section has more than one track specified "
"with a=ssrc lines which is not supported with "
"Unified Plan.");
}
}
}
return RTCError::OK();
}

80
pc/peerconnection_integrationtest.cc
View File

@ -4258,10 +4258,22 @@ TEST_P(PeerConnectionIntegrationInteropTest,
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
// Test that if one side offers two video tracks then the other side will only
// see the first one and ignore the second.
TEST_P(PeerConnectionIntegrationInteropTest, TwoVideoLocalToNoMediaRemote) {
ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics());
INSTANTIATE_TEST_CASE_P(
PeerConnectionIntegrationTest,
PeerConnectionIntegrationInteropTest,
Values(std::make_tuple(SdpSemantics::kPlanB, SdpSemantics::kUnifiedPlan),
std::make_tuple(SdpSemantics::kUnifiedPlan, SdpSemantics::kPlanB)));
// Test that if the Unified Plan side offers two video tracks then the Plan B
// side will only see the first one and ignore the second.
TEST_F(PeerConnectionIntegrationTestPlanB, TwoVideoUnifiedPlanToNoMediaPlanB) {
RTCConfiguration caller_config;
caller_config.sdp_semantics = SdpSemantics::kUnifiedPlan;
RTCConfiguration callee_config;
callee_config.sdp_semantics = SdpSemantics::kPlanB;
ASSERT_TRUE(
CreatePeerConnectionWrappersWithConfig(caller_config, callee_config));
ConnectFakeSignaling();
auto first_sender = caller()->AddVideoTrack();
caller()->AddVideoTrack();
@ -4281,66 +4293,6 @@ TEST_P(PeerConnectionIntegrationInteropTest, TwoVideoLocalToNoMediaRemote) {
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
// Test that in the multi-track case each endpoint only looks at the first track
// and ignores the second one.
TEST_P(PeerConnectionIntegrationInteropTest, TwoVideoLocalToTwoVideoRemote) {
ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics());
ConnectFakeSignaling();
caller()->AddVideoTrack();
caller()->AddVideoTrack();
callee()->AddVideoTrack();
callee()->AddVideoTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
PeerConnectionWrapper* plan_b =
(caller_semantics_ == SdpSemantics::kPlanB ? caller() : callee());
PeerConnectionWrapper* unified_plan =
(caller_semantics_ == SdpSemantics::kUnifiedPlan ? caller() : callee());
// Should have two senders each, one for each track.
EXPECT_EQ(2u, plan_b->pc()->GetSenders().size());
EXPECT_EQ(2u, unified_plan->pc()->GetSenders().size());
// Plan B will have one receiver since it only looks at the first video
// section. The receiver should have the same track ID as the sender's first
// track.
ASSERT_EQ(1u, plan_b->pc()->GetReceivers().size());
EXPECT_EQ(unified_plan->pc()->GetSenders()[0]->track()->id(),
plan_b->pc()->GetReceivers()[0]->track()->id());
// Unified Plan will have two receivers since they were created with the
// transceivers when the tracks were added.
ASSERT_EQ(2u, unified_plan->pc()->GetReceivers().size());
if (unified_plan == caller()) {
// If the Unified Plan endpoint was the caller, then the Plan B endpoint
// would have rejected the second video media section so we would expect the
// transceiver to be stopped.
EXPECT_FALSE(unified_plan->pc()->GetTransceivers()[0]->stopped());
EXPECT_TRUE(unified_plan->pc()->GetTransceivers()[1]->stopped());
} else {
// If the Unified Plan endpoint was the callee, then the Plan B endpoint
// would have offered only one video section so we would expect the first
// transceiver to map to the first track and the second transceiver to be
// missing a mid.
EXPECT_TRUE(unified_plan->pc()->GetTransceivers()[0]->mid());
EXPECT_FALSE(unified_plan->pc()->GetTransceivers()[1]->mid());
}
// Should be exchanging video frames for the first tracks on each endpoint.
MediaExpectations media_expectations;
media_expectations.ExpectBidirectionalVideo();
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
INSTANTIATE_TEST_CASE_P(
PeerConnectionIntegrationTest,
PeerConnectionIntegrationInteropTest,
Values(std::make_tuple(SdpSemantics::kPlanB, SdpSemantics::kUnifiedPlan),
std::make_tuple(SdpSemantics::kUnifiedPlan, SdpSemantics::kPlanB)));
} // namespace
#endif // if !defined(THREAD_SANITIZER)

49
pc/peerconnection_jsep_unittest.cc
View File

@ -1263,18 +1263,51 @@ TEST_F(PeerConnectionJsepTest, CurrentDirectionResetWhenRtpTransceiverStopped) {
EXPECT_FALSE(transceiver->current_direction());
}
// Tests that you can't set an answer on a PeerConnection before setting
// the offer.
// Test that you can't set an answer on a PeerConnection before setting the
// offer.
TEST_F(PeerConnectionJsepTest, AnswerBeforeOfferFails) {
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
RTCError error_out;
caller->AddAudioTrack("audio");
auto offer = caller->CreateOffer();
callee->SetRemoteDescription(std::move(offer), &error_out);
auto answer = callee->CreateAnswer();
EXPECT_FALSE(caller->SetRemoteDescription(std::move(answer), &error_out));
EXPECT_EQ(RTCErrorType::INVALID_STATE, error_out.type());
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
RTCError error;
ASSERT_FALSE(caller->SetRemoteDescription(callee->CreateAnswer(), &error));
EXPECT_EQ(RTCErrorType::INVALID_STATE, error.type());
}
// Test that a Unified Plan PeerConnection fails to set a Plan B offer if it has
// two video tracks.
TEST_F(PeerConnectionJsepTest, TwoVideoPlanBToUnifiedPlanFails) {
RTCConfiguration config_planb;
config_planb.sdp_semantics = SdpSemantics::kPlanB;
auto caller = CreatePeerConnection(config_planb);
auto callee = CreatePeerConnection();
caller->AddVideoTrack("video1");
caller->AddVideoTrack("video2");
RTCError error;
ASSERT_FALSE(callee->SetRemoteDescription(caller->CreateOffer(), &error));
EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.type());
}
// Test that a Unified Plan PeerConnection fails to set a Plan B answer if it
// has two video tracks.
TEST_F(PeerConnectionJsepTest, OneVideoUnifiedPlanToTwoVideoPlanBFails) {
auto caller = CreatePeerConnection();
RTCConfiguration config_planb;
config_planb.sdp_semantics = SdpSemantics::kPlanB;
auto callee = CreatePeerConnection(config_planb);
caller->AddVideoTrack("video");
callee->AddVideoTrack("video1");
callee->AddVideoTrack("video2");
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
RTCError error;
ASSERT_FALSE(caller->SetRemoteDescription(caller->CreateAnswer(), &error));
EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, error.type());
}
} // namespace webrtc

5
pc/peerconnection_rtp_unittest.cc
View File

@ -1294,7 +1294,10 @@ TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) {
auto callee = CreatePeerConnectionWithUnifiedPlan();
auto callee_metrics = callee->RegisterFakeMetricsObserver();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
// This fails since Unified Plan cannot set a session description with
// multiple "Plan B tracks" in the same media section. But we still expect the
// SDP Format to be recorded.
ASSERT_FALSE(callee->SetRemoteDescription(caller->CreateOffer()));
EXPECT_TRUE(callee_metrics->ExpectOnlySingleEnumCount(
kEnumCounterSdpFormatReceived, kSdpFormatReceivedComplexPlanB));