admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ignore packets sent on old network route when receiving feedback.

Browse Source 
BUG=webrtc:7347
R=philipel@webrtc.org

Review-Url: https://codereview.webrtc.org/2755553003 .
Cr-Commit-Position: refs/heads/master@{#17243}
This commit is contained in:
Stefan Holmer 2017-03-15 12:40:25 +01:00
parent 83722268d6
commit 9ea46b5286
12 changed files with 109 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
webrtc/call/call.cc
View File

@ -970,8 +970,8 @@ void Call::OnNetworkRouteChanged(const std::string& transport_name,
<< " bps, max: " << config_.bitrate_config.start_bitrate_bps
<< " bps.";
RTC_DCHECK_GT(config_.bitrate_config.start_bitrate_bps, 0);
congestion_controller_->ResetBweAndBitrates(
config_.bitrate_config.start_bitrate_bps,
congestion_controller_->OnNetworkRouteChanged(
network_route, config_.bitrate_config.start_bitrate_bps,
config_.bitrate_config.min_bitrate_bps,
config_.bitrate_config.max_bitrate_bps);
}

13
webrtc/modules/congestion_controller/congestion_controller.cc
View File

@ -221,9 +221,13 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps,
MaybeTriggerOnNetworkChanged();
}
void CongestionController::ResetBweAndBitrates(int bitrate_bps,
int min_bitrate_bps,
int max_bitrate_bps) {
// TODO(holmer): Split this up and use SetBweBitrates in combination with
// OnNetworkRouteChanged.
void CongestionController::OnNetworkRouteChanged(
const rtc::NetworkRoute& network_route,
int bitrate_bps,
int min_bitrate_bps,
int max_bitrate_bps) {
ClampBitrates(&bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
// TODO(honghaiz): Recreate this object once the bitrate controller is
// no longer exposed outside CongestionController.
@ -235,7 +239,8 @@ void CongestionController::ResetBweAndBitrates(int bitrate_bps,
// no longer exposed outside CongestionController.
remote_bitrate_estimator_.SetMinBitrate(min_bitrate_bps);
transport_feedback_adapter_.ClearSendTimeHistory();
transport_feedback_adapter_.SetNetworkIds(network_route.local_network_id,
network_route.remote_network_id);
{
rtc::CritScope cs(&bwe_lock_);
delay_based_bwe_.reset(new DelayBasedBwe(event_log_, clock_));

17
webrtc/modules/congestion_controller/congestion_controller_unittest.cc
View File

@ -215,12 +215,14 @@ TEST_F(CongestionControllerTest, SignalNetworkState) {
controller_->SignalNetworkState(kNetworkDown);
}
TEST_F(CongestionControllerTest, ResetBweAndBitrates) {
TEST_F(CongestionControllerTest, OnNetworkRouteChanged) {
int new_bitrate = 200000;
testing::Mock::VerifyAndClearExpectations(pacer_);
EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _, _));
EXPECT_CALL(*pacer_, SetEstimatedBitrate(new_bitrate));
controller_->ResetBweAndBitrates(new_bitrate, -1, -1);
rtc::NetworkRoute route;
route.local_network_id = 1;
controller_->OnNetworkRouteChanged(route, new_bitrate, -1, -1);
// If the bitrate is reset to -1, the new starting bitrate will be
// the minimum default bitrate kMinBitrateBps.
@ -229,7 +231,8 @@ TEST_F(CongestionControllerTest, ResetBweAndBitrates) {
OnNetworkChanged(congestion_controller::GetMinBitrateBps(), _, _, _));
EXPECT_CALL(*pacer_,
SetEstimatedBitrate(congestion_controller::GetMinBitrateBps()));
controller_->ResetBweAndBitrates(-1, -1, -1);
route.local_network_id = 2;
controller_->OnNetworkRouteChanged(route, -1, -1, -1);
}
TEST_F(CongestionControllerTest,
@ -316,13 +319,15 @@ TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) {
EXPECT_EQ(header.ssrc, ssrcs[0]);
}
TEST_F(CongestionControllerTest, ProbeOnBweReset) {
TEST_F(CongestionControllerTest, ProbeOnRouteChange) {
testing::Mock::VerifyAndClearExpectations(pacer_);
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 6));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 12));
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _));
controller_->ResetBweAndBitrates(2 * kInitialBitrateBps, 0,
20 * kInitialBitrateBps);
rtc::NetworkRoute route;
route.local_network_id = 1;
controller_->OnNetworkRouteChanged(route, 2 * kInitialBitrateBps, 0,
20 * kInitialBitrateBps);
}
// Estimated bitrate reduced when the feedbacks arrive with such a long delay,

