Remove unused WebRTC-LimitPaddingSize field trial

Bug: webrtc:11508
Change-Id: Ib7d48e23bd44e2f948d51800090fc14b873d11eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268122
Auto-Submit: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37485}

modules/rtp_rtcp/BUILD.gn

@@ -656,6 +656,7 @@ if (rtc_include_tests) {
       "../../test:mock_transport",
       "../../test:rtp_test_utils",
       "../../test:run_loop",
+      "../../test:scoped_key_value_config",
       "../../test:test_support",
       "../../test/time_controller:time_controller",
       "../video_coding:codec_globals_headers",

modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc

@@ -165,19 +165,13 @@ class FieldTrialConfig : public FieldTrialsView {
     return trials;
   }
 
-  FieldTrialConfig() : overhead_enabled_(false), max_padding_factor_(1200) {}
+  FieldTrialConfig() : overhead_enabled_(false) {}
   ~FieldTrialConfig() override {}
 
   void SetOverHeadEnabled(bool enabled) { overhead_enabled_ = enabled; }
-  void SetMaxPaddingFactor(double factor) { max_padding_factor_ = factor; }
 
   std::string Lookup(absl::string_view key) const override {
-    if (key == "WebRTC-LimitPaddingSize") {
+    if (key == "WebRTC-SendSideBwe-WithOverhead") {
-      char string_buf[32];
-      rtc::SimpleStringBuilder ssb(string_buf);
-      ssb << "factor:" << max_padding_factor_;
-      return ssb.str();
-    } else if (key == "WebRTC-SendSideBwe-WithOverhead") {
       return overhead_enabled_ ? "Enabled" : "Disabled";
     }
     return "";
@@ -185,7 +179,6 @@ class FieldTrialConfig : public FieldTrialsView {
 
  private:
   bool overhead_enabled_;
-  double max_padding_factor_;
 };
 
 class RtpRtcpModule : public RtcpPacketTypeCounterObserver,

modules/rtp_rtcp/source/rtp_sender.cc

@@ -46,6 +46,10 @@ constexpr size_t kRtpHeaderLength = 12;
 // Min size needed to get payload padding from packet history.
 constexpr int kMinPayloadPaddingBytes = 50;
 
+// Determines how much larger a payload padding packet may be, compared to the
+// requested padding size.
+constexpr double kMaxPaddingSizeFactor = 3.0;
+
 template <typename Extension>
 constexpr RtpExtensionSize CreateExtensionSize() {
   return {Extension::kId, Extension::kValueSizeBytes};
@@ -139,21 +143,6 @@ bool HasBweExtension(const RtpHeaderExtensionMap& extensions_map) {
          extensions_map.IsRegistered(kRtpExtensionTransmissionTimeOffset);
 }
 
-double GetMaxPaddingSizeFactor(const FieldTrialsView* field_trials) {
-  // Too low factor means RTX payload padding is rarely used and ineffective.
-  // Too high means we risk interrupting regular media packets.
-  // In practice, 3x seems to yield reasonable results.
-  constexpr double kDefaultFactor = 3.0;
-  if (!field_trials) {
-    return kDefaultFactor;
-  }
-
-  FieldTrialOptional<double> factor("factor", kDefaultFactor);
-  ParseFieldTrial({&factor}, field_trials->Lookup("WebRTC-LimitPaddingSize"));
-  RTC_CHECK_GE(factor.Value(), 0.0);
-  return factor.Value();
-}
-
 }  // namespace
 
 RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config,
@@ -166,7 +155,6 @@ RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config,
       rtx_ssrc_(config.rtx_send_ssrc),
       flexfec_ssrc_(config.fec_generator ? config.fec_generator->FecSsrc()
                                          : absl::nullopt),
-      max_padding_size_factor_(GetMaxPaddingSizeFactor(config.field_trials)),
       packet_history_(packet_history),
       paced_sender_(packet_sender),
       sending_media_(true),                   // Default to sending media.
@@ -392,11 +380,10 @@ std::vector<std::unique_ptr<RtpPacketToSend>> RTPSender::GeneratePadding(
           packet_history_->GetPayloadPaddingPacket(
               [&](const RtpPacketToSend& packet)
                   -> std::unique_ptr<RtpPacketToSend> {
-                // Limit overshoot, generate <= `max_padding_size_factor_` *
+                // Limit overshoot, generate <= `kMaxPaddingSizeFactor` *
-                // target_size_bytes.
+                // `target_size_bytes`.
                 const size_t max_overshoot_bytes = static_cast<size_t>(
-                    ((max_padding_size_factor_ - 1.0) * target_size_bytes) +
+                    ((kMaxPaddingSizeFactor - 1.0) * target_size_bytes) + 0.5);
-                    0.5);
                 if (packet.payload_size() + kRtxHeaderSize >
                     max_overshoot_bytes + bytes_left) {
                   return nullptr;

modules/rtp_rtcp/source/rtp_sender.h

@@ -175,9 +175,6 @@ class RTPSender {
   const uint32_t ssrc_;
   const absl::optional<uint32_t> rtx_ssrc_;
   const absl::optional<uint32_t> flexfec_ssrc_;
-  // Limits GeneratePadding() outcome to <=
-  //  `max_padding_size_factor_` * `target_size_bytes`
-  const double max_padding_size_factor_;
 
   RtpPacketHistory* const packet_history_;
   RtpPacketSender* const paced_sender_;

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -40,6 +40,7 @@
 #include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/mock_transport.h"
+#include "test/scoped_key_value_config.h"
 #include "test/time_controller/simulated_time_controller.h"
 
 namespace webrtc {
@@ -73,6 +74,7 @@ const size_t kMaxPaddingLength = 224;      // Value taken from rtp_sender.cc.
 const uint32_t kTimestampTicksPerMs = 90;  // 90kHz clock.
 constexpr absl::string_view kMid = "mid";
 constexpr absl::string_view kRid = "f";
+constexpr bool kMarkerBit = true;
 
 using ::testing::_;
 using ::testing::AllOf;
@@ -102,27 +104,6 @@ class MockRtpPacketPacer : public RtpPacketSender {
               (override));
 };
 
-class FieldTrialConfig : public FieldTrialsView {
- public:
-  FieldTrialConfig() : max_padding_factor_(1200) {}
-  ~FieldTrialConfig() override {}
-
-  void SetMaxPaddingFactor(double factor) { max_padding_factor_ = factor; }
-
-  std::string Lookup(absl::string_view key) const override {
-    if (key == "WebRTC-LimitPaddingSize") {
-      char string_buf[32];
-      rtc::SimpleStringBuilder ssb(string_buf);
-      ssb << "factor:" << max_padding_factor_;
-      return ssb.str();
-    }
-    return "";
-  }
-
- private:
-  double max_padding_factor_;
-};
-
 }  // namespace
 
 class RtpSenderTest : public ::testing::Test {
@@ -138,8 +119,7 @@ class RtpSenderTest : public ::testing::Test {
                         std::vector<RtpExtension>(),
                         std::vector<RtpExtensionSize>(),
                         nullptr,
-                        clock_),
+                        clock_) {}
-        kMarkerBit(true) {}
 
   void SetUp() override { SetUpRtpSender(true, false, nullptr); }
 
@@ -191,8 +171,7 @@ class RtpSenderTest : public ::testing::Test {
   std::unique_ptr<RtpPacketHistory> packet_history_;
   std::unique_ptr<RTPSender> rtp_sender_;
 
-  const bool kMarkerBit;
+  const test::ScopedKeyValueConfig field_trials_;
-  FieldTrialConfig field_trials_;
 
   std::unique_ptr<RtpPacketToSend> BuildRtpPacket(int payload_type,
                                                   bool marker_bit,
@@ -562,7 +541,7 @@ TEST_F(RtpSenderTest, KeepsTimestampsOnPayloadPadding) {
   // Timestamps as set based on capture time in RtpSenderTest.
   const int64_t start_time = clock_->TimeInMilliseconds();
   const uint32_t start_timestamp = start_time * kTimestampTicksPerMs;
-  const size_t kPayloadSize = 600;
+  const size_t kPayloadSize = 200;
   const size_t kRtxHeaderSize = 2;
 
   // Start by sending one media packet and putting in the packet history.
@@ -1108,9 +1087,8 @@ TEST_F(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
 }
 
 TEST_F(RtpSenderTest, LimitsPayloadPaddingSize) {
-  // Limit RTX payload padding to 2x target size.
+  // RTX payload padding is limited to 3x target size.
-  const double kFactor = 2.0;
+  const double kFactor = 3.0;
-  field_trials_.SetMaxPaddingFactor(kFactor);
   SetUpRtpSender(false, false, nullptr);
   rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload);
   rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads);
@@ -1313,11 +1291,10 @@ TEST_F(RtpSenderTest, DoesntFecProtectRetransmissions) {
 }
 
 TEST_F(RtpSenderTest, MarksPacketsWithKeyframeStatus) {
-  FieldTrialBasedConfig field_trials;
   RTPSenderVideo::Config video_config;
   video_config.clock = clock_;
   video_config.rtp_sender = rtp_sender_.get();
-  video_config.field_trials = &field_trials;
+  video_config.field_trials = &field_trials_;
   RTPSenderVideo rtp_sender_video(video_config);
 
   const uint8_t kPayloadType = 127;