diff --git a/pc/channel.cc b/pc/channel.cc index accc233aa1..0f6cdd728b 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -380,6 +380,18 @@ void BaseChannel::OnNetworkRouteChanged( }); } +sigslot::signal1& BaseChannel::SignalFirstPacketReceived() { + RTC_DCHECK_RUN_ON(signaling_thread_); + return SignalFirstPacketReceived_; +} + +sigslot::signal1& BaseChannel::SignalSentPacket() { + // TODO(bugs.webrtc.org/11994): Uncomment this check once callers have been + // fixed to access this variable from the correct thread. + // RTC_DCHECK_RUN_ON(worker_thread_); + return SignalSentPacket_; +} + void BaseChannel::OnTransportReadyToSend(bool ready) { invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [=] { media_channel_->OnReadyToSend(ready); }); @@ -776,6 +788,7 @@ void BaseChannel::OnMessage(rtc::Message* pmsg) { break; } case MSG_FIRSTPACKETRECEIVED: { + RTC_DCHECK_RUN_ON(signaling_thread_); SignalFirstPacketReceived_(this); break; } @@ -809,7 +822,7 @@ void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) { invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, sent_packet] { RTC_DCHECK_RUN_ON(worker_thread()); - SignalSentPacket(sent_packet); + SignalSentPacket()(sent_packet); }); } diff --git a/pc/channel.h b/pc/channel.h index 15915b4cfe..e99ba2428b 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -145,22 +145,11 @@ class BaseChannel : public ChannelInterface, return remote_streams_; } - sigslot::signal2 SignalDtlsSrtpSetupFailure; - void SignalDtlsSrtpSetupFailure_n(bool rtcp); - void SignalDtlsSrtpSetupFailure_s(bool rtcp); - // Used for latency measurements. - sigslot::signal1& SignalFirstPacketReceived() override { - return SignalFirstPacketReceived_; - } + sigslot::signal1& SignalFirstPacketReceived() override; // Forward SignalSentPacket to worker thread. - sigslot::signal1 SignalSentPacket; - - // Emitted whenever rtcp-mux is fully negotiated and the rtcp-transport can - // be destroyed. - // Fired on the network thread. - sigslot::signal1 SignalRtcpMuxFullyActive; + sigslot::signal1& SignalSentPacket(); // From RtpTransport - public for testing only void OnTransportReadyToSend(bool ready); @@ -299,7 +288,10 @@ class BaseChannel : public ChannelInterface, rtc::Thread* const network_thread_; rtc::Thread* const signaling_thread_; rtc::AsyncInvoker invoker_; - sigslot::signal1 SignalFirstPacketReceived_; + sigslot::signal1 SignalFirstPacketReceived_ + RTC_GUARDED_BY(signaling_thread_); + sigslot::signal1 SignalSentPacket_ + RTC_GUARDED_BY(worker_thread_); const std::string content_name_; diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 479340c520..4eef70f4ea 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -228,10 +228,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { rtp_transport1_.get(), flags1); channel2_ = CreateChannel(worker_thread, network_thread_, std::move(ch2), rtp_transport2_.get(), flags2); - channel1_->SignalRtcpMuxFullyActive.connect( - this, &ChannelTest::OnRtcpMuxFullyActive1); - channel2_->SignalRtcpMuxFullyActive.connect( - this, &ChannelTest::OnRtcpMuxFullyActive2); CreateContent(flags1, kPcmuCodec, kH264Codec, &local_media_content1_); CreateContent(flags2, kPcmuCodec, kH264Codec, &local_media_content2_); CopyContent(local_media_content1_, &remote_media_content1_); @@ -490,13 +486,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { return false; // overridden in specialized classes } - void OnRtcpMuxFullyActive1(const std::string&) { - rtcp_mux_activated_callbacks1_++; - } - void OnRtcpMuxFullyActive2(const std::string&) { - rtcp_mux_activated_callbacks2_++; - } - cricket::CandidatePairInterface* last_selected_candidate_pair() { return last_selected_candidate_pair_; } @@ -597,29 +586,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel2_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); } - // Test that SetLocalContent and SetRemoteContent properly set RTCP - // mux when a provisional answer is received. - void TestSetContentsRtcpMuxWithPrAnswer() { - CreateChannels(0, 0); - typename T::Content content; - CreateContent(0, kPcmuCodec, kH264Codec, &content); - content.set_rtcp_mux(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); - EXPECT_TRUE( - channel1_->SetRemoteContent(&content, SdpType::kPrAnswer, NULL)); - // Both sides agree on mux. Should signal RTCP mux as fully activated. - EXPECT_EQ(0, rtcp_mux_activated_callbacks1_); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); - EXPECT_EQ(1, rtcp_mux_activated_callbacks1_); - // Only initiator supports mux. Should still have a separate RTCP channel. - EXPECT_TRUE(channel2_->SetLocalContent(&content, SdpType::kOffer, NULL)); - content.set_rtcp_mux(false); - EXPECT_TRUE( - channel2_->SetRemoteContent(&content, SdpType::kPrAnswer, NULL)); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); - EXPECT_EQ(0, rtcp_mux_activated_callbacks2_); - } - // Test that SetLocalContent and SetRemoteContent properly // handles adding and removing StreamParams when the action is a full // SdpType::kOffer / SdpType::kAnswer. @@ -1412,8 +1378,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // The RTP and RTCP packets to send in the tests. rtc::Buffer rtp_packet_; rtc::Buffer rtcp_packet_; - int rtcp_mux_activated_callbacks1_ = 0; - int rtcp_mux_activated_callbacks2_ = 0; cricket::CandidatePairInterface* last_selected_candidate_pair_; rtc::UniqueRandomIdGenerator ssrc_generator_; }; diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index df2fabc93f..4c8a00b1c3 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -100,10 +100,6 @@ const char kSdpWithoutIceUfragPwd[] = "Called with SDP without ice-ufrag and ice-pwd."; const char kSessionError[] = "Session error code: "; const char kSessionErrorDesc[] = "Session error description: "; -const char kDtlsSrtpSetupFailureRtp[] = - "Couldn't set up DTLS-SRTP on RTP channel."; -const char kDtlsSrtpSetupFailureRtcp[] = - "Couldn't set up DTLS-SRTP on RTCP channel."; namespace { @@ -6349,11 +6345,6 @@ void PeerConnection::OnCertificateReady( transport_controller_->SetLocalCertificate(certificate); } -void PeerConnection::OnDtlsSrtpSetupFailure(cricket::BaseChannel*, bool rtcp) { - SetSessionError(SessionError::kTransport, - rtcp ? kDtlsSrtpSetupFailureRtcp : kDtlsSrtpSetupFailureRtp); -} - void PeerConnection::OnTransportControllerConnectionState( cricket::IceConnectionState state) { switch (state) { @@ -6680,10 +6671,8 @@ cricket::VoiceChannel* PeerConnection::CreateVoiceChannel( if (!voice_channel) { return nullptr; } - voice_channel->SignalDtlsSrtpSetupFailure.connect( - this, &PeerConnection::OnDtlsSrtpSetupFailure); - voice_channel->SignalSentPacket.connect(this, - &PeerConnection::OnSentPacket_w); + voice_channel->SignalSentPacket().connect(this, + &PeerConnection::OnSentPacket_w); voice_channel->SetRtpTransport(rtp_transport); return voice_channel; @@ -6704,10 +6693,8 @@ cricket::VideoChannel* PeerConnection::CreateVideoChannel( if (!video_channel) { return nullptr; } - video_channel->SignalDtlsSrtpSetupFailure.connect( - this, &PeerConnection::OnDtlsSrtpSetupFailure); - video_channel->SignalSentPacket.connect(this, - &PeerConnection::OnSentPacket_w); + video_channel->SignalSentPacket().connect(this, + &PeerConnection::OnSentPacket_w); video_channel->SetRtpTransport(rtp_transport); return video_channel; @@ -6737,10 +6724,7 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { if (!data_channel_controller_.rtp_data_channel()) { return false; } - data_channel_controller_.rtp_data_channel() - ->SignalDtlsSrtpSetupFailure.connect( - this, &PeerConnection::OnDtlsSrtpSetupFailure); - data_channel_controller_.rtp_data_channel()->SignalSentPacket.connect( + data_channel_controller_.rtp_data_channel()->SignalSentPacket().connect( this, &PeerConnection::OnSentPacket_w); data_channel_controller_.rtp_data_channel()->SetRtpTransport( rtp_transport); diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 12775ba232..d92c734608 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -920,7 +920,6 @@ class PeerConnection : public PeerConnectionInternal, // WebRTCSessionDescriptionFactory. Should happen before setLocalDescription. void OnCertificateReady( const rtc::scoped_refptr& certificate); - void OnDtlsSrtpSetupFailure(cricket::BaseChannel*, bool rtcp); // Non-const versions of local_description()/remote_description(), for use // internally.