8
webrtc/modules/congestion_controller/include/congestion_controller.h
View File

@ -16,6 +16,7 @@
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/networkroute.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/common_types.h"
#include "webrtc/modules/congestion_controller/delay_based_bwe.h"
@ -81,9 +82,10 @@ class CongestionController : public CallStatsObserver,
int max_bitrate_bps);
// Resets both the BWE state and the bitrate estimator. Note the first
// argument is the bitrate_bps.
virtual void ResetBweAndBitrates(int bitrate_bps,
int min_bitrate_bps,
int max_bitrate_bps);
virtual void OnNetworkRouteChanged(const rtc::NetworkRoute& network_route,
int bitrate_bps,
int min_bitrate_bps,
int max_bitrate_bps);
virtual void SignalNetworkState(NetworkState state);
virtual void SetTransportOverhead(size_t transport_overhead_bytes_per_packet);

25
webrtc/modules/congestion_controller/transport_feedback_adapter.cc
View File

@ -32,7 +32,9 @@ TransportFeedbackAdapter::TransportFeedbackAdapter(const Clock* clock)
send_time_history_(clock, kSendTimeHistoryWindowMs),
clock_(clock),
current_offset_ms_(kNoTimestamp),
last_timestamp_us_(kNoTimestamp) {}
last_timestamp_us_(kNoTimestamp),
local_net_id_(0),
remote_net_id_(0) {}
TransportFeedbackAdapter::~TransportFeedbackAdapter() {}
@ -43,7 +45,10 @@ void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number,
if (send_side_bwe_with_overhead_) {
length += transport_overhead_bytes_per_packet_;
}
send_time_history_.AddAndRemoveOld(sequence_number, length, pacing_info);
const int64_t creation_time_ms = clock_->TimeInMilliseconds();
send_time_history_.AddAndRemoveOld(
PacketFeedback(creation_time_ms, sequence_number, length, local_net_id_,
remote_net_id_, pacing_info));
}
void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number,
@ -58,9 +63,11 @@ void TransportFeedbackAdapter::SetTransportOverhead(
transport_overhead_bytes_per_packet_ = transport_overhead_bytes_per_packet;
}
void TransportFeedbackAdapter::ClearSendTimeHistory() {
void TransportFeedbackAdapter::SetNetworkIds(uint16_t local_id,
uint16_t remote_id) {
rtc::CritScope cs(&lock_);
send_time_history_.Clear();
local_net_id_ = local_id;
remote_net_id_ = remote_id;
}
std::vector<PacketFeedback> TransportFeedbackAdapter::GetPacketFeedbackVector(
@ -115,7 +122,10 @@ std::vector<PacketFeedback> TransportFeedbackAdapter::GetPacketFeedbackVector(
// as received by another feedback.
if (!send_time_history_.GetFeedback(&packet_feedback, false))
++failed_lookups;
packet_feedback_vector.push_back(packet_feedback);
if (packet_feedback.local_net_id == local_net_id_ &&
packet_feedback.remote_net_id == remote_net_id_) {
packet_feedback_vector.push_back(packet_feedback);
}
}
// Handle this iteration's received packet.
@ -124,7 +134,10 @@ std::vector<PacketFeedback> TransportFeedbackAdapter::GetPacketFeedbackVector(
PacketFeedback packet_feedback(timestamp_ms, packet.sequence_number());
if (!send_time_history_.GetFeedback(&packet_feedback, true))
++failed_lookups;
packet_feedback_vector.push_back(packet_feedback);
if (packet_feedback.local_net_id == local_net_id_ &&
packet_feedback.remote_net_id == remote_net_id_) {
packet_feedback_vector.push_back(packet_feedback);
}
++seq_num;
}

4
webrtc/modules/congestion_controller/transport_feedback_adapter.h
View File

@ -43,7 +43,7 @@ class TransportFeedbackAdapter {
void SetTransportOverhead(int transport_overhead_bytes_per_packet);
void ClearSendTimeHistory();
void SetNetworkIds(uint16_t local_id, uint16_t remote_id);
private:
std::vector<PacketFeedback> GetPacketFeedbackVector(
@ -57,6 +57,8 @@ class TransportFeedbackAdapter {
int64_t current_offset_ms_;
int64_t last_timestamp_us_;
std::vector<PacketFeedback> last_packet_feedback_vector_;
uint16_t local_net_id_;
uint16_t remote_net_id_;
};
} // namespace webrtc

3
webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
View File

@ -250,7 +250,8 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
rtcp::TransportFeedback::kDeltaScaleFactor *
std::numeric_limits<int16_t>::min();
PacketFeedback packet_feedback(100, 200, 0, 1500, true, PacedPacketInfo());
PacketFeedback packet_feedback(100, 200, 0, 1500, true, 0, 0,
PacedPacketInfo());
sent_packets.push_back(packet_feedback);
packet_feedback.send_time_ms += kSmallDeltaUs / 1000;

6
webrtc/modules/remote_bitrate_estimator/include/send_time_history.h
View File

@ -26,12 +26,8 @@ class SendTimeHistory {
SendTimeHistory(const Clock* clock, int64_t packet_age_limit_ms);
~SendTimeHistory();
void Clear();
// Cleanup old entries, then add new packet info with provided parameters.
void AddAndRemoveOld(uint16_t sequence_number,
size_t payload_size,
const PacedPacketInfo& pacing_info);
void AddAndRemoveOld(const PacketFeedback& packet);
// Updates packet info identified by |sequence_number| with |send_time_ms|.
// Return false if not found.

18
webrtc/modules/remote_bitrate_estimator/send_time_history.cc
View File

@ -22,13 +22,7 @@ SendTimeHistory::SendTimeHistory(const Clock* clock,
SendTimeHistory::~SendTimeHistory() {}
void SendTimeHistory::Clear() {
history_.clear();
}
void SendTimeHistory::AddAndRemoveOld(uint16_t sequence_number,
size_t payload_size,
const PacedPacketInfo& pacing_info) {
void SendTimeHistory::AddAndRemoveOld(const PacketFeedback& packet) {
int64_t now_ms = clock_->TimeInMilliseconds();
// Remove old.
while (!history_.empty() &&
@ -39,14 +33,8 @@ void SendTimeHistory::AddAndRemoveOld(uint16_t sequence_number,
}
// Add new.
int64_t unwrapped_seq_num = seq_num_unwrapper_.Unwrap(sequence_number);
int64_t creation_time_ms = now_ms;
constexpr int64_t kNoArrivalTimeMs = -1; // Arrival time is ignored.
constexpr int64_t kNoSendTimeMs = -1; // Send time is set by OnSentPacket.
history_.insert(std::make_pair(
unwrapped_seq_num,
PacketFeedback(creation_time_ms, kNoArrivalTimeMs, kNoSendTimeMs,
sequence_number, payload_size, pacing_info)));
int64_t unwrapped_seq_num = seq_num_unwrapper_.Unwrap(packet.sequence_number);
history_.insert(std::make_pair(unwrapped_seq_num, packet));
}
bool SendTimeHistory::OnSentPacket(uint16_t sequence_number,

50
webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc
View File

@ -36,7 +36,9 @@ class SendTimeHistoryTest : public ::testing::Test {
size_t length,
int64_t send_time_ms,
const PacedPacketInfo& pacing_info) {
history_.AddAndRemoveOld(sequence_number, length, pacing_info);
PacketFeedback packet(clock_.TimeInMilliseconds(), sequence_number, length,
0, 0, pacing_info);
history_.AddAndRemoveOld(packet);
history_.OnSentPacket(sequence_number, send_time_ms);
}
@ -44,6 +46,22 @@ class SendTimeHistoryTest : public ::testing::Test {
SendTimeHistory history_;
};
TEST_F(SendTimeHistoryTest, SaveAndRestoreNetworkId) {
const PacedPacketInfo kPacingInfo(0, 5, 1200);
uint16_t sequence_number = 0;
int64_t now_ms = clock_.TimeInMilliseconds();
for (int i = 1; i < 5; ++i) {
PacketFeedback packet(now_ms, sequence_number++, 1000, i, i - 1,
kPacingInfo);
history_.AddAndRemoveOld(packet);
history_.OnSentPacket(sequence_number, now_ms);
PacketFeedback restored(now_ms, sequence_number);
EXPECT_TRUE(history_.GetFeedback(&restored, sequence_number));
EXPECT_EQ(packet.local_net_id, restored.local_net_id);
EXPECT_EQ(packet.remote_net_id, restored.remote_net_id);
}
}
TEST_F(SendTimeHistoryTest, AddRemoveOne) {
const uint16_t kSeqNo = 10;
// TODO(philipel): Fix PacedPacketInfo constructor?
@ -97,9 +115,10 @@ TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) {
static_cast<uint16_t>(i), kPacketSize, PacedPacketInfo()));
}
for (size_t i = 0; i < num_items; ++i) {
history_.AddAndRemoveOld(sent_packets[i].sequence_number,
sent_packets[i].payload_size,
PacedPacketInfo(1, 2, 200));
PacketFeedback packet = sent_packets[i];
packet.arrival_time_ms = -1;
packet.send_time_ms = -1;
history_.AddAndRemoveOld(packet);
}
for (size_t i = 0; i < num_items; ++i)
history_.OnSentPacket(sent_packets[i].sequence_number,
@ -212,28 +231,5 @@ TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) {
EXPECT_TRUE(history_.GetFeedback(&packet3, true));
EXPECT_EQ(packets[2], packet3);
}
TEST_F(SendTimeHistoryTest, Clear) {
const uint16_t kSeqNo = 1;
const int64_t kTimestamp = 2;
const PacedPacketInfo kPacingInfo(0, 5, 1200);
PacketFeedback packets[] = {{0, kTimestamp, kSeqNo, 0, kPacingInfo},
{0, kTimestamp + 1, kSeqNo + 1, 0, kPacingInfo}};
AddPacketWithSendTime(packets[0].sequence_number, packets[0].payload_size,
packets[0].send_time_ms, kPacingInfo);
AddPacketWithSendTime(packets[1].sequence_number, packets[1].payload_size,
packets[1].send_time_ms, kPacingInfo);
PacketFeedback info(0, 0, packets[0].sequence_number, 0, kPacingInfo);
EXPECT_TRUE(history_.GetFeedback(&info, true));
EXPECT_EQ(packets[0], info);
history_.Clear();
PacketFeedback info2(0, 0, packets[1].sequence_number, 0, kPacingInfo);
EXPECT_FALSE(history_.GetFeedback(&info2, true));
}
} // namespace test
} // namespace webrtc

9
webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc
View File

@ -111,9 +111,10 @@ void SendSideBweSender::OnPacketsSent(const Packets& packets) {
MediaPacket* media_packet = static_cast<MediaPacket*>(packet);
// TODO(philipel): Add probe_cluster_id to Packet class in order
// to create tests for probing using cluster ids.
send_time_history_.AddAndRemoveOld(media_packet->header().sequenceNumber,
media_packet->payload_size(),
PacedPacketInfo());
PacketFeedback packet_feedback(
clock_->TimeInMilliseconds(), media_packet->header().sequenceNumber,
media_packet->payload_size(), 0, 0, PacedPacketInfo());
send_time_history_.AddAndRemoveOld(packet_feedback);
send_time_history_.OnSentPacket(media_packet->header().sequenceNumber,
media_packet->sender_timestamp_ms());
}
@ -146,7 +147,7 @@ void SendSideBweReceiver::ReceivePacket(int64_t arrival_time_ms,
packet_feedback_vector_.push_back(
PacketFeedback(-1, arrival_time_ms, media_packet.sender_timestamp_ms(),
media_packet.header().sequenceNumber,
media_packet.payload_size(), PacedPacketInfo()));
media_packet.payload_size(), 0, 0, PacedPacketInfo()));
// Log received packet information.
BweReceiver::ReceivePacket(arrival_time_ms, media_packet);

26
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h
View File

@ -251,6 +251,8 @@ struct PacketFeedback {
-1,
sequence_number,
0,
0,
0,
PacedPacketInfo()) {}
PacketFeedback(int64_t arrival_time_ms,
@ -263,6 +265,23 @@ struct PacketFeedback {
send_time_ms,
sequence_number,
payload_size,
0,
0,
pacing_info) {}
PacketFeedback(int64_t creation_time_ms,
uint16_t sequence_number,
size_t payload_size,
uint16_t local_net_id,
uint16_t remote_net_id,
const PacedPacketInfo& pacing_info)
: PacketFeedback(creation_time_ms,
-1,
-1,
sequence_number,
payload_size,
local_net_id,
remote_net_id,
pacing_info) {}
PacketFeedback(int64_t creation_time_ms,
@ -270,12 +289,16 @@ struct PacketFeedback {
int64_t send_time_ms,
uint16_t sequence_number,
size_t payload_size,
uint16_t local_net_id,
uint16_t remote_net_id,
const PacedPacketInfo& pacing_info)
: creation_time_ms(creation_time_ms),
arrival_time_ms(arrival_time_ms),
send_time_ms(send_time_ms),
sequence_number(sequence_number),
payload_size(payload_size),
local_net_id(local_net_id),
remote_net_id(remote_net_id),
pacing_info(pacing_info) {}
static constexpr int kNotAProbe = -1;
@ -307,6 +330,9 @@ struct PacketFeedback {
uint16_t sequence_number;
// Size of the packet excluding RTP headers.
size_t payload_size;
// The network route ids that this packet is associated with.
uint16_t local_net_id;
uint16_t remote_net_id;
// Pacing information about this packet.
PacedPacketInfo pacing_info;
};