Minor fixes in FEC and RtpSender{,Video}
- Rename GetNumberOfFecPackets -> NumFecPackets and
PacketOverhead -> MaxPacketOverhead in ForwardErrorCorrection.
- Rename FECPacketOverhead -> FecPacketOverhead in ProducerFec.
- Move ownership of ForwardErrorCorrection from RTPSenderVideo
to ProducerFec.
- Make MaxPacketOverhead a member function of ForwardErrorCorrection.
This will allow for changing it, based on FEC header types, later on.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2275443002
Cr-Commit-Position: refs/heads/master@{#14194}
diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc
index e666421..b214f60 100644
--- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc
+++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc
@@ -119,18 +119,16 @@
bool l_bit = (num_media_packets > 8 * kMaskSizeLBitClear);
int num_mask_bytes = l_bit ? kMaskSizeLBitSet : kMaskSizeLBitClear;
- // Do some error checking on the media packets.
+ // Error check the media packets.
for (const auto& media_packet : media_packets) {
RTC_DCHECK(media_packet);
-
if (media_packet->length < kRtpHeaderSize) {
LOG(LS_WARNING) << "Media packet " << media_packet->length << " bytes "
<< "is smaller than RTP header.";
return -1;
}
-
- // Ensure our FEC packets will fit in a typical MTU.
- if (media_packet->length + PacketOverhead() + kTransportOverhead >
+ // Ensure the FEC packets will fit in a typical MTU.
+ if (media_packet->length + MaxPacketOverhead() + kTransportOverhead >
IP_PACKET_SIZE) {
LOG(LS_WARNING) << "Media packet " << media_packet->length << " bytes "
<< "with overhead is larger than " << IP_PACKET_SIZE
@@ -138,8 +136,7 @@
}
}
- int num_fec_packets = GetNumberOfFecPackets(num_media_packets,
- protection_factor);
+ int num_fec_packets = NumFecPackets(num_media_packets, protection_factor);
if (num_fec_packets == 0) {
return 0;
}
@@ -177,8 +174,8 @@
return 0;
}
-int ForwardErrorCorrection::GetNumberOfFecPackets(int num_media_packets,
- int protection_factor) {
+int ForwardErrorCorrection::NumFecPackets(int num_media_packets,
+ int protection_factor) {
// Result in Q0 with an unsigned round.
int num_fec_packets = (num_media_packets * protection_factor + (1 << 7)) >> 8;
// Generate at least one FEC packet if we need protection.
@@ -776,7 +773,7 @@
return 0;
}
-size_t ForwardErrorCorrection::PacketOverhead() {
+size_t ForwardErrorCorrection::MaxPacketOverhead() const {
return kFecHeaderSize + kUlpHeaderSizeLBitSet;
}
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
index ac2cfa7..7d2e5aa 100644
--- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
+++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
@@ -186,11 +186,11 @@
// Get the number of generated FEC packets, given the number of media packets
// and the protection factor.
- int GetNumberOfFecPackets(int num_media_packets, int protection_factor);
+ static int NumFecPackets(int num_media_packets, int protection_factor);
- // Gets the size in bytes of the FEC/ULP headers, which must be accounted for
- // as packet overhead. Returns the packet overhead in bytes.
- static size_t PacketOverhead();
+ // Gets the maximum size of the FEC headers in bytes, which must be
+ // accounted for as packet overhead.
+ size_t MaxPacketOverhead() const;
// Reset internal states from last frame and clear |recovered_packets|.
// Frees all memory allocated by this class.
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
index 941773e..cf58cee 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
@@ -92,15 +92,10 @@
return length_;
}
-ProducerFec::ProducerFec(ForwardErrorCorrection* fec)
- : fec_(fec),
- media_packets_(),
- generated_fec_packets_(),
- num_protected_frames_(0),
+ProducerFec::ProducerFec()
+ : num_protected_frames_(0),
num_important_packets_(0),
- min_num_media_packets_(1),
- params_(),
- new_params_() {
+ min_num_media_packets_(1) {
memset(¶ms_, 0, sizeof(params_));
memset(&new_params_, 0, sizeof(new_params_));
}
@@ -180,9 +175,9 @@
// 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_);
+ 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();
@@ -217,6 +212,10 @@
return generated_fec_packets_.size();
}
+size_t ProducerFec::MaxPacketOverhead() const {
+ return fec_.MaxPacketOverhead();
+}
+
std::vector<std::unique_ptr<RedPacket>> ProducerFec::GetFecPacketsAsRed(
int red_payload_type,
int ulpfec_payload_type,
@@ -258,7 +257,7 @@
// generation is implemented.
RTC_DCHECK(!media_packets_.empty());
int num_fec_packets =
- fec_->GetNumberOfFecPackets(media_packets_.size(), params_.fec_rate);
+ fec_.NumFecPackets(media_packets_.size(), params_.fec_rate);
// Return the overhead in Q8.
return (num_fec_packets << 8) / media_packets_.size();
}
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h
index a65bb05..f09f181 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.h
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h
@@ -41,7 +41,7 @@
class ProducerFec {
public:
- explicit ProducerFec(ForwardErrorCorrection* fec);
+ ProducerFec();
~ProducerFec();
static std::unique_ptr<RedPacket> BuildRedPacket(const uint8_t* data_buffer,
@@ -77,6 +77,8 @@
size_t NumAvailableFecPackets() const;
+ size_t MaxPacketOverhead() const;
+
// Returns generated FEC packets with RED headers added.
std::vector<std::unique_ptr<RedPacket>> GetFecPacketsAsRed(
int red_payload_type,
@@ -87,7 +89,7 @@
private:
void DeleteMediaPackets();
int Overhead() const;
- ForwardErrorCorrection* fec_;
+ ForwardErrorCorrection fec_;
ForwardErrorCorrection::PacketList media_packets_;
std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
int num_protected_frames_;
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
index e3a49b4..5f26401 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
@@ -41,9 +41,6 @@
class ProducerFecTest : public ::testing::Test {
protected:
- ProducerFecTest() : producer_(&fec_) {}
-
- ForwardErrorCorrection fec_;
ProducerFec producer_;
FrameGenerator generator_;
};
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
index 012bc56..1bb1d65 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
@@ -876,7 +876,7 @@
constexpr uint32_t kMinPacketSize = kRtpHeaderSize;
const uint32_t kMaxPacketSize = IP_PACKET_SIZE - kRtpHeaderSize -
kTransportOverhead -
- ForwardErrorCorrection::PacketOverhead();
+ fec_.MaxPacketOverhead();
media_packet->length = random_.Rand(kMinPacketSize, kMaxPacketSize);
// Generate random values for the first 2 bytes
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index 9622f8b..b771551 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -342,7 +342,7 @@
return max_payload_length_ - RtpHeaderLength();
} else {
return max_payload_length_ - RtpHeaderLength() // RTP overhead.
- - video_->FECPacketOverhead() // FEC/ULP/RED overhead.
+ - video_->FecPacketOverhead() // FEC/ULP/RED overhead.
- (RtxStatus() ? kRtxHeaderSize : 0); // RTX overhead.
}
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
index b419568..b37a2f9 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -28,13 +28,13 @@
namespace webrtc {
-enum { REDForFECHeaderLength = 1 };
+namespace {
+constexpr size_t kRedForFecHeaderLength = 1;
+} // namespace
RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSender* rtp_sender)
: rtp_sender_(rtp_sender),
clock_(clock),
- // Generic FEC
- producer_fec_(&fec_),
fec_bitrate_(1000, RateStatistics::kBpsScale),
video_bitrate_(1000, RateStatistics::kBpsScale) {}
@@ -177,7 +177,7 @@
*payload_type_fec = fec_payload_type_;
}
-size_t RTPSenderVideo::FECPacketOverhead() const {
+size_t RTPSenderVideo::FecPacketOverhead() const {
rtc::CritScope cs(&crit_);
size_t overhead = 0;
if (red_payload_type_ != 0) {
@@ -186,11 +186,11 @@
// This reason for the header extensions to be included here is that
// from an FEC viewpoint, they are part of the payload to be protected.
// (The base RTP header is already protected by the FEC header.)
- return ForwardErrorCorrection::PacketOverhead() + REDForFECHeaderLength +
+ return producer_fec_.MaxPacketOverhead() + kRedForFecHeaderLength +
(rtp_sender_->RtpHeaderLength() - kRtpHeaderSize);
}
if (fec_enabled_)
- overhead += ForwardErrorCorrection::PacketOverhead();
+ overhead += producer_fec_.MaxPacketOverhead();
return overhead;
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h
index 59224cc..aaf295f 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -36,7 +36,7 @@
virtual RtpVideoCodecTypes VideoCodecType() const;
- size_t FECPacketOverhead() const;
+ size_t FecPacketOverhead() const;
static RtpUtility::Payload* CreateVideoPayload(
const char payload_name[RTP_PAYLOAD_NAME_SIZE],
@@ -102,7 +102,6 @@
int32_t retransmission_settings_ GUARDED_BY(crit_) = kRetransmitBaseLayer;
// FEC
- ForwardErrorCorrection fec_;
bool fec_enabled_ GUARDED_BY(crit_) = false;
int8_t red_payload_type_ GUARDED_BY(crit_) = 0;
int8_t fec_payload_type_ GUARDED_BY(crit_) = 0;
diff --git a/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc b/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc
index 697df48..bea52d7 100644
--- a/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc
+++ b/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc
@@ -238,8 +238,7 @@
new ForwardErrorCorrection::Packet());
const uint32_t kMinPacketSize = 12;
const uint32_t kMaxPacketSize = static_cast<uint32_t>(
- IP_PACKET_SIZE - 12 - 28 -
- ForwardErrorCorrection::PacketOverhead());
+ IP_PACKET_SIZE - 12 - 28 - fec.MaxPacketOverhead());
media_packet->length = random.Rand(kMinPacketSize,
kMaxPacketSize);
diff --git a/webrtc/test/fuzzers/producer_fec_fuzzer.cc b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
index cac31d1..50a5e5e 100644
--- a/webrtc/test/fuzzers/producer_fec_fuzzer.cc
+++ b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
@@ -18,8 +18,7 @@
namespace webrtc {
void FuzzOneInput(const uint8_t* data, size_t size) {
- ForwardErrorCorrection fec;
- ProducerFec producer(&fec);
+ ProducerFec producer;
size_t i = 0;
if (size < 4)
return;
