Cleanup obsolete filtering of small packets in delay based estimator.
Also deletes unused constructor in Results struct.
Bug: webrtc:10932
Change-Id: Id33f57db30df49aa23fb0b5959812cc3834f1eaf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/196508
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Christoffer Rodbro <crodbro@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32777}
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
index 2390c14..dec36b7 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
@@ -46,23 +46,8 @@
constexpr uint32_t kFixedSsrc = 0;
} // namespace
-constexpr char BweIgnoreSmallPacketsSettings::kKey[];
constexpr char BweSeparateAudioPacketsSettings::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);
-}
-
BweSeparateAudioPacketsSettings::BweSeparateAudioPacketsSettings(
const WebRtcKeyValueConfig* key_value_config) {
Parser()->Parse(
@@ -84,20 +69,12 @@
recovered_from_overuse(false),
backoff_in_alr(false) {}
-DelayBasedBwe::Result::Result(bool probe, DataRate target_bitrate)
- : updated(true),
- probe(probe),
- target_bitrate(target_bitrate),
- recovered_from_overuse(false),
- backoff_in_alr(false) {}
DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config,
RtcEventLog* event_log,
NetworkStatePredictor* network_state_predictor)
: event_log_(event_log),
key_value_config_(key_value_config),
- ignore_small_(key_value_config),
- fraction_large_packets_(0.5),
separate_audio_(key_value_config),
audio_packets_since_last_video_(0),
last_video_packet_recv_time_(Timestamp::MinusInfinity()),
@@ -118,12 +95,10 @@
alr_limited_backoff_enabled_(absl::StartsWith(
key_value_config->Lookup("WebRTC-Bwe-AlrLimitedBackoff"),
"Enabled")) {
- RTC_LOG(LS_INFO) << "Initialized DelayBasedBwe with small packet filtering "
- << ignore_small_.Parser()->Encode()
- << ", separate audio overuse detection"
- << separate_audio_.Parser()->Encode()
- << " and alr limited backoff "
- << (alr_limited_backoff_enabled_ ? "enabled" : "disabled");
+ RTC_LOG(LS_INFO)
+ << "Initialized DelayBasedBwe with separate audio overuse detection"
+ << separate_audio_.Parser()->Encode() << " and alr limited backoff "
+ << (alr_limited_backoff_enabled_ ? "enabled" : "disabled");
}
DelayBasedBwe::~DelayBasedBwe() {}
@@ -193,22 +168,6 @@
}
last_seen_packet_ = at_time;
- // 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;
- }
- }
-
// As an alternative to ignoring small packets, we can separate audio and
// video packets for overuse detection.
InterArrival* inter_arrival_for_packet = video_inter_arrival_.get();
@@ -246,6 +205,7 @@
uint32_t timestamp_delta = 0;
int64_t recv_delta_ms = 0;
int size_delta = 0;
+ DataSize packet_size = packet_feedback.sent_packet.size;
bool calculated_deltas = inter_arrival_for_packet->ComputeDeltas(
timestamp, packet_feedback.receive_time.ms(), at_time.ms(),
packet_size.bytes(), ×tamp_delta, &recv_delta_ms, &size_delta);
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.h b/modules/congestion_controller/goog_cc/delay_based_bwe.h
index 85ad0dd..a87ad4a 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe.h
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe.h
@@ -31,21 +31,6 @@
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();
-};
-
struct BweSeparateAudioPacketsSettings {
static constexpr char kKey[] = "WebRTC-Bwe-SeparateAudioPackets";
@@ -64,7 +49,6 @@
public:
struct Result {
Result();
- Result(bool probe, DataRate target_bitrate);
~Result() = default;
bool updated;
bool probe;
@@ -112,19 +96,14 @@
Timestamp at_time);
// Updates the current remote rate estimate and returns true if a valid
// estimate exists.
- bool UpdateEstimate(Timestamp now,
+ bool UpdateEstimate(Timestamp at_time,
absl::optional<DataRate> acked_bitrate,
- DataRate* target_bitrate);
+ DataRate* target_rate);
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_;
-
// Alternatively, run two separate overuse detectors for audio and video,
// and fall back to the audio one if we haven't seen a video packet in a
// while.