Add a "Smart flushing" feature to NetEq.
Instead of flushing all packets, it makes sense to flush down to the target level instead. This CL also initiates a flush when the packet buffer is a multiple of the target level, instead of waiting until it is completely full.
Bug: webrtc:12201
Change-Id: I8775147624536824eb88752f6e8ffe57ec6199cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/193941
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32701}
diff --git a/modules/audio_coding/neteq/packet_buffer_unittest.cc b/modules/audio_coding/neteq/packet_buffer_unittest.cc
index 40e7d53..4286006 100644
--- a/modules/audio_coding/neteq/packet_buffer_unittest.cc
+++ b/modules/audio_coding/neteq/packet_buffer_unittest.cc
@@ -19,6 +19,7 @@
#include "modules/audio_coding/neteq/mock/mock_decoder_database.h"
#include "modules/audio_coding/neteq/mock/mock_statistics_calculator.h"
#include "modules/audio_coding/neteq/packet.h"
+#include "test/field_trial.h"
#include "test/gmock.h"
#include "test/gtest.h"
@@ -117,10 +118,16 @@
PacketBuffer buffer(10, &tick_timer); // 10 packets.
PacketGenerator gen(17u, 4711u, 0, 10);
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
const int payload_len = 100;
const Packet packet = gen.NextPacket(payload_len, nullptr);
- EXPECT_EQ(0, buffer.InsertPacket(packet.Clone(), &mock_stats));
+ EXPECT_EQ(0, buffer.InsertPacket(/*packet=*/packet.Clone(),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/10000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
uint32_t next_ts;
EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts));
EXPECT_EQ(4711u, next_ts);
@@ -128,6 +135,7 @@
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
const Packet* next_packet = buffer.PeekNextPacket();
EXPECT_EQ(packet, *next_packet); // Compare contents.
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
// Do not explicitly flush buffer or delete packet to test that it is deleted
// with the buffer. (Tested with Valgrind or similar tool.)
@@ -140,20 +148,28 @@
PacketGenerator gen(0, 0, 0, 10);
const int payload_len = 10;
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
// Insert 10 small packets; should be ok.
for (int i = 0; i < 10; ++i) {
EXPECT_EQ(
PacketBuffer::kOK,
- buffer.InsertPacket(gen.NextPacket(payload_len, nullptr), &mock_stats));
+ buffer.InsertPacket(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
EXPECT_FALSE(buffer.Empty());
- buffer.Flush();
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(10);
+ buffer.Flush(&mock_stats);
// Buffer should delete the payloads itself.
EXPECT_EQ(0u, buffer.NumPacketsInBuffer());
EXPECT_TRUE(buffer.Empty());
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
// Test to fill the buffer over the limits, and verify that it flushes.
@@ -162,6 +178,7 @@
PacketBuffer buffer(10, &tick_timer); // 10 packets.
PacketGenerator gen(0, 0, 0, 10);
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
// Insert 10 small packets; should be ok.
const int payload_len = 10;
@@ -169,7 +186,99 @@
for (i = 0; i < 10; ++i) {
EXPECT_EQ(
PacketBuffer::kOK,
- buffer.InsertPacket(gen.NextPacket(payload_len, nullptr), &mock_stats));
+ buffer.InsertPacket(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
+ }
+ EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
+ uint32_t next_ts;
+ EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts));
+ EXPECT_EQ(0u, next_ts); // Expect first inserted packet to be first in line.
+
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(10);
+ const Packet packet = gen.NextPacket(payload_len, nullptr);
+ // Insert 11th packet; should flush the buffer and insert it after flushing.
+ EXPECT_EQ(PacketBuffer::kFlushed,
+ buffer.InsertPacket(/*packet=*/packet.Clone(),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
+ EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
+ EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts));
+ // Expect last inserted packet to be first in line.
+ EXPECT_EQ(packet.timestamp, next_ts);
+
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
+}
+
+// Test a partial buffer flush.
+TEST(PacketBuffer, PartialFlush) {
+ // Use a field trial to configure smart flushing.
+ test::ScopedFieldTrials field_trials(
+ "WebRTC-Audio-NetEqSmartFlushing/enabled:true,"
+ "target_level_threshold_ms:0,target_level_multiplier:2/");
+ TickTimer tick_timer;
+ PacketBuffer buffer(10, &tick_timer); // 10 packets.
+ PacketGenerator gen(0, 0, 0, 10);
+ const int payload_len = 10;
+ StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
+
+ // Insert 10 small packets; should be ok.
+ for (int i = 0; i < 10; ++i) {
+ EXPECT_EQ(
+ PacketBuffer::kOK,
+ buffer.InsertPacket(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/100,
+ /*decoder_database=*/decoder_database));
+ }
+ EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
+ EXPECT_FALSE(buffer.Empty());
+
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(7);
+ buffer.PartialFlush(/*target_level_ms=*/30,
+ /*sample_rate=*/1000,
+ /*last_decoded_length=*/payload_len,
+ /*stats=*/&mock_stats);
+ // There should still be some packets left in the buffer.
+ EXPECT_EQ(3u, buffer.NumPacketsInBuffer());
+ EXPECT_FALSE(buffer.Empty());
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
+}
+
+// Test to fill the buffer over the limits, and verify that the smart flush
+// functionality works as expected.
+TEST(PacketBuffer, SmartFlushOverfillBuffer) {
+ // Use a field trial to configure smart flushing.
+ test::ScopedFieldTrials field_trials(
+ "WebRTC-Audio-NetEqSmartFlushing/enabled:true,"
+ "target_level_threshold_ms:0,target_level_multiplier:2/");
+ TickTimer tick_timer;
+ PacketBuffer buffer(10, &tick_timer); // 10 packets.
+ PacketGenerator gen(0, 0, 0, 10);
+ StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
+
+ // 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(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/100,
+ /*decoder_database=*/decoder_database));
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
uint32_t next_ts;
@@ -177,16 +286,18 @@
EXPECT_EQ(0u, next_ts); // Expect first inserted packet to be first in line.
const Packet packet = gen.NextPacket(payload_len, nullptr);
- // Insert 11th packet; should flush the buffer and insert it after flushing.
- 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.
- EXPECT_EQ(packet.timestamp, next_ts);
-
- // Flush buffer to delete all packets.
- buffer.Flush();
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(6);
+ // Insert 11th packet; should cause a partial flush and insert the packet
+ // after flushing.
+ EXPECT_EQ(PacketBuffer::kPartialFlush,
+ buffer.InsertPacket(/*packet=*/packet.Clone(),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/40,
+ /*decoder_database=*/decoder_database));
+ EXPECT_EQ(5u, buffer.NumPacketsInBuffer());
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
// Test inserting a list of packets.
@@ -213,16 +324,21 @@
absl::optional<uint8_t> current_pt;
absl::optional<uint8_t> current_cng_pt;
- EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_EQ(
+ PacketBuffer::kOK,
+ buffer.InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
EXPECT_EQ(0, current_pt); // Current payload type changed to 0.
EXPECT_EQ(absl::nullopt, current_cng_pt); // CNG payload type not changed.
- buffer.Flush(); // Clean up.
-
EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
@@ -262,16 +378,22 @@
absl::optional<uint8_t> current_pt;
absl::optional<uint8_t> current_cng_pt;
- EXPECT_EQ(PacketBuffer::kFlushed,
- buffer.InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1)).Times(10);
+ EXPECT_EQ(
+ PacketBuffer::kFlushed,
+ buffer.InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(1u, buffer.NumPacketsInBuffer()); // Only the last packet.
EXPECT_EQ(1, current_pt); // Current payload type changed to 1.
EXPECT_EQ(absl::nullopt, current_cng_pt); // CNG payload type not changed.
- buffer.Flush(); // Clean up.
-
EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
@@ -293,6 +415,7 @@
{0x0005, 0x0000001E, 0, true, -1}, {0x0005, 0x00000014, 1, false, -1},
{0x0006, 0x00000028, 0, true, 8}, {0x0006, 0x0000001E, 1, false, -1},
};
+ MockDecoderDatabase decoder_database;
const size_t kExpectPacketsInBuffer = 9;
@@ -321,7 +444,12 @@
}
EXPECT_CALL(check, Call(i));
EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacket(packet.Clone(), &mock_stats));
+ buffer.InsertPacket(/*packet=*/packet.Clone(),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/kPayloadLength,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
if (packet_facts[i].extract_order >= 0) {
expect_order[packet_facts[i].extract_order] = std::move(packet);
}
@@ -335,6 +463,7 @@
EXPECT_EQ(packet, expect_order[i]); // Compare contents.
}
EXPECT_TRUE(buffer.Empty());
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
TEST(PacketBuffer, DiscardPackets) {
@@ -347,11 +476,17 @@
PacketList list;
const int payload_len = 10;
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
constexpr int kTotalPackets = 10;
// Insert 10 small packets.
for (int i = 0; i < kTotalPackets; ++i) {
- buffer.InsertPacket(gen.NextPacket(payload_len, nullptr), &mock_stats);
+ buffer.InsertPacket(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database);
}
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
@@ -399,6 +534,7 @@
&mock_stats);
EXPECT_TRUE(buffer.Empty());
+ EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
TEST(PacketBuffer, Reordering) {
@@ -434,9 +570,16 @@
StrictMock<MockStatisticsCalculator> mock_stats;
- EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_EQ(
+ PacketBuffer::kOK,
+ buffer.InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_EQ(10u, buffer.NumPacketsInBuffer());
// Extract them and make sure that come out in the right order.
@@ -483,9 +626,16 @@
StrictMock<MockStatisticsCalculator> mock_stats;
- EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_EQ(
+ PacketBuffer::kOK,
+ buffer.InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/kPayloadLen,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_TRUE(list.empty());
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
ASSERT_TRUE(buffer.PeekNextPacket());
@@ -501,9 +651,17 @@
}
// Expect the buffer to flush out the CNG packet, since it does not match the
// new speech sample rate.
- EXPECT_EQ(PacketBuffer::kFlushed,
- buffer.InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_CALL(mock_stats, PacketsDiscarded(1));
+ EXPECT_EQ(
+ PacketBuffer::kFlushed,
+ buffer.InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/kPayloadLen,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_TRUE(list.empty());
EXPECT_EQ(1u, buffer.NumPacketsInBuffer());
ASSERT_TRUE(buffer.PeekNextPacket());
@@ -512,7 +670,6 @@
EXPECT_EQ(kSpeechPt, current_pt); // Current payload type set.
EXPECT_EQ(absl::nullopt, current_cng_pt); // CNG payload type reset.
- buffer.Flush(); // Clean up.
EXPECT_CALL(decoder_database, Die()); // Called when object is deleted.
}
@@ -524,13 +681,19 @@
PacketGenerator gen(start_seq_no, start_ts, 0, ts_increment);
TickTimer tick_timer;
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
PacketBuffer* buffer = new PacketBuffer(100, &tick_timer); // 100 packets.
{
Packet packet = gen.NextPacket(payload_len, nullptr);
packet.payload.Clear();
EXPECT_EQ(PacketBuffer::kInvalidPacket,
- buffer->InsertPacket(std::move(packet), &mock_stats));
+ buffer->InsertPacket(/*packet=*/std::move(packet),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
}
// Buffer should still be empty. Test all empty-checks.
uint32_t temp_ts;
@@ -548,7 +711,12 @@
// Insert one packet to make the buffer non-empty.
EXPECT_EQ(
PacketBuffer::kOK,
- buffer->InsertPacket(gen.NextPacket(payload_len, nullptr), &mock_stats));
+ buffer->InsertPacket(/*packet=*/gen.NextPacket(payload_len, nullptr),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
EXPECT_EQ(PacketBuffer::kInvalidPointer, buffer->NextTimestamp(NULL));
EXPECT_EQ(PacketBuffer::kInvalidPointer,
buffer->NextHigherTimestamp(0, NULL));
@@ -566,7 +734,6 @@
list.push_back(std::move(packet));
}
list.push_back(gen.NextPacket(payload_len, nullptr)); // Valid packet.
- MockDecoderDatabase decoder_database;
auto factory = CreateBuiltinAudioDecoderFactory();
const DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1),
absl::nullopt, factory);
@@ -574,9 +741,16 @@
.WillRepeatedly(Return(&info));
absl::optional<uint8_t> current_pt;
absl::optional<uint8_t> current_cng_pt;
- EXPECT_EQ(PacketBuffer::kInvalidPacket,
- buffer->InsertPacketList(&list, decoder_database, ¤t_pt,
- ¤t_cng_pt, &mock_stats));
+ EXPECT_EQ(
+ PacketBuffer::kInvalidPacket,
+ buffer->InsertPacketList(/*packet_list=*/&list,
+ /*decoder_database=*/decoder_database,
+ /*current_rtp_payload_type=*/¤t_pt,
+ /*current_cng_rtp_payload_type=*/¤t_cng_pt,
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/payload_len,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/30));
EXPECT_TRUE(list.empty()); // The PacketBuffer should have depleted the list.
EXPECT_EQ(1u, buffer->NumPacketsInBuffer());
delete buffer;
@@ -702,6 +876,7 @@
PacketBuffer buffer(3, &tick_timer);
PacketGenerator gen(0, kStartTimeStamp, 0, kFrameSizeSamples);
StrictMock<MockStatisticsCalculator> mock_stats;
+ MockDecoderDatabase decoder_database;
Packet packet_1 = gen.NextPacket(kPayloadSizeBytes, nullptr);
@@ -716,7 +891,12 @@
packet_2.timestamp); // Tmestamp wrapped around.
EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacket(std::move(packet_1), &mock_stats));
+ buffer.InsertPacket(/*packet=*/std::move(packet_1),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/kFrameSizeSamples,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
constexpr size_t kLastDecodedSizeSamples = 2;
// packet_1 has no access to duration, and relies last decoded duration as
@@ -726,7 +906,12 @@
KCountDtxWaitingTime));
EXPECT_EQ(PacketBuffer::kOK,
- buffer.InsertPacket(std::move(packet_2), &mock_stats));
+ buffer.InsertPacket(/*packet=*/std::move(packet_2),
+ /*stats=*/&mock_stats,
+ /*last_decoded_length=*/kFrameSizeSamples,
+ /*sample_rate=*/1000,
+ /*target_level_ms=*/60,
+ /*decoder_database=*/decoder_database));
EXPECT_EQ(kFrameSizeSamples * 2,
buffer.GetSpanSamples(0, kSampleRateHz, KCountDtxWaitingTime));