diff --git a/webrtc/pc/BUILD.gn b/webrtc/pc/BUILD.gn index fe9348b59e..ff99c19097 100644 --- a/webrtc/pc/BUILD.gn +++ b/webrtc/pc/BUILD.gn @@ -190,6 +190,7 @@ if (rtc_include_tests) { "currentspeakermonitor_unittest.cc", "mediasession_unittest.cc", "rtcpmuxfilter_unittest.cc", + "rtptransport_unittest.cc", "srtpfilter_unittest.cc", ] diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 59fca4a9d2..f5428a43ef 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -168,6 +168,7 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, network_thread_(network_thread), signaling_thread_(signaling_thread), content_name_(content_name), + rtcp_mux_required_(rtcp_mux_required), rtp_transport_(rtcp_mux_required), srtp_required_(srtp_required), media_channel_(media_channel), @@ -176,6 +177,8 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, #if defined(ENABLE_EXTERNAL_AUTH) srtp_filter_.EnableExternalAuth(); #endif + rtp_transport_.SignalReadyToSend.connect( + this, &BaseChannel::OnTransportReadyToSend); LOG(LS_INFO) << "Created channel for " << content_name; } @@ -242,7 +245,7 @@ bool BaseChannel::InitNetwork_n( SetTransports_n(rtp_dtls_transport, rtcp_dtls_transport, rtp_packet_transport, rtcp_packet_transport); - if (rtp_transport_.rtcp_mux_required()) { + if (rtcp_mux_required_) { rtcp_mux_filter_.SetActive(); } return true; @@ -335,19 +338,6 @@ void BaseChannel::SetTransports_n( // Update aggregate writable/ready-to-send state between RTP and RTCP upon // setting new transport channels. UpdateWritableState_n(); - // We can only update ready-to-send after updating writability. - // - // On setting a new channel, assume it's ready to send if it's writable, - // because we have no way of knowing otherwise (the channel doesn't give us - // "was last send successful?"). - // - // This won't always be accurate (the last SendPacket call from another - // BaseChannel could have resulted in an error), but even so, we'll just - // encounter the error again and update "ready to send" accordingly. - SetTransportChannelReadyToSend( - false, rtp_packet_transport && rtp_packet_transport->writable()); - SetTransportChannelReadyToSend( - true, rtcp_packet_transport && rtcp_packet_transport->writable()); } void BaseChannel::SetTransport_n( @@ -374,9 +364,9 @@ void BaseChannel::SetTransport_n( } if (rtcp) { - rtp_transport_.set_rtcp_packet_transport(new_packet_transport); + rtp_transport_.SetRtcpPacketTransport(new_packet_transport); } else { - rtp_transport_.set_rtp_packet_transport(new_packet_transport); + rtp_transport_.SetRtpPacketTransport(new_packet_transport); } old_dtls_transport = new_dtls_transport; @@ -390,6 +380,7 @@ void BaseChannel::SetTransport_n( << "Setting RTCP for DTLS/SRTP after SrtpFilter is active " << "should never happen."; } + if (new_dtls_transport) { ConnectToDtlsTransport(new_dtls_transport); } else { @@ -404,9 +395,9 @@ void BaseChannel::SetTransport_n( void BaseChannel::ConnectToDtlsTransport(DtlsTransportInternal* transport) { RTC_DCHECK(network_thread_->IsCurrent()); + // TODO(zstein): de-dup with ConnectToPacketTransport transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState); transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead); - transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend); transport->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState); transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n); transport->ice_transport()->SignalSelectedCandidatePairChanged.connect( @@ -421,7 +412,6 @@ void BaseChannel::DisconnectFromDtlsTransport( transport->SignalWritableState.disconnect(this); transport->SignalReadPacket.disconnect(this); - transport->SignalReadyToSend.disconnect(this); transport->SignalDtlsState.disconnect(this); transport->SignalSentPacket.disconnect(this); transport->ice_transport()->SignalSelectedCandidatePairChanged.disconnect( @@ -433,7 +423,6 @@ void BaseChannel::ConnectToPacketTransport( RTC_DCHECK_RUN_ON(network_thread_); transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState); transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead); - transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend); transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n); } @@ -442,7 +431,6 @@ void BaseChannel::DisconnectFromPacketTransport( RTC_DCHECK_RUN_ON(network_thread_); transport->SignalWritableState.disconnect(this); transport->SignalReadPacket.disconnect(this); - transport->SignalReadyToSend.disconnect(this); transport->SignalSentPacket.disconnect(this); } @@ -522,8 +510,7 @@ bool BaseChannel::GetConnectionStats(ConnectionInfos* infos) { bool BaseChannel::NeedsRtcpTransport() { // If this BaseChannel doesn't require RTCP mux and we haven't fully // negotiated RTCP mux, we need an RTCP transport. - return !rtp_transport_.rtcp_mux_required() && - !rtcp_mux_filter_.IsFullyActive(); + return !rtcp_mux_required_ && !rtcp_mux_filter_.IsFullyActive(); } bool BaseChannel::IsReadyToReceiveMedia_w() const { @@ -605,13 +592,6 @@ void BaseChannel::OnPacketRead(rtc::PacketTransportInternal* transport, HandlePacket(rtcp, &packet, packet_time); } -void BaseChannel::OnReadyToSend(rtc::PacketTransportInternal* transport) { - RTC_DCHECK(transport == rtp_transport_.rtp_packet_transport() || - transport == rtp_transport_.rtcp_packet_transport()); - SetTransportChannelReadyToSend( - transport == rtp_transport_.rtcp_packet_transport(), true); -} - void BaseChannel::OnDtlsState(DtlsTransportInternal* transport, DtlsTransportState state) { if (!ShouldSetupDtlsSrtp_n()) { @@ -655,22 +635,10 @@ void BaseChannel::OnSelectedCandidatePairChanged( network_route)); } -void BaseChannel::SetTransportChannelReadyToSend(bool rtcp, bool ready) { - RTC_DCHECK(network_thread_->IsCurrent()); - if (rtcp) { - rtcp_ready_to_send_ = ready; - } else { - rtp_ready_to_send_ = ready; - } - - bool ready_to_send = - (rtp_ready_to_send_ && - // In the case of rtcp mux |rtcp_packet_transport_| will be null. - (rtcp_ready_to_send_ || !rtp_transport_.rtcp_packet_transport())); - +void BaseChannel::OnTransportReadyToSend(bool ready) { invoker_.AsyncInvoke( RTC_FROM_HERE, worker_thread_, - Bind(&MediaChannel::OnReadyToSend, media_channel_, ready_to_send)); + Bind(&MediaChannel::OnReadyToSend, media_channel_, ready)); } bool BaseChannel::PacketIsRtcp(const rtc::PacketTransportInternal* transport, @@ -705,11 +673,7 @@ bool BaseChannel::SendPacket(bool rtcp, // packet before doing anything. (We might get RTCP packets that we don't // intend to send.) If we've negotiated RTCP mux, send RTCP over the RTP // transport. - rtc::PacketTransportInternal* transport = - (!rtcp || rtcp_mux_filter_.IsActive()) - ? rtp_transport_.rtp_packet_transport() - : rtp_transport_.rtcp_packet_transport(); - if (!transport || !transport->writable()) { + if (!rtp_transport_.IsWritable(rtcp)) { return false; } @@ -805,16 +769,7 @@ bool BaseChannel::SendPacket(bool rtcp, // Bon voyage. int flags = (secure() && secure_dtls()) ? PF_SRTP_BYPASS : PF_NORMAL; - int ret = transport->SendPacket(packet->data(), packet->size(), - updated_options, flags); - if (ret != static_cast(packet->size())) { - if (transport->GetError() == ENOTCONN) { - LOG(LS_WARNING) << "Got ENOTCONN from transport."; - SetTransportChannelReadyToSend(rtcp, false); - } - return false; - } - return true; + return rtp_transport_.SendPacket(rtcp, packet, updated_options, flags); } bool BaseChannel::WantsPacket(bool rtcp, const rtc::CopyOnWriteBuffer* packet) { @@ -1233,7 +1188,7 @@ bool BaseChannel::SetRtcpMux_n(bool enable, std::string* error_desc) { // Provide a more specific error message for the RTCP mux "require" policy // case. - if (rtp_transport_.rtcp_mux_required() && !enable) { + if (rtcp_mux_required_ && !enable) { SafeSetError( "rtcpMuxPolicy is 'require', but media description does not " "contain 'a=rtcp-mux'.", @@ -1267,7 +1222,6 @@ bool BaseChannel::SetRtcpMux_n(bool enable, SignalRtcpMuxFullyActive(transport_name_); } UpdateWritableState_n(); - SetTransportChannelReadyToSend(true, false); } break; case CA_UPDATE: @@ -1281,6 +1235,7 @@ bool BaseChannel::SetRtcpMux_n(bool enable, SafeSetError("Failed to setup RTCP mux filter.", error_desc); return false; } + rtp_transport_.SetRtcpMuxEnabled(rtcp_mux_filter_.IsActive()); // |rtcp_mux_filter_| can be active if |action| is CA_PRANSWER or // CA_ANSWER, but we only want to tear down the RTCP transport if we received // a final answer. diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index 56d51f64da..48259e5fd9 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -183,16 +183,8 @@ class BaseChannel bool NeedsRtcpTransport(); - // Made public for easier testing. - // - // Updates "ready to send" for an individual channel, and informs the media - // channel that the transport is ready to send if each channel (in use) is - // ready to send. This is more specific than just "writable"; it means the - // last send didn't return ENOTCONN. - // - // This should be called whenever a channel's ready-to-send state changes, - // or when RTCP muxing becomes active/inactive. - void SetTransportChannelReadyToSend(bool rtcp, bool ready); + // From RtpTransport - public for testing only + void OnTransportReadyToSend(bool ready); // Only public for unit tests. Otherwise, consider protected. int SetOption(SocketType type, rtc::Socket::Option o, int val) @@ -261,7 +253,6 @@ class BaseChannel size_t len, const rtc::PacketTime& packet_time, int flags); - void OnReadyToSend(rtc::PacketTransportInternal* transport); void OnDtlsState(DtlsTransportInternal* transport, DtlsTransportState state); @@ -391,6 +382,8 @@ class BaseChannel // Won't be set when using raw packet transports. SDP-specific thing. std::string transport_name_; + const bool rtcp_mux_required_; + // Separate DTLS/non-DTLS pointers to support using BaseChannel without DTLS. // Temporary measure until more refactoring is done. // If non-null, "X_dtls_transport_" will always equal "X_packet_transport_". @@ -402,8 +395,6 @@ class BaseChannel SrtpFilter srtp_filter_; RtcpMuxFilter rtcp_mux_filter_; BundleFilter bundle_filter_; - bool rtp_ready_to_send_ = false; - bool rtcp_ready_to_send_ = false; bool writable_ = false; bool was_ever_writable_ = false; bool has_received_packet_ = false; diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index baacac8eb7..6c4aa0f9ce 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -1844,52 +1844,20 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(Terminate()); } - void TestOnReadyToSend() { + void TestOnTransportReadyToSend() { CreateChannels(0, 0); - cricket::FakeDtlsTransport* rtp = fake_rtp_dtls_transport1_.get(); - cricket::FakeDtlsTransport* rtcp = fake_rtcp_dtls_transport1_.get(); EXPECT_FALSE(media_channel1_->ready_to_send()); - network_thread_->Invoke(RTC_FROM_HERE, - [rtp] { rtp->SignalReadyToSend(rtp); }); - WaitForThreads(); - EXPECT_FALSE(media_channel1_->ready_to_send()); - - network_thread_->Invoke(RTC_FROM_HERE, - [rtcp] { rtcp->SignalReadyToSend(rtcp); }); - WaitForThreads(); - // MediaChannel::OnReadyToSend only be called when both rtp and rtcp - // channel are ready to send. - EXPECT_TRUE(media_channel1_->ready_to_send()); - - // rtp channel becomes not ready to send will be propagated to mediachannel - network_thread_->Invoke(RTC_FROM_HERE, [this] { - channel1_->SetTransportChannelReadyToSend(false, false); - }); - WaitForThreads(); - EXPECT_FALSE(media_channel1_->ready_to_send()); - - network_thread_->Invoke(RTC_FROM_HERE, [this] { - channel1_->SetTransportChannelReadyToSend(false, true); - }); + channel1_->OnTransportReadyToSend(true); WaitForThreads(); EXPECT_TRUE(media_channel1_->ready_to_send()); - // rtcp channel becomes not ready to send will be propagated to mediachannel - network_thread_->Invoke(RTC_FROM_HERE, [this] { - channel1_->SetTransportChannelReadyToSend(true, false); - }); + channel1_->OnTransportReadyToSend(false); WaitForThreads(); EXPECT_FALSE(media_channel1_->ready_to_send()); - - network_thread_->Invoke(RTC_FROM_HERE, [this] { - channel1_->SetTransportChannelReadyToSend(true, true); - }); - WaitForThreads(); - EXPECT_TRUE(media_channel1_->ready_to_send()); } - void TestOnReadyToSendWithRtcpMux() { + void TestOnTransportReadyToSendWithRtcpMux() { CreateChannels(0, 0); typename T::Content content; CreateContent(0, kPcmuCodec, kH264Codec, &content); @@ -1908,9 +1876,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { WaitForThreads(); EXPECT_TRUE(media_channel1_->ready_to_send()); - network_thread_->Invoke(RTC_FROM_HERE, [this] { - channel1_->SetTransportChannelReadyToSend(false, false); - }); + // TODO(zstein): Find a way to test this without making + // OnTransportReadyToSend public. + network_thread_->Invoke( + RTC_FROM_HERE, [this] { channel1_->OnTransportReadyToSend(false); }); WaitForThreads(); EXPECT_FALSE(media_channel1_->ready_to_send()); } @@ -2390,12 +2359,12 @@ TEST_F(VoiceChannelSingleThreadTest, TestSrtpError) { Base::TestSrtpError(kAudioPts[0]); } -TEST_F(VoiceChannelSingleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(VoiceChannelSingleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(VoiceChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(VoiceChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } // Test that we can scale the output volume properly for 1:1 calls. @@ -2711,12 +2680,12 @@ TEST_F(VoiceChannelDoubleThreadTest, TestSrtpError) { Base::TestSrtpError(kAudioPts[0]); } -TEST_F(VoiceChannelDoubleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(VoiceChannelDoubleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(VoiceChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(VoiceChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } // Test that we can scale the output volume properly for 1:1 calls. @@ -3024,12 +2993,12 @@ TEST_F(VideoChannelSingleThreadTest, TestSrtpError) { Base::TestSrtpError(kVideoPts[0]); } -TEST_F(VideoChannelSingleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(VideoChannelSingleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(VideoChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(VideoChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } TEST_F(VideoChannelSingleThreadTest, DefaultMaxBitrateIsUnlimited) { @@ -3259,12 +3228,12 @@ TEST_F(VideoChannelDoubleThreadTest, TestSrtpError) { Base::TestSrtpError(kVideoPts[0]); } -TEST_F(VideoChannelDoubleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(VideoChannelDoubleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(VideoChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(VideoChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } TEST_F(VideoChannelDoubleThreadTest, DefaultMaxBitrateIsUnlimited) { @@ -3411,12 +3380,12 @@ TEST_F(RtpDataChannelSingleThreadTest, TestCallTeardownRtcpMux) { Base::TestCallTeardownRtcpMux(); } -TEST_F(RtpDataChannelSingleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(RtpDataChannelSingleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(RtpDataChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(RtpDataChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } TEST_F(RtpDataChannelSingleThreadTest, SendRtpToRtp) { @@ -3543,12 +3512,12 @@ TEST_F(RtpDataChannelDoubleThreadTest, TestCallTeardownRtcpMux) { Base::TestCallTeardownRtcpMux(); } -TEST_F(RtpDataChannelDoubleThreadTest, TestOnReadyToSend) { - Base::TestOnReadyToSend(); +TEST_F(RtpDataChannelDoubleThreadTest, TestOnTransportReadyToSend) { + Base::TestOnTransportReadyToSend(); } -TEST_F(RtpDataChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) { - Base::TestOnReadyToSendWithRtcpMux(); +TEST_F(RtpDataChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) { + Base::TestOnTransportReadyToSendWithRtcpMux(); } TEST_F(RtpDataChannelDoubleThreadTest, SendRtpToRtp) { diff --git a/webrtc/pc/rtptransport.cc b/webrtc/pc/rtptransport.cc index 76bc639cbc..2ee27e02fb 100644 --- a/webrtc/pc/rtptransport.cc +++ b/webrtc/pc/rtptransport.cc @@ -11,18 +11,80 @@ #include "webrtc/pc/rtptransport.h" #include "webrtc/base/checks.h" +#include "webrtc/base/copyonwritebuffer.h" #include "webrtc/p2p/base/packettransportinterface.h" namespace webrtc { -void RtpTransport::set_rtp_packet_transport(rtc::PacketTransportInternal* rtp) { - rtp_packet_transport_ = rtp; +void RtpTransport::SetRtcpMuxEnabled(bool enable) { + rtcp_mux_enabled_ = enable; + MaybeSignalReadyToSend(); } -void RtpTransport::set_rtcp_packet_transport( - rtc::PacketTransportInternal* rtcp) { - RTC_DCHECK(!rtcp_mux_required_); - rtcp_packet_transport_ = rtcp; +void RtpTransport::SetRtpPacketTransport( + rtc::PacketTransportInternal* new_packet_transport) { + if (new_packet_transport == rtp_packet_transport_) { + return; + } + if (rtp_packet_transport_) { + rtp_packet_transport_->SignalReadyToSend.disconnect(this); + } + if (new_packet_transport) { + new_packet_transport->SignalReadyToSend.connect( + this, &RtpTransport::OnReadyToSend); + } + rtp_packet_transport_ = new_packet_transport; + + // Assumes the transport is ready to send if it is writable. If we are wrong, + // ready to send will be updated the next time we try to send. + SetReadyToSend(false, + rtp_packet_transport_ && rtp_packet_transport_->writable()); +} + +void RtpTransport::SetRtcpPacketTransport( + rtc::PacketTransportInternal* new_packet_transport) { + if (new_packet_transport == rtcp_packet_transport_) { + return; + } + if (rtcp_packet_transport_) { + rtcp_packet_transport_->SignalReadyToSend.disconnect(this); + } + if (new_packet_transport) { + new_packet_transport->SignalReadyToSend.connect( + this, &RtpTransport::OnReadyToSend); + } + rtcp_packet_transport_ = new_packet_transport; + + // Assumes the transport is ready to send if it is writable. If we are wrong, + // ready to send will be updated the next time we try to send. + SetReadyToSend(true, + rtcp_packet_transport_ && rtcp_packet_transport_->writable()); +} + +bool RtpTransport::IsWritable(bool rtcp) const { + rtc::PacketTransportInternal* transport = rtcp && !rtcp_mux_enabled_ + ? rtcp_packet_transport_ + : rtp_packet_transport_; + return transport && transport->writable(); +} + +bool RtpTransport::SendPacket(bool rtcp, + const rtc::CopyOnWriteBuffer* packet, + const rtc::PacketOptions& options, + int flags) { + rtc::PacketTransportInternal* transport = rtcp && !rtcp_mux_enabled_ + ? rtcp_packet_transport_ + : rtp_packet_transport_; + int ret = transport->SendPacket(packet->data(), packet->size(), options, + flags); + if (ret != static_cast(packet->size())) { + if (transport->GetError() == ENOTCONN) { + LOG(LS_WARNING) << "Got ENOTCONN from transport."; + SetReadyToSend(rtcp, false); + } + return false; + } + return true; } PacketTransportInterface* RtpTransport::GetRtpPacketTransport() const { @@ -57,4 +119,27 @@ RtpTransportAdapter* RtpTransport::GetInternal() { return nullptr; } +void RtpTransport::OnReadyToSend(rtc::PacketTransportInternal* transport) { + SetReadyToSend(transport == rtcp_packet_transport_, true); +} + +void RtpTransport::SetReadyToSend(bool rtcp, bool ready) { + if (rtcp) { + rtcp_ready_to_send_ = ready; + } else { + rtp_ready_to_send_ = ready; + } + + MaybeSignalReadyToSend(); +} + +void RtpTransport::MaybeSignalReadyToSend() { + bool ready_to_send = + rtp_ready_to_send_ && (rtcp_ready_to_send_ || rtcp_mux_enabled_); + if (ready_to_send != ready_to_send_) { + ready_to_send_ = ready_to_send; + SignalReadyToSend(ready_to_send); + } +} + } // namespace webrtc diff --git a/webrtc/pc/rtptransport.h b/webrtc/pc/rtptransport.h index f5ffe3fe5f..f9bee1b6cc 100644 --- a/webrtc/pc/rtptransport.h +++ b/webrtc/pc/rtptransport.h @@ -12,34 +12,38 @@ #define WEBRTC_PC_RTPTRANSPORT_H_ #include "webrtc/api/ortc/rtptransportinterface.h" +#include "webrtc/base/sigslot.h" namespace rtc { +class CopyOnWriteBuffer; +struct PacketOptions; class PacketTransportInternal; } // namespace rtc namespace webrtc { -class RtpTransport : public RtpTransportInterface { +class RtpTransport : public RtpTransportInterface, public sigslot::has_slots<> { public: RtpTransport(const RtpTransport&) = delete; RtpTransport& operator=(const RtpTransport&) = delete; - explicit RtpTransport(bool rtcp_mux_required) - : rtcp_mux_required_(rtcp_mux_required) {} + explicit RtpTransport(bool rtcp_mux_enabled) + : rtcp_mux_enabled_(rtcp_mux_enabled) {} - bool rtcp_mux_required() const { return rtcp_mux_required_; } + bool rtcp_mux_enabled() const { return rtcp_mux_enabled_; } + void SetRtcpMuxEnabled(bool enable); rtc::PacketTransportInternal* rtp_packet_transport() const { return rtp_packet_transport_; } - void set_rtp_packet_transport(rtc::PacketTransportInternal* rtp); + void SetRtpPacketTransport(rtc::PacketTransportInternal* rtp); rtc::PacketTransportInternal* rtcp_packet_transport() const { return rtcp_packet_transport_; } - void set_rtcp_packet_transport(rtc::PacketTransportInternal* rtcp); + void SetRtcpPacketTransport(rtc::PacketTransportInternal* rtcp); PacketTransportInterface* GetRtpPacketTransport() const override; PacketTransportInterface* GetRtcpPacketTransport() const override; @@ -48,18 +52,40 @@ class RtpTransport : public RtpTransportInterface { RTCError SetRtcpParameters(const RtcpParameters& parameters) override; RtcpParameters GetRtcpParameters() const override; + // Called whenever a transport's ready-to-send state changes. The argument + // is true if all used transports are ready to send. This is more specific + // than just "writable"; it means the last send didn't return ENOTCONN. + sigslot::signal1 SignalReadyToSend; + + bool IsWritable(bool rtcp) const; + + bool SendPacket(bool rtcp, + const rtc::CopyOnWriteBuffer* packet, + const rtc::PacketOptions& options, + int flags); + protected: // TODO(zstein): Remove this when we remove RtpTransportAdapter. RtpTransportAdapter* GetInternal() override; private: - // True if RTCP-multiplexing is required. rtcp_packet_transport_ should - // always be null in this case. - const bool rtcp_mux_required_; + void OnReadyToSend(rtc::PacketTransportInternal* transport); + + // Updates "ready to send" for an individual channel and fires + // SignalReadyToSend. + void SetReadyToSend(bool rtcp, bool ready); + + void MaybeSignalReadyToSend(); + + bool rtcp_mux_enabled_; rtc::PacketTransportInternal* rtp_packet_transport_ = nullptr; rtc::PacketTransportInternal* rtcp_packet_transport_ = nullptr; + bool ready_to_send_ = false; + bool rtp_ready_to_send_ = false; + bool rtcp_ready_to_send_ = false; + RtcpParameters rtcp_parameters_; }; diff --git a/webrtc/pc/rtptransport_unittest.cc b/webrtc/pc/rtptransport_unittest.cc new file mode 100644 index 0000000000..ba492517b5 --- /dev/null +++ b/webrtc/pc/rtptransport_unittest.cc @@ -0,0 +1,150 @@ +/* + * Copyright 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include + +#include "webrtc/base/gunit.h" +#include "webrtc/p2p/base/fakepackettransport.h" +#include "webrtc/pc/rtptransport.h" + +namespace webrtc { + +class RtpTransportTest : public testing::Test {}; + +constexpr bool kMuxDisabled = false; +constexpr bool kMuxEnabled = true; + +TEST_F(RtpTransportTest, SetRtcpParametersCantDisableRtcpMux) { + RtpTransport transport(kMuxDisabled); + RtcpParameters params; + transport.SetRtcpParameters(params); + params.mux = false; + EXPECT_FALSE(transport.SetRtcpParameters(params).ok()); +} + +TEST_F(RtpTransportTest, SetRtcpParametersEmptyCnameUsesExisting) { + static const char kName[] = "name"; + RtpTransport transport(kMuxDisabled); + RtcpParameters params_with_name; + params_with_name.cname = kName; + transport.SetRtcpParameters(params_with_name); + EXPECT_EQ(transport.GetRtcpParameters().cname, kName); + + RtcpParameters params_without_name; + transport.SetRtcpParameters(params_without_name); + EXPECT_EQ(transport.GetRtcpParameters().cname, kName); +} + +class SignalObserver : public sigslot::has_slots<> { + public: + explicit SignalObserver(RtpTransport* transport) { + transport->SignalReadyToSend.connect(this, &SignalObserver::OnReadyToSend); + } + void OnReadyToSend(bool ready) { ready_ = ready; } + bool ready_ = false; +}; + +TEST_F(RtpTransportTest, SettingRtcpAndRtpSignalsReady) { + RtpTransport transport(kMuxDisabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtcp("fake_rtcp"); + fake_rtcp.SetWritable(true); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + transport.SetRtcpPacketTransport(&fake_rtcp); // rtcp ready + EXPECT_FALSE(observer.ready_); + transport.SetRtpPacketTransport(&fake_rtp); // rtp ready + EXPECT_TRUE(observer.ready_); +} + +TEST_F(RtpTransportTest, SettingRtpAndRtcpSignalsReady) { + RtpTransport transport(kMuxDisabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtcp("fake_rtcp"); + fake_rtcp.SetWritable(true); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + transport.SetRtpPacketTransport(&fake_rtp); // rtp ready + EXPECT_FALSE(observer.ready_); + transport.SetRtcpPacketTransport(&fake_rtcp); // rtcp ready + EXPECT_TRUE(observer.ready_); +} + +TEST_F(RtpTransportTest, SettingRtpWithRtcpMuxEnabledSignalsReady) { + RtpTransport transport(kMuxEnabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + transport.SetRtpPacketTransport(&fake_rtp); // rtp ready + EXPECT_TRUE(observer.ready_); +} + +TEST_F(RtpTransportTest, DisablingRtcpMuxSignalsNotReady) { + RtpTransport transport(kMuxEnabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + transport.SetRtpPacketTransport(&fake_rtp); // rtp ready + EXPECT_TRUE(observer.ready_); + + transport.SetRtcpMuxEnabled(false); + EXPECT_FALSE(observer.ready_); +} + +TEST_F(RtpTransportTest, EnablingRtcpMuxSignalsReady) { + RtpTransport transport(kMuxDisabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + transport.SetRtpPacketTransport(&fake_rtp); // rtp ready + EXPECT_FALSE(observer.ready_); + + transport.SetRtcpMuxEnabled(true); + EXPECT_TRUE(observer.ready_); +} + +class SignalCounter : public sigslot::has_slots<> { + public: + explicit SignalCounter(RtpTransport* transport) { + transport->SignalReadyToSend.connect(this, &SignalCounter::OnReadyToSend); + } + void OnReadyToSend(bool ready) { ++count_; } + int count_ = 0; +}; + +TEST_F(RtpTransportTest, ChangingReadyToSendStateOnlySignalsWhenChanged) { + RtpTransport transport(kMuxEnabled); + SignalCounter observer(&transport); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + fake_rtp.SetWritable(true); + + // State changes, so we should signal. + transport.SetRtpPacketTransport(&fake_rtp); + EXPECT_EQ(observer.count_, 1); + + // State does not change, so we should not signal. + transport.SetRtpPacketTransport(&fake_rtp); + EXPECT_EQ(observer.count_, 1); + + // State does not change, so we should not signal. + transport.SetRtcpMuxEnabled(true); + EXPECT_EQ(observer.count_, 1); + + // State changes, so we should signal. + transport.SetRtcpMuxEnabled(false); + EXPECT_EQ(observer.count_, 2); +} + +} // namespace webrtc