admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Report histograms in dtor of UlpfecReceiver.

Browse Source 
The data that's used to report the histograms is owned by UlpfecReceiver
and moving the reporting there, simplifies things as configuration
changes happen in RtpVideoStreamReceiver2 (which currently require all
receive streams to be deleted+reconstructed).

Additional updates:
* Consistently using `Clock` for timestamps. Before there was
  a mix of Clock and rtc::TimeMillis.
* Update code to use Timestamp and TimeDelta.

Bug: none
Change-Id: I89ca28ec7067a49d6b357315ae733b04e7c5a2e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271027
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37729}
This commit is contained in:
Tommi 2022-08-10 07:45:43 +02:00 committed by WebRTC LUCI CQ
parent 39c90e08f9
commit 7fbab87b60
6 changed files with 73 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
modules/rtp_rtcp/source/ulpfec_receiver.cc
View File

@ -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<const RtpExtension> extensions)
rtc::ArrayView<const RtpExtension> 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<int>(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<int>(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<int>(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) {

15
modules/rtp_rtcp/source/ulpfec_receiver.h
View File

@ -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<const RtpExtension> extensions);
rtc::ArrayView<const RtpExtension> 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_;

31
modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
View File

@ -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) {

5
test/fuzzers/ulpfec_receiver_fuzzer.cc
View File

@ -36,7 +36,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
uint16_t media_seq_num = ByteReader<uint16_t>::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();

36
video/rtp_video_stream_receiver2.cc
View File

@ -236,8 +236,10 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2(
rtp_receive_statistics_(rtp_receive_statistics),
ulpfec_receiver_(
std::make_unique<UlpfecReceiver>(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<int>(counter.num_fec_packets * 100 / counter.num_packets));
}
if (counter.num_fec_packets > 0) {
RTC_HISTOGRAM_PERCENTAGE("WebRTC.Video.RecoveredMediaPacketsInPercentOfFec",
static_cast<int>(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<int>(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_);

1
video/rtp_video_stream_receiver2.h
View File

@ -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_);