From 07d64b407259561b41149109b85268e72dfb1de1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 4 Jan 2023 09:28:27 +0000 Subject: [PATCH] Revert "Remove 'trackId' dependency in stats selector algorithm." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 81aab488781c1a736c9d85ff1532631be2989523. Reason for revert: external/wpt/webrtc/simulcast/setParameters-active.https.html is failing with this change Original change's description: > Remove 'trackId' dependency in stats selector algorithm. > > In preparation for the deletion of deprecated 'track' stats, the > stats selector algorithm needs to be rewritten not to use 'trackId'. > > This is achieved by finding RTP stats by their SSRC, as obtained via > getParameters(). This unfortunately adds a block-invoke (in the sender > case the block-invoke happens inside GetParametersInternal and in the > receiver case the block-invoke is explicit at the calling place), but > it can't be helped and it's just once per getStats() call and only if > the selector argument is used. > > Bug: webrtc:14175 > Change-Id: If0e14cdbdc76d141e0042e43757970893bf32119 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289101 > Reviewed-by: Harald Alvestrand > Commit-Queue: Henrik Boström > Cr-Commit-Position: refs/heads/main@{#38981} Bug: webrtc:14175 Change-Id: Id1cbe892250fe88bd6db0b47269bcefa346709b4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290502 Commit-Queue: Christoffer Jansson Auto-Submit: Henrik Boström Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Christoffer Jansson Cr-Commit-Position: refs/heads/main@{#38993} --- pc/rtc_stats_collector.cc | 63 +++++++++++++++--------------- pc/rtc_stats_collector.h | 6 --- pc/rtc_stats_collector_unittest.cc | 5 +-- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index afb819f8f0..a9fb3b7d76 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -1249,10 +1249,7 @@ void ProduceReceiverMediaTrackStats( } } -} // namespace - -rtc::scoped_refptr -RTCStatsCollector::CreateReportFilteredBySelector( +rtc::scoped_refptr CreateReportFilteredBySelector( bool filter_by_sender_selector, rtc::scoped_refptr report, rtc::scoped_refptr sender_selector, @@ -1261,40 +1258,40 @@ RTCStatsCollector::CreateReportFilteredBySelector( if (filter_by_sender_selector) { // Filter mode: RTCStatsCollector::RequestInfo::kSenderSelector if (sender_selector) { - // Find outbound-rtp(s) of the sender using ssrc lookup. - auto encodings = sender_selector->GetParametersInternal().encodings; - for (const auto* outbound_rtp : - report->GetStatsOfType()) { - RTC_DCHECK(outbound_rtp->ssrc.is_defined()); - auto it = std::find_if( - encodings.begin(), encodings.end(), - [ssrc = - *outbound_rtp->ssrc](const RtpEncodingParameters& encoding) { - return encoding.ssrc.has_value() && encoding.ssrc.value() == ssrc; - }); - if (it != encodings.end()) { - rtpstream_ids.push_back(outbound_rtp->id()); + // Find outbound-rtp(s) of the sender, i.e. the outbound-rtp(s) that + // reference the sender stats. + // Because we do not implement sender stats, we look at outbound-rtp(s) + // that reference the track attachment stats for the sender instead. + std::string track_id = + DEPRECATED_RTCMediaStreamTrackStatsIDFromDirectionAndAttachment( + kDirectionOutbound, sender_selector->AttachmentId()); + for (const auto& stats : *report) { + if (stats.type() != RTCOutboundRTPStreamStats::kType) + continue; + const auto& outbound_rtp = stats.cast_to(); + if (outbound_rtp.track_id.is_defined() && + *outbound_rtp.track_id == track_id) { + rtpstream_ids.push_back(outbound_rtp.id()); } } } } else { // Filter mode: RTCStatsCollector::RequestInfo::kReceiverSelector if (receiver_selector) { - // Find the inbound-rtp of the receiver using ssrc lookup. - absl::optional ssrc; - worker_thread_->BlockingCall([&] { - auto encodings = receiver_selector->GetParameters().encodings; - if (!encodings.empty()) { - ssrc = encodings[0].ssrc; - } - }); - if (ssrc.has_value()) { - for (const auto* inbound_rtp : - report->GetStatsOfType()) { - RTC_DCHECK(inbound_rtp->ssrc.is_defined()); - if (*inbound_rtp->ssrc == *ssrc) { - rtpstream_ids.push_back(inbound_rtp->id()); - } + // Find inbound-rtp(s) of the receiver, i.e. the inbound-rtp(s) that + // reference the receiver stats. + // Because we do not implement receiver stats, we look at inbound-rtp(s) + // that reference the track attachment stats for the receiver instead. + std::string track_id = + DEPRECATED_RTCMediaStreamTrackStatsIDFromDirectionAndAttachment( + kDirectionInbound, receiver_selector->AttachmentId()); + for (const auto& stats : *report) { + if (stats.type() != RTCInboundRTPStreamStats::kType) + continue; + const auto& inbound_rtp = stats.cast_to(); + if (inbound_rtp.track_id.is_defined() && + *inbound_rtp.track_id == track_id) { + rtpstream_ids.push_back(inbound_rtp.id()); } } } @@ -1304,6 +1301,8 @@ RTCStatsCollector::CreateReportFilteredBySelector( return TakeReferencedStats(report->Copy(), rtpstream_ids); } +} // namespace + RTCStatsCollector::CertificateStatsPair RTCStatsCollector::CertificateStatsPair::Copy() const { CertificateStatsPair copy; diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h index 0f1cab19db..91175289e8 100644 --- a/pc/rtc_stats_collector.h +++ b/pc/rtc_stats_collector.h @@ -244,12 +244,6 @@ class RTCStatsCollector : public rtc::RefCountInterface, // This is a NO-OP if `network_report_` is null. void MergeNetworkReport_s(); - rtc::scoped_refptr CreateReportFilteredBySelector( - bool filter_by_sender_selector, - rtc::scoped_refptr report, - rtc::scoped_refptr sender_selector, - rtc::scoped_refptr receiver_selector); - // Slots for signals (sigslot) that are wired up to `pc_`. void OnSctpDataChannelCreated(SctpDataChannel* channel); // Slots for signals (sigslot) that are wired up to `channel`. diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 5a6ac545ca..3dbf0a09d0 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -388,10 +388,7 @@ rtc::scoped_refptr CreateMockSender( EXPECT_CALL(*sender, track()).WillRepeatedly(Return(track)); EXPECT_CALL(*sender, ssrc()).WillRepeatedly(Return(ssrc)); EXPECT_CALL(*sender, media_type()).WillRepeatedly(Return(media_type)); - EXPECT_CALL(*sender, GetParameters()) - .WillRepeatedly( - Invoke([s = sender.get()]() { return s->GetParametersInternal(); })); - EXPECT_CALL(*sender, GetParametersInternal()).WillRepeatedly(Invoke([ssrc]() { + EXPECT_CALL(*sender, GetParameters()).WillRepeatedly(Invoke([ssrc]() { RtpParameters params; params.encodings.push_back(RtpEncodingParameters()); params.encodings[0].ssrc = ssrc;