diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 4f36a2ed85..f78b4efc5e 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -253,6 +253,7 @@ rtc_source_set("rtcp_transceiver") { "../../api/video:video_bitrate_allocation", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", + "../../rtc_base:rtc_cancelable_task", "../../rtc_base:rtc_task_queue", "../../rtc_base:weak_ptr", "../../system_wrappers", diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index a4da63a10f..2c6c3acc39 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -29,6 +29,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/sdes.h" #include "modules/rtp_rtcp/source/rtcp_packet/sender_report.h" #include "modules/rtp_rtcp/source/time_util.h" +#include "rtc_base/cancelable_periodic_task.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/task_queue.h" @@ -87,15 +88,19 @@ class RtcpTransceiverImpl::PacketSender { }; RtcpTransceiverImpl::RtcpTransceiverImpl(const RtcpTransceiverConfig& config) - : config_(config), - ready_to_send_(config.initial_ready_to_send), - ptr_factory_(this) { + : config_(config), ready_to_send_(config.initial_ready_to_send) { RTC_CHECK(config_.Validate()); if (ready_to_send_ && config_.schedule_periodic_compound_packets) SchedulePeriodicCompoundPackets(config_.initial_report_delay_ms); } -RtcpTransceiverImpl::~RtcpTransceiverImpl() = default; +RtcpTransceiverImpl::~RtcpTransceiverImpl() { + // If RtcpTransceiverImpl is destroyed off task queue, assume it is destroyed + // after TaskQueue. In that case there is no need to Cancel periodic task. + if (config_.task_queue == rtc::TaskQueue::Current()) { + periodic_task_handle_.Cancel(); + } +} void RtcpTransceiverImpl::AddMediaReceiverRtcpObserver( uint32_t remote_ssrc, @@ -120,8 +125,8 @@ void RtcpTransceiverImpl::RemoveMediaReceiverRtcpObserver( void RtcpTransceiverImpl::SetReadyToSend(bool ready) { if (config_.schedule_periodic_compound_packets) { - if (ready_to_send_ && !ready) // Stop existent send task. - ptr_factory_.InvalidateWeakPtrs(); + if (ready_to_send_ && !ready) + periodic_task_handle_.Cancel(); if (!ready_to_send_ && ready) // Restart periodic sending. SchedulePeriodicCompoundPackets(config_.report_period_ms / 2); @@ -318,36 +323,20 @@ void RtcpTransceiverImpl::HandleTargetBitrate( void RtcpTransceiverImpl::ReschedulePeriodicCompoundPackets() { if (!config_.schedule_periodic_compound_packets) return; - // Stop existent send task. - ptr_factory_.InvalidateWeakPtrs(); + periodic_task_handle_.Cancel(); + RTC_DCHECK(ready_to_send_); SchedulePeriodicCompoundPackets(config_.report_period_ms); } void RtcpTransceiverImpl::SchedulePeriodicCompoundPackets(int64_t delay_ms) { - class SendPeriodicCompoundPacketTask : public rtc::QueuedTask { - public: - SendPeriodicCompoundPacketTask(rtc::TaskQueue* task_queue, - rtc::WeakPtr ptr) - : task_queue_(task_queue), ptr_(std::move(ptr)) {} - bool Run() override { - RTC_DCHECK(task_queue_->IsCurrent()); - if (!ptr_) - return true; - ptr_->SendPeriodicCompoundPacket(); - task_queue_->PostDelayedTask(absl::WrapUnique(this), - ptr_->config_.report_period_ms); - return false; - } + auto task = rtc::CreateCancelablePeriodicTask([this] { + RTC_DCHECK(config_.schedule_periodic_compound_packets); + RTC_DCHECK(ready_to_send_); + SendPeriodicCompoundPacket(); + return config_.report_period_ms; + }); + periodic_task_handle_ = task->GetCancellationHandle(); - private: - rtc::TaskQueue* const task_queue_; - const rtc::WeakPtr ptr_; - }; - - RTC_DCHECK(config_.schedule_periodic_compound_packets); - - auto task = absl::make_unique( - config_.task_queue, ptr_factory_.GetWeakPtr()); if (delay_ms > 0) config_.task_queue->PostDelayedTask(std::move(task), delay_ms); else diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 7bcd16fa71..c97b2c5c11 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -24,8 +24,8 @@ #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" #include "modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h" #include "modules/rtp_rtcp/source/rtcp_transceiver_config.h" +#include "rtc_base/cancelable_task_handle.h" #include "rtc_base/constructormagic.h" -#include "rtc_base/weak_ptr.h" #include "system_wrappers/include/ntp_time.h" namespace webrtc { @@ -95,7 +95,7 @@ class RtcpTransceiverImpl { // TODO(danilchap): Remove entries from remote_senders_ that are no longer // needed. std::map remote_senders_; - rtc::WeakPtrFactory ptr_factory_; + rtc::CancelableTaskHandle periodic_task_handle_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcpTransceiverImpl); }; diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 1675b668e9..e5db0862cc 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -137,6 +137,40 @@ RtcpTransceiverConfig DefaultTestConfig() { return config; } +TEST(RtcpTransceiverImplTest, CanDestroyOnTaskQueue) { + FakeRtcpTransport transport; + rtc::TaskQueue queue("rtcp"); + RtcpTransceiverConfig config = DefaultTestConfig(); + config.task_queue = &queue; + config.schedule_periodic_compound_packets = true; + config.outgoing_transport = &transport; + auto* rtcp_transceiver = new RtcpTransceiverImpl(config); + // Wait for a periodic packet. + EXPECT_TRUE(transport.WaitPacket()); + + rtc::Event done(false, false); + queue.PostTask([rtcp_transceiver, &done] { + delete rtcp_transceiver; + done.Set(); + }); + ASSERT_TRUE(done.Wait(/*milliseconds=*/1000)); +} + +TEST(RtcpTransceiverImplTest, CanDestroyAfterTaskQueue) { + FakeRtcpTransport transport; + auto* queue = new rtc::TaskQueue("rtcp"); + RtcpTransceiverConfig config = DefaultTestConfig(); + config.task_queue = queue; + config.schedule_periodic_compound_packets = true; + config.outgoing_transport = &transport; + auto* rtcp_transceiver = new RtcpTransceiverImpl(config); + // Wait for a periodic packet. + EXPECT_TRUE(transport.WaitPacket()); + + delete queue; + delete rtcp_transceiver; +} + TEST(RtcpTransceiverImplTest, DelaysSendingFirstCompondPacket) { rtc::TaskQueue queue("rtcp"); FakeRtcpTransport transport; @@ -290,7 +324,7 @@ TEST(RtcpTransceiverImplTest, SendsPeriodicRtcpWhenNetworkStateIsUp) { absl::optional rtcp_transceiver; rtcp_transceiver.emplace(config); - rtcp_transceiver->SetReadyToSend(true); + queue.PostTask([&] { rtcp_transceiver->SetReadyToSend(true); }); EXPECT_TRUE(transport.WaitPacket());