From 37cf2455a420124b341ad06ac27fa3c4dbd29d3c Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sun, 14 Oct 2018 19:44:29 +0000 Subject: [PATCH] Revert "Propagate media transport to media channel." This reverts commit 8c16f745ab92cb6d305283e87fa8a661ae500ce4. Reason for revert: Breaks downstream project Original change's description: > 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} TBR=steveanton@webrtc.org,nisse@webrtc.org,psla@webrtc.org,sukhanov@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:9719 Change-Id: Ic78cdc142a2145682ad74eac8b72c71c50f0a5c1 Reviewed-on: https://webrtc-review.googlesource.com/c/105840 Reviewed-by: Oleh Prypin Commit-Queue: Oleh Prypin Cr-Commit-Position: refs/heads/master@{#25154} --- 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, 60 insertions(+), 223 deletions(-) diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index 563ed90120..48970a5fac 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -62,6 +62,9 @@ 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 f1471b63e7..cba3be30ed 100644 --- a/media/base/mediachannel.cc +++ b/media/base/mediachannel.cc @@ -16,18 +16,15 @@ VideoOptions::VideoOptions() = default; VideoOptions::~VideoOptions() = default; MediaChannel::MediaChannel(const MediaConfig& config) - : enable_dscp_(config.enable_dscp) {} + : enable_dscp_(config.enable_dscp), network_interface_(NULL) {} -MediaChannel::MediaChannel() : enable_dscp_(false) {} +MediaChannel::MediaChannel() : enable_dscp_(false), network_interface_(NULL) {} MediaChannel::~MediaChannel() {} -void MediaChannel::SetInterface( - NetworkInterface* iface, - webrtc::MediaTransportInterface* media_transport) { +void MediaChannel::SetInterface(NetworkInterface* iface) { 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 9948d96e5b..ff3368cf40 100644 --- a/media/base/mediachannel.h +++ b/media/base/mediachannel.h @@ -22,7 +22,6 @@ #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" @@ -184,14 +183,8 @@ class MediaChannel : public sigslot::has_slots<> { MediaChannel(); ~MediaChannel() override; - // 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); + // Sets the abstract interface class for sending RTP/RTCP data. + virtual void SetInterface(NetworkInterface* iface); // Called when a RTP packet is received. virtual void OnPacketReceived(rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time) = 0; @@ -258,10 +251,6 @@ 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; @@ -294,8 +283,7 @@ 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_ = nullptr; - webrtc::MediaTransportInterface* media_transport_ = nullptr; + NetworkInterface* network_interface_; }; // The stats information is structured as follows: diff --git a/media/base/rtpdataengine_unittest.cc b/media/base/rtpdataengine_unittest.cc index a636ea41d3..c15c55f5e6 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(), /*media_transport=*/nullptr); + channel->SetInterface(iface_.get()); channel->SignalDataReceived.connect(receiver_.get(), &FakeDataReceiver::OnDataReceived); return channel; diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 087dfdc546..70468a8c86 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1417,13 +1417,8 @@ void WebRtcVideoChannel::OnNetworkRouteChanged( network_route.packet_overhead); } -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); +void WebRtcVideoChannel::SetInterface(NetworkInterface* iface) { + MediaChannel::SetInterface(iface); // 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 bbc1c68b04..8a1e6ad4d1 100644 --- a/media/engine/webrtcvideoengine.h +++ b/media/engine/webrtcvideoengine.h @@ -153,8 +153,7 @@ 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, - webrtc::MediaTransportInterface* media_transport) override; + void SetInterface(NetworkInterface* iface) override; // Implemented for VideoMediaChannelTest. bool sending() const { return sending_; } diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 7aaf43e1a5..2302f8473a 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_, /*media_transport=*/nullptr); + channel_->SetInterface(&network_interface_); 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(), /*media_transport=*/nullptr); + channel->SetInterface(network_interface.get()); // 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(), /*media_transport=*/nullptr); + channel->SetInterface(network_interface.get()); 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(), /*media_transport=*/nullptr); + channel->SetInterface(network_interface.get()); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); } diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 0e586e1465..bb816eed11 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, /*media_transport=*/nullptr); + channel->SetInterface(&network_interface); // 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, /*media_transport=*/nullptr); + channel->SetInterface(&network_interface); 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, /*media_transport=*/nullptr); + channel->SetInterface(&network_interface); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); - channel->SetInterface(nullptr, nullptr); + channel->SetInterface(nullptr); } TEST_F(WebRtcVoiceEngineTestFake, SetOutputVolume) { diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 9b6829f584..b6e9c66fcb 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -514,7 +514,6 @@ 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 91d8cbf669..cd1534ffec 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -155,21 +155,19 @@ void BaseChannel::DisconnectFromRtpTransport() { rtp_transport_->SignalSentPacket.disconnect(this); } -void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport, - webrtc::MediaTransportInterface* media_transport) { +void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_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_transport); + media_channel_->SetInterface(this); } void BaseChannel::Deinit() { RTC_DCHECK(worker_thread_->IsCurrent()); - media_channel_->SetInterface(/*iface=*/nullptr, - /*media_transport=*/nullptr); + media_channel_->SetInterface(NULL); // 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. @@ -1038,7 +1036,7 @@ RtpDataChannel::~RtpDataChannel() { } void RtpDataChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport) { - BaseChannel::Init_w(rtp_transport, /*media_transport=*/nullptr); + BaseChannel::Init_w(rtp_transport); media_channel()->SignalDataReceived.connect(this, &RtpDataChannel::OnDataReceived); media_channel()->SignalReadyToSend.connect( diff --git a/pc/channel.h b/pc/channel.h index cbd9994937..535f64ee68 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -42,7 +42,6 @@ namespace webrtc { class AudioSinkInterface; -class MediaTransportInterface; } // namespace webrtc namespace cricket { @@ -85,8 +84,7 @@ class BaseChannel : public rtc::MessageHandler, bool srtp_required, webrtc::CryptoOptions crypto_options); virtual ~BaseChannel(); - void Init_w(webrtc::RtpTransportInternal* rtp_transport, - webrtc::MediaTransportInterface* media_transport); + void Init_w(webrtc::RtpTransportInternal* rtp_transport); // Deinit may be called multiple times and is simply ignored if it's already // done. @@ -164,11 +162,6 @@ 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); @@ -314,11 +307,6 @@ 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 f17032c3b5..94f38c9a39 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, /*media_transport=*/nullptr); + channel->Init_w(rtp_transport); 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, /*media_transport=*/nullptr); + channel->Init_w(rtp_transport); return channel; } diff --git a/pc/channelmanager.cc b/pc/channelmanager.cc index eb27bd7d52..1c80719949 100644 --- a/pc/channelmanager.cc +++ b/pc/channelmanager.cc @@ -156,7 +156,6 @@ 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, @@ -165,8 +164,8 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { return CreateVoiceChannel(call, media_config, rtp_transport, - media_transport, signaling_thread, content_name, - srtp_required, crypto_options, options); + signaling_thread, content_name, srtp_required, + crypto_options, options); }); } @@ -188,7 +187,7 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options); - voice_channel->Init_w(rtp_transport, media_transport); + voice_channel->Init_w(rtp_transport); VoiceChannel* voice_channel_ptr = voice_channel.get(); voice_channels_.push_back(std::move(voice_channel)); @@ -254,9 +253,7 @@ VideoChannel* ChannelManager::CreateVideoChannel( worker_thread_, network_thread_, signaling_thread, absl::WrapUnique(media_channel), content_name, srtp_required, crypto_options); - - // TODO(sukhanov): Add media_transport support for video channel. - video_channel->Init_w(rtp_transport, /*media_transport=*/nullptr); + video_channel->Init_w(rtp_transport); 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 5cafd8cfa7..6430f8ee84 100644 --- a/pc/channelmanager.h +++ b/pc/channelmanager.h @@ -80,16 +80,14 @@ 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, - webrtc::MediaTransportInterface* media_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, + 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 6e9cab6cda..053166ba13 100644 --- a/pc/channelmanager_unittest.cc +++ b/pc/channelmanager_unittest.cc @@ -11,7 +11,6 @@ #include #include -#include "api/test/fake_media_transport.h" #include "media/base/fakemediaengine.h" #include "media/base/testutils.h" #include "media/engine/fakewebrtccall.h" @@ -62,21 +61,9 @@ class ChannelManagerTest : public testing::Test { return dtls_srtp_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) { + void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) { cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, media_transport, + &fake_call_, cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, webrtc::CryptoOptions(), AudioOptions()); EXPECT_TRUE(voice_channel != nullptr); @@ -103,7 +90,6 @@ 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. @@ -168,15 +154,7 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { TEST_F(ChannelManagerTest, CreateDestroyChannels) { EXPECT_TRUE(cm_->Init()); auto rtp_transport = CreateDtlsSrtpTransport(); - 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()); + TestCreateDestroyChannels(rtp_transport.get()); } TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { @@ -186,7 +164,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(), /*media_transport=*/nullptr); + TestCreateDestroyChannels(rtp_transport.get()); } } // namespace cricket diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index f806a4356c..cce75286f2 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -939,18 +939,6 @@ 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)); @@ -5524,11 +5512,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 = GetRtpTransport(mid); - MediaTransportInterface* media_transport = GetMediaTransport(mid); - + RtpTransportInternal* rtp_transport = + transport_controller_->GetRtpTransport(mid); + RTC_DCHECK(rtp_transport); cricket::VoiceChannel* voice_channel = channel_manager()->CreateVoiceChannel( - call_.get(), configuration_.media_config, rtp_transport, media_transport, + call_.get(), configuration_.media_config, rtp_transport, signaling_thread(), mid, SrtpRequired(), factory_->options().crypto_options, audio_options_); if (!voice_channel) { @@ -5546,9 +5534,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 = GetRtpTransport(mid); - - // TODO(sukhanov): Propagate media_transport to video channel. + RtpTransportInternal* rtp_transport = + transport_controller_->GetRtpTransport(mid); + RTC_DCHECK(rtp_transport); cricket::VideoChannel* video_channel = channel_manager()->CreateVideoChannel( call_.get(), configuration_.media_config, rtp_transport, signaling_thread(), mid, SrtpRequired(), @@ -5583,7 +5571,9 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { channel->OnTransportChannelCreated(); } } else { - RtpTransportInternal* rtp_transport = GetRtpTransport(mid); + RtpTransportInternal* rtp_transport = + transport_controller_->GetRtpTransport(mid); + RTC_DCHECK(rtp_transport); 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 0cd976a1df..b42d620107 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -915,22 +915,6 @@ 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 6b500915f0..6f4fe1e3f4 100644 --- a/pc/peerconnection_media_unittest.cc +++ b/pc/peerconnection_media_unittest.cc @@ -15,7 +15,6 @@ #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" @@ -72,26 +71,13 @@ 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(); - - 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 pc_factory = CreateModularPeerConnectionFactory( + rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(), + std::move(media_engine), CreateCallFactory(), + CreateRtcEventLogFactory()); auto fake_port_allocator = absl::make_unique( rtc::Thread::Current(), nullptr); @@ -1086,69 +1072,6 @@ 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 70b1601bcb..fad839cc4c 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(), - /*media_transport=*/nullptr, rtc::Thread::Current(), cricket::CN_AUDIO, - srtp_required, webrtc::CryptoOptions(), cricket::AudioOptions()); + 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,