From 8c16f745ab92cb6d305283e87fa8a661ae500ce4 Mon Sep 17 00:00:00 2001 From: Anton Sukhanov Date: Fri, 12 Oct 2018 14:59:21 -0700 Subject: [PATCH] Propagate media transport to media channel. 1. Pass media transport factory to JSEP transport controller. 2. Pass media transport to voice media channel. 3. Add basic unit test that make sure if peer connection is created with media transport, it is propagated to voice media channel. Change-Id: Ie922db78ade0efd893e019cd2b4441a9947a2f71 Bug: webrtc:9719 Reviewed-on: https://webrtc-review.googlesource.com/c/105542 Reviewed-by: Steve Anton Reviewed-by: Niels Moller Reviewed-by: Peter Slatala Commit-Queue: Anton Sukhanov Cr-Commit-Position: refs/heads/master@{#25152} --- api/test/fake_media_transport.h | 3 - media/base/mediachannel.cc | 9 ++- media/base/mediachannel.h | 18 ++++- media/base/rtpdataengine_unittest.cc | 2 +- media/engine/webrtcvideoengine.cc | 9 ++- media/engine/webrtcvideoengine.h | 3 +- media/engine/webrtcvideoengine_unittest.cc | 8 +- media/engine/webrtcvoiceengine_unittest.cc | 8 +- pc/BUILD.gn | 1 + pc/channel.cc | 10 ++- pc/channel.h | 14 +++- pc/channel_unittest.cc | 4 +- pc/channelmanager.cc | 11 ++- pc/channelmanager.h | 18 +++-- pc/channelmanager_unittest.cc | 30 +++++++- pc/peerconnection.cc | 30 +++++--- pc/peerconnection.h | 16 ++++ pc/peerconnection_media_unittest.cc | 85 +++++++++++++++++++++- pc/rtpsenderreceiver_unittest.cc | 4 +- 19 files changed, 223 insertions(+), 60 deletions(-) diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index 48970a5fac..563ed90120 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -62,9 +62,6 @@ class FakeMediaTransportFactory : public MediaTransportFactory { rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, bool is_caller) override { - RTC_CHECK(network_thread != nullptr); - RTC_CHECK(packet_transport != nullptr); - std::unique_ptr media_transport = absl::make_unique(is_caller); diff --git a/media/base/mediachannel.cc b/media/base/mediachannel.cc index cba3be30ed..f1471b63e7 100644 --- a/media/base/mediachannel.cc +++ b/media/base/mediachannel.cc @@ -16,15 +16,18 @@ VideoOptions::VideoOptions() = default; VideoOptions::~VideoOptions() = default; MediaChannel::MediaChannel(const MediaConfig& config) - : enable_dscp_(config.enable_dscp), network_interface_(NULL) {} + : enable_dscp_(config.enable_dscp) {} -MediaChannel::MediaChannel() : enable_dscp_(false), network_interface_(NULL) {} +MediaChannel::MediaChannel() : enable_dscp_(false) {} MediaChannel::~MediaChannel() {} -void MediaChannel::SetInterface(NetworkInterface* iface) { +void MediaChannel::SetInterface( + NetworkInterface* iface, + webrtc::MediaTransportInterface* media_transport) { rtc::CritScope cs(&network_interface_crit_); network_interface_ = iface; + media_transport_ = media_transport; SetDscp(enable_dscp_ ? PreferredDscp() : rtc::DSCP_DEFAULT); } diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h index ff3368cf40..9948d96e5b 100644 --- a/media/base/mediachannel.h +++ b/media/base/mediachannel.h @@ -22,6 +22,7 @@ #include "api/audio_options.h" #include "api/crypto/framedecryptorinterface.h" #include "api/crypto/frameencryptorinterface.h" +#include "api/media_transport_interface.h" #include "api/rtcerror.h" #include "api/rtpparameters.h" #include "api/rtpreceiverinterface.h" @@ -183,8 +184,14 @@ class MediaChannel : public sigslot::has_slots<> { MediaChannel(); ~MediaChannel() override; - // Sets the abstract interface class for sending RTP/RTCP data. - virtual void SetInterface(NetworkInterface* iface); + // Sets the abstract interface class for sending RTP/RTCP data and + // interface for media transport (experimental). If media transport is + // provided, it should be used instead of RTP/RTCP. + // TODO(sukhanov): Currently media transport can co-exist with RTP/RTCP, but + // in the future we will refactor code to send all frames with media + // transport. + virtual void SetInterface(NetworkInterface* iface, + webrtc::MediaTransportInterface* media_transport); // Called when a RTP packet is received. virtual void OnPacketReceived(rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time) = 0; @@ -251,6 +258,10 @@ class MediaChannel : public sigslot::has_slots<> { return network_interface_->SetOption(type, opt, option); } + webrtc::MediaTransportInterface* media_transport() { + return media_transport_; + } + protected: virtual rtc::DiffServCodePoint PreferredDscp() const; @@ -283,7 +294,8 @@ class MediaChannel : public sigslot::has_slots<> { // from any MediaEngine threads. This critical section is to protect accessing // of network_interface_ object. rtc::CriticalSection network_interface_crit_; - NetworkInterface* network_interface_; + NetworkInterface* network_interface_ = nullptr; + webrtc::MediaTransportInterface* media_transport_ = nullptr; }; // The stats information is structured as follows: diff --git a/media/base/rtpdataengine_unittest.cc b/media/base/rtpdataengine_unittest.cc index c15c55f5e6..a636ea41d3 100644 --- a/media/base/rtpdataengine_unittest.cc +++ b/media/base/rtpdataengine_unittest.cc @@ -73,7 +73,7 @@ class RtpDataMediaChannelTest : public testing::Test { cricket::MediaConfig config; cricket::RtpDataMediaChannel* channel = static_cast(dme->CreateChannel(config)); - channel->SetInterface(iface_.get()); + channel->SetInterface(iface_.get(), /*media_transport=*/nullptr); channel->SignalDataReceived.connect(receiver_.get(), &FakeDataReceiver::OnDataReceived); return channel; diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 70468a8c86..087dfdc546 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1417,8 +1417,13 @@ void WebRtcVideoChannel::OnNetworkRouteChanged( network_route.packet_overhead); } -void WebRtcVideoChannel::SetInterface(NetworkInterface* iface) { - MediaChannel::SetInterface(iface); +void WebRtcVideoChannel::SetInterface( + NetworkInterface* iface, + webrtc::MediaTransportInterface* media_transport) { + // TODO(sukhanov): Video is not currently supported with media transport. + RTC_CHECK(media_transport == nullptr); + + MediaChannel::SetInterface(iface, media_transport); // Set the RTP recv/send buffer to a bigger size. // The group here can be either a positive integer with an explicit size, in diff --git a/media/engine/webrtcvideoengine.h b/media/engine/webrtcvideoengine.h index 8a1e6ad4d1..bbc1c68b04 100644 --- a/media/engine/webrtcvideoengine.h +++ b/media/engine/webrtcvideoengine.h @@ -153,7 +153,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { void OnReadyToSend(bool ready) override; void OnNetworkRouteChanged(const std::string& transport_name, const rtc::NetworkRoute& network_route) override; - void SetInterface(NetworkInterface* iface) override; + void SetInterface(NetworkInterface* iface, + webrtc::MediaTransportInterface* media_transport) override; // Implemented for VideoMediaChannelTest. bool sending() const { return sending_; } diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 2302f8473a..7aaf43e1a5 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -1260,7 +1260,7 @@ class WebRtcVideoChannelBaseTest : public testing::Test { channel_->OnReadyToSend(true); EXPECT_TRUE(channel_.get() != NULL); network_interface_.SetDestination(channel_.get()); - channel_->SetInterface(&network_interface_); + channel_->SetInterface(&network_interface_, /*media_transport=*/nullptr); cricket::VideoRecvParameters parameters; parameters.codecs = engine_.codecs(); channel_->SetRecvParameters(parameters); @@ -4597,14 +4597,14 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { channel.reset(static_cast( engine_.CreateChannel(call_.get(), config, VideoOptions()))); - channel->SetInterface(network_interface.get()); + channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); config.enable_dscp = true; channel.reset(static_cast( engine_.CreateChannel(call_.get(), config, VideoOptions()))); - channel->SetInterface(network_interface.get()); + channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp()); // Packets should also self-identify their dscp in PacketOptions. @@ -4618,7 +4618,7 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { config.enable_dscp = false; channel.reset(static_cast( engine_.CreateChannel(call_.get(), config, VideoOptions()))); - channel->SetInterface(network_interface.get()); + channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); } diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index bb816eed11..0e586e1465 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -3027,14 +3027,14 @@ TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { channel.reset(static_cast( engine_->CreateChannel(&call_, config, cricket::AudioOptions()))); - channel->SetInterface(&network_interface); + channel->SetInterface(&network_interface, /*media_transport=*/nullptr); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); config.enable_dscp = true; channel.reset(static_cast( engine_->CreateChannel(&call_, config, cricket::AudioOptions()))); - channel->SetInterface(&network_interface); + channel->SetInterface(&network_interface, /*media_transport=*/nullptr); EXPECT_EQ(rtc::DSCP_EF, network_interface.dscp()); // Packets should also self-identify their dscp in PacketOptions. @@ -3047,11 +3047,11 @@ TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { config.enable_dscp = false; channel.reset(static_cast( engine_->CreateChannel(&call_, config, cricket::AudioOptions()))); - channel->SetInterface(&network_interface); + channel->SetInterface(&network_interface, /*media_transport=*/nullptr); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); - channel->SetInterface(nullptr); + channel->SetInterface(nullptr, nullptr); } TEST_F(WebRtcVoiceEngineTestFake, SetOutputVolume) { diff --git a/pc/BUILD.gn b/pc/BUILD.gn index b6e9c66fcb..9b6829f584 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -514,6 +514,7 @@ if (rtc_include_tests) { ":pc_test_utils", "..:webrtc_common", "../api:callfactory_api", + "../api:fake_media_transport", "../api:libjingle_peerconnection_test_api", "../api:rtc_stats_api", "../api/audio_codecs:audio_codecs_api", diff --git a/pc/channel.cc b/pc/channel.cc index cd1534ffec..91d8cbf669 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -155,19 +155,21 @@ void BaseChannel::DisconnectFromRtpTransport() { rtp_transport_->SignalSentPacket.disconnect(this); } -void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport) { +void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport, + webrtc::MediaTransportInterface* media_transport) { RTC_DCHECK_RUN_ON(worker_thread_); network_thread_->Invoke( RTC_FROM_HERE, [this, rtp_transport] { SetRtpTransport(rtp_transport); }); // Both RTP and RTCP channels should be set, we can call SetInterface on // the media channel and it can set network options. - media_channel_->SetInterface(this); + media_channel_->SetInterface(this, media_transport); } void BaseChannel::Deinit() { RTC_DCHECK(worker_thread_->IsCurrent()); - media_channel_->SetInterface(NULL); + media_channel_->SetInterface(/*iface=*/nullptr, + /*media_transport=*/nullptr); // Packets arrive on the network thread, processing packets calls virtual // functions, so need to stop this process in Deinit that is called in // derived classes destructor. @@ -1036,7 +1038,7 @@ RtpDataChannel::~RtpDataChannel() { } void RtpDataChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport) { - BaseChannel::Init_w(rtp_transport); + BaseChannel::Init_w(rtp_transport, /*media_transport=*/nullptr); media_channel()->SignalDataReceived.connect(this, &RtpDataChannel::OnDataReceived); media_channel()->SignalReadyToSend.connect( diff --git a/pc/channel.h b/pc/channel.h index 535f64ee68..cbd9994937 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -42,6 +42,7 @@ namespace webrtc { class AudioSinkInterface; +class MediaTransportInterface; } // namespace webrtc namespace cricket { @@ -84,7 +85,8 @@ class BaseChannel : public rtc::MessageHandler, bool srtp_required, webrtc::CryptoOptions crypto_options); virtual ~BaseChannel(); - void Init_w(webrtc::RtpTransportInternal* rtp_transport); + void Init_w(webrtc::RtpTransportInternal* rtp_transport, + webrtc::MediaTransportInterface* media_transport); // Deinit may be called multiple times and is simply ignored if it's already // done. @@ -162,6 +164,11 @@ class BaseChannel : public rtc::MessageHandler, return nullptr; } + // Returns media transport, can be null if media transport is not available. + webrtc::MediaTransportInterface* media_transport() { + return media_transport_; + } + // From RtpTransport - public for testing only void OnTransportReadyToSend(bool ready); @@ -307,6 +314,11 @@ class BaseChannel : public rtc::MessageHandler, webrtc::RtpTransportInternal* rtp_transport_ = nullptr; + // Optional media transport (experimental). + // If provided, audio and video will be sent through media_transport instead + // of RTP/RTCP. Currently media_transport can co-exist with rtp_transport. + webrtc::MediaTransportInterface* media_transport_ = nullptr; + std::vector > socket_options_; std::vector > rtcp_socket_options_; bool writable_ = false; diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 94f38c9a39..f17032c3b5 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -251,7 +251,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { auto channel = absl::make_unique( worker_thread, network_thread, signaling_thread, engine, std::move(ch), cricket::CN_AUDIO, (flags & DTLS) != 0, webrtc::CryptoOptions()); - channel->Init_w(rtp_transport); + channel->Init_w(rtp_transport, /*media_transport=*/nullptr); return channel; } @@ -1546,7 +1546,7 @@ std::unique_ptr ChannelTest::CreateChannel( auto channel = absl::make_unique( worker_thread, network_thread, signaling_thread, std::move(ch), cricket::CN_VIDEO, (flags & DTLS) != 0, webrtc::CryptoOptions()); - channel->Init_w(rtp_transport); + channel->Init_w(rtp_transport, /*media_transport=*/nullptr); return channel; } diff --git a/pc/channelmanager.cc b/pc/channelmanager.cc index 1c80719949..eb27bd7d52 100644 --- a/pc/channelmanager.cc +++ b/pc/channelmanager.cc @@ -156,6 +156,7 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( webrtc::Call* call, const cricket::MediaConfig& media_config, webrtc::RtpTransportInternal* rtp_transport, + webrtc::MediaTransportInterface* media_transport, rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, @@ -164,8 +165,8 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { return CreateVoiceChannel(call, media_config, rtp_transport, - signaling_thread, content_name, srtp_required, - crypto_options, options); + media_transport, signaling_thread, content_name, + srtp_required, crypto_options, options); }); } @@ -187,7 +188,7 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options); - voice_channel->Init_w(rtp_transport); + voice_channel->Init_w(rtp_transport, media_transport); VoiceChannel* voice_channel_ptr = voice_channel.get(); voice_channels_.push_back(std::move(voice_channel)); @@ -253,7 +254,9 @@ VideoChannel* ChannelManager::CreateVideoChannel( worker_thread_, network_thread_, signaling_thread, absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options); - video_channel->Init_w(rtp_transport); + + // TODO(sukhanov): Add media_transport support for video channel. + video_channel->Init_w(rtp_transport, /*media_transport=*/nullptr); VideoChannel* video_channel_ptr = video_channel.get(); video_channels_.push_back(std::move(video_channel)); diff --git a/pc/channelmanager.h b/pc/channelmanager.h index 6430f8ee84..5cafd8cfa7 100644 --- a/pc/channelmanager.h +++ b/pc/channelmanager.h @@ -80,14 +80,16 @@ class ChannelManager final { // call the appropriate Destroy*Channel method when done. // Creates a voice channel, to be associated with the specified session. - VoiceChannel* CreateVoiceChannel(webrtc::Call* call, - const cricket::MediaConfig& media_config, - webrtc::RtpTransportInternal* rtp_transport, - rtc::Thread* signaling_thread, - const std::string& content_name, - bool srtp_required, - const webrtc::CryptoOptions& crypto_options, - const AudioOptions& options); + VoiceChannel* CreateVoiceChannel( + webrtc::Call* call, + const cricket::MediaConfig& media_config, + webrtc::RtpTransportInternal* rtp_transport, + webrtc::MediaTransportInterface* media_transport, + rtc::Thread* signaling_thread, + const std::string& content_name, + bool srtp_required, + const webrtc::CryptoOptions& crypto_options, + const AudioOptions& options); // Destroys a voice channel created by CreateVoiceChannel. void DestroyVoiceChannel(VoiceChannel* voice_channel); diff --git a/pc/channelmanager_unittest.cc b/pc/channelmanager_unittest.cc index 053166ba13..6e9cab6cda 100644 --- a/pc/channelmanager_unittest.cc +++ b/pc/channelmanager_unittest.cc @@ -11,6 +11,7 @@ #include #include +#include "api/test/fake_media_transport.h" #include "media/base/fakemediaengine.h" #include "media/base/testutils.h" #include "media/engine/fakewebrtccall.h" @@ -61,9 +62,21 @@ class ChannelManagerTest : public testing::Test { return dtls_srtp_transport; } - void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) { + std::unique_ptr CreateMediaTransport( + rtc::PacketTransportInternal* packet_transport) { + auto media_transport_result = + fake_media_transport_factory_.CreateMediaTransport(packet_transport, + network_.get(), + /*is_caller=*/true); + RTC_CHECK(media_transport_result.ok()); + return media_transport_result.MoveValue(); + } + + void TestCreateDestroyChannels( + webrtc::RtpTransportInternal* rtp_transport, + webrtc::MediaTransportInterface* media_transport) { cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, + &fake_call_, cricket::MediaConfig(), rtp_transport, media_transport, rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, webrtc::CryptoOptions(), AudioOptions()); EXPECT_TRUE(voice_channel != nullptr); @@ -90,6 +103,7 @@ class ChannelManagerTest : public testing::Test { cricket::FakeDataEngine* fdme_; std::unique_ptr cm_; cricket::FakeCall fake_call_; + webrtc::FakeMediaTransportFactory fake_media_transport_factory_; }; // Test that we startup/shutdown properly. @@ -154,7 +168,15 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { TEST_F(ChannelManagerTest, CreateDestroyChannels) { EXPECT_TRUE(cm_->Init()); auto rtp_transport = CreateDtlsSrtpTransport(); - TestCreateDestroyChannels(rtp_transport.get()); + TestCreateDestroyChannels(rtp_transport.get(), /*media_transport=*/nullptr); +} + +TEST_F(ChannelManagerTest, CreateDestroyChannelsWithMediaTransport) { + EXPECT_TRUE(cm_->Init()); + auto rtp_transport = CreateDtlsSrtpTransport(); + auto media_transport = + CreateMediaTransport(rtp_transport->rtcp_packet_transport()); + TestCreateDestroyChannels(rtp_transport.get(), media_transport.get()); } TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { @@ -164,7 +186,7 @@ TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { EXPECT_TRUE(cm_->set_network_thread(network_.get())); EXPECT_TRUE(cm_->Init()); auto rtp_transport = CreateDtlsSrtpTransport(); - TestCreateDestroyChannels(rtp_transport.get()); + TestCreateDestroyChannels(rtp_transport.get(), /*media_transport=*/nullptr); } } // namespace cricket diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index cce75286f2..f806a4356c 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -939,6 +939,18 @@ bool PeerConnection::Initialize( config.enable_external_auth = true; #endif config.active_reset_srtp_params = configuration.active_reset_srtp_params; + + if (configuration.use_media_transport) { + if (!factory_->media_transport_factory()) { + RTC_DCHECK(false) + << "PeerConnecton is initialized with use_media_transport = true, " + << "but media transport factory is not set in PeerConnectioFactory"; + return false; + } + + config.media_transport_factory = factory_->media_transport_factory(); + } + transport_controller_.reset(new JsepTransportController( signaling_thread(), network_thread(), port_allocator_.get(), async_resolver_factory_.get(), config)); @@ -5512,11 +5524,11 @@ RTCError PeerConnection::CreateChannels(const SessionDescription& desc) { // TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. cricket::VoiceChannel* PeerConnection::CreateVoiceChannel( const std::string& mid) { - RtpTransportInternal* rtp_transport = - transport_controller_->GetRtpTransport(mid); - RTC_DCHECK(rtp_transport); + RtpTransportInternal* rtp_transport = GetRtpTransport(mid); + MediaTransportInterface* media_transport = GetMediaTransport(mid); + cricket::VoiceChannel* voice_channel = channel_manager()->CreateVoiceChannel( - call_.get(), configuration_.media_config, rtp_transport, + call_.get(), configuration_.media_config, rtp_transport, media_transport, signaling_thread(), mid, SrtpRequired(), factory_->options().crypto_options, audio_options_); if (!voice_channel) { @@ -5534,9 +5546,9 @@ cricket::VoiceChannel* PeerConnection::CreateVoiceChannel( // TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. cricket::VideoChannel* PeerConnection::CreateVideoChannel( const std::string& mid) { - RtpTransportInternal* rtp_transport = - transport_controller_->GetRtpTransport(mid); - RTC_DCHECK(rtp_transport); + RtpTransportInternal* rtp_transport = GetRtpTransport(mid); + + // TODO(sukhanov): Propagate media_transport to video channel. cricket::VideoChannel* video_channel = channel_manager()->CreateVideoChannel( call_.get(), configuration_.media_config, rtp_transport, signaling_thread(), mid, SrtpRequired(), @@ -5571,9 +5583,7 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { channel->OnTransportChannelCreated(); } } else { - RtpTransportInternal* rtp_transport = - transport_controller_->GetRtpTransport(mid); - RTC_DCHECK(rtp_transport); + RtpTransportInternal* rtp_transport = GetRtpTransport(mid); rtp_data_channel_ = channel_manager()->CreateRtpDataChannel( configuration_.media_config, rtp_transport, signaling_thread(), mid, SrtpRequired(), factory_->options().crypto_options); diff --git a/pc/peerconnection.h b/pc/peerconnection.h index b42d620107..0cd976a1df 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -915,6 +915,22 @@ class PeerConnection : public PeerConnectionInternal, // Returns the observer. Will crash on CHECK if the observer is removed. PeerConnectionObserver* Observer() const; + // Returns rtp transport, result can not be nullptr. + RtpTransportInternal* GetRtpTransport(const std::string& mid) { + auto rtp_transport = transport_controller_->GetRtpTransport(mid); + RTC_DCHECK(rtp_transport); + return rtp_transport; + } + + // Returns media transport, if PeerConnection was created with configuration + // to use media transport. Otherwise returns nullptr. + MediaTransportInterface* GetMediaTransport(const std::string& mid) { + auto media_transport = transport_controller_->GetMediaTransport(mid); + RTC_DCHECK(configuration_.use_media_transport == + (media_transport != nullptr)); + return media_transport; + } + sigslot::signal1 SignalDataChannelCreated_; // Storing the factory as a scoped reference pointer ensures that the memory diff --git a/pc/peerconnection_media_unittest.cc b/pc/peerconnection_media_unittest.cc index 6f4fe1e3f4..6b500915f0 100644 --- a/pc/peerconnection_media_unittest.cc +++ b/pc/peerconnection_media_unittest.cc @@ -15,6 +15,7 @@ #include #include "api/call/callfactoryinterface.h" +#include "api/test/fake_media_transport.h" #include "logging/rtc_event_log/rtc_event_log_factory.h" #include "media/base/fakemediaengine.h" #include "p2p/base/fakeportallocator.h" @@ -71,13 +72,26 @@ class PeerConnectionMediaBaseTest : public ::testing::Test { return CreatePeerConnection(RTCConfiguration()); } + // Creates PeerConnectionFactory and PeerConnection for given configuration. + // Note that PeerConnectionFactory is created with MediaTransportFactory, + // because some tests pass config.use_media_transport = true. WrapperPtr CreatePeerConnection(const RTCConfiguration& config) { auto media_engine = absl::make_unique(); auto* media_engine_ptr = media_engine.get(); - auto pc_factory = CreateModularPeerConnectionFactory( - rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(), - std::move(media_engine), CreateCallFactory(), - CreateRtcEventLogFactory()); + + PeerConnectionFactoryDependencies factory_dependencies; + + factory_dependencies.network_thread = rtc::Thread::Current(); + factory_dependencies.worker_thread = rtc::Thread::Current(); + factory_dependencies.signaling_thread = rtc::Thread::Current(); + factory_dependencies.media_engine = std::move(media_engine); + factory_dependencies.call_factory = CreateCallFactory(); + factory_dependencies.event_log_factory = CreateRtcEventLogFactory(); + factory_dependencies.media_transport_factory = + absl::make_unique(); + + auto pc_factory = + CreateModularPeerConnectionFactory(std::move(factory_dependencies)); auto fake_port_allocator = absl::make_unique( rtc::Thread::Current(), nullptr); @@ -1072,6 +1086,69 @@ TEST_P(PeerConnectionMediaTest, audio_options.combined_audio_video_bwe); } +TEST_P(PeerConnectionMediaTest, MediaTransportPropagatedToVoiceEngine) { + RTCConfiguration config; + + // Setup PeerConnection to use media transport. + config.use_media_transport = true; + + auto caller = CreatePeerConnectionWithAudioVideo(config); + auto callee = CreatePeerConnectionWithAudioVideo(config); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + auto answer = callee->CreateAnswer(); + ASSERT_TRUE(callee->SetLocalDescription(std::move(answer))); + + auto caller_voice = caller->media_engine()->GetVoiceChannel(0); + auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(caller_voice); + ASSERT_TRUE(callee_voice); + + // Make sure media transport is propagated to voice channel. + FakeMediaTransport* caller_voice_media_transport = + static_cast(caller_voice->media_transport()); + FakeMediaTransport* callee_voice_media_transport = + static_cast(callee_voice->media_transport()); + ASSERT_NE(nullptr, caller_voice_media_transport); + ASSERT_NE(nullptr, callee_voice_media_transport); + + // Make sure media transport is created with correct is_caller. + EXPECT_TRUE(caller_voice_media_transport->is_caller()); + EXPECT_FALSE(callee_voice_media_transport->is_caller()); + + // TODO(sukhanov): Propagate media transport to video channel. This test + // will fail once media transport is propagated to video channel and it will + // serve as a reminder to add a test for video channel propagation. + auto caller_video = caller->media_engine()->GetVideoChannel(0); + auto callee_video = callee->media_engine()->GetVideoChannel(0); + ASSERT_EQ(nullptr, caller_video->media_transport()); + ASSERT_EQ(nullptr, callee_video->media_transport()); +} + +TEST_P(PeerConnectionMediaTest, MediaTransportNotPropagatedToVoiceEngine) { + auto caller = CreatePeerConnectionWithAudioVideo(); + auto callee = CreatePeerConnectionWithAudioVideo(); + + ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); + auto answer = callee->CreateAnswer(); + ASSERT_TRUE(callee->SetLocalDescription(std::move(answer))); + + auto caller_voice = caller->media_engine()->GetVoiceChannel(0); + auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(caller_voice); + ASSERT_TRUE(callee_voice); + + // Since we did not setup PeerConnection to use media transport, media + // transport should not be created / propagated to the voice engine. + ASSERT_EQ(nullptr, caller_voice->media_transport()); + ASSERT_EQ(nullptr, callee_voice->media_transport()); + + auto caller_video = caller->media_engine()->GetVideoChannel(0); + auto callee_video = callee->media_engine()->GetVideoChannel(0); + ASSERT_EQ(nullptr, caller_video->media_transport()); + ASSERT_EQ(nullptr, callee_video->media_transport()); +} + INSTANTIATE_TEST_CASE_P(PeerConnectionMediaTest, PeerConnectionMediaTest, Values(SdpSemantics::kPlanB, diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index fad839cc4c..70b1601bcb 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -80,8 +80,8 @@ class RtpSenderReceiverTest : public testing::Test, voice_channel_ = channel_manager_.CreateVoiceChannel( &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), - rtc::Thread::Current(), cricket::CN_AUDIO, srtp_required, - webrtc::CryptoOptions(), cricket::AudioOptions()); + /*media_transport=*/nullptr, rtc::Thread::Current(), cricket::CN_AUDIO, + srtp_required, webrtc::CryptoOptions(), cricket::AudioOptions()); video_channel_ = channel_manager_.CreateVideoChannel( &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), rtc::Thread::Current(), cricket::CN_VIDEO, srtp_required,