Correct the calculation of discard rate.

Bug: webrtc:7903
Change-Id: Ib5d6fd882a994dd542b616e5fe1c75710346dd31
Reviewed-on: https://chromium-review.googlesource.com/575057
Commit-Queue: Minyue Li <minyue@webrtc.org>
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19101}
This commit is contained in:
minyue-webrtc
2017-07-19 11:44:06 +02:00
committed by Commit Bot
parent 5ed09048f8
commit 12d30840d8
8 changed files with 85 additions and 49 deletions

View File

@ -27,19 +27,20 @@ class MockPacketBuffer : public PacketBuffer {
void());
MOCK_CONST_METHOD0(Empty,
bool());
int InsertPacket(Packet&& packet) {
return InsertPacketWrapped(&packet);
int InsertPacket(Packet&& packet, StatisticsCalculator* stats) {
return InsertPacketWrapped(&packet, stats);
}
// Since gtest does not properly support move-only types, InsertPacket is
// implemented as a wrapper. You'll have to implement InsertPacketWrapped
// instead and move from |*packet|.
MOCK_METHOD1(InsertPacketWrapped,
int(Packet* packet));
MOCK_METHOD4(InsertPacketList,
int(PacketList* packet_list,
const DecoderDatabase& decoder_database,
rtc::Optional<uint8_t>* current_rtp_payload_type,
rtc::Optional<uint8_t>* current_cng_rtp_payload_type));
MOCK_METHOD2(InsertPacketWrapped,
int(Packet* packet, StatisticsCalculator* stats));
MOCK_METHOD5(InsertPacketList,
int(PacketList* packet_list,
const DecoderDatabase& decoder_database,
rtc::Optional<uint8_t>* current_rtp_payload_type,
rtc::Optional<uint8_t>* current_cng_rtp_payload_type,
StatisticsCalculator* stats));
MOCK_CONST_METHOD1(NextTimestamp,
int(uint32_t* next_timestamp));
MOCK_CONST_METHOD2(NextHigherTimestamp,

View File

@ -728,7 +728,7 @@ int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header,
packet_buffer_->NumPacketsInBuffer();
const int ret = packet_buffer_->InsertPacketList(
&parsed_packet_list, *decoder_database_, &current_rtp_payload_type_,
&current_cng_rtp_payload_type_);
&current_cng_rtp_payload_type_, &stats_);
if (ret == PacketBuffer::kFlushed) {
// Reset DSP timestamp etc. if packet buffer flushed.
new_codec_ = true;

View File

@ -350,7 +350,7 @@ TEST_F(NetEqImplTest, InsertPacket) {
.WillOnce(Return(false)); // Called once after first packet is inserted.
EXPECT_CALL(*mock_packet_buffer_, Flush())
.Times(1);
EXPECT_CALL(*mock_packet_buffer_, InsertPacketList(_, _, _, _))
EXPECT_CALL(*mock_packet_buffer_, InsertPacketList(_, _, _, _, _))
.Times(2)
.WillRepeatedly(
DoAll(SetArgPointee<2>(rtc::Optional<uint8_t>(kPayloadType)),

View File

@ -273,6 +273,7 @@ NetEqNetworkStatsTest(NetEqDecoder codec,
expects.stats_ref.packet_loss_rate = 0;
expects.stats_ref.expand_rate = expects.stats_ref.speech_expand_rate = 0;
expects.stats_ref.secondary_decoded_rate = 2006;
expects.stats_ref.packet_discard_rate = 13374;
RunTest(50, expects);
}

View File

@ -482,10 +482,10 @@ TEST_F(NetEqDecodingTest, MAYBE_TestOpusBitExactness) {
"6237dd113ad80d7764fe4c90b55b2ec035eae64e");
const std::string network_stats_checksum =
PlatformChecksum("7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
"7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
"7a29daca4dfa4fb6eaec06d7e63c1729da7708f6",
"7a29daca4dfa4fb6eaec06d7e63c1729da7708f6");
PlatformChecksum("dda4cee006d9369c7114a03790c5761346cf5e23",
"dda4cee006d9369c7114a03790c5761346cf5e23",
"dda4cee006d9369c7114a03790c5761346cf5e23",
"dda4cee006d9369c7114a03790c5761346cf5e23");
const std::string rtcp_stats_checksum = PlatformChecksum(
"e37c797e3de6a64dda88c9ade7a013d022a2e1e0",

View File

@ -68,7 +68,7 @@ bool PacketBuffer::Empty() const {
return buffer_.empty();
}
int PacketBuffer::InsertPacket(Packet&& packet) {
int PacketBuffer::InsertPacket(Packet&& packet, StatisticsCalculator* stats) {
if (packet.empty()) {
LOG(LS_WARNING) << "InsertPacket invalid packet";
return kInvalidPacket;
@ -99,6 +99,8 @@ int PacketBuffer::InsertPacket(Packet&& packet) {
// timestamp as |rit|, which has a higher priority, do not insert the new
// packet to list.
if (rit != buffer_.rend() && packet.timestamp == rit->timestamp) {
RTC_CHECK(stats);
stats->PacketsDiscarded(1);
return return_val;
}
@ -108,6 +110,8 @@ int PacketBuffer::InsertPacket(Packet&& packet) {
PacketList::iterator it = rit.base();
if (it != buffer_.end() && packet.timestamp == it->timestamp) {
it = buffer_.erase(it);
RTC_CHECK(stats);
stats->PacketsDiscarded(1);
}
buffer_.insert(it, std::move(packet)); // Insert the packet at that position.
@ -118,7 +122,9 @@ int PacketBuffer::InsertPacketList(
PacketList* packet_list,
const DecoderDatabase& decoder_database,
rtc::Optional<uint8_t>* current_rtp_payload_type,
rtc::Optional<uint8_t>* current_cng_rtp_payload_type) {
rtc::Optional<uint8_t>* current_cng_rtp_payload_type,
StatisticsCalculator* stats) {
RTC_DCHECK(stats);
bool flushed = false;
for (auto& packet : *packet_list) {
if (decoder_database.IsComfortNoise(packet.payload_type)) {
@ -145,7 +151,7 @@ int PacketBuffer::InsertPacketList(
}
*current_rtp_payload_type = rtc::Optional<uint8_t>(packet.payload_type);
}
int return_val = InsertPacket(std::move(packet));
int return_val = InsertPacket(std::move(packet), stats);
if (return_val == kFlushed) {
// The buffer flushed, but this is not an error. We can still continue.
flushed = true;
@ -214,6 +220,7 @@ int PacketBuffer::DiscardNextPacket(StatisticsCalculator* stats) {
// Assert that the packet sanity checks in InsertPacket method works.
RTC_DCHECK(!buffer_.front().empty());
buffer_.pop_front();
RTC_CHECK(stats);
stats->PacketsDiscarded(1);
return kOK;
}
@ -227,6 +234,7 @@ void PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit,
IsObsoleteTimestamp(p.timestamp, timestamp_limit, horizon_samples);
});
if (old_size > buffer_.size()) {
RTC_CHECK(stats);
stats->PacketsDiscarded(old_size - buffer_.size());
}
}
@ -248,8 +256,10 @@ void PacketBuffer::DiscardPacketsWithPayloadType(uint8_t payload_type,
++it;
}
}
if (packets_discarded > 0)
if (packets_discarded > 0) {
RTC_CHECK(stats);
stats->PacketsDiscarded(packets_discarded);
}
}
size_t PacketBuffer::NumPacketsInBuffer() const {

View File

@ -52,7 +52,7 @@ class PacketBuffer {
// the packet object.
// Returns PacketBuffer::kOK on success, PacketBuffer::kFlushed if the buffer
// was flushed due to overfilling.
virtual int InsertPacket(Packet&& packet);
virtual int InsertPacket(Packet&& packet, StatisticsCalculator* stats);
// Inserts a list of packets into the buffer. The buffer will take over
// ownership of the packet objects.
@ -66,7 +66,8 @@ class PacketBuffer {
PacketList* packet_list,
const DecoderDatabase& decoder_database,
rtc::Optional<uint8_t>* current_rtp_payload_type,
rtc::Optional<uint8_t>* current_cng_rtp_payload_type);
rtc::Optional<uint8_t>* current_cng_rtp_payload_type,
StatisticsCalculator* stats);
// Gets the timestamp for the first packet in the buffer and writes it to the
// output variable |next_timestamp|.

View File

@ -89,10 +89,11 @@ TEST(PacketBuffer, InsertPacket) {
TickTimer tick_timer;
PacketBuffer buffer(10, &tick_timer); // 10 packets.
PacketGenerator gen(17u, 4711u, 0, 10);
StrictMock<MockStatisticsCalculator> mock_stats;
const int payload_len = 100;
const Packet packet = gen.NextPacket(payload_len);
EXPECT_EQ(0, buffer.InsertPacket(packet.Clone()));
EXPECT_EQ(0, buffer.InsertPacket(packet.Clone(), &mock_stats));
uint32_t next_ts;
EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts));
EXPECT_EQ(4711u, next_ts);
@ -111,11 +112,12 @@ TEST(PacketBuffer, FlushBuffer) {
PacketBuffer buffer(10, &tick_timer); // 10 packets.
PacketGenerator gen(0, 0, 0, 10);
const int payload_len = 10;
StrictMock<MockStatisticsCalculator> mock_stats;
// Insert 10 small packets; should be ok.
for (int i = 0; i < 10; ++i) {
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacket(gen.NextPacket(payload_len)));
buffer.InsertPacket(gen.NextPacket(payload_len), &mock_stats));
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
EXPECT_FALSE(buffer.Empty());
@ -131,13 +133,14 @@ TEST(PacketBuffer, OverfillBuffer) {
TickTimer tick_timer;
PacketBuffer buffer(10, &tick_timer); // 10 packets.
PacketGenerator gen(0, 0, 0, 10);
StrictMock<MockStatisticsCalculator> mock_stats;
// Insert 10 small packets; should be ok.
const int payload_len = 10;
int i;
for (i = 0; i < 10; ++i) {
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacket(gen.NextPacket(payload_len)));
buffer.InsertPacket(gen.NextPacket(payload_len), &mock_stats));
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
uint32_t next_ts;
@ -146,7 +149,8 @@ TEST(PacketBuffer, OverfillBuffer) {
const Packet packet = gen.NextPacket(payload_len);
// Insert 11th packet; should flush the buffer and insert it after flushing.
EXPECT_EQ(PacketBuffer::kFlushed, buffer.InsertPacket(packet.Clone()));
EXPECT_EQ(PacketBuffer::kFlushed,
buffer.InsertPacket(packet.Clone(), &mock_stats));
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts));
// Expect last inserted packet to be first in line.
@ -174,12 +178,14 @@ TEST(PacketBuffer, InsertPacketList) {
const DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, factory);
EXPECT_CALL(decoder_database, GetDecoderInfo(0))
.WillRepeatedly(Return(&info));
StrictMock<MockStatisticsCalculator> mock_stats;
rtc::Optional<uint8_t> current_pt;
rtc::Optional<uint8_t> current_cng_pt;
EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacketList(&list,
decoder_database,
&current_pt,
&current_cng_pt));
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt, &mock_stats));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
EXPECT_EQ(rtc::Optional<uint8_t>(0),
@ -220,12 +226,14 @@ TEST(PacketBuffer, InsertPacketListChangePayloadType) {
const DecoderDatabase::DecoderInfo info1(NetEqDecoder::kDecoderPCMa, factory);
EXPECT_CALL(decoder_database, GetDecoderInfo(1))
.WillRepeatedly(Return(&info1));
StrictMock<MockStatisticsCalculator> mock_stats;
rtc::Optional<uint8_t> current_pt;
rtc::Optional<uint8_t> current_cng_pt;
EXPECT_EQ(PacketBuffer::kFlushed, buffer.InsertPacketList(&list,
decoder_database,
&current_pt,
&current_cng_pt));
EXPECT_EQ(PacketBuffer::kFlushed,
buffer.InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt, &mock_stats));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(1u, buffer.NumPacketsInBuffer()); // Only the last packet.
EXPECT_EQ(rtc::Optional<uint8_t>(1),
@ -271,6 +279,13 @@ TEST(PacketBuffer, ExtractOrderRedundancy) {
PacketGenerator gen(0, 0, 0, kFrameSize);
StrictMock<MockStatisticsCalculator> mock_stats;
// Interleaving the EXPECT_CALL sequence with expectations on the MockFunction
// check ensures that exactly one call to PacketsDiscarded happens in each
// DiscardNextPacket call.
InSequence s;
MockFunction<void(int check_point_id)> check;
for (int i = 0; i < kPackets; ++i) {
gen.Reset(packet_facts[i].sequence_number,
packet_facts[i].timestamp,
@ -278,10 +293,16 @@ TEST(PacketBuffer, ExtractOrderRedundancy) {
kFrameSize);
Packet packet = gen.NextPacket(kPayloadLength);
packet.priority.red_level = packet_facts[i].primary ? 0 : 1;
EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacket(packet.Clone()));
if (packet_facts[i].extract_order < 0) {
EXPECT_CALL(mock_stats, PacketsDiscarded(1));
}
EXPECT_CALL(check, Call(i));
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacket(packet.Clone(), &mock_stats));
if (packet_facts[i].extract_order >= 0) {
expect_order[packet_facts[i].extract_order] = std::move(packet);
}
check.Call(i);
}
EXPECT_EQ(kExpectPacketsInBuffer, buffer.NumPacketsInBuffer());
@ -302,15 +323,15 @@ TEST(PacketBuffer, DiscardPackets) {
PacketGenerator gen(start_seq_no, start_ts, 0, ts_increment);
PacketList list;
const int payload_len = 10;
StrictMock<MockStatisticsCalculator> mock_stats;
constexpr int kTotalPackets = 10;
// Insert 10 small packets.
for (int i = 0; i < kTotalPackets; ++i) {
buffer.InsertPacket(gen.NextPacket(payload_len));
buffer.InsertPacket(gen.NextPacket(payload_len), &mock_stats);
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
StrictMock<MockStatisticsCalculator> mock_stats;
uint32_t current_ts = start_ts;
// Discard them one by one and make sure that the right packets are at the
@ -386,10 +407,11 @@ TEST(PacketBuffer, Reordering) {
rtc::Optional<uint8_t> current_pt;
rtc::Optional<uint8_t> current_cng_pt;
EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacketList(&list,
decoder_database,
&current_pt,
&current_cng_pt));
StrictMock<MockStatisticsCalculator> mock_stats;
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt, &mock_stats));
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
// Extract them and make sure that come out in the right order.
@ -433,9 +455,12 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) {
list.push_back(gen.NextPacket(kPayloadLen));
rtc::Optional<uint8_t> current_pt;
rtc::Optional<uint8_t> current_cng_pt;
StrictMock<MockStatisticsCalculator> mock_stats;
EXPECT_EQ(PacketBuffer::kOK,
buffer.InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt));
&current_cng_pt, &mock_stats));
EXPECT_TRUE(list.empty());
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
ASSERT_TRUE(buffer.PeekNextPacket());
@ -454,7 +479,7 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) {
// new speech sample rate.
EXPECT_EQ(PacketBuffer::kFlushed,
buffer.InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt));
&current_cng_pt, &mock_stats));
EXPECT_TRUE(list.empty());
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
ASSERT_TRUE(buffer.PeekNextPacket());
@ -475,13 +500,14 @@ TEST(PacketBuffer, Failures) {
int payload_len = 100;
PacketGenerator gen(start_seq_no, start_ts, 0, ts_increment);
TickTimer tick_timer;
StrictMock<MockStatisticsCalculator> mock_stats;
PacketBuffer* buffer = new PacketBuffer(100, &tick_timer); // 100 packets.
{
Packet packet = gen.NextPacket(payload_len);
packet.payload.Clear();
EXPECT_EQ(PacketBuffer::kInvalidPacket,
buffer->InsertPacket(std::move(packet)));
buffer->InsertPacket(std::move(packet), &mock_stats));
}
// Buffer should still be empty. Test all empty-checks.
uint32_t temp_ts;
@ -491,7 +517,6 @@ TEST(PacketBuffer, Failures) {
EXPECT_EQ(NULL, buffer->PeekNextPacket());
EXPECT_FALSE(buffer->GetNextPacket());
StrictMock<MockStatisticsCalculator> mock_stats;
// Discarding packets will not invoke mock_stats.PacketDiscarded() because the
// packet buffer is empty.
EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->DiscardNextPacket(&mock_stats));
@ -499,7 +524,7 @@ TEST(PacketBuffer, Failures) {
// Insert one packet to make the buffer non-empty.
EXPECT_EQ(PacketBuffer::kOK,
buffer->InsertPacket(gen.NextPacket(payload_len)));
buffer->InsertPacket(gen.NextPacket(payload_len), &mock_stats));
EXPECT_EQ(PacketBuffer::kInvalidPointer, buffer->NextTimestamp(NULL));
EXPECT_EQ(PacketBuffer::kInvalidPointer,
buffer->NextHigherTimestamp(0, NULL));
@ -525,10 +550,8 @@ TEST(PacketBuffer, Failures) {
rtc::Optional<uint8_t> current_pt;
rtc::Optional<uint8_t> current_cng_pt;
EXPECT_EQ(PacketBuffer::kInvalidPacket,
buffer->InsertPacketList(&list,
decoder_database,
&current_pt,
&current_cng_pt));
buffer->InsertPacketList(&list, decoder_database, &current_pt,
&current_cng_pt, &mock_stats));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(1u, buffer->NumPacketsInBuffer());
delete buffer;