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

Change ForwardErrorCorrection class to accept one received packet at a time.

Browse Source 
BUG=None

Review-Url: https://codereview.webrtc.org/3012243002
Cr-Commit-Position: refs/heads/master@{#19893}
This commit is contained in:
nisse 2017-09-18 07:58:59 -07:00 committed by Commit Bot
parent dd3abbb532
commit a5f043f9cd
9 changed files with 281 additions and 211 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
modules/rtp_rtcp/include/flexfec_receiver.h
View File

@ -39,8 +39,10 @@ class FlexfecReceiver {
// Protected to aid testing.
protected:
bool AddReceivedPacket(const RtpPacketReceived& packet);
bool ProcessReceivedPackets();
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> AddReceivedPacket(
const RtpPacketReceived& packet);
void ProcessReceivedPacket(
const ForwardErrorCorrection::ReceivedPacket& received_packet);
private:
// Config.
@ -50,8 +52,6 @@ class FlexfecReceiver {
// Erasure code interfacing and callback.
std::unique_ptr<ForwardErrorCorrection> erasure_code_
RTC_GUARDED_BY(sequence_checker_);
ForwardErrorCorrection::ReceivedPacketList received_packets_
RTC_GUARDED_BY(sequence_checker_);
ForwardErrorCorrection::RecoveredPacketList recovered_packets_
RTC_GUARDED_BY(sequence_checker_);
RecoveredPacketReceiver* const recovered_packet_receiver_;

29
modules/rtp_rtcp/source/flexfec_receiver.cc
View File

@ -48,10 +48,11 @@ FlexfecReceiver::~FlexfecReceiver() = default;
void FlexfecReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
if (!AddReceivedPacket(packet)) {
std::unique_ptr<ReceivedPacket> received_packet = AddReceivedPacket(packet);
if (!received_packet)
return;
}
ProcessReceivedPackets();
ProcessReceivedPacket(*received_packet);
}
FecPacketCounter FlexfecReceiver::GetPacketCounter() const {
@ -61,7 +62,8 @@ FecPacketCounter FlexfecReceiver::GetPacketCounter() const {
// TODO(eladalon): Consider using packet.recovered() to avoid processing
// recovered packets here.
bool FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
std::unique_ptr<ReceivedPacket> FlexfecReceiver::AddReceivedPacket(
const RtpPacketReceived& packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
// RTP packets with a full base header (12 bytes), but without payload,
@ -77,7 +79,7 @@ bool FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
// This is a FlexFEC packet.
if (packet.payload_size() < kMinFlexfecHeaderSize) {
LOG(LS_WARNING) << "Truncated FlexFEC packet, discarding.";
return false;
return nullptr;
}
received_packet->is_fec = true;
++packet_counter_.num_fec_packets;
@ -93,7 +95,7 @@ bool FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
// This is a media packet, or a FlexFEC packet belonging to some
// other FlexFEC stream.
if (received_packet->ssrc != protected_media_ssrc_) {
return false;
return nullptr;
}
received_packet->is_fec = false;
@ -104,10 +106,9 @@ bool FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
received_packet->pkt->length = packet.size();
}
received_packets_.push_back(std::move(received_packet));
++packet_counter_.num_packets;
return true;
return received_packet;
}
// Note that the implementation of this member function and the implementation
@ -119,16 +120,13 @@ bool FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
// Here, however, the received media pipeline is more decoupled from the
// FlexFEC decoder, and we therefore do not interfere with the reception
// of non-recovered media packets.
bool FlexfecReceiver::ProcessReceivedPackets() {
void FlexfecReceiver::ProcessReceivedPacket(
const ReceivedPacket& received_packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
// Decode.
if (!received_packets_.empty()) {
if (erasure_code_->DecodeFec(&received_packets_, &recovered_packets_) !=
0) {
return false;
}
}
erasure_code_->DecodeFec(received_packet, &recovered_packets_);
// Return recovered packets through callback.
for (const auto& recovered_packet : recovered_packets_) {
if (recovered_packet->returned) {
@ -150,7 +148,6 @@ bool FlexfecReceiver::ProcessReceivedPackets() {
last_recovered_packet_ms_ = now_ms;
}
}
return true;
}
} // namespace webrtc

44
modules/rtp_rtcp/source/flexfec_receiver_unittest.cc
View File

@ -54,7 +54,7 @@ class FlexfecReceiverForTest : public FlexfecReceiver {
}
// Expose methods for tests.
using FlexfecReceiver::AddReceivedPacket;
using FlexfecReceiver::ProcessReceivedPackets;
using FlexfecReceiver::ProcessReceivedPacket;
};
class FlexfecReceiverTest : public ::testing::Test {
@ -113,8 +113,10 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaPacket) {
std::unique_ptr<Packet> media_packet(
packet_generator_.NextPacket(0, kPayloadLength));
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*media_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
}
TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) {
@ -127,10 +129,13 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) {
const auto& media_packet = media_packets.front();
auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front());
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*media_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
received_packet = receiver_.AddReceivedPacket(ParsePacket(*fec_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
}
TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) {
@ -145,8 +150,10 @@ TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) {
fec_packets.front()->length = 1;
auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front());
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*media_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
EXPECT_FALSE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
}
@ -180,8 +187,10 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) {
fec_packet->data[10] = 6;
fec_packet->data[11] = 7;
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*media_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
EXPECT_FALSE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
}
@ -195,17 +204,20 @@ TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) {
// Receive all media packets.
for (const auto& media_packet : media_packets) {
EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*media_packet));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
}
// Receive FEC packet.
auto fec_packet = fec_packets.front();
std::unique_ptr<Packet> packet_with_rtp_header =
packet_generator_.BuildFlexfecPacket(*fec_packet);
EXPECT_TRUE(
receiver_.AddReceivedPacket(ParsePacket(*packet_with_rtp_header)));
EXPECT_TRUE(receiver_.ProcessReceivedPackets());
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet =
receiver_.AddReceivedPacket(ParsePacket(*packet_with_rtp_header));
ASSERT_TRUE(received_packet);
receiver_.ProcessReceivedPacket(*received_packet);
}
TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {

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

@ -342,16 +342,14 @@ void ForwardErrorCorrection::ResetState(
void ForwardErrorCorrection::InsertMediaPacket(
RecoveredPacketList* recovered_packets,
ReceivedPacket* received_packet) {
RTC_DCHECK_EQ(received_packet->ssrc, protected_media_ssrc_);
const ReceivedPacket& received_packet) {
RTC_DCHECK_EQ(received_packet.ssrc, protected_media_ssrc_);
// Search for duplicate packets.
for (const auto& recovered_packet : *recovered_packets) {
RTC_DCHECK_EQ(recovered_packet->ssrc, received_packet->ssrc);
if (recovered_packet->seq_num == received_packet->seq_num) {
RTC_DCHECK_EQ(recovered_packet->ssrc, received_packet.ssrc);
if (recovered_packet->seq_num == received_packet.seq_num) {
// Duplicate packet, no need to add to list.
// Delete duplicate media packet data.
received_packet->pkt = nullptr;
return;
}
}
@ -361,10 +359,10 @@ void ForwardErrorCorrection::InsertMediaPacket(
recovered_packet->was_recovered = false;
// This media packet has already been passed on.
recovered_packet->returned = true;
recovered_packet->ssrc = received_packet->ssrc;
recovered_packet->seq_num = received_packet->seq_num;
recovered_packet->pkt = received_packet->pkt;
recovered_packet->pkt->length = received_packet->pkt->length;
recovered_packet->ssrc = received_packet.ssrc;
recovered_packet->seq_num = received_packet.seq_num;
recovered_packet->pkt = received_packet.pkt;
recovered_packet->pkt->length = received_packet.pkt->length;
// TODO(holmer): Consider replacing this with a binary search for the right
// position, and then just insert the new packet. Would get rid of the sort.
RecoveredPacket* recovered_packet_ptr = recovered_packet.get();
@ -390,23 +388,22 @@ void ForwardErrorCorrection::UpdateCoveringFecPackets(
void ForwardErrorCorrection::InsertFecPacket(
const RecoveredPacketList& recovered_packets,
ReceivedPacket* received_packet) {
RTC_DCHECK_EQ(received_packet->ssrc, ssrc_);
const ReceivedPacket& received_packet) {
RTC_DCHECK_EQ(received_packet.ssrc, ssrc_);
// Check for duplicate.
for (const auto& existing_fec_packet : received_fec_packets_) {
RTC_DCHECK_EQ(existing_fec_packet->ssrc, received_packet->ssrc);
if (existing_fec_packet->seq_num == received_packet->seq_num) {
// Delete duplicate FEC packet data.
received_packet->pkt = nullptr;
RTC_DCHECK_EQ(existing_fec_packet->ssrc, received_packet.ssrc);
if (existing_fec_packet->seq_num == received_packet.seq_num) {
// Drop duplicate FEC packet data.
return;
}
}
std::unique_ptr<ReceivedFecPacket> fec_packet(new ReceivedFecPacket());
fec_packet->pkt = received_packet->pkt;
fec_packet->ssrc = received_packet->ssrc;
fec_packet->seq_num = received_packet->seq_num;
fec_packet->pkt = received_packet.pkt;
fec_packet->ssrc = received_packet.ssrc;
fec_packet->seq_num = received_packet.seq_num;
// Parse ULPFEC/FlexFEC header specific info.
bool ret = fec_header_reader_->ReadFecHeader(fec_packet.get());
if (!ret) {
@ -483,49 +480,46 @@ void ForwardErrorCorrection::AssignRecoveredPackets(
}
}
void ForwardErrorCorrection::InsertPackets(
ReceivedPacketList* received_packets,
void ForwardErrorCorrection::InsertPacket(
const ReceivedPacket& received_packet,
RecoveredPacketList* recovered_packets) {
while (!received_packets->empty()) {
ReceivedPacket* received_packet = received_packets->front().get();
// Discard old FEC packets such that the sequence numbers in
// |received_fec_packets_| span at most 1/2 of the sequence number space.
// This is important for keeping |received_fec_packets_| sorted, and may
// also reduce the possibility of incorrect decoding due to sequence number
// wrap-around.
// TODO(marpan/holmer): We should be able to improve detection/discarding of
// old FEC packets based on timestamp information or better sequence number
// thresholding (e.g., to distinguish between wrap-around and reordering).
if (!received_fec_packets_.empty() &&
received_packet->ssrc == received_fec_packets_.front()->ssrc) {
// It only makes sense to detect wrap-around when |received_packet|
// and |front_received_fec_packet| belong to the same sequence number
// space, i.e., the same SSRC. This happens when |received_packet|
// is a FEC packet, or if |received_packet| is a media packet and
// RED+ULPFEC is used.
auto it = received_fec_packets_.begin();
while (it != received_fec_packets_.end()) {
uint16_t seq_num_diff = abs(static_cast<int>(received_packet->seq_num) -
static_cast<int>((*it)->seq_num));
if (seq_num_diff > 0x3fff) {
it = received_fec_packets_.erase(it);
} else {
// No need to keep iterating, since |received_fec_packets_| is sorted.
break;
}
// Discard old FEC packets such that the sequence numbers in
// |received_fec_packets_| span at most 1/2 of the sequence number space.
// This is important for keeping |received_fec_packets_| sorted, and may
// also reduce the possibility of incorrect decoding due to sequence number
// wrap-around.
// TODO(marpan/holmer): We should be able to improve detection/discarding of
// old FEC packets based on timestamp information or better sequence number
// thresholding (e.g., to distinguish between wrap-around and reordering).
if (!received_fec_packets_.empty() &&
received_packet.ssrc == received_fec_packets_.front()->ssrc) {
// It only makes sense to detect wrap-around when |received_packet|
// and |front_received_fec_packet| belong to the same sequence number
// space, i.e., the same SSRC. This happens when |received_packet|
// is a FEC packet, or if |received_packet| is a media packet and
// 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));
if (seq_num_diff > 0x3fff) {
it = received_fec_packets_.erase(it);
} else {
// No need to keep iterating, since |received_fec_packets_| is sorted.
break;
}
}
if (received_packet->is_fec) {
InsertFecPacket(*recovered_packets, received_packet);
} else {
InsertMediaPacket(recovered_packets, received_packet);
}
// Delete the received packet "wrapper".
received_packets->pop_front();
}
RTC_DCHECK(received_packets->empty());
if (received_packet.is_fec) {
InsertFecPacket(*recovered_packets, received_packet);
} else {
InsertMediaPacket(recovered_packets, received_packet);
}
DiscardOldRecoveredPackets(recovered_packets);
}
@ -716,38 +710,35 @@ uint32_t ForwardErrorCorrection::ParseSsrc(uint8_t* packet) {
return (packet[8] << 24) + (packet[9] << 16) + (packet[10] << 8) + packet[11];
}
int ForwardErrorCorrection::DecodeFec(
ReceivedPacketList* received_packets,
RecoveredPacketList* recovered_packets) {
RTC_DCHECK(received_packets);
void ForwardErrorCorrection::DecodeFec(const ReceivedPacket& received_packet,
RecoveredPacketList* recovered_packets) {
RTC_DCHECK(recovered_packets);
const size_t max_media_packets = fec_header_reader_->MaxMediaPackets();
if (recovered_packets->size() == max_media_packets) {
const RecoveredPacket* back_recovered_packet =
recovered_packets->back().get();
for (const auto& received_packet : *received_packets) {
if (received_packet->ssrc == back_recovered_packet->ssrc) {
const unsigned int seq_num_diff =
abs(static_cast<int>(received_packet->seq_num) -
static_cast<int>(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.
LOG(LS_INFO) << "Big gap in media/ULPFEC sequence numbers. No need "
"to keep the old packets in the FEC buffers, thus "
"resetting them.";
ResetState(recovered_packets);
break;
}
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));
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.
LOG(LS_INFO) << "Big gap in media/ULPFEC sequence numbers. No need "
"to keep the old packets in the FEC buffers, thus "
"resetting them.";
ResetState(recovered_packets);
}
}
}
InsertPackets(received_packets, recovered_packets);
InsertPacket(received_packet, recovered_packets);
AttemptRecovery(recovered_packets);
return 0;
}
size_t ForwardErrorCorrection::MaxPacketOverhead() const {

22
modules/rtp_rtcp/source/forward_error_correction.h
View File

@ -75,9 +75,10 @@ class ForwardErrorCorrection {
uint16_t seq_num;
};
// The received list parameter of DecodeFec() references structs of this type.
// Used for the input to DecodeFec().
//
// TODO(holmer): Refactor into a proper class.
// TODO(nisse): Delete class, instead passing |is_fec| and |pkt| as separate
// arguments.
class ReceivedPacket : public SortablePacket {
public:
ReceivedPacket();
@ -141,7 +142,6 @@ class ForwardErrorCorrection {
};
using PacketList = std::list<std::unique_ptr<Packet>>;
using ReceivedPacketList = std::list<std::unique_ptr<ReceivedPacket>>;
using RecoveredPacketList = std::list<std::unique_ptr<RecoveredPacket>>;
using ReceivedFecPacketList = std::list<std::unique_ptr<ReceivedFecPacket>>;
@ -218,10 +218,8 @@ class ForwardErrorCorrection {
// list will be valid until the next call to
// DecodeFec().
//
// Returns 0 on success, -1 on failure.
//
int DecodeFec(ReceivedPacketList* received_packets,
RecoveredPacketList* recovered_packets);
void DecodeFec(const ReceivedPacket& received_packet,
RecoveredPacketList* recovered_packets);
// Get the number of generated FEC packets, given the number of media packets
// and the protection factor.
@ -266,14 +264,14 @@ class ForwardErrorCorrection {
uint32_t media_ssrc,
uint16_t seq_num_base);
// Inserts the |received_packets| into the internal received FEC packet list
// Inserts the |received_packet| into the internal received FEC packet list
// or into |recovered_packets|.
void InsertPackets(ReceivedPacketList* received_packets,
RecoveredPacketList* recovered_packets);
void InsertPacket(const ReceivedPacket& received_packet,
RecoveredPacketList* recovered_packets);
// Inserts the |received_packet| into |recovered_packets|. Deletes duplicates.
void InsertMediaPacket(RecoveredPacketList* recovered_packets,
ReceivedPacket* received_packet);
const ReceivedPacket& received_packet);
// Assigns pointers to the recovered packet from all FEC packets which cover
// it.
@ -284,7 +282,7 @@ class ForwardErrorCorrection {
// Insert |received_packet| into internal FEC list. Deletes duplicates.
void InsertFecPacket(const RecoveredPacketList& recovered_packets,
ReceivedPacket* received_packet);
const ReceivedPacket& received_packet);
// Assigns pointers to already recovered packets covered by |fec_packet|.
static void AssignRecoveredPackets(

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

@ -86,7 +86,8 @@ class RtpFecTest : public ::testing::Test {
ForwardErrorCorrection::PacketList media_packets_;
std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
ForwardErrorCorrection::ReceivedPacketList received_packets_;
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>
received_packets_;
ForwardErrorCorrection::RecoveredPacketList recovered_packets_;
int media_loss_mask_[kUlpfecMaxMediaPackets];
@ -98,6 +99,7 @@ void RtpFecTest<ForwardErrorCorrectionType>::NetworkReceivedPackets(
int* media_loss_mask,
int* fec_loss_mask) {
constexpr bool kFecPacket = true;
this->received_packets_.clear();
ReceivedPackets(media_packets_, media_loss_mask, !kFecPacket);
ReceivedPackets(generated_fec_packets_, fec_loss_mask, kFecPacket);
}
@ -237,8 +239,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNoLoss) {
memset(this->fec_loss_mask_, 0, sizeof(this->fec_loss_mask_));
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// No packets lost, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -267,8 +271,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -281,8 +287,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// 2 packets lost, one FEC packet, cannot get complete recovery.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -330,8 +338,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) {
// Add packets #65535, and #1 to received list.
this->ReceivedPackets(this->media_packets_, this->media_loss_mask_, false);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect that no decoding is done to get missing packet (seq#0) of second
// frame, using old FEC packet (seq#2) from first (old) frame. So number of
@ -370,8 +380,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameRecovery) {
this->ReceivedPackets(this->generated_fec_packets_, this->fec_loss_mask_,
true);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect 3 media packets in recovered list, and complete recovery.
// Wrap-around won't remove FEC packet, as it follows the wrap.
@ -380,13 +392,18 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameRecovery) {
}
// Sequence number wrap occurs within the ULPFEC packets for the frame.
// In this case we will discard ULPFEC packet and full recovery is not expected.
// Same problem will occur if wrap is within media packets but ULPFEC packet is
// received before the media packets. This may be improved if timing information
// is used to detect old ULPFEC packets.
// TODO(marpan): Update test if wrap-around handling changes in FEC decoding.
// TODO(nisse): There's some logic to discard ULPFEC packets at wrap-around,
// however, that is not actually exercised by this test: When the first FEC
// packet is processed, it results in full recovery of one media packet and the
// FEC packet is forgotten. And then the wraparound isn't noticed when the next
// FEC packet is received. We should fix wraparound handling, which currently
// appears broken, and then figure out how to test it properly.
using RtpFecTestUlpfecOnly = RtpFecTest<UlpfecForwardErrorCorrection>;
TEST_F(RtpFecTestUlpfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
TEST_F(RtpFecTestUlpfecOnly, FecRecoveryWithSeqNumGapOneFrameRecovery) {
constexpr int kNumImportantPackets = 0;
constexpr bool kUseUnequalProtection = false;
constexpr uint8_t kProtectionFactor = 200;
@ -415,22 +432,28 @@ TEST_F(RtpFecTestUlpfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
this->ReceivedPackets(this->generated_fec_packets_, this->fec_loss_mask_,
true);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// The two FEC packets are received and should allow for complete recovery,
// but because of the wrap the first FEC packet will be discarded, and only
// one media packet is recoverable. So expect 2 media packets on recovered
// list and no complete recovery.
EXPECT_EQ(2u, this->recovered_packets_.size());
EXPECT_TRUE(this->recovered_packets_.size() != this->media_packets_.size());
EXPECT_FALSE(this->IsRecoveryComplete());
EXPECT_EQ(3u, this->recovered_packets_.size());
EXPECT_EQ(this->recovered_packets_.size(), this->media_packets_.size());
EXPECT_TRUE(this->IsRecoveryComplete());
}
// TODO(brandtr): This test mimics the one above, ensuring that the recovery
// strategy of FlexFEC matches the recovery strategy of ULPFEC. Since FlexFEC
// does not share the sequence number space with the media, however, having a
// matching recovery strategy may be suboptimal. Study this further.
// TODO(nisse): In this test, recovery based on the first FEC packet fails with
// the log message "The recovered packet had a length larger than a typical IP
// packet, and is thus dropped." This is probably not intended, and needs
// investigation.
using RtpFecTestFlexfecOnly = RtpFecTest<FlexfecForwardErrorCorrection>;
TEST_F(RtpFecTestFlexfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
constexpr int kNumImportantPackets = 0;
@ -468,8 +491,10 @@ TEST_F(RtpFecTestFlexfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
this->ReceivedPackets(this->generated_fec_packets_, this->fec_loss_mask_,
true);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// The two FEC packets are received and should allow for complete recovery,
// but because of the wrap the first FEC packet will be discarded, and only
@ -512,8 +537,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithMediaOutOfOrder) {
it1++;
std::swap(*it0, *it1);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect 3 media packets in recovered list, and complete recovery.
EXPECT_EQ(3u, this->recovered_packets_.size());
@ -550,8 +577,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithFecOutOfOrder) {
// Add media packets to received list.
this->ReceivedPackets(this->media_packets_, this->media_loss_mask_, false);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect 3 media packets in recovered list, and complete recovery.
EXPECT_EQ(3u, this->recovered_packets_.size());
@ -597,8 +626,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percRandomMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// With media packet#1 and FEC packets #1, #2, #3, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -613,8 +644,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percRandomMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Cannot get complete recovery for this loss configuration with random mask.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -659,8 +692,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percBurstyMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect complete recovery for consecutive packet loss <= 50%.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -675,8 +710,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percBurstyMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Expect complete recovery for consecutive packet loss <= 50%.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -691,8 +728,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percBurstyMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Cannot get complete recovery for this loss configuration.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -720,8 +759,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNoLossUep) {
memset(this->fec_loss_mask_, 0, sizeof(this->fec_loss_mask_));
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// No packets lost, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -750,8 +791,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLossUep) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -764,8 +807,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLossUep) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// 2 packets lost, one FEC packet, cannot get complete recovery.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -808,8 +853,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percUepRandomMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// With media packet#3 and FEC packets #0, #1, #3, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -825,8 +872,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithLoss50percUepRandomMask) {
this->media_loss_mask_[3] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Cannot get complete recovery for this loss configuration.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -860,8 +909,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePackets) {
this->media_loss_mask_[2] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -873,8 +924,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePackets) {
this->media_loss_mask_[1] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Unprotected packet lost. Recovery not possible.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -887,8 +940,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePackets) {
this->media_loss_mask_[2] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// 2 protected packets lost, one FEC packet, cannot get complete recovery.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -925,8 +980,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) {
this->media_loss_mask_[kNumMediaPackets - 1] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -938,8 +995,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) {
this->media_loss_mask_[kNumMediaPackets - 2] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Unprotected packet lost. Recovery not possible.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -956,8 +1015,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) {
this->media_loss_mask_[kNumMediaPackets - 1] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// 5 protected packets lost, one FEC packet, cannot get complete recovery.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -994,8 +1055,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) {
this->media_loss_mask_[kNumMediaPackets - 1] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(this->IsRecoveryComplete());
@ -1007,8 +1070,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) {
this->media_loss_mask_[kNumMediaPackets - 2] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// Unprotected packet lost. Recovery not possible.
EXPECT_FALSE(this->IsRecoveryComplete());
@ -1025,8 +1090,10 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) {
this->media_loss_mask_[kNumMediaPackets - 1] = 1;
this->NetworkReceivedPackets(this->media_loss_mask_, this->fec_loss_mask_);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
for (const auto& received_packet : this->received_packets_) {
this->fec_.DecodeFec(*received_packet,
&this->recovered_packets_);
}
// 5 protected packets lost, one FEC packet, cannot get complete recovery.
EXPECT_FALSE(this->IsRecoveryComplete());

15
modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
View File

@ -216,23 +216,22 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket(
return 0;
}
// TODO(nisse): Drop always-zero return value.
int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
crit_sect_.Enter();
if (!received_packets_.empty()) {
for (const auto& received_packet : received_packets_) {
// Send received media packet to VCM.
if (!received_packets_.front()->is_fec) {
ForwardErrorCorrection::Packet* packet = received_packets_.front()->pkt;
if (!received_packet->is_fec) {
ForwardErrorCorrection::Packet* packet = received_packet->pkt;
crit_sect_.Leave();
recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length);
crit_sect_.Enter();
}
if (fec_->DecodeFec(&received_packets_, &recovered_packets_) != 0) {
crit_sect_.Leave();
return -1;
}
RTC_DCHECK(received_packets_.empty());
fec_->DecodeFec(*received_packet, &recovered_packets_);
}
received_packets_.clear();
// Send any recovered media packets to VCM.
for (const auto& recovered_packet : recovered_packets_) {
if (recovered_packet->returned) {

11
modules/rtp_rtcp/source/ulpfec_receiver_impl.h
View File

@ -12,6 +12,7 @@
#define MODULES_RTP_RTCP_SOURCE_ULPFEC_RECEIVER_IMPL_H_
#include <memory>
#include <vector>
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/include/ulpfec_receiver.h"
@ -41,10 +42,12 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
rtc::CriticalSection crit_sect_;
RecoveredPacketReceiver* recovered_packet_callback_;
std::unique_ptr<ForwardErrorCorrection> fec_;
// TODO(holmer): In the current version |received_packets_| is never more
// than one packet, since we process FEC every time a new packet
// arrives. We should remove the list.
ForwardErrorCorrection::ReceivedPacketList received_packets_;
// TODO(nisse): The AddReceivedRedPacket method adds one or two packets to
// this list at a time, after which it is emptied by ProcessReceivedFec. It
// will make things simpler to merge AddReceivedRedPacket and
// ProcessReceivedFec into a single method, and we can then delete this list.
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>
received_packets_;
ForwardErrorCorrection::RecoveredPacketList recovered_packets_;
FecPacketCounter packet_counter_;
};

21
modules/rtp_rtcp/test/testFec/test_fec.cc
View File

@ -35,8 +35,10 @@ namespace test {
using fec_private_tables::kPacketMaskBurstyTbl;
void ReceivePackets(
ForwardErrorCorrection::ReceivedPacketList* to_decode_list,
ForwardErrorCorrection::ReceivedPacketList* received_packet_list,
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>*
to_decode_list,
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>*
received_packet_list,
size_t num_packets_to_decode,
float reorder_rate,
float duplicate_rate,
@ -103,8 +105,10 @@ void RunTest(bool use_flexfec) {
ForwardErrorCorrection::PacketList media_packet_list;
std::list<ForwardErrorCorrection::Packet*> fec_packet_list;
ForwardErrorCorrection::ReceivedPacketList to_decode_list;
ForwardErrorCorrection::ReceivedPacketList received_packet_list;
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>
to_decode_list;
std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>
received_packet_list;
ForwardErrorCorrection::RecoveredPacketList recovered_packet_list;
std::list<uint8_t*> fec_mask_list;
@ -403,11 +407,10 @@ void RunTest(bool use_flexfec) {
}
}
}
ASSERT_EQ(0,
fec->DecodeFec(&to_decode_list, &recovered_packet_list))
<< "DecodeFec() failed";
ASSERT_TRUE(to_decode_list.empty())
<< "Received packet list is not empty.";
for (const auto& received_packet : to_decode_list) {
fec->DecodeFec(*received_packet, &recovered_packet_list);
}
to_decode_list.clear();
}
media_packet_idx = 0;
for (const auto& media_packet : media_packet_list) {