diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 1f63874e3b..cce90216c1 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -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(); } diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index f7ac830a65..b9a0227c76 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -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) diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index caa0ea0587..635591caaa 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -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 diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 5f4f34e634..942bd4652b 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -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));