From 7b463c5f671e91b2991ca654f4e3e5152f55d23d Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Wed, 25 Nov 2020 11:32:40 +0100 Subject: [PATCH] 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 Reviewed-by: Jakob Ivarsson Cr-Commit-Position: refs/heads/master@{#32701} --- api/neteq/neteq_controller.h | 1 + .../audio_coding/neteq/buffer_level_filter.cc | 4 + .../audio_coding/neteq/buffer_level_filter.h | 5 + modules/audio_coding/neteq/decision_logic.cc | 8 +- modules/audio_coding/neteq/decision_logic.h | 1 + .../neteq/mock/mock_neteq_controller.h | 1 + .../neteq/mock/mock_packet_buffer.h | 21 +- modules/audio_coding/neteq/neteq_impl.cc | 17 +- .../audio_coding/neteq/neteq_impl_unittest.cc | 4 +- modules/audio_coding/neteq/packet_buffer.cc | 108 ++++++- modules/audio_coding/neteq/packet_buffer.h | 30 +- .../neteq/packet_buffer_unittest.cc | 273 +++++++++++++++--- 12 files changed, 403 insertions(+), 70 deletions(-) diff --git a/api/neteq/neteq_controller.h b/api/neteq/neteq_controller.h index 2c09c3e15f..4c49a0c24a 100644 --- a/api/neteq/neteq_controller.h +++ b/api/neteq/neteq_controller.h @@ -103,6 +103,7 @@ class NetEqController { uint16_t main_sequence_number; bool is_cng_or_dtmf; bool is_dtx; + bool buffer_flush; }; virtual ~NetEqController() = default; diff --git a/modules/audio_coding/neteq/buffer_level_filter.cc b/modules/audio_coding/neteq/buffer_level_filter.cc index 7ad006545d..5d503e9918 100644 --- a/modules/audio_coding/neteq/buffer_level_filter.cc +++ b/modules/audio_coding/neteq/buffer_level_filter.cc @@ -45,6 +45,10 @@ void BufferLevelFilter::Update(size_t buffer_size_samples, filtered_current_level - (int64_t{time_stretched_samples} * (1 << 8)))); } +void BufferLevelFilter::SetFilteredBufferLevel(int buffer_size_samples) { + filtered_current_level_ = buffer_size_samples * 256; +} + void BufferLevelFilter::SetTargetBufferLevel(int target_buffer_level_ms) { if (target_buffer_level_ms <= 20) { level_factor_ = 251; diff --git a/modules/audio_coding/neteq/buffer_level_filter.h b/modules/audio_coding/neteq/buffer_level_filter.h index bb3185667c..89fcaf4612 100644 --- a/modules/audio_coding/neteq/buffer_level_filter.h +++ b/modules/audio_coding/neteq/buffer_level_filter.h @@ -28,6 +28,11 @@ class BufferLevelFilter { // bypassing the filter operation). virtual void Update(size_t buffer_size_samples, int time_stretched_samples); + // Set the filtered buffer level to a particular value directly. This should + // only be used in case of large changes in buffer size, such as buffer + // flushes. + virtual void SetFilteredBufferLevel(int buffer_size_samples); + // The target level is used to select the appropriate filter coefficient. virtual void SetTargetBufferLevel(int target_buffer_level_ms); diff --git a/modules/audio_coding/neteq/decision_logic.cc b/modules/audio_coding/neteq/decision_logic.cc index 9c0ee96824..266e675148 100644 --- a/modules/audio_coding/neteq/decision_logic.cc +++ b/modules/audio_coding/neteq/decision_logic.cc @@ -211,6 +211,7 @@ absl::optional DecisionLogic::PacketArrived( int fs_hz, bool should_update_stats, const PacketArrivedInfo& info) { + buffer_flush_ = buffer_flush_ || info.buffer_flush; if (info.is_cng_or_dtmf) { last_pack_cng_or_dtmf_ = true; return absl::nullopt; @@ -238,7 +239,12 @@ void DecisionLogic::FilterBufferLevel(size_t buffer_size_samples) { timescale_countdown_ = tick_timer_->GetNewCountdown(kMinTimescaleInterval); } - buffer_level_filter_->Update(buffer_size_samples, time_stretched_samples); + if (buffer_flush_) { + buffer_level_filter_->SetFilteredBufferLevel(buffer_size_samples); + buffer_flush_ = false; + } else { + buffer_level_filter_->Update(buffer_size_samples, time_stretched_samples); + } prev_time_scale_ = false; time_stretched_cn_samples_ = 0; } diff --git a/modules/audio_coding/neteq/decision_logic.h b/modules/audio_coding/neteq/decision_logic.h index 08feba64db..8be4511419 100644 --- a/modules/audio_coding/neteq/decision_logic.h +++ b/modules/audio_coding/neteq/decision_logic.h @@ -188,6 +188,7 @@ class DecisionLogic : public NetEqController { int num_consecutive_expands_ = 0; int time_stretched_cn_samples_ = 0; bool last_pack_cng_or_dtmf_ = true; + bool buffer_flush_ = false; FieldTrialParameter estimate_dtx_delay_; FieldTrialParameter time_stretch_cn_; FieldTrialConstrained target_level_window_ms_; diff --git a/modules/audio_coding/neteq/mock/mock_neteq_controller.h b/modules/audio_coding/neteq/mock/mock_neteq_controller.h index fdfdbb4d1b..6d88e09216 100644 --- a/modules/audio_coding/neteq/mock/mock_neteq_controller.h +++ b/modules/audio_coding/neteq/mock/mock_neteq_controller.h @@ -48,6 +48,7 @@ class MockNetEqController : public NetEqController { bool should_update_stats, const PacketArrivedInfo& info), (override)); + MOCK_METHOD(void, NotifyMutedState, (), (override)); MOCK_METHOD(bool, PeakFound, (), (const, override)); MOCK_METHOD(int, GetFilteredBufferLevel, (), (const, override)); MOCK_METHOD(void, set_sample_memory, (int32_t value), (override)); diff --git a/modules/audio_coding/neteq/mock/mock_packet_buffer.h b/modules/audio_coding/neteq/mock/mock_packet_buffer.h index e466ea6c8b..48357ea466 100644 --- a/modules/audio_coding/neteq/mock/mock_packet_buffer.h +++ b/modules/audio_coding/neteq/mock/mock_packet_buffer.h @@ -22,11 +22,23 @@ class MockPacketBuffer : public PacketBuffer { : PacketBuffer(max_number_of_packets, tick_timer) {} ~MockPacketBuffer() override { Die(); } MOCK_METHOD(void, Die, ()); - MOCK_METHOD(void, Flush, (), (override)); + MOCK_METHOD(void, Flush, (StatisticsCalculator * stats), (override)); + MOCK_METHOD(void, + PartialFlush, + (int target_level_ms, + size_t sample_rate, + size_t last_decoded_length, + StatisticsCalculator* stats), + (override)); MOCK_METHOD(bool, Empty, (), (const, override)); MOCK_METHOD(int, InsertPacket, - (Packet && packet, StatisticsCalculator* stats), + (Packet && packet, + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms, + const DecoderDatabase& decoder_database), (override)); MOCK_METHOD(int, InsertPacketList, @@ -34,7 +46,10 @@ class MockPacketBuffer : public PacketBuffer { const DecoderDatabase& decoder_database, absl::optional* current_rtp_payload_type, absl::optional* current_cng_rtp_payload_type, - StatisticsCalculator* stats), + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms), (override)); MOCK_METHOD(int, NextTimestamp, diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index f8d5d9dc16..9ec7bd5bca 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -499,7 +499,7 @@ absl::optional NetEqImpl::GetDecoderFormat( void NetEqImpl::FlushBuffers() { MutexLock lock(&mutex_); RTC_LOG(LS_VERBOSE) << "FlushBuffers"; - packet_buffer_->Flush(); + packet_buffer_->Flush(stats_.get()); assert(sync_buffer_.get()); assert(expand_.get()); sync_buffer_->Flush(); @@ -607,7 +607,7 @@ int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header, // the packet has been successfully inserted into the packet buffer. // Flush the packet buffer and DTMF buffer. - packet_buffer_->Flush(); + packet_buffer_->Flush(stats_.get()); dtmf_buffer_->Flush(); // Update audio buffer timestamp. @@ -746,13 +746,23 @@ int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header, } // Insert packets in buffer. + const int target_level_ms = controller_->TargetLevelMs(); const int ret = packet_buffer_->InsertPacketList( &parsed_packet_list, *decoder_database_, ¤t_rtp_payload_type_, - ¤t_cng_rtp_payload_type_, stats_.get()); + ¤t_cng_rtp_payload_type_, stats_.get(), decoder_frame_length_, + last_output_sample_rate_hz_, target_level_ms); + bool buffer_flush_occured = false; if (ret == PacketBuffer::kFlushed) { // Reset DSP timestamp etc. if packet buffer flushed. new_codec_ = true; update_sample_rate_and_channels = true; + buffer_flush_occured = true; + } else if (ret == PacketBuffer::kPartialFlush) { + // Forward sync buffer timestamp + timestamp_ = packet_buffer_->PeekNextPacket()->timestamp; + sync_buffer_->IncreaseEndTimestamp(timestamp_ - + sync_buffer_->end_timestamp()); + buffer_flush_occured = true; } else if (ret != PacketBuffer::kOK) { return kOtherError; } @@ -810,6 +820,7 @@ int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header, info.main_timestamp = main_timestamp; info.main_sequence_number = main_sequence_number; info.is_dtx = is_dtx; + info.buffer_flush = buffer_flush_occured; // Only update statistics if incoming packet is not older than last played // out packet or RTX handling is enabled, and if new codec flag is not // set. diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index 28dd8f0857..c66a0e25f9 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -328,8 +328,8 @@ TEST_F(NetEqImplTest, InsertPacket) { // Expectations for packet buffer. EXPECT_CALL(*mock_packet_buffer_, Empty()) .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_, Flush(_)).Times(1); + EXPECT_CALL(*mock_packet_buffer_, InsertPacketList(_, _, _, _, _, _, _, _)) .Times(2) .WillRepeatedly(DoAll(SetArgPointee<2>(kPayloadType), WithArg<0>(Invoke(DeletePacketsAndReturnOk)))); diff --git a/modules/audio_coding/neteq/packet_buffer.cc b/modules/audio_coding/neteq/packet_buffer.cc index 059308f7fe..86ae8475ce 100644 --- a/modules/audio_coding/neteq/packet_buffer.cc +++ b/modules/audio_coding/neteq/packet_buffer.cc @@ -25,8 +25,10 @@ #include "modules/audio_coding/neteq/decoder_database.h" #include "modules/audio_coding/neteq/statistics_calculator.h" #include "rtc_base/checks.h" +#include "rtc_base/experiments/struct_parameters_parser.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace { @@ -61,27 +63,80 @@ void LogPacketDiscarded(int codec_level, StatisticsCalculator* stats) { } } +absl::optional GetSmartflushingConfig() { + absl::optional result; + std::string field_trial_string = + field_trial::FindFullName("WebRTC-Audio-NetEqSmartFlushing"); + result = SmartFlushingConfig(); + bool enabled = false; + auto parser = StructParametersParser::Create( + "enabled", &enabled, "target_level_threshold_ms", + &result->target_level_threshold_ms, "target_level_multiplier", + &result->target_level_multiplier); + parser->Parse(field_trial_string); + if (!enabled) { + return absl::nullopt; + } + RTC_LOG(LS_INFO) << "Using smart flushing, target_level_threshold_ms: " + << result->target_level_threshold_ms + << ", target_level_multiplier: " + << result->target_level_multiplier; + return result; +} + } // namespace PacketBuffer::PacketBuffer(size_t max_number_of_packets, const TickTimer* tick_timer) - : max_number_of_packets_(max_number_of_packets), tick_timer_(tick_timer) {} + : smart_flushing_config_(GetSmartflushingConfig()), + max_number_of_packets_(max_number_of_packets), + tick_timer_(tick_timer) {} // Destructor. All packets in the buffer will be destroyed. PacketBuffer::~PacketBuffer() { - Flush(); + buffer_.clear(); } // Flush the buffer. All packets in the buffer will be destroyed. -void PacketBuffer::Flush() { +void PacketBuffer::Flush(StatisticsCalculator* stats) { + for (auto& p : buffer_) { + LogPacketDiscarded(p.priority.codec_level, stats); + } buffer_.clear(); + stats->FlushedPacketBuffer(); +} + +void PacketBuffer::PartialFlush(int target_level_ms, + size_t sample_rate, + size_t last_decoded_length, + StatisticsCalculator* stats) { + // Make sure that at least half the packet buffer capacity will be available + // after the flush. This is done to avoid getting stuck if the target level is + // very high. + int target_level_samples = + std::min(target_level_ms * sample_rate / 1000, + max_number_of_packets_ * last_decoded_length / 2); + // We should avoid flushing to very low levels. + target_level_samples = std::max( + target_level_samples, smart_flushing_config_->target_level_threshold_ms); + while (GetSpanSamples(last_decoded_length, sample_rate, true) > + static_cast(target_level_samples) || + buffer_.size() > max_number_of_packets_ / 2) { + LogPacketDiscarded(PeekNextPacket()->priority.codec_level, stats); + buffer_.pop_front(); + } } bool PacketBuffer::Empty() const { return buffer_.empty(); } -int PacketBuffer::InsertPacket(Packet&& packet, StatisticsCalculator* stats) { +int PacketBuffer::InsertPacket(Packet&& packet, + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms, + const DecoderDatabase& decoder_database) { if (packet.empty()) { RTC_LOG(LS_WARNING) << "InsertPacket invalid packet"; return kInvalidPacket; @@ -94,12 +149,32 @@ int PacketBuffer::InsertPacket(Packet&& packet, StatisticsCalculator* stats) { packet.waiting_time = tick_timer_->GetNewStopwatch(); - if (buffer_.size() >= max_number_of_packets_) { - // Buffer is full. Flush it. - Flush(); - stats->FlushedPacketBuffer(); - RTC_LOG(LS_WARNING) << "Packet buffer flushed"; - return_val = kFlushed; + // Perform a smart flush if the buffer size exceeds a multiple of the target + // level. + const size_t span_threshold = + smart_flushing_config_ + ? smart_flushing_config_->target_level_multiplier * + std::max(smart_flushing_config_->target_level_threshold_ms, + target_level_ms) * + sample_rate / 1000 + : 0; + const bool smart_flush = + smart_flushing_config_.has_value() && + GetSpanSamples(last_decoded_length, sample_rate, true) >= span_threshold; + if (buffer_.size() >= max_number_of_packets_ || smart_flush) { + size_t buffer_size_before_flush = buffer_.size(); + if (smart_flushing_config_.has_value()) { + // Flush down to the target level. + PartialFlush(target_level_ms, sample_rate, last_decoded_length, stats); + return_val = kPartialFlush; + } else { + // Buffer is full. + Flush(stats); + return_val = kFlushed; + } + RTC_LOG(LS_WARNING) << "Packet buffer flushed, " + << (buffer_size_before_flush - buffer_.size()) + << " packets discarded."; } // Get an iterator pointing to the place in the buffer where the new packet @@ -134,7 +209,10 @@ int PacketBuffer::InsertPacketList( const DecoderDatabase& decoder_database, absl::optional* current_rtp_payload_type, absl::optional* current_cng_rtp_payload_type, - StatisticsCalculator* stats) { + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms) { RTC_DCHECK(stats); bool flushed = false; for (auto& packet : *packet_list) { @@ -143,7 +221,7 @@ int PacketBuffer::InsertPacketList( **current_cng_rtp_payload_type != packet.payload_type) { // New CNG payload type implies new codec type. *current_rtp_payload_type = absl::nullopt; - Flush(); + Flush(stats); flushed = true; } *current_cng_rtp_payload_type = packet.payload_type; @@ -156,12 +234,14 @@ int PacketBuffer::InsertPacketList( **current_cng_rtp_payload_type, decoder_database))) { *current_cng_rtp_payload_type = absl::nullopt; - Flush(); + Flush(stats); flushed = true; } *current_rtp_payload_type = packet.payload_type; } - int return_val = InsertPacket(std::move(packet), stats); + int return_val = + InsertPacket(std::move(packet), stats, last_decoded_length, sample_rate, + target_level_ms, decoder_database); if (return_val == kFlushed) { // The buffer flushed, but this is not an error. We can still continue. flushed = true; diff --git a/modules/audio_coding/neteq/packet_buffer.h b/modules/audio_coding/neteq/packet_buffer.h index c00db294c0..cd2adf7111 100644 --- a/modules/audio_coding/neteq/packet_buffer.h +++ b/modules/audio_coding/neteq/packet_buffer.h @@ -22,6 +22,14 @@ namespace webrtc { class DecoderDatabase; class StatisticsCalculator; class TickTimer; +struct SmartFlushingConfig { + // When calculating the flushing threshold, the maximum between the target + // level and this value is used. + int target_level_threshold_ms = 500; + // A smart flush is triggered when the packet buffer contains a multiple of + // the target level. + int target_level_multiplier = 3; +}; // This is the actual buffer holding the packets before decoding. class PacketBuffer { @@ -29,6 +37,7 @@ class PacketBuffer { enum BufferReturnCodes { kOK = 0, kFlushed, + kPartialFlush, kNotFound, kBufferEmpty, kInvalidPacket, @@ -43,7 +52,13 @@ class PacketBuffer { virtual ~PacketBuffer(); // Flushes the buffer and deletes all packets in it. - virtual void Flush(); + virtual void Flush(StatisticsCalculator* stats); + + // Partial flush. Flush packets but leave some packets behind. + virtual void PartialFlush(int target_level_ms, + size_t sample_rate, + size_t last_decoded_length, + StatisticsCalculator* stats); // Returns true for an empty buffer. virtual bool Empty() const; @@ -52,7 +67,12 @@ 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, StatisticsCalculator* stats); + virtual int InsertPacket(Packet&& packet, + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms, + const DecoderDatabase& decoder_database); // Inserts a list of packets into the buffer. The buffer will take over // ownership of the packet objects. @@ -67,7 +87,10 @@ class PacketBuffer { const DecoderDatabase& decoder_database, absl::optional* current_rtp_payload_type, absl::optional* current_cng_rtp_payload_type, - StatisticsCalculator* stats); + StatisticsCalculator* stats, + size_t last_decoded_length, + size_t sample_rate, + int target_level_ms); // Gets the timestamp for the first packet in the buffer and writes it to the // output variable |next_timestamp|. @@ -146,6 +169,7 @@ class PacketBuffer { } private: + absl::optional smart_flushing_config_; size_t max_number_of_packets_; PacketList buffer_; const TickTimer* tick_timer_; diff --git a/modules/audio_coding/neteq/packet_buffer_unittest.cc b/modules/audio_coding/neteq/packet_buffer_unittest.cc index 40e7d5371a..4286006b6e 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 @@ TEST(PacketBuffer, InsertPacket) { PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(17u, 4711u, 0, 10); StrictMock 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 @@ TEST(PacketBuffer, InsertPacket) { 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 @@ TEST(PacketBuffer, FlushBuffer) { PacketGenerator gen(0, 0, 0, 10); const int payload_len = 10; StrictMock 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 @@ TEST(PacketBuffer, OverfillBuffer) { PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(0, 0, 0, 10); StrictMock mock_stats; + MockDecoderDatabase decoder_database; // Insert 10 small packets; should be ok. const int payload_len = 10; @@ -169,7 +186,99 @@ TEST(PacketBuffer, OverfillBuffer) { 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 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 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 @@ TEST(PacketBuffer, OverfillBuffer) { 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 @@ TEST(PacketBuffer, InsertPacketList) { absl::optional current_pt; absl::optional 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 @@ TEST(PacketBuffer, InsertPacketListChangePayloadType) { absl::optional current_pt; absl::optional 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 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { {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 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { } 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 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { 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 @@ TEST(PacketBuffer, DiscardPackets) { PacketList list; const int payload_len = 10; StrictMock 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 @@ TEST(PacketBuffer, DiscardPackets) { &mock_stats); EXPECT_TRUE(buffer.Empty()); + EXPECT_CALL(decoder_database, Die()); // Called when object is deleted. } TEST(PacketBuffer, Reordering) { @@ -434,9 +570,16 @@ TEST(PacketBuffer, Reordering) { StrictMock 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 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) { StrictMock 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 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) { } // 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 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) { 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 @@ TEST(PacketBuffer, Failures) { PacketGenerator gen(start_seq_no, start_ts, 0, ts_increment); TickTimer tick_timer; StrictMock 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 @@ TEST(PacketBuffer, Failures) { // 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 @@ TEST(PacketBuffer, Failures) { 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 @@ TEST(PacketBuffer, Failures) { .WillRepeatedly(Return(&info)); absl::optional current_pt; absl::optional 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 @@ TEST(PacketBuffer, GetSpanSamples) { PacketBuffer buffer(3, &tick_timer); PacketGenerator gen(0, kStartTimeStamp, 0, kFrameSizeSamples); StrictMock mock_stats; + MockDecoderDatabase decoder_database; Packet packet_1 = gen.NextPacket(kPayloadSizeBytes, nullptr); @@ -716,7 +891,12 @@ TEST(PacketBuffer, GetSpanSamples) { 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 @@ TEST(PacketBuffer, GetSpanSamples) { 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));