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

Reland of Only compare sequence numbers from the same SSRC in ForwardErrorCorrection. (patchset #1 id:1 of https://codereview.webrtc.org/2919313005/ )

Browse Source 
Reason for revert:
Fix RtpStreamReceiver to not recover RTX packets with incorrect SSRC.

Original issue's description:
> Revert of Only compare sequence numbers from the same SSRC in ForwardErrorCorrection. (patchset #5 id:120001 of https://codereview.webrtc.org/2893293003/ )
>
> Reason for revert:
> Breaks fuzzer.
>
> Original issue's description:
> > Only compare sequence numbers from the same SSRC in ForwardErrorCorrection.
> >
> > Prior to this CL, the ForwardErrorCorrection state would be reset whenever
> > the difference in sequence numbers of the last recovered media packet
> > and the new packet (media or FEC) was too large. This comparison did not
> > take into account that FlexFEC uses a different SSRC for the FEC packets,
> > meaning that the the state would be reset very frequently when FlexFEC
> > is used. This should not have led to any major problems, except for a
> > decreased decoding efficiency.
> >
> > This CL verifies that whenever we compare sequence numbers in
> > ForwardErrorCorrection, they do indeed belong to the same SSRC.
> >
> > BUG=webrtc:5654
> >
> > Review-Url: https://codereview.webrtc.org/2893293003
> > Cr-Commit-Position: refs/heads/master@{#18399}
> > Committed: 1476a9d789
>
> TBR=stefan@webrtc.org,holmer@google.com
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5654
>
> Review-Url: https://codereview.webrtc.org/2919313005
> Cr-Commit-Position: refs/heads/master@{#18446}
> Committed: 92732ecc5c

R=stefan@webrtc.org
BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2918333002
Cr-Commit-Position: refs/heads/master@{#18827}
This commit is contained in:
brandtr 2017-06-29 02:45:35 -07:00 committed by Commit Bot
parent e4350197ec
commit d726a3f487
15 changed files with 280 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h
View File

@ -31,7 +31,8 @@ struct FecPacketCounter {
class UlpfecReceiver {
public:
static UlpfecReceiver* Create(RecoveredPacketReceiver* callback);
static UlpfecReceiver* Create(uint32_t ssrc,
RecoveredPacketReceiver* callback);
virtual ~UlpfecReceiver() {}

6
webrtc/modules/rtp_rtcp/source/fec_test_helper.cc
View File

@ -78,7 +78,7 @@ ForwardErrorCorrection::PacketList MediaPacketGenerator::ConstructMediaPackets(
RTC_DCHECK(media_packet);
media_packet->data[1] |= 0x80;
fec_seq_num_ = seq_num;
next_seq_num_ = seq_num;
return media_packets;
}
@ -88,8 +88,8 @@ ForwardErrorCorrection::PacketList MediaPacketGenerator::ConstructMediaPackets(
return ConstructMediaPackets(num_media_packets, random_->Rand<uint16_t>());
}
uint16_t MediaPacketGenerator::GetFecSeqNum() {
return fec_seq_num_;
uint16_t MediaPacketGenerator::GetNextSeqNum() {
return next_seq_num_;
}
AugmentedPacketGenerator::AugmentedPacketGenerator(uint32_t ssrc)

4
webrtc/modules/rtp_rtcp/source/fec_test_helper.h
View File

@ -49,7 +49,7 @@ class MediaPacketGenerator {
ForwardErrorCorrection::PacketList ConstructMediaPackets(
int num_media_packets);
uint16_t GetFecSeqNum();
uint16_t GetNextSeqNum();
private:
uint32_t min_packet_size_;
@ -58,7 +58,7 @@ class MediaPacketGenerator {
Random* random_;
ForwardErrorCorrection::PacketList media_packets_;
uint16_t fec_seq_num_;
uint16_t next_seq_num_;
};
// This class generates media packets with a certain structure of the payload.

3
webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc
View File

@ -34,7 +34,8 @@ FlexfecReceiver::FlexfecReceiver(
RecoveredPacketReceiver* recovered_packet_receiver)
: ssrc_(ssrc),
protected_media_ssrc_(protected_media_ssrc),
erasure_code_(ForwardErrorCorrection::CreateFlexfec()),
erasure_code_(
ForwardErrorCorrection::CreateFlexfec(ssrc, protected_media_ssrc)),
recovered_packet_receiver_(recovered_packet_receiver),
clock_(Clock::GetRealTimeClock()),
last_recovered_packet_ms_(-1) {

3
webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc
View File

@ -61,7 +61,8 @@ class FlexfecReceiverTest : public ::testing::Test {
protected:
FlexfecReceiverTest()
: receiver_(kFlexfecSsrc, kMediaSsrc, &recovered_packet_receiver_),
erasure_code_(ForwardErrorCorrection::CreateFlexfec()),
erasure_code_(
ForwardErrorCorrection::CreateFlexfec(kFlexfecSsrc, kMediaSsrc)),
packet_generator_(kMediaSsrc, kFlexfecSsrc) {}
// Generates |num_media_packets| corresponding to a single frame.

3
webrtc/modules/rtp_rtcp/source/flexfec_sender.cc
View File

@ -80,7 +80,8 @@ FlexfecSender::FlexfecSender(
protected_media_ssrc_(protected_media_ssrc),
seq_num_(rtp_state ? rtp_state->sequence_number
: random_.Rand(1, kMaxInitRtpSeqNumber)),
ulpfec_generator_(ForwardErrorCorrection::CreateFlexfec()),
ulpfec_generator_(
ForwardErrorCorrection::CreateFlexfec(ssrc, protected_media_ssrc)),
rtp_header_extension_map_(RegisterBweExtensions(rtp_header_extensions)),
header_extensions_size_(
rtp_header_extension_map_.GetTotalLengthInBytes(extension_sizes)) {

116
webrtc/modules/rtp_rtcp/source/forward_error_correction.cc
View File

@ -54,6 +54,7 @@ template <typename S, typename T>
bool ForwardErrorCorrection::SortablePacket::LessThan::operator() (
const S& first,
const T& second) {
RTC_DCHECK_EQ(first->ssrc, second->ssrc);
return IsNewerSequenceNumber(second->seq_num, first->seq_num);
}
@ -71,27 +72,34 @@ ForwardErrorCorrection::ReceivedFecPacket::~ReceivedFecPacket() = default;
ForwardErrorCorrection::ForwardErrorCorrection(
std::unique_ptr<FecHeaderReader> fec_header_reader,
std::unique_ptr<FecHeaderWriter> fec_header_writer)
: fec_header_reader_(std::move(fec_header_reader)),
std::unique_ptr<FecHeaderWriter> fec_header_writer,
uint32_t ssrc,
uint32_t protected_media_ssrc)
: ssrc_(ssrc),
protected_media_ssrc_(protected_media_ssrc),
fec_header_reader_(std::move(fec_header_reader)),
fec_header_writer_(std::move(fec_header_writer)),
generated_fec_packets_(fec_header_writer_->MaxFecPackets()),
packet_mask_size_(0) {}
ForwardErrorCorrection::~ForwardErrorCorrection() = default;
std::unique_ptr<ForwardErrorCorrection> ForwardErrorCorrection::CreateUlpfec() {
std::unique_ptr<ForwardErrorCorrection> ForwardErrorCorrection::CreateUlpfec(
uint32_t ssrc) {
std::unique_ptr<FecHeaderReader> fec_header_reader(new UlpfecHeaderReader());
std::unique_ptr<FecHeaderWriter> fec_header_writer(new UlpfecHeaderWriter());
return std::unique_ptr<ForwardErrorCorrection>(new ForwardErrorCorrection(
std::move(fec_header_reader), std::move(fec_header_writer)));
std::move(fec_header_reader), std::move(fec_header_writer), ssrc, ssrc));
}
std::unique_ptr<ForwardErrorCorrection>
ForwardErrorCorrection::CreateFlexfec() {
std::unique_ptr<ForwardErrorCorrection> ForwardErrorCorrection::CreateFlexfec(
uint32_t ssrc,
uint32_t protected_media_ssrc) {
std::unique_ptr<FecHeaderReader> fec_header_reader(new FlexfecHeaderReader());
std::unique_ptr<FecHeaderWriter> fec_header_writer(new FlexfecHeaderWriter());
return std::unique_ptr<ForwardErrorCorrection>(new ForwardErrorCorrection(
std::move(fec_header_reader), std::move(fec_header_writer)));
std::move(fec_header_reader), std::move(fec_header_writer), ssrc,
protected_media_ssrc));
}
int ForwardErrorCorrection::EncodeFec(const PacketList& media_packets,
@ -335,20 +343,25 @@ void ForwardErrorCorrection::ResetState(
void ForwardErrorCorrection::InsertMediaPacket(
RecoveredPacketList* recovered_packets,
ReceivedPacket* received_packet) {
RTC_DCHECK_EQ(received_packet->ssrc, protected_media_ssrc_);
// Search for duplicate packets.
for (const auto& recovered_packet : *recovered_packets) {
if (received_packet->seq_num == recovered_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;
}
}
std::unique_ptr<RecoveredPacket> recovered_packet(new RecoveredPacket());
// This "recovered packet" was not recovered using parity packets.
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;
@ -378,23 +391,35 @@ void ForwardErrorCorrection::UpdateCoveringFecPackets(
void ForwardErrorCorrection::InsertFecPacket(
const RecoveredPacketList& recovered_packets,
ReceivedPacket* received_packet) {
RTC_DCHECK_EQ(received_packet->ssrc, ssrc_);
// Check for duplicate.
for (const auto& existing_fec_packet : received_fec_packets_) {
if (received_packet->seq_num == existing_fec_packet->seq_num) {
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;
return;
}
}
std::unique_ptr<ReceivedFecPacket> fec_packet(new ReceivedFecPacket());
fec_packet->pkt = received_packet->pkt;
fec_packet->seq_num = received_packet->seq_num;
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) {
return;
}
// TODO(brandtr): Update here when we support multistream protection.
if (fec_packet->protected_ssrc != protected_media_ssrc_) {
LOG(LS_INFO)
<< "Received FEC packet is protecting an unknown media SSRC; dropping.";
return;
}
// Parse packet mask from header and represent as protected packets.
for (uint16_t byte_idx = 0; byte_idx < fec_packet->packet_mask_size;
++byte_idx) {
@ -405,6 +430,7 @@ void ForwardErrorCorrection::InsertFecPacket(
std::unique_ptr<ProtectedPacket> protected_packet(
new ProtectedPacket());
// This wraps naturally with the sequence number.
protected_packet->ssrc = protected_media_ssrc_;
protected_packet->seq_num = static_cast<uint16_t>(
fec_packet->seq_num_base + (byte_idx << 3) + bit_idx);
protected_packet->pkt = nullptr;
@ -412,6 +438,7 @@ void ForwardErrorCorrection::InsertFecPacket(
}
}
}
if (fec_packet->protected_packets.empty()) {
// All-zero packet mask; we can discard this FEC packet.
LOG(LS_WARNING) << "Received FEC packet has an all-zero packet mask.";
@ -419,11 +446,6 @@ void ForwardErrorCorrection::InsertFecPacket(
AssignRecoveredPackets(recovered_packets, fec_packet.get());
// 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.
//
// For correct decoding, |received_fec_packets_| does not necessarily
// need to be sorted by sequence number (see decoding algorithm in
// AttemptRecover()). By keeping it sorted we try to recover the
// oldest lost packets first, however.
received_fec_packets_.push_back(std::move(fec_packet));
received_fec_packets_.sort(SortablePacket::LessThan());
const size_t max_fec_packets = fec_header_reader_->MaxFecPackets();
@ -467,19 +489,31 @@ void ForwardErrorCorrection::InsertPackets(
while (!received_packets->empty()) {
ReceivedPacket* received_packet = received_packets->front().get();
// Check for discarding oldest FEC packet, to avoid wrong FEC decoding from
// sequence number wrap-around. Detection of old FEC packet is based on
// sequence number difference of received packet and oldest packet in FEC
// packet list.
// 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()) {
uint16_t seq_num_diff =
abs(static_cast<int>(received_packet->seq_num) -
static_cast<int>(received_fec_packets_.front()->seq_num));
if (seq_num_diff > 0x3fff) {
received_fec_packets_.pop_front();
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;
}
}
}
@ -549,6 +583,7 @@ bool ForwardErrorCorrection::FinishPacketRecovery(
// Set the SSRC field.
ByteWriter<uint32_t>::WriteBigEndian(&recovered_packet->pkt->data[8],
fec_packet.protected_ssrc);
recovered_packet->ssrc = fec_packet.protected_ssrc;
return true;
}
@ -684,21 +719,34 @@ uint32_t ForwardErrorCorrection::ParseSsrc(uint8_t* packet) {
int ForwardErrorCorrection::DecodeFec(
ReceivedPacketList* received_packets,
RecoveredPacketList* recovered_packets) {
// TODO(marpan/ajm): can we check for multiple ULP headers, and return an
// error?
RTC_DCHECK(received_packets);
RTC_DCHECK(recovered_packets);
const size_t max_media_packets = fec_header_reader_->MaxMediaPackets();
if (recovered_packets->size() == max_media_packets) {
const unsigned int seq_num_diff =
abs(static_cast<int>(received_packets->front()->seq_num) -
static_cast<int>(recovered_packets->back()->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.
ResetState(recovered_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;
}
}
}
}
InsertPackets(received_packets, recovered_packets);
AttemptRecovery(recovered_packets);
return 0;
}

30
webrtc/modules/rtp_rtcp/source/forward_error_correction.h
View File

@ -33,6 +33,8 @@ class FecHeaderWriter;
// Option exists to enable unequal protection (UEP) across packets.
// This is not to be confused with protection within packets
// (referred to as uneven level protection (ULP) in RFC 5109).
// TODO(brandtr): Split this class into a separate encoder
// and a separate decoder.
class ForwardErrorCorrection {
public:
// TODO(holmer): As a next step all these struct-like packet classes should be
@ -62,29 +64,25 @@ class ForwardErrorCorrection {
class SortablePacket {
public:
// Functor which returns true if the sequence number of |first|
// is < the sequence number of |second|.
// is < the sequence number of |second|. Should only ever be called for
// packets belonging to the same SSRC.
struct LessThan {
template <typename S, typename T>
bool operator() (const S& first, const T& second);
};
uint32_t ssrc;
uint16_t seq_num;
};
// The received list parameter of DecodeFec() references structs of this type.
//
// The ssrc member is needed to ensure that we can restore the SSRC field of
// recovered packets. In most situations this could be retrieved from other
// media packets, but in the case of an FEC packet protecting a single
// missing media packet, we have no other means of obtaining it.
// TODO(holmer): Refactor into a proper class.
class ReceivedPacket : public SortablePacket {
public:
ReceivedPacket();
~ReceivedPacket();
uint32_t ssrc; // SSRC of the current frame. Must be set for FEC
// packets, but not required for media packets.
bool is_fec; // Set to true if this is an FEC packet and false
// otherwise.
rtc::scoped_refptr<Packet> pkt; // Pointer to the packet storage.
@ -109,7 +107,7 @@ class ForwardErrorCorrection {
// Used to link media packets to their protecting FEC packets.
//
// TODO(holmer): Refactor into a proper class.
class ProtectedPacket : public ForwardErrorCorrection::SortablePacket {
class ProtectedPacket : public SortablePacket {
public:
ProtectedPacket();
~ProtectedPacket();
@ -122,7 +120,7 @@ class ForwardErrorCorrection {
// Used for internal storage of received FEC packets in a list.
//
// TODO(holmer): Refactor into a proper class.
class ReceivedFecPacket : public ForwardErrorCorrection::SortablePacket {
class ReceivedFecPacket : public SortablePacket {
public:
ReceivedFecPacket();
~ReceivedFecPacket();
@ -150,8 +148,10 @@ class ForwardErrorCorrection {
~ForwardErrorCorrection();
// Creates a ForwardErrorCorrection tailored for a specific FEC scheme.
static std::unique_ptr<ForwardErrorCorrection> CreateUlpfec();
static std::unique_ptr<ForwardErrorCorrection> CreateFlexfec();
static std::unique_ptr<ForwardErrorCorrection> CreateUlpfec(uint32_t ssrc);
static std::unique_ptr<ForwardErrorCorrection> CreateFlexfec(
uint32_t ssrc,
uint32_t protected_media_ssrc);
// Generates a list of FEC packets from supplied media packets.
//
@ -242,7 +242,9 @@ class ForwardErrorCorrection {
protected:
ForwardErrorCorrection(std::unique_ptr<FecHeaderReader> fec_header_reader,
std::unique_ptr<FecHeaderWriter> fec_header_writer);
std::unique_ptr<FecHeaderWriter> fec_header_writer,
uint32_t ssrc,
uint32_t protected_media_ssrc);
private:
// Analyzes |media_packets| for holes in the sequence and inserts zero columns
@ -330,6 +332,10 @@ class ForwardErrorCorrection {
// for recovering lost packets.
void DiscardOldRecoveredPackets(RecoveredPacketList* recovered_packets);
// These SSRCs are only used by the decoder.
const uint32_t ssrc_;
const uint32_t protected_media_ssrc_;
std::unique_ptr<FecHeaderReader> fec_header_reader_;
std::unique_ptr<FecHeaderWriter> fec_header_writer_;

156
webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
View File

@ -108,7 +108,8 @@ void RtpFecTest<ForwardErrorCorrectionType>::ReceivedPackets(
const PacketListType& packet_list,
int* loss_mask,
bool is_fec) {
uint16_t fec_seq_num = media_packet_generator_.GetFecSeqNum();
uint16_t fec_seq_num = ForwardErrorCorrectionType::GetFirstFecSeqNum(
media_packet_generator_.GetNextSeqNum());
int packet_idx = 0;
for (const auto& packet : packet_list) {
@ -120,19 +121,17 @@ void RtpFecTest<ForwardErrorCorrectionType>::ReceivedPackets(
memcpy(received_packet->pkt->data, packet->data, packet->length);
received_packet->is_fec = is_fec;
if (!is_fec) {
// For media packets, the sequence number and marker bit is
// obtained from RTP header. These were set in ConstructMediaPackets().
received_packet->ssrc = kMediaSsrc;
// For media packets, the sequence number is obtained from the
// RTP header as written by MediaPacketGenerator::ConstructMediaPackets.
received_packet->seq_num =
ByteReader<uint16_t>::ReadBigEndian(&packet->data[2]);
} else {
// The sequence number, marker bit, and ssrc number are defined in the
// RTP header of the FEC packet, which is not constructed in this test.
// So we set these values below based on the values generated in
// ConstructMediaPackets().
received_packet->ssrc = ForwardErrorCorrectionType::kFecSsrc;
// For FEC packets, we simulate the sequence numbers differently
// depending on if ULPFEC or FlexFEC is used. See the definition of
// ForwardErrorCorrectionType::GetFirstFecSeqNum.
received_packet->seq_num = fec_seq_num;
// The ssrc value for FEC packets is set to the one used for the
// media packets in ConstructMediaPackets().
received_packet->ssrc = kMediaSsrc;
}
received_packets_.push_back(std::move(received_packet));
}
@ -177,18 +176,39 @@ bool RtpFecTest<ForwardErrorCorrectionType>::IsRecoveryComplete() {
class FlexfecForwardErrorCorrection : public ForwardErrorCorrection {
public:
static const uint32_t kFecSsrc = kFlexfecSsrc;
FlexfecForwardErrorCorrection()
: ForwardErrorCorrection(
std::unique_ptr<FecHeaderReader>(new FlexfecHeaderReader()),
std::unique_ptr<FecHeaderWriter>(new FlexfecHeaderWriter())) {}
std::unique_ptr<FecHeaderWriter>(new FlexfecHeaderWriter()),
kFecSsrc,
kMediaSsrc) {}
// For FlexFEC we let the FEC packet sequence numbers be independent of
// the media packet sequence numbers.
static uint16_t GetFirstFecSeqNum(uint16_t next_media_seq_num) {
Random random(0xbe110);
return random.Rand<uint16_t>();
}
};
class UlpfecForwardErrorCorrection : public ForwardErrorCorrection {
public:
static const uint32_t kFecSsrc = kMediaSsrc;
UlpfecForwardErrorCorrection()
: ForwardErrorCorrection(
std::unique_ptr<FecHeaderReader>(new UlpfecHeaderReader()),
std::unique_ptr<FecHeaderWriter>(new UlpfecHeaderWriter())) {}
std::unique_ptr<FecHeaderWriter>(new UlpfecHeaderWriter()),
kFecSsrc,
kMediaSsrc) {}
// For ULPFEC we assume that the FEC packets are subsequent to the media
// packets in terms of sequence number.
static uint16_t GetFirstFecSeqNum(uint16_t next_media_seq_num) {
return next_media_seq_num;
}
};
using FecTypes =
@ -359,13 +379,14 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameRecovery) {
EXPECT_TRUE(this->IsRecoveryComplete());
}
// Sequence number wrap occurs within the FEC packets for the frame.
// In this case we will discard FEC packet and full recovery is not expected.
// Same problem will occur if wrap is within media packets but FEC packet is
// 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 FEC packets.
// is used to detect old ULPFEC packets.
// TODO(marpan): Update test if wrap-around handling changes in FEC decoding.
TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
using RtpFecTestUlpfecOnly = RtpFecTest<UlpfecForwardErrorCorrection>;
TEST_F(RtpFecTestUlpfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
constexpr int kNumImportantPackets = 0;
constexpr bool kUseUnequalProtection = false;
constexpr uint8_t kProtectionFactor = 200;
@ -398,8 +419,61 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
&this->recovered_packets_));
// The two FEC packets are received and should allow for complete recovery,
// but because of the wrap the second FEC packet will be discarded, and only
// one media packet is recoverable. So exepct 2 media packets on recovered
// 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());
}
// 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.
using RtpFecTestFlexfecOnly = RtpFecTest<FlexfecForwardErrorCorrection>;
TEST_F(RtpFecTestFlexfecOnly, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
constexpr int kNumImportantPackets = 0;
constexpr bool kUseUnequalProtection = false;
constexpr uint8_t kProtectionFactor = 200;
// 1 frame: 3 media packets and 2 FEC packets.
// Sequence number wrap in FEC packets.
// -----Frame 1----
// #65532(media) #65533(media) #65534(media) #65535(FEC) #0(FEC).
this->media_packets_ =
this->media_packet_generator_.ConstructMediaPackets(3, 65532);
EXPECT_EQ(
0, this->fec_.EncodeFec(this->media_packets_, kProtectionFactor,
kNumImportantPackets, kUseUnequalProtection,
kFecMaskBursty, &this->generated_fec_packets_));
// Expect 2 FEC packets.
EXPECT_EQ(2u, this->generated_fec_packets_.size());
// Overwrite the sequence numbers generated by ConstructMediaPackets,
// to make sure that we do have a wrap.
auto it = this->generated_fec_packets_.begin();
ByteWriter<uint16_t>::WriteBigEndian(&(*it)->data[2], 65535);
++it;
ByteWriter<uint16_t>::WriteBigEndian(&(*it)->data[2], 0);
// Lose the last two media packets (seq# 65533, 65534).
memset(this->media_loss_mask_, 0, sizeof(this->media_loss_mask_));
memset(this->fec_loss_mask_, 0, sizeof(this->fec_loss_mask_));
this->media_loss_mask_[1] = 1;
this->media_loss_mask_[2] = 1;
this->ReceivedPackets(this->media_packets_, this->media_loss_mask_, false);
this->ReceivedPackets(this->generated_fec_packets_, this->fec_loss_mask_,
true);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&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());
@ -434,9 +508,9 @@ TYPED_TEST(RtpFecTest, FecRecoveryWithMediaOutOfOrder) {
// Reorder received media packets.
auto it0 = this->received_packets_.begin();
auto it2 = this->received_packets_.begin();
it2++;
std::swap(*it0, *it2);
auto it1 = this->received_packets_.begin();
it1++;
std::swap(*it0, *it1);
EXPECT_EQ(0, this->fec_.DecodeFec(&this->received_packets_,
&this->recovered_packets_));
@ -958,42 +1032,4 @@ TYPED_TEST(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) {
EXPECT_FALSE(this->IsRecoveryComplete());
}
// 'using' directive needed for compiler to be happy.
using RtpFecTestWithFlexfec = RtpFecTest<FlexfecForwardErrorCorrection>;
TEST_F(RtpFecTestWithFlexfec,
FecRecoveryWithLossAndDifferentMediaAndFlexfecSsrcs) {
constexpr int kNumImportantPackets = 0;
constexpr bool kUseUnequalProtection = false;
constexpr int kNumMediaPackets = 4;
constexpr uint8_t kProtectionFactor = 60;
media_packets_ =
media_packet_generator_.ConstructMediaPackets(kNumMediaPackets);
EXPECT_EQ(0, fec_.EncodeFec(media_packets_, kProtectionFactor,
kNumImportantPackets, kUseUnequalProtection,
kFecMaskBursty, &generated_fec_packets_));
// Expect 1 FEC packet.
EXPECT_EQ(1u, generated_fec_packets_.size());
// 1 media packet lost
memset(media_loss_mask_, 0, sizeof(media_loss_mask_));
memset(fec_loss_mask_, 0, sizeof(fec_loss_mask_));
media_loss_mask_[3] = 1;
NetworkReceivedPackets(media_loss_mask_, fec_loss_mask_);
// Simulate FlexFEC packet received on different SSRC.
auto it = received_packets_.begin();
++it;
++it;
++it; // Now at the FEC packet.
(*it)->ssrc = kFlexfecSsrc;
EXPECT_EQ(0, fec_.DecodeFec(&received_packets_, &recovered_packets_));
// One packet lost, one FEC packet, expect complete recovery.
EXPECT_TRUE(IsRecoveryComplete());
}
} // namespace webrtc

9
webrtc/modules/rtp_rtcp/source/ulpfec_generator.cc
View File

@ -49,6 +49,13 @@ constexpr uint8_t kHighProtectionThreshold = 80;
// |kMinMediaPackets| + 1 packets are sent to the FEC code.
constexpr float kMinMediaPacketsAdaptationThreshold = 2.0f;
// At construction time, we don't know the SSRC that is used for the generated
// FEC packets, but we still need to give it to the ForwardErrorCorrection ctor
// to be used in the decoding.
// TODO(brandtr): Get rid of this awkwardness by splitting
// ForwardErrorCorrection in two objects -- one encoder and one decoder.
constexpr uint32_t kUnknownSsrc = 0;
} // namespace
RedPacket::RedPacket(size_t length)
@ -94,7 +101,7 @@ size_t RedPacket::length() const {
}
UlpfecGenerator::UlpfecGenerator()
: UlpfecGenerator(ForwardErrorCorrection::CreateUlpfec()) {}
: UlpfecGenerator(ForwardErrorCorrection::CreateUlpfec(kUnknownSsrc)) {}
UlpfecGenerator::UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec)
: fec_(std::move(fec)),

21
webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
View File

@ -21,13 +21,16 @@
namespace webrtc {
UlpfecReceiver* UlpfecReceiver::Create(RecoveredPacketReceiver* callback) {
return new UlpfecReceiverImpl(callback);
UlpfecReceiver* UlpfecReceiver::Create(uint32_t ssrc,
RecoveredPacketReceiver* callback) {
return new UlpfecReceiverImpl(ssrc, callback);
}
UlpfecReceiverImpl::UlpfecReceiverImpl(RecoveredPacketReceiver* callback)
: recovered_packet_callback_(callback),
fec_(ForwardErrorCorrection::CreateUlpfec()) {}
UlpfecReceiverImpl::UlpfecReceiverImpl(uint32_t ssrc,
RecoveredPacketReceiver* callback)
: ssrc_(ssrc),
recovered_packet_callback_(callback),
fec_(ForwardErrorCorrection::CreateUlpfec(ssrc_)) {}
UlpfecReceiverImpl::~UlpfecReceiverImpl() {
received_packets_.clear();
@ -72,6 +75,12 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket(
const uint8_t* incoming_rtp_packet,
size_t packet_length,
uint8_t ulpfec_payload_type) {
if (header.ssrc != ssrc_) {
LOG(LS_INFO)
<< "Received RED packet with different SSRC than expected; dropping.";
return -1;
}
rtc::CritScope cs(&crit_sect_);
uint8_t red_header_length = 1;
@ -90,6 +99,7 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket(
// Get payload type from RED header and sequence number from RTP header.
uint8_t payload_type = incoming_rtp_packet[header.headerLength] & 0x7f;
received_packet->is_fec = payload_type == ulpfec_payload_type;
received_packet->ssrc = header.ssrc;
received_packet->seq_num = header.sequenceNumber;
uint16_t block_length = 0;
@ -155,6 +165,7 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket(
second_received_packet->pkt = new ForwardErrorCorrection::Packet;
second_received_packet->is_fec = true;
second_received_packet->ssrc = header.ssrc;
second_received_packet->seq_num = header.sequenceNumber;
++packet_counter_.num_fec_packets;

4
webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
View File

@ -23,7 +23,7 @@ namespace webrtc {
class UlpfecReceiverImpl : public UlpfecReceiver {
public:
explicit UlpfecReceiverImpl(RecoveredPacketReceiver* callback);
explicit UlpfecReceiverImpl(uint32_t ssrc, RecoveredPacketReceiver* callback);
virtual ~UlpfecReceiverImpl();
int32_t AddReceivedRedPacket(const RTPHeader& rtp_header,
@ -36,6 +36,8 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
FecPacketCounter GetPacketCounter() const override;
private:
const uint32_t ssrc_;
rtc::CriticalSection crit_sect_;
RecoveredPacketReceiver* recovered_packet_callback_;
std::unique_ptr<ForwardErrorCorrection> fec_;

7
webrtc/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
View File

@ -48,8 +48,9 @@ class NullRecoveredPacketReceiver : public RecoveredPacketReceiver {
class UlpfecReceiverTest : public ::testing::Test {
protected:
UlpfecReceiverTest()
: fec_(ForwardErrorCorrection::CreateUlpfec()),
receiver_fec_(UlpfecReceiver::Create(&recovered_packet_receiver_)),
: fec_(ForwardErrorCorrection::CreateUlpfec(kMediaSsrc)),
receiver_fec_(
UlpfecReceiver::Create(kMediaSsrc, &recovered_packet_receiver_)),
packet_generator_(kMediaSsrc) {}
// Generates |num_fec_packets| FEC packets, given |media_packets|.
@ -179,7 +180,7 @@ void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data,
NullRecoveredPacketReceiver null_callback;
std::unique_ptr<UlpfecReceiver> receiver_fec(
UlpfecReceiver::Create(&null_callback));
UlpfecReceiver::Create(kMediaSsrc, &null_callback));
receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type);
}

52
webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc
View File

@ -78,13 +78,7 @@ void ReceivePackets(
}
}
// Too slow to finish before timeout on iOS. See webrtc:4755.
#if defined(WEBRTC_IOS)
#define MAYBE_FecTest DISABLED_FecTest
#else
#define MAYBE_FecTest FecTest
#endif
TEST(FecTest, MAYBE_FecTest) {
void RunTest(bool use_flexfec) {
// TODO(marpan): Split this function into subroutines/helper functions.
enum { kMaxNumberMediaPackets = 48 };
enum { kMaxNumberFecPackets = 48 };
@ -107,8 +101,6 @@ TEST(FecTest, MAYBE_FecTest) {
ASSERT_EQ(12, kMaxMediaPackets[1]) << "Max media packets for bursty mode not "
<< "equal to 12.";
std::unique_ptr<ForwardErrorCorrection> fec =
ForwardErrorCorrection::CreateUlpfec();
ForwardErrorCorrection::PacketList media_packet_list;
std::list<ForwardErrorCorrection::Packet*> fec_packet_list;
ForwardErrorCorrection::ReceivedPacketList to_decode_list;
@ -138,7 +130,24 @@ TEST(FecTest, MAYBE_FecTest) {
uint16_t seq_num = 0;
uint32_t timestamp = random.Rand<uint32_t>();
const uint32_t ssrc = random.Rand(1u, 0xfffffffe);
const uint32_t media_ssrc = random.Rand(1u, 0xfffffffe);
uint32_t fec_ssrc;
uint16_t fec_seq_num_offset;
if (use_flexfec) {
fec_ssrc = random.Rand(1u, 0xfffffffe);
fec_seq_num_offset = random.Rand<uint16_t>();
} else {
fec_ssrc = media_ssrc;
fec_seq_num_offset = 0;
}
std::unique_ptr<ForwardErrorCorrection> fec;
if (use_flexfec) {
fec = ForwardErrorCorrection::CreateFlexfec(fec_ssrc, media_ssrc);
} else {
RTC_DCHECK_EQ(media_ssrc, fec_ssrc);
fec = ForwardErrorCorrection::CreateUlpfec(fec_ssrc);
}
// Loop over the mask types: random and bursty.
for (int mask_type_idx = 0; mask_type_idx < kNumFecMaskTypes;
@ -268,7 +277,7 @@ TEST(FecTest, MAYBE_FecTest) {
ByteWriter<uint32_t>::WriteBigEndian(&media_packet->data[4],
timestamp);
ByteWriter<uint32_t>::WriteBigEndian(&media_packet->data[8],
ssrc);
media_ssrc);
// Generate random values for payload
for (size_t j = 12; j < media_packet->length; ++j) {
media_packet->data[j] = random.Rand<uint8_t>();
@ -302,6 +311,7 @@ TEST(FecTest, MAYBE_FecTest) {
received_packet->pkt->length = media_packet->length;
memcpy(received_packet->pkt->data, media_packet->data,
media_packet->length);
received_packet->ssrc = media_ssrc;
received_packet->seq_num =
ByteReader<uint16_t>::ReadBigEndian(&media_packet->data[2]);
received_packet->is_fec = false;
@ -323,9 +333,9 @@ TEST(FecTest, MAYBE_FecTest) {
received_packet->pkt->length = fec_packet->length;
memcpy(received_packet->pkt->data, fec_packet->data,
fec_packet->length);
received_packet->seq_num = seq_num;
received_packet->seq_num = fec_seq_num_offset + seq_num;
received_packet->is_fec = true;
received_packet->ssrc = ssrc;
received_packet->ssrc = fec_ssrc;
received_packet_list.push_back(std::move(received_packet));
fec_mask_list.push_back(fec_packet_masks[fec_packet_idx]);
@ -453,5 +463,21 @@ TEST(FecTest, MAYBE_FecTest) {
<< "Recovered packet list is not empty";
}
// Too slow to finish before timeout on iOS. See webrtc:4755.
#if defined(WEBRTC_IOS)
#define MAYBE_UlpecTest DISABLED_UlpecTest
#define MAYBE_FlexfecTest DISABLED_FlexfecTest
#else
#define MAYBE_UlpecTest UlpecTest
#define MAYBE_FlexfecTest FlexfecTest
#endif
TEST(FecTest, MAYBE_UlpecTest) {
RunTest(false);
}
TEST(FecTest, MAYBE_FlexfecTest) {
RunTest(true);
}
} // namespace test
} // namespace webrtc

2
webrtc/video/rtp_video_stream_receiver.cc
View File

@ -100,7 +100,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
this,
&rtp_payload_registry_)),
rtp_receive_statistics_(ReceiveStatistics::Create(clock_)),
ulpfec_receiver_(UlpfecReceiver::Create(this)),
ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)),
receiving_(false),
restored_packet_in_use_(false),
last_packet_log_ms_(-1),