diff --git a/pc/channel.cc b/pc/channel.cc index be40a7e446..13341e3e1b 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -254,8 +254,7 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { rtp_transport_ = rtp_transport; if (rtp_transport_) { - RTC_DCHECK(rtp_transport_->rtp_packet_transport()); - transport_name_ = rtp_transport_->rtp_packet_transport()->transport_name(); + transport_name_ = rtp_transport_->transport_name(); if (!ConnectToRtpTransport()) { RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport."; @@ -266,13 +265,11 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { // Set the cached socket options. for (const auto& pair : socket_options_) { - rtp_transport_->rtp_packet_transport()->SetOption(pair.first, - pair.second); + rtp_transport_->SetRtpOption(pair.first, pair.second); } - if (rtp_transport_->rtcp_packet_transport()) { + if (!rtp_transport_->rtcp_mux_enabled()) { for (const auto& pair : rtcp_socket_options_) { - rtp_transport_->rtp_packet_transport()->SetOption(pair.first, - pair.second); + rtp_transport_->SetRtcpOption(pair.first, pair.second); } } } @@ -348,20 +345,17 @@ int BaseChannel::SetOption_n(SocketType type, int value) { RTC_DCHECK(network_thread_->IsCurrent()); RTC_DCHECK(rtp_transport_); - rtc::PacketTransportInternal* transport = nullptr; switch (type) { case ST_RTP: - transport = rtp_transport_->rtp_packet_transport(); socket_options_.push_back( std::pair(opt, value)); - break; + return rtp_transport_->SetRtpOption(opt, value); case ST_RTCP: - transport = rtp_transport_->rtcp_packet_transport(); rtcp_socket_options_.push_back( std::pair(opt, value)); - break; + return rtp_transport_->SetRtcpOption(opt, value); } - return transport ? transport->SetOption(opt, value) : -1; + return -1; } void BaseChannel::OnWritableState(bool writable) { diff --git a/pc/channel.h b/pc/channel.h index 9d41419cc7..8a75a1a5f3 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -120,6 +120,8 @@ class BaseChannel : public ChannelInterface, // internally. It would replace the |SetTransports| and its variants. bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) override; + webrtc::RtpTransportInternal* rtp_transport() const { return rtp_transport_; } + // Channel control bool SetLocalContent(const MediaContentDescription* content, webrtc::SdpType type, @@ -154,20 +156,6 @@ class BaseChannel : public ChannelInterface, // Fired on the network thread. sigslot::signal1 SignalRtcpMuxFullyActive; - rtc::PacketTransportInternal* rtp_packet_transport() { - if (rtp_transport_) { - return rtp_transport_->rtp_packet_transport(); - } - return nullptr; - } - - rtc::PacketTransportInternal* rtcp_packet_transport() { - if (rtp_transport_) { - return rtp_transport_->rtcp_packet_transport(); - } - return nullptr; - } - // Returns media transport, can be null if media transport is not available. webrtc::MediaTransportInterface* media_transport() { return media_transport_config_.media_transport; diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 91cd0c77e7..3d7e01ab57 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -193,8 +193,7 @@ TEST_F(ChannelManagerTest, CreateDestroyChannels) { TEST_F(ChannelManagerTest, CreateDestroyChannelsWithMediaTransport) { EXPECT_TRUE(cm_->Init()); auto rtp_transport = CreateDtlsSrtpTransport(); - auto media_transport = - CreateMediaTransport(rtp_transport->rtcp_packet_transport()); + auto media_transport = CreateMediaTransport(rtp_dtls_transport_.get()); TestCreateDestroyChannels( rtp_transport.get(), webrtc::MediaTransportConfig(media_transport.get())); } diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 4a64f7aa7a..e4252b2e8b 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -971,8 +971,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_EQ(nullptr, channel1_->rtcp_packet_transport()); - EXPECT_EQ(nullptr, channel2_->rtcp_packet_transport()); + EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -1000,8 +1000,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(0, 0); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_NE(nullptr, channel1_->rtcp_packet_transport()); - EXPECT_NE(nullptr, channel2_->rtcp_packet_transport()); + EXPECT_FALSE(channel1_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_FALSE(channel2_->rtp_transport()->rtcp_mux_enabled()); SendRtcp1(); SendRtcp2(); WaitForThreads(); @@ -1047,8 +1047,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(SendProvisionalAnswer()); EXPECT_TRUE(channel1_->srtp_active()); EXPECT_TRUE(channel2_->srtp_active()); - EXPECT_EQ(nullptr, channel1_->rtcp_packet_transport()); - EXPECT_EQ(nullptr, channel2_->rtcp_packet_transport()); + EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); WaitForThreads(); // Wait for 'sending' flag go through network thread. SendCustomRtcp1(kSsrc1); SendCustomRtp1(kSsrc1, ++sequence_number1_1); @@ -1107,8 +1107,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_EQ(nullptr, channel1_->rtcp_packet_transport()); - EXPECT_EQ(nullptr, channel2_->rtcp_packet_transport()); + EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -1343,8 +1343,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(0, 0); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_NE(nullptr, channel1_->rtcp_packet_transport()); - EXPECT_NE(nullptr, channel2_->rtcp_packet_transport()); + EXPECT_FALSE(channel1_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_FALSE(channel2_->rtp_transport()->rtcp_mux_enabled()); // Send RTCP1 from a different thread. ScopedCallThread send_rtcp([this] { SendRtcp1(); }); @@ -1422,19 +1422,15 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { rtc::Socket::Option::OPT_RCVBUF, kRcvBufSize); new_rtp_transport_ = CreateDtlsSrtpTransport( - static_cast(channel2_->rtp_packet_transport()), - static_cast( - channel2_->rtcp_packet_transport())); + fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get()); channel1_->SetRtpTransport(new_rtp_transport_.get()); int option_val; - ASSERT_TRUE( - static_cast(channel1_->rtp_packet_transport()) - ->GetOption(rtc::Socket::Option::OPT_SNDBUF, &option_val)); + ASSERT_TRUE(fake_rtp_dtls_transport2_->GetOption( + rtc::Socket::Option::OPT_SNDBUF, &option_val)); EXPECT_EQ(kSndBufSize, option_val); - ASSERT_TRUE( - static_cast(channel1_->rtp_packet_transport()) - ->GetOption(rtc::Socket::Option::OPT_RCVBUF, &option_val)); + ASSERT_TRUE(fake_rtp_dtls_transport2_->GetOption( + rtc::Socket::Option::OPT_RCVBUF, &option_val)); EXPECT_EQ(kRcvBufSize, option_val); } diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 346168d1fa..8081df9432 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -1662,7 +1662,6 @@ TEST_F(JsepTransportControllerTest, MultipleMediaSectionsOfSameTypeWithBundle) { // Verify the DtlsTransport for the SCTP data channel is reset correctly. auto it2 = changed_dtls_transport_by_mid_.find(kDataMid1); ASSERT_TRUE(it2 != changed_dtls_transport_by_mid_.end()); - EXPECT_EQ(transport1->rtp_packet_transport(), it2->second); } // Tests that only a subset of all the m= sections are bundled. diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index b369db23da..57d059c06e 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -83,14 +83,8 @@ class PeerConnectionWrapperForBundleTest : public PeerConnectionWrapper { return false; } - rtc::PacketTransportInternal* voice_rtp_transport() { - return (voice_channel() ? voice_channel()->rtp_packet_transport() - : nullptr); - } - - rtc::PacketTransportInternal* voice_rtcp_transport() { - return (voice_channel() ? voice_channel()->rtcp_packet_transport() - : nullptr); + RtpTransportInternal* voice_rtp_transport() { + return (voice_channel() ? voice_channel()->rtp_transport() : nullptr); } cricket::VoiceChannel* voice_channel() { @@ -104,14 +98,8 @@ class PeerConnectionWrapperForBundleTest : public PeerConnectionWrapper { return nullptr; } - rtc::PacketTransportInternal* video_rtp_transport() { - return (video_channel() ? video_channel()->rtp_packet_transport() - : nullptr); - } - - rtc::PacketTransportInternal* video_rtcp_transport() { - return (video_channel() ? video_channel()->rtcp_packet_transport() - : nullptr); + RtpTransportInternal* video_rtp_transport() { + return (video_channel() ? video_channel()->rtp_transport() : nullptr); } cricket::VideoChannel* video_channel() { @@ -552,14 +540,14 @@ TEST_P(PeerConnectionBundleTest, NeverCreateRtcpTransportWithRtcpMuxRequired) { ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - EXPECT_FALSE(caller->voice_rtcp_transport()); - EXPECT_FALSE(caller->video_rtcp_transport()); + EXPECT_FALSE(caller->voice_rtp_transport()->rtcp_mux_enabled()); + EXPECT_FALSE(caller->video_rtp_transport()->rtcp_mux_enabled()); ASSERT_TRUE( caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal())); - EXPECT_FALSE(caller->voice_rtcp_transport()); - EXPECT_FALSE(caller->video_rtcp_transport()); + EXPECT_TRUE(caller->voice_rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(caller->video_rtp_transport()->rtcp_mux_enabled()); } // When negotiating RTCP multiplexing, the PeerConnection makes RTCP transports @@ -573,14 +561,14 @@ TEST_P(PeerConnectionBundleTest, ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); - EXPECT_TRUE(caller->voice_rtcp_transport()); - EXPECT_TRUE(caller->video_rtcp_transport()); + EXPECT_FALSE(caller->voice_rtp_transport()->rtcp_mux_enabled()); + EXPECT_FALSE(caller->video_rtp_transport()->rtcp_mux_enabled()); ASSERT_TRUE( caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal())); - EXPECT_FALSE(caller->voice_rtcp_transport()); - EXPECT_FALSE(caller->video_rtcp_transport()); + EXPECT_TRUE(caller->voice_rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(caller->video_rtp_transport()->rtcp_mux_enabled()); } TEST_P(PeerConnectionBundleTest, FailToSetDescriptionWithBundleAndNoRtcpMux) { diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index afc859434f..3b8b4db951 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -216,17 +216,9 @@ class PeerConnectionIceBaseTest : public ::testing::Test { PeerConnection* pc = static_cast(pc_proxy->internal()); for (const auto& transceiver : pc->GetTransceiversInternal()) { if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) { - // TODO(amithi): This test seems to be using a method that should not - // be public |rtp_packet_transport|. Because the test is not mocking - // the channels or transceiver, workaround will be to |static_cast| - // the channel until the method is rewritten. - cricket::BaseChannel* channel = static_cast( - transceiver->internal()->channel()); - if (channel) { - auto dtls_transport = static_cast( - channel->rtp_packet_transport()); - return dtls_transport->ice_transport()->GetIceRole(); - } + auto dtls_transport = pc->LookupDtlsTransportByMidInternal( + transceiver->internal()->channel()->content_name()); + return dtls_transport->ice_transport()->internal()->GetIceRole(); } } RTC_NOTREACHED(); diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc index b7d3a9d893..6cfbed9cdd 100644 --- a/pc/rtp_transport.cc +++ b/pc/rtp_transport.cc @@ -31,6 +31,21 @@ void RtpTransport::SetRtcpMuxEnabled(bool enable) { MaybeSignalReadyToSend(); } +const std::string& RtpTransport::transport_name() const { + return rtp_packet_transport_->transport_name(); +} + +int RtpTransport::SetRtpOption(rtc::Socket::Option opt, int value) { + return rtp_packet_transport_->SetOption(opt, value); +} + +int RtpTransport::SetRtcpOption(rtc::Socket::Option opt, int value) { + if (rtcp_packet_transport_) { + return rtcp_packet_transport_->SetOption(opt, value); + } + return -1; +} + void RtpTransport::SetRtpPacketTransport( rtc::PacketTransportInternal* new_packet_transport) { if (new_packet_transport == rtp_packet_transport_) { diff --git a/pc/rtp_transport.h b/pc/rtp_transport.h index 269a61a09f..57ad9e5fd0 100644 --- a/pc/rtp_transport.h +++ b/pc/rtp_transport.h @@ -39,15 +39,20 @@ class RtpTransport : public RtpTransportInternal { bool rtcp_mux_enabled() const override { return rtcp_mux_enabled_; } void SetRtcpMuxEnabled(bool enable) override; - rtc::PacketTransportInternal* rtp_packet_transport() const override { + const std::string& transport_name() const override; + + int SetRtpOption(rtc::Socket::Option opt, int value) override; + int SetRtcpOption(rtc::Socket::Option opt, int value) override; + + rtc::PacketTransportInternal* rtp_packet_transport() const { return rtp_packet_transport_; } - void SetRtpPacketTransport(rtc::PacketTransportInternal* rtp) override; + void SetRtpPacketTransport(rtc::PacketTransportInternal* rtp); - rtc::PacketTransportInternal* rtcp_packet_transport() const override { + rtc::PacketTransportInternal* rtcp_packet_transport() const { return rtcp_packet_transport_; } - void SetRtcpPacketTransport(rtc::PacketTransportInternal* rtcp) override; + void SetRtcpPacketTransport(rtc::PacketTransportInternal* rtcp); bool IsReadyToSend() const override { return ready_to_send_; } diff --git a/pc/rtp_transport_internal.h b/pc/rtp_transport_internal.h index fce1f9bf71..dfcdbbfc61 100644 --- a/pc/rtp_transport_internal.h +++ b/pc/rtp_transport_internal.h @@ -37,17 +37,14 @@ class RtpTransportInternal : public sigslot::has_slots<> { virtual void SetRtcpMuxEnabled(bool enable) = 0; - // TODO(zstein): Remove PacketTransport setters. Clients should pass these - // in to constructors instead and construct a new RtpTransportInternal instead - // of updating them. + virtual const std::string& transport_name() const = 0; + + // Sets socket options on the underlying RTP or RTCP transports. + virtual int SetRtpOption(rtc::Socket::Option opt, int value) = 0; + virtual int SetRtcpOption(rtc::Socket::Option opt, int value) = 0; + virtual bool rtcp_mux_enabled() const = 0; - virtual rtc::PacketTransportInternal* rtp_packet_transport() const = 0; - virtual void SetRtpPacketTransport(rtc::PacketTransportInternal* rtp) = 0; - - virtual rtc::PacketTransportInternal* rtcp_packet_transport() const = 0; - virtual void SetRtcpPacketTransport(rtc::PacketTransportInternal* rtcp) = 0; - virtual bool IsReadyToSend() const = 0; // Called whenever a transport's ready-to-send state changes. The argument