Style updates to ProducerFec/FecReceiver.
- Make more use of std::unique_ptr.
- Auto type deduction for iterator type names.
- More extensive comments.
- Variable renaming.
- Make ProducerFec::BuildRedPacket() static.
- Avoid dynamic allocation of ProducerFec::fec_.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2110763002
Cr-Commit-Position: refs/heads/master@{#13700}
diff --git a/webrtc/modules/rtp_rtcp/include/fec_receiver.h b/webrtc/modules/rtp_rtcp/include/fec_receiver.h
index 65e85ad..453ede4 100644
--- a/webrtc/modules/rtp_rtcp/include/fec_receiver.h
+++ b/webrtc/modules/rtp_rtcp/include/fec_receiver.h
@@ -33,13 +33,18 @@
virtual ~FecReceiver() {}
+ // Takes a RED packet, strips the RED header, and adds the resulting
+ // "virtual" RTP packet(s) into the internal buffer.
virtual int32_t AddReceivedRedPacket(const RTPHeader& rtp_header,
const uint8_t* incoming_rtp_packet,
size_t packet_length,
uint8_t ulpfec_payload_type) = 0;
+ // Sends the received packets to the FEC and returns all packets
+ // (both original media and recovered) through the callback.
virtual int32_t ProcessReceivedFec() = 0;
+ // Returns a counter describing the added and recovered packets.
virtual FecPacketCounter GetPacketCounter() const = 0;
};
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
index 9a1d218..a4a6d29 100644
--- a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
@@ -18,7 +18,6 @@
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h"
-// RFC 5109
namespace webrtc {
FecReceiver* FecReceiver::Create(RtpData* callback) {
@@ -26,15 +25,11 @@
}
FecReceiverImpl::FecReceiverImpl(RtpData* callback)
- : recovered_packet_callback_(callback),
- fec_(new ForwardErrorCorrection()) {}
+ : recovered_packet_callback_(callback) {}
FecReceiverImpl::~FecReceiverImpl() {
- received_packet_list_.clear();
- if (fec_ != NULL) {
- fec_->ResetState(&recovered_packet_list_);
- delete fec_;
- }
+ received_packets_.clear();
+ fec_.ResetState(&recovered_packets_);
}
FecPacketCounter FecReceiverImpl::GetPacketCounter() const {
@@ -74,7 +69,8 @@
const RTPHeader& header, const uint8_t* incoming_rtp_packet,
size_t packet_length, uint8_t ulpfec_payload_type) {
rtc::CritScope cs(&crit_sect_);
- uint8_t REDHeaderLength = 1;
+
+ uint8_t red_header_length = 1;
size_t payload_data_length = packet_length - header.headerLength;
if (payload_data_length == 0) {
@@ -82,31 +78,27 @@
return -1;
}
- // Add to list without RED header, aka a virtual RTP packet
- // we remove the RED header
-
+ // Remove RED header of incoming packet and store as a virtual RTP packet.
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet(
new ForwardErrorCorrection::ReceivedPacket());
received_packet->pkt = new ForwardErrorCorrection::Packet();
- // get payload type from RED header
- uint8_t payload_type =
- incoming_rtp_packet[header.headerLength] & 0x7f;
-
+ // 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->seq_num = header.sequenceNumber;
- uint16_t blockLength = 0;
+ uint16_t block_length = 0;
if (incoming_rtp_packet[header.headerLength] & 0x80) {
- // f bit set in RED header
- REDHeaderLength = 4;
- if (payload_data_length < REDHeaderLength + 1u) {
+ // f bit set in RED header, i.e. there are more than one RED header blocks.
+ red_header_length = 4;
+ if (payload_data_length < red_header_length + 1u) {
LOG(LS_WARNING) << "Corrupt/truncated FEC packet.";
return -1;
}
- uint16_t timestamp_offset =
- (incoming_rtp_packet[header.headerLength + 1]) << 8;
+ uint16_t timestamp_offset = incoming_rtp_packet[header.headerLength + 1]
+ << 8;
timestamp_offset +=
incoming_rtp_packet[header.headerLength + 2];
timestamp_offset = timestamp_offset >> 2;
@@ -115,18 +107,17 @@
return -1;
}
- blockLength =
- (0x03 & incoming_rtp_packet[header.headerLength + 2]) << 8;
- blockLength += (incoming_rtp_packet[header.headerLength + 3]);
+ block_length = (0x3 & incoming_rtp_packet[header.headerLength + 2]) << 8;
+ block_length += incoming_rtp_packet[header.headerLength + 3];
- // check next RED header
+ // Check next RED header block.
if (incoming_rtp_packet[header.headerLength + 4] & 0x80) {
LOG(LS_WARNING) << "More than 2 blocks in packet not supported.";
return -1;
}
// Check that the packet is long enough to contain data in the following
// block.
- if (blockLength > payload_data_length - (REDHeaderLength + 1)) {
+ if (block_length > payload_data_length - (red_header_length + 1)) {
LOG(LS_WARNING) << "Block length longer than packet.";
return -1;
}
@@ -135,92 +126,84 @@
std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>
second_received_packet;
- if (blockLength > 0) {
- // handle block length, split into 2 packets
- REDHeaderLength = 5;
+ if (block_length > 0) {
+ // Handle block length, split into two packets.
+ red_header_length = 5;
- // copy the RTP header
+ // Copy RTP header.
memcpy(received_packet->pkt->data, incoming_rtp_packet,
header.headerLength);
- // replace the RED payload type
- received_packet->pkt->data[1] &= 0x80; // reset the payload
- received_packet->pkt->data[1] +=
- payload_type; // set the media payload type
+ // Set payload type.
+ received_packet->pkt->data[1] &= 0x80; // Reset RED payload type.
+ received_packet->pkt->data[1] += payload_type; // Set media payload type.
- // copy the payload data
- memcpy(
- received_packet->pkt->data + header.headerLength,
- incoming_rtp_packet + header.headerLength + REDHeaderLength,
- blockLength);
+ // Copy payload data.
+ memcpy(received_packet->pkt->data + header.headerLength,
+ incoming_rtp_packet + header.headerLength + red_header_length,
+ block_length);
+ received_packet->pkt->length = block_length;
- received_packet->pkt->length = blockLength;
-
- second_received_packet.reset(new ForwardErrorCorrection::ReceivedPacket());
- second_received_packet->pkt = new ForwardErrorCorrection::Packet();
+ second_received_packet.reset(new ForwardErrorCorrection::ReceivedPacket);
+ second_received_packet->pkt = new ForwardErrorCorrection::Packet;
second_received_packet->is_fec = true;
second_received_packet->seq_num = header.sequenceNumber;
++packet_counter_.num_fec_packets;
- // copy the FEC payload data
+ // Copy FEC payload data.
memcpy(second_received_packet->pkt->data,
- incoming_rtp_packet + header.headerLength +
- REDHeaderLength + blockLength,
- payload_data_length - REDHeaderLength - blockLength);
+ incoming_rtp_packet + header.headerLength + red_header_length +
+ block_length,
+ payload_data_length - red_header_length - block_length);
second_received_packet->pkt->length =
- payload_data_length - REDHeaderLength - blockLength;
+ payload_data_length - red_header_length - block_length;
} else if (received_packet->is_fec) {
++packet_counter_.num_fec_packets;
// everything behind the RED header
- memcpy(
- received_packet->pkt->data,
- incoming_rtp_packet + header.headerLength + REDHeaderLength,
- payload_data_length - REDHeaderLength);
- received_packet->pkt->length = payload_data_length - REDHeaderLength;
+ memcpy(received_packet->pkt->data,
+ incoming_rtp_packet + header.headerLength + red_header_length,
+ payload_data_length - red_header_length);
+ received_packet->pkt->length = payload_data_length - red_header_length;
received_packet->ssrc =
ByteReader<uint32_t>::ReadBigEndian(&incoming_rtp_packet[8]);
} else {
- // copy the RTP header
+ // Copy RTP header.
memcpy(received_packet->pkt->data, incoming_rtp_packet,
header.headerLength);
- // replace the RED payload type
- received_packet->pkt->data[1] &= 0x80; // reset the payload
- received_packet->pkt->data[1] +=
- payload_type; // set the media payload type
+ // Set payload type.
+ received_packet->pkt->data[1] &= 0x80; // Reset RED payload type.
+ received_packet->pkt->data[1] += payload_type; // Set media payload type.
- // copy the media payload data
- memcpy(
- received_packet->pkt->data + header.headerLength,
- incoming_rtp_packet + header.headerLength + REDHeaderLength,
- payload_data_length - REDHeaderLength);
-
+ // Copy payload data.
+ memcpy(received_packet->pkt->data + header.headerLength,
+ incoming_rtp_packet + header.headerLength + red_header_length,
+ payload_data_length - red_header_length);
received_packet->pkt->length =
- header.headerLength + payload_data_length - REDHeaderLength;
+ header.headerLength + payload_data_length - red_header_length;
}
if (received_packet->pkt->length == 0) {
return 0;
}
- received_packet_list_.push_back(std::move(received_packet));
+ received_packets_.push_back(std::move(received_packet));
if (second_received_packet) {
- received_packet_list_.push_back(std::move(second_received_packet));
+ received_packets_.push_back(std::move(second_received_packet));
}
return 0;
}
int32_t FecReceiverImpl::ProcessReceivedFec() {
crit_sect_.Enter();
- if (!received_packet_list_.empty()) {
+ if (!received_packets_.empty()) {
// Send received media packet to VCM.
- if (!received_packet_list_.front()->is_fec) {
- ForwardErrorCorrection::Packet* packet =
- received_packet_list_.front()->pkt;
+ if (!received_packets_.front()->is_fec) {
+ ForwardErrorCorrection::Packet* packet = received_packets_.front()->pkt;
crit_sect_.Leave();
if (!recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length)) {
@@ -228,14 +211,14 @@
}
crit_sect_.Enter();
}
- if (fec_->DecodeFec(&received_packet_list_, &recovered_packet_list_) != 0) {
+ if (fec_.DecodeFec(&received_packets_, &recovered_packets_) != 0) {
crit_sect_.Leave();
return -1;
}
- RTC_DCHECK(received_packet_list_.empty());
+ RTC_DCHECK(received_packets_.empty());
}
// Send any recovered media packets to VCM.
- for (const auto& recovered_packet : recovered_packet_list_) {
+ for (const auto& recovered_packet : recovered_packets_) {
if (recovered_packet->returned) {
// Already sent to the VCM and the jitter buffer.
continue;
diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h
index 0ebca9b..2a3b854 100644
--- a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h
+++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h
@@ -11,8 +11,6 @@
#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_FEC_RECEIVER_IMPL_H_
#define WEBRTC_MODULES_RTP_RTCP_SOURCE_FEC_RECEIVER_IMPL_H_
-// This header is included to get the nested declaration of Packet structure.
-
#include "webrtc/base/criticalsection.h"
#include "webrtc/modules/rtp_rtcp/include/fec_receiver.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -38,12 +36,12 @@
private:
rtc::CriticalSection crit_sect_;
RtpData* recovered_packet_callback_;
- ForwardErrorCorrection* fec_;
- // TODO(holmer): In the current version received_packet_list_ is never more
+ 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_packet_list_;
- ForwardErrorCorrection::RecoveredPacketList recovered_packet_list_;
+ ForwardErrorCorrection::ReceivedPacketList received_packets_;
+ ForwardErrorCorrection::RecoveredPacketList recovered_packets_;
FecPacketCounter packet_counter_;
};
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
index 32d4a73..1c30f37 100644
--- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc
@@ -72,19 +72,19 @@
}
void BuildAndAddRedMediaPacket(test::RawRtpPacket* packet) {
- test::RawRtpPacket* red_packet = generator_->BuildMediaRedPacket(packet);
+ std::unique_ptr<test::RawRtpPacket> red_packet(
+ generator_->BuildMediaRedPacket(packet));
EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
red_packet->header.header, red_packet->data,
red_packet->length, kFecPayloadType));
- delete red_packet;
}
void BuildAndAddRedFecPacket(Packet* packet) {
- test::RawRtpPacket* red_packet = generator_->BuildFecRedPacket(packet);
+ std::unique_ptr<test::RawRtpPacket> red_packet(
+ generator_->BuildFecRedPacket(packet));
EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
red_packet->header.header, red_packet->data,
red_packet->length, kFecPayloadType));
- delete red_packet;
}
void InjectGarbagePacketLength(size_t fec_garbage_offset);
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
index c27472b..b928020 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
@@ -13,27 +13,39 @@
#include <memory>
#include <utility>
+#include "webrtc/base/basictypes.h"
+#include "webrtc/base/checks.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
#include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h"
namespace webrtc {
-enum { kREDForFECHeaderLength = 1 };
+constexpr size_t kRedForFecHeaderLength = 1;
+
// This controls the maximum amount of excess overhead (actual - target)
// allowed in order to trigger GenerateFec(), before |params_.max_fec_frames|
// is reached. Overhead here is defined as relative to number of media packets.
-enum { kMaxExcessOverhead = 50 }; // Q8.
+constexpr int kMaxExcessOverhead = 50; // Q8.
+
// This is the minimum number of media packets required (above some protection
// level) in order to trigger GenerateFec(), before |params_.max_fec_frames| is
// reached.
-enum { kMinimumMediaPackets = 4 };
+constexpr size_t kMinMediaPackets = 4;
+
// Threshold on the received FEC protection level, above which we enforce at
-// least |kMinimumMediaPackets| packets for the FEC code. Below this
-// threshold |kMinimumMediaPackets| is set to default value of 1.
-enum { kHighProtectionThreshold = 80 }; // Corresponds to ~30 overhead, range
-// is 0 to 255, where 255 corresponds to 100% overhead (relative to number of
-// media packets).
+// least |kMinMediaPackets| packets for the FEC code. Below this
+// threshold |kMinMediaPackets| is set to default value of 1.
+//
+// The range is between 0 and 255, where 255 corresponds to 100% overhead
+// (relative to the number of protected media packets).
+constexpr uint8_t kHighProtectionThreshold = 80;
+
+// This threshold is used to adapt the |kMinMediaPackets| threshold, based
+// on the average number of packets per frame seen so far. When there are few
+// packets per frame (as given by this threshold), at least
+// |kMinMediaPackets| + 1 packets are sent to the FEC code.
+constexpr float kMinMediaPacketsAdaptationThreshold = 2.0f;
RedPacket::RedPacket(size_t length)
: data_(new uint8_t[length]),
@@ -41,32 +53,31 @@
header_length_(0) {
}
-RedPacket::~RedPacket() {
- delete [] data_;
-}
-
-void RedPacket::CreateHeader(const uint8_t* rtp_header, size_t header_length,
- int red_pl_type, int pl_type) {
- assert(header_length + kREDForFECHeaderLength <= length_);
- memcpy(data_, rtp_header, header_length);
+void RedPacket::CreateHeader(const uint8_t* rtp_header,
+ size_t header_length,
+ int red_payload_type,
+ int payload_type) {
+ RTC_DCHECK_LT(header_length + kRedForFecHeaderLength, length_);
+ memcpy(data_.get(), rtp_header, header_length);
// Replace payload type.
data_[1] &= 0x80;
- data_[1] += red_pl_type;
+ data_[1] += red_payload_type;
// Add RED header
// f-bit always 0
- data_[header_length] = static_cast<uint8_t>(pl_type);
- header_length_ = header_length + kREDForFECHeaderLength;
+ data_[header_length] = static_cast<uint8_t>(payload_type);
+ header_length_ = header_length + kRedForFecHeaderLength;
}
void RedPacket::SetSeqNum(int seq_num) {
- assert(seq_num >= 0 && seq_num < (1<<16));
+ RTC_DCHECK_GE(seq_num, 0);
+ RTC_DCHECK_LT(seq_num, 1 << 16);
ByteWriter<uint16_t>::WriteBigEndian(&data_[2], seq_num);
}
void RedPacket::AssignPayload(const uint8_t* payload, size_t length) {
- assert(header_length_ + length <= length_);
- memcpy(data_ + header_length_, payload, length);
+ RTC_DCHECK_LE(header_length_ + length, length_);
+ memcpy(data_.get() + header_length_, payload, length);
}
void RedPacket::ClearMarkerBit() {
@@ -74,7 +85,7 @@
}
uint8_t* RedPacket::data() const {
- return data_;
+ return data_.get();
}
size_t RedPacket::length() const {
@@ -83,11 +94,11 @@
ProducerFec::ProducerFec(ForwardErrorCorrection* fec)
: fec_(fec),
- media_packets_fec_(),
- fec_packets_(),
- num_frames_(0),
- num_first_partition_(0),
- minimum_media_packets_fec_(1),
+ media_packets_(),
+ generated_fec_packets_(),
+ num_protected_frames_(0),
+ num_important_packets_(0),
+ min_num_media_packets_(1),
params_(),
new_params_() {
memset(¶ms_, 0, sizeof(params_));
@@ -95,166 +106,165 @@
}
ProducerFec::~ProducerFec() {
- DeletePackets();
+ DeleteMediaPackets();
+}
+
+std::unique_ptr<RedPacket> ProducerFec::BuildRedPacket(
+ const uint8_t* data_buffer,
+ size_t payload_length,
+ size_t rtp_header_length,
+ int red_payload_type) {
+ std::unique_ptr<RedPacket> red_packet(new RedPacket(
+ payload_length + kRedForFecHeaderLength + rtp_header_length));
+ int payload_type = data_buffer[1] & 0x7f;
+ red_packet->CreateHeader(data_buffer, rtp_header_length, red_payload_type,
+ payload_type);
+ red_packet->AssignPayload(data_buffer + rtp_header_length, payload_length);
+ return red_packet;
}
void ProducerFec::SetFecParameters(const FecProtectionParams* params,
- int num_first_partition) {
- // Number of first partition packets cannot exceed kMaxMediaPackets
- assert(params->fec_rate >= 0 && params->fec_rate < 256);
- if (num_first_partition >
+ int num_important_packets) {
+ // Number of important packets (i.e. number of packets receiving additional
+ // protection in 'unequal protection mode') cannot exceed kMaxMediaPackets.
+ RTC_DCHECK_GE(params->fec_rate, 0);
+ RTC_DCHECK_LE(params->fec_rate, 255);
+ if (num_important_packets >
static_cast<int>(ForwardErrorCorrection::kMaxMediaPackets)) {
- num_first_partition =
- ForwardErrorCorrection::kMaxMediaPackets;
+ num_important_packets = ForwardErrorCorrection::kMaxMediaPackets;
}
// Store the new params and apply them for the next set of FEC packets being
// produced.
new_params_ = *params;
- num_first_partition_ = num_first_partition;
+ num_important_packets_ = num_important_packets;
if (params->fec_rate > kHighProtectionThreshold) {
- minimum_media_packets_fec_ = kMinimumMediaPackets;
+ min_num_media_packets_ = kMinMediaPackets;
} else {
- minimum_media_packets_fec_ = 1;
+ min_num_media_packets_ = 1;
}
}
-RedPacket* ProducerFec::BuildRedPacket(const uint8_t* data_buffer,
- size_t payload_length,
- size_t rtp_header_length,
- int red_pl_type) {
- RedPacket* red_packet = new RedPacket(
- payload_length + kREDForFECHeaderLength + rtp_header_length);
- int pl_type = data_buffer[1] & 0x7f;
- red_packet->CreateHeader(data_buffer, rtp_header_length,
- red_pl_type, pl_type);
- red_packet->AssignPayload(data_buffer + rtp_header_length, payload_length);
- return red_packet;
-}
-
int ProducerFec::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer,
size_t payload_length,
size_t rtp_header_length) {
- assert(fec_packets_.empty());
- if (media_packets_fec_.empty()) {
+ RTC_DCHECK(generated_fec_packets_.empty());
+ if (media_packets_.empty()) {
params_ = new_params_;
}
bool complete_frame = false;
const bool marker_bit = (data_buffer[1] & kRtpMarkerBitMask) ? true : false;
- if (media_packets_fec_.size() < ForwardErrorCorrection::kMaxMediaPackets) {
- // Generic FEC can only protect up to kMaxMediaPackets packets.
+ if (media_packets_.size() < ForwardErrorCorrection::kMaxMediaPackets) {
+ // Generic FEC can only protect up to |kMaxMediaPackets| packets.
std::unique_ptr<ForwardErrorCorrection::Packet> packet(
new ForwardErrorCorrection::Packet());
packet->length = payload_length + rtp_header_length;
memcpy(packet->data, data_buffer, packet->length);
- media_packets_fec_.push_back(std::move(packet));
+ media_packets_.push_back(std::move(packet));
}
if (marker_bit) {
- ++num_frames_;
+ ++num_protected_frames_;
complete_frame = true;
}
// Produce FEC over at most |params_.max_fec_frames| frames, or as soon as:
// (1) the excess overhead (actual overhead - requested/target overhead) is
// less than |kMaxExcessOverhead|, and
- // (2) at least |minimum_media_packets_fec_| media packets is reached.
+ // (2) at least |min_num_media_packets_| media packets is reached.
if (complete_frame &&
- (num_frames_ == params_.max_fec_frames ||
- (ExcessOverheadBelowMax() && MinimumMediaPacketsReached()))) {
- assert(num_first_partition_ <=
- static_cast<int>(ForwardErrorCorrection::kMaxMediaPackets));
+ (num_protected_frames_ == params_.max_fec_frames ||
+ (ExcessOverheadBelowMax() && MinimumMediaPacketsReached()))) {
+ RTC_DCHECK_LE(num_important_packets_,
+ static_cast<int>(ForwardErrorCorrection::kMaxMediaPackets));
// TODO(pbos): Consider whether unequal protection should be enabled or not,
// it is currently always disabled.
- int ret = fec_->GenerateFec(media_packets_fec_, params_.fec_rate,
- num_first_partition_, false,
- params_.fec_mask_type, &fec_packets_);
- if (fec_packets_.empty()) {
- num_frames_ = 0;
- DeletePackets();
+ //
+ // Since unequal protection is disabled, the value of
+ // |num_important_packets_| has no importance when calling GenerateFec().
+ constexpr bool kUseUnequalProtection = false;
+ int ret = fec_->GenerateFec(media_packets_, params_.fec_rate,
+ num_important_packets_, kUseUnequalProtection,
+ params_.fec_mask_type, &generated_fec_packets_);
+ if (generated_fec_packets_.empty()) {
+ num_protected_frames_ = 0;
+ DeleteMediaPackets();
}
return ret;
}
return 0;
}
-// Returns true if the excess overhead (actual - target) for the FEC is below
-// the amount |kMaxExcessOverhead|. This effects the lower protection level
-// cases and low number of media packets/frame. The target overhead is given by
-// |params_.fec_rate|, and is only achievable in the limit of large number of
-// media packets.
-bool ProducerFec::ExcessOverheadBelowMax() {
+bool ProducerFec::ExcessOverheadBelowMax() const {
return ((Overhead() - params_.fec_rate) < kMaxExcessOverhead);
}
-// Returns true if the media packet list for the FEC is at least
-// |minimum_media_packets_fec_|. This condition tries to capture the effect
-// that, for the same amount of protection/overhead, longer codes
-// (e.g. (2k,2m) vs (k,m)) are generally more effective at recovering losses.
-bool ProducerFec::MinimumMediaPacketsReached() {
- float avg_num_packets_frame = static_cast<float>(media_packets_fec_.size()) /
- num_frames_;
- if (avg_num_packets_frame < 2.0f) {
- return (static_cast<int>(media_packets_fec_.size()) >=
- minimum_media_packets_fec_);
+bool ProducerFec::MinimumMediaPacketsReached() const {
+ float average_num_packets_per_frame =
+ static_cast<float>(media_packets_.size()) / num_protected_frames_;
+ int num_media_packets = static_cast<int>(media_packets_.size());
+ if (average_num_packets_per_frame < kMinMediaPacketsAdaptationThreshold) {
+ return num_media_packets >= min_num_media_packets_;
} else {
// For larger rates (more packets/frame), increase the threshold.
- return (static_cast<int>(media_packets_fec_.size()) >=
- minimum_media_packets_fec_ + 1);
+ // TODO(brandtr): Investigate what impact this adaptation has.
+ return num_media_packets >= min_num_media_packets_ + 1;
}
}
bool ProducerFec::FecAvailable() const {
- return !fec_packets_.empty();
+ return !generated_fec_packets_.empty();
}
size_t ProducerFec::NumAvailableFecPackets() const {
- return fec_packets_.size();
+ return generated_fec_packets_.size();
}
-std::vector<RedPacket*> ProducerFec::GetFecPackets(int red_pl_type,
- int fec_pl_type,
- uint16_t first_seq_num,
- size_t rtp_header_length) {
- std::vector<RedPacket*> fec_packets;
- fec_packets.reserve(fec_packets_.size());
- uint16_t sequence_number = first_seq_num;
- while (!fec_packets_.empty()) {
- // Build FEC packet. The FEC packets in |fec_packets_| doesn't
- // have RTP headers, so we're reusing the header from the last
- // media packet.
- ForwardErrorCorrection::Packet* packet_to_send = fec_packets_.front();
- ForwardErrorCorrection::Packet* last_media_packet =
- media_packets_fec_.back().get();
-
- RedPacket* red_packet = new RedPacket(
- packet_to_send->length + kREDForFECHeaderLength + rtp_header_length);
+std::vector<std::unique_ptr<RedPacket>> ProducerFec::GetFecPacketsAsRed(
+ int red_payload_type,
+ int ulpfec_payload_type,
+ uint16_t first_seq_num,
+ size_t rtp_header_length) {
+ std::vector<std::unique_ptr<RedPacket>> red_packets;
+ red_packets.reserve(generated_fec_packets_.size());
+ RTC_DCHECK(!media_packets_.empty());
+ ForwardErrorCorrection::Packet* last_media_packet =
+ media_packets_.back().get();
+ uint16_t seq_num = first_seq_num;
+ for (const auto& fec_packet : generated_fec_packets_) {
+ // Wrap FEC packet (including FEC headers) in a RED packet. Since the
+ // FEC packets in |generated_fec_packets_| don't have RTP headers, we
+ // reuse the header from the last media packet.
+ std::unique_ptr<RedPacket> red_packet(new RedPacket(
+ fec_packet->length + kRedForFecHeaderLength + rtp_header_length));
red_packet->CreateHeader(last_media_packet->data, rtp_header_length,
- red_pl_type, fec_pl_type);
- red_packet->SetSeqNum(sequence_number++);
+ red_payload_type, ulpfec_payload_type);
+ red_packet->SetSeqNum(seq_num++);
red_packet->ClearMarkerBit();
- red_packet->AssignPayload(packet_to_send->data, packet_to_send->length);
+ red_packet->AssignPayload(fec_packet->data, fec_packet->length);
- fec_packets.push_back(red_packet);
-
- fec_packets_.pop_front();
+ red_packets.push_back(std::move(red_packet));
}
- DeletePackets();
- num_frames_ = 0;
- return fec_packets;
+
+ // Reset state.
+ DeleteMediaPackets();
+ generated_fec_packets_.clear();
+ num_protected_frames_ = 0;
+
+ return red_packets;
}
int ProducerFec::Overhead() const {
// Overhead is defined as relative to the number of media packets, and not
- // relative to total number of packets. This definition is inhereted from the
+ // relative to total number of packets. This definition is inherited from the
// protection factor produced by video_coding module and how the FEC
// generation is implemented.
- assert(!media_packets_fec_.empty());
- int num_fec_packets = fec_->GetNumberOfFecPackets(media_packets_fec_.size(),
- params_.fec_rate);
+ RTC_DCHECK(!media_packets_.empty());
+ int num_fec_packets =
+ fec_->GetNumberOfFecPackets(media_packets_.size(), params_.fec_rate);
// Return the overhead in Q8.
- return (num_fec_packets << 8) / media_packets_fec_.size();
+ return (num_fec_packets << 8) / media_packets_.size();
}
-void ProducerFec::DeletePackets() {
- media_packets_fec_.clear();
+void ProducerFec::DeleteMediaPackets() {
+ media_packets_.clear();
}
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h
index fc2e8e4a..a65bb05 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.h
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h
@@ -12,6 +12,7 @@
#define WEBRTC_MODULES_RTP_RTCP_SOURCE_PRODUCER_FEC_H_
#include <list>
+#include <memory>
#include <vector>
#include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h"
@@ -21,9 +22,11 @@
class RedPacket {
public:
explicit RedPacket(size_t length);
- ~RedPacket();
- void CreateHeader(const uint8_t* rtp_header, size_t header_length,
- int red_pl_type, int pl_type);
+
+ void CreateHeader(const uint8_t* rtp_header,
+ size_t header_length,
+ int red_payload_type,
+ int payload_type);
void SetSeqNum(int seq_num);
void AssignPayload(const uint8_t* payload, size_t length);
void ClearMarkerBit();
@@ -31,7 +34,7 @@
size_t length() const;
private:
- uint8_t* data_;
+ std::unique_ptr<uint8_t[]> data_;
size_t length_;
size_t header_length_;
};
@@ -41,42 +44,55 @@
explicit ProducerFec(ForwardErrorCorrection* fec);
~ProducerFec();
+ static std::unique_ptr<RedPacket> BuildRedPacket(const uint8_t* data_buffer,
+ size_t payload_length,
+ size_t rtp_header_length,
+ int red_payload_type);
+
void SetFecParameters(const FecProtectionParams* params,
- int max_fec_frames);
+ int num_first_partition);
- // The caller is expected to delete the memory when done.
- RedPacket* BuildRedPacket(const uint8_t* data_buffer,
- size_t payload_length,
- size_t rtp_header_length,
- int red_pl_type);
-
+ // Adds a media packet to the internal buffer. When enough media packets
+ // have been added, the FEC packets are generated and stored internally.
+ // These FEC packets are then obtained by calling GetFecPacketsAsRed().
int AddRtpPacketAndGenerateFec(const uint8_t* data_buffer,
size_t payload_length,
size_t rtp_header_length);
- bool ExcessOverheadBelowMax();
+ // Returns true if the excess overhead (actual - target) for the FEC is below
+ // the amount |kMaxExcessOverhead|. This effects the lower protection level
+ // cases and low number of media packets/frame. The target overhead is given
+ // by |params_.fec_rate|, and is only achievable in the limit of large number
+ // of media packets.
+ bool ExcessOverheadBelowMax() const;
- bool MinimumMediaPacketsReached();
+ // Returns true if the number of added media packets is at least
+ // |min_num_media_packets_|. This condition tries to capture the effect
+ // that, for the same amount of protection/overhead, longer codes
+ // (e.g. (2k,2m) vs (k,m)) are generally more effective at recovering losses.
+ bool MinimumMediaPacketsReached() const;
+ // Returns true if there are generated FEC packets available.
bool FecAvailable() const;
+
size_t NumAvailableFecPackets() const;
- // GetFecPackets allocates memory and creates FEC packets, but the caller is
- // assumed to delete the memory when done with the packets.
- std::vector<RedPacket*> GetFecPackets(int red_pl_type,
- int fec_pl_type,
- uint16_t first_seq_num,
- size_t rtp_header_length);
+ // Returns generated FEC packets with RED headers added.
+ std::vector<std::unique_ptr<RedPacket>> GetFecPacketsAsRed(
+ int red_payload_type,
+ int ulpfec_payload_type,
+ uint16_t first_seq_num,
+ size_t rtp_header_length);
private:
- void DeletePackets();
+ void DeleteMediaPackets();
int Overhead() const;
ForwardErrorCorrection* fec_;
- ForwardErrorCorrection::PacketList media_packets_fec_;
- std::list<ForwardErrorCorrection::Packet*> fec_packets_;
- int num_frames_;
- int num_first_partition_;
- int minimum_media_packets_fec_;
+ ForwardErrorCorrection::PacketList media_packets_;
+ std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
+ int num_protected_frames_;
+ int num_important_packets_;
+ int min_num_media_packets_;
FecProtectionParams params_;
FecProtectionParams new_params_;
};
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
index ea59dc1..8b84cf5 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
@@ -22,8 +22,8 @@
void VerifyHeader(uint16_t seq_num,
uint32_t timestamp,
- int red_pltype,
- int fec_pltype,
+ int red_payload_type,
+ int fec_payload_type,
RedPacket* packet,
bool marker_bit) {
EXPECT_GT(packet->length(), kRtpHeaderSize);
@@ -31,30 +31,21 @@
uint8_t* data = packet->data();
// Marker bit not set.
EXPECT_EQ(marker_bit ? 0x80 : 0, data[1] & 0x80);
- EXPECT_EQ(red_pltype, data[1] & 0x7F);
+ EXPECT_EQ(red_payload_type, data[1] & 0x7F);
EXPECT_EQ(seq_num, (data[2] << 8) + data[3]);
uint32_t parsed_timestamp = (data[4] << 24) + (data[5] << 16) +
(data[6] << 8) + data[7];
EXPECT_EQ(timestamp, parsed_timestamp);
- EXPECT_EQ(static_cast<uint8_t>(fec_pltype), data[kRtpHeaderSize]);
+ EXPECT_EQ(static_cast<uint8_t>(fec_payload_type), data[kRtpHeaderSize]);
}
class ProducerFecTest : public ::testing::Test {
protected:
- virtual void SetUp() {
- fec_ = new ForwardErrorCorrection();
- producer_ = new ProducerFec(fec_);
- generator_ = new FrameGenerator();
- }
+ ProducerFecTest() : producer_(&fec_) {}
- virtual void TearDown() {
- delete producer_;
- delete fec_;
- delete generator_;
- }
- ForwardErrorCorrection* fec_;
- ProducerFec* producer_;
- FrameGenerator* generator_;
+ ForwardErrorCorrection fec_;
+ ProducerFec producer_;
+ FrameGenerator generator_;
};
// Verifies bug found via fuzzing, where a gap in the packet sequence caused us
@@ -80,7 +71,7 @@
protected_packets.push_back({21, 0, 55, 0});
protected_packets.push_back({13, 3, 57, 1});
FecProtectionParams params = {117, 3, kFecMaskBursty};
- producer_->SetFecParameters(¶ms, 0);
+ producer_.SetFecParameters(¶ms, 0);
uint8_t packet[28] = {0};
for (Packet p : protected_packets) {
if (p.marker_bit) {
@@ -89,18 +80,14 @@
packet[1] &= ~0x80;
}
ByteWriter<uint16_t>::WriteBigEndian(&packet[2], p.seq_num);
- producer_->AddRtpPacketAndGenerateFec(packet, p.payload_size,
- p.header_size);
- uint16_t num_fec_packets = producer_->NumAvailableFecPackets();
- std::vector<RedPacket*> fec_packets;
+ producer_.AddRtpPacketAndGenerateFec(packet, p.payload_size, p.header_size);
+ uint16_t num_fec_packets = producer_.NumAvailableFecPackets();
if (num_fec_packets > 0) {
- fec_packets =
- producer_->GetFecPackets(kRedPayloadType, 99, 100, p.header_size);
+ std::vector<std::unique_ptr<RedPacket>> fec_packets =
+ producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
+ p.header_size);
EXPECT_EQ(num_fec_packets, fec_packets.size());
}
- for (RedPacket* fec_packet : fec_packets) {
- delete fec_packet;
- }
}
}
@@ -114,32 +101,29 @@
const int kNumPackets = 4;
FecProtectionParams params = {15, 3, kFecMaskRandom};
std::list<test::RawRtpPacket*> rtp_packets;
- generator_->NewFrame(kNumPackets);
- producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet.
+ generator_.NewFrame(kNumPackets);
+ producer_.SetFecParameters(¶ms, 0); // Expecting one FEC packet.
uint32_t last_timestamp = 0;
for (int i = 0; i < kNumPackets; ++i) {
- test::RawRtpPacket* rtp_packet = generator_->NextPacket(i, 10);
+ test::RawRtpPacket* rtp_packet = generator_.NextPacket(i, 10);
rtp_packets.push_back(rtp_packet);
- EXPECT_EQ(0, producer_->AddRtpPacketAndGenerateFec(rtp_packet->data,
- rtp_packet->length,
- kRtpHeaderSize));
+ EXPECT_EQ(0, producer_.AddRtpPacketAndGenerateFec(
+ rtp_packet->data, rtp_packet->length, kRtpHeaderSize));
last_timestamp = rtp_packet->header.header.timestamp;
}
- EXPECT_TRUE(producer_->FecAvailable());
- uint16_t seq_num = generator_->NextSeqNum();
- std::vector<RedPacket*> packets = producer_->GetFecPackets(kRedPayloadType,
- kFecPayloadType,
- seq_num,
- kRtpHeaderSize);
- EXPECT_FALSE(producer_->FecAvailable());
+ EXPECT_TRUE(producer_.FecAvailable());
+ uint16_t seq_num = generator_.NextSeqNum();
+ std::vector<std::unique_ptr<RedPacket>> packets =
+ producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
+ kRtpHeaderSize);
+ EXPECT_FALSE(producer_.FecAvailable());
ASSERT_EQ(1u, packets.size());
- VerifyHeader(seq_num, last_timestamp,
- kRedPayloadType, kFecPayloadType, packets.front(), false);
+ VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
+ packets.front().get(), false);
while (!rtp_packets.empty()) {
delete rtp_packets.front();
rtp_packets.pop_front();
}
- delete packets.front();
}
TEST_F(ProducerFecTest, TwoFrameFec) {
@@ -155,43 +139,40 @@
FecProtectionParams params = {15, 3, kFecMaskRandom};
std::list<test::RawRtpPacket*> rtp_packets;
- producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet.
+ producer_.SetFecParameters(¶ms, 0); // Expecting one FEC packet.
uint32_t last_timestamp = 0;
for (int i = 0; i < kNumFrames; ++i) {
- generator_->NewFrame(kNumPackets);
+ generator_.NewFrame(kNumPackets);
for (int j = 0; j < kNumPackets; ++j) {
test::RawRtpPacket* rtp_packet =
- generator_->NextPacket(i * kNumPackets + j, 10);
+ generator_.NextPacket(i * kNumPackets + j, 10);
rtp_packets.push_back(rtp_packet);
- EXPECT_EQ(0, producer_->AddRtpPacketAndGenerateFec(rtp_packet->data,
- rtp_packet->length,
- kRtpHeaderSize));
+ EXPECT_EQ(0, producer_.AddRtpPacketAndGenerateFec(
+ rtp_packet->data, rtp_packet->length, kRtpHeaderSize));
last_timestamp = rtp_packet->header.header.timestamp;
}
}
- EXPECT_TRUE(producer_->FecAvailable());
- uint16_t seq_num = generator_->NextSeqNum();
- std::vector<RedPacket*> packets = producer_->GetFecPackets(kRedPayloadType,
- kFecPayloadType,
- seq_num,
- kRtpHeaderSize);
- EXPECT_FALSE(producer_->FecAvailable());
+ EXPECT_TRUE(producer_.FecAvailable());
+ uint16_t seq_num = generator_.NextSeqNum();
+ std::vector<std::unique_ptr<RedPacket>> packets =
+ producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
+ kRtpHeaderSize);
+ EXPECT_FALSE(producer_.FecAvailable());
ASSERT_EQ(1u, packets.size());
VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
- packets.front(), false);
+ packets.front().get(), false);
while (!rtp_packets.empty()) {
delete rtp_packets.front();
rtp_packets.pop_front();
}
- delete packets.front();
}
TEST_F(ProducerFecTest, BuildRedPacket) {
- generator_->NewFrame(1);
- test::RawRtpPacket* packet = generator_->NextPacket(0, 10);
- std::unique_ptr<RedPacket> red_packet(producer_->BuildRedPacket(
- packet->data, packet->length - kRtpHeaderSize, kRtpHeaderSize,
- kRedPayloadType));
+ generator_.NewFrame(1);
+ test::RawRtpPacket* packet = generator_.NextPacket(0, 10);
+ std::unique_ptr<RedPacket> red_packet =
+ ProducerFec::BuildRedPacket(packet->data, packet->length - kRtpHeaderSize,
+ kRtpHeaderSize, kRedPayloadType);
EXPECT_EQ(packet->length + 1, red_packet->length());
VerifyHeader(packet->header.header.sequenceNumber,
packet->header.header.timestamp,
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
index d1b3ce3..a7d5537 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -102,14 +102,14 @@
StorageType media_packet_storage,
bool protect) {
std::unique_ptr<RedPacket> red_packet;
- std::vector<RedPacket*> fec_packets;
+ std::vector<std::unique_ptr<RedPacket>> fec_packets;
StorageType fec_storage = kDontRetransmit;
uint16_t next_fec_sequence_number = 0;
{
// Only protect while creating RED and FEC packets, not when sending.
rtc::CritScope cs(&crit_);
- red_packet.reset(producer_fec_.BuildRedPacket(
- data_buffer, payload_length, rtp_header_length, red_payload_type_));
+ red_packet = ProducerFec::BuildRedPacket(
+ data_buffer, payload_length, rtp_header_length, red_payload_type_);
if (protect) {
producer_fec_.AddRtpPacketAndGenerateFec(data_buffer, payload_length,
rtp_header_length);
@@ -118,7 +118,7 @@
if (num_fec_packets > 0) {
next_fec_sequence_number =
rtp_sender_->AllocateSequenceNumber(num_fec_packets);
- fec_packets = producer_fec_.GetFecPackets(
+ fec_packets = producer_fec_.GetFecPacketsAsRed(
red_payload_type_, fec_payload_type_, next_fec_sequence_number,
rtp_header_length);
RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
@@ -138,7 +138,7 @@
} else {
LOG(LS_WARNING) << "Failed to send RED packet " << media_seq_num;
}
- for (RedPacket* fec_packet : fec_packets) {
+ for (const auto& fec_packet : fec_packets) {
if (rtp_sender_->SendToNetwork(
fec_packet->data(), fec_packet->length() - rtp_header_length,
rtp_header_length, capture_time_ms, fec_storage,
@@ -152,7 +152,6 @@
LOG(LS_WARNING) << "Failed to send FEC packet "
<< next_fec_sequence_number;
}
- delete fec_packet;
++next_fec_sequence_number;
}
}
@@ -229,7 +228,11 @@
rtc::CritScope cs(&crit_);
FecProtectionParams* fec_params =
frame_type == kVideoFrameKey ? &key_fec_params_ : &delta_fec_params_;
- producer_fec_.SetFecParameters(fec_params, 0);
+ // We currently do not use unequal protection in the FEC.
+ // This is signalled both here (by setting the number of important
+ // packets to zero), as well as in ProducerFec::AddRtpPacketAndGenerateFec.
+ constexpr int kNumImportantPackets = 0;
+ producer_fec_.SetFecParameters(fec_params, kNumImportantPackets);
storage = packetizer->GetStorageType(retransmission_settings_);
red_payload_type = red_payload_type_;
}
diff --git a/webrtc/test/fuzzers/producer_fec_fuzzer.cc b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
index 53f7493..cac31d1 100644
--- a/webrtc/test/fuzzers/producer_fec_fuzzer.cc
+++ b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
@@ -12,6 +12,7 @@
#include "webrtc/base/checks.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+#include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h"
#include "webrtc/modules/rtp_rtcp/source/producer_fec.h"
namespace webrtc {
@@ -38,9 +39,8 @@
ByteWriter<uint16_t>::WriteBigEndian(&packet[2], seq_num++);
i += payload_size + rtp_header_length;
// Make sure sequence numbers are increasing.
- const int kRedPayloadType = 98;
- std::unique_ptr<RedPacket> red_packet(producer.BuildRedPacket(
- packet.get(), payload_size, rtp_header_length, kRedPayloadType));
+ std::unique_ptr<RedPacket> red_packet = ProducerFec::BuildRedPacket(
+ packet.get(), payload_size, rtp_header_length, kRedPayloadType);
const bool protect = data[i++] % 2 == 1;
if (protect) {
producer.AddRtpPacketAndGenerateFec(packet.get(), payload_size,
@@ -48,11 +48,10 @@
}
const size_t num_fec_packets = producer.NumAvailableFecPackets();
if (num_fec_packets > 0) {
- std::vector<RedPacket*> fec_packets =
- producer.GetFecPackets(kRedPayloadType, 99, 100, rtp_header_length);
+ std::vector<std::unique_ptr<RedPacket>> fec_packets =
+ producer.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
+ rtp_header_length);
RTC_CHECK_EQ(num_fec_packets, fec_packets.size());
- for (RedPacket* fec_packet : fec_packets)
- delete fec_packet;
}
}
}
