Ensure TestPeers are destroyed at the end of Run.
In order to correctly close audio dump files, TestPeers have to be
destroyed after the call is finished.
Bug: webrtc:10138
Change-Id: I948e4e1844dfbffd1eef7926a4dd4d7631dbe632
Reviewed-on: https://webrtc-review.googlesource.com/c/122301
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26661}diff --git a/test/pc/e2e/api/create_peerconnection_quality_test_fixture.cc b/test/pc/e2e/api/create_peerconnection_quality_test_fixture.cc
index 0ef36e9..3732dab 100644
--- a/test/pc/e2e/api/create_peerconnection_quality_test_fixture.cc
+++ b/test/pc/e2e/api/create_peerconnection_quality_test_fixture.cc
@@ -19,16 +19,9 @@
std::unique_ptr<PeerConnectionE2EQualityTestFixture>
CreatePeerConnectionE2EQualityTestFixture(
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::InjectableComponents>
- alice_components,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::Params> alice_params,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::InjectableComponents>
- bob_components,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::Params> bob_params,
std::unique_ptr<PeerConnectionE2EQualityTestFixture::Analyzers> analyzers) {
return absl::make_unique<webrtc::test::PeerConnectionE2EQualityTest>(
- std::move(alice_components), std::move(alice_params),
- std::move(bob_components), std::move(bob_params), std::move(analyzers));
+ std::move(analyzers));
}
} // namespace webrtc
diff --git a/test/pc/e2e/api/create_peerconnection_quality_test_fixture.h b/test/pc/e2e/api/create_peerconnection_quality_test_fixture.h
index a9e768f..09b71d5 100644
--- a/test/pc/e2e/api/create_peerconnection_quality_test_fixture.h
+++ b/test/pc/e2e/api/create_peerconnection_quality_test_fixture.h
@@ -21,12 +21,6 @@
// During the test Alice will be caller and Bob will answer the call.
std::unique_ptr<PeerConnectionE2EQualityTestFixture>
CreatePeerConnectionE2EQualityTestFixture(
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::InjectableComponents>
- alice_components,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::Params> alice_params,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::InjectableComponents>
- bob_components,
- std::unique_ptr<PeerConnectionE2EQualityTestFixture::Params> bob_params,
std::unique_ptr<PeerConnectionE2EQualityTestFixture::Analyzers> analyzers);
} // namespace webrtc
diff --git a/test/pc/e2e/api/peerconnection_quality_test_fixture.h b/test/pc/e2e/api/peerconnection_quality_test_fixture.h
index adffd0b..57bff9b 100644
--- a/test/pc/e2e/api/peerconnection_quality_test_fixture.h
+++ b/test/pc/e2e/api/peerconnection_quality_test_fixture.h
@@ -182,7 +182,11 @@
TimeDelta run_duration;
};
- virtual void Run(RunParams run_params) = 0;
+ virtual void Run(std::unique_ptr<InjectableComponents> alice_components,
+ std::unique_ptr<Params> alice_params,
+ std::unique_ptr<InjectableComponents> bob_components,
+ std::unique_ptr<Params> bob_params,
+ RunParams run_params) = 0;
virtual ~PeerConnectionE2EQualityTestFixture() = default;
};
diff --git a/test/pc/e2e/peer_connection_e2e_smoke_test.cc b/test/pc/e2e/peer_connection_e2e_smoke_test.cc
index f0c11f2..a126580 100644
--- a/test/pc/e2e/peer_connection_e2e_smoke_test.cc
+++ b/test/pc/e2e/peer_connection_e2e_smoke_test.cc
@@ -105,11 +105,11 @@
auto* video_analyzer = static_cast<ExampleVideoQualityAnalyzer*>(
analyzers->video_quality_analyzer.get());
- auto fixture = CreatePeerConnectionE2EQualityTestFixture(
- std::move(alice_components), std::move(alice_params),
- std::move(bob_components), absl::make_unique<Params>(),
- std::move(analyzers));
- fixture->Run(RunParams{TimeDelta::seconds(5)});
+ auto fixture =
+ CreatePeerConnectionE2EQualityTestFixture(std::move(analyzers));
+ fixture->Run(std::move(alice_components), std::move(alice_params),
+ std::move(bob_components), absl::make_unique<Params>(),
+ RunParams{TimeDelta::seconds(5)});
RTC_LOG(INFO) << "Captured: " << video_analyzer->frames_captured();
RTC_LOG(INFO) << "Sent : " << video_analyzer->frames_sent();
diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc
index 174a20c..44170c4 100644
--- a/test/pc/e2e/peer_connection_quality_test.cc
+++ b/test/pc/e2e/peer_connection_quality_test.cc
@@ -52,37 +52,10 @@
} // namespace
PeerConnectionE2EQualityTest::PeerConnectionE2EQualityTest(
- std::unique_ptr<InjectableComponents> alice_components,
- std::unique_ptr<Params> alice_params,
- std::unique_ptr<InjectableComponents> bob_components,
- std::unique_ptr<Params> bob_params,
std::unique_ptr<Analyzers> analyzers)
- : clock_(Clock::GetRealTimeClock()),
- signaling_thread_(rtc::Thread::Create()) {
- RTC_CHECK(alice_components);
- RTC_CHECK(alice_params);
- RTC_CHECK(bob_components);
- RTC_CHECK(bob_params);
+ : clock_(Clock::GetRealTimeClock()) {
RTC_CHECK(analyzers);
- // Print test summary
- RTC_LOG(INFO)
- << "Media quality test: Alice will make a call to Bob with media video="
- << !alice_params->video_configs.empty()
- << "; audio=" << alice_params->audio_config.has_value()
- << ". Bob will respond with media video="
- << !bob_params->video_configs.empty()
- << "; audio=" << bob_params->audio_config.has_value();
-
- // Check that at least Alice or Bob has at least one media stream.
- RTC_CHECK(!alice_params->video_configs.empty() ||
- alice_params->audio_config || !bob_params->video_configs.empty() ||
- bob_params->audio_config)
- << "No media in the call";
-
- signaling_thread_->SetName(kSignalThreadName, nullptr);
- signaling_thread_->Start();
-
// Create default video quality analyzer. We will always create an analyzer,
// even if there are no video streams, because it will be installed into video
// encoder/decoder factories.
@@ -97,6 +70,34 @@
std::move(analyzers->video_quality_analyzer),
encoded_image_id_controller_.get(),
encoded_image_id_controller_.get());
+}
+
+void PeerConnectionE2EQualityTest::Run(
+ std::unique_ptr<InjectableComponents> alice_components,
+ std::unique_ptr<Params> alice_params,
+ std::unique_ptr<InjectableComponents> bob_components,
+ std::unique_ptr<Params> bob_params,
+ RunParams run_params) {
+ RTC_CHECK(alice_components);
+ RTC_CHECK(alice_params);
+ RTC_CHECK(bob_components);
+ RTC_CHECK(bob_params);
+
+ SetMissedVideoStreamLabels({alice_params.get(), bob_params.get()});
+ ValidateParams({alice_params.get(), bob_params.get()});
+
+ // Print test summary
+ RTC_LOG(INFO)
+ << "Media quality test: Alice will make a call to Bob with media video="
+ << !alice_params->video_configs.empty()
+ << "; audio=" << alice_params->audio_config.has_value()
+ << ". Bob will respond with media video="
+ << !bob_params->video_configs.empty()
+ << "; audio=" << bob_params->audio_config.has_value();
+
+ const std::unique_ptr<rtc::Thread> signaling_thread = rtc::Thread::Create();
+ signaling_thread->SetName(kSignalThreadName, nullptr);
+ signaling_thread->Start();
// Create call participants: Alice and Bob.
// Audio streams are intercepted in AudioDeviceModule, so if it is required to
@@ -111,17 +112,12 @@
: absl::nullopt;
alice_ = TestPeer::CreateTestPeer(
std::move(alice_components), std::move(alice_params),
- video_quality_analyzer_injection_helper_.get(), signaling_thread_.get(),
+ video_quality_analyzer_injection_helper_.get(), signaling_thread.get(),
alice_audio_output_dump_file_name);
bob_ = TestPeer::CreateTestPeer(
std::move(bob_components), std::move(bob_params),
- video_quality_analyzer_injection_helper_.get(), signaling_thread_.get(),
+ video_quality_analyzer_injection_helper_.get(), signaling_thread.get(),
bob_audio_output_dump_file_name);
-}
-
-void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
- SetMissedVideoStreamLabels({alice_->params(), bob_->params()});
- ValidateParams({alice_->params(), bob_->params()});
int num_cores = CpuInfo::DetectNumberOfCores();
RTC_DCHECK_GE(num_cores, 1);
@@ -136,11 +132,20 @@
RTC_LOG(INFO) << "video_analyzer_threads=" << video_analyzer_threads;
video_quality_analyzer_injection_helper_->Start(video_analyzer_threads);
- signaling_thread_->Invoke<void>(
+ signaling_thread->Invoke<void>(
RTC_FROM_HERE,
rtc::Bind(&PeerConnectionE2EQualityTest::RunOnSignalingThread, this,
run_params));
video_quality_analyzer_injection_helper_->Stop();
+
+ // Ensuring that TestPeers have been destroyed in order to correctly close
+ // Audio dumps.
+ RTC_CHECK(!alice_);
+ RTC_CHECK(!bob_);
+ // Ensuring that FrameGeneratorCapturerVideoTrackSource and VideoFrameWriter
+ // are destroyed on the right thread.
+ RTC_CHECK(video_sources_.empty());
+ RTC_CHECK(video_writers_.empty());
}
void PeerConnectionE2EQualityTest::SetMissedVideoStreamLabels(
@@ -163,7 +168,14 @@
void PeerConnectionE2EQualityTest::ValidateParams(std::vector<Params*> params) {
std::set<std::string> video_labels;
+ int media_streams_count = 0;
+
for (Params* p : params) {
+ if (p->audio_config) {
+ media_streams_count++;
+ }
+ media_streams_count += p->video_configs.size();
+
// Validate that each video config has exactly one of |generator|,
// |input_file_name| or |screen_share_config| set. Also validate that all
// video stream labels are unique.
@@ -195,13 +207,15 @@
}
}
}
+
+ RTC_CHECK_GT(media_streams_count, 0) << "No media in the call.";
}
void PeerConnectionE2EQualityTest::RunOnSignalingThread(RunParams run_params) {
AddMedia(alice_.get());
AddMedia(bob_.get());
- SetupCall(alice_.get(), bob_.get());
+ SetupCall();
WaitForTransceiversSetup(alice_->params(), bob_.get());
WaitForTransceiversSetup(bob_->params(), alice_.get());
@@ -301,21 +315,21 @@
peer->AddTransceiver(track);
}
-void PeerConnectionE2EQualityTest::SetupCall(TestPeer* alice, TestPeer* bob) {
+void PeerConnectionE2EQualityTest::SetupCall() {
// Connect peers.
- ASSERT_TRUE(alice->ExchangeOfferAnswerWith(bob));
+ ASSERT_TRUE(alice_->ExchangeOfferAnswerWith(bob_.get()));
// Do the SDP negotiation, and also exchange ice candidates.
- ASSERT_EQ_WAIT(alice->signaling_state(), PeerConnectionInterface::kStable,
+ ASSERT_EQ_WAIT(alice_->signaling_state(), PeerConnectionInterface::kStable,
kDefaultTimeoutMs);
- ASSERT_TRUE_WAIT(alice->IsIceGatheringDone(), kDefaultTimeoutMs);
- ASSERT_TRUE_WAIT(bob->IsIceGatheringDone(), kDefaultTimeoutMs);
+ ASSERT_TRUE_WAIT(alice_->IsIceGatheringDone(), kDefaultTimeoutMs);
+ ASSERT_TRUE_WAIT(bob_->IsIceGatheringDone(), kDefaultTimeoutMs);
// Connect an ICE candidate pairs.
- ASSERT_TRUE(bob->AddIceCandidates(alice->observer()->GetAllCandidates()));
- ASSERT_TRUE(alice->AddIceCandidates(bob->observer()->GetAllCandidates()));
+ ASSERT_TRUE(bob_->AddIceCandidates(alice_->observer()->GetAllCandidates()));
+ ASSERT_TRUE(alice_->AddIceCandidates(bob_->observer()->GetAllCandidates()));
// This means that ICE and DTLS are connected.
- ASSERT_TRUE_WAIT(bob->IsIceConnected(), kDefaultTimeoutMs);
- ASSERT_TRUE_WAIT(alice->IsIceConnected(), kDefaultTimeoutMs);
+ ASSERT_TRUE_WAIT(bob_->IsIceConnected(), kDefaultTimeoutMs);
+ ASSERT_TRUE_WAIT(alice_->IsIceConnected(), kDefaultTimeoutMs);
}
void PeerConnectionE2EQualityTest::WaitForTransceiversSetup(
@@ -379,6 +393,11 @@
for (const auto& video_writer : video_writers_) {
video_writer->Close();
}
+
+ video_sources_.clear();
+ video_writers_.clear();
+ alice_.reset();
+ bob_.reset();
}
VideoFrameWriter* PeerConnectionE2EQualityTest::MaybeCreateVideoWriter(
diff --git a/test/pc/e2e/peer_connection_quality_test.h b/test/pc/e2e/peer_connection_quality_test.h
index a1908af..8cce506 100644
--- a/test/pc/e2e/peer_connection_quality_test.h
+++ b/test/pc/e2e/peer_connection_quality_test.h
@@ -38,16 +38,15 @@
using RunParams = PeerConnectionE2EQualityTestFixture::RunParams;
using VideoConfig = PeerConnectionE2EQualityTestFixture::VideoConfig;
- PeerConnectionE2EQualityTest(
- std::unique_ptr<InjectableComponents> alice_components,
- std::unique_ptr<Params> alice_params,
- std::unique_ptr<InjectableComponents> bob_components,
- std::unique_ptr<Params> bob_params,
- std::unique_ptr<Analyzers> analyzers);
+ PeerConnectionE2EQualityTest(std::unique_ptr<Analyzers> analyzers);
~PeerConnectionE2EQualityTest() override = default;
- void Run(RunParams run_params) override;
+ void Run(std::unique_ptr<InjectableComponents> alice_components,
+ std::unique_ptr<Params> alice_params,
+ std::unique_ptr<InjectableComponents> bob_components,
+ std::unique_ptr<Params> bob_params,
+ RunParams run_params) override;
private:
// Sets video stream labels that are not specified in VideoConfigs to unique
@@ -63,7 +62,7 @@
std::unique_ptr<FrameGenerator> CreateFrameGenerator(
const VideoConfig& video_config);
void AddAudio(TestPeer* peer);
- void SetupCall(TestPeer* alice, TestPeer* bob);
+ void SetupCall();
void WaitForTransceiversSetup(Params* params, TestPeer* remote_peer);
void SetupVideoSink(Params* params, TestPeer* remote_peer);
void StartVideo();
@@ -77,7 +76,6 @@
video_quality_analyzer_injection_helper_;
std::unique_ptr<SingleProcessEncodedImageIdInjector>
encoded_image_id_controller_;
- const std::unique_ptr<rtc::Thread> signaling_thread_;
std::unique_ptr<TestPeer> alice_;
std::unique_ptr<TestPeer> bob_;
