Fix filtering of small packets in delay-based BWE
crodbro@ found that the previous field trial, which filtered the deltas
in the trendline estimator, can increase the noise caused by varying
packet sizes. Moving the filtering to the DelayBasedBwe class fixes the
issue.
To avoid confusion, we've updated the field trial name, so e.g.
WebRTC-BweIgnoreSmallPacketsFix/small:200bytes,large:200bytes,
fraction_large:0.25,smoothing:0.1/
should be used to enable the feature.
Bug: webrtc:10932
Change-Id: If77e83043c37fff909038405f634e541ce41abb8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159711
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Christoffer Rodbro <crodbro@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29804}
This commit is contained in:
parent
7b46e17c31
commit
fd0e32a87a
@ -42,6 +42,22 @@ constexpr uint32_t kFixedSsrc = 0;
|
||||
|
||||
} // namespace
|
||||
|
||||
constexpr char BweIgnoreSmallPacketsSettings::kKey[];
|
||||
|
||||
BweIgnoreSmallPacketsSettings::BweIgnoreSmallPacketsSettings(
|
||||
const WebRtcKeyValueConfig* key_value_config) {
|
||||
Parser()->Parse(
|
||||
key_value_config->Lookup(BweIgnoreSmallPacketsSettings::kKey));
|
||||
}
|
||||
|
||||
std::unique_ptr<StructParametersParser>
|
||||
BweIgnoreSmallPacketsSettings::Parser() {
|
||||
return StructParametersParser::Create("smoothing", &smoothing_factor, //
|
||||
"fraction_large", &fraction_large, //
|
||||
"large", &large_threshold, //
|
||||
"small", &small_threshold);
|
||||
}
|
||||
|
||||
DelayBasedBwe::Result::Result()
|
||||
: updated(false),
|
||||
probe(false),
|
||||
@ -63,6 +79,8 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config,
|
||||
NetworkStatePredictor* network_state_predictor)
|
||||
: event_log_(event_log),
|
||||
key_value_config_(key_value_config),
|
||||
ignore_small_(key_value_config),
|
||||
fraction_large_packets_(0.5),
|
||||
network_state_predictor_(network_state_predictor),
|
||||
inter_arrival_(),
|
||||
delay_detector_(
|
||||
@ -75,7 +93,12 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config,
|
||||
prev_state_(BandwidthUsage::kBwNormal),
|
||||
alr_limited_backoff_enabled_(
|
||||
key_value_config->Lookup("WebRTC-Bwe-AlrLimitedBackoff")
|
||||
.find("Enabled") == 0) {}
|
||||
.find("Enabled") == 0) {
|
||||
RTC_LOG(LS_INFO) << "Initialized DelayBasedBwe with field trial "
|
||||
<< ignore_small_.Parser()->Encode()
|
||||
<< " and alr limited backoff "
|
||||
<< (alr_limited_backoff_enabled_ ? "enabled" : "disabled");
|
||||
}
|
||||
|
||||
DelayBasedBwe::~DelayBasedBwe() {}
|
||||
|
||||
@ -151,18 +174,33 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback,
|
||||
// so wrapping works properly.
|
||||
uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift;
|
||||
|
||||
// Ignore "small" packets if many/most packets in the call are "large". The
|
||||
// packet size may have a significant effect on the propagation delay,
|
||||
// especially at low bandwidths. Variations in packet size will then show up
|
||||
// as noise in the delay measurement. By default, we include all packets.
|
||||
DataSize packet_size = packet_feedback.sent_packet.size;
|
||||
if (!ignore_small_.small_threshold.IsZero()) {
|
||||
double is_large =
|
||||
static_cast<double>(packet_size >= ignore_small_.large_threshold);
|
||||
fraction_large_packets_ +=
|
||||
ignore_small_.smoothing_factor * (is_large - fraction_large_packets_);
|
||||
if (packet_size <= ignore_small_.small_threshold &&
|
||||
fraction_large_packets_ >= ignore_small_.fraction_large) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
uint32_t ts_delta = 0;
|
||||
int64_t t_delta = 0;
|
||||
int size_delta = 0;
|
||||
bool calculated_deltas = inter_arrival_->ComputeDeltas(
|
||||
timestamp, packet_feedback.receive_time.ms(), at_time.ms(),
|
||||
packet_feedback.sent_packet.size.bytes(), &ts_delta, &t_delta,
|
||||
&size_delta);
|
||||
packet_size.bytes(), &ts_delta, &t_delta, &size_delta);
|
||||
double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift);
|
||||
delay_detector_->Update(
|
||||
t_delta, ts_delta_ms, packet_feedback.sent_packet.send_time.ms(),
|
||||
packet_feedback.receive_time.ms(),
|
||||
packet_feedback.sent_packet.size.bytes(), calculated_deltas);
|
||||
delay_detector_->Update(t_delta, ts_delta_ms,
|
||||
packet_feedback.sent_packet.send_time.ms(),
|
||||
packet_feedback.receive_time.ms(),
|
||||
packet_size.bytes(), calculated_deltas);
|
||||
}
|
||||
|
||||
DataRate DelayBasedBwe::TriggerOveruse(Timestamp at_time,
|
||||
|
||||
@ -27,11 +27,27 @@
|
||||
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
|
||||
#include "modules/remote_bitrate_estimator/inter_arrival.h"
|
||||
#include "rtc_base/constructor_magic.h"
|
||||
#include "rtc_base/experiments/struct_parameters_parser.h"
|
||||
#include "rtc_base/race_checker.h"
|
||||
|
||||
namespace webrtc {
|
||||
class RtcEventLog;
|
||||
|
||||
struct BweIgnoreSmallPacketsSettings {
|
||||
static constexpr char kKey[] = "WebRTC-BweIgnoreSmallPacketsFix";
|
||||
|
||||
BweIgnoreSmallPacketsSettings() = default;
|
||||
explicit BweIgnoreSmallPacketsSettings(
|
||||
const WebRtcKeyValueConfig* key_value_config);
|
||||
|
||||
double smoothing_factor = 0.1;
|
||||
double fraction_large = 1.0;
|
||||
DataSize large_threshold = DataSize::Zero();
|
||||
DataSize small_threshold = DataSize::Zero();
|
||||
|
||||
std::unique_ptr<StructParametersParser> Parser();
|
||||
};
|
||||
|
||||
class DelayBasedBwe {
|
||||
public:
|
||||
struct Result {
|
||||
@ -86,6 +102,12 @@ class DelayBasedBwe {
|
||||
rtc::RaceChecker network_race_;
|
||||
RtcEventLog* const event_log_;
|
||||
const WebRtcKeyValueConfig* const key_value_config_;
|
||||
|
||||
// Filtering out small packets. Intention is to base the detection only
|
||||
// on video packets even if we have TWCC sequence numbers for audio.
|
||||
BweIgnoreSmallPacketsSettings ignore_small_;
|
||||
double fraction_large_packets_;
|
||||
|
||||
NetworkStatePredictor* network_state_predictor_;
|
||||
std::unique_ptr<InterArrival> inter_arrival_;
|
||||
std::unique_ptr<DelayIncreaseDetectorInterface> delay_detector_;
|
||||
@ -96,7 +118,6 @@ class DelayBasedBwe {
|
||||
bool has_once_detected_overuse_;
|
||||
BandwidthUsage prev_state_;
|
||||
bool alr_limited_backoff_enabled_;
|
||||
|
||||
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe);
|
||||
};
|
||||
|
||||
|
||||
@ -25,23 +25,6 @@
|
||||
|
||||
namespace webrtc {
|
||||
|
||||
constexpr char BweIgnoreSmallPacketsSettings::kKey[];
|
||||
|
||||
BweIgnoreSmallPacketsSettings::BweIgnoreSmallPacketsSettings(
|
||||
const WebRtcKeyValueConfig* key_value_config) {
|
||||
Parser()->Parse(
|
||||
key_value_config->Lookup(BweIgnoreSmallPacketsSettings::kKey));
|
||||
}
|
||||
|
||||
std::unique_ptr<StructParametersParser>
|
||||
BweIgnoreSmallPacketsSettings::Parser() {
|
||||
return StructParametersParser::Create(
|
||||
"smoothing_factor", &smoothing_factor, //
|
||||
"min_fraction_large_packets", &min_fraction_large_packets, //
|
||||
"large_packet_size", &large_packet_size, //
|
||||
"ignored_size", &ignored_size);
|
||||
}
|
||||
|
||||
namespace {
|
||||
|
||||
// Parameters for linear least squares fit of regression line to noisy data.
|
||||
@ -102,9 +85,7 @@ constexpr int kDeltaCounterMax = 1000;
|
||||
TrendlineEstimator::TrendlineEstimator(
|
||||
const WebRtcKeyValueConfig* key_value_config,
|
||||
NetworkStatePredictor* network_state_predictor)
|
||||
: ignore_small_packets_(key_value_config),
|
||||
fraction_large_packets_(0.5),
|
||||
window_size_(key_value_config->Lookup(kBweWindowSizeInPacketsExperiment)
|
||||
: window_size_(key_value_config->Lookup(kBweWindowSizeInPacketsExperiment)
|
||||
.find("Enabled") == 0
|
||||
? ReadTrendlineFilterWindowSize(key_value_config)
|
||||
: kDefaultTrendlineWindowSize),
|
||||
@ -129,8 +110,7 @@ TrendlineEstimator::TrendlineEstimator(
|
||||
network_state_predictor_(network_state_predictor) {
|
||||
RTC_LOG(LS_INFO)
|
||||
<< "Using Trendline filter for delay change estimation with window size "
|
||||
<< window_size_ << ", field trial "
|
||||
<< ignore_small_packets_.Parser()->Encode() << " and "
|
||||
<< window_size_ << " and "
|
||||
<< (network_state_predictor_ ? "injected" : "no")
|
||||
<< " network state predictor";
|
||||
}
|
||||
@ -142,23 +122,6 @@ void TrendlineEstimator::UpdateTrendline(double recv_delta_ms,
|
||||
int64_t send_time_ms,
|
||||
int64_t arrival_time_ms,
|
||||
size_t packet_size) {
|
||||
if (ignore_small_packets_.ignored_size > 0) {
|
||||
// Process the packet if it is "large" or if all packets in the call are
|
||||
// "small". The packet size may have a significant effect on the propagation
|
||||
// delay, especially at low bandwidths. Variations in packet size will then
|
||||
// show up as noise in the delay measurement.
|
||||
// By default, we include all packets.
|
||||
fraction_large_packets_ =
|
||||
(1 - ignore_small_packets_.smoothing_factor) * fraction_large_packets_ +
|
||||
ignore_small_packets_.smoothing_factor *
|
||||
(packet_size >= ignore_small_packets_.large_packet_size);
|
||||
if (packet_size <= ignore_small_packets_.ignored_size &&
|
||||
fraction_large_packets_ >=
|
||||
ignore_small_packets_.min_fraction_large_packets) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const double delta_ms = recv_delta_ms - send_delta_ms;
|
||||
++num_of_deltas_;
|
||||
num_of_deltas_ = std::min(num_of_deltas_, kDeltaCounterMax);
|
||||
|
||||
@ -22,25 +22,9 @@
|
||||
#include "modules/congestion_controller/goog_cc/delay_increase_detector_interface.h"
|
||||
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
|
||||
#include "rtc_base/constructor_magic.h"
|
||||
#include "rtc_base/experiments/struct_parameters_parser.h"
|
||||
|
||||
namespace webrtc {
|
||||
|
||||
struct BweIgnoreSmallPacketsSettings {
|
||||
static constexpr char kKey[] = "WebRTC-BweIgnoreSmallPackets";
|
||||
|
||||
BweIgnoreSmallPacketsSettings() = default;
|
||||
explicit BweIgnoreSmallPacketsSettings(
|
||||
const WebRtcKeyValueConfig* key_value_config);
|
||||
|
||||
double smoothing_factor = 0.1;
|
||||
double min_fraction_large_packets = 1.0;
|
||||
unsigned large_packet_size = 0;
|
||||
unsigned ignored_size = 0;
|
||||
|
||||
std::unique_ptr<StructParametersParser> Parser();
|
||||
};
|
||||
|
||||
class TrendlineEstimator : public DelayIncreaseDetectorInterface {
|
||||
public:
|
||||
TrendlineEstimator(const WebRtcKeyValueConfig* key_value_config,
|
||||
@ -72,11 +56,6 @@ class TrendlineEstimator : public DelayIncreaseDetectorInterface {
|
||||
|
||||
void UpdateThreshold(double modified_offset, int64_t now_ms);
|
||||
|
||||
// Filtering out small packets. (Intention is to base the detection only
|
||||
// on video packets even if we have TWCC sequence number for audio.)
|
||||
BweIgnoreSmallPacketsSettings ignore_small_packets_;
|
||||
double fraction_large_packets_;
|
||||
|
||||
// Parameters.
|
||||
const size_t window_size_;
|
||||
const double smoothing_coef_;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user