From bba1a2e476bcf1694e468477002268fe4ff8f786 Mon Sep 17 00:00:00 2001 From: Joachim Reiersen Date: Wed, 18 Sep 2024 10:51:54 -0700 Subject: [PATCH] Propagate Environment to RtpPacketHistory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passing Environment instead of Clock into this class simplifies some plumbing for downstream consumers that need to read field trials within this class. Bug: webrtc:362762208 Change-Id: Ia501e9f7f1d91a8115a2f71fb005dd35146db172 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362535 Reviewed-by: Danil Chapovalov Commit-Queue: Danil Chapovalov Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/main@{#43048} --- modules/rtp_rtcp/source/rtp_packet_history.cc | 9 +++++++++ modules/rtp_rtcp/source/rtp_packet_history.h | 7 ++++++- .../source/rtp_packet_history_unittest.cc | 17 ++++++++++++----- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 3 +-- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 3 +-- .../source/rtp_sender_egress_unittest.cc | 2 +- modules/rtp_rtcp/source/rtp_sender_unittest.cc | 2 +- 7 files changed, 31 insertions(+), 12 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc index 936f6f228f..717b22de98 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history.cc @@ -50,6 +50,15 @@ void RtpPacketHistory::StoredPacket::IncrementTimesRetransmitted() { ++times_retransmitted_; } +RtpPacketHistory::RtpPacketHistory(const Environment& env, + PaddingMode padding_mode) + : clock_(&env.clock()), + padding_mode_(padding_mode), + number_to_store_(0), + mode_(StorageMode::kDisabled), + rtt_(TimeDelta::MinusInfinity()), + packets_inserted_(0) {} + RtpPacketHistory::RtpPacketHistory(Clock* clock, PaddingMode padding_mode) : clock_(clock), padding_mode_(padding_mode), diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h index 512ffda1c1..3f7077408d 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.h +++ b/modules/rtp_rtcp/source/rtp_packet_history.h @@ -19,6 +19,7 @@ #include #include +#include "api/environment/environment.h" #include "api/function_view.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -56,7 +57,11 @@ class RtpPacketHistory { // With kStoreAndCull, always remove packets after 3x max(1000ms, 3x rtt). static constexpr int kPacketCullingDelayFactor = 3; - RtpPacketHistory(Clock* clock, PaddingMode padding_mode); + RtpPacketHistory(const Environment& env, PaddingMode padding_mode); + + [[deprecated("Use Environment constructor")]] RtpPacketHistory( + Clock* clock, + PaddingMode padding_mode); RtpPacketHistory() = delete; RtpPacketHistory(const RtpPacketHistory&) = delete; diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc index 99d6b0628f..13febf3853 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc @@ -15,6 +15,7 @@ #include #include +#include "api/environment/environment_factory.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -57,9 +58,11 @@ class RtpPacketHistoryTest protected: RtpPacketHistoryTest() : fake_clock_(123456), - hist_(&fake_clock_, /*enable_padding_prio=*/GetParam()) {} + env_(CreateEnvironment(&fake_clock_)), + hist_(env_, /*enable_padding_prio=*/GetParam()) {} SimulatedClock fake_clock_; + Environment env_; RtpPacketHistory hist_; std::unique_ptr CreateRtpPacket(uint16_t seq_num) { @@ -626,7 +629,8 @@ INSTANTIATE_TEST_SUITE_P( TEST(RtpPacketHistoryRecentLargePacketMode, GetPayloadPaddingPacketAfterCullWithAcksReturnOldPacket) { SimulatedClock fake_clock(1234); - RtpPacketHistory history(&fake_clock, + Environment env = CreateEnvironment(&fake_clock); + RtpPacketHistory history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket); history.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); @@ -646,7 +650,8 @@ TEST(RtpPacketHistoryRecentLargePacketMode, TEST(RtpPacketHistoryRecentLargePacketMode, GetPayloadPaddingPacketIgnoreSmallRecentPackets) { SimulatedClock fake_clock(1234); - RtpPacketHistory history(&fake_clock, + Environment env = CreateEnvironment(&fake_clock); + RtpPacketHistory history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket); history.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); std::unique_ptr packet = CreatePacket(kStartSeqNum); @@ -667,7 +672,8 @@ TEST(RtpPacketHistoryRecentLargePacketMode, TEST(RtpPacketHistoryRecentLargePacketMode, GetPayloadPaddingPacketReturnsRecentPacketIfSizeNearMax) { SimulatedClock fake_clock(1234); - RtpPacketHistory history(&fake_clock, + Environment env = CreateEnvironment(&fake_clock); + RtpPacketHistory history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket); history.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); std::unique_ptr packet = CreatePacket(kStartSeqNum); @@ -688,7 +694,8 @@ TEST(RtpPacketHistoryRecentLargePacketMode, TEST(RtpPacketHistoryRecentLargePacketMode, GetPayloadPaddingPacketReturnsLastPacketAfterLargeSequenceNumberGap) { SimulatedClock fake_clock(1234); - RtpPacketHistory history(&fake_clock, + Environment env = CreateEnvironment(&fake_clock); + RtpPacketHistory history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket); history.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); uint16_t sequence_number = std::numeric_limits::max() - 50; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 2bfbb1a22d..568b1e22d2 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -58,8 +58,7 @@ constexpr TimeDelta kDefaultExpectedRetransmissionTime = TimeDelta::Millis(125); ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext( const Environment& env, const RtpRtcpInterface::Configuration& config) - : packet_history(&env.clock(), - RtpPacketHistory::PaddingMode::kRecentLargePacket), + : packet_history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket), sequencer_(config.local_media_ssrc, config.rtx_send_ssrc, /*require_marker_before_media_padding=*/!config.audio, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 4806c70250..fe21008cf1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -73,8 +73,7 @@ ModuleRtpRtcpImpl2::RtpSenderContext::RtpSenderContext( const Environment& env, TaskQueueBase& worker_queue, const RtpRtcpInterface::Configuration& config) - : packet_history(&env.clock(), - RtpPacketHistory::PaddingMode::kRecentLargePacket), + : packet_history(env, RtpPacketHistory::PaddingMode::kRecentLargePacket), sequencer(config.local_media_ssrc, config.rtx_send_ssrc, /*require_marker_before_media_padding=*/!config.audio, diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index b62239e349..c1e4772b85 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc @@ -124,7 +124,7 @@ class RtpSenderEgressTest : public ::testing::Test { : time_controller_(kStartTime), env_(CreateEnvironment(time_controller_.GetClock())), transport_(&header_extensions_), - packet_history_(&env_.clock(), + packet_history_(env_, RtpPacketHistory::PaddingMode::kRecentLargePacket), sequence_number_(kStartSequenceNumber) {} diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 42ecbb5c11..2b9a2c6428 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -154,7 +154,7 @@ class RtpSenderTest : public ::testing::Test { void CreateSender(const RtpRtcpInterface::Configuration& config) { packet_history_ = std::make_unique( - &env_.clock(), RtpPacketHistory::PaddingMode::kRecentLargePacket); + env_, RtpPacketHistory::PaddingMode::kRecentLargePacket); sequencer_.emplace(kSsrc, kRtxSsrc, /*require_marker_before_media_padding=*/!config.audio, &env_.clock());