Simplify public interface of ProducerFec.
- Change some member functions to be private. These were only
called by other private member functions.
- Replace DeleteMediaPackets() with direct calls to
media_packets_.clear()
- Rename GetFecPacketsAsRed to GetUlpfecPacketsAsRed.
No functional changes are intended by this CL.
BUG=webrtc:5654
R=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2305793003 .
Cr-Commit-Position: refs/heads/master@{#14491}
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
index cdd898d..2a2327c 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc
@@ -21,6 +21,8 @@
namespace webrtc {
+namespace {
+
constexpr size_t kRedForFecHeaderLength = 1;
// This controls the maximum amount of excess overhead (actual - target)
@@ -47,6 +49,8 @@
// |kMinMediaPackets| + 1 packets are sent to the FEC code.
constexpr float kMinMediaPacketsAdaptationThreshold = 2.0f;
+} // namespace
+
RedPacket::RedPacket(size_t length)
: data_(new uint8_t[length]),
length_(length),
@@ -101,9 +105,7 @@
memset(&new_params_, 0, sizeof(new_params_));
}
-ProducerFec::~ProducerFec() {
- DeleteMediaPackets();
-}
+ProducerFec::~ProducerFec() = default;
std::unique_ptr<RedPacket> ProducerFec::BuildRedPacket(
const uint8_t* data_buffer,
@@ -179,8 +181,7 @@
num_important_packets_, kUseUnequalProtection,
params_.fec_mask_type, &generated_fec_packets_);
if (generated_fec_packets_.empty()) {
- num_protected_frames_ = 0;
- DeleteMediaPackets();
+ ResetState();
}
return ret;
}
@@ -216,7 +217,7 @@
return fec_->MaxPacketOverhead();
}
-std::vector<std::unique_ptr<RedPacket>> ProducerFec::GetFecPacketsAsRed(
+std::vector<std::unique_ptr<RedPacket>> ProducerFec::GetUlpfecPacketsAsRed(
int red_payload_type,
int ulpfec_payload_type,
uint16_t first_seq_num,
@@ -240,18 +241,13 @@
red_packet->AssignPayload(fec_packet->data, fec_packet->length);
red_packets.push_back(std::move(red_packet));
}
- // Reset state.
- DeleteMediaPackets();
- generated_fec_packets_.clear();
- num_protected_frames_ = 0;
+
+ ResetState();
+
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 inherited from the
- // protection factor produced by video_coding module and how the FEC
- // generation is implemented.
RTC_DCHECK(!media_packets_.empty());
int num_fec_packets =
fec_->NumFecPackets(media_packets_.size(), params_.fec_rate);
@@ -259,8 +255,10 @@
return (num_fec_packets << 8) / media_packets_.size();
}
-void ProducerFec::DeleteMediaPackets() {
+void ProducerFec::ResetState() {
media_packets_.clear();
+ generated_fec_packets_.clear();
+ num_protected_frames_ = 0;
}
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h
index 7ea0740..d52b4ed 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec.h
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h
@@ -59,6 +59,28 @@
size_t payload_length,
size_t rtp_header_length);
+ // Returns true if there are generated FEC packets available.
+ bool FecAvailable() const;
+
+ size_t NumAvailableFecPackets() const;
+
+ // Returns the overhead, per packet, for FEC (and possibly RED).
+ size_t MaxPacketOverhead() const;
+
+ // Returns generated FEC packets with RED headers added.
+ std::vector<std::unique_ptr<RedPacket>> GetUlpfecPacketsAsRed(
+ int red_payload_type,
+ int ulpfec_payload_type,
+ uint16_t first_seq_num,
+ size_t rtp_header_length);
+
+ private:
+ // Overhead is defined as relative to the number of media packets, and not
+ // 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.
+ int Overhead() const;
+
// 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
@@ -72,23 +94,8 @@
// (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;
+ void ResetState();
- 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,
- int ulpfec_payload_type,
- uint16_t first_seq_num,
- size_t rtp_header_length);
-
- private:
- void DeleteMediaPackets();
- int Overhead() const;
std::unique_ptr<ForwardErrorCorrection> fec_;
ForwardErrorCorrection::PacketList media_packets_;
std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
index 31304b4..d705bd0 100644
--- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc
@@ -94,8 +94,8 @@
size_t num_fec_packets = producer_.NumAvailableFecPackets();
if (num_fec_packets > 0) {
std::vector<std::unique_ptr<RedPacket>> fec_packets =
- producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
- p.header_size);
+ producer_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
+ p.header_size);
EXPECT_EQ(num_fec_packets, fec_packets.size());
}
}
@@ -123,8 +123,8 @@
EXPECT_TRUE(producer_.FecAvailable());
uint16_t seq_num = packet_generator_.NextPacketSeqNum();
std::vector<std::unique_ptr<RedPacket>> red_packets =
- producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
- kRtpHeaderSize);
+ producer_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
+ kRtpHeaderSize);
EXPECT_FALSE(producer_.FecAvailable());
ASSERT_EQ(1u, red_packets.size());
VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
@@ -158,8 +158,8 @@
EXPECT_TRUE(producer_.FecAvailable());
uint16_t seq_num = packet_generator_.NextPacketSeqNum();
std::vector<std::unique_ptr<RedPacket>> red_packets =
- producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
- kRtpHeaderSize);
+ producer_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num,
+ kRtpHeaderSize);
EXPECT_FALSE(producer_.FecAvailable());
ASSERT_EQ(1u, red_packets.size());
VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
index 9020f6a..722a484 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -129,7 +129,7 @@
if (num_fec_packets > 0) {
uint16_t first_fec_sequence_number =
rtp_sender_->AllocateSequenceNumber(num_fec_packets);
- fec_packets = producer_fec_.GetFecPacketsAsRed(
+ fec_packets = producer_fec_.GetUlpfecPacketsAsRed(
red_payload_type_, fec_payload_type_, first_fec_sequence_number,
media_packet->headers_size());
RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
diff --git a/webrtc/test/fuzzers/producer_fec_fuzzer.cc b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
index 3319138..269b4a1 100644
--- a/webrtc/test/fuzzers/producer_fec_fuzzer.cc
+++ b/webrtc/test/fuzzers/producer_fec_fuzzer.cc
@@ -53,8 +53,8 @@
const size_t num_fec_packets = producer.NumAvailableFecPackets();
if (num_fec_packets > 0) {
std::vector<std::unique_ptr<RedPacket>> fec_packets =
- producer.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
- rtp_header_length);
+ producer.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
+ rtp_header_length);
RTC_CHECK_EQ(num_fec_packets, fec_packets.size());
}
}
