Propagate field trials into audio NackTracker
Bug: webrtc:42220378
Change-Id: Ibf831e15b5931925a9efa9099178f71b1a23c147
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359280
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42769}
diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index 15eb29c..e96a40a 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -711,6 +711,7 @@
":webrtc_cng",
"..:module_api_public",
"../../api:array_view",
+ "../../api:field_trials_view",
"../../api:rtp_headers",
"../../api:rtp_packet_info",
"../../api:scoped_refptr",
diff --git a/modules/audio_coding/neteq/nack_tracker.cc b/modules/audio_coding/neteq/nack_tracker.cc
index 71c6fc3..a1e973b 100644
--- a/modules/audio_coding/neteq/nack_tracker.cc
+++ b/modules/audio_coding/neteq/nack_tracker.cc
@@ -13,10 +13,10 @@
#include <cstdint>
#include <utility>
+#include "api/field_trials_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/experiments/struct_parameters_parser.h"
#include "rtc_base/logging.h"
-#include "system_wrappers/include/field_trial.h"
namespace webrtc {
namespace {
@@ -28,14 +28,13 @@
} // namespace
-NackTracker::Config::Config() {
+NackTracker::Config::Config(const FieldTrialsView& field_trials) {
auto parser = StructParametersParser::Create(
"packet_loss_forget_factor", &packet_loss_forget_factor,
"ms_per_loss_percent", &ms_per_loss_percent, "never_nack_multiple_times",
&never_nack_multiple_times, "require_valid_rtt", &require_valid_rtt,
"max_loss_rate", &max_loss_rate);
- parser->Parse(
- webrtc::field_trial::FindFullName(kNackTrackerConfigFieldTrial));
+ parser->Parse(field_trials.Lookup(kNackTrackerConfigFieldTrial));
RTC_LOG(LS_INFO) << "Nack tracker config:"
" packet_loss_forget_factor="
<< packet_loss_forget_factor
@@ -45,8 +44,9 @@
<< " max_loss_rate=" << max_loss_rate;
}
-NackTracker::NackTracker()
- : sequence_num_last_received_rtp_(0),
+NackTracker::NackTracker(const FieldTrialsView& field_trials)
+ : config_(field_trials),
+ sequence_num_last_received_rtp_(0),
timestamp_last_received_rtp_(0),
any_rtp_received_(false),
sequence_num_last_decoded_rtp_(0),
diff --git a/modules/audio_coding/neteq/nack_tracker.h b/modules/audio_coding/neteq/nack_tracker.h
index d9005da..c95496d 100644
--- a/modules/audio_coding/neteq/nack_tracker.h
+++ b/modules/audio_coding/neteq/nack_tracker.h
@@ -18,6 +18,7 @@
#include <vector>
#include "absl/types/optional.h"
+#include "api/field_trials_view.h"
#include "modules/include/module_common_types_public.h"
#include "rtc_base/gtest_prod_util.h"
@@ -54,7 +55,7 @@
// A limit for the size of the NACK list.
static const size_t kNackListSizeLimit = 500; // 10 seconds for 20 ms frame
// packets.
- NackTracker();
+ explicit NackTracker(const FieldTrialsView& field_trials);
~NackTracker();
// Set a maximum for the size of the NACK list. If the last received packet
@@ -99,7 +100,7 @@
// Options that can be configured via field trial.
struct Config {
- Config();
+ explicit Config(const FieldTrialsView& field_trials);
// The exponential decay factor used to estimate the packet loss rate.
double packet_loss_forget_factor = 0.996;
diff --git a/modules/audio_coding/neteq/nack_tracker_unittest.cc b/modules/audio_coding/neteq/nack_tracker_unittest.cc
index a843425..89d3555 100644
--- a/modules/audio_coding/neteq/nack_tracker_unittest.cc
+++ b/modules/audio_coding/neteq/nack_tracker_unittest.cc
@@ -16,12 +16,14 @@
#include <memory>
#include "modules/audio_coding/include/audio_coding_module_typedefs.h"
-#include "test/field_trial.h"
+#include "test/explicit_key_value_config.h"
#include "test/gtest.h"
namespace webrtc {
namespace {
+using test::ExplicitKeyValueConfig;
+
const int kSampleRateHz = 16000;
const int kPacketSizeMs = 30;
const uint32_t kTimestampIncrement = 480; // 30 ms.
@@ -54,7 +56,8 @@
} // namespace
TEST(NackTrackerTest, EmptyListWhenNoPacketLoss) {
- NackTracker nack;
+ ExplicitKeyValueConfig field_trials("");
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
int seq_num = 1;
@@ -72,12 +75,13 @@
}
TEST(NackTrackerTest, LatePacketsMovedToNackThenNackListDoesNotChange) {
+ ExplicitKeyValueConfig field_trials("");
const uint16_t kSequenceNumberLostPackets[] = {2, 3, 4, 5, 6, 7, 8, 9};
static const int kNumAllLostPackets = sizeof(kSequenceNumberLostPackets) /
sizeof(kSequenceNumberLostPackets[0]);
for (int k = 0; k < 2; k++) { // Two iteration with/without wrap around.
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
uint16_t sequence_num_lost_packets[kNumAllLostPackets];
@@ -119,12 +123,13 @@
}
TEST(NackTrackerTest, ArrivedPacketsAreRemovedFromNackList) {
+ ExplicitKeyValueConfig field_trials("");
const uint16_t kSequenceNumberLostPackets[] = {2, 3, 4, 5, 6, 7, 8, 9};
static const int kNumAllLostPackets = sizeof(kSequenceNumberLostPackets) /
sizeof(kSequenceNumberLostPackets[0]);
for (int k = 0; k < 2; ++k) { // Two iteration with/without wrap around.
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
uint16_t sequence_num_lost_packets[kNumAllLostPackets];
@@ -180,13 +185,14 @@
// Assess if estimation of timestamps and time-to-play is correct. Introduce all
// combinations that timestamps and sequence numbers might have wrap around.
TEST(NackTrackerTest, EstimateTimestampAndTimeToPlay) {
+ ExplicitKeyValueConfig field_trials("");
const uint16_t kLostPackets[] = {2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15};
static const int kNumAllLostPackets =
sizeof(kLostPackets) / sizeof(kLostPackets[0]);
for (int k = 0; k < 4; ++k) {
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
// Sequence number wrap around if `k` is 2 or 3;
@@ -243,9 +249,10 @@
TEST(NackTrackerTest,
MissingPacketsPriorToLastDecodedRtpShouldNotBeInNackList) {
+ ExplicitKeyValueConfig field_trials("");
for (int m = 0; m < 2; ++m) {
uint16_t seq_num_offset = (m == 0) ? 0 : 65531; // Wrap around if `m` is 1.
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
// Two consecutive packets to have a correct estimate of timestamp increase.
@@ -296,7 +303,8 @@
}
TEST(NackTrackerTest, Reset) {
- NackTracker nack;
+ ExplicitKeyValueConfig field_trials("");
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
// Two consecutive packets to have a correct estimate of timestamp increase.
@@ -320,10 +328,11 @@
}
TEST(NackTrackerTest, ListSizeAppliedFromBeginning) {
+ ExplicitKeyValueConfig field_trials("");
const size_t kNackListSize = 10;
for (int m = 0; m < 2; ++m) {
uint16_t seq_num_offset = (m == 0) ? 0 : 65525; // Wrap around if `m` is 1.
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
@@ -344,10 +353,11 @@
}
TEST(NackTrackerTest, ChangeOfListSizeAppliedAndOldElementsRemoved) {
+ ExplicitKeyValueConfig field_trials("");
const size_t kNackListSize = 10;
for (int m = 0; m < 2; ++m) {
uint16_t seq_num_offset = (m == 0) ? 0 : 65525; // Wrap around if `m` is 1.
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
uint16_t seq_num = seq_num_offset;
@@ -399,8 +409,9 @@
}
TEST(NackTrackerTest, RoudTripTimeIsApplied) {
+ ExplicitKeyValueConfig field_trials("");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
@@ -430,12 +441,12 @@
// Set never_nack_multiple_times to true with a field trial and verify that
// packets are not nacked multiple times.
TEST(NackTrackerTest, DoNotNackMultipleTimes) {
- test::ScopedFieldTrials field_trials(
+ ExplicitKeyValueConfig field_trials(
"WebRTC-Audio-NetEqNackTrackerConfig/"
"packet_loss_forget_factor:0.996,ms_per_loss_percent:20,"
"never_nack_multiple_times:true/");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
@@ -461,8 +472,9 @@
// Test if estimated packet loss rate is correct.
TEST(NackTrackerTest, PacketLossRateCorrect) {
+ ExplicitKeyValueConfig field_trials("");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
uint16_t seq_num = 0;
@@ -487,8 +499,9 @@
}
TEST(NackTrackerTest, DoNotNackAfterDtx) {
+ ExplicitKeyValueConfig field_trials("");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
uint16_t seq_num = 0;
@@ -502,10 +515,10 @@
}
TEST(NackTrackerTest, DoNotNackIfLossRateIsTooHigh) {
- test::ScopedFieldTrials field_trials(
+ ExplicitKeyValueConfig field_trials(
"WebRTC-Audio-NetEqNackTrackerConfig/max_loss_rate:0.4/");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
uint16_t seq_num = 0;
@@ -527,10 +540,10 @@
}
TEST(NackTrackerTest, OnlyNackIfRttIsValid) {
- test::ScopedFieldTrials field_trials(
+ ExplicitKeyValueConfig field_trials(
"WebRTC-Audio-NetEqNackTrackerConfig/require_valid_rtt:true/");
const int kNackListSize = 200;
- NackTracker nack;
+ NackTracker nack(field_trials);
nack.UpdateSampleRate(kSampleRateHz);
nack.SetMaxNackListSize(kNackListSize);
uint16_t seq_num = 0;
diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 5ea78e3..01a4d7f 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -424,7 +424,7 @@
void NetEqImpl::EnableNack(size_t max_nack_list_size) {
MutexLock lock(&mutex_);
if (!nack_enabled_) {
- nack_ = std::make_unique<NackTracker>();
+ nack_ = std::make_unique<NackTracker>(env_.field_trials());
nack_enabled_ = true;
nack_->UpdateSampleRate(fs_hz_);
}
