[PCLF] Store params as value and return const ref
This is the first step in the next refactoring:
1. Make general peer params immutable and accessible only via const ref.
2. Extract params which can be changed during the call
3. Expose mutators for mutable params protecting them with proper
locking.
Bug: b/213863770
Change-Id: Ie3ac17918f1fed3b8ec84992f8b0afc402bce7f4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260003
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36647}
diff --git a/test/pc/e2e/media/media_helper.cc b/test/pc/e2e/media/media_helper.cc
index 7985bd0..1689905 100644
--- a/test/pc/e2e/media/media_helper.cc
+++ b/test/pc/e2e/media/media_helper.cc
@@ -33,10 +33,10 @@
} // namespace
void MediaHelper::MaybeAddAudio(TestPeer* peer) {
- if (!peer->params()->audio_config) {
+ if (!peer->params2().audio_config) {
return;
}
- const AudioConfig& audio_config = peer->params()->audio_config.value();
+ const AudioConfig& audio_config = peer->params2().audio_config.value();
rtc::scoped_refptr<webrtc::AudioSourceInterface> source =
peer->pc_factory()->CreateAudioSource(audio_config.audio_options);
rtc::scoped_refptr<AudioTrackInterface> track =
@@ -51,15 +51,15 @@
std::vector<rtc::scoped_refptr<TestVideoCapturerVideoTrackSource>>
MediaHelper::MaybeAddVideo(TestPeer* peer) {
// Params here valid because of pre-run validation.
- Params* params = peer->params();
+ const Params& params = peer->params2();
std::vector<rtc::scoped_refptr<TestVideoCapturerVideoTrackSource>> out;
- for (size_t i = 0; i < params->video_configs.size(); ++i) {
- auto video_config = params->video_configs[i];
+ for (size_t i = 0; i < params.video_configs.size(); ++i) {
+ auto video_config = params.video_configs[i];
// Setup input video source into peer connection.
std::unique_ptr<test::TestVideoCapturer> capturer = CreateVideoCapturer(
video_config, peer->ReleaseVideoSource(i),
video_quality_analyzer_injection_helper_->CreateFramePreprocessor(
- params->name.value(), video_config));
+ params.name.value(), video_config));
bool is_screencast =
video_config.content_hint == VideoTrackInterface::ContentHint::kText ||
video_config.content_hint ==
diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc
index 8729eea..4841116 100644
--- a/test/pc/e2e/peer_connection_quality_test.cc
+++ b/test/pc/e2e/peer_connection_quality_test.cc
@@ -287,8 +287,8 @@
video_quality_analyzer_injection_helper_->Start(
test_case_name_,
- std::vector<std::string>{alice_->params()->name.value(),
- bob_->params()->name.value()},
+ std::vector<std::string>{alice_->params2().name.value(),
+ bob_->params2().name.value()},
video_analyzer_threads);
audio_quality_analyzer_->Start(test_case_name_, &analyzer_helper_);
for (auto& reporter : quality_metrics_reporters_) {
@@ -296,15 +296,15 @@
}
// Start RTCEventLog recording if requested.
- if (alice_->params()->rtc_event_log_path) {
+ if (alice_->params2().rtc_event_log_path) {
auto alice_rtc_event_log = std::make_unique<webrtc::RtcEventLogOutputFile>(
- alice_->params()->rtc_event_log_path.value());
+ alice_->params2().rtc_event_log_path.value());
alice_->pc()->StartRtcEventLog(std::move(alice_rtc_event_log),
webrtc::RtcEventLog::kImmediateOutput);
}
- if (bob_->params()->rtc_event_log_path) {
+ if (bob_->params2().rtc_event_log_path) {
auto bob_rtc_event_log = std::make_unique<webrtc::RtcEventLogOutputFile>(
- bob_->params()->rtc_event_log_path.value());
+ bob_->params2().rtc_event_log_path.value());
bob_->pc()->StartRtcEventLog(std::move(bob_rtc_event_log),
webrtc::RtcEventLog::kImmediateOutput);
}
@@ -317,8 +317,8 @@
return kAliveMessageLogInterval;
});
- RTC_LOG(LS_INFO) << "Configuration is done. Now " << *alice_->params()->name
- << " is calling to " << *bob_->params()->name << "...";
+ RTC_LOG(LS_INFO) << "Configuration is done. Now " << *alice_->params2().name
+ << " is calling to " << *bob_->params2().name << "...";
// Setup stats poller.
std::vector<StatsObserverInterface*> observers = {
@@ -327,8 +327,8 @@
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, {{*alice_->params2().name, alice_.get()},
+ {*bob_->params2().name, bob_.get()}});
executor_->ScheduleActivity(TimeDelta::Zero(), kStatsUpdateInterval,
[&stats_poller](TimeDelta) {
stats_poller.PollStatsAndNotifyObservers();
@@ -456,7 +456,7 @@
RtpTransceiverInit receive_only_transceiver_init;
receive_only_transceiver_init.direction = RtpTransceiverDirection::kRecvOnly;
int alice_transceivers_counter = 0;
- if (bob_->params()->audio_config) {
+ if (bob_->params2().audio_config) {
// Setup receive audio transceiver if Bob has audio to send. If we'll need
// multiple audio streams, then we need transceiver for each Bob's audio
// stream.
@@ -468,13 +468,13 @@
}
size_t alice_video_transceivers_non_simulcast_counter = 0;
- for (auto& video_config : alice_->params()->video_configs) {
+ for (auto& video_config : alice_->params2().video_configs) {
RtpTransceiverInit transceiver_params;
if (video_config.simulcast_config) {
transceiver_params.direction = RtpTransceiverDirection::kSendOnly;
- // Because simulcast enabled `alice_->params()->video_codecs` has only 1
+ // Because simulcast enabled `alice_->params2().video_codecs` has only 1
// element.
- if (alice_->params()->video_codecs[0].name == cricket::kVp8CodecName) {
+ if (alice_->params2().video_codecs[0].name == cricket::kVp8CodecName) {
// For Vp8 simulcast we need to add as many RtpEncodingParameters to the
// track as many simulcast streams requested. If they specified in
// `video_config.simulcast_config` it should be copied from there.
@@ -510,7 +510,7 @@
// Add receive only transceivers in case Bob has more video_configs than
// Alice.
for (size_t i = alice_video_transceivers_non_simulcast_counter;
- i < bob_->params()->video_configs.size(); ++i) {
+ i < bob_->params2().video_configs.size(); ++i) {
RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>> result =
alice_->AddTransceiver(cricket::MediaType::MEDIA_TYPE_VIDEO,
receive_only_transceiver_init);
@@ -535,15 +535,15 @@
void PeerConnectionE2EQualityTest::SetPeerCodecPreferences(TestPeer* peer) {
std::vector<RtpCodecCapability> with_rtx_video_capabilities =
FilterVideoCodecCapabilities(
- peer->params()->video_codecs, true, peer->params()->use_ulp_fec,
- peer->params()->use_flex_fec,
+ peer->params2().video_codecs, true, peer->params2().use_ulp_fec,
+ peer->params2().use_flex_fec,
peer->pc_factory()
->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
.codecs);
std::vector<RtpCodecCapability> without_rtx_video_capabilities =
FilterVideoCodecCapabilities(
- peer->params()->video_codecs, false, peer->params()->use_ulp_fec,
- peer->params()->use_flex_fec,
+ peer->params2().video_codecs, false, peer->params2().use_ulp_fec,
+ peer->params2().use_flex_fec,
peer->pc_factory()
->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
.codecs);
@@ -572,7 +572,7 @@
std::map<std::string, int> stream_label_to_simulcast_streams_count;
// We add only Alice here, because simulcast/svc is supported only from the
// first peer.
- for (auto& video_config : alice_->params()->video_configs) {
+ for (auto& video_config : alice_->params2().video_configs) {
if (video_config.simulcast_config) {
stream_label_to_simulcast_streams_count.insert(
{*video_config.stream_label,
@@ -621,7 +621,7 @@
offer->ToString(&log_output);
RTC_LOG(LS_INFO) << "Original offer: " << log_output;
LocalAndRemoteSdp patch_result = signaling_interceptor->PatchOffer(
- std::move(offer), alice_->params()->video_codecs[0]);
+ std::move(offer), alice_->params2().video_codecs[0]);
patch_result.local_sdp->ToString(&log_output);
RTC_LOG(LS_INFO) << "Offer to set as local description: " << log_output;
patch_result.remote_sdp->ToString(&log_output);
@@ -638,7 +638,7 @@
answer->ToString(&log_output);
RTC_LOG(LS_INFO) << "Original answer: " << log_output;
patch_result = signaling_interceptor->PatchAnswer(
- std::move(answer), bob_->params()->video_codecs[0]);
+ std::move(answer), bob_->params2().video_codecs[0]);
patch_result.local_sdp->ToString(&log_output);
RTC_LOG(LS_INFO) << "Answer to set as local description: " << log_output;
patch_result.remote_sdp->ToString(&log_output);
@@ -661,7 +661,7 @@
for (auto& candidate : alice_candidates) {
std::string candidate_str;
RTC_CHECK(candidate->ToString(&candidate_str));
- RTC_LOG(LS_INFO) << *alice_->params()->name
+ RTC_LOG(LS_INFO) << *alice_->params2().name
<< " ICE candidate(mid= " << candidate->sdp_mid()
<< "): " << candidate_str;
}
@@ -672,7 +672,7 @@
for (auto& candidate : bob_candidates) {
std::string candidate_str;
RTC_CHECK(candidate->ToString(&candidate_str));
- RTC_LOG(LS_INFO) << *bob_->params()->name
+ RTC_LOG(LS_INFO) << *bob_->params2().name
<< " ICE candidate(mid= " << candidate->sdp_mid()
<< "): " << candidate_str;
}
@@ -707,11 +707,11 @@
}
void PeerConnectionE2EQualityTest::ReportGeneralTestResults() {
- test::PrintResult(*alice_->params()->name + "_connected", "", test_case_name_,
+ test::PrintResult(*alice_->params2().name + "_connected", "", test_case_name_,
alice_connected_, "unitless",
/*important=*/false,
test::ImproveDirection::kBiggerIsBetter);
- test::PrintResult(*bob_->params()->name + "_connected", "", test_case_name_,
+ test::PrintResult(*bob_->params2().name + "_connected", "", test_case_name_,
bob_connected_, "unitless",
/*important=*/false,
test::ImproveDirection::kBiggerIsBetter);
diff --git a/test/pc/e2e/test_peer.cc b/test/pc/e2e/test_peer.cc
index 78347ed..02bbafc 100644
--- a/test/pc/e2e/test_peer.cc
+++ b/test/pc/e2e/test_peer.cc
@@ -50,8 +50,7 @@
pc()->SetRemoteDescription(std::move(desc), observer);
RTC_CHECK(observer->is_called());
if (!observer->error().ok()) {
- RTC_LOG(LS_ERROR) << *params_->name
- << ": Failed to set remote description: "
+ RTC_LOG(LS_ERROR) << *params_.name << ": Failed to set remote description: "
<< observer->error().message();
if (error_out) {
*error_out = observer->error().message();
@@ -96,11 +95,11 @@
std::vector<PeerConfigurerImpl::VideoSource> video_sources,
rtc::scoped_refptr<AudioProcessing> audio_processing,
std::unique_ptr<rtc::Thread> worker_thread)
- : worker_thread_(std::move(worker_thread)),
+ : params_(std::move(*params)),
+ worker_thread_(std::move(worker_thread)),
wrapper_(std::make_unique<PeerConnectionWrapper>(std::move(pc_factory),
std::move(pc),
std::move(observer))),
- params_(std::move(params)),
video_sources_(std::move(video_sources)),
audio_processing_(audio_processing) {}
diff --git a/test/pc/e2e/test_peer.h b/test/pc/e2e/test_peer.h
index 0aadeb6..310725e 100644
--- a/test/pc/e2e/test_peer.h
+++ b/test/pc/e2e/test_peer.h
@@ -33,7 +33,12 @@
// Describes a single participant in the call.
class TestPeer final {
public:
- Params* params() const { return params_.get(); }
+ // TODO(titovartem): remove when downstream projects will stop using it.
+ Params* params() { return ¶ms_; }
+
+ // TODO(titovartem): rename to params after removing the method above.
+ const Params& params2() const { return params_; }
+
PeerConfigurerImpl::VideoSource ReleaseVideoSource(size_t i) {
RTC_CHECK(wrapper_) << "TestPeer is already closed";
return std::move(video_sources_[i]);
@@ -57,11 +62,11 @@
void CreateOffer(
rtc::scoped_refptr<CreateSessionDescriptionObserver> observer) {
RTC_CHECK(wrapper_) << "TestPeer is already closed";
- pc()->CreateOffer(observer.release(), params_->rtc_offer_answer_options);
+ pc()->CreateOffer(observer.release(), params_.rtc_offer_answer_options);
}
std::unique_ptr<SessionDescriptionInterface> CreateOffer() {
RTC_CHECK(wrapper_) << "TestPeer is already closed";
- return wrapper_->CreateOffer(params_->rtc_offer_answer_options);
+ return wrapper_->CreateOffer(params_.rtc_offer_answer_options);
}
std::unique_ptr<SessionDescriptionInterface> CreateAnswer() {
@@ -145,10 +150,10 @@
std::unique_ptr<rtc::Thread> worker_thread);
private:
+ Params params_;
// Keeps ownership of worker thread. It has to be destroyed after `wrapper_`.
std::unique_ptr<rtc::Thread> worker_thread_;
std::unique_ptr<PeerConnectionWrapper> wrapper_;
- std::unique_ptr<Params> params_;
std::vector<PeerConfigurerImpl::VideoSource> video_sources_;
rtc::scoped_refptr<AudioProcessing> audio_processing_;
