Removes usage of uninitialized units in network control code.

This removes places where the units types are implicitly left
uninitialized in network_types.h and adds rtc::Optional where needed.

Also removing the change indicator in the NetworkEstimate struct as it
is not used in practice.

Bug: webrtc:9155
Change-Id: I7e30e338effba96bd466ae91e380e6a8e90f66e1
Reviewed-on: https://webrtc-review.googlesource.com/73369
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23126}
This commit is contained in:
Sebastian Jansson 2018-05-04 17:07:16 +02:00 committed by Commit Bot
parent c6ff757b24
commit ec6e550a8f
13 changed files with 110 additions and 106 deletions

View File

@ -214,8 +214,10 @@ NetworkControlUpdate BbrNetworkController::CreateRateUpdate(Timestamp at_time) {
target_rate = std::min(target_rate, pacing_rate);
if (constraints_) {
target_rate = std::min(target_rate, constraints_->max_data_rate);
target_rate = std::max(target_rate, constraints_->min_data_rate);
if (constraints_->max_data_rate)
target_rate = std::min(target_rate, *constraints_->max_data_rate);
if (constraints_->min_data_rate)
target_rate = std::max(target_rate, *constraints_->min_data_rate);
}
bool probing_for_bandwidth = IsProbingForMoreBandwidth();
if (last_update_state_.mode == mode_ &&
@ -260,9 +262,7 @@ NetworkControlUpdate BbrNetworkController::CreateRateUpdate(Timestamp at_time) {
pacer_config.at_time = at_time;
update.pacer_config = pacer_config;
CongestionWindow congestion_window;
congestion_window.data_window = GetCongestionWindow();
update.congestion_window = congestion_window;
update.congestion_window = GetCongestionWindow();
return update;
}
@ -277,8 +277,9 @@ NetworkControlUpdate BbrNetworkController::OnNetworkRouteChange(
NetworkRouteChange msg) {
constraints_ = msg.constraints;
Reset();
if (msg.starting_rate.IsFinite())
default_bandwidth_ = msg.starting_rate;
if (msg.starting_rate)
default_bandwidth_ = *msg.starting_rate;
rtt_stats_.OnConnectionMigration();
return CreateRateUpdate(msg.at_time);
}

View File

@ -92,9 +92,7 @@ TEST_F(BbrNetworkControllerTest, SendsConfigurationOnFirstProcess) {
EXPECT_THAT(*update.target_rate, TargetRateCloseTo(kInitialBitrate));
EXPECT_THAT(*update.pacer_config,
Property(&PacerConfig::data_rate, Ge(kInitialBitrate)));
EXPECT_THAT(*update.congestion_window,
Field(&CongestionWindow::data_window,
Property(&DataSize::IsFinite, true)));
EXPECT_THAT(*update.congestion_window, Property(&DataSize::IsFinite, true));
}
TEST_F(BbrNetworkControllerTest, SendsConfigurationOnNetworkRouteChanged) {

View File

@ -117,6 +117,7 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log,
pacing_factor_(kDefaultPaceMultiplier),
min_pacing_rate_(DataRate::Zero()),
max_padding_rate_(DataRate::Zero()),
max_total_allocated_bitrate_(DataRate::Zero()),
in_cwnd_experiment_(CwndExperimentEnabled()),
accepted_queue_ms_(kDefaultAcceptedQueueMs) {
delay_based_bwe_->SetMinBitrate(congestion_controller::GetMinBitrateBps());
@ -140,9 +141,9 @@ NetworkControlUpdate GoogCcNetworkController::OnNetworkAvailability(
NetworkControlUpdate GoogCcNetworkController::OnNetworkRouteChange(
NetworkRouteChange msg) {
int64_t min_bitrate_bps = msg.constraints.min_data_rate.bps_or(-1);
int64_t max_bitrate_bps = msg.constraints.max_data_rate.bps_or(-1);
int64_t start_bitrate_bps = msg.starting_rate.bps_or(-1);
int64_t min_bitrate_bps = GetBpsOrDefault(msg.constraints.min_data_rate, -1);
int64_t max_bitrate_bps = GetBpsOrDefault(msg.constraints.max_data_rate, -1);
int64_t start_bitrate_bps = GetBpsOrDefault(msg.starting_rate, -1);
ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
@ -234,17 +235,16 @@ NetworkControlUpdate GoogCcNetworkController::OnStreamsConfig(
NetworkControlUpdate GoogCcNetworkController::OnTargetRateConstraints(
TargetRateConstraints constraints) {
NetworkControlUpdate update;
UpdateBitrateConstraints(constraints, DataRate());
UpdateBitrateConstraints(constraints, rtc::nullopt);
return MaybeTriggerOnNetworkChanged(constraints.at_time);
}
void GoogCcNetworkController::UpdateBitrateConstraints(
TargetRateConstraints constraints,
DataRate starting_rate) {
int64_t min_bitrate_bps = constraints.min_data_rate.bps_or(0);
int64_t max_bitrate_bps = constraints.max_data_rate.bps_or(-1);
int64_t start_bitrate_bps = starting_rate.bps_or(-1);
rtc::Optional<DataRate> starting_rate) {
int64_t min_bitrate_bps = GetBpsOrDefault(constraints.min_data_rate, 0);
int64_t max_bitrate_bps = GetBpsOrDefault(constraints.max_data_rate, -1);
int64_t start_bitrate_bps = GetBpsOrDefault(starting_rate, -1);
ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
@ -327,8 +327,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
return update;
}
rtc::Optional<CongestionWindow>
GoogCcNetworkController::MaybeUpdateCongestionWindow() {
rtc::Optional<DataSize> GoogCcNetworkController::MaybeUpdateCongestionWindow() {
if (!in_cwnd_experiment_)
return rtc::nullopt;
// No valid RTT. Could be because send-side BWE isn't used, in which case
@ -340,12 +339,10 @@ GoogCcNetworkController::MaybeUpdateCongestionWindow() {
TimeDelta time_window =
TimeDelta::ms(*min_feedback_rtt_ms_ + accepted_queue_ms_);
DataSize data_window = last_bandwidth_ * time_window;
CongestionWindow msg;
msg.enabled = true;
msg.data_window = std::max(kMinCwnd, data_window);
data_window = std::max(kMinCwnd, data_window);
RTC_LOG(LS_INFO) << "Feedback rtt: " << *min_feedback_rtt_ms_
<< " Bitrate: " << last_bandwidth_.bps();
return msg;
return data_window;
}
NetworkControlUpdate GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
@ -366,7 +363,6 @@ NetworkControlUpdate GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
new_estimate.bandwidth = DataRate::bps(estimated_bitrate_bps);
new_estimate.loss_rate_ratio = fraction_loss / 255.0f;
new_estimate.bwe_period = bwe_period;
new_estimate.changed = true;
last_bandwidth_ = new_estimate.bandwidth;
return OnNetworkEstimate(new_estimate);
}
@ -405,9 +401,6 @@ bool GoogCcNetworkController::GetNetworkParameters(
NetworkControlUpdate GoogCcNetworkController::OnNetworkEstimate(
NetworkEstimate estimate) {
NetworkControlUpdate update;
if (!estimate.changed)
return update;
update.pacer_config = UpdatePacingRates(estimate.at_time);
alr_detector_->SetEstimatedBitrate(estimate.bandwidth.bps());
probe_controller_->SetEstimatedBitrate(estimate.bandwidth.bps(),

View File

@ -51,8 +51,8 @@ class GoogCcNetworkController : public NetworkControllerInterface {
private:
void UpdateBitrateConstraints(TargetRateConstraints constraints,
DataRate starting_rate);
rtc::Optional<CongestionWindow> MaybeUpdateCongestionWindow();
rtc::Optional<DataRate> starting_rate);
rtc::Optional<DataSize> MaybeUpdateCongestionWindow();
NetworkControlUpdate MaybeTriggerOnNetworkChanged(Timestamp at_time);
bool GetNetworkParameters(int32_t* estimated_bitrate_bps,
uint8_t* fraction_loss,

View File

@ -37,7 +37,7 @@ struct NetworkControllerConfig {
// The initial bandwidth estimate to base target rate on. This should be used
// as the basis for initial OnTargetTransferRate and OnPacerConfig callbacks.
// Note that starting rate is only provided on construction.
DataRate starting_bandwidth;
DataRate starting_bandwidth = DataRate::Infinity();
};
// NetworkControllerInterface is implemented by network controllers. A network

View File

@ -32,7 +32,7 @@ struct StreamsConfig {
StreamsConfig();
StreamsConfig(const StreamsConfig&);
~StreamsConfig();
Timestamp at_time;
Timestamp at_time = Timestamp::Infinity();
bool requests_alr_probing = false;
rtc::Optional<double> pacing_factor;
rtc::Optional<DataRate> min_pacing_rate;
@ -41,62 +41,59 @@ struct StreamsConfig {
};
struct TargetRateConstraints {
Timestamp at_time;
DataRate min_data_rate;
DataRate max_data_rate;
TargetRateConstraints();
TargetRateConstraints(const TargetRateConstraints&);
~TargetRateConstraints();
Timestamp at_time = Timestamp::Infinity();
rtc::Optional<DataRate> min_data_rate;
rtc::Optional<DataRate> max_data_rate;
};
// Send side information
struct NetworkAvailability {
Timestamp at_time;
Timestamp at_time = Timestamp::Infinity();
bool network_available = false;
};
struct NetworkRouteChange {
Timestamp at_time;
NetworkRouteChange();
NetworkRouteChange(const NetworkRouteChange&);
~NetworkRouteChange();
Timestamp at_time = Timestamp::Infinity();
// The TargetRateConstraints are set here so they can be changed synchronously
// when network route changes.
TargetRateConstraints constraints;
DataRate starting_rate;
rtc::Optional<DataRate> starting_rate;
};
struct SentPacket {
Timestamp send_time;
DataSize size;
Timestamp send_time = Timestamp::Infinity();
DataSize size = DataSize::Zero();
PacedPacketInfo pacing_info;
};
struct PacerQueueUpdate {
Timestamp at_time;
TimeDelta expected_queue_time;
};
// Transport level feedback
struct RemoteBitrateReport {
Timestamp receive_time;
DataRate bandwidth;
Timestamp receive_time = Timestamp::Infinity();
DataRate bandwidth = DataRate::Infinity();
};
struct RoundTripTimeUpdate {
Timestamp receive_time;
TimeDelta round_trip_time;
Timestamp receive_time = Timestamp::Infinity();
TimeDelta round_trip_time = TimeDelta::PlusInfinity();
bool smoothed = false;
};
struct TransportLossReport {
Timestamp receive_time;
Timestamp start_time;
Timestamp end_time;
Timestamp receive_time = Timestamp::Infinity();
Timestamp start_time = Timestamp::Infinity();
Timestamp end_time = Timestamp::Infinity();
uint64_t packets_lost_delta = 0;
uint64_t packets_received_delta = 0;
};
struct OutstandingData {
DataSize in_flight_data;
};
// Packet level feedback
struct PacketResult {
@ -105,7 +102,7 @@ struct PacketResult {
~PacketResult();
rtc::Optional<SentPacket> sent_packet;
Timestamp receive_time;
Timestamp receive_time = Timestamp::Infinity();
};
struct TransportPacketsFeedback {
@ -113,9 +110,9 @@ struct TransportPacketsFeedback {
TransportPacketsFeedback(const TransportPacketsFeedback& other);
~TransportPacketsFeedback();
Timestamp feedback_time;
DataSize data_in_flight;
DataSize prior_in_flight;
Timestamp feedback_time = Timestamp::Infinity();
DataSize data_in_flight = DataSize::Zero();
DataSize prior_in_flight = DataSize::Zero();
std::vector<PacketResult> packet_feedbacks;
std::vector<PacketResult> ReceivedWithSendInfo() const;
@ -126,44 +123,39 @@ struct TransportPacketsFeedback {
// Network estimation
struct NetworkEstimate {
Timestamp at_time;
DataRate bandwidth;
TimeDelta round_trip_time;
TimeDelta bwe_period;
Timestamp at_time = Timestamp::Infinity();
DataRate bandwidth = DataRate::Infinity();
TimeDelta round_trip_time = TimeDelta::PlusInfinity();
TimeDelta bwe_period = TimeDelta::PlusInfinity();
float loss_rate_ratio = 0;
bool changed = true;
};
// Network control
struct CongestionWindow {
bool enabled = true;
DataSize data_window;
};
struct PacerConfig {
Timestamp at_time;
Timestamp at_time = Timestamp::Infinity();
// Pacer should send at most data_window data over time_window duration.
DataSize data_window;
TimeDelta time_window;
DataSize data_window = DataSize::Infinity();
TimeDelta time_window = TimeDelta::PlusInfinity();
// Pacer should send at least pad_window data over time_window duration.
DataSize pad_window;
DataSize pad_window = DataSize::Zero();
DataRate data_rate() const { return data_window / time_window; }
DataRate pad_rate() const { return pad_window / time_window; }
};
struct ProbeClusterConfig {
Timestamp at_time;
DataRate target_data_rate;
TimeDelta target_duration;
uint32_t target_probe_count;
Timestamp at_time = Timestamp::Infinity();
DataRate target_data_rate = DataRate::Zero();
TimeDelta target_duration = TimeDelta::Zero();
int32_t target_probe_count = 0;
};
struct TargetTransferRate {
Timestamp at_time;
DataRate target_rate;
Timestamp at_time = Timestamp::Infinity();
// The estimate on which the target rate is based on.
NetworkEstimate network_estimate;
DataRate target_rate = DataRate::Zero();
};
// Contains updates of network controller comand state. Using optionals to
@ -173,7 +165,7 @@ struct NetworkControlUpdate {
NetworkControlUpdate();
NetworkControlUpdate(const NetworkControlUpdate&);
~NetworkControlUpdate();
rtc::Optional<CongestionWindow> congestion_window;
rtc::Optional<DataSize> congestion_window;
rtc::Optional<PacerConfig> pacer_config;
std::vector<ProbeClusterConfig> probe_cluster_configs;
rtc::Optional<TargetTransferRate> target_rate;
@ -181,7 +173,7 @@ struct NetworkControlUpdate {
// Process control
struct ProcessInterval {
Timestamp at_time;
Timestamp at_time = Timestamp::Infinity();
};
} // namespace webrtc

View File

@ -16,6 +16,15 @@ StreamsConfig::StreamsConfig() = default;
StreamsConfig::StreamsConfig(const StreamsConfig&) = default;
StreamsConfig::~StreamsConfig() = default;
TargetRateConstraints::TargetRateConstraints() = default;
TargetRateConstraints::TargetRateConstraints(const TargetRateConstraints&) =
default;
TargetRateConstraints::~TargetRateConstraints() = default;
NetworkRouteChange::NetworkRouteChange() = default;
NetworkRouteChange::NetworkRouteChange(const NetworkRouteChange&) = default;
NetworkRouteChange::~NetworkRouteChange() = default;
PacketResult::PacketResult() = default;
PacketResult::PacketResult(const PacketResult& other) = default;
PacketResult::~PacketResult() = default;

View File

@ -21,7 +21,7 @@ namespace {
void Update(NetworkControlUpdate* target, const NetworkControlUpdate& update) {
if (update.congestion_window) {
RTC_LOG(LS_INFO) << "Received window="
<< ToString(update.congestion_window->data_window) << "\n";
<< ToString(*update.congestion_window) << "\n";
target->congestion_window = update.congestion_window;
}
if (update.pacer_config) {
@ -98,11 +98,11 @@ void NetworkControllerTester::RunSimulation(TimeDelta duration,
Timestamp last_process_time = current_time_;
while (current_time_ - start_time < duration) {
bool send_packet = true;
if (state_.congestion_window && state_.congestion_window->enabled) {
if (state_.congestion_window && state_.congestion_window->IsFinite()) {
DataSize data_in_flight = DataSize::Zero();
for (PacketResult& packet : outstanding_packets_)
data_in_flight += packet.sent_packet->size;
if (data_in_flight > state_.congestion_window->data_window)
if (data_in_flight > *state_.congestion_window)
send_packet = false;
}

View File

@ -64,7 +64,7 @@ class NetworkControllerTester {
TimeDelta propagation_delay,
DataRate actual_bandwidth);
std::unique_ptr<NetworkControllerInterface> controller_;
TimeDelta process_interval_;
TimeDelta process_interval_ = TimeDelta::PlusInfinity();
Timestamp current_time_;
TimeDelta accumulated_delay_;
std::deque<PacketResult> outstanding_packets_;

View File

@ -16,6 +16,7 @@
#include <limits>
#include <string>
#include "api/optional.h"
#include "rtc_base/checks.h"
#include "modules/congestion_controller/network_control/units/data_size.h"
@ -134,6 +135,15 @@ inline DataSize operator*(const TimeDelta& duration, const DataRate& rate) {
return rate * duration;
}
inline int64_t GetBpsOrDefault(const rtc::Optional<DataRate>& rate,
int64_t fallback_bps) {
if (rate && rate->IsFinite()) {
return rate->bps();
} else {
return fallback_bps;
}
}
std::string ToString(const DataRate& value);
} // namespace webrtc

View File

@ -22,10 +22,10 @@ PacerController::PacerController(PacedSender* pacer) : pacer_(pacer) {
PacerController::~PacerController() = default;
void PacerController::OnCongestionWindow(CongestionWindow congestion_window) {
void PacerController::OnCongestionWindow(DataSize congestion_window) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_);
if (congestion_window.enabled)
pacer_->SetCongestionWindow(congestion_window.data_window.bytes());
if (congestion_window.IsFinite())
pacer_->SetCongestionWindow(congestion_window.bytes());
else
pacer_->SetCongestionWindow(PacedSender::kNoCongestionWindow);
}
@ -55,9 +55,9 @@ void PacerController::OnProbeClusterConfig(ProbeClusterConfig config) {
pacer_->CreateProbeCluster(bitrate_bps);
}
void PacerController::OnOutstandingData(OutstandingData msg) {
void PacerController::OnOutstandingData(DataSize in_flight_data) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_);
pacer_->UpdateOutstandingData(msg.in_flight_data.bytes());
pacer_->UpdateOutstandingData(in_flight_data.bytes());
}
void PacerController::SetPacerState(bool paused) {

View File

@ -30,10 +30,10 @@ class PacerController {
public:
explicit PacerController(PacedSender* pacer);
~PacerController();
void OnCongestionWindow(CongestionWindow msg);
void OnCongestionWindow(DataSize msg);
void OnNetworkAvailability(NetworkAvailability msg);
void OnNetworkRouteChange(NetworkRouteChange msg);
void OnOutstandingData(OutstandingData msg);
void OnOutstandingData(DataSize in_flight_data);
void OnPacerConfig(PacerConfig msg);
void OnProbeClusterConfig(ProbeClusterConfig msg);

View File

@ -166,7 +166,7 @@ class ControlHandler {
void PostUpdates(NetworkControlUpdate update);
void OnNetworkAvailability(NetworkAvailability msg);
void OnPacerQueueUpdate(PacerQueueUpdate msg);
void OnPacerQueueUpdate(TimeDelta expected_queue_time);
rtc::Optional<TargetTransferRate> last_transfer_rate();
@ -227,9 +227,9 @@ void ControlHandler::OnNetworkAvailability(NetworkAvailability msg) {
OnNetworkInvalidation();
}
void ControlHandler::OnPacerQueueUpdate(PacerQueueUpdate msg) {
void ControlHandler::OnPacerQueueUpdate(TimeDelta expected_queue_time) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_);
pacer_expected_queue_ms_ = msg.expected_queue_time.ms();
pacer_expected_queue_ms_ = expected_queue_time.ms();
OnNetworkInvalidation();
}
@ -318,6 +318,7 @@ SendSideCongestionController::SendSideCongestionController(
rtc::MakeUnique<GoogCcNetworkControllerFactory>(event_log)),
pacer_controller_(MakeUnique<PacerController>(pacer_)),
process_interval_(controller_factory_fallback_->GetProcessInterval()),
last_report_block_time_(Timestamp::ms(clock_->TimeInMilliseconds())),
observer_(nullptr),
send_side_bwe_with_overhead_(
webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
@ -456,14 +457,15 @@ void SendSideCongestionController::OnNetworkRouteChanged(
msg.at_time = Timestamp::ms(clock_->TimeInMilliseconds());
msg.constraints =
ConvertConstraints(min_bitrate_bps, max_bitrate_bps, clock_);
msg.starting_rate =
start_bitrate_bps > 0 ? DataRate::bps(start_bitrate_bps) : DataRate();
if (start_bitrate_bps > 0)
msg.starting_rate = DataRate::bps(start_bitrate_bps);
task_queue_->PostTask([this, msg]() {
RTC_DCHECK_RUN_ON(task_queue_ptr_);
if (controller_) {
control_handler_->PostUpdates(controller_->OnNetworkRouteChange(msg));
} else {
initial_config_.starting_bandwidth = msg.starting_rate;
if (msg.starting_rate)
initial_config_.starting_bandwidth = *msg.starting_rate;
initial_config_.constraints = msg.constraints;
}
pacer_controller_->OnNetworkRouteChange(msg);
@ -626,9 +628,9 @@ void SendSideCongestionController::UpdateControllerWithTimeInterval() {
void SendSideCongestionController::UpdatePacerQueue() {
if (control_handler_) {
PacerQueueUpdate msg;
msg.expected_queue_time = TimeDelta::ms(pacer_->ExpectedQueueTimeMs());
control_handler_->OnPacerQueueUpdate(msg);
TimeDelta expected_queue_time =
TimeDelta::ms(pacer_->ExpectedQueueTimeMs());
control_handler_->OnPacerQueueUpdate(expected_queue_time);
}
}
@ -675,12 +677,11 @@ void SendSideCongestionController::OnTransportFeedback(
}
void SendSideCongestionController::MaybeUpdateOutstandingData() {
OutstandingData msg;
msg.in_flight_data =
DataSize in_flight_data =
DataSize::bytes(transport_feedback_adapter_.GetOutstandingBytes());
task_queue_->PostTask([this, msg]() {
task_queue_->PostTask([this, in_flight_data]() {
RTC_DCHECK_RUN_ON(task_queue_ptr_);
pacer_controller_->OnOutstandingData(msg);
pacer_controller_->OnOutstandingData(in_flight_data);
});
}