diff --git a/pc/mediasession.cc b/pc/mediasession.cc index 692bcc2ac9..f53785df93 100644 --- a/pc/mediasession.cc +++ b/pc/mediasession.cc @@ -579,7 +579,7 @@ static void PruneCryptos(const CryptoParamsVec& filter, target_cryptos->end()); } -static bool IsRtpProtocol(const std::string& protocol) { +bool IsRtpProtocol(const std::string& protocol) { return protocol.empty() || (protocol.find(cricket::kMediaProtocolRtpPrefix) != std::string::npos); } diff --git a/pc/mediasession.h b/pc/mediasession.h index d3a9b3c1ec..7c531aaee6 100644 --- a/pc/mediasession.h +++ b/pc/mediasession.h @@ -329,6 +329,9 @@ void GetSupportedDataSdesCryptoSuiteNames( const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names); +// Returns true if the given media section protocol indicates use of RTP. +bool IsRtpProtocol(const std::string& protocol); + } // namespace cricket #endif // PC_MEDIASESSION_H_ diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 94f2db1e26..ee1cffec54 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2099,8 +2099,14 @@ RTCError PeerConnection::UpdateTransceiversAndDataChannels( return error; } } else if (media_type == cricket::MEDIA_TYPE_DATA) { - // TODO(bugs.webrtc.org/7600): Add support for data channels with Unified - // Plan. + if (GetDataMid() && new_content.name != *GetDataMid()) { + // Ignore all but the first data section. + continue; + } + RTCError error = UpdateDataChannel(source, new_content, bundle_group); + if (!error.ok()) { + return error; + } } else { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Unknown section type."); @@ -2147,6 +2153,34 @@ RTCError PeerConnection::UpdateTransceiverChannel( return RTCError::OK(); } +RTCError PeerConnection::UpdateDataChannel( + cricket::ContentSource source, + const cricket::ContentInfo& content, + const cricket::ContentGroup* bundle_group) { + if (data_channel_type_ == cricket::DCT_NONE) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "Data channels are not supported."); + } + if (content.rejected) { + DestroyDataChannel(); + } else { + if (!rtp_data_channel_ && !sctp_transport_) { + if (!CreateDataChannel(content.name, GetTransportNameForMediaSection( + content.name, bundle_group))) { + LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, + "Failed to create data channel."); + } + } + if (source == cricket::CS_REMOTE) { + const MediaContentDescription* data_desc = content.media_description(); + if (data_desc && cricket::IsRtpProtocol(data_desc->protocol())) { + UpdateRemoteRtpDataChannels(GetActiveStreams(data_desc)); + } + } + } + return RTCError::OK(); +} + RTCErrorOr>> PeerConnection::AssociateTransceiver(cricket::ContentSource source, size_t mline_index, @@ -2946,6 +2980,15 @@ void PeerConnection::GetOptionsForOffer( GetOptionsForPlanBOffer(offer_answer_options, session_options); } + // Intentionally unset the data channel type for RTP data channel with the + // second condition. Otherwise the RTP data channels would be successfully + // negotiated by default and the unit tests in WebRtcDataBrowserTest will fail + // when building with chromium. We want to leave RTP data channels broken, so + // people won't try to use them. + if (!rtp_data_channels_.empty() || data_channel_type() != cricket::DCT_RTP) { + session_options->data_channel_type = data_channel_type(); + } + // Apply ICE restart flag and renomination flag. for (auto& options : session_options->media_description_options) { options.transport_options.ice_restart = offer_answer_options.ice_restart; @@ -3022,9 +3065,7 @@ void PeerConnection::GetOptionsForPlanBOffer( } if (!data_index && offer_new_data_description) { session_options->media_description_options.push_back( - cricket::MediaDescriptionOptions( - cricket::MEDIA_TYPE_DATA, cricket::CN_DATA, - RtpTransceiverDirection::kSendRecv, false)); + GetMediaDescriptionOptionsForActiveData(cricket::CN_DATA)); data_index = session_options->media_description_options.size() - 1; } @@ -3034,22 +3075,9 @@ void PeerConnection::GetOptionsForPlanBOffer( cricket::MediaDescriptionOptions* video_media_description_options = !video_index ? nullptr : &session_options->media_description_options[*video_index]; - cricket::MediaDescriptionOptions* data_media_description_options = - !data_index ? nullptr - : &session_options->media_description_options[*data_index]; AddRtpSenderOptions(GetSendersInternal(), audio_media_description_options, video_media_description_options); - AddRtpDataChannelOptions(rtp_data_channels_, data_media_description_options); - - // Intentionally unset the data channel type for RTP data channel with the - // second condition. Otherwise the RTP data channels would be successfully - // negotiated by default and the unit tests in WebRtcDataBrowserTest will fail - // when building with chromium. We want to leave RTP data channels broken, so - // people won't try to use them. - if (!rtp_data_channels_.empty() || data_channel_type() != cricket::DCT_RTP) { - session_options->data_channel_type = data_channel_type(); - } } // Find a new MID that is not already in |used_mids|, then add it to |used_mids| @@ -3150,8 +3178,14 @@ void PeerConnection::GetOptionsForUnifiedPlanOffer( } } else { RTC_CHECK_EQ(cricket::MEDIA_TYPE_DATA, media_type); - // TODO(bugs.webrtc.org/7600): Add support for data channels with Unified - // Plan. + RTC_CHECK(GetDataMid()); + if (had_been_rejected || mid != *GetDataMid()) { + session_options->media_description_options.push_back( + GetMediaDescriptionOptionsForRejectedData(mid)); + } else { + session_options->media_description_options.push_back( + GetMediaDescriptionOptionsForActiveData(mid)); + } } } // Next, look for transceivers that are newly added (that is, are not stopped @@ -3178,8 +3212,12 @@ void PeerConnection::GetOptionsForUnifiedPlanOffer( // See comment above for why CreateOffer changes the transceiver's state. transceiver->internal()->set_mline_index(mline_index); } - // TODO(bugs.webrtc.org/7600): Add support for data channels with Unified - // Plan. + // Lastly, add a m-section if we have local data channels and an m section + // does not already exist. + if (!GetDataMid() && HasDataChannels()) { + session_options->media_description_options.push_back( + GetMediaDescriptionOptionsForActiveData(AllocateMid(&used_mids))); + } } void PeerConnection::GetOptionsForAnswer( @@ -3193,6 +3231,14 @@ void PeerConnection::GetOptionsForAnswer( GetOptionsForPlanBAnswer(offer_answer_options, session_options); } + // Intentionally unset the data channel type for RTP data channel. Otherwise + // the RTP data channels would be successfully negotiated by default and the + // unit tests in WebRtcDataBrowserTest will fail when building with chromium. + // We want to leave RTP data channels broken, so people won't try to use them. + if (!rtp_data_channels_.empty() || data_channel_type() != cricket::DCT_RTP) { + session_options->data_channel_type = data_channel_type(); + } + // Apply ICE renomination flag. for (auto& options : session_options->media_description_options) { options.transport_options.enable_ice_renomination = @@ -3247,21 +3293,9 @@ void PeerConnection::GetOptionsForPlanBAnswer( cricket::MediaDescriptionOptions* video_media_description_options = !video_index ? nullptr : &session_options->media_description_options[*video_index]; - cricket::MediaDescriptionOptions* data_media_description_options = - !data_index ? nullptr - : &session_options->media_description_options[*data_index]; AddRtpSenderOptions(GetSendersInternal(), audio_media_description_options, video_media_description_options); - AddRtpDataChannelOptions(rtp_data_channels_, data_media_description_options); - - // Intentionally unset the data channel type for RTP data channel. Otherwise - // the RTP data channels would be successfully negotiated by default and the - // unit tests in WebRtcDataBrowserTest will fail when building with chromium. - // We want to leave RTP data channels broken, so people won't try to use them. - if (!rtp_data_channels_.empty() || data_channel_type() != cricket::DCT_RTP) { - session_options->data_channel_type = data_channel_type(); - } } void PeerConnection::GetOptionsForUnifiedPlanAnswer( @@ -3282,8 +3316,16 @@ void PeerConnection::GetOptionsForUnifiedPlanAnswer( GetMediaDescriptionOptionsForTransceiver(transceiver, content.name)); } else { RTC_CHECK_EQ(cricket::MEDIA_TYPE_DATA, media_type); - // TODO(bugs.webrtc.org/7600): Add support for data channels with Unified - // Plan. + RTC_CHECK(GetDataMid()); + // Always reject a data section if it has already been rejected. + // Additionally, reject all data sections except for the first one. + if (content.rejected || content.name != *GetDataMid()) { + session_options->media_description_options.push_back( + GetMediaDescriptionOptionsForRejectedData(content.name)); + } else { + session_options->media_description_options.push_back( + GetMediaDescriptionOptionsForActiveData(content.name)); + } } } } @@ -3331,22 +3373,52 @@ void PeerConnection::GenerateMediaDescriptionOptions( // If we already have an data m= section, reject this extra one. if (*data_index) { session_options->media_description_options.push_back( - cricket::MediaDescriptionOptions( - cricket::MEDIA_TYPE_DATA, content.name, - RtpTransceiverDirection::kInactive, true)); + GetMediaDescriptionOptionsForRejectedData(content.name)); } else { session_options->media_description_options.push_back( - cricket::MediaDescriptionOptions( - cricket::MEDIA_TYPE_DATA, content.name, - // Direction for data sections is meaningless, but legacy - // endpoints might expect sendrecv. - RtpTransceiverDirection::kSendRecv, false)); + GetMediaDescriptionOptionsForActiveData(content.name)); *data_index = session_options->media_description_options.size() - 1; } } } } +cricket::MediaDescriptionOptions +PeerConnection::GetMediaDescriptionOptionsForActiveData( + const std::string& mid) const { + // Direction for data sections is meaningless, but legacy endpoints might + // expect sendrecv. + cricket::MediaDescriptionOptions options(cricket::MEDIA_TYPE_DATA, mid, + RtpTransceiverDirection::kSendRecv, + /*stopped=*/false); + AddRtpDataChannelOptions(rtp_data_channels_, &options); + return options; +} + +cricket::MediaDescriptionOptions +PeerConnection::GetMediaDescriptionOptionsForRejectedData( + const std::string& mid) const { + cricket::MediaDescriptionOptions options(cricket::MEDIA_TYPE_DATA, mid, + RtpTransceiverDirection::kInactive, + /*stopped=*/true); + AddRtpDataChannelOptions(rtp_data_channels_, &options); + return options; +} + +rtc::Optional PeerConnection::GetDataMid() const { + switch (data_channel_type_) { + case cricket::DCT_RTP: + if (!rtp_data_channel_) { + return rtc::nullopt; + } + return rtp_data_channel_->content_name(); + case cricket::DCT_SCTP: + return sctp_content_name_; + default: + return rtc::nullopt; + } +} + void PeerConnection::RemoveSenders(cricket::MediaType media_type) { UpdateLocalSenders(std::vector(), media_type); UpdateRemoteSendersList(std::vector(), false, diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 66061f10c0..a3797f4c7b 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -428,6 +428,12 @@ class PeerConnection : public PeerConnectionInterface, const cricket::ContentInfo& content, const cricket::ContentGroup* bundle_group); + // Either creates or destroys the local data channel according to the given + // media section. + RTCError UpdateDataChannel(cricket::ContentSource source, + const cricket::ContentInfo& content, + const cricket::ContentGroup* bundle_group); + // Associate the given transceiver according to the JSEP rules. RTCErrorOr< rtc::scoped_refptr>> @@ -501,6 +507,21 @@ class PeerConnection : public PeerConnectionInterface, rtc::Optional* data_index, cricket::MediaSessionOptions* session_options); + // Generates the active MediaDescriptionOptions for the local data channel + // given the specified MID. + cricket::MediaDescriptionOptions GetMediaDescriptionOptionsForActiveData( + const std::string& mid) const; + + // Generates the rejected MediaDescriptionOptions for the local data channel + // given the specified MID. + cricket::MediaDescriptionOptions GetMediaDescriptionOptionsForRejectedData( + const std::string& mid) const; + + // Returns the MID for the data section associated with either the + // RtpDataChannel or SCTP data channel, if it has been set. If no data + // channels are configured this will return nullopt. + rtc::Optional GetDataMid() const; + // Remove all local and remote senders of type |media_type|. // Called when a media type is rejected (m-line set to port 0). void RemoveSenders(cricket::MediaType media_type); diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index 119b1351a0..5edd1a38c4 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -10,13 +10,17 @@ #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" +#include "media/engine/webrtcmediaengine.h" +#include "modules/audio_processing/include/audio_processing.h" #include "pc/mediasession.h" +#include "pc/peerconnectionfactory.h" #include "pc/peerconnectionwrapper.h" #include "pc/sdputils.h" #ifdef WEBRTC_ANDROID #include "pc/test/androidtestinitializer.h" #endif #include "pc/test/fakeaudiocapturemodule.h" +#include "pc/test/fakesctptransport.h" #include "rtc_base/gunit.h" #include "rtc_base/ptr_util.h" #include "rtc_base/virtualsocketserver.h" @@ -36,6 +40,30 @@ using ::testing::Values; using ::testing::Combine; using ::testing::ElementsAre; +class PeerConnectionFactoryForJsepTest : public PeerConnectionFactory { + public: + PeerConnectionFactoryForJsepTest() + : PeerConnectionFactory( + rtc::Thread::Current(), + rtc::Thread::Current(), + rtc::Thread::Current(), + rtc::WrapUnique(cricket::WebRtcMediaEngineFactory::Create( + FakeAudioCaptureModule::Create(), + CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory(), + nullptr, + nullptr, + nullptr, + AudioProcessing::Create())), + CreateCallFactory(), + nullptr) {} + + std::unique_ptr + CreateSctpTransportInternalFactory() { + return rtc::MakeUnique(); + } +}; + class PeerConnectionJsepTest : public ::testing::Test { protected: typedef std::unique_ptr WrapperPtr; @@ -45,10 +73,6 @@ class PeerConnectionJsepTest : public ::testing::Test { #ifdef WEBRTC_ANDROID InitializeAndroidObjects(); #endif - pc_factory_ = CreatePeerConnectionFactory( - rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(), - FakeAudioCaptureModule::Create(), CreateBuiltinAudioEncoderFactory(), - CreateBuiltinAudioDecoderFactory(), nullptr, nullptr); } WrapperPtr CreatePeerConnection() { @@ -58,20 +82,22 @@ class PeerConnectionJsepTest : public ::testing::Test { } WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { + rtc::scoped_refptr pc_factory( + new rtc::RefCountedObject()); + RTC_CHECK(pc_factory->Initialize()); auto observer = rtc::MakeUnique(); - auto pc = pc_factory_->CreatePeerConnection(config, nullptr, nullptr, - observer.get()); + auto pc = pc_factory->CreatePeerConnection(config, nullptr, nullptr, + observer.get()); if (!pc) { return nullptr; } - return rtc::MakeUnique(pc_factory_, pc, + return rtc::MakeUnique(pc_factory, pc, std::move(observer)); } std::unique_ptr vss_; rtc::AutoSocketServerThread main_; - rtc::scoped_refptr pc_factory_; }; // Tests for JSEP initial offer generation. @@ -80,8 +106,9 @@ class PeerConnectionJsepTest : public ::testing::Test { // no media sections. TEST_F(PeerConnectionJsepTest, EmptyInitialOffer) { auto caller = CreatePeerConnection(); + auto offer = caller->CreateOffer(); - EXPECT_EQ(0u, offer->description()->contents().size()); + ASSERT_EQ(0u, offer->description()->contents().size()); } // Test that an initial offer with one audio track generates one audio media @@ -89,8 +116,8 @@ TEST_F(PeerConnectionJsepTest, EmptyInitialOffer) { TEST_F(PeerConnectionJsepTest, AudioOnlyInitialOffer) { auto caller = CreatePeerConnection(); caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); - auto offer = caller->CreateOffer(); + auto offer = caller->CreateOffer(); auto contents = offer->description()->contents(); ASSERT_EQ(1u, contents.size()); EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, contents[0].media_description()->type()); @@ -101,13 +128,37 @@ TEST_F(PeerConnectionJsepTest, AudioOnlyInitialOffer) { TEST_F(PeerConnectionJsepTest, VideoOnlyInitialOffer) { auto caller = CreatePeerConnection(); caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); - auto offer = caller->CreateOffer(); + auto offer = caller->CreateOffer(); auto contents = offer->description()->contents(); ASSERT_EQ(1u, contents.size()); EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, contents[0].media_description()->type()); } +// Test that an initial offer with one data channel generates one data media +// section. +TEST_F(PeerConnectionJsepTest, DataOnlyInitialOffer) { + auto caller = CreatePeerConnection(); + caller->CreateDataChannel("dc"); + + auto offer = caller->CreateOffer(); + auto contents = offer->description()->contents(); + ASSERT_EQ(1u, contents.size()); + EXPECT_EQ(cricket::MEDIA_TYPE_DATA, contents[0].media_description()->type()); +} + +// Test that creating multiple data channels only results in one data section +// generated in the offer. +TEST_F(PeerConnectionJsepTest, MultipleDataChannelsCreateOnlyOneDataSection) { + auto caller = CreatePeerConnection(); + caller->CreateDataChannel("first"); + caller->CreateDataChannel("second"); + caller->CreateDataChannel("third"); + + auto offer = caller->CreateOffer(); + ASSERT_EQ(1u, offer->description()->contents().size()); +} + // Test that multiple media sections in the initial offer are ordered in the // order the transceivers were added to the PeerConnection. This is required by // JSEP section 5.2.1. @@ -118,8 +169,8 @@ TEST_F(PeerConnectionJsepTest, MediaSectionsInInitialOfferOrderedCorrectly) { RtpTransceiverInit init; init.direction = RtpTransceiverDirection::kSendOnly; caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); - auto offer = caller->CreateOffer(); + auto offer = caller->CreateOffer(); auto contents = offer->description()->contents(); ASSERT_EQ(3u, contents.size()); @@ -147,12 +198,8 @@ TEST_F(PeerConnectionJsepTest, MediaSectionsInInitialOfferHaveDifferentMids) { auto caller = CreatePeerConnection(); caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto offer = caller->CreateOffer(); - - std::string sdp; - offer->ToString(&sdp); - RTC_LOG(LS_INFO) << sdp; - auto contents = offer->description()->contents(); ASSERT_EQ(2u, contents.size()); EXPECT_NE(contents[0].name, contents[1].name); @@ -172,6 +219,7 @@ TEST_F(PeerConnectionJsepTest, TEST_F(PeerConnectionJsepTest, SetLocalEmptyOfferCreatesNoTransceivers) { auto caller = CreatePeerConnection(); + ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer())); EXPECT_THAT(caller->pc()->GetTransceivers(), ElementsAre()); @@ -348,19 +396,26 @@ TEST_F(PeerConnectionJsepTest, CreateAnswerHasSameMidsAsOffer) { auto first_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); auto second_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); auto third_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); + caller->CreateDataChannel("dc"); auto callee = CreatePeerConnection(); - ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + auto offer = caller->CreateOffer(); + const auto* offer_data = cricket::GetFirstDataContent(offer->description()); + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); auto answer = callee->CreateAnswer(); auto contents = answer->description()->contents(); - ASSERT_EQ(3u, contents.size()); + ASSERT_EQ(4u, contents.size()); EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, contents[0].media_description()->type()); - EXPECT_EQ(*first_transceiver->mid(), contents[0].name); + EXPECT_EQ(first_transceiver->mid(), contents[0].name); EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, contents[1].media_description()->type()); - EXPECT_EQ(*second_transceiver->mid(), contents[1].name); + EXPECT_EQ(second_transceiver->mid(), contents[1].name); EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, contents[2].media_description()->type()); - EXPECT_EQ(*third_transceiver->mid(), contents[2].name); + EXPECT_EQ(third_transceiver->mid(), contents[2].name); + EXPECT_EQ(cricket::MEDIA_TYPE_DATA, contents[3].media_description()->type()); + EXPECT_EQ(offer_data->name, contents[3].name); } // Test that an answering media section is marked as rejected if the underlying @@ -619,6 +674,62 @@ INSTANTIATE_TEST_CASE_P( Combine(Values(cricket::MEDIA_TYPE_AUDIO, cricket::MEDIA_TYPE_VIDEO), Values(cricket::MEDIA_TYPE_AUDIO, cricket::MEDIA_TYPE_VIDEO))); +// Test that a new data channel section will not reuse a recycleable audio or +// video media section. Additionally, tests that the new section is added to the +// end of the session description. +TEST_F(PeerConnectionJsepTest, DataChannelDoesNotRecycleMediaSection) { + auto caller = CreatePeerConnection(); + auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + auto callee = CreatePeerConnection(); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + transceiver->Stop(); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + caller->CreateDataChannel("dc"); + + auto offer = caller->CreateOffer(); + auto offer_contents = offer->description()->contents(); + ASSERT_EQ(2u, offer_contents.size()); + EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, + offer_contents[0].media_description()->type()); + EXPECT_EQ(cricket::MEDIA_TYPE_DATA, + offer_contents[1].media_description()->type()); + + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); + + auto answer = callee->CreateAnswer(); + auto answer_contents = answer->description()->contents(); + ASSERT_EQ(2u, answer_contents.size()); + EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, + answer_contents[0].media_description()->type()); + EXPECT_EQ(cricket::MEDIA_TYPE_DATA, + answer_contents[1].media_description()->type()); +} + +// Test that if a new track is added to an existing session that has a data, +// the new section comes at the end of the new offer, after the existing data +// section. +TEST_F(PeerConnectionJsepTest, AudioTrackAddedAfterDataSectionInReoffer) { + auto caller = CreatePeerConnection(); + caller->CreateDataChannel("dc"); + auto callee = CreatePeerConnection(); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + caller->AddAudioTrack("a"); + + auto offer = caller->CreateOffer(); + auto contents = offer->description()->contents(); + ASSERT_EQ(2u, contents.size()); + EXPECT_EQ(cricket::MEDIA_TYPE_DATA, contents[0].media_description()->type()); + EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, contents[1].media_description()->type()); +} + // Tests for MID properties. static void RenameSection(size_t mline_index, @@ -712,6 +823,40 @@ TEST_F(PeerConnectionJsepTest, CreateOfferGeneratesUniqueMidIfAlreadyTaken) { EXPECT_NE(reoffer_contents[0].name, reoffer_contents[1].name); } +// Test that if an audio or video section has the default data section MID, then +// CreateOffer will generate a unique MID for the newly added data section. +TEST_F(PeerConnectionJsepTest, + CreateOfferGeneratesUniqueMidForDataSectionIfAlreadyTaken) { + // First, find what the default MID is for the data channel. + auto pc = CreatePeerConnection(); + pc->CreateDataChannel("dc"); + auto default_offer = pc->CreateOffer(); + std::string default_data_mid = + default_offer->description()->contents()[0].name; + + // Now do an offer/answer with one audio track which has a MID set to the + // default data MID. + auto caller = CreatePeerConnection(); + caller->AddAudioTrack("a"); + auto callee = CreatePeerConnection(); + + auto offer = caller->CreateOffer(); + RenameSection(0, default_data_mid, offer.get()); + + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + + // Add a data channel and ensure that the MID is different. + caller->CreateDataChannel("dc"); + + auto reoffer = caller->CreateOffer(); + auto reoffer_contents = reoffer->description()->contents(); + EXPECT_EQ(default_data_mid, reoffer_contents[0].name); + EXPECT_NE(reoffer_contents[0].name, reoffer_contents[1].name); +} + // Test that a reoffer initiated by the callee adds a new track to the caller. TEST_F(PeerConnectionJsepTest, CalleeDoesReoffer) { auto caller = CreatePeerConnection(); diff --git a/pc/peerconnectionwrapper.cc b/pc/peerconnectionwrapper.cc index d297054696..5d804a8ca7 100644 --- a/pc/peerconnectionwrapper.cc +++ b/pc/peerconnectionwrapper.cc @@ -278,6 +278,11 @@ rtc::scoped_refptr PeerConnectionWrapper::AddVideoTrack( return pc()->AddTrack(CreateVideoTrack(track_label), std::move(streams)); } +rtc::scoped_refptr +PeerConnectionWrapper::CreateDataChannel(const std::string& label) { + return pc()->CreateDataChannel(label, nullptr); +} + PeerConnectionInterface::SignalingState PeerConnectionWrapper::signaling_state() { return pc()->signaling_state(); diff --git a/pc/peerconnectionwrapper.h b/pc/peerconnectionwrapper.h index 9208207df6..0830b5502d 100644 --- a/pc/peerconnectionwrapper.h +++ b/pc/peerconnectionwrapper.h @@ -142,6 +142,11 @@ class PeerConnectionWrapper { const std::string& track_label, std::vector streams = {}); + // Calls the underlying PeerConnection's CreateDataChannel method with default + // initialization parameters. + rtc::scoped_refptr CreateDataChannel( + const std::string& label); + // Returns the signaling state of the underlying PeerConnection. PeerConnectionInterface::SignalingState signaling_state();