From c2b1bad4c87a43d7e1af85e809d5abe04baf3104 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 23 Feb 2022 20:15:20 +0100 Subject: [PATCH] In RtcpTransceiver use TimeDelta instead of raw int to represent time Bug: webrtc:8239, webrtc:13757 Change-Id: Idda3fe5761665b4b3fedaf2dd1a28bb0119ae1f1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252287 Reviewed-by: Emil Lundmark Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#36094} --- .../source/rtcp_transceiver_config.cc | 8 +++---- .../rtp_rtcp/source/rtcp_transceiver_config.h | 4 ++-- .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 14 +++++------ .../rtp_rtcp/source/rtcp_transceiver_impl.h | 2 +- .../source/rtcp_transceiver_impl_unittest.cc | 24 +++++++++---------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.cc b/modules/rtp_rtcp/source/rtcp_transceiver_config.cc index 02c0fef9f9..dea6286096 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.cc @@ -48,13 +48,13 @@ bool RtcpTransceiverConfig::Validate() const { RTC_LOG(LS_ERROR) << debug_id << "outgoing transport must be set"; return false; } - if (initial_report_delay_ms < 0) { - RTC_LOG(LS_ERROR) << debug_id << "delay " << initial_report_delay_ms + if (initial_report_delay < TimeDelta::Zero()) { + RTC_LOG(LS_ERROR) << debug_id << "delay " << initial_report_delay.ms() << "ms before first report shouldn't be negative."; return false; } - if (report_period_ms <= 0) { - RTC_LOG(LS_ERROR) << debug_id << "period " << report_period_ms + if (report_period <= TimeDelta::Zero()) { + RTC_LOG(LS_ERROR) << debug_id << "period " << report_period.ms() << "ms between reports should be positive."; return false; } diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 2400f9b908..34789dbd0b 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h @@ -149,10 +149,10 @@ struct RtcpTransceiverConfig { // Initial state if `outgoing_transport` ready to accept packets. bool initial_ready_to_send = true; // Delay before 1st periodic compound packet. - int initial_report_delay_ms = 500; + TimeDelta initial_report_delay = TimeDelta::Millis(500); // Period between periodic compound packets. - int report_period_ms = 1000; + TimeDelta report_period = TimeDelta::Seconds(1); // // Flags for features and experiments. diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index 5909af93ce..382e9f19ff 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -100,7 +100,7 @@ RtcpTransceiverImpl::RtcpTransceiverImpl(const RtcpTransceiverConfig& config) : 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); + SchedulePeriodicCompoundPackets(config_.initial_report_delay); } } @@ -159,7 +159,7 @@ void RtcpTransceiverImpl::SetReadyToSend(bool ready) { periodic_task_handle_.Stop(); if (!ready_to_send_ && ready) // Restart periodic sending. - SchedulePeriodicCompoundPackets(config_.report_period_ms / 2); + SchedulePeriodicCompoundPackets(config_.report_period / 2); } ready_to_send_ = ready; } @@ -470,16 +470,16 @@ void RtcpTransceiverImpl::ReschedulePeriodicCompoundPackets() { return; periodic_task_handle_.Stop(); RTC_DCHECK(ready_to_send_); - SchedulePeriodicCompoundPackets(config_.report_period_ms); + SchedulePeriodicCompoundPackets(config_.report_period); } -void RtcpTransceiverImpl::SchedulePeriodicCompoundPackets(int64_t delay_ms) { - periodic_task_handle_ = RepeatingTaskHandle::DelayedStart( - config_.task_queue, TimeDelta::Millis(delay_ms), [this] { +void RtcpTransceiverImpl::SchedulePeriodicCompoundPackets(TimeDelta delay) { + periodic_task_handle_ = + RepeatingTaskHandle::DelayedStart(config_.task_queue, delay, [this] { RTC_DCHECK(config_.schedule_periodic_compound_packets); RTC_DCHECK(ready_to_send_); SendPeriodicCompoundPacket(); - return TimeDelta::Millis(config_.report_period_ms); + return config_.report_period; }); } diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 8dc50b89e6..be9a98188e 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -116,7 +116,7 @@ class RtcpTransceiverImpl { rtc::ArrayView report_blocks); void ReschedulePeriodicCompoundPackets(); - void SchedulePeriodicCompoundPackets(int64_t delay_ms); + void SchedulePeriodicCompoundPackets(TimeDelta delay); // Appends RTCP sender and receiver reports to the `sender`. // Both sender and receiver reports may have attached report blocks. // Uses up to `config_.max_packet_size - reserved_bytes` diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 82bfadecef..744b4eb6a2 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -111,7 +111,7 @@ class MockNetworkLinkRtcpObserver : public NetworkLinkRtcpObserver { // Since some tests will need to wait for this period, make it small to avoid // slowing tests too much. As long as there are test bots with high scheduler // granularity, small period should be ok. -constexpr int kReportPeriodMs = 10; +constexpr TimeDelta kReportPeriod = TimeDelta::Millis(10); // On some systems task queue might be slow, instead of guessing right // grace period, use very large timeout, 100x larger expected wait time. // Use finite timeout to fail tests rather than hang them. @@ -176,8 +176,8 @@ RtcpTransceiverConfig DefaultTestConfig() { config.clock = &null_clock; config.outgoing_transport = &null_transport; config.schedule_periodic_compound_packets = false; - config.initial_report_delay_ms = 10; - config.report_period_ms = kReportPeriodMs; + config.initial_report_delay = TimeDelta::Millis(10); + config.report_period = kReportPeriod; return config; } @@ -246,7 +246,7 @@ TEST(RtcpTransceiverImplTest, DelaysSendingFirstCompondPacket) { RtcpTransceiverConfig config; config.clock = &clock; config.outgoing_transport = &transport; - config.initial_report_delay_ms = 10; + config.initial_report_delay = TimeDelta::Millis(10); config.task_queue = queue.Get(); absl::optional rtcp_transceiver; @@ -254,7 +254,7 @@ TEST(RtcpTransceiverImplTest, DelaysSendingFirstCompondPacket) { queue.PostTask([&] { rtcp_transceiver.emplace(config); }); EXPECT_TRUE(transport.WaitPacket()); - EXPECT_GE(rtc::TimeMillis() - started_ms, config.initial_report_delay_ms); + EXPECT_GE(rtc::TimeMillis() - started_ms, config.initial_report_delay.ms()); // Cleanup. rtc::Event done; @@ -273,8 +273,8 @@ TEST(RtcpTransceiverImplTest, PeriodicallySendsPackets) { RtcpTransceiverConfig config; config.clock = &clock; config.outgoing_transport = &transport; - config.initial_report_delay_ms = 0; - config.report_period_ms = kReportPeriodMs; + config.initial_report_delay = TimeDelta::Zero(); + config.report_period = kReportPeriod; config.task_queue = queue.Get(); absl::optional rtcp_transceiver; int64_t time_just_before_1st_packet_ms = 0; @@ -290,7 +290,7 @@ TEST(RtcpTransceiverImplTest, PeriodicallySendsPackets) { int64_t time_just_after_2nd_packet_ms = rtc::TimeMillis(); EXPECT_GE(time_just_after_2nd_packet_ms - time_just_before_1st_packet_ms, - config.report_period_ms - 1); + config.report_period.ms() - 1); // Cleanup. rtc::Event done; @@ -309,8 +309,8 @@ TEST(RtcpTransceiverImplTest, SendCompoundPacketDelaysPeriodicSendPackets) { RtcpTransceiverConfig config; config.clock = &clock; config.outgoing_transport = &transport; - config.initial_report_delay_ms = 0; - config.report_period_ms = kReportPeriodMs; + config.initial_report_delay = TimeDelta::Zero(); + config.report_period = kReportPeriod; config.task_queue = queue.Get(); absl::optional rtcp_transceiver; queue.PostTask([&] { rtcp_transceiver.emplace(config); }); @@ -326,7 +326,7 @@ TEST(RtcpTransceiverImplTest, SendCompoundPacketDelaysPeriodicSendPackets) { rtcp_transceiver->SendCompoundPacket(); non_periodic.Set(); }, - config.report_period_ms / 2); + (config.report_period / 2).ms()); // Though non-periodic packet is scheduled just in between periodic, due to // small period and task queue flakiness it migth end-up 1ms after next // periodic packet. To be sure duration after non-periodic packet is tested @@ -338,7 +338,7 @@ TEST(RtcpTransceiverImplTest, SendCompoundPacketDelaysPeriodicSendPackets) { int64_t time_of_last_periodic_packet_ms = rtc::TimeMillis(); EXPECT_GE(time_of_last_periodic_packet_ms - time_of_non_periodic_packet_ms, - config.report_period_ms - 1); + config.report_period.ms() - 1); // Cleanup. rtc::Event done;