From 84f8cd6df467cf78c73d4acd4a10ca702b4ab320 Mon Sep 17 00:00:00 2001 From: "henrik.lundin" Date: Tue, 26 Apr 2016 07:45:16 -0700 Subject: [PATCH] NetEq: Use TickTimer in PacketBuffer This change makes use of the TickTimer::Stopwatch in Packets. When a packet is inserted into the PacketBuffer, a Stopwatch object is attached to it. When the packet is extracted from the buffer, the Stopwatch is read to know how long the packet waited in the buffer. BUG=webrtc:5608 Review URL: https://codereview.webrtc.org/1917913002 Cr-Commit-Position: refs/heads/master@{#12508} --- webrtc/modules/audio_coding/BUILD.gn | 2 ++ .../neteq/decision_logic_unittest.cc | 4 ++- .../neteq/mock/mock_packet_buffer.h | 4 +-- webrtc/modules/audio_coding/neteq/neteq.cc | 3 +- webrtc/modules/audio_coding/neteq/neteq.gypi | 2 ++ .../modules/audio_coding/neteq/neteq_impl.cc | 7 ++-- .../audio_coding/neteq/neteq_impl_unittest.cc | 6 ++-- webrtc/modules/audio_coding/neteq/packet.cc | 19 +++++++++++ webrtc/modules/audio_coding/neteq/packet.h | 23 ++++++------- .../audio_coding/neteq/packet_buffer.cc | 15 ++++----- .../audio_coding/neteq/packet_buffer.h | 9 ++--- .../neteq/packet_buffer_unittest.cc | 33 ++++++++++++------- .../audio_coding/neteq/payload_splitter.cc | 4 ++- 13 files changed, 81 insertions(+), 50 deletions(-) create mode 100644 webrtc/modules/audio_coding/neteq/packet.cc diff --git a/webrtc/modules/audio_coding/BUILD.gn b/webrtc/modules/audio_coding/BUILD.gn index 99fa140da3..37978181a5 100644 --- a/webrtc/modules/audio_coding/BUILD.gn +++ b/webrtc/modules/audio_coding/BUILD.gn @@ -791,6 +791,8 @@ source_set("neteq") { "neteq/neteq_impl.h", "neteq/normal.cc", "neteq/normal.h", + "neteq/packet.cc", + "neteq/packet.h", "neteq/packet_buffer.cc", "neteq/packet_buffer.h", "neteq/payload_splitter.cc", diff --git a/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc b/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc index 499f946434..ff8a0c4aa7 100644 --- a/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/decision_logic_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/modules/audio_coding/neteq/delay_manager.h" #include "webrtc/modules/audio_coding/neteq/delay_peak_detector.h" #include "webrtc/modules/audio_coding/neteq/packet_buffer.h" +#include "webrtc/modules/audio_coding/neteq/tick_timer.h" namespace webrtc { @@ -24,7 +25,8 @@ TEST(DecisionLogic, CreateAndDestroy) { int fs_hz = 8000; int output_size_samples = fs_hz / 100; // Samples per 10 ms. DecoderDatabase decoder_database; - PacketBuffer packet_buffer(10); + TickTimer tick_timer; + PacketBuffer packet_buffer(10, &tick_timer); DelayPeakDetector delay_peak_detector; DelayManager delay_manager(240, &delay_peak_detector); BufferLevelFilter buffer_level_filter; diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h index 97e54d83a5..6bb95901d8 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h @@ -19,8 +19,8 @@ namespace webrtc { class MockPacketBuffer : public PacketBuffer { public: - MockPacketBuffer(size_t max_number_of_packets) - : PacketBuffer(max_number_of_packets) {} + MockPacketBuffer(size_t max_number_of_packets, const TickTimer* tick_timer) + : PacketBuffer(max_number_of_packets, tick_timer) {} virtual ~MockPacketBuffer() { Die(); } MOCK_METHOD0(Die, void()); MOCK_METHOD0(Flush, diff --git a/webrtc/modules/audio_coding/neteq/neteq.cc b/webrtc/modules/audio_coding/neteq/neteq.cc index bc6319dedb..1af7966e41 100644 --- a/webrtc/modules/audio_coding/neteq/neteq.cc +++ b/webrtc/modules/audio_coding/neteq/neteq.cc @@ -54,7 +54,8 @@ NetEq* NetEq::Create(const NetEq::Config& config) { delay_manager->SetMaximumDelay(config.max_delay_ms); DtmfBuffer* dtmf_buffer = new DtmfBuffer(config.sample_rate_hz); DtmfToneGenerator* dtmf_tone_generator = new DtmfToneGenerator; - PacketBuffer* packet_buffer = new PacketBuffer(config.max_packets_in_buffer); + PacketBuffer* packet_buffer = + new PacketBuffer(config.max_packets_in_buffer, tick_timer.get()); PayloadSplitter* payload_splitter = new PayloadSplitter; TimestampScaler* timestamp_scaler = new TimestampScaler(*decoder_database); AccelerateFactory* accelerate_factory = new AccelerateFactory; diff --git a/webrtc/modules/audio_coding/neteq/neteq.gypi b/webrtc/modules/audio_coding/neteq/neteq.gypi index 7e0f558ee5..fee51dfe2b 100644 --- a/webrtc/modules/audio_coding/neteq/neteq.gypi +++ b/webrtc/modules/audio_coding/neteq/neteq.gypi @@ -105,6 +105,8 @@ 'statistics_calculator.h', 'normal.cc', 'normal.h', + 'packet.cc', + 'packet.h', 'packet_buffer.cc', 'packet_buffer.h', 'payload_splitter.cc', diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index ef470e5d4e..cca1c4c9e9 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -536,7 +536,8 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, packet->header.numCSRCs = 0; packet->payload_length = payload.size(); packet->primary = true; - packet->waiting_time = 0; + // Waiting time will be set upon inserting the packet in the buffer. + RTC_DCHECK(!packet->waiting_time); packet->payload = new uint8_t[packet->payload_length]; packet->sync_packet = is_sync_packet; if (!packet->payload) { @@ -1002,7 +1003,6 @@ int NetEqImpl::GetDecision(Operations* operation, *operation = kUndefined; // Increment time counters. - packet_buffer_->IncrementWaitingTimes(); stats_.IncreaseCounter(output_size_samples_, fs_hz_); assert(sync_buffer_.get()); @@ -1931,8 +1931,7 @@ int NetEqImpl::ExtractPackets(size_t required_samples, return -1; } stats_.PacketsDiscarded(discard_count); - // Store waiting time in ms; packets->waiting_time is in "output blocks". - stats_.StoreWaitingTime(packet->waiting_time * kOutputSizeMs); + stats_.StoreWaitingTime(packet->waiting_time->ElapsedMs()); assert(packet->payload_length > 0); packet_list->push_back(packet); // Store packet in list. diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index e95522ba44..4433d2067b 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -130,10 +130,12 @@ class NetEqImplTest : public ::testing::Test { dtmf_tone_generator_ = new DtmfToneGenerator; } if (use_mock_packet_buffer_) { - mock_packet_buffer_ = new MockPacketBuffer(config_.max_packets_in_buffer); + mock_packet_buffer_ = + new MockPacketBuffer(config_.max_packets_in_buffer, tick_timer_); packet_buffer_ = mock_packet_buffer_; } else { - packet_buffer_ = new PacketBuffer(config_.max_packets_in_buffer); + packet_buffer_ = + new PacketBuffer(config_.max_packets_in_buffer, tick_timer_); } if (use_mock_payload_splitter_) { mock_payload_splitter_ = new MockPayloadSplitter; diff --git a/webrtc/modules/audio_coding/neteq/packet.cc b/webrtc/modules/audio_coding/neteq/packet.cc new file mode 100644 index 0000000000..8a19fe4d59 --- /dev/null +++ b/webrtc/modules/audio_coding/neteq/packet.cc @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/modules/audio_coding/neteq/packet.h" + +namespace webrtc { + +Packet::Packet() = default; + +Packet::~Packet() = default; + +} // namespace webrtc diff --git a/webrtc/modules/audio_coding/neteq/packet.h b/webrtc/modules/audio_coding/neteq/packet.h index 64b325e027..d6f64c7e08 100644 --- a/webrtc/modules/audio_coding/neteq/packet.h +++ b/webrtc/modules/audio_coding/neteq/packet.h @@ -12,7 +12,9 @@ #define WEBRTC_MODULES_AUDIO_CODING_NETEQ_PACKET_H_ #include +#include +#include "webrtc/modules/audio_coding/neteq/tick_timer.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/typedefs.h" @@ -21,20 +23,15 @@ namespace webrtc { // Struct for holding RTP packets. struct Packet { RTPHeader header; - uint8_t* payload; // Datagram excluding RTP header and header extension. - size_t payload_length; - bool primary; // Primary, i.e., not redundant payload. - int waiting_time; - bool sync_packet; + // Datagram excluding RTP header and header extension. + uint8_t* payload = nullptr; + size_t payload_length = 0; + bool primary = true; // Primary, i.e., not redundant payload. + bool sync_packet = false; + std::unique_ptr waiting_time; - // Constructor. - Packet() - : payload(NULL), - payload_length(0), - primary(true), - waiting_time(0), - sync_packet(false) { - } + Packet(); + ~Packet(); // Comparison operators. Establish a packet ordering based on (1) timestamp, // (2) sequence number, (3) regular packet vs sync-packet and (4) redundancy. diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.cc b/webrtc/modules/audio_coding/neteq/packet_buffer.cc index c89de12318..f1b898e34c 100644 --- a/webrtc/modules/audio_coding/neteq/packet_buffer.cc +++ b/webrtc/modules/audio_coding/neteq/packet_buffer.cc @@ -19,6 +19,7 @@ #include "webrtc/base/logging.h" #include "webrtc/modules/audio_coding/codecs/audio_decoder.h" #include "webrtc/modules/audio_coding/neteq/decoder_database.h" +#include "webrtc/modules/audio_coding/neteq/tick_timer.h" namespace webrtc { @@ -37,8 +38,9 @@ class NewTimestampIsLarger { const Packet* new_packet_; }; -PacketBuffer::PacketBuffer(size_t max_number_of_packets) - : max_number_of_packets_(max_number_of_packets) {} +PacketBuffer::PacketBuffer(size_t max_number_of_packets, + const TickTimer* tick_timer) + : max_number_of_packets_(max_number_of_packets), tick_timer_(tick_timer) {} // Destructor. All packets in the buffer will be destroyed. PacketBuffer::~PacketBuffer() { @@ -65,6 +67,8 @@ int PacketBuffer::InsertPacket(Packet* packet) { int return_val = kOK; + packet->waiting_time = tick_timer_->GetNewStopwatch(); + if (buffer_.size() >= max_number_of_packets_) { // Buffer is full. Flush it. Flush(); @@ -268,13 +272,6 @@ size_t PacketBuffer::NumSamplesInBuffer(DecoderDatabase* decoder_database, return num_samples; } -void PacketBuffer::IncrementWaitingTimes(int inc) { - PacketList::iterator it; - for (it = buffer_.begin(); it != buffer_.end(); ++it) { - (*it)->waiting_time += inc; - } -} - bool PacketBuffer::DeleteFirstPacket(PacketList* packet_list) { if (packet_list->empty()) { return false; diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.h b/webrtc/modules/audio_coding/neteq/packet_buffer.h index 03c11e61b6..6867b4cb37 100644 --- a/webrtc/modules/audio_coding/neteq/packet_buffer.h +++ b/webrtc/modules/audio_coding/neteq/packet_buffer.h @@ -17,8 +17,8 @@ namespace webrtc { -// Forward declaration. class DecoderDatabase; +class TickTimer; // This is the actual buffer holding the packets before decoding. class PacketBuffer { @@ -34,7 +34,7 @@ class PacketBuffer { // Constructor creates a buffer which can hold a maximum of // |max_number_of_packets| packets. - PacketBuffer(size_t max_number_of_packets); + PacketBuffer(size_t max_number_of_packets, const TickTimer* tick_timer); // Deletes all packets in the buffer before destroying the buffer. virtual ~PacketBuffer(); @@ -116,10 +116,6 @@ class PacketBuffer { virtual size_t NumSamplesInBuffer(DecoderDatabase* decoder_database, size_t last_decoded_length) const; - // Increase the waiting time counter for every packet in the buffer by |inc|. - // The default value for |inc| is 1. - virtual void IncrementWaitingTimes(int inc = 1); - virtual void BufferStat(int* num_packets, int* max_num_packets) const; // Static method that properly deletes the first packet, and its payload @@ -148,6 +144,7 @@ class PacketBuffer { private: size_t max_number_of_packets_; PacketList buffer_; + const TickTimer* tick_timer_; RTC_DISALLOW_COPY_AND_ASSIGN(PacketBuffer); }; diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc index 435b6c848d..da35301085 100644 --- a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc @@ -16,6 +16,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h" #include "webrtc/modules/audio_coding/neteq/packet.h" +#include "webrtc/modules/audio_coding/neteq/tick_timer.h" using ::testing::Return; using ::testing::_; @@ -80,13 +81,15 @@ struct PacketsToInsert { // Start of test definitions. TEST(PacketBuffer, CreateAndDestroy) { - PacketBuffer* buffer = new PacketBuffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer* buffer = new PacketBuffer(10, &tick_timer); // 10 packets. EXPECT_TRUE(buffer->Empty()); delete buffer; } TEST(PacketBuffer, InsertPacket) { - PacketBuffer buffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(17u, 4711u, 0, 10); const int payload_len = 100; @@ -107,7 +110,8 @@ TEST(PacketBuffer, InsertPacket) { // Test to flush buffer. TEST(PacketBuffer, FlushBuffer) { - PacketBuffer buffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(0, 0, 0, 10); const int payload_len = 10; @@ -127,7 +131,8 @@ TEST(PacketBuffer, FlushBuffer) { // Test to fill the buffer over the limits, and verify that it flushes. TEST(PacketBuffer, OverfillBuffer) { - PacketBuffer buffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(0, 0, 0, 10); // Insert 10 small packets; should be ok. @@ -156,7 +161,8 @@ TEST(PacketBuffer, OverfillBuffer) { // Test inserting a list of packets. TEST(PacketBuffer, InsertPacketList) { - PacketBuffer buffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(0, 0, 0, 10); PacketList list; const int payload_len = 10; @@ -192,7 +198,8 @@ TEST(PacketBuffer, InsertPacketList) { // Expecting the buffer to flush. // TODO(hlundin): Remove this test when legacy operation is no longer needed. TEST(PacketBuffer, InsertPacketListChangePayloadType) { - PacketBuffer buffer(10); // 10 packets. + TickTimer tick_timer; + PacketBuffer buffer(10, &tick_timer); // 10 packets. PacketGenerator gen(0, 0, 0, 10); PacketList list; const int payload_len = 10; @@ -230,7 +237,8 @@ TEST(PacketBuffer, InsertPacketListChangePayloadType) { } TEST(PacketBuffer, ExtractOrderRedundancy) { - PacketBuffer buffer(100); // 100 packets. + TickTimer tick_timer; + PacketBuffer buffer(100, &tick_timer); // 100 packets. const int kPackets = 18; const int kFrameSize = 10; const int kPayloadLength = 10; @@ -289,7 +297,8 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { } TEST(PacketBuffer, DiscardPackets) { - PacketBuffer buffer(100); // 100 packets. + TickTimer tick_timer; + PacketBuffer buffer(100, &tick_timer); // 100 packets. const uint16_t start_seq_no = 17; const uint32_t start_ts = 4711; const uint32_t ts_increment = 10; @@ -318,7 +327,8 @@ TEST(PacketBuffer, DiscardPackets) { } TEST(PacketBuffer, Reordering) { - PacketBuffer buffer(100); // 100 packets. + TickTimer tick_timer; + PacketBuffer buffer(100, &tick_timer); // 100 packets. const uint16_t start_seq_no = 17; const uint32_t start_ts = 4711; const uint32_t ts_increment = 10; @@ -373,8 +383,9 @@ TEST(PacketBuffer, Failures) { const uint32_t ts_increment = 10; int payload_len = 100; PacketGenerator gen(start_seq_no, start_ts, 0, ts_increment); + TickTimer tick_timer; - PacketBuffer* buffer = new PacketBuffer(100); // 100 packets. + PacketBuffer* buffer = new PacketBuffer(100, &tick_timer); // 100 packets. Packet* packet = NULL; EXPECT_EQ(PacketBuffer::kInvalidPacket, buffer->InsertPacket(packet)); packet = gen.NextPacket(payload_len); @@ -404,7 +415,7 @@ TEST(PacketBuffer, Failures) { // Insert packet list of three packets, where the second packet has an invalid // payload. Expect first packet to be inserted, and the remaining two to be // discarded. - buffer = new PacketBuffer(100); // 100 packets. + buffer = new PacketBuffer(100, &tick_timer); // 100 packets. PacketList list; list.push_back(gen.NextPacket(payload_len)); // Valid packet. packet = gen.NextPacket(payload_len); diff --git a/webrtc/modules/audio_coding/neteq/payload_splitter.cc b/webrtc/modules/audio_coding/neteq/payload_splitter.cc index 542863970b..530e9d064d 100644 --- a/webrtc/modules/audio_coding/neteq/payload_splitter.cc +++ b/webrtc/modules/audio_coding/neteq/payload_splitter.cc @@ -12,6 +12,7 @@ #include +#include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/modules/audio_coding/neteq/decoder_database.h" @@ -168,8 +169,9 @@ int PayloadSplitter::SplitFec(PacketList* packet_list, memcpy(new_packet->payload, packet->payload, packet->payload_length); new_packet->payload_length = packet->payload_length; new_packet->primary = false; - new_packet->waiting_time = packet->waiting_time; new_packet->sync_packet = packet->sync_packet; + // Waiting time should not be set here. + RTC_DCHECK(!packet->waiting_time); packet_list->insert(it, new_packet); break;