diff --git a/modules/rtp_rtcp/source/ulpfec_receiver.cc b/modules/rtp_rtcp/source/ulpfec_receiver.cc index 0558a2b680..f226a66786 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver.cc @@ -17,19 +17,50 @@ #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/logging.h" #include "rtc_base/time_utils.h" +#include "system_wrappers/include/metrics.h" namespace webrtc { UlpfecReceiver::UlpfecReceiver(uint32_t ssrc, + int ulpfec_payload_type, RecoveredPacketReceiver* callback, - rtc::ArrayView extensions) + rtc::ArrayView extensions, + Clock* clock) : ssrc_(ssrc), + ulpfec_payload_type_(ulpfec_payload_type), + clock_(clock), extensions_(extensions), recovered_packet_callback_(callback), fec_(ForwardErrorCorrection::CreateUlpfec(ssrc_)) {} UlpfecReceiver::~UlpfecReceiver() { RTC_DCHECK_RUN_ON(&sequence_checker_); + + if (packet_counter_.first_packet_time != Timestamp::MinusInfinity()) { + const Timestamp now = clock_->CurrentTime(); + TimeDelta elapsed = (now - packet_counter_.first_packet_time); + if (elapsed.seconds() >= metrics::kMinRunTimeInSeconds) { + if (packet_counter_.num_packets > 0) { + RTC_HISTOGRAM_PERCENTAGE( + "WebRTC.Video.ReceivedFecPacketsInPercent", + static_cast(packet_counter_.num_fec_packets * 100 / + packet_counter_.num_packets)); + } + if (packet_counter_.num_fec_packets > 0) { + RTC_HISTOGRAM_PERCENTAGE( + "WebRTC.Video.RecoveredMediaPacketsInPercentOfFec", + static_cast(packet_counter_.num_recovered_packets * 100 / + packet_counter_.num_fec_packets)); + } + if (ulpfec_payload_type_ != -1) { + RTC_HISTOGRAM_COUNTS_10000( + "WebRTC.Video.FecBitrateReceivedInKbps", + static_cast(packet_counter_.num_bytes * 8 / elapsed.seconds() / + 1000)); + } + } + } + received_packets_.clear(); fec_->ResetState(&recovered_packets_); } @@ -73,8 +104,7 @@ void UlpfecReceiver::SetRtpExtensions( // block length: 10 bits Length in bytes of the corresponding data // block excluding header. -bool UlpfecReceiver::AddReceivedRedPacket(const RtpPacketReceived& rtp_packet, - uint8_t ulpfec_payload_type) { +bool UlpfecReceiver::AddReceivedRedPacket(const RtpPacketReceived& rtp_packet) { RTC_DCHECK_RUN_ON(&sequence_checker_); // TODO(bugs.webrtc.org/11993): We get here via Call::DeliverRtp, so should be // moved to the network thread. @@ -104,7 +134,7 @@ bool UlpfecReceiver::AddReceivedRedPacket(const RtpPacketReceived& rtp_packet, // Get payload type from RED header and sequence number from RTP header. uint8_t payload_type = rtp_packet.payload()[0] & 0x7f; - received_packet->is_fec = payload_type == ulpfec_payload_type; + received_packet->is_fec = payload_type == ulpfec_payload_type_; received_packet->is_recovered = rtp_packet.recovered(); received_packet->ssrc = rtp_packet.Ssrc(); received_packet->seq_num = rtp_packet.SequenceNumber(); @@ -118,8 +148,8 @@ bool UlpfecReceiver::AddReceivedRedPacket(const RtpPacketReceived& rtp_packet, ++packet_counter_.num_packets; packet_counter_.num_bytes += rtp_packet.size(); - if (packet_counter_.first_packet_time_ms == -1) { - packet_counter_.first_packet_time_ms = rtc::TimeMillis(); + if (packet_counter_.first_packet_time == Timestamp::MinusInfinity()) { + packet_counter_.first_packet_time = clock_->CurrentTime(); } if (received_packet->is_fec) { diff --git a/modules/rtp_rtcp/source/ulpfec_receiver.h b/modules/rtp_rtcp/source/ulpfec_receiver.h index 727dba6439..b8ac8d8c30 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver.h +++ b/modules/rtp_rtcp/source/ulpfec_receiver.h @@ -23,6 +23,7 @@ #include "modules/rtp_rtcp/source/forward_error_correction.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/system/no_unique_address.h" +#include "system_wrappers/include/clock.h" namespace webrtc { @@ -33,18 +34,22 @@ struct FecPacketCounter { size_t num_fec_packets = 0; // Number of received FEC packets. size_t num_recovered_packets = 0; // Number of recovered media packets using FEC. - int64_t first_packet_time_ms = -1; // Time when first packet is received. + // Time when first packet is received. + Timestamp first_packet_time = Timestamp::MinusInfinity(); }; class UlpfecReceiver { public: UlpfecReceiver(uint32_t ssrc, + int ulpfec_payload_type, RecoveredPacketReceiver* callback, - rtc::ArrayView extensions); + rtc::ArrayView extensions, + Clock* clock); ~UlpfecReceiver(); - bool AddReceivedRedPacket(const RtpPacketReceived& rtp_packet, - uint8_t ulpfec_payload_type); + int ulpfec_payload_type() const { return ulpfec_payload_type_; } + + bool AddReceivedRedPacket(const RtpPacketReceived& rtp_packet); void ProcessReceivedFec(); @@ -54,6 +59,8 @@ class UlpfecReceiver { private: const uint32_t ssrc_; + const int ulpfec_payload_type_; + Clock* const clock_; RtpHeaderExtensionMap extensions_ RTC_GUARDED_BY(&sequence_checker_); RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index a0f71e31d8..1b0b0daf56 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -50,7 +50,11 @@ class UlpfecReceiverTest : public ::testing::Test { protected: UlpfecReceiverTest() : fec_(ForwardErrorCorrection::CreateUlpfec(kMediaSsrc)), - receiver_fec_(kMediaSsrc, &recovered_packet_receiver_, {}), + receiver_fec_(kMediaSsrc, + kFecPayloadType, + &recovered_packet_receiver_, + {}, + Clock::GetRealTimeClock()), packet_generator_(kMediaSsrc) {} // Generates `num_fec_packets` FEC packets, given `media_packets`. @@ -125,13 +129,13 @@ void UlpfecReceiverTest::BuildAndAddRedMediaPacket(AugmentedPacket* packet, bool is_recovered) { RtpPacketReceived red_packet = packet_generator_.BuildMediaRedPacket(*packet, is_recovered); - EXPECT_TRUE(receiver_fec_.AddReceivedRedPacket(red_packet, kFecPayloadType)); + EXPECT_TRUE(receiver_fec_.AddReceivedRedPacket(red_packet)); } void UlpfecReceiverTest::BuildAndAddRedFecPacket(Packet* packet) { RtpPacketReceived red_packet = packet_generator_.BuildUlpfecRedPacket(*packet); - EXPECT_TRUE(receiver_fec_.AddReceivedRedPacket(red_packet, kFecPayloadType)); + EXPECT_TRUE(receiver_fec_.AddReceivedRedPacket(red_packet)); } void UlpfecReceiverTest::VerifyReconstructedMediaPacket( @@ -175,11 +179,12 @@ void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data, size_t length, uint8_t ulpfec_payload_type) { NullRecoveredPacketReceiver null_callback; - UlpfecReceiver receiver_fec(kMediaSsrc, &null_callback, {}); + UlpfecReceiver receiver_fec(kMediaSsrc, ulpfec_payload_type, &null_callback, + {}, Clock::GetRealTimeClock()); RtpPacketReceived rtp_packet; ASSERT_TRUE(rtp_packet.Parse(data, length)); - receiver_fec.AddReceivedRedPacket(rtp_packet, ulpfec_payload_type); + receiver_fec.AddReceivedRedPacket(rtp_packet); } TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { @@ -192,7 +197,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { FecPacketCounter counter = receiver_fec_.GetPacketCounter(); EXPECT_EQ(0u, counter.num_packets); - EXPECT_EQ(-1, counter.first_packet_time_ms); + EXPECT_EQ(Timestamp::MinusInfinity(), counter.first_packet_time); // Recovery auto it = augmented_media_packets.begin(); @@ -203,8 +208,8 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { EXPECT_EQ(1u, counter.num_packets); EXPECT_EQ(0u, counter.num_fec_packets); EXPECT_EQ(0u, counter.num_recovered_packets); - const int64_t first_packet_time_ms = counter.first_packet_time_ms; - EXPECT_NE(-1, first_packet_time_ms); + const Timestamp first_packet_time = counter.first_packet_time; + EXPECT_NE(Timestamp::MinusInfinity(), first_packet_time); // Drop one media packet. auto fec_it = fec_packets.begin(); @@ -217,7 +222,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { EXPECT_EQ(2u, counter.num_packets); EXPECT_EQ(1u, counter.num_fec_packets); EXPECT_EQ(1u, counter.num_recovered_packets); - EXPECT_EQ(first_packet_time_ms, counter.first_packet_time_ms); + EXPECT_EQ(first_packet_time, counter.first_packet_time); } TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { @@ -230,7 +235,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { FecPacketCounter counter = receiver_fec_.GetPacketCounter(); EXPECT_EQ(0u, counter.num_packets); - EXPECT_EQ(-1, counter.first_packet_time_ms); + EXPECT_EQ(Timestamp::MinusInfinity(), counter.first_packet_time); // Recovery auto it = augmented_media_packets.begin(); @@ -241,8 +246,8 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { EXPECT_EQ(1u, counter.num_packets); EXPECT_EQ(0u, counter.num_fec_packets); EXPECT_EQ(0u, counter.num_recovered_packets); - const int64_t first_packet_time_ms = counter.first_packet_time_ms; - EXPECT_NE(-1, first_packet_time_ms); + const Timestamp first_packet_time = counter.first_packet_time; + EXPECT_NE(Timestamp::MinusInfinity(), first_packet_time); // Drop one media packet. auto fec_it = fec_packets.begin(); @@ -254,7 +259,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { EXPECT_EQ(2u, counter.num_packets); EXPECT_EQ(1u, counter.num_fec_packets); EXPECT_EQ(0u, counter.num_recovered_packets); - EXPECT_EQ(first_packet_time_ms, counter.first_packet_time_ms); + EXPECT_EQ(first_packet_time, counter.first_packet_time); } TEST_F(UlpfecReceiverTest, InjectGarbageFecHeaderLengthRecovery) { diff --git a/test/fuzzers/ulpfec_receiver_fuzzer.cc b/test/fuzzers/ulpfec_receiver_fuzzer.cc index c76744ec13..d88bee2c1b 100644 --- a/test/fuzzers/ulpfec_receiver_fuzzer.cc +++ b/test/fuzzers/ulpfec_receiver_fuzzer.cc @@ -36,7 +36,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { uint16_t media_seq_num = ByteReader::ReadLittleEndian(data + 10); DummyCallback callback; - UlpfecReceiver receiver(ulpfec_ssrc, &callback, {}); + UlpfecReceiver receiver(ulpfec_ssrc, 0, &callback, {}, + Clock::GetRealTimeClock()); test::FuzzDataHelper fuzz_data(rtc::MakeArrayView(data, size)); while (fuzz_data.CanReadBytes(kMinDataNeeded)) { @@ -62,7 +63,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { parsed_packet.SetSsrc(media_ssrc); } - receiver.AddReceivedRedPacket(parsed_packet, 0); + receiver.AddReceivedRedPacket(parsed_packet); } receiver.ProcessReceivedFec(); diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 756b6e4510..284a50185e 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -236,8 +236,10 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( rtp_receive_statistics_(rtp_receive_statistics), ulpfec_receiver_( std::make_unique(config->rtp.remote_ssrc, + config_.rtp.ulpfec_payload_type, this, - config->rtp.extensions)), + config->rtp.extensions, + clock)), packet_sink_(config->rtp.packet_sink_), receiving_(false), last_packet_log_ms_(-1), @@ -319,7 +321,7 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( RtpVideoStreamReceiver2::~RtpVideoStreamReceiver2() { if (packet_router_) packet_router_->RemoveReceiveRtpModule(rtp_rtcp_.get()); - UpdateHistograms(); + ulpfec_receiver_.reset(); if (frame_transformer_delegate_) frame_transformer_delegate_->Reset(); } @@ -1027,8 +1029,7 @@ void RtpVideoStreamReceiver2::ParseAndHandleEncapsulatingHeader( // packets. NotifyReceiverOfEmptyPacket(packet.SequenceNumber()); } - if (!ulpfec_receiver_->AddReceivedRedPacket( - packet, config_.rtp.ulpfec_payload_type)) { + if (!ulpfec_receiver_->AddReceivedRedPacket(packet)) { return; } ulpfec_receiver_->ProcessReceivedFec(); @@ -1148,33 +1149,6 @@ void RtpVideoStreamReceiver2::StopReceive() { receiving_ = false; } -void RtpVideoStreamReceiver2::UpdateHistograms() { - FecPacketCounter counter = ulpfec_receiver_->GetPacketCounter(); - if (counter.first_packet_time_ms == -1) - return; - - int64_t elapsed_sec = - (clock_->TimeInMilliseconds() - counter.first_packet_time_ms) / 1000; - if (elapsed_sec < metrics::kMinRunTimeInSeconds) - return; - - if (counter.num_packets > 0) { - RTC_HISTOGRAM_PERCENTAGE( - "WebRTC.Video.ReceivedFecPacketsInPercent", - static_cast(counter.num_fec_packets * 100 / counter.num_packets)); - } - if (counter.num_fec_packets > 0) { - RTC_HISTOGRAM_PERCENTAGE("WebRTC.Video.RecoveredMediaPacketsInPercentOfFec", - static_cast(counter.num_recovered_packets * - 100 / counter.num_fec_packets)); - } - if (config_.rtp.ulpfec_payload_type != -1) { - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.Video.FecBitrateReceivedInKbps", - static_cast(counter.num_bytes * 8 / elapsed_sec / 1000)); - } -} - void RtpVideoStreamReceiver2::InsertSpsPpsIntoTracker(uint8_t payload_type) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); RTC_DCHECK_RUN_ON(&worker_task_checker_); diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index 39d34cb524..73ed80e0ed 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -288,7 +288,6 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, RTC_RUN_ON(packet_sequence_checker_); void NotifyReceiverOfEmptyPacket(uint16_t seq_num) RTC_RUN_ON(packet_sequence_checker_); - void UpdateHistograms(); bool IsRedEnabled() const; void InsertSpsPpsIntoTracker(uint8_t payload_type) RTC_RUN_ON(packet_sequence_checker_);