From 4f40fa5cef0f9a5dfcc539365aef7bd3c2986099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Thu, 19 Dec 2019 13:27:27 +0100 Subject: [PATCH] Implement RTCOutboundRtpStreamStats::remoteId. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL also removes RTCRtpStreamStats::associateStatsId, which is the legacy name for this stat, which was never implemented (existed in C++ but the member always had the value undefined and was thus never exposed in JavaScript). Bug: webrtc:11228 Change-Id: I28c332e4bdf2f55caaedf993482dca58b6b8b9a0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162800 Reviewed-by: Harald Alvestrand Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#30171} --- api/stats/rtcstats_objects.h | 4 +--- pc/rtc_stats_collector.cc | 22 ++++++++++++++++------ pc/rtc_stats_collector_unittest.cc | 9 ++++++++- pc/rtc_stats_integrationtest.cc | 3 ++- pc/rtc_stats_traversal.cc | 2 +- stats/rtcstats_objects.cc | 6 +++--- 6 files changed, 31 insertions(+), 15 deletions(-) diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index d5202042fe..af91a85aed 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -370,9 +370,6 @@ class RTC_EXPORT RTCRTPStreamStats : public RTCStats { ~RTCRTPStreamStats() override; RTCStatsMember ssrc; - // TODO(hbos): When the remote case is supported |RTCStatsCollector| needs to - // set this. crbug.com/657855, 657856 - RTCStatsMember associate_stats_id; // TODO(hbos): Remote case not supported by |RTCStatsCollector|. // crbug.com/657855, 657856 RTCStatsMember is_remote; // = false @@ -468,6 +465,7 @@ class RTC_EXPORT RTCOutboundRTPStreamStats final : public RTCRTPStreamStats { ~RTCOutboundRTPStreamStats() override; RTCStatsMember media_source_id; + RTCStatsMember remote_id; RTCStatsMember packets_sent; RTCStatsMember retransmitted_packets_sent; RTCStatsMember bytes_sent; diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 98b3bd42d4..116b4ba497 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -429,6 +429,7 @@ std::unique_ptr ProduceRemoteInboundRtpStreamStatsFromReportBlockData( const ReportBlockData& report_block_data, cricket::MediaType media_type, + std::map outbound_rtps, const RTCStatsReport& report) { const auto& report_block = report_block_data.report_block(); // RTCStats' timestamp generally refers to when the metric was sampled, but @@ -448,11 +449,12 @@ ProduceRemoteInboundRtpStreamStatsFromReportBlockData( std::string local_id = RTCOutboundRTPStreamStatsIDFromSSRC( media_type == cricket::MEDIA_TYPE_AUDIO, report_block.source_ssrc); - const auto* local_id_stat = report.Get(local_id); - if (local_id_stat) { + // Look up local stat from |outbound_rtps| where the pointers are non-const. + auto local_id_it = outbound_rtps.find(local_id); + if (local_id_it != outbound_rtps.end()) { remote_inbound->local_id = local_id; - const auto& outbound_rtp = - local_id_stat->cast_to(); + auto& outbound_rtp = *local_id_it->second; + outbound_rtp.remote_id = remote_inbound->id(); // The RTP/RTCP transport is obtained from the // RTCOutboundRtpStreamStats's transport. const auto* transport_from_id = outbound_rtp.transport_id.is_defined() @@ -1546,6 +1548,7 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( report->AddStats(std::move(inbound_audio)); } // Outbound + std::map audio_outbound_rtps; for (const cricket::VoiceSenderInfo& voice_sender_info : track_media_info_map.voice_media_info()->senders) { if (!voice_sender_info.connected()) @@ -1568,6 +1571,8 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( attachment_id); } outbound_audio->transport_id = transport_id; + audio_outbound_rtps.insert( + std::make_pair(outbound_audio->id(), outbound_audio.get())); report->AddStats(std::move(outbound_audio)); } // Remote-inbound @@ -1579,7 +1584,8 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( track_media_info_map.voice_media_info()->senders) { for (const auto& report_block_data : voice_sender_info.report_block_datas) { report->AddStats(ProduceRemoteInboundRtpStreamStatsFromReportBlockData( - report_block_data, cricket::MEDIA_TYPE_AUDIO, *report)); + report_block_data, cricket::MEDIA_TYPE_AUDIO, + std::move(audio_outbound_rtps), *report)); } } } @@ -1619,6 +1625,7 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( report->AddStats(std::move(inbound_video)); } // Outbound + std::map video_outbound_rtps; for (const cricket::VideoSenderInfo& video_sender_info : track_media_info_map.video_media_info()->senders) { if (!video_sender_info.connected()) @@ -1641,6 +1648,8 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( attachment_id); } outbound_video->transport_id = transport_id; + video_outbound_rtps.insert( + std::make_pair(outbound_video->id(), outbound_video.get())); report->AddStats(std::move(outbound_video)); } // Remote-inbound @@ -1652,7 +1661,8 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( track_media_info_map.video_media_info()->senders) { for (const auto& report_block_data : video_sender_info.report_block_datas) { report->AddStats(ProduceRemoteInboundRtpStreamStatsFromReportBlockData( - report_block_data, cricket::MEDIA_TYPE_VIDEO, *report)); + report_block_data, cricket::MEDIA_TYPE_VIDEO, + std::move(video_outbound_rtps), *report)); } } } diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index b5e3c6b084..97658d0173 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -1935,6 +1935,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { RTCOutboundRTPStreamStats expected_audio("RTCOutboundRTPAudioStream_1", report->timestamp_us()); expected_audio.media_source_id = "RTCAudioSource_50"; + // |expected_audio.remote_id| should be undefined. expected_audio.ssrc = 1; expected_audio.is_remote = false; expected_audio.media_type = "audio"; @@ -2013,6 +2014,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { RTCOutboundRTPStreamStats expected_video(stats_of_my_type[0]->id(), report->timestamp_us()); expected_video.media_source_id = "RTCVideoSource_50"; + // |expected_video.remote_id| should be undefined. expected_video.ssrc = 1; expected_video.is_remote = false; expected_video.media_type = "video"; @@ -2606,7 +2608,12 @@ TEST_P(RTCStatsCollectorTestWithParamKind, ->cast_to(), expected_remote_inbound_rtp); EXPECT_TRUE(report->Get(*expected_remote_inbound_rtp.transport_id)); - EXPECT_TRUE(report->Get(*expected_remote_inbound_rtp.local_id)); + ASSERT_TRUE(report->Get(*expected_remote_inbound_rtp.local_id)); + // Lookup works in both directions. + EXPECT_EQ(*report->Get(*expected_remote_inbound_rtp.local_id) + ->cast_to() + .remote_id, + expected_remote_inbound_rtp.id()); } TEST_P(RTCStatsCollectorTestWithParamKind, diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index 075dc63257..ed4ac5bcda 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -755,7 +755,6 @@ class RTCStatsReportVerifier { void VerifyRTCRTPStreamStats(const RTCRTPStreamStats& stream, RTCStatsVerifier* verifier) { verifier->TestMemberIsDefined(stream.ssrc); - verifier->TestMemberIsUndefined(stream.associate_stats_id); verifier->TestMemberIsDefined(stream.is_remote); verifier->TestMemberIsDefined(stream.media_type); verifier->TestMemberIsDefined(stream.kind); @@ -863,6 +862,8 @@ class RTCStatsReportVerifier { RTCAudioSourceStats::kType); verifier.TestMemberIsUndefined(outbound_stream.qp_sum); } + verifier.TestMemberIsOptionalIDReference( + outbound_stream.remote_id, RTCRemoteInboundRtpStreamStats::kType); verifier.TestMemberIsNonNegative(outbound_stream.packets_sent); verifier.TestMemberIsNonNegative( outbound_stream.retransmitted_packets_sent); diff --git a/pc/rtc_stats_traversal.cc b/pc/rtc_stats_traversal.cc index a824675a6b..c08643eba8 100644 --- a/pc/rtc_stats_traversal.cc +++ b/pc/rtc_stats_traversal.cc @@ -101,7 +101,6 @@ std::vector GetStatsReferencedIds(const RTCStats& stats) { } else if (type == RTCInboundRTPStreamStats::kType || type == RTCOutboundRTPStreamStats::kType) { const auto& rtp = static_cast(stats); - AddIdIfDefined(rtp.associate_stats_id, &neighbor_ids); AddIdIfDefined(rtp.track_id, &neighbor_ids); AddIdIfDefined(rtp.transport_id, &neighbor_ids); AddIdIfDefined(rtp.codec_id, &neighbor_ids); @@ -109,6 +108,7 @@ std::vector GetStatsReferencedIds(const RTCStats& stats) { const auto& outbound_rtp = static_cast(stats); AddIdIfDefined(outbound_rtp.media_source_id, &neighbor_ids); + AddIdIfDefined(outbound_rtp.remote_id, &neighbor_ids); } } else if (type == RTCRemoteInboundRtpStreamStats::kType) { const auto& remote_inbound_rtp = diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index 4de2aa125c..f8122f458c 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -541,7 +541,6 @@ RTCPeerConnectionStats::~RTCPeerConnectionStats() {} // clang-format off WEBRTC_RTCSTATS_IMPL(RTCRTPStreamStats, RTCStats, "rtp", &ssrc, - &associate_stats_id, &is_remote, &media_type, &kind, @@ -562,7 +561,6 @@ RTCRTPStreamStats::RTCRTPStreamStats(const std::string& id, RTCRTPStreamStats::RTCRTPStreamStats(std::string&& id, int64_t timestamp_us) : RTCStats(std::move(id), timestamp_us), ssrc("ssrc"), - associate_stats_id("associateStatsId"), is_remote("isRemote", false), media_type("mediaType"), kind("kind"), @@ -578,7 +576,6 @@ RTCRTPStreamStats::RTCRTPStreamStats(std::string&& id, int64_t timestamp_us) RTCRTPStreamStats::RTCRTPStreamStats(const RTCRTPStreamStats& other) : RTCStats(other.id(), other.timestamp_us()), ssrc(other.ssrc), - associate_stats_id(other.associate_stats_id), is_remote(other.is_remote), media_type(other.media_type), kind(other.kind), @@ -695,6 +692,7 @@ RTCInboundRTPStreamStats::~RTCInboundRTPStreamStats() {} WEBRTC_RTCSTATS_IMPL( RTCOutboundRTPStreamStats, RTCRTPStreamStats, "outbound-rtp", &media_source_id, + &remote_id, &packets_sent, &retransmitted_packets_sent, &bytes_sent, @@ -720,6 +718,7 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(std::string&& id, int64_t timestamp_us) : RTCRTPStreamStats(std::move(id), timestamp_us), media_source_id("mediaSourceId"), + remote_id("remoteId"), packets_sent("packetsSent"), retransmitted_packets_sent("retransmittedPacketsSent"), bytes_sent("bytesSent"), @@ -741,6 +740,7 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats( const RTCOutboundRTPStreamStats& other) : RTCRTPStreamStats(other), media_source_id(other.media_source_id), + remote_id(other.remote_id), packets_sent(other.packets_sent), retransmitted_packets_sent(other.retransmitted_packets_sent), bytes_sent(other.bytes_sent),