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}
This commit is contained in:
parent
07d9e545ff
commit
6631e8a21b
@ -119,18 +119,16 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packets,
|
||||
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 ForwardErrorCorrection::GenerateFec(const PacketList& media_packets,
|
||||
}
|
||||
}
|
||||
|
||||
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 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packets,
|
||||
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 @@ int ForwardErrorCorrection::DecodeFec(
|
||||
return 0;
|
||||
}
|
||||
|
||||
size_t ForwardErrorCorrection::PacketOverhead() {
|
||||
size_t ForwardErrorCorrection::MaxPacketOverhead() const {
|
||||
return kFecHeaderSize + kUlpHeaderSizeLBitSet;
|
||||
}
|
||||
} // namespace webrtc
|
||||
|
||||
@ -186,11 +186,11 @@ class ForwardErrorCorrection {
|
||||
|
||||
// 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.
|
||||
|
||||
@ -92,15 +92,10 @@ size_t RedPacket::length() const {
|
||||
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 @@ int ProducerFec::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer,
|
||||
// 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 @@ size_t ProducerFec::NumAvailableFecPackets() const {
|
||||
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 @@ int ProducerFec::Overhead() const {
|
||||
// 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();
|
||||
}
|
||||
|
||||
@ -41,7 +41,7 @@ class RedPacket {
|
||||
|
||||
class ProducerFec {
|
||||
public:
|
||||
explicit ProducerFec(ForwardErrorCorrection* fec);
|
||||
ProducerFec();
|
||||
~ProducerFec();
|
||||
|
||||
static std::unique_ptr<RedPacket> BuildRedPacket(const uint8_t* data_buffer,
|
||||
@ -77,6 +77,8 @@ class ProducerFec {
|
||||
|
||||
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 @@ class ProducerFec {
|
||||
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_;
|
||||
|
||||
@ -41,9 +41,6 @@ void VerifyHeader(uint16_t seq_num,
|
||||
|
||||
class ProducerFecTest : public ::testing::Test {
|
||||
protected:
|
||||
ProducerFecTest() : producer_(&fec_) {}
|
||||
|
||||
ForwardErrorCorrection fec_;
|
||||
ProducerFec producer_;
|
||||
FrameGenerator generator_;
|
||||
};
|
||||
|
||||
@ -876,7 +876,7 @@ int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets,
|
||||
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
|
||||
|
||||
@ -342,7 +342,7 @@ size_t RTPSender::MaxDataPayloadLength() const {
|
||||
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.
|
||||
}
|
||||
}
|
||||
|
||||
@ -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 @@ void RTPSenderVideo::GenericFECStatus(bool* enable,
|
||||
*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 @@ size_t RTPSenderVideo::FECPacketOverhead() const {
|
||||
// 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;
|
||||
}
|
||||
|
||||
|
||||
@ -36,7 +36,7 @@ class RTPSenderVideo {
|
||||
|
||||
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 @@ class RTPSenderVideo {
|
||||
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;
|
||||
|
||||
@ -238,8 +238,7 @@ TEST(FecTest, MAYBE_FecTest) {
|
||||
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);
|
||||
|
||||
|
||||
@ -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;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user