From ea33f7f6a3ec68b95ff6a5fae3f861524b3116c1 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 4 May 2023 12:12:33 +0200 Subject: [PATCH] Cleanup usasge of ReportBlockData::report_block accessor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reduces dependency on the struct RTCPReportBlock and would allow to delete it in favor of class ReportBlockData Bug: None Change-Id: Ia46a2516e26453724eed2e499f475f65df6cd3fa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304163 Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#39990} --- call/video_send_stream.cc | 6 ++---- media/engine/webrtc_video_engine.cc | 8 ++------ media/engine/webrtc_video_engine_unittest.cc | 16 ++++++---------- video/send_statistics_proxy.cc | 13 ++++++------- video/send_statistics_proxy_unittest.cc | 16 +++++++--------- 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc index 241d44a230..e8532a7a26 100644 --- a/call/video_send_stream.cc +++ b/call/video_send_stream.cc @@ -53,11 +53,9 @@ std::string VideoSendStream::StreamStats::ToString() const { ss << "avg_delay_ms: " << avg_delay_ms << ", "; ss << "max_delay_ms: " << max_delay_ms << ", "; if (report_block_data) { - ss << "cum_loss: " << report_block_data->report_block().packets_lost - << ", "; + ss << "cum_loss: " << report_block_data->cumulative_lost() << ", "; ss << "max_ext_seq: " - << report_block_data->report_block().extended_highest_sequence_number - << ", "; + << report_block_data->extended_highest_sequence_number() << ", "; } ss << "nack: " << rtcp_packet_type_counts.nack_packets << ", "; ss << "fir: " << rtcp_packet_type_counts.fir_packets << ", "; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index af97dfaff4..cc1edc8ec8 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2790,12 +2790,8 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetPerLayerVideoSenderInfos( info.nacks_received = stream_stats.rtcp_packet_type_counts.nack_packets; info.plis_received = stream_stats.rtcp_packet_type_counts.pli_packets; if (stream_stats.report_block_data.has_value()) { - info.packets_lost = - stream_stats.report_block_data->report_block().packets_lost; - info.fraction_lost = - static_cast( - stream_stats.report_block_data->report_block().fraction_lost) / - (1 << 8); + info.packets_lost = stream_stats.report_block_data->cumulative_lost(); + info.fraction_lost = stream_stats.report_block_data->fraction_lost(); info.report_block_datas.push_back(*stream_stats.report_block_data); } info.qp_sum = stream_stats.qp_sum; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index bbabddf9fb..a0d60bf6fb 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -5900,11 +5900,9 @@ TEST_F(WebRtcVideoChannelTest, GetAggregatedStatsReportForSubStreams) { EXPECT_EQ(sender.total_packet_send_delay, 2 * substream.rtp_stats.transmitted.total_packet_delay); EXPECT_EQ(sender.packets_lost, - 2 * substream.report_block_data->report_block().packets_lost); - EXPECT_EQ(sender.fraction_lost, - static_cast( - substream.report_block_data->report_block().fraction_lost) / - (1 << 8)); + 2 * substream.report_block_data->cumulative_lost()); + EXPECT_FLOAT_EQ(sender.fraction_lost, + substream.report_block_data->fraction_lost()); EXPECT_EQ(sender.rtt_ms, 0); EXPECT_EQ(sender.codec_name, DefaultCodec().name); EXPECT_EQ(sender.codec_payload_type, DefaultCodec().id); @@ -6026,11 +6024,9 @@ TEST_F(WebRtcVideoChannelTest, GetPerLayerStatsReportForSubStreams) { EXPECT_EQ(sender.retransmitted_packets_sent, substream.rtp_stats.retransmitted.packets); EXPECT_EQ(sender.packets_lost, - substream.report_block_data->report_block().packets_lost); - EXPECT_EQ(sender.fraction_lost, - static_cast( - substream.report_block_data->report_block().fraction_lost) / - (1 << 8)); + substream.report_block_data->cumulative_lost()); + EXPECT_FLOAT_EQ(sender.fraction_lost, + substream.report_block_data->fraction_lost()); EXPECT_EQ(sender.rtt_ms, 0); EXPECT_EQ(sender.codec_name, DefaultCodec().name); EXPECT_EQ(sender.codec_payload_type, DefaultCodec().id); diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index ac11692175..fd576bb38b 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -1303,20 +1303,19 @@ void SendStatisticsProxy::RtcpPacketTypesCounterUpdated( } void SendStatisticsProxy::OnReportBlockDataUpdated( - ReportBlockData report_block_data) { + ReportBlockData report_block) { MutexLock lock(&mutex_); VideoSendStream::StreamStats* stats = - GetStatsEntry(report_block_data.report_block().source_ssrc); + GetStatsEntry(report_block.source_ssrc()); if (!stats) return; - const RTCPReportBlock& report_block = report_block_data.report_block(); uma_container_->report_block_stats_.Store( - /*ssrc=*/report_block.source_ssrc, - /*packets_lost=*/report_block.packets_lost, + /*ssrc=*/report_block.source_ssrc(), + /*packets_lost=*/report_block.cumulative_lost(), /*extended_highest_sequence_number=*/ - report_block.extended_highest_sequence_number); + report_block.extended_highest_sequence_number()); - stats->report_block_data = std::move(report_block_data); + stats->report_block_data = std::move(report_block); } void SendStatisticsProxy::DataCountersUpdated( diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index d3d14d6859..c374acc5a5 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -169,15 +169,13 @@ class SendStatisticsProxyTest : public ::testing::Test { EXPECT_EQ(a.report_block_data.has_value(), b.report_block_data.has_value()); if (a.report_block_data.has_value()) { - const RTCPReportBlock& a_rtcp_stats = - a.report_block_data->report_block(); - const RTCPReportBlock& b_rtcp_stats = - b.report_block_data->report_block(); - EXPECT_EQ(a_rtcp_stats.fraction_lost, b_rtcp_stats.fraction_lost); - EXPECT_EQ(a_rtcp_stats.packets_lost, b_rtcp_stats.packets_lost); - EXPECT_EQ(a_rtcp_stats.extended_highest_sequence_number, - b_rtcp_stats.extended_highest_sequence_number); - EXPECT_EQ(a_rtcp_stats.jitter, b_rtcp_stats.jitter); + EXPECT_EQ(a.report_block_data->fraction_lost_raw(), + b.report_block_data->fraction_lost_raw()); + EXPECT_EQ(a.report_block_data->cumulative_lost(), + b.report_block_data->cumulative_lost()); + EXPECT_EQ(a.report_block_data->extended_highest_sequence_number(), + b.report_block_data->extended_highest_sequence_number()); + EXPECT_EQ(a.report_block_data->jitter(), b.report_block_data->jitter()); } } }