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

Revert of Propagate probing cluster id to SendTimeHistory. (patchset #5 id:80001 of https://codereview.webrtc.org/2005313003/ )

Browse Source 
Reason for revert:
Breaks google3 buildbot:  http://webrtc-buildbot-master.mtv.corp.google.com:21000/builders/WebRTC%20google3%20Importer/builds/8640

Original issue's description:
> Propagate probing cluster id to SendTimeHistory, both for packets and padding.
>
> BUG=webrtc:5859
>
> Committed: https://crrev.com/5be28c848b91bc6e4800eac07a3f5ac09a32ad70
> Cr-Commit-Position: refs/heads/master@{#12985}

TBR=danilchap@webrtc.org,stefan@webrtc.org,mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5859

Review-Url: https://codereview.webrtc.org/2032463003
Cr-Commit-Position: refs/heads/master@{#12987}
This commit is contained in:
philipel
2016-06-01 04:04:40 -07:00
committed by Commit bot
parent ce8d58c20e
commit 46948c17fd
24 changed files with 192 additions and 279 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -24,10 +24,7 @@ class SendTimeHistory {
SendTimeHistory(Clock* clock, int64_t packet_age_limit);
virtual ~SendTimeHistory();
void AddAndRemoveOld(uint16_t sequence_number,
size_t length,
bool was_paced,
int probe_cluster_id);
void AddAndRemoveOld(uint16_t sequence_number, size_t length, bool was_paced);
bool OnSentPacket(uint16_t sequence_number, int64_t timestamp);
// Look up PacketInfo for a sent packet, based on the sequence number, and
// populate all fields except for receive_time. The packet parameter must

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

@ -28,17 +28,15 @@ void SendTimeHistory::Clear() {
void SendTimeHistory::AddAndRemoveOld(uint16_t sequence_number,
size_t length,
bool was_paced,
int probe_cluster_id) {
bool was_paced) {
EraseOld();
if (history_.empty())
oldest_sequence_number_ = sequence_number;
history_.insert(std::pair<uint16_t, PacketInfo>(
sequence_number,
PacketInfo(clock_->TimeInMilliseconds(), 0, -1, sequence_number, length,
was_paced, probe_cluster_id)));
sequence_number, PacketInfo(clock_->TimeInMilliseconds(), 0, -1,
sequence_number, length, was_paced)));
}
bool SendTimeHistory::OnSentPacket(uint16_t sequence_number,

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

@ -34,10 +34,8 @@ class SendTimeHistoryTest : public ::testing::Test {
void AddPacketWithSendTime(uint16_t sequence_number,
size_t length,
bool was_paced,
int64_t send_time_ms,
int probe_cluster_id) {
history_.AddAndRemoveOld(sequence_number, length, was_paced,
probe_cluster_id);
int64_t send_time_ms) {
history_.AddAndRemoveOld(sequence_number, length, was_paced);
history_.OnSentPacket(sequence_number, send_time_ms);
}
@ -48,58 +46,42 @@ class SendTimeHistoryTest : public ::testing::Test {
// Help class extended so we can do EXPECT_EQ and collections.
class PacketInfo : public webrtc::PacketInfo {
public:
PacketInfo()
: webrtc::PacketInfo(-1,
0,
0,
0,
0,
false,
webrtc::PacketInfo::kNotAProbe) {}
PacketInfo() : webrtc::PacketInfo(-1, 0, 0, 0, 0, false) {}
PacketInfo(int64_t arrival_time_ms, uint16_t sequence_number)
: PacketInfo(arrival_time_ms,
0,
sequence_number,
0,
false,
PacketInfo::kNotAProbe) {}
: PacketInfo(arrival_time_ms, 0, sequence_number, 0, false) {}
PacketInfo(int64_t arrival_time_ms,
int64_t send_time_ms,
uint16_t sequence_number,
size_t payload_size,
bool was_paced,
int probe_cluster_id)
bool was_paced)
: webrtc::PacketInfo(-1,
arrival_time_ms,
send_time_ms,
sequence_number,
payload_size,
was_paced,
probe_cluster_id) {}
was_paced) {}
bool operator==(const PacketInfo& other) const {
return arrival_time_ms == other.arrival_time_ms &&
send_time_ms == other.send_time_ms &&
sequence_number == other.sequence_number &&
payload_size == other.payload_size && was_paced == other.was_paced &&
probe_cluster_id == other.probe_cluster_id;
payload_size == other.payload_size && was_paced == other.was_paced;
}
};
TEST_F(SendTimeHistoryTest, AddRemoveOne) {
const uint16_t kSeqNo = 10;
const int kProbeClusterId = 0;
const PacketInfo kSentPacket(0, 1, kSeqNo, 1, true, kProbeClusterId);
AddPacketWithSendTime(kSeqNo, 1, true, 1, kProbeClusterId);
const PacketInfo kSentPacket(0, 1, kSeqNo, 1, true);
AddPacketWithSendTime(kSeqNo, 1, true, 1);
PacketInfo received_packet(0, 0, kSeqNo, 0, false, kProbeClusterId);
PacketInfo received_packet(0, 0, kSeqNo, 0, false);
EXPECT_TRUE(history_.GetInfo(&received_packet, false));
EXPECT_EQ(kSentPacket, received_packet);
PacketInfo received_packet2(0, 0, kSeqNo, 0, false, kProbeClusterId);
PacketInfo received_packet2(0, 0, kSeqNo, 0, false);
EXPECT_TRUE(history_.GetInfo(&received_packet2, true));
EXPECT_EQ(kSentPacket, received_packet2);
PacketInfo received_packet3(0, 0, kSeqNo, 0, false, kProbeClusterId);
PacketInfo received_packet3(0, 0, kSeqNo, 0, false);
EXPECT_FALSE(history_.GetInfo(&received_packet3, true));
}
@ -110,8 +92,7 @@ TEST_F(SendTimeHistoryTest, PopulatesExpectedFields) {
const size_t kPayloadSize = 42;
const bool kPaced = true;
AddPacketWithSendTime(kSeqNo, kPayloadSize, kPaced, kSendTime,
PacketInfo::kNotAProbe);
AddPacketWithSendTime(kSeqNo, kPayloadSize, kPaced, kSendTime);
PacketInfo info(kReceiveTime, kSeqNo);
EXPECT_TRUE(history_.GetInfo(&info, true));
@ -129,19 +110,18 @@ TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) {
const size_t kPacketSize = 400;
const size_t kTransmissionTime = 1234;
const bool kPaced = true;
const int kProbeClusterId = 1;
for (size_t i = 0; i < num_items; ++i) {
sent_packets.push_back(PacketInfo(0, static_cast<int64_t>(i),
static_cast<uint16_t>(i), kPacketSize,
kPaced, kProbeClusterId));
received_packets.push_back(PacketInfo(
static_cast<int64_t>(i) + kTransmissionTime, 0,
static_cast<uint16_t>(i), kPacketSize, false, PacketInfo::kNotAProbe));
kPaced));
received_packets.push_back(
PacketInfo(static_cast<int64_t>(i) + kTransmissionTime, 0,
static_cast<uint16_t>(i), kPacketSize, false));
}
for (size_t i = 0; i < num_items; ++i) {
history_.AddAndRemoveOld(
sent_packets[i].sequence_number, sent_packets[i].payload_size,
sent_packets[i].was_paced, sent_packets[i].probe_cluster_id);
history_.AddAndRemoveOld(sent_packets[i].sequence_number,
sent_packets[i].payload_size,
sent_packets[i].was_paced);
}
for (size_t i = 0; i < num_items; ++i)
history_.OnSentPacket(sent_packets[i].sequence_number,
@ -163,21 +143,19 @@ TEST_F(SendTimeHistoryTest, HistorySize) {
const int kItems = kDefaultHistoryLengthMs / 100;
for (int i = 0; i < kItems; ++i) {
clock_.AdvanceTimeMilliseconds(100);
AddPacketWithSendTime(i, 0, false, i * 100, PacketInfo::kNotAProbe);
AddPacketWithSendTime(i, 0, false, i * 100);
}
for (int i = 0; i < kItems; ++i) {
PacketInfo info(0, 0, static_cast<uint16_t>(i), 0, false,
PacketInfo::kNotAProbe);
PacketInfo info(0, 0, static_cast<uint16_t>(i), 0, false);
EXPECT_TRUE(history_.GetInfo(&info, false));
EXPECT_EQ(i * 100, info.send_time_ms);
}
clock_.AdvanceTimeMilliseconds(101);
AddPacketWithSendTime(kItems, 0, false, kItems * 101, PacketInfo::kNotAProbe);
PacketInfo info(0, 0, 0, 0, false, PacketInfo::kNotAProbe);
AddPacketWithSendTime(kItems, 0, false, kItems * 101);
PacketInfo info(0, 0, 0, 0, false);
EXPECT_FALSE(history_.GetInfo(&info, false));
for (int i = 1; i < (kItems + 1); ++i) {
PacketInfo info2(0, 0, static_cast<uint16_t>(i), 0, false,
PacketInfo::kNotAProbe);
PacketInfo info2(0, 0, static_cast<uint16_t>(i), 0, false);
EXPECT_TRUE(history_.GetInfo(&info2, false));
int64_t expected_time_ms = (i == kItems) ? i * 101 : i * 100;
EXPECT_EQ(expected_time_ms, info2.send_time_ms);
@ -186,17 +164,16 @@ TEST_F(SendTimeHistoryTest, HistorySize) {
TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) {
const uint16_t kMaxSeqNo = std::numeric_limits<uint16_t>::max();
AddPacketWithSendTime(kMaxSeqNo - 2, 0, false, 0, PacketInfo::kNotAProbe);
AddPacketWithSendTime(kMaxSeqNo - 2, 0, false, 0);
clock_.AdvanceTimeMilliseconds(100);
AddPacketWithSendTime(kMaxSeqNo - 1, 1, false, 100, PacketInfo::kNotAProbe);
AddPacketWithSendTime(kMaxSeqNo - 1, 1, false, 100);
clock_.AdvanceTimeMilliseconds(100);
AddPacketWithSendTime(kMaxSeqNo, 0, false, 200, PacketInfo::kNotAProbe);
AddPacketWithSendTime(kMaxSeqNo, 0, false, 200);
clock_.AdvanceTimeMilliseconds(kDefaultHistoryLengthMs - 200 + 1);
AddPacketWithSendTime(0, 0, false, kDefaultHistoryLengthMs,
PacketInfo::kNotAProbe);
AddPacketWithSendTime(0, 0, false, kDefaultHistoryLengthMs);
PacketInfo info(0, static_cast<uint16_t>(kMaxSeqNo - 2));
EXPECT_FALSE(history_.GetInfo(&info, false));
@ -212,7 +189,7 @@ TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) {
EXPECT_TRUE(history_.GetInfo(&info5, true));
clock_.AdvanceTimeMilliseconds(100);
AddPacketWithSendTime(1, 0, false, 1100, PacketInfo::kNotAProbe);
AddPacketWithSendTime(1, 0, false, 1100);
PacketInfo info6(0, static_cast<uint16_t>(kMaxSeqNo - 2));
EXPECT_FALSE(history_.GetInfo(&info6, false));
@ -229,26 +206,26 @@ TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) {
TEST_F(SendTimeHistoryTest, InterlievedGetAndRemove) {
const uint16_t kSeqNo = 1;
const int64_t kTimestamp = 2;
PacketInfo packets[3] = {{0, kTimestamp, kSeqNo, 0, false, 0},
{0, kTimestamp + 1, kSeqNo + 1, 0, false, 1},
{0, kTimestamp + 2, kSeqNo + 2, 0, false, 2}};
PacketInfo packets[3] = {{0, kTimestamp, kSeqNo, 0, false},
{0, kTimestamp + 1, kSeqNo + 1, 0, false},
{0, kTimestamp + 2, kSeqNo + 2, 0, false}};
AddPacketWithSendTime(packets[0].sequence_number, packets[0].payload_size,
packets[0].was_paced, packets[0].send_time_ms, 0);
packets[0].was_paced, packets[0].send_time_ms);
AddPacketWithSendTime(packets[1].sequence_number, packets[1].payload_size,
packets[1].was_paced, packets[1].send_time_ms, 1);
PacketInfo info(0, 0, packets[0].sequence_number, 0, false, 0);
packets[1].was_paced, packets[1].send_time_ms);
PacketInfo info(0, 0, packets[0].sequence_number, 0, false);
EXPECT_TRUE(history_.GetInfo(&info, true));
EXPECT_EQ(packets[0], info);
AddPacketWithSendTime(packets[2].sequence_number, packets[2].payload_size,
packets[2].was_paced, packets[2].send_time_ms, 2);
packets[2].was_paced, packets[2].send_time_ms);
PacketInfo info2(0, 0, packets[1].sequence_number, 0, false, 1);
PacketInfo info2(0, 0, packets[1].sequence_number, 0, false);
EXPECT_TRUE(history_.GetInfo(&info2, true));
EXPECT_EQ(packets[1], info2);
PacketInfo info3(0, 0, packets[2].sequence_number, 0, false, 2);
PacketInfo info3(0, 0, packets[2].sequence_number, 0, false);
EXPECT_TRUE(history_.GetInfo(&info3, true));
EXPECT_EQ(packets[2], info3);
}

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

@ -92,11 +92,9 @@ void FullBweSender::OnPacketsSent(const Packets& packets) {
for (Packet* packet : packets) {
if (packet->GetPacketType() == Packet::kMedia) {
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(),
packet->paced(), PacketInfo::kNotAProbe);
send_time_history_.AddAndRemoveOld(media_packet->header().sequenceNumber,
media_packet->payload_size(),
packet->paced());
send_time_history_.OnSentPacket(media_packet->header().sequenceNumber,
media_packet->sender_timestamp_ms());
}

2
webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc
View File

@ -299,7 +299,7 @@ bool PacedVideoSender::TimeToSendPacket(uint32_t ssrc,
return false;
}
size_t PacedVideoSender::TimeToSendPadding(size_t bytes, int probe_cluster_id) {
size_t PacedVideoSender::TimeToSendPadding(size_t bytes) {
return 0;
}

2
webrtc/modules/remote_bitrate_estimator/test/packet_sender.h
View File

@ -115,7 +115,7 @@ class PacedVideoSender : public VideoSender, public PacedSender::PacketSender {
int64_t capture_time_ms,
bool retransmission,
int probe_cluster_id) override;
size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override;
size_t TimeToSendPadding(size_t bytes) override;
// Implements BitrateObserver.
void OnNetworkChanged(uint32_t target_bitrate_bps,

6
webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
View File

@ -59,11 +59,9 @@ void TransportFeedbackAdapter::SetBitrateEstimator(
void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number,
size_t length,
bool was_paced,
int probe_cluster_id) {
bool was_paced) {
rtc::CritScope cs(&lock_);
send_time_history_.AddAndRemoveOld(sequence_number, length, was_paced,
probe_cluster_id);
send_time_history_.AddAndRemoveOld(sequence_number, length, was_paced);
}
void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number,

3
webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h
View File

@ -40,8 +40,7 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver,
// Implements TransportFeedbackObserver.
void AddPacket(uint16_t sequence_number,
size_t length,
bool was_paced,
int probe_cluster_id) override;
bool was_paced) override;
void OnSentPacket(uint16_t sequence_number, int64_t send_time_ms);
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;

42
webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
View File

@ -93,15 +93,14 @@ class TransportFeedbackAdapterTest : public ::testing::Test {
EXPECT_EQ(truth[i].sequence_number, input[i].sequence_number);
EXPECT_EQ(truth[i].payload_size, input[i].payload_size);
EXPECT_EQ(truth[i].was_paced, input[i].was_paced);
EXPECT_EQ(truth[i].probe_cluster_id, input[i].probe_cluster_id);
}
}
// Utility method, to reset arrival_time_ms before adding send time.
void OnSentPacket(PacketInfo info) {
info.arrival_time_ms = 0;
adapter_->AddPacket(info.sequence_number, info.payload_size, info.was_paced,
info.probe_cluster_id);
adapter_->AddPacket(info.sequence_number, info.payload_size,
info.was_paced);
adapter_->OnSentPacket(info.sequence_number, info.send_time_ms);
}
@ -115,11 +114,11 @@ class TransportFeedbackAdapterTest : public ::testing::Test {
TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
std::vector<PacketInfo> packets;
packets.push_back(PacketInfo(100, 200, 0, 1500, true, 0));
packets.push_back(PacketInfo(110, 210, 1, 1500, true, 0));
packets.push_back(PacketInfo(120, 220, 2, 1500, true, 0));
packets.push_back(PacketInfo(130, 230, 3, 1500, true, 1));
packets.push_back(PacketInfo(140, 240, 4, 1500, true, 1));
packets.push_back(PacketInfo(100, 200, 0, 1500, true));
packets.push_back(PacketInfo(110, 210, 1, 1500, true));
packets.push_back(PacketInfo(120, 220, 2, 1500, true));
packets.push_back(PacketInfo(130, 230, 3, 1500, true));
packets.push_back(PacketInfo(140, 240, 4, 1500, true));
for (const PacketInfo& packet : packets)
OnSentPacket(packet);
@ -146,11 +145,11 @@ TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) {
std::vector<PacketInfo> packets;
packets.push_back(PacketInfo(100, 200, 0, 1500, true, 1));
packets.push_back(PacketInfo(110, 210, 1, 1500, true, 2));
packets.push_back(PacketInfo(120, 220, 2, 1500, true, 3));
packets.push_back(PacketInfo(130, 230, 3, 1500, true, 4));
packets.push_back(PacketInfo(140, 240, 4, 1500, true, 5));
packets.push_back(PacketInfo(100, 200, 0, 1500, true));
packets.push_back(PacketInfo(110, 210, 1, 1500, true));
packets.push_back(PacketInfo(120, 220, 2, 1500, true));
packets.push_back(PacketInfo(130, 230, 3, 1500, true));
packets.push_back(PacketInfo(140, 240, 4, 1500, true));
const uint16_t kSendSideDropBefore = 1;
const uint16_t kReceiveSideDropAfter = 3;
@ -191,12 +190,9 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
static_cast<int64_t>(1 << 8) *
static_cast<int64_t>((1 << 23) - 1) / 1000;
std::vector<PacketInfo> packets;
packets.push_back(PacketInfo(kHighArrivalTimeMs - 64, 200, 0, 1500, true,
PacketInfo::kNotAProbe));
packets.push_back(PacketInfo(kHighArrivalTimeMs + 64, 210, 1, 1500, true,
PacketInfo::kNotAProbe));
packets.push_back(PacketInfo(kHighArrivalTimeMs, 220, 2, 1500, true,
PacketInfo::kNotAProbe));
packets.push_back(PacketInfo(kHighArrivalTimeMs - 64, 200, 0, 1500, true));
packets.push_back(PacketInfo(kHighArrivalTimeMs + 64, 210, 1, 1500, true));
packets.push_back(PacketInfo(kHighArrivalTimeMs, 220, 2, 1500, true));
for (const PacketInfo& packet : packets)
OnSentPacket(packet);
@ -229,9 +225,9 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
TEST_F(TransportFeedbackAdapterTest, HandlesReordering) {
std::vector<PacketInfo> packets;
packets.push_back(PacketInfo(120, 200, 0, 1500, true, 0));
packets.push_back(PacketInfo(110, 210, 1, 1500, true, 0));
packets.push_back(PacketInfo(100, 220, 2, 1500, true, 0));
packets.push_back(PacketInfo(120, 200, 0, 1500, true));
packets.push_back(PacketInfo(110, 210, 1, 1500, true));
packets.push_back(PacketInfo(100, 220, 2, 1500, true));
std::vector<PacketInfo> expected_packets;
expected_packets.push_back(packets[2]);
expected_packets.push_back(packets[1]);
@ -271,7 +267,7 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
rtcp::TransportFeedback::kDeltaScaleFactor *
std::numeric_limits<int16_t>::min();
PacketInfo info(100, 200, 0, 1500, true, PacketInfo::kNotAProbe);
PacketInfo info(100, 200, 0, 1500, true);
sent_packets.push_back(info);
info.send_time_ms += kSmallDeltaUs / 1000;