From 5f308fdd89241fdc2526d127739c105da75e472a Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Thu, 9 Jun 2022 18:04:50 +0200 Subject: [PATCH] [PCLF] Make it possible to unregister participant for stats poller Bug: b/231397778 Change-Id: I54c95543cbcf7d6ec9ae0bd121a07fd4e2a1fd4c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265408 Reviewed-by: Mirko Bonadei Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/main@{#37169} --- test/pc/e2e/BUILD.gn | 20 +++++ test/pc/e2e/peer_connection_quality_test.cc | 6 +- test/pc/e2e/stats_poller.cc | 27 ++++++- test/pc/e2e/stats_poller.h | 15 +++- test/pc/e2e/stats_poller_test.cc | 90 +++++++++++++++++++++ test/pc/e2e/stats_provider.h | 29 +++++++ test/pc/e2e/test_peer.h | 9 ++- 7 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 test/pc/e2e/stats_poller_test.cc create mode 100644 test/pc/e2e/stats_provider.h diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 3e7bc158ad..3832bba1dc 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -39,6 +39,7 @@ if (!build_with_chromium) { ":multi_head_queue_test", ":peer_connection_e2e_smoke_test", ":single_process_encoded_image_data_injector_unittest", + ":stats_poller_test", ":video_frame_tracking_id_injector_unittest", ] } @@ -226,6 +227,7 @@ if (!build_with_chromium) { deps = [ ":peer_configurer", ":peer_connection_quality_test_params", + ":stats_provider", "../../../api:frame_generator_api", "../../../api:function_view", "../../../api:libjingle_peerconnection_api", @@ -500,6 +502,13 @@ if (!build_with_chromium) { } } + rtc_library("stats_provider") { + visibility = [ "*" ] + testonly = true + sources = [ "stats_provider.h" ] + deps = [ "../../../api:rtc_stats_api" ] + } + rtc_library("stats_poller") { visibility = [ "*" ] testonly = true @@ -508,6 +517,7 @@ if (!build_with_chromium) { "stats_poller.h", ] deps = [ + ":stats_provider", ":test_peer", "../../../api:libjingle_peerconnection_api", "../../../api:rtc_stats_api", @@ -518,6 +528,16 @@ if (!build_with_chromium) { ] } + rtc_library("stats_poller_test") { + testonly = true + sources = [ "stats_poller_test.cc" ] + deps = [ + ":stats_poller", + "../..:test_support", + "../../../api:rtc_stats_api", + ] + } + rtc_library("default_video_quality_analyzer_test") { testonly = true sources = [ "analyzer/video/default_video_quality_analyzer_test.cc" ] diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 13a0880f79..232442fa59 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -329,8 +329,10 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { for (auto& reporter : quality_metrics_reporters_) { observers.push_back(reporter.get()); } - StatsPoller stats_poller(observers, {{*alice_->params().name, alice_.get()}, - {*bob_->params().name, bob_.get()}}); + StatsPoller stats_poller(observers, + std::map{ + {*alice_->params().name, alice_.get()}, + {*bob_->params().name, bob_.get()}}); executor_->ScheduleActivity(TimeDelta::Zero(), kStatsUpdateInterval, [&stats_poller](TimeDelta) { stats_poller.PollStatsAndNotifyObservers(); diff --git a/test/pc/e2e/stats_poller.cc b/test/pc/e2e/stats_poller.cc index ecf640be29..c04805fb20 100644 --- a/test/pc/e2e/stats_poller.cc +++ b/test/pc/e2e/stats_poller.cc @@ -19,7 +19,7 @@ namespace webrtc { namespace webrtc_pc_e2e { void InternalStatsObserver::PollStats() { - peer_->pc()->GetStats(this); + peer_->GetStats(this); } void InternalStatsObserver::OnStatsDelivered( @@ -29,9 +29,19 @@ void InternalStatsObserver::OnStatsDelivered( } } +StatsPoller::StatsPoller(std::vector observers, + std::map peers) + : observers_(std::move(observers)) { + webrtc::MutexLock lock(&mutex_); + for (auto& peer : peers) { + pollers_.push_back(rtc::make_ref_counted( + peer.first, peer.second, observers_)); + } +} + StatsPoller::StatsPoller(std::vector observers, std::map peers) - : observers_(observers) { + : observers_(std::move(observers)) { webrtc::MutexLock lock(&mutex_); for (auto& peer : peers) { pollers_.push_back(rtc::make_ref_counted( @@ -47,11 +57,22 @@ void StatsPoller::PollStatsAndNotifyObservers() { } void StatsPoller::RegisterParticipantInCall(absl::string_view peer_name, - TestPeer* peer) { + StatsProvider* peer) { webrtc::MutexLock lock(&mutex_); pollers_.push_back(rtc::make_ref_counted( peer_name, peer, observers_)); } +bool StatsPoller::UnregisterParticipantInCall(absl::string_view peer_name) { + webrtc::MutexLock lock(&mutex_); + for (auto it = pollers_.begin(); it != pollers_.end(); ++it) { + if ((*it)->pc_label() == peer_name) { + pollers_.erase(it); + return true; + } + } + return false; +} + } // namespace webrtc_pc_e2e } // namespace webrtc diff --git a/test/pc/e2e/stats_poller.h b/test/pc/e2e/stats_poller.h index 2b3f52ca22..3576f1bf05 100644 --- a/test/pc/e2e/stats_poller.h +++ b/test/pc/e2e/stats_poller.h @@ -21,6 +21,7 @@ #include "api/test/stats_observer_interface.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_annotations.h" +#include "test/pc/e2e/stats_provider.h" #include "test/pc/e2e/test_peer.h" namespace webrtc { @@ -31,10 +32,12 @@ namespace webrtc_pc_e2e { class InternalStatsObserver : public RTCStatsCollectorCallback { public: InternalStatsObserver(absl::string_view pc_label, - TestPeer* peer, + StatsProvider* peer, std::vector observers) : pc_label_(pc_label), peer_(peer), observers_(std::move(observers)) {} + std::string pc_label() const { return pc_label_; } + void PollStats(); void OnStatsDelivered( @@ -42,7 +45,7 @@ class InternalStatsObserver : public RTCStatsCollectorCallback { private: std::string pc_label_; - TestPeer* peer_; + StatsProvider* peer_; std::vector observers_; }; @@ -51,12 +54,18 @@ class InternalStatsObserver : public RTCStatsCollectorCallback { // webrtc::test::StatsObserverInterface subscribed. class StatsPoller { public: + StatsPoller(std::vector observers, + std::map peers_to_observe); StatsPoller(std::vector observers, std::map peers_to_observe); void PollStatsAndNotifyObservers(); - void RegisterParticipantInCall(absl::string_view peer_name, TestPeer* peer); + void RegisterParticipantInCall(absl::string_view peer_name, + StatsProvider* peer); + // Unregister participant from stats poller. Returns true if participant was + // removed and false if participant wasn't found. + bool UnregisterParticipantInCall(absl::string_view peer_name); private: const std::vector observers_; diff --git a/test/pc/e2e/stats_poller_test.cc b/test/pc/e2e/stats_poller_test.cc new file mode 100644 index 0000000000..02a323127b --- /dev/null +++ b/test/pc/e2e/stats_poller_test.cc @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "test/pc/e2e/stats_poller.h" + +#include "api/stats/rtc_stats_collector_callback.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { +namespace webrtc_pc_e2e { +namespace { + +using ::testing::Eq; + +class TestStatsProvider : public StatsProvider { + public: + ~TestStatsProvider() override = default; + + void GetStats(RTCStatsCollectorCallback* callback) override { + stats_collections_count_++; + } + + int stats_collections_count() const { return stats_collections_count_; } + + private: + int stats_collections_count_ = 0; +}; + +class MockStatsObserver : public StatsObserverInterface { + public: + ~MockStatsObserver() override = default; + + MOCK_METHOD(void, + OnStatsReports, + (absl::string_view pc_label, + const rtc::scoped_refptr& report)); +}; + +TEST(StatsPollerTest, UnregisterParticipantAddedInCtor) { + TestStatsProvider alice; + TestStatsProvider bob; + + MockStatsObserver stats_observer; + + StatsPoller poller(/*observers=*/{&stats_observer}, + /*peers_to_observe=*/{{"alice", &alice}, {"bob", &bob}}); + poller.PollStatsAndNotifyObservers(); + + EXPECT_THAT(alice.stats_collections_count(), Eq(1)); + EXPECT_THAT(bob.stats_collections_count(), Eq(1)); + + poller.UnregisterParticipantInCall("bob"); + poller.PollStatsAndNotifyObservers(); + + EXPECT_THAT(alice.stats_collections_count(), Eq(2)); + EXPECT_THAT(bob.stats_collections_count(), Eq(1)); +} + +TEST(StatsPollerTest, UnregisterParticipantRegisteredInCall) { + TestStatsProvider alice; + TestStatsProvider bob; + + MockStatsObserver stats_observer; + + StatsPoller poller(/*observers=*/{&stats_observer}, + /*peers_to_observe=*/{{"alice", &alice}}); + poller.RegisterParticipantInCall("bob", &bob); + poller.PollStatsAndNotifyObservers(); + + EXPECT_THAT(alice.stats_collections_count(), Eq(1)); + EXPECT_THAT(bob.stats_collections_count(), Eq(1)); + + poller.UnregisterParticipantInCall("bob"); + poller.PollStatsAndNotifyObservers(); + + EXPECT_THAT(alice.stats_collections_count(), Eq(2)); + EXPECT_THAT(bob.stats_collections_count(), Eq(1)); +} + +} // namespace +} // namespace webrtc_pc_e2e +} // namespace webrtc diff --git a/test/pc/e2e/stats_provider.h b/test/pc/e2e/stats_provider.h new file mode 100644 index 0000000000..eef62d779c --- /dev/null +++ b/test/pc/e2e/stats_provider.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef TEST_PC_E2E_STATS_PROVIDER_H_ +#define TEST_PC_E2E_STATS_PROVIDER_H_ + +#include "api/stats/rtc_stats_collector_callback.h" + +namespace webrtc { +namespace webrtc_pc_e2e { + +class StatsProvider { + public: + virtual ~StatsProvider() = default; + + virtual void GetStats(RTCStatsCollectorCallback* callback) = 0; +}; + +} // namespace webrtc_pc_e2e +} // namespace webrtc + +#endif // TEST_PC_E2E_STATS_PROVIDER_H_ diff --git a/test/pc/e2e/test_peer.h b/test/pc/e2e/test_peer.h index e21c054fa4..216b9d9dbe 100644 --- a/test/pc/e2e/test_peer.h +++ b/test/pc/e2e/test_peer.h @@ -28,13 +28,16 @@ #include "rtc_base/synchronization/mutex.h" #include "test/pc/e2e/peer_configurer.h" #include "test/pc/e2e/peer_connection_quality_test_params.h" +#include "test/pc/e2e/stats_provider.h" namespace webrtc { namespace webrtc_pc_e2e { // Describes a single participant in the call. -class TestPeer final { +class TestPeer final : public StatsProvider { public: + ~TestPeer() override = default; + const Params& params() const { return params_; } ConfigurableParams configurable_params() const; @@ -45,6 +48,10 @@ class TestPeer final { void SetVideoSubscription( PeerConnectionE2EQualityTestFixture::VideoSubscription subscription); + void GetStats(RTCStatsCollectorCallback* callback) override { + pc()->GetStats(callback); + } + PeerConfigurerImpl::VideoSource ReleaseVideoSource(size_t i) { RTC_CHECK(wrapper_) << "TestPeer is already closed"; return std::move(video_sources_[i]);