From 67ba914249c970c35ece92a74f391f247f550250 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 10 May 2024 15:10:24 +0200 Subject: [PATCH] Propagate FieldTrialsView through FEC protection method helpers And thus in those helpers query RateControlSettings field trials via propagated FieldTrialView instead of the via global field trial string Bug: webrtc:42220378 Change-Id: I84f4bf42037d864519c4d2031d25cf909fd5010f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350305 Commit-Queue: Philip Eliasson Reviewed-by: Philip Eliasson Auto-Submit: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#42286} --- .../video_coding/fec_controller_default.cc | 3 +-- modules/video_coding/media_opt_util.cc | 25 +++++++++++-------- modules/video_coding/media_opt_util.h | 11 +++++--- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/modules/video_coding/fec_controller_default.cc b/modules/video_coding/fec_controller_default.cc index d548d6580c..9933782226 100644 --- a/modules/video_coding/fec_controller_default.cc +++ b/modules/video_coding/fec_controller_default.cc @@ -30,8 +30,7 @@ FecControllerDefault::FecControllerDefault( VCMProtectionCallback* protection_callback) : env_(env), protection_callback_(protection_callback), - loss_prot_logic_(new media_optimization::VCMLossProtectionLogic( - env_.clock().TimeInMilliseconds())), + loss_prot_logic_(new media_optimization::VCMLossProtectionLogic(env_)), max_payload_size_(1460), overhead_threshold_(GetProtectionOverheadRateThreshold()) {} diff --git a/modules/video_coding/media_opt_util.cc b/modules/video_coding/media_opt_util.cc index 7580c95fc7..cdcc6e7652 100644 --- a/modules/video_coding/media_opt_util.cc +++ b/modules/video_coding/media_opt_util.cc @@ -13,13 +13,16 @@ #include #include +#include +#include "api/field_trials_view.h" #include "modules/video_coding/fec_rate_table.h" #include "modules/video_coding/internal_defines.h" #include "modules/video_coding/utility/simulcast_rate_allocator.h" #include "rtc_base/checks.h" #include "rtc_base/experiments/rate_control_settings.h" #include "rtc_base/numerics/safe_conversions.h" +#include "system_wrappers/include/clock.h" namespace webrtc { // Max value of loss rates in off-line model @@ -80,9 +83,10 @@ int VCMProtectionMethod::MaxFramesFec() const { return 1; } -VCMNackFecMethod::VCMNackFecMethod(int64_t lowRttNackThresholdMs, +VCMNackFecMethod::VCMNackFecMethod(const FieldTrialsView& field_trials, + int64_t lowRttNackThresholdMs, int64_t highRttNackThresholdMs) - : VCMFecMethod(), + : VCMFecMethod(field_trials), _lowRttNackMs(lowRttNackThresholdMs), _highRttNackMs(highRttNackThresholdMs), _maxFramesFec(1) { @@ -244,9 +248,8 @@ bool VCMNackMethod::UpdateParameters( return true; } -VCMFecMethod::VCMFecMethod() - : VCMProtectionMethod(), - rate_control_settings_(RateControlSettings::ParseFromFieldTrials()) { +VCMFecMethod::VCMFecMethod(const FieldTrialsView& field_trials) + : rate_control_settings_(field_trials) { _type = kFec; } @@ -489,8 +492,9 @@ bool VCMFecMethod::UpdateParameters(const VCMProtectionParameters* parameters) { return true; } -VCMLossProtectionLogic::VCMLossProtectionLogic(int64_t nowMs) - : _currentParameters(), +VCMLossProtectionLogic::VCMLossProtectionLogic(const Environment& env) + : env_(env), + _currentParameters(), _rtt(0), _lossPr(0.0f), _bitRate(0.0f), @@ -507,7 +511,7 @@ VCMLossProtectionLogic::VCMLossProtectionLogic(int64_t nowMs) _codecWidth(704), _codecHeight(576), _numLayers(1) { - Reset(nowMs); + Reset(env_.clock().CurrentTime().ms()); } VCMLossProtectionLogic::~VCMLossProtectionLogic() { @@ -524,10 +528,11 @@ void VCMLossProtectionLogic::SetMethod( _selectedMethod.reset(new VCMNackMethod()); break; case kFec: - _selectedMethod.reset(new VCMFecMethod()); + _selectedMethod = std::make_unique(env_.field_trials()); break; case kNackFec: - _selectedMethod.reset(new VCMNackFecMethod(kLowRttNackMs, -1)); + _selectedMethod = std::make_unique(env_.field_trials(), + kLowRttNackMs, -1); break; case kNone: _selectedMethod.reset(); diff --git a/modules/video_coding/media_opt_util.h b/modules/video_coding/media_opt_util.h index a74d1af6cb..ad77f52fcb 100644 --- a/modules/video_coding/media_opt_util.h +++ b/modules/video_coding/media_opt_util.h @@ -16,6 +16,8 @@ #include +#include "api/environment/environment.h" +#include "api/field_trials_view.h" #include "modules/video_coding/internal_defines.h" #include "rtc_base/experiments/rate_control_settings.h" #include "rtc_base/numerics/exp_filter.h" @@ -153,7 +155,7 @@ class VCMNackMethod : public VCMProtectionMethod { class VCMFecMethod : public VCMProtectionMethod { public: - VCMFecMethod(); + explicit VCMFecMethod(const FieldTrialsView& field_trials); ~VCMFecMethod() override; bool UpdateParameters(const VCMProtectionParameters* parameters) override; // Get the effective packet loss for ER @@ -190,7 +192,8 @@ class VCMFecMethod : public VCMProtectionMethod { class VCMNackFecMethod : public VCMFecMethod { public: - VCMNackFecMethod(int64_t lowRttNackThresholdMs, + VCMNackFecMethod(const FieldTrialsView& field_trials, + int64_t lowRttNackThresholdMs, int64_t highRttNackThresholdMs); ~VCMNackFecMethod() override; bool UpdateParameters(const VCMProtectionParameters* parameters) override; @@ -213,7 +216,7 @@ class VCMNackFecMethod : public VCMFecMethod { class VCMLossProtectionLogic { public: - explicit VCMLossProtectionLogic(int64_t nowMs); + explicit VCMLossProtectionLogic(const Environment& env); ~VCMLossProtectionLogic(); // Set the protection method to be used @@ -322,6 +325,8 @@ class VCMLossProtectionLogic { // Sets the available loss protection methods. void UpdateMaxLossHistory(uint8_t lossPr255, int64_t now); uint8_t MaxFilteredLossPr(int64_t nowMs) const; + + const Environment env_; std::unique_ptr _selectedMethod; VCMProtectionParameters _currentParameters; int64_t _rtt;