From a92d051e0f4f3dcd4c43086a8f1edbbe5859235f Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Wed, 27 Apr 2022 09:47:43 +0200 Subject: [PATCH] [PCLF] Introduce API to safely mutate ConfigurableParams in TestPeer Bug: b/213863770 Change-Id: I90b7b5cd55ac5a8ebee5d790205a4fa6700dfff4 No-Try: True Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260117 Reviewed-by: Mirko Bonadei Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/main@{#36668} --- .../peerconnection_quality_test_fixture.h | 6 ++-- test/pc/e2e/BUILD.gn | 2 ++ test/pc/e2e/test_peer.cc | 36 +++++++++++++++++++ test/pc/e2e/test_peer.h | 20 ++++++++--- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h index 18107760b7..0e7d85adf6 100644 --- a/api/test/peerconnection_quality_test_fixture.h +++ b/api/test/peerconnection_quality_test_fixture.h @@ -233,10 +233,10 @@ class PeerConnectionE2EQualityTestFixture { stream_label(std::move(stream_label)) {} // Video stream width. - const size_t width; + size_t width; // Video stream height. - const size_t height; - const int32_t fps; + size_t height; + int32_t fps; VideoResolution GetResolution() const { return VideoResolution(width, height, fps); } diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 33d51711e5..09629308c4 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -235,9 +235,11 @@ if (!build_with_chromium) { "../../../pc:peerconnection_wrapper", "../../../rtc_base:logging", "../../../rtc_base:refcount", + "../../../rtc_base/synchronization:mutex", ] absl_deps = [ "//third_party/abseil-cpp/absl/memory", + "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:variant", ] } diff --git a/test/pc/e2e/test_peer.cc b/test/pc/e2e/test_peer.cc index bf552ddf06..4f801f5c12 100644 --- a/test/pc/e2e/test_peer.cc +++ b/test/pc/e2e/test_peer.cc @@ -13,6 +13,7 @@ #include #include "absl/memory/memory.h" +#include "absl/strings/string_view.h" #include "api/scoped_refptr.h" #include "modules/audio_processing/include/audio_processing.h" @@ -20,6 +21,11 @@ namespace webrtc { namespace webrtc_pc_e2e { namespace { +using VideoSubscription = ::webrtc::webrtc_pc_e2e:: + PeerConnectionE2EQualityTestFixture::VideoSubscription; +using VideoConfig = + ::webrtc::webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig; + class SetRemoteDescriptionCallback : public webrtc::SetRemoteDescriptionObserverInterface { public: @@ -39,6 +45,36 @@ class SetRemoteDescriptionCallback } // namespace +ConfigurableParams TestPeer::configurable_params() const { + MutexLock lock(&mutex_); + return configurable_params_; +} + +void TestPeer::AddVideoConfig(VideoConfig config) { + MutexLock lock(&mutex_); + configurable_params_.video_configs.push_back(std::move(config)); +} + +void TestPeer::RemoveVideoConfig(absl::string_view stream_label) { + MutexLock lock(&mutex_); + bool config_removed = false; + for (auto it = configurable_params_.video_configs.begin(); + it != configurable_params_.video_configs.end(); ++it) { + if (*it->stream_label == stream_label) { + configurable_params_.video_configs.erase(it); + config_removed = true; + break; + } + } + RTC_CHECK(config_removed) << *params_.name << ": No video config with label [" + << stream_label << "] was found"; +} + +void TestPeer::SetVideoSubscription(VideoSubscription subscription) { + MutexLock lock(&mutex_); + configurable_params_.video_subscription = std::move(subscription); +} + bool TestPeer::SetRemoteDescription( std::unique_ptr desc, std::string* error_out) { diff --git a/test/pc/e2e/test_peer.h b/test/pc/e2e/test_peer.h index 6f068fe429..2a8b3a1497 100644 --- a/test/pc/e2e/test_peer.h +++ b/test/pc/e2e/test_peer.h @@ -15,6 +15,7 @@ #include #include "absl/memory/memory.h" +#include "absl/strings/string_view.h" #include "api/function_view.h" #include "api/scoped_refptr.h" #include "api/sequence_checker.h" @@ -24,6 +25,7 @@ #include "pc/peer_connection_wrapper.h" #include "rtc_base/logging.h" #include "rtc_base/ref_counted_object.h" +#include "rtc_base/synchronization/mutex.h" #include "test/pc/e2e/peer_configurer.h" #include "test/pc/e2e/peer_connection_quality_test_params.h" @@ -34,9 +36,14 @@ namespace webrtc_pc_e2e { class TestPeer final { public: const Params& params() const { return params_; } - ConfigurableParams configurable_params() const { - return configurable_params_; - } + + ConfigurableParams configurable_params() const; + void AddVideoConfig(PeerConnectionE2EQualityTestFixture::VideoConfig config); + // Removes video config with specified name. Crashes if the config with + // specified name isn't found. + void RemoveVideoConfig(absl::string_view stream_label); + void SetVideoSubscription( + PeerConnectionE2EQualityTestFixture::VideoSubscription subscription); PeerConfigurerImpl::VideoSource ReleaseVideoSource(size_t i) { RTC_CHECK(wrapper_) << "TestPeer is already closed"; @@ -150,8 +157,11 @@ class TestPeer final { std::unique_ptr worker_thread); private: - Params params_; - ConfigurableParams configurable_params_; + const Params params_; + + mutable Mutex mutex_; + ConfigurableParams configurable_params_ RTC_GUARDED_BY(mutex_); + // Keeps ownership of worker thread. It has to be destroyed after `wrapper_`. std::unique_ptr worker_thread_; std::unique_ptr wrapper_;