From 23ea1d850a0b7df98830f0bf38a310551c8cfcce Mon Sep 17 00:00:00 2001 From: Olof Kallander Date: Wed, 25 Oct 2023 16:14:20 +0200 Subject: [PATCH] use jblist to handle long seqno gaps --- codec/AudioReceivePipeline.h | 4 +- rtp/JitterBufferList.cpp | 14 +-- rtp/JitterBufferList.h | 12 ++- test/transport/AdaptiveJitterTest.cpp | 4 +- test/transport/JitterTest.cpp | 135 +++++++++++++------------- 5 files changed, 80 insertions(+), 89 deletions(-) diff --git a/codec/AudioReceivePipeline.h b/codec/AudioReceivePipeline.h index f0977a502..469b9eb2a 100644 --- a/codec/AudioReceivePipeline.h +++ b/codec/AudioReceivePipeline.h @@ -2,7 +2,7 @@ #include "codec/NoiseFloor.h" #include "codec/OpusDecoder.h" #include "codec/SpscAudioBuffer.h" -#include "rtp/JitterBuffer.h" +#include "rtp/JitterBufferList.h" #include "rtp/JitterEstimator.h" #include "utils/Time.h" #include @@ -85,7 +85,7 @@ class AudioReceivePipeline const uint32_t _rtpFrequency; const uint32_t _samplesPerPacket; - rtp::JitterBuffer _jitterBuffer; + rtp::JitterBufferList _jitterBuffer; rtp::JitterEstimator _estimator; const int _audioLevelExtensionId; diff --git a/rtp/JitterBufferList.cpp b/rtp/JitterBufferList.cpp index 5e5498eb9..67f97dadf 100644 --- a/rtp/JitterBufferList.cpp +++ b/rtp/JitterBufferList.cpp @@ -5,25 +5,15 @@ namespace rtp { -JitterBufferList::JitterBufferList(size_t maxLength) - : _freeItems(nullptr), - _itemStore(new ListItem[maxLength]), - _head(nullptr), - _tail(nullptr), - _count(0) +JitterBufferList::JitterBufferList() : _freeItems(nullptr), _head(nullptr), _tail(nullptr), _count(0) { - for (size_t i = 0; i < maxLength; ++i) + for (size_t i = 0; i < SIZE; ++i) { _itemStore[i].next = _freeItems; _freeItems = &_itemStore[i]; } } -JitterBufferList::~JitterBufferList() -{ - delete[] _itemStore; -} - JitterBufferList::ListItem* JitterBufferList::allocItem() { if (!_freeItems) diff --git a/rtp/JitterBufferList.h b/rtp/JitterBufferList.h index 59147d297..952931593 100644 --- a/rtp/JitterBufferList.h +++ b/rtp/JitterBufferList.h @@ -6,14 +6,18 @@ namespace rtp struct RtpHeader; /** * Effective jitter buffer. Packets are stored in linked list. Ordered packets are quickly added to the end of the list. - * Rarely occurring out of order packets has to be inserted in the list after a quick scan. + * Rarely occurring out of order packets have to be inserted in the list after a quick scan. */ class JitterBufferList { public: - JitterBufferList(size_t maxLength); - ~JitterBufferList(); + enum + { + SIZE = 300 + }; + + JitterBufferList(); bool add(memory::UniquePacket packet); memory::UniquePacket pop(); @@ -35,7 +39,7 @@ class JitterBufferList ListItem* allocItem(); ListItem* _freeItems; - ListItem* const _itemStore; + ListItem _itemStore[SIZE]; ListItem* _head; ListItem* _tail; uint32_t _count; diff --git a/test/transport/AdaptiveJitterTest.cpp b/test/transport/AdaptiveJitterTest.cpp index 000ca6986..b10b4ecd3 100644 --- a/test/transport/AdaptiveJitterTest.cpp +++ b/test/transport/AdaptiveJitterTest.cpp @@ -115,7 +115,7 @@ TEST_P(AudioPipelineTest, DISABLED_fileReRun) } } -INSTANTIATE_TEST_SUITE_P(ArpFileReRun, +INSTANTIATE_TEST_SUITE_P(AudioPipelineRerun, AudioPipelineTest, ::testing::Values("Transport-6-4G-1-5Mbps", "Transport-22-4G-2.3Mbps", @@ -503,7 +503,7 @@ TEST_F(AudioPipelineTest, everIncreasing) header->sequenceNumber.get() - static_cast(extendedSequenceNumber & 0xFFFFu)); extendedSequenceNumber += adv; bool posted = pipeline->onRtpPacket(extendedSequenceNumber, std::move(packet), timestamp); - if (timeSteps == 32859) + if (timeSteps == 32969) { ASSERT_FALSE(posted); } diff --git a/test/transport/JitterTest.cpp b/test/transport/JitterTest.cpp index cf56f5ff5..98d9be4c9 100644 --- a/test/transport/JitterTest.cpp +++ b/test/transport/JitterTest.cpp @@ -6,7 +6,7 @@ #include "math/Matrix.h" #include "math/WelfordVariance.h" #include "memory/PacketPoolAllocator.h" -#include "rtp/JitterBuffer.h" +#include "rtp/JitterBufferList.h" #include "rtp/JitterEstimator.h" #include "rtp/JitterTracker.h" #include "rtp/RtpHeader.h" @@ -171,6 +171,15 @@ TEST(Welford, normal) EXPECT_NEAR(w2.getVariance(), 100.0 * 100, 2500.0); } +class JitterBufferTest : public ::testing::Test +{ +public: + JitterBufferTest() : _allocator(400, "test") {} + + memory::PacketPoolAllocator _allocator; + rtp::JitterBufferList _buffer; +}; + // jitter below 10ms will never affect subsequent packet. It is just a delay inflicted on each packet unrelated to // previous events TEST(JitterTest, a10ms) @@ -769,48 +778,45 @@ TEST(JitterTest, jitterTracker) EXPECT_NEAR(tracker.get(), 2 * 48, 15); } -TEST(JitterTest, bufferPlain) +TEST_F(JitterBufferTest, bufferPlain) { - memory::PacketPoolAllocator allocator(400, "test"); - rtp::JitterBuffer buffer; - memory::Packet stageArea; { auto header = rtp::RtpHeader::create(stageArea); header->ssrc = 4000; stageArea.setLength(250); } - const auto oneFromFull = rtp::JitterBuffer::SIZE - 2; + const auto oneFromFull = _buffer.SIZE - 2; for (int i = 0; i < oneFromFull; ++i) { - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = i + 100; header->timestamp = 56000 + i * 960; - EXPECT_TRUE(buffer.add(std::move(p))); + EXPECT_TRUE(_buffer.add(std::move(p))); } - EXPECT_EQ(buffer.count(), oneFromFull); - EXPECT_EQ(buffer.getRtpDelay(), 960 * (oneFromFull - 1)); - EXPECT_FALSE(buffer.empty()); - EXPECT_EQ(buffer.getFrontRtp()->timestamp.get(), 56000); + EXPECT_EQ(_buffer.count(), oneFromFull); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * (oneFromFull - 1)); + EXPECT_FALSE(_buffer.empty()); + EXPECT_EQ(_buffer.getFrontRtp()->timestamp.get(), 56000); // make a gap of 3 pkts - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); { auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = 100 + oneFromFull; header->timestamp = 56000 + oneFromFull * 960; } - EXPECT_TRUE(buffer.add(std::move(p))); - EXPECT_EQ(buffer.getRtpDelay(), 960 * oneFromFull); + EXPECT_TRUE(_buffer.add(std::move(p))); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * oneFromFull); uint16_t prevSeq = 99; uint32_t count = 0; - EXPECT_EQ(buffer.count(), oneFromFull + 1); - for (auto packet = buffer.pop(); packet; packet = buffer.pop()) + EXPECT_EQ(_buffer.count(), oneFromFull + 1); + for (auto packet = _buffer.pop(); packet; packet = _buffer.pop()) { auto header = rtp::RtpHeader::fromPacket(*packet); EXPECT_GT(header->sequenceNumber.get(), prevSeq); @@ -820,11 +826,8 @@ TEST(JitterTest, bufferPlain) EXPECT_EQ(count, oneFromFull + 1); } -TEST(JitterTest, bufferReorder) +TEST_F(JitterBufferTest, bufferReorder) { - memory::PacketPoolAllocator allocator(300, "test"); - rtp::JitterBuffer buffer; - memory::Packet stageArea; { auto header = rtp::RtpHeader::create(stageArea); @@ -834,51 +837,51 @@ TEST(JitterTest, bufferReorder) for (int i = 0; i < 5; ++i) { - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = i + 100; header->timestamp = 56000 + i * 960; - EXPECT_TRUE(buffer.add(std::move(p))); + EXPECT_TRUE(_buffer.add(std::move(p))); } - EXPECT_EQ(buffer.count(), 5); - EXPECT_EQ(buffer.getRtpDelay(), 960 * 4); - EXPECT_FALSE(buffer.empty()); - EXPECT_EQ(buffer.getFrontRtp()->timestamp.get(), 56000); + EXPECT_EQ(_buffer.count(), 5); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * 4); + EXPECT_FALSE(_buffer.empty()); + EXPECT_EQ(_buffer.getFrontRtp()->timestamp.get(), 56000); // make a gap - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); { auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = 110; header->timestamp = 56000 + 10 * 960; } - EXPECT_TRUE(buffer.add(std::move(p))); - EXPECT_EQ(buffer.getRtpDelay(), 960 * 10); + EXPECT_TRUE(_buffer.add(std::move(p))); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * 10); // reorder - p = memory::makeUniquePacket(allocator, stageArea); + p = memory::makeUniquePacket(_allocator, stageArea); { auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = 105; header->timestamp = 56000 + 5 * 960; } - EXPECT_TRUE(buffer.add(std::move(p))); - EXPECT_EQ(buffer.getRtpDelay(), 960 * 10); - EXPECT_EQ(buffer.count(), 7); + EXPECT_TRUE(_buffer.add(std::move(p))); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * 10); + EXPECT_EQ(_buffer.count(), 7); - p = memory::makeUniquePacket(allocator, stageArea); + p = memory::makeUniquePacket(_allocator, stageArea); { auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = 108; header->timestamp = 56000 + 28 * 960; } - EXPECT_TRUE(buffer.add(std::move(p))); + EXPECT_TRUE(_buffer.add(std::move(p))); uint16_t prevSeq = 99; uint32_t count = 0; - for (auto packet = buffer.pop(); packet; packet = buffer.pop()) + for (auto packet = _buffer.pop(); packet; packet = _buffer.pop()) { auto header = rtp::RtpHeader::fromPacket(*packet); EXPECT_GT(header->sequenceNumber.get(), prevSeq); @@ -888,11 +891,8 @@ TEST(JitterTest, bufferReorder) EXPECT_EQ(count, 8); } -TEST(JitterTest, bufferEmptyFull) +TEST_F(JitterBufferTest, bufferEmptyFull) { - memory::PacketPoolAllocator allocator(300, "test"); - rtp::JitterBuffer buffer; - memory::Packet stageArea; { auto header = rtp::RtpHeader::create(stageArea); @@ -900,52 +900,49 @@ TEST(JitterTest, bufferEmptyFull) stageArea.setLength(250); } - EXPECT_EQ(buffer.pop(), nullptr); + EXPECT_EQ(_buffer.pop(), nullptr); - for (int i = 0; i < rtp::JitterBuffer::SIZE - 1; ++i) + for (int i = 0; i < _buffer.SIZE; ++i) { - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = i + 100; header->timestamp = 56000 + i * 960; - EXPECT_TRUE(buffer.add(std::move(p))); + EXPECT_TRUE(_buffer.add(std::move(p))); } - EXPECT_EQ(buffer.count(), rtp::JitterBuffer::SIZE - 1); - EXPECT_EQ(buffer.getRtpDelay(), 960 * (rtp::JitterBuffer::SIZE - 2)); - EXPECT_FALSE(buffer.empty()); - EXPECT_EQ(buffer.getFrontRtp()->timestamp.get(), 56000); + EXPECT_EQ(_buffer.count(), _buffer.SIZE); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * (_buffer.SIZE - 1)); + EXPECT_FALSE(_buffer.empty()); + EXPECT_EQ(_buffer.getFrontRtp()->timestamp.get(), 56000); - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); { auto header = rtp::RtpHeader::create(*p); - header->sequenceNumber = 100 + buffer.count(); + header->sequenceNumber = 100 + _buffer.count(); header->timestamp = 56000 + 10 * 960; } - EXPECT_FALSE(buffer.add(std::move(p))); - EXPECT_EQ(buffer.getRtpDelay(), 960 * (rtp::JitterBuffer::SIZE - 2)); + EXPECT_FALSE(_buffer.add(std::move(p))); + EXPECT_EQ(_buffer.getRtpDelay(), 960 * (_buffer.SIZE - 1)); uint16_t prevSeq = 99; uint32_t count = 0; - EXPECT_EQ(buffer.count(), rtp::JitterBuffer::SIZE - 1); - for (auto packet = buffer.pop(); packet; packet = buffer.pop()) + EXPECT_EQ(_buffer.count(), _buffer.SIZE); + for (auto packet = _buffer.pop(); packet; packet = _buffer.pop()) { auto header = rtp::RtpHeader::fromPacket(*packet); EXPECT_GT(header->sequenceNumber.get(), prevSeq); prevSeq = header->sequenceNumber.get(); ++count; } - EXPECT_EQ(count, rtp::JitterBuffer::SIZE - 1); - EXPECT_TRUE(buffer.empty()); - EXPECT_EQ(buffer.pop(), nullptr); + EXPECT_EQ(count, _buffer.SIZE); + EXPECT_TRUE(_buffer.empty()); + EXPECT_EQ(_buffer.pop(), nullptr); } -TEST(JitterTest, reorderedFull) +TEST_F(JitterBufferTest, reorderedFull) { - memory::PacketPoolAllocator allocator(300, "test"); - rtp::JitterBuffer buffer; - memory::Packet stageArea; { auto header = rtp::RtpHeader::create(stageArea); @@ -953,23 +950,23 @@ TEST(JitterTest, reorderedFull) stageArea.setLength(250); } - EXPECT_EQ(buffer.pop(), nullptr); + EXPECT_EQ(_buffer.pop(), nullptr); - for (int i = 0; i < rtp::JitterBuffer::SIZE - 50; ++i) + for (int i = 0; i < _buffer.SIZE - 50; ++i) { - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = i + 100; header->timestamp = 56000 + i * 960; - EXPECT_TRUE(buffer.add(std::move(p))); + EXPECT_TRUE(_buffer.add(std::move(p))); } - auto p = memory::makeUniquePacket(allocator, stageArea); + auto p = memory::makeUniquePacket(_allocator, stageArea); auto header = rtp::RtpHeader::create(*p); header->sequenceNumber = 49; header->timestamp = 56100; - EXPECT_EQ(buffer.count(), rtp::JitterBuffer::SIZE - 50); - EXPECT_FALSE(buffer.add(std::move(p))); + EXPECT_EQ(_buffer.count(), _buffer.SIZE - 50); + EXPECT_TRUE(_buffer.add(std::move(p))); }