zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[PCLF] Store params as value and return const ref

Browse Source 
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}
This commit is contained in:
Artem Titov
2022-04-25 18:40:09 +02:00
committed by WebRTC LUCI CQ
parent 7765f9e3df
commit bf282bc3d6
4 changed files with 44 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
test/pc/e2e/media/media_helper.cc
View File

@ -33,10 +33,10 @@ using CapturingDeviceIndex = ::webrtc::webrtc_pc_e2e::
} // 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 @@ void MediaHelper::MaybeAddAudio(TestPeer* peer) {
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 ==

52
test/pc/e2e/peer_connection_quality_test.cc
View File

@ -287,8 +287,8 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
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 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
}
// 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 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
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 @@ 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, {{*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 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
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 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
}
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 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
// 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::TearDownCallOnSignalingThread() {
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 @@ PeerConnectionE2EQualityTest::CreateSignalingInterceptor(
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 @@ void PeerConnectionE2EQualityTest::ExchangeOfferAnswer(
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 @@ void PeerConnectionE2EQualityTest::ExchangeOfferAnswer(
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 @@ void PeerConnectionE2EQualityTest::ExchangeIceCandidates(
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 @@ void PeerConnectionE2EQualityTest::ExchangeIceCandidates(
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::TearDownCall() {
}
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);

7
test/pc/e2e/test_peer.cc
View File

@ -50,8 +50,7 @@ bool TestPeer::SetRemoteDescription(
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 @@ TestPeer::TestPeer(
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) {}

13
test/pc/e2e/test_peer.h
View File

@ -33,7 +33,12 @@ namespace webrtc_pc_e2e {
// 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 &params_; }
// 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 @@ class TestPeer final {
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 @@ class TestPeer final {
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_;