diff --git a/pc/channel.cc b/pc/channel.cc index 5c0b204cf0..897b1aeeb1 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -271,16 +271,25 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { return true; } -bool BaseChannel::Enable(bool enable) { - worker_thread_->Invoke(RTC_FROM_HERE, [this, enable] { +void BaseChannel::Enable(bool enable) { + RTC_DCHECK_RUN_ON(signaling_thread()); + + if (enable == enabled_s_) + return; + + enabled_s_ = enable; + + worker_thread_->PostTask(ToQueuedTask(alive_, [this, enable] { RTC_DCHECK_RUN_ON(worker_thread()); + // Sanity check to make sure that enabled_ and enabled_s_ + // stay in sync. + RTC_DCHECK_NE(enabled_, enable); if (enable) { EnableMedia_w(); } else { DisableMedia_w(); } - }); - return true; + })); } bool BaseChannel::SetLocalContent(const MediaContentDescription* content, @@ -313,14 +322,14 @@ bool BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) { bool BaseChannel::IsReadyToReceiveMedia_w() const { // Receive data if we are enabled and have local content, - return enabled() && + return enabled_ && webrtc::RtpTransceiverDirectionHasRecv(local_content_direction_); } bool BaseChannel::IsReadyToSendMedia_w() const { // Send outgoing data if we are enabled, have local and remote content, // and we have had some form of connectivity. - return enabled() && + return enabled_ && webrtc::RtpTransceiverDirectionHasRecv(remote_content_direction_) && webrtc::RtpTransceiverDirectionHasSend(local_content_direction_) && was_ever_writable(); diff --git a/pc/channel.h b/pc/channel.h index 7dd3f8baf6..24d609144e 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -131,7 +131,6 @@ class BaseChannel : public ChannelInterface, // TODO(tommi): Delete this variable. return transport_name_; } - bool enabled() const override { return enabled_; } // This function returns true if using SRTP (DTLS-based keying or SDES). bool srtp_active() const { @@ -167,7 +166,7 @@ class BaseChannel : public ChannelInterface, // actually belong to a new channel. See: crbug.com/webrtc/11477 bool SetPayloadTypeDemuxingEnabled(bool enabled) override; - bool Enable(bool enable) override; + void Enable(bool enable) override; const std::vector& local_streams() const override { return local_streams_; @@ -356,7 +355,8 @@ class BaseChannel : public ChannelInterface, // Currently the |enabled_| flag is accessed from the signaling thread as // well, but it can be changed only when signaling thread does a synchronous // call to the worker thread, so it should be safe. - bool enabled_ = false; + bool enabled_ RTC_GUARDED_BY(worker_thread()) = false; + bool enabled_s_ RTC_GUARDED_BY(signaling_thread()) = false; bool payload_type_demuxing_enabled_ RTC_GUARDED_BY(worker_thread()) = true; std::vector local_streams_ RTC_GUARDED_BY(worker_thread()); std::vector remote_streams_ RTC_GUARDED_BY(worker_thread()); diff --git a/pc/channel_interface.h b/pc/channel_interface.h index d3da37ac23..46170a721b 100644 --- a/pc/channel_interface.h +++ b/pc/channel_interface.h @@ -37,10 +37,8 @@ class ChannelInterface { virtual const std::string& content_name() const = 0; - virtual bool enabled() const = 0; - // Enables or disables this channel - virtual bool Enable(bool enable) = 0; + virtual void Enable(bool enable) = 0; // Used for latency measurements. virtual sigslot::signal1& SignalFirstPacketReceived() = 0; diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 35413edf8f..c2e2311f4c 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -336,6 +336,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { SdpType::kOffer, NULL); if (result) { channel1_->Enable(true); + FlushCurrentThread(); result = channel2_->SetRemoteContent(&remote_media_content1_, SdpType::kOffer, NULL); if (result) { @@ -349,6 +350,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { bool SendAccept() { channel2_->Enable(true); + FlushCurrentThread(); return channel1_->SetRemoteContent(&remote_media_content2_, SdpType::kAnswer, NULL); } @@ -633,7 +635,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content1); content1.AddStream(stream1); EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, NULL)); - EXPECT_TRUE(channel1_->Enable(true)); + channel1_->Enable(true); EXPECT_EQ(1u, media_channel1_->send_streams().size()); EXPECT_TRUE(channel2_->SetRemoteContent(&content1, SdpType::kOffer, NULL)); @@ -646,7 +648,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, NULL)); EXPECT_EQ(0u, media_channel1_->recv_streams().size()); EXPECT_TRUE(channel2_->SetLocalContent(&content2, SdpType::kAnswer, NULL)); - EXPECT_TRUE(channel2_->Enable(true)); + channel2_->Enable(true); EXPECT_EQ(0u, media_channel2_->send_streams().size()); SendCustomRtp1(kSsrc1, 0); @@ -690,7 +692,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_FALSE(media_channel2_->playout()); } EXPECT_FALSE(media_channel2_->sending()); - EXPECT_TRUE(channel1_->Enable(true)); + channel1_->Enable(true); + FlushCurrentThread(); if (verify_playout_) { EXPECT_FALSE(media_channel1_->playout()); } @@ -722,7 +725,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_FALSE(media_channel2_->playout()); } EXPECT_FALSE(media_channel2_->sending()); - EXPECT_TRUE(channel2_->Enable(true)); + channel2_->Enable(true); + FlushCurrentThread(); if (verify_playout_) { EXPECT_TRUE(media_channel2_->playout()); } @@ -746,8 +750,9 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Set |content2| to be InActive. content2.set_direction(RtpTransceiverDirection::kInactive); - EXPECT_TRUE(channel1_->Enable(true)); - EXPECT_TRUE(channel2_->Enable(true)); + channel1_->Enable(true); + channel2_->Enable(true); + FlushCurrentThread(); if (verify_playout_) { EXPECT_FALSE(media_channel1_->playout()); } @@ -1365,6 +1370,9 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { thread->ProcessMessages(0); } } + static void FlushCurrentThread() { + rtc::Thread::Current()->ProcessMessages(0); + } void WaitForThreads(rtc::ArrayView threads) { // |threads| and current thread post packets to network thread. for (rtc::Thread* thread : threads) { diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index c9ee82495b..50d6b9a9e6 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -4137,7 +4137,7 @@ void SdpOfferAnswerHandler::EnableSending() { RTC_DCHECK_RUN_ON(signaling_thread()); for (const auto& transceiver : transceivers()->ListInternal()) { cricket::ChannelInterface* channel = transceiver->channel(); - if (channel && !channel->enabled()) { + if (channel) { channel->Enable(true); } } diff --git a/pc/test/mock_channel_interface.h b/pc/test/mock_channel_interface.h index 726519cf97..5d3c66d1ae 100644 --- a/pc/test/mock_channel_interface.h +++ b/pc/test/mock_channel_interface.h @@ -28,8 +28,7 @@ class MockChannelInterface : public cricket::ChannelInterface { MOCK_METHOD(MediaChannel*, media_channel, (), (const, override)); MOCK_METHOD(const std::string&, transport_name, (), (const, override)); MOCK_METHOD(const std::string&, content_name, (), (const, override)); - MOCK_METHOD(bool, enabled, (), (const, override)); - MOCK_METHOD(bool, Enable, (bool), (override)); + MOCK_METHOD(void, Enable, (bool), (override)); MOCK_METHOD(sigslot::signal1&, SignalFirstPacketReceived, (),