Remove unused flag GoogCcFactoryConfig.feedback_only
Bug: webrtc:423841921
Change-Id: I2a3ac3ff4dce110295d77bf0ba69ec6e843b0c77
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/396041
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44879}
diff --git a/api/transport/goog_cc_factory.cc b/api/transport/goog_cc_factory.cc
index 11dd368..7d2c079 100644
--- a/api/transport/goog_cc_factory.cc
+++ b/api/transport/goog_cc_factory.cc
@@ -27,7 +27,6 @@
std::unique_ptr<NetworkControllerInterface>
GoogCcNetworkControllerFactory::Create(NetworkControllerConfig config) {
GoogCcConfig goog_cc_config;
- goog_cc_config.feedback_only = factory_config_.feedback_only;
if (factory_config_.network_state_estimator_factory) {
goog_cc_config.network_state_estimator =
factory_config_.network_state_estimator_factory->Create(
diff --git a/api/transport/goog_cc_factory.h b/api/transport/goog_cc_factory.h
index 5694730..fd3f6e3 100644
--- a/api/transport/goog_cc_factory.h
+++ b/api/transport/goog_cc_factory.h
@@ -24,7 +24,6 @@
std::unique_ptr<NetworkStateEstimatorFactory> network_state_estimator_factory;
NetworkStatePredictorFactoryInterface* network_state_predictor_factory =
nullptr;
- bool feedback_only = false;
};
class RTC_EXPORT GoogCcNetworkControllerFactory
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
index f318e85..e3ee7f3 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -15,7 +15,6 @@
#include <algorithm>
#include <cstdint>
#include <memory>
-#include <numeric>
#include <optional>
#include <utility>
#include <vector>
@@ -48,8 +47,6 @@
namespace webrtc {
namespace {
-// From RTCPSender video report interval.
-constexpr TimeDelta kLossUpdateInterval = TimeDelta::Millis(1000);
// Pacing-rate relative to our target send rate.
// Multiplicative factor that is applied to the target bitrate to calculate
@@ -93,7 +90,6 @@
GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config,
GoogCcConfig goog_cc_config)
: env_(config.env),
- packet_feedback_only_(goog_cc_config.feedback_only),
safe_reset_on_route_change_("Enabled"),
safe_reset_acknowledged_rate_("ack"),
use_min_allocatable_as_lower_bound_(
@@ -250,10 +246,6 @@
NetworkControlUpdate GoogCcNetworkController::OnRemoteBitrateReport(
RemoteBitrateReport msg) {
- if (packet_feedback_only_) {
- RTC_LOG(LS_ERROR) << "Received REMB for packet feedback only GoogCC";
- return NetworkControlUpdate();
- }
bandwidth_estimation_->UpdateReceiverEstimate(msg.receive_time,
msg.bandwidth);
return NetworkControlUpdate();
@@ -261,8 +253,9 @@
NetworkControlUpdate GoogCcNetworkController::OnRoundTripTimeUpdate(
RoundTripTimeUpdate msg) {
- if (packet_feedback_only_ || msg.smoothed)
+ if (msg.smoothed) {
return NetworkControlUpdate();
+ }
RTC_DCHECK(!msg.round_trip_time.IsZero());
if (delay_based_bwe_)
delay_based_bwe_->OnRttUpdate(msg.round_trip_time);
@@ -388,8 +381,6 @@
NetworkControlUpdate GoogCcNetworkController::OnTransportLossReport(
TransportLossReport msg) {
- if (packet_feedback_only_)
- return NetworkControlUpdate();
int64_t total_packets_delta =
msg.packets_received_delta + msg.packets_lost_delta;
bandwidth_estimation_->UpdatePacketsLost(
@@ -455,46 +446,9 @@
bandwidth_estimation_->UpdatePropagationRtt(report.feedback_time,
min_propagation_rtt);
}
- if (packet_feedback_only_) {
- if (!feedback_max_rtts_.empty()) {
- int64_t sum_rtt_ms =
- std::accumulate(feedback_max_rtts_.begin(), feedback_max_rtts_.end(),
- static_cast<int64_t>(0));
- int64_t mean_rtt_ms = sum_rtt_ms / feedback_max_rtts_.size();
- if (delay_based_bwe_)
- delay_based_bwe_->OnRttUpdate(TimeDelta::Millis(mean_rtt_ms));
- }
- TimeDelta feedback_min_rtt = TimeDelta::PlusInfinity();
- for (const auto& packet_feedback : feedbacks) {
- TimeDelta pending_time = max_recv_time - packet_feedback.receive_time;
- TimeDelta rtt = report.feedback_time -
- packet_feedback.sent_packet.send_time - pending_time;
- // Value used for predicting NACK round trip time in FEC controller.
- feedback_min_rtt = std::min(rtt, feedback_min_rtt);
- }
- if (feedback_min_rtt.IsFinite()) {
- bandwidth_estimation_->UpdateRtt(feedback_min_rtt, report.feedback_time);
- }
-
- expected_packets_since_last_loss_update_ +=
- report.PacketsWithFeedback().size();
- for (const auto& packet_feedback : report.PacketsWithFeedback()) {
- if (!packet_feedback.IsReceived())
- lost_packets_since_last_loss_update_ += 1;
- }
- if (report.feedback_time > next_loss_update_) {
- next_loss_update_ = report.feedback_time + kLossUpdateInterval;
- bandwidth_estimation_->UpdatePacketsLost(
- lost_packets_since_last_loss_update_,
- expected_packets_since_last_loss_update_, report.feedback_time);
- expected_packets_since_last_loss_update_ = 0;
- lost_packets_since_last_loss_update_ = 0;
- }
- }
std::optional<int64_t> alr_start_time =
alr_detector_->GetApplicationLimitedRegionStartTime();
-
if (previously_in_alr_ && !alr_start_time.has_value()) {
int64_t now_ms = report.feedback_time.ms();
acknowledged_bitrate_estimator_->SetAlrEndedTime(report.feedback_time);
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
index 46d1cab..ade7b91 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
@@ -41,7 +41,6 @@
struct GoogCcConfig {
std::unique_ptr<NetworkStateEstimator> network_state_estimator = nullptr;
std::unique_ptr<NetworkStatePredictor> network_state_predictor = nullptr;
- bool feedback_only = false;
};
class GoogCcNetworkController : public NetworkControllerInterface {
@@ -86,7 +85,6 @@
void SetNetworkStateEstimate(std::optional<NetworkStateEstimate> estimate);
const Environment env_;
- const bool packet_feedback_only_;
FieldTrialFlag safe_reset_on_route_change_;
FieldTrialFlag safe_reset_acknowledged_rate_;
const bool use_min_allocatable_as_lower_bound_;
@@ -119,10 +117,6 @@
std::optional<NetworkStateEstimate> estimate_;
- Timestamp next_loss_update_ = Timestamp::MinusInfinity();
- int lost_packets_since_last_loss_update_ = 0;
- int expected_packets_since_last_loss_update_ = 0;
-
std::deque<int64_t> feedback_max_rtts_;
DataRate last_loss_based_target_rate_;
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
index ea44c34..dbbdd4d 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
@@ -71,11 +71,6 @@
}
return dips;
}
-GoogCcNetworkControllerFactory CreateFeedbackOnlyFactory() {
- GoogCcFactoryConfig config;
- config.feedback_only = true;
- return GoogCcNetworkControllerFactory(std::move(config));
-}
const uint32_t kInitialBitrateKbps = 60;
const DataRate kInitialBitrate = DataRate::KilobitsPerSec(kInitialBitrateKbps);
@@ -158,31 +153,11 @@
return target_bitrate;
}
-// Create transport packets feedback with a built-up delay.
-TransportPacketsFeedback CreateTransportPacketsFeedback(
- TimeDelta per_packet_network_delay,
- TimeDelta one_way_delay,
- Timestamp send_time) {
- TimeDelta delay_buildup = one_way_delay;
- constexpr int kFeedbackSize = 3;
- constexpr size_t kPayloadSize = 1000;
- TransportPacketsFeedback feedback;
- for (int i = 0; i < kFeedbackSize; ++i) {
- PacketResult packet = CreatePacketResult(
- /*arrival_time=*/send_time + delay_buildup, send_time, kPayloadSize,
- PacedPacketInfo());
- delay_buildup += per_packet_network_delay;
- feedback.feedback_time = packet.receive_time + one_way_delay;
- feedback.packet_feedbacks.push_back(packet);
- }
- return feedback;
-}
-
// Scenarios:
void UpdatesTargetRateBasedOnLinkCapacity(absl::string_view test_name = "",
absl::string_view field_trials = "") {
- auto factory = CreateFeedbackOnlyFactory();
+ GoogCcNetworkControllerFactory factory;
Scenario s("googcc_unit/target_capacity" + std::string(test_name), false);
CallClientConfig config;
config.field_trials.Merge(FieldTrials(field_trials));
@@ -474,7 +449,7 @@
// Test congestion window pushback on network delay happens.
TEST(GoogCcScenario, CongestionWindowPushbackOnNetworkDelay) {
- auto factory = CreateFeedbackOnlyFactory();
+ GoogCcNetworkControllerFactory factory;
Scenario s("googcc_unit/cwnd_on_delay", false);
auto send_net =
s.CreateMutableSimulationNode([=](NetworkSimulationConfig* c) {
@@ -508,7 +483,7 @@
// Test congestion window pushback on network delay happens.
TEST(GoogCcScenario, CongestionWindowPushbackDropFrameOnNetworkDelay) {
- auto factory = CreateFeedbackOnlyFactory();
+ GoogCcNetworkControllerFactory factory;
Scenario s("googcc_unit/cwnd_on_delay", false);
auto send_net =
s.CreateMutableSimulationNode([=](NetworkSimulationConfig* c) {
@@ -616,7 +591,7 @@
}
TEST(GoogCcScenario, StableEstimateDoesNotVaryInSteadyState) {
- auto factory = CreateFeedbackOnlyFactory();
+ GoogCcNetworkControllerFactory factory;
Scenario s("googcc_unit/stable_target", false);
CallClientConfig config;
config.transport.cc_factory = &factory;
@@ -1020,45 +995,5 @@
EXPECT_LE(client->target_rate().kbps(), 300);
}
-class GoogCcRttTest : public ::testing::TestWithParam<bool> {
- protected:
- GoogCcFactoryConfig Config(bool feedback_only) {
- GoogCcFactoryConfig config;
- config.feedback_only = feedback_only;
- return config;
- }
-};
-
-TEST_P(GoogCcRttTest, CalculatesRttFromTransporFeedback) {
- GoogCcFactoryConfig config(Config(/*feedback_only=*/GetParam()));
- if (!GetParam()) {
- // TODO(diepbp): understand the usage difference between
- // UpdatePropagationRtt and UpdateRtt
- GTEST_SKIP() << "This test should run only if "
- "feedback_only is enabled";
- }
- NetworkControllerTestFixture fixture(std::move(config));
- std::unique_ptr<NetworkControllerInterface> controller =
- fixture.CreateController();
- Timestamp current_time = Timestamp::Millis(123);
- TimeDelta one_way_delay = TimeDelta::Millis(10);
- std::optional<TimeDelta> rtt = std::nullopt;
-
- TransportPacketsFeedback feedback = CreateTransportPacketsFeedback(
- /*per_packet_network_delay=*/TimeDelta::Millis(50), one_way_delay,
- /*send_time=*/current_time);
- NetworkControlUpdate update =
- controller->OnTransportPacketsFeedback(feedback);
- current_time += TimeDelta::Millis(50);
- update = controller->OnProcessInterval({.at_time = current_time});
- if (update.target_rate) {
- rtt = update.target_rate->network_estimate.round_trip_time;
- }
- ASSERT_TRUE(rtt.has_value());
- EXPECT_EQ(rtt->ms(), 2 * one_way_delay.ms());
-}
-
-INSTANTIATE_TEST_SUITE_P(GoogCcRttTests, GoogCcRttTest, ::testing::Bool());
-
} // namespace test
} // namespace webrtc
