zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Makes send time information in feedback non-optional.

Browse Source 
This makes it safer to reason about the common case where send
time information is available. We don't have to either assume that
it's available, or check it everywhere the PacketResult struct is used.

To achieve this, a new field is added to TransportPacketsFeedback
and a new interface is introduced to clearly separate which field is
used. A possible followup would be to introduce a separate struct.
That would complicate the signature of ProcessTransportFeedback.

Bug: webrtc:9934
Change-Id: I2b319e4df2b557fbd4de66b812744bca7d91ca15
Reviewed-on: https://webrtc-review.googlesource.com/c/107080
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25465}
This commit is contained in:
Sebastian Jansson
2018-11-01 12:58:25 +01:00
committed by Commit Bot
parent be837ac3bc
commit af6d741fe1
13 changed files with 100 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
api/transport/network_types.cc
View File

@ -38,7 +38,7 @@ std::vector<PacketResult> TransportPacketsFeedback::ReceivedWithSendInfo()
const {
std::vector<PacketResult> res;
for (const PacketResult& fb : packet_feedbacks) {
if (fb.receive_time.IsFinite() && fb.sent_packet.has_value()) {
if (fb.receive_time.IsFinite()) {
res.push_back(fb);
}
}
@ -48,7 +48,7 @@ std::vector<PacketResult> TransportPacketsFeedback::ReceivedWithSendInfo()
std::vector<PacketResult> TransportPacketsFeedback::LostWithSendInfo() const {
std::vector<PacketResult> res;
for (const PacketResult& fb : packet_feedbacks) {
if (fb.receive_time.IsPlusInfinity() && fb.sent_packet.has_value()) {
if (fb.receive_time.IsPlusInfinity()) {
res.push_back(fb);
}
}

13
api/transport/network_types.h
View File

@ -126,14 +126,10 @@ struct PacketResult {
PacketResult(const PacketResult&);
~PacketResult();
absl::optional<SentPacket> sent_packet;
SentPacket sent_packet;
Timestamp receive_time = Timestamp::PlusInfinity();
// TODO(bugs.webrtc.org/9934): Remove this when sent_packet is made
// non-optional.
const SentPacket& GetSentPacket() const {
RTC_DCHECK(sent_packet.has_value());
return *sent_packet;
}
// TODO(bugs.webrtc.org/9934): Remove this when downsrteam projects are ready.
const SentPacket& GetSentPacket() const { return sent_packet; }
};
struct TransportPacketsFeedback {
@ -146,6 +142,9 @@ struct TransportPacketsFeedback {
DataSize prior_in_flight = DataSize::Zero();
std::vector<PacketResult> packet_feedbacks;
// Arrival times for messages without send time information.
std::vector<Timestamp> sendless_arrival_times;
std::vector<PacketResult> ReceivedWithSendInfo() const;
std::vector<PacketResult> LostWithSendInfo() const;
std::vector<PacketResult> PacketsWithFeedback() const;

8
api/transport/test/network_control_tester.cc
View File

@ -87,7 +87,7 @@ void NetworkControllerTester::RunSimulation(TimeDelta duration,
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;
data_in_flight += packet.sent_packet.size;
if (data_in_flight > *state_.congestion_window)
send_packet = false;
}
@ -98,7 +98,7 @@ void NetworkControllerTester::RunSimulation(TimeDelta duration,
sent_packet.sequence_number = packet_sequence_number_++;
sent_packet.data_in_flight = sent_packet.size;
for (PacketResult& packet : outstanding_packets_)
sent_packet.data_in_flight += packet.sent_packet->size;
sent_packet.data_in_flight += packet.sent_packet.size;
Update(&state_, controller_->OnSentPacket(sent_packet));
TimeDelta time_in_flight = sent_packet.size / actual_bandwidth;
@ -120,7 +120,7 @@ void NetworkControllerTester::RunSimulation(TimeDelta duration,
TransportPacketsFeedback feedback;
feedback.prior_in_flight = DataSize::Zero();
for (PacketResult& packet : outstanding_packets_)
feedback.prior_in_flight += packet.sent_packet->size;
feedback.prior_in_flight += packet.sent_packet.size;
while (!outstanding_packets_.empty() &&
current_time_ >= outstanding_packets_.front().receive_time +
propagation_delay) {
@ -131,7 +131,7 @@ void NetworkControllerTester::RunSimulation(TimeDelta duration,
feedback.packet_feedbacks.back().receive_time + propagation_delay;
feedback.data_in_flight = DataSize::Zero();
for (PacketResult& packet : outstanding_packets_)
feedback.data_in_flight += packet.sent_packet->size;
feedback.data_in_flight += packet.sent_packet.size;
Update(&state_, controller_->OnTransportPacketsFeedback(feedback));
}
current_time_ += packet_interval;

25
modules/congestion_controller/bbr/bbr_network_controller.cc
View File

@ -401,16 +401,15 @@ bool BbrNetworkController::IsProbingForMoreBandwidth() const {
NetworkControlUpdate BbrNetworkController::OnTransportPacketsFeedback(
TransportPacketsFeedback msg) {
if (msg.packet_feedbacks.empty())
return NetworkControlUpdate();
Timestamp feedback_recv_time = msg.feedback_time;
absl::optional<SentPacket> last_sent_packet =
msg.PacketsWithFeedback().back().sent_packet;
if (!last_sent_packet.has_value()) {
RTC_LOG(LS_WARNING) << "Last ack packet not in history, no RTT update";
} else {
Timestamp send_time = last_sent_packet->send_time;
SentPacket last_sent_packet = msg.PacketsWithFeedback().back().sent_packet;
Timestamp send_time = last_sent_packet.send_time;
TimeDelta send_delta = feedback_recv_time - send_time;
rtt_stats_.UpdateRtt(send_delta, TimeDelta::Zero(), feedback_recv_time);
}
const DataSize total_data_acked_before = sampler_->total_data_acked();
@ -431,7 +430,7 @@ NetworkControlUpdate BbrNetworkController::OnTransportPacketsFeedback(
// Input the new data into the BBR model of the connection.
if (!acked_packets.empty()) {
int64_t last_acked_packet =
acked_packets.rbegin()->sent_packet->sequence_number;
acked_packets.rbegin()->sent_packet.sequence_number;
is_round_start = UpdateRoundTripCounter(last_acked_packet);
min_rtt_expired =
@ -472,7 +471,7 @@ NetworkControlUpdate BbrNetworkController::OnTransportPacketsFeedback(
DataSize data_acked = sampler_->total_data_acked() - total_data_acked_before;
DataSize data_lost = DataSize::Zero();
for (const PacketResult& packet : lost_packets) {
data_lost += packet.sent_packet->size;
data_lost += packet.sent_packet.size;
}
// After the model is updated, recalculate the pacing rate and congestion
@ -483,7 +482,7 @@ NetworkControlUpdate BbrNetworkController::OnTransportPacketsFeedback(
// Cleanup internal state.
if (!acked_packets.empty()) {
sampler_->RemoveObsoletePackets(
acked_packets.back().sent_packet->sequence_number);
acked_packets.back().sent_packet.sequence_number);
}
return CreateRateUpdate(msg.feedback_time);
}
@ -550,7 +549,7 @@ void BbrNetworkController::EnterProbeBandwidthMode(Timestamp now) {
void BbrNetworkController::DiscardLostPackets(
const std::vector<PacketResult>& lost_packets) {
for (const PacketResult& packet : lost_packets) {
sampler_->OnPacketLost(packet.sent_packet->sequence_number);
sampler_->OnPacketLost(packet.sent_packet.sequence_number);
}
}
@ -569,8 +568,8 @@ bool BbrNetworkController::UpdateBandwidthAndMinRtt(
const std::vector<PacketResult>& acked_packets) {
TimeDelta sample_rtt = TimeDelta::PlusInfinity();
for (const auto& packet : acked_packets) {
BandwidthSample bandwidth_sample = sampler_->OnPacketAcknowledged(
now, packet.sent_packet->sequence_number);
BandwidthSample bandwidth_sample =
sampler_->OnPacketAcknowledged(now, packet.sent_packet.sequence_number);
last_sample_is_app_limited_ = bandwidth_sample.is_app_limited;
if (!bandwidth_sample.rtt.IsZero()) {
sample_rtt = std::min(sample_rtt, bandwidth_sample.rtt);

17
modules/congestion_controller/goog_cc/delay_based_bwe.cc
View File

@ -148,11 +148,8 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
}
if (delayed_feedback) {
++consecutive_delayed_feedbacks_;
if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
consecutive_delayed_feedbacks_ = 0;
return OnLongFeedbackDelay(packet_feedback_vector.back().arrival_time_ms);
}
return OnDelayedFeedback(packet_feedback_vector.back().arrival_time_ms);
} else {
consecutive_delayed_feedbacks_ = 0;
return MaybeUpdateEstimate(acked_bitrate_bps, recovered_from_overuse,
@ -161,6 +158,16 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
return Result();
}
DelayBasedBwe::Result DelayBasedBwe::OnDelayedFeedback(
int64_t receive_time_ms) {
++consecutive_delayed_feedbacks_;
if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
consecutive_delayed_feedbacks_ = 0;
return OnLongFeedbackDelay(receive_time_ms);
}
return Result();
}
DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay(
int64_t arrival_time_ms) {
// Estimate should always be valid since a start bitrate always is set in the

1
modules/congestion_controller/goog_cc/delay_based_bwe.h
View File

@ -48,6 +48,7 @@ class DelayBasedBwe {
const std::vector<PacketFeedback>& packet_feedback_vector,
absl::optional<uint32_t> acked_bitrate_bps,
int64_t at_time_ms);
Result OnDelayedFeedback(int64_t receive_time_ms);
void OnRttUpdate(int64_t avg_rtt_ms);
bool LatestEstimate(std::vector<uint32_t>* ssrcs,
uint32_t* bitrate_bps) const;

28
modules/congestion_controller/goog_cc/goog_cc_network_control.cc
View File

@ -106,14 +106,10 @@ std::vector<PacketFeedback> ReceivedPacketsFeedbackAsRtp(
if (fb.receive_time.IsFinite()) {
PacketFeedback pf(fb.receive_time.ms(), 0);
pf.creation_time_ms = report.feedback_time.ms();
if (fb.sent_packet.has_value()) {
pf.payload_size = fb.sent_packet->size.bytes();
pf.pacing_info = fb.sent_packet->pacing_info;
pf.send_time_ms = fb.sent_packet->send_time.ms();
pf.unacknowledged_data = fb.sent_packet->prior_unacked_data.bytes();
} else {
pf.send_time_ms = PacketFeedback::kNoSendTime;
}
pf.payload_size = fb.sent_packet.size.bytes();
pf.pacing_info = fb.sent_packet.pacing_info;
pf.send_time_ms = fb.sent_packet.send_time.ms();
pf.unacknowledged_data = fb.sent_packet.prior_unacked_data.bytes();
packet_feedback_vector.push_back(pf);
}
}
@ -382,6 +378,18 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportLossReport(
NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
TransportPacketsFeedback report) {
if (report.packet_feedbacks.empty()) {
DelayBasedBwe::Result result = delay_based_bwe_->OnDelayedFeedback(
report.sendless_arrival_times.back().ms());
NetworkControlUpdate update;
if (result.updated) {
bandwidth_estimation_->UpdateDelayBasedEstimate(
report.feedback_time, DataRate::bps(result.target_bitrate_bps));
MaybeTriggerOnNetworkChanged(&update, report.feedback_time);
}
return update;
}
if (congestion_window_pushback_controller_) {
congestion_window_pushback_controller_->UpdateOutstandingData(
report.data_in_flight.bytes());
@ -396,7 +404,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
for (const auto& feedback : feedbacks) {
TimeDelta feedback_rtt =
report.feedback_time - feedback.sent_packet->send_time;
report.feedback_time - feedback.sent_packet.send_time;
TimeDelta min_pending_time = feedback.receive_time - max_recv_time;
TimeDelta propagation_rtt = feedback_rtt - min_pending_time;
max_feedback_rtt = std::max(max_feedback_rtt, feedback_rtt);
@ -423,7 +431,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
for (const auto& packet_feedback : feedbacks) {
TimeDelta pending_time = packet_feedback.receive_time - max_recv_time;
TimeDelta rtt = report.feedback_time -
packet_feedback.sent_packet->send_time - pending_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);
}

19
modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc
View File

@ -84,9 +84,9 @@ class GoogCcNetworkControllerTest : public ::testing::Test {
PacedPacketInfo pacing_info) {
PacketResult packet_result;
packet_result.sent_packet = SentPacket();
packet_result.sent_packet->send_time = Timestamp::ms(send_time_ms);
packet_result.sent_packet->size = DataSize::bytes(payload_size);
packet_result.sent_packet->pacing_info = pacing_info;
packet_result.sent_packet.send_time = Timestamp::ms(send_time_ms);
packet_result.sent_packet.size = DataSize::bytes(payload_size);
packet_result.sent_packet.pacing_info = pacing_info;
packet_result.receive_time = Timestamp::ms(arrival_time_ms);
return packet_result;
}
@ -122,7 +122,7 @@ class GoogCcNetworkControllerTest : public ::testing::Test {
CreateResult(current_time_.ms() + delay_buildup, current_time_.ms(),
kPayloadSize, PacedPacketInfo());
delay_buildup += delay;
controller_->OnSentPacket(*packet.sent_packet);
controller_->OnSentPacket(packet.sent_packet);
TransportPacketsFeedback feedback;
feedback.feedback_time = packet.receive_time;
feedback.packet_feedbacks.push_back(packet);
@ -213,16 +213,13 @@ TEST_F(GoogCcNetworkControllerTest, LongFeedbackDelays) {
packets.push_back(
CreateResult(i * 100 + 40, 2 * i * 100 + 40, 1500, kPacingInfo1));
TransportPacketsFeedback feedback;
for (PacketResult& packet : packets) {
controller_->OnSentPacket(*packet.sent_packet);
// Simulate packet timeout
packet.sent_packet = absl::nullopt;
controller_->OnSentPacket(packet.sent_packet);
feedback.sendless_arrival_times.push_back(packet.receive_time);
}
TransportPacketsFeedback feedback;
feedback.feedback_time = packets[0].receive_time;
feedback.packet_feedbacks = packets;
AdvanceTimeMilliseconds(kFeedbackTimeoutMs);
SentPacket later_packet;
later_packet.send_time = Timestamp::ms(kFeedbackTimeoutMs + i * 200 + 40);
@ -246,7 +243,7 @@ TEST_F(GoogCcNetworkControllerTest, LongFeedbackDelays) {
packets.push_back(CreateResult(140, 240, 1500, kPacingInfo1));
for (const PacketResult& packet : packets)
controller_->OnSentPacket(*packet.sent_packet);
controller_->OnSentPacket(packet.sent_packet);
TransportPacketsFeedback feedback;
feedback.feedback_time = packets[0].receive_time;

13
modules/congestion_controller/pcc/monitor_interval.cc
View File

@ -31,25 +31,24 @@ PccMonitorInterval::PccMonitorInterval(const PccMonitorInterval& other) =
void PccMonitorInterval::OnPacketsFeedback(
const std::vector<PacketResult>& packets_results) {
for (const PacketResult& packet_result : packets_results) {
if (!packet_result.sent_packet.has_value() ||
packet_result.sent_packet->send_time <= start_time_) {
if (packet_result.sent_packet.send_time <= start_time_) {
continue;
}
// Here we assume that if some packets are reordered with packets sent
// after the end of the monitor interval, then they are lost. (Otherwise
// it is not clear how long should we wait for packets feedback to arrive).
if (packet_result.sent_packet->send_time >
if (packet_result.sent_packet.send_time >
start_time_ + interval_duration_) {
feedback_collection_done_ = true;
return;
}
if (packet_result.receive_time.IsInfinite()) {
lost_packets_sent_time_.push_back(packet_result.sent_packet->send_time);
lost_packets_sent_time_.push_back(packet_result.sent_packet.send_time);
} else {
received_packets_.push_back(
{packet_result.receive_time - packet_result.sent_packet->send_time,
packet_result.sent_packet->send_time});
received_packets_size_ += packet_result.sent_packet->size;
{packet_result.receive_time - packet_result.sent_packet.send_time,
packet_result.sent_packet.send_time});
received_packets_size_ += packet_result.sent_packet.size;
}
}
}

6
modules/congestion_controller/pcc/pcc_network_controller.cc
View File

@ -148,7 +148,7 @@ NetworkControlUpdate PccNetworkController::OnSentPacket(SentPacket msg) {
if (IsTimeoutExpired(msg.send_time)) {
DataSize received_size = DataSize::Zero();
for (size_t i = 1; i < last_received_packets_.size(); ++i) {
received_size += last_received_packets_[i].sent_packet->size;
received_size += last_received_packets_[i].sent_packet.size;
}
TimeDelta sending_time = TimeDelta::Zero();
if (last_received_packets_.size() > 0)
@ -166,7 +166,7 @@ NetworkControlUpdate PccNetworkController::OnSentPacket(SentPacket msg) {
msg.send_time - start_time_ >= kStartupDuration) {
DataSize received_size = DataSize::Zero();
for (size_t i = 1; i < last_received_packets_.size(); ++i) {
received_size += last_received_packets_[i].sent_packet->size;
received_size += last_received_packets_[i].sent_packet.size;
}
TimeDelta sending_time = TimeDelta::Zero();
if (last_received_packets_.size() > 0)
@ -260,6 +260,8 @@ bool PccNetworkController::IsFeedbackCollectionDone() const {
NetworkControlUpdate PccNetworkController::OnTransportPacketsFeedback(
TransportPacketsFeedback msg) {
if (msg.packet_feedbacks.empty())
return NetworkControlUpdate();
// Save packets to last_received_packets_ array.
for (const PacketResult& packet_result : msg.ReceivedWithSendInfo()) {
last_received_packets_.push_back(packet_result);

5
modules/congestion_controller/pcc/rtt_tracker.cc
View File

@ -23,12 +23,11 @@ void RttTracker::OnPacketsFeedback(
Timestamp feedback_received_time) {
TimeDelta packet_rtt = TimeDelta::MinusInfinity();
for (const PacketResult& packet_result : packet_feedbacks) {
if (!packet_result.sent_packet.has_value() ||
packet_result.receive_time.IsInfinite())
if (packet_result.receive_time.IsInfinite())
continue;
packet_rtt = std::max<TimeDelta>(
packet_rtt,
feedback_received_time - packet_result.sent_packet->send_time);
feedback_received_time - packet_result.sent_packet.send_time);
}
if (packet_rtt.IsFinite())
rtt_estimate_ = (1 - alpha_) * rtt_estimate_ + alpha_ * packet_rtt;

44
modules/congestion_controller/rtp/transport_feedback_adapter.cc
View File

@ -27,35 +27,19 @@ void SortPacketFeedbackVector(std::vector<webrtc::PacketFeedback>* input) {
PacketResult NetworkPacketFeedbackFromRtpPacketFeedback(
const webrtc::PacketFeedback& pf) {
PacketResult feedback;
if (pf.arrival_time_ms == webrtc::PacketFeedback::kNotReceived)
if (pf.arrival_time_ms == webrtc::PacketFeedback::kNotReceived) {
feedback.receive_time = Timestamp::PlusInfinity();
else
} else {
feedback.receive_time = Timestamp::ms(pf.arrival_time_ms);
if (pf.send_time_ms != webrtc::PacketFeedback::kNoSendTime) {
feedback.sent_packet = SentPacket();
feedback.sent_packet->sequence_number = pf.long_sequence_number;
feedback.sent_packet->send_time = Timestamp::ms(pf.send_time_ms);
feedback.sent_packet->size = DataSize::bytes(pf.payload_size);
feedback.sent_packet->pacing_info = pf.pacing_info;
feedback.sent_packet->prior_unacked_data =
}
feedback.sent_packet.sequence_number = pf.long_sequence_number;
feedback.sent_packet.send_time = Timestamp::ms(pf.send_time_ms);
feedback.sent_packet.size = DataSize::bytes(pf.payload_size);
feedback.sent_packet.pacing_info = pf.pacing_info;
feedback.sent_packet.prior_unacked_data =
DataSize::bytes(pf.unacknowledged_data);
}
return feedback;
}
std::vector<PacketResult> PacketResultsFromRtpFeedbackVector(
const std::vector<PacketFeedback>& feedback_vector) {
RTC_DCHECK(std::is_sorted(feedback_vector.begin(), feedback_vector.end(),
PacketFeedbackComparator()));
std::vector<PacketResult> packet_feedbacks;
packet_feedbacks.reserve(feedback_vector.size());
for (const PacketFeedback& rtp_feedback : feedback_vector) {
auto feedback = NetworkPacketFeedbackFromRtpPacketFeedback(rtp_feedback);
packet_feedbacks.push_back(feedback);
}
return packet_feedbacks;
}
} // namespace
const int64_t kNoTimestamp = -1;
const int64_t kSendTimeHistoryWindowMs = 60000;
@ -151,7 +135,17 @@ TransportFeedbackAdapter::ProcessTransportFeedback(
SortPacketFeedbackVector(&feedback_vector);
TransportPacketsFeedback msg;
msg.packet_feedbacks = PacketResultsFromRtpFeedbackVector(feedback_vector);
for (const PacketFeedback& rtp_feedback : feedback_vector) {
if (rtp_feedback.send_time_ms != PacketFeedback::kNoSendTime) {
auto feedback = NetworkPacketFeedbackFromRtpPacketFeedback(rtp_feedback);
msg.packet_feedbacks.push_back(feedback);
} else if (rtp_feedback.arrival_time_ms == PacketFeedback::kNotReceived) {
msg.sendless_arrival_times.push_back(Timestamp::PlusInfinity());
} else {
msg.sendless_arrival_times.push_back(
Timestamp::ms(rtp_feedback.arrival_time_ms));
}
}
msg.feedback_time = Timestamp::ms(feedback_time_ms);
msg.prior_in_flight = prior_in_flight;
msg.data_in_flight = GetOutstandingData();

8
test/scenario/simulated_time.cc
View File

@ -144,7 +144,7 @@ TransportPacketsFeedback SimulatedSender::PullFeedbackReport(
}
}
data_in_flight_ -= feedback.sent_packet->size;
data_in_flight_ -= feedback.sent_packet.size;
report.packet_feedbacks.push_back(feedback);
}
}
@ -284,9 +284,9 @@ bool SimulatedTimeClient::TryDeliverPacket(rtc::CopyOnWriteBuffer raw_buffer,
for (PacketResult& feedback : report.packet_feedbacks) {
if (packet_log_)
fprintf(packet_log_, "%" PRId64 " %" PRId64 " %.3lf %.3lf %.3lf\n",
feedback.sent_packet->sequence_number,
feedback.sent_packet->size.bytes(),
feedback.sent_packet->send_time.seconds<double>(),
feedback.sent_packet.sequence_number,
feedback.sent_packet.size.bytes(),
feedback.sent_packet.send_time.seconds<double>(),
feedback.receive_time.seconds<double>(),
at_time.seconds<double>());
}