diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc index e666421e21..b214f60c06 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 @@ 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 diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h index ac2cfa76b6..7d2e5aa51a 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 @@ 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. diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc index 941773e6e3..cf58cee1d2 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc @@ -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> 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(); } diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h index a65bb053e0..f09f18100c 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.h +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h @@ -41,7 +41,7 @@ class RedPacket { class ProducerFec { public: - explicit ProducerFec(ForwardErrorCorrection* fec); + ProducerFec(); ~ProducerFec(); static std::unique_ptr 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> 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 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 e3a49b43cc..5f26401fa0 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 @@ void VerifyHeader(uint16_t seq_num, 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 012bc561e7..1bb1d6543c 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 @@ 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 diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 9622f8b826..b771551233 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -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. } } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index b419568293..b37a2f9c26 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 @@ 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; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h index 59224cc7d9..aaf295f61a 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 @@ 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; diff --git a/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc b/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc index 697df48587..bea52d7cd6 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 @@ TEST(FecTest, MAYBE_FecTest) { new ForwardErrorCorrection::Packet()); const uint32_t kMinPacketSize = 12; const uint32_t kMaxPacketSize = static_cast( - 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 cac31d126f..50a5e5e5cd 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;