admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix wrap-around logic in ForwardErrorCorrection.

Browse Source 
New function AbsSequenceNumberDifference.

Bug: None
Change-Id: I3906e3be313ec69973a20096c17c06e20448149d
Reviewed-on: https://webrtc-review.googlesource.com/4384
Commit-Queue: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20086}
This commit is contained in:
Niels Möller 2017-10-02 13:51:36 +02:00 committed by Commit Bot
parent d4404c232d
commit 958288a640
2 changed files with 25 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
modules/rtp_rtcp/source/forward_error_correction.cc
View File

@ -23,6 +23,7 @@
#include "modules/rtp_rtcp/source/ulpfec_header_reader_writer.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/mod_ops.h"
namespace webrtc {
@ -500,11 +501,7 @@ void ForwardErrorCorrection::InsertPacket(
// RED+ULPFEC is used.
auto it = received_fec_packets_.begin();
while (it != received_fec_packets_.end()) {
// TODO(nisse): This handling of wraparound appears broken, should be
// static_cast<int16_t>(
// received_packet.seq_num - back_recovered_packet->seq_num)
uint16_t seq_num_diff = abs(static_cast<int>(received_packet.seq_num) -
static_cast<int>((*it)->seq_num));
uint16_t seq_num_diff = MinDiff(received_packet.seq_num, (*it)->seq_num);
if (seq_num_diff > 0x3fff) {
it = received_fec_packets_.erase(it);
} else {
@ -720,12 +717,8 @@ void ForwardErrorCorrection::DecodeFec(const ReceivedPacket& received_packet,
recovered_packets->back().get();
if (received_packet.ssrc == back_recovered_packet->ssrc) {
// TODO(nisse): This handling of wraparound appears broken, should be
// static_cast<int16_t>(
// received_packet.seq_num - back_recovered_packet->seq_num)
const unsigned int seq_num_diff =
abs(static_cast<int>(received_packet.seq_num) -
static_cast<int>(back_recovered_packet->seq_num));
MinDiff(received_packet.seq_num, back_recovered_packet->seq_num);
if (seq_num_diff > max_media_packets) {
// A big gap in sequence numbers. The old recovered packets
// are now useless, so it's safe to do a reset.

30
modules/rtp_rtcp/source/rtp_fec_unittest.cc
View File

@ -297,16 +297,18 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss) {
}
// Verify that we don't use an old FEC packet for FEC decoding.
TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) {
TYPED_TEST(RtpFecTest, NoFecRecoveryWithOldFecPacket) {
constexpr int kNumImportantPackets = 0;
constexpr bool kUseUnequalProtection = false;
constexpr uint8_t kProtectionFactor = 20;
// Two frames: first frame (old) with two media packets and 1 FEC packet.
// Second frame (new) with 3 media packets, and no FEC packets.
// ---Frame 1---- ----Frame 2------
// #0(media) #1(media) #2(FEC) #65535(media) #0(media) #1(media).
// If we lose either packet 0 or 1 of second frame, FEC decoding should not
// Third frame (new) with 3 media packets, and no FEC packets.
//
// #0(media) #1(media) #2(FEC) ----Frame 1-----
// #32767(media) 32768(media) 32769(media) ----Frame 2-----
// #65535(media) #0(media) #1(media). ----Frame 3-----
// If we lose either packet 0 or 1 of third frame, FEC decoding should not
// try to decode using "old" FEC packet #2.
// Construct media packets for first frame, starting at sequence number 0.
@ -326,6 +328,17 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) {
true);
// Construct media packets for second frame, with sequence number wrap.
this->media_packets_ =
this->media_packet_generator_.ConstructMediaPackets(3, 32767);
// Expect 3 media packets for this frame.
EXPECT_EQ(3u, this->media_packets_.size());
// No packets lost
memset(this->media_loss_mask_, 0, sizeof(this->media_loss_mask_));
this->ReceivedPackets(this->media_packets_, this->media_loss_mask_, false);
// Construct media packets for third frame, with sequence number wrap.
this->media_packets_ =
this->media_packet_generator_.ConstructMediaPackets(3, 65535);
@ -343,10 +356,11 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) {
&this->recovered_packets_);
}
// Expect that no decoding is done to get missing packet (seq#0) of second
// Expect that no decoding is done to get missing packet (seq#0) of third
// frame, using old FEC packet (seq#2) from first (old) frame. So number of
// recovered packets is 2, and not equal to number of media packets (=3).
EXPECT_EQ(2u, this->recovered_packets_.size());
// recovered packets is 5 (0 from first frame, three from second frame, and 2
// for the third frame, with no packets recovered via FEC).
EXPECT_EQ(5u, this->recovered_packets_.size());
EXPECT_TRUE(this->recovered_packets_.size() != this->media_packets_.size());
}