Fix to allow small negative jumps due to out of order packets in packet buffer

This resolves an issue where when packets appear out of order at the
beginning of a stream, packet_buffer.cc might drop the entire packet
buffer because it detects a "large negative jump" even though the
difference in sequence numbers is very minor and is caused by network
congestion / packet re-ordering. Currently, when the issue occurs, this
can cause video corruption/artifacts. More details and reproduction is
available on the attached webrtc bug report 390329776.

Bug: webrtc:390329776
Change-Id: Idb56eb2e066d596d8afd7ec904359baf0cb3feef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374540
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43753}
This commit is contained in:
thebongy 2025-01-16 19:41:42 +05:30 committed by WebRTC LUCI CQ
parent 3cc17eed68
commit d23d04163d
3 changed files with 16 additions and 1 deletions

View File

@ -114,6 +114,7 @@ Ralph Giles <giles@ghostscript.com>
Raman Budny <budnyjj@gmail.com>
Ramprakash Jelari <ennajelari@gmail.com>
Riku Voipio <riku.voipio@linaro.org>
Rishit Bansal <rishitbansal0@gmail.com>
Robert Bares <robert@bares.me>
Robert Mader <robert.mader@collabora.com>
Robert Mader <robert.mader@posteo.de>

View File

@ -82,7 +82,8 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket(
return result;
}
if (ForwardDiff<uint16_t>(first_seq_num_, seq_num) >= max_size_) {
if (ForwardDiff<uint16_t>(first_seq_num_, seq_num) >= max_size_ &&
ForwardDiff<uint16_t>(seq_num, first_seq_num_) >= max_size_ / 2) {
// Large negative jump in rtp sequence number: clear the buffer and treat
// latest packet as the new first packet.
Clear();

View File

@ -356,6 +356,19 @@ TEST_F(PacketBufferTest, FramesReordered) {
StartSeqNumsAre(seq_num + 2));
}
TEST_F(PacketBufferTest, FramesReorderedReconstruction) {
Insert(100, kKeyFrame, kFirst, kNotLast, {}, 2);
Insert(98, kKeyFrame, kFirst, kNotLast, {}, 1);
EXPECT_THAT(Insert(99, kDeltaFrame, kNotFirst, kLast, {}, 1),
StartSeqNumsAre(98));
// Ideally frame with timestamp 2, seq No 100 should be
// reconstructed here from the first Insert() call in the test
EXPECT_THAT(Insert(101, kDeltaFrame, kNotFirst, kLast, {}, 2),
StartSeqNumsAre(100));
}
TEST_F(PacketBufferTest, InsertPacketAfterSequenceNumberWrapAround) {
int64_t kFirstSeqNum = 0;
uint32_t kTimestampDelta = 100;