Refactor RTPPacketHistory to use a packet struct.

Collects packet information within a struct instead of spreading it out
over different vectors. Adds a fixed-size buffer to the stored packet
instead of using vectors.

BUG=
R=stefan@webrtc.org

Review URL: https://codereview.webrtc.org/1340573002

Cr-Commit-Position: refs/heads/master@{#9926}
This commit is contained in:
pbos
2015-09-11 08:33:35 -07:00
committed by Commit bot
parent 92068ee683
commit c32d2db69b
4 changed files with 76 additions and 125 deletions

View File

@ -25,12 +25,10 @@ namespace webrtc {
static const int kMinPacketRequestBytes = 50;
RTPPacketHistory::RTPPacketHistory(Clock* clock)
: clock_(clock),
critsect_(CriticalSectionWrapper::CreateCriticalSection()),
store_(false),
prev_index_(0),
max_packet_length_(0) {
}
: clock_(clock),
critsect_(CriticalSectionWrapper::CreateCriticalSection()),
store_(false),
prev_index_(0) {}
RTPPacketHistory::~RTPPacketHistory() {
}
@ -55,11 +53,6 @@ void RTPPacketHistory::Allocate(size_t number_to_store) {
assert(number_to_store <= kMaxHistoryCapacity);
store_ = true;
stored_packets_.resize(number_to_store);
stored_seq_nums_.resize(number_to_store);
stored_lengths_.resize(number_to_store);
stored_times_.resize(number_to_store);
stored_send_times_.resize(number_to_store);
stored_types_.resize(number_to_store);
}
void RTPPacketHistory::Free() {
@ -67,21 +60,10 @@ void RTPPacketHistory::Free() {
return;
}
std::vector<std::vector<uint8_t> >::iterator it;
for (it = stored_packets_.begin(); it != stored_packets_.end(); ++it) {
it->clear();
}
stored_packets_.clear();
stored_seq_nums_.clear();
stored_lengths_.clear();
stored_times_.clear();
stored_send_times_.clear();
stored_types_.clear();
store_ = false;
prev_index_ = 0;
max_packet_length_ = 0;
}
bool RTPPacketHistory::StorePackets() const {
@ -89,31 +71,8 @@ bool RTPPacketHistory::StorePackets() const {
return store_;
}
void RTPPacketHistory::VerifyAndAllocatePacketLength(size_t packet_length,
uint32_t start_index) {
assert(packet_length > 0);
if (!store_) {
return;
}
// If start_index > 0 this is a resize and we must check any new (empty)
// packets created during the resize.
if (start_index == 0 && packet_length <= max_packet_length_) {
return;
}
max_packet_length_ = std::max(packet_length, max_packet_length_);
std::vector<std::vector<uint8_t> >::iterator it;
for (it = stored_packets_.begin() + start_index; it != stored_packets_.end();
++it) {
it->resize(max_packet_length_);
}
}
int32_t RTPPacketHistory::PutRTPPacket(const uint8_t* packet,
size_t packet_length,
size_t max_packet_length,
int64_t capture_time_ms,
StorageType type) {
if (type == kDontStore) {
@ -128,9 +87,7 @@ int32_t RTPPacketHistory::PutRTPPacket(const uint8_t* packet,
assert(packet);
assert(packet_length > 3);
VerifyAndAllocatePacketLength(max_packet_length, 0);
if (packet_length > max_packet_length_) {
if (packet_length > IP_PACKET_SIZE) {
LOG(LS_WARNING) << "Failed to store RTP packet with length: "
<< packet_length;
return -1;
@ -141,14 +98,13 @@ int32_t RTPPacketHistory::PutRTPPacket(const uint8_t* packet,
// If index we're about to overwrite contains a packet that has not
// yet been sent (probably pending in paced sender), we need to expand
// the buffer.
if (stored_lengths_[prev_index_] > 0 &&
stored_send_times_[prev_index_] == 0) {
if (stored_packets_[prev_index_].length > 0 &&
stored_packets_[prev_index_].send_time == 0) {
size_t current_size = static_cast<uint16_t>(stored_packets_.size());
if (current_size < kMaxHistoryCapacity) {
size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);
expanded_size = std::min(expanded_size, kMaxHistoryCapacity);
Allocate(expanded_size);
VerifyAndAllocatePacketLength(max_packet_length, current_size);
// Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
// but may be slower - at least until buffer has wrapped around once.
prev_index_ = current_size;
@ -156,21 +112,19 @@ int32_t RTPPacketHistory::PutRTPPacket(const uint8_t* packet,
}
// Store packet
std::vector<std::vector<uint8_t> >::iterator it =
stored_packets_.begin() + prev_index_;
// TODO(sprang): Overhaul this class and get rid of this copy step.
// (Finally introduce the RtpPacket class?)
std::copy(packet, packet + packet_length, it->begin());
memcpy(stored_packets_[prev_index_].data, packet, packet_length);
stored_packets_[prev_index_].length = packet_length;
stored_seq_nums_[prev_index_] = seq_num;
stored_lengths_[prev_index_] = packet_length;
stored_times_[prev_index_] = (capture_time_ms > 0) ? capture_time_ms :
clock_->TimeInMilliseconds();
stored_send_times_[prev_index_] = 0; // Packet not sent.
stored_types_[prev_index_] = type;
stored_packets_[prev_index_].sequence_number = seq_num;
stored_packets_[prev_index_].time_ms =
(capture_time_ms > 0) ? capture_time_ms : clock_->TimeInMilliseconds();
stored_packets_[prev_index_].send_time = 0; // Packet not sent.
stored_packets_[prev_index_].storage_type = type;
++prev_index_;
if (prev_index_ >= stored_seq_nums_.size()) {
if (prev_index_ >= stored_packets_.size()) {
prev_index_ = 0;
}
return 0;
@ -188,8 +142,7 @@ bool RTPPacketHistory::HasRTPPacket(uint16_t sequence_number) const {
return false;
}
size_t length = stored_lengths_.at(index);
if (length == 0 || length > max_packet_length_) {
if (stored_packets_[index].length == 0) {
// Invalid length.
return false;
}
@ -209,11 +162,11 @@ bool RTPPacketHistory::SetSent(uint16_t sequence_number) {
}
// Send time already set.
if (stored_send_times_[index] != 0) {
if (stored_packets_[index].send_time != 0) {
return false;
}
stored_send_times_[index] = clock_->TimeInMilliseconds();
stored_packets_[index].send_time = clock_->TimeInMilliseconds();
return true;
}
@ -224,7 +177,7 @@ bool RTPPacketHistory::GetPacketAndSetSendTime(uint16_t sequence_number,
size_t* packet_length,
int64_t* stored_time_ms) {
CriticalSectionScoped cs(critsect_.get());
assert(*packet_length >= max_packet_length_);
assert(*packet_length >= IP_PACKET_SIZE);
if (!store_) {
return false;
}
@ -236,8 +189,8 @@ bool RTPPacketHistory::GetPacketAndSetSendTime(uint16_t sequence_number,
return false;
}
size_t length = stored_lengths_.at(index);
assert(length <= max_packet_length_);
size_t length = stored_packets_[index].length;
assert(length <= IP_PACKET_SIZE);
if (length == 0) {
LOG(LS_WARNING) << "No match for getting seqNum " << sequence_number
<< ", len " << length;
@ -247,16 +200,16 @@ bool RTPPacketHistory::GetPacketAndSetSendTime(uint16_t sequence_number,
// Verify elapsed time since last retrieve.
int64_t now = clock_->TimeInMilliseconds();
if (min_elapsed_time_ms > 0 &&
((now - stored_send_times_.at(index)) < min_elapsed_time_ms)) {
((now - stored_packets_[index].send_time) < min_elapsed_time_ms)) {
return false;
}
if (retransmit && stored_types_.at(index) == kDontRetransmit) {
if (retransmit && stored_packets_[index].storage_type == kDontRetransmit) {
// No bytes copied since this packet shouldn't be retransmitted or is
// of zero size.
return false;
}
stored_send_times_[index] = clock_->TimeInMilliseconds();
stored_packets_[index].send_time = clock_->TimeInMilliseconds();
GetPacket(index, packet, packet_length, stored_time_ms);
return true;
}
@ -266,13 +219,10 @@ void RTPPacketHistory::GetPacket(int index,
size_t* packet_length,
int64_t* stored_time_ms) const {
// Get packet.
size_t length = stored_lengths_.at(index);
std::vector<std::vector<uint8_t> >::const_iterator it_found_packet =
stored_packets_.begin() + index;
std::copy(it_found_packet->begin(), it_found_packet->begin() + length,
packet);
size_t length = stored_packets_[index].length;
memcpy(packet, stored_packets_[index].data, length);
*packet_length = length;
*stored_time_ms = stored_times_.at(index);
*stored_time_ms = stored_packets_[index].time_ms;
}
bool RTPPacketHistory::GetBestFittingPacket(uint8_t* packet,
@ -294,24 +244,24 @@ bool RTPPacketHistory::FindSeqNum(uint16_t sequence_number,
uint16_t temp_sequence_number = 0;
if (prev_index_ > 0) {
*index = prev_index_ - 1;
temp_sequence_number = stored_seq_nums_[*index];
temp_sequence_number = stored_packets_[*index].sequence_number;
} else {
*index = stored_seq_nums_.size() - 1;
temp_sequence_number = stored_seq_nums_[*index]; // wrap
*index = stored_packets_.size() - 1;
temp_sequence_number = stored_packets_[*index].sequence_number; // wrap
}
int32_t idx = (prev_index_ - 1) - (temp_sequence_number - sequence_number);
if (idx >= 0 && idx < static_cast<int>(stored_seq_nums_.size())) {
if (idx >= 0 && idx < static_cast<int>(stored_packets_.size())) {
*index = idx;
temp_sequence_number = stored_seq_nums_[*index];
temp_sequence_number = stored_packets_[*index].sequence_number;
}
if (temp_sequence_number != sequence_number) {
// We did not found a match, search all.
for (uint16_t m = 0; m < stored_seq_nums_.size(); m++) {
if (stored_seq_nums_[m] == sequence_number) {
for (uint16_t m = 0; m < stored_packets_.size(); m++) {
if (stored_packets_[m].sequence_number == sequence_number) {
*index = m;
temp_sequence_number = stored_seq_nums_[*index];
temp_sequence_number = stored_packets_[*index].sequence_number;
break;
}
}
@ -324,15 +274,16 @@ bool RTPPacketHistory::FindSeqNum(uint16_t sequence_number,
}
int RTPPacketHistory::FindBestFittingPacket(size_t size) const {
if (size < kMinPacketRequestBytes || stored_lengths_.empty())
if (size < kMinPacketRequestBytes || stored_packets_.empty())
return -1;
size_t min_diff = std::numeric_limits<size_t>::max();
int best_index = -1; // Returned unchanged if we don't find anything.
for (size_t i = 0; i < stored_lengths_.size(); ++i) {
if (stored_lengths_[i] == 0)
for (size_t i = 0; i < stored_packets_.size(); ++i) {
if (stored_packets_[i].length == 0)
continue;
size_t diff = (stored_lengths_[i] > size) ?
(stored_lengths_[i] - size) : (size - stored_lengths_[i]);
size_t diff = (stored_packets_[i].length > size)
? (stored_packets_[i].length - size)
: (size - stored_packets_[i].length);
if (diff < min_diff) {
min_diff = diff;
best_index = static_cast<int>(i);
@ -340,4 +291,7 @@ int RTPPacketHistory::FindBestFittingPacket(size_t size) const {
}
return best_index;
}
RTPPacketHistory::StoredPacket::StoredPacket() {}
} // namespace webrtc

View File

@ -39,7 +39,6 @@ class RTPPacketHistory {
// Stores RTP packet.
int32_t PutRTPPacket(const uint8_t* packet,
size_t packet_length,
size_t max_packet_length,
int64_t capture_time_ms,
StorageType type);
@ -88,14 +87,18 @@ class RTPPacketHistory {
rtc::scoped_ptr<CriticalSectionWrapper> critsect_;
bool store_ GUARDED_BY(critsect_);
uint32_t prev_index_ GUARDED_BY(critsect_);
size_t max_packet_length_ GUARDED_BY(critsect_);
std::vector<std::vector<uint8_t> > stored_packets_ GUARDED_BY(critsect_);
std::vector<uint16_t> stored_seq_nums_ GUARDED_BY(critsect_);
std::vector<size_t> stored_lengths_ GUARDED_BY(critsect_);
std::vector<int64_t> stored_times_ GUARDED_BY(critsect_);
std::vector<int64_t> stored_send_times_ GUARDED_BY(critsect_);
std::vector<StorageType> stored_types_ GUARDED_BY(critsect_);
struct StoredPacket {
StoredPacket();
uint16_t sequence_number = 0;
int64_t time_ms = 0;
int64_t send_time = 0;
StorageType storage_type = kDontStore;
uint8_t data[IP_PACKET_SIZE];
size_t length = 0;
};
std::vector<StoredPacket> stored_packets_ GUARDED_BY(critsect_);
};
} // namespace webrtc
#endif // WEBRTC_MODULES_RTP_RTCP_RTP_PACKET_HISTORY_H_

View File

@ -70,8 +70,8 @@ TEST_F(RtpPacketHistoryTest, NoStoreStatus) {
size_t len = 0;
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
// Packet should not be stored.
len = kMaxPacketLength;
int64_t time;
@ -84,8 +84,7 @@ TEST_F(RtpPacketHistoryTest, DontStore) {
size_t len = 0;
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kDontStore));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms, kDontStore));
// Packet should not be stored.
len = kMaxPacketLength;
@ -97,11 +96,8 @@ TEST_F(RtpPacketHistoryTest, DontStore) {
TEST_F(RtpPacketHistoryTest, PutRtpPacket_TooLargePacketLength) {
hist_->SetStorePacketsStatus(true, 10);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
EXPECT_EQ(-1, hist_->PutRTPPacket(packet_,
kMaxPacketLength + 1,
kMaxPacketLength,
capture_time_ms,
kAllowRetransmission));
EXPECT_EQ(-1, hist_->PutRTPPacket(packet_, kMaxPacketLength + 1,
capture_time_ms, kAllowRetransmission));
}
TEST_F(RtpPacketHistoryTest, GetRtpPacket_NotStored) {
@ -119,8 +115,8 @@ TEST_F(RtpPacketHistoryTest, PutRtpPacket) {
EXPECT_FALSE(hist_->HasRTPPacket(kSeqNum));
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
EXPECT_TRUE(hist_->HasRTPPacket(kSeqNum));
}
@ -129,8 +125,8 @@ TEST_F(RtpPacketHistoryTest, GetRtpPacket) {
size_t len = 0;
int64_t capture_time_ms = 1;
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
size_t len_out = kMaxPacketLength;
int64_t time;
@ -149,8 +145,7 @@ TEST_F(RtpPacketHistoryTest, NoCaptureTime) {
fake_clock_.AdvanceTimeMilliseconds(1);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
-1, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, -1, kAllowRetransmission));
size_t len_out = kMaxPacketLength;
int64_t time;
@ -168,8 +163,8 @@ TEST_F(RtpPacketHistoryTest, DontRetransmit) {
size_t len = 0;
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kDontRetransmit));
EXPECT_EQ(
0, hist_->PutRTPPacket(packet_, len, capture_time_ms, kDontRetransmit));
size_t len_out = kMaxPacketLength;
int64_t time;
@ -184,8 +179,8 @@ TEST_F(RtpPacketHistoryTest, MinResendTime) {
size_t len = 0;
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
CreateRtpPacket(kSeqNum, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
int64_t time;
len = kMaxPacketLength;
@ -215,8 +210,8 @@ TEST_F(RtpPacketHistoryTest, DynamicExpansion) {
for (int i = 0; i < 4; ++i) {
len = 0;
CreateRtpPacket(kSeqNum + i, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
}
for (int i = 0; i < 4; ++i) {
len = kMaxPacketLength;
@ -230,8 +225,8 @@ TEST_F(RtpPacketHistoryTest, DynamicExpansion) {
for (int i = 4; i < 20; ++i) {
len = 0;
CreateRtpPacket(kSeqNum + i, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
}
for (int i = 4; i < 20; ++i) {
len = kMaxPacketLength;
@ -257,8 +252,8 @@ TEST_F(RtpPacketHistoryTest, FullExpansion) {
for (size_t i = 0; i < kMaxHistoryCapacity + 1; ++i) {
len = 0;
CreateRtpPacket(kSeqNum + i, kSsrc, kPayload, kTimestamp, packet_, &len);
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
capture_time_ms, kAllowRetransmission));
EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, capture_time_ms,
kAllowRetransmission));
}
fake_clock_.AdvanceTimeMilliseconds(100);

View File

@ -1026,8 +1026,7 @@ int32_t RTPSender::SendToNetwork(
// Used for NACK and to spread out the transmission of packets.
if (packet_history_.PutRTPPacket(buffer, rtp_header_length + payload_length,
max_payload_length_, capture_time_ms,
storage) != 0) {
capture_time_ms, storage) != 0) {
return -1;
}