From 74255ffb398aef81d5a2bcc5f032af62897f4441 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Wed, 24 Jan 2018 18:32:57 -0800 Subject: [PATCH] Add PeerConnection interop integration tests These tests verify the behavior between Plan B and Unified Plan PeerConnections. Bug: webrtc:7600 Change-Id: Ic41a0e692d32cde6fe7719ada2dbffd4281c008c Reviewed-on: https://webrtc-review.googlesource.com/43244 Reviewed-by: Harald Alvestrand Reviewed-by: Taylor Brandstetter Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#21782} --- pc/peerconnection.cc | 37 +---- pc/peerconnection_integrationtest.cc | 229 ++++++++++++++++++++++++++- pc/peerconnection_jsep_unittest.cc | 12 -- 3 files changed, 232 insertions(+), 46 deletions(-) diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 07e326d219..6ffe561a75 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1788,12 +1788,12 @@ RTCError PeerConnection::ApplyLocalDescription( if (content->rejected && !transceiver->stopped()) { transceiver->Stop(); } - if (!content->rejected) { - const auto& stream = content->media_description()->streams()[0]; + const auto& streams = content->media_description()->streams(); + if (!content->rejected && !streams.empty()) { transceiver->internal()->sender_internal()->set_stream_ids( - {stream.sync_label}); + {streams[0].sync_label}); transceiver->internal()->sender_internal()->SetSsrc( - stream.first_ssrc()); + streams[0].first_ssrc()); } } } else { @@ -2032,7 +2032,8 @@ RTCError PeerConnection::ApplyRemoteDescription( if (RtpTransceiverDirectionHasRecv(local_direction) && (!transceiver->current_direction() || !RtpTransceiverDirectionHasRecv( - *transceiver->current_direction()))) { + *transceiver->current_direction())) && + !media_desc->streams().empty()) { const std::string& sync_label = media_desc->streams()[0].sync_label; rtc::scoped_refptr stream = remote_streams_->find(sync_label); @@ -2061,7 +2062,7 @@ RTCError PeerConnection::ApplyRemoteDescription( if (content->rejected && !transceiver->stopped()) { transceiver->Stop(); } - if (!content->rejected) { + if (!content->rejected && !media_desc->streams().empty()) { const auto& stream = content->media_description()->streams()[0]; transceiver->internal()->receiver_internal()->SetupMediaChannel( stream.first_ssrc()); @@ -5375,30 +5376,6 @@ RTCError PeerConnection::ValidateSessionDescription( } } - // Unified Plan SDP should have exactly one stream per m= section for audio - // and video. - if (IsUnifiedPlan()) { - for (const ContentInfo& content : sdesc->description()->contents()) { - if (content.rejected) { - continue; - } - if (content.media_description()) { - cricket::MediaType media_type = content.media_description()->type(); - if (!(media_type == cricket::MEDIA_TYPE_AUDIO || - media_type == cricket::MEDIA_TYPE_VIDEO)) { - continue; - } - const cricket::StreamParamsVec& streams = - content.media_description()->streams(); - if (streams.size() != 1u) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, - "Unified Plan SDP should have exactly one " - "track per media section for audio and video."); - } - } - } - } - return RTCError::OK(); } diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index aee5ca1894..2768358d82 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -80,9 +80,11 @@ using webrtc::MockStatsObserver; using webrtc::ObserverInterface; using webrtc::PeerConnection; using webrtc::PeerConnectionInterface; +using RTCConfiguration = PeerConnectionInterface::RTCConfiguration; using webrtc::PeerConnectionFactory; using webrtc::PeerConnectionProxy; using webrtc::RTCErrorType; +using webrtc::RtpSenderInterface; using webrtc::RtpReceiverInterface; using webrtc::SdpSemantics; using webrtc::SdpType; @@ -309,9 +311,13 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, AddVideoTrack(); } - void AddAudioTrack() { AddTrack(CreateLocalAudioTrack()); } + rtc::scoped_refptr AddAudioTrack() { + return AddTrack(CreateLocalAudioTrack()); + } - void AddVideoTrack() { AddTrack(CreateLocalVideoTrack()); } + rtc::scoped_refptr AddVideoTrack() { + return AddTrack(CreateLocalVideoTrack()); + } rtc::scoped_refptr CreateLocalAudioTrack() { FakeConstraints constraints; @@ -340,10 +346,23 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, return CreateLocalVideoTrackInternal(FakeConstraints(), rotation); } - void AddTrack(rtc::scoped_refptr track, - const std::vector& stream_labels = {}) { + rtc::scoped_refptr AddTrack( + rtc::scoped_refptr track, + const std::vector& stream_labels = {}) { auto result = pc()->AddTrack(track, stream_labels); EXPECT_EQ(RTCErrorType::NONE, result.error().type()); + return result.MoveValue(); + } + + std::vector> GetReceiversOfType( + cricket::MediaType media_type) { + std::vector> receivers; + for (auto receiver : pc()->GetReceivers()) { + if (receiver->media_type() == media_type) { + receivers.push_back(receiver); + } + } + return receivers; } bool SignalingStateStable() { @@ -3662,6 +3681,208 @@ TEST_F(PeerConnectionIntegrationTest, UnifiedPlanMediaFlows) { kMaxWaitForFramesMs); } +// Tests that verify interoperability between Plan B and Unified Plan +// PeerConnections. +class PeerConnectionIntegrationInteropTest + : public PeerConnectionIntegrationTest, + public ::testing::WithParamInterface< + std::tuple> { + protected: + PeerConnectionIntegrationInteropTest() + : caller_semantics_(std::get<0>(GetParam())), + callee_semantics_(std::get<1>(GetParam())) {} + + bool CreatePeerConnectionWrappersWithSemantics() { + RTCConfiguration caller_config; + caller_config.sdp_semantics = caller_semantics_; + RTCConfiguration callee_config; + callee_config.sdp_semantics = callee_semantics_; + return CreatePeerConnectionWrappersWithConfig(caller_config, callee_config); + } + + const SdpSemantics caller_semantics_; + const SdpSemantics callee_semantics_; +}; + +TEST_P(PeerConnectionIntegrationInteropTest, NoMediaLocalToNoMediaRemote) { + ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics()); + ConnectFakeSignaling(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); +} + +TEST_P(PeerConnectionIntegrationInteropTest, OneAudioLocalToNoMediaRemote) { + ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics()); + ConnectFakeSignaling(); + auto audio_sender = caller()->AddAudioTrack(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Verify that one audio receiver has been created on the remote and that it + // has the same track ID as the sending track. + auto receivers = callee()->pc()->GetReceivers(); + ASSERT_EQ(1u, receivers.size()); + EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, receivers[0]->media_type()); + EXPECT_EQ(receivers[0]->track()->id(), audio_sender->track()->id()); + + // Expect to receive only audio frames on the callee. + ExpectNewFramesReceivedWithWait(0, 0, kDefaultExpectedAudioFrameCount, 0, + kMaxWaitForFramesMs); +} + +TEST_P(PeerConnectionIntegrationInteropTest, OneAudioOneVideoToNoMediaRemote) { + ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics()); + ConnectFakeSignaling(); + auto video_sender = caller()->AddVideoTrack(); + auto audio_sender = caller()->AddAudioTrack(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Verify that one audio and one video receiver have been created on the + // remote and that they have the same track IDs as the sending tracks. + auto audio_receivers = + callee()->GetReceiversOfType(cricket::MEDIA_TYPE_AUDIO); + ASSERT_EQ(1u, audio_receivers.size()); + EXPECT_EQ(audio_receivers[0]->track()->id(), audio_sender->track()->id()); + auto video_receivers = + callee()->GetReceiversOfType(cricket::MEDIA_TYPE_VIDEO); + ASSERT_EQ(1u, video_receivers.size()); + EXPECT_EQ(video_receivers[0]->track()->id(), video_sender->track()->id()); + + // Expect to receive audio and video frames only on the callee. + ExpectNewFramesReceivedWithWait(0, 0, kDefaultExpectedAudioFrameCount, + kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + +TEST_P(PeerConnectionIntegrationInteropTest, + OneAudioOneVideoLocalToOneAudioOneVideoRemote) { + ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics()); + ConnectFakeSignaling(); + caller()->AddAudioVideoTracks(); + callee()->AddAudioVideoTracks(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + ExpectNewFramesReceivedWithWait( + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + +TEST_P(PeerConnectionIntegrationInteropTest, + ReverseRolesOneAudioLocalToOneVideoRemote) { + ASSERT_TRUE(CreatePeerConnectionWrappersWithSemantics()); + ConnectFakeSignaling(); + caller()->AddAudioTrack(); + callee()->AddVideoTrack(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Verify that only the audio track has been negotiated. + EXPECT_EQ(0u, caller()->GetReceiversOfType(cricket::MEDIA_TYPE_VIDEO).size()); + // Might also check that the callee's NegotiationNeeded flag is set. + + // Reverse roles. + callee()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Expect to receive audio frames on the callee and video frames on the + // caller. + ExpectNewFramesReceivedWithWait(0, kDefaultExpectedVideoFrameCount, + kDefaultExpectedAudioFrameCount, 0, + kMaxWaitForFramesMs); +} + +// 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()); + ConnectFakeSignaling(); + auto first_sender = caller()->AddVideoTrack(); + caller()->AddVideoTrack(); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Verify that there is only one receiver and it corresponds to the first + // added track. + auto receivers = callee()->pc()->GetReceivers(); + ASSERT_EQ(1u, receivers.size()); + EXPECT_TRUE(receivers[0]->track()->enabled()); + EXPECT_EQ(first_sender->track()->id(), receivers[0]->track()->id()); + + // Expect to receive video frames from the one track. + ExpectNewFramesReceivedWithWait(0, 0, + 0, kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + +// 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. + ExpectNewFramesReceivedWithWait(0, kDefaultExpectedVideoFrameCount, 0, + kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + +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 0ba96e1a3b..092b3c74b9 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -243,18 +243,6 @@ TEST_F(PeerConnectionJsepTest, SetLocalOfferSetsTransceiverMid) { EXPECT_EQ(video_mid, video_transceiver->mid()); } -TEST_F(PeerConnectionJsepTest, SetLocalOfferFailsWithNoTrackInMediaSection) { - auto caller = CreatePeerConnection(); - caller->AddAudioTrack("audio"); - - auto offer = caller->CreateOffer(); - auto& contents = offer->description()->contents(); - ASSERT_EQ(1u, contents.size()); - contents[0].description->mutable_streams().clear(); - - ASSERT_FALSE(caller->SetLocalDescription(std::move(offer))); -} - // Tests for JSEP SetRemoteDescription with a remote offer. // Test that setting a remote offer with sendrecv audio and video creates two