admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move field trial check WebRTC-DisableRtxRateLimiter

Browse Source 
Checking in sending classes avoids using global field trial string in favor of the injected one.

In addition to that RateLimiter looks wrong layer for check that field trial:
checking inside RateLimiter class might be surprising if it is used for limiting something else than RTX bitrate.
evaluating field trial for each retransmitting packet might be expensive

Bug: webrtc:15184, webrtc:10335
Change-Id: I87bae3522bbd9692629d4f9b6caa119be03f2bd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322720
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40908}
This commit is contained in:
Danil Chapovalov 2023-10-09 15:28:57 +02:00 committed by WebRTC LUCI CQ
parent c4f3919fdd
commit c941579e95
5 changed files with 8 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
audio/channel_send.cc
View File

@ -411,8 +411,10 @@ ChannelSend::ChannelSend(
configuration.event_log = event_log_; configuration.event_log = event_log_;
configuration.rtt_stats = rtcp_rtt_stats; configuration.rtt_stats = rtcp_rtt_stats;
configuration.retransmission_rate_limiter = if (!field_trials.IsEnabled("WebRTC-DisableRtxRateLimiter")) {
retransmission_rate_limiter_.get(); configuration.retransmission_rate_limiter =
retransmission_rate_limiter_.get();
}
configuration.extmap_allow_mixed = extmap_allow_mixed; configuration.extmap_allow_mixed = extmap_allow_mixed;
configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; configuration.rtcp_report_interval_ms = rtcp_report_interval_ms;
configuration.rtcp_packet_type_counter_observer = this; configuration.rtcp_packet_type_counter_observer = this;

4
call/rtp_video_sender.cc
View File

@ -226,7 +226,9 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
configuration.send_bitrate_observer = observers.bitrate_observer; configuration.send_bitrate_observer = observers.bitrate_observer;
configuration.send_packet_observer = observers.send_packet_observer; configuration.send_packet_observer = observers.send_packet_observer;
configuration.event_log = event_log; configuration.event_log = event_log;
configuration.retransmission_rate_limiter = retransmission_rate_limiter; if (!trials.IsEnabled("WebRTC-DisableRtxRateLimiter")) {
configuration.retransmission_rate_limiter = retransmission_rate_limiter;
}
configuration.rtp_stats_callback = observers.rtp_stats; configuration.rtp_stats_callback = observers.rtp_stats;
configuration.frame_encryptor = frame_encryptor; configuration.frame_encryptor = frame_encryptor;
configuration.require_frame_encryption = configuration.require_frame_encryption =

2
rtc_base/BUILD.gn
View File

@ -525,7 +525,6 @@ rtc_library("rate_limiter") {
":macromagic", ":macromagic",
":rate_statistics", ":rate_statistics",
"../system_wrappers", "../system_wrappers",
"../system_wrappers:field_trial",
"synchronization:mutex", "synchronization:mutex",
] ]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
@ -1964,7 +1963,6 @@ if (rtc_include_tests) {
"../api/units:time_delta", "../api/units:time_delta",
"../api/units:timestamp", "../api/units:timestamp",
"../system_wrappers", "../system_wrappers",
"../test:field_trial",
"../test:fileutils", "../test:fileutils",
"../test:test_main", "../test:test_main",
"../test:test_support", "../test:test_support",

4
rtc_base/rate_limiter.cc
View File

@ -14,7 +14,6 @@
#include "absl/types/optional.h" #include "absl/types/optional.h"
#include "system_wrappers/include/clock.h" #include "system_wrappers/include/clock.h"
#include "system_wrappers/include/field_trial.h"
namespace webrtc { namespace webrtc {
@ -35,8 +34,7 @@ bool RateLimiter::TryUseRate(size_t packet_size_bytes) {
MutexLock lock(&lock_); MutexLock lock(&lock_);
int64_t now_ms = clock_->TimeInMilliseconds(); int64_t now_ms = clock_->TimeInMilliseconds();
absl::optional<uint32_t> current_rate = current_rate_.Rate(now_ms); absl::optional<uint32_t> current_rate = current_rate_.Rate(now_ms);
if (!webrtc::field_trial::IsEnabled("WebRTC-DisableRtxRateLimiter") && if (current_rate) {
current_rate) {
// If there is a current rate, check if adding bytes would cause maximum // If there is a current rate, check if adding bytes would cause maximum
// bitrate target to be exceeded. If there is NOT a valid current rate, // bitrate target to be exceeded. If there is NOT a valid current rate,
// allow allocating rate even if target is exceeded. This prevents // allow allocating rate even if target is exceeded. This prevents

14
rtc_base/rate_limiter_unittest.cc
View File

@ -15,7 +15,6 @@
#include "rtc_base/event.h" #include "rtc_base/event.h"
#include "rtc_base/platform_thread.h" #include "rtc_base/platform_thread.h"
#include "system_wrappers/include/clock.h" #include "system_wrappers/include/clock.h"
#include "test/field_trial.h"
#include "test/gtest.h" #include "test/gtest.h"
namespace webrtc { namespace webrtc {
@ -107,19 +106,6 @@ TEST_F(RateLimitTest, WindowSizeLimits) {
EXPECT_FALSE(rate_limiter->SetWindowSize(kWindowSizeMs + 1)); EXPECT_FALSE(rate_limiter->SetWindowSize(kWindowSizeMs + 1));
} }
TEST_F(RateLimitTest, DiablesRtxRateLimiterByFieldTrial) {
webrtc::test::ScopedFieldTrials trial(
"WebRTC-DisableRtxRateLimiter/Enabled/");
// Fill rate, extend window to full size.
EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2));
clock_.AdvanceTimeMilliseconds(kWindowSizeMs - 1);
EXPECT_TRUE(rate_limiter->TryUseRate(kRateFillingBytes / 2));
// Does not limit rate even when all rate consumed.
EXPECT_TRUE(rate_limiter->TryUseRate(1));
}
static constexpr TimeDelta kMaxTimeout = TimeDelta::Seconds(30); static constexpr TimeDelta kMaxTimeout = TimeDelta::Seconds(30);
class ThreadTask { class ThreadTask {