Add ability to configure video codecs at peer level (PC level framework)
Bug: b/192821182
Change-Id: Ic1b55028102fbd331f0fb6a3a8c758c311267cbc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226220
Reviewed-by: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34477}
diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h
index 8717e8f..20d9c74 100644
--- a/api/test/peerconnection_quality_test_fixture.h
+++ b/api/test/peerconnection_quality_test_fixture.h
@@ -268,6 +268,27 @@
absl::optional<std::string> sync_group;
};
+ struct VideoCodecConfig {
+ explicit VideoCodecConfig(std::string name)
+ : name(std::move(name)), required_params() {}
+ VideoCodecConfig(std::string name,
+ std::map<std::string, std::string> required_params)
+ : name(std::move(name)), required_params(std::move(required_params)) {}
+ // Next two fields are used to specify concrete video codec, that should be
+ // used in the test. Video code will be negotiated in SDP during offer/
+ // answer exchange.
+ // Video codec name. You can find valid names in
+ // media/base/media_constants.h
+ std::string name = cricket::kVp8CodecName;
+ // Map of parameters, that have to be specified on SDP codec. Each parameter
+ // is described by key and value. Codec parameters will match the specified
+ // map if and only if for each key from |required_params| there will be
+ // a parameter with name equal to this key and parameter value will be equal
+ // to the value from |required_params| for this key.
+ // If empty then only name will be used to match the codec.
+ std::map<std::string, std::string> required_params;
+ };
+
// This class is used to fully configure one peer inside the call.
class PeerConfigurer {
public:
@@ -343,6 +364,14 @@
// applied to all summed RTP streams for this peer.
virtual PeerConfigurer* SetBitrateSettings(
BitrateSettings bitrate_settings) = 0;
+ // Set the list of video codecs used by the peer during the test. These
+ // codecs will be negotiated in SDP during offer/answer exchange. The order
+ // of these codecs during negotiation will be the same as in |video_codecs|.
+ // Codecs have to be available in codecs list provided by peer connection to
+ // be negotiated. If some of specified codecs won't be found, the test will
+ // crash.
+ virtual PeerConfigurer* SetVideoCodecs(
+ std::vector<VideoCodecConfig> video_codecs) = 0;
};
// Contains configuration for echo emulator.
@@ -352,27 +381,6 @@
TimeDelta echo_delay = TimeDelta::Millis(50);
};
- struct VideoCodecConfig {
- explicit VideoCodecConfig(std::string name)
- : name(std::move(name)), required_params() {}
- VideoCodecConfig(std::string name,
- std::map<std::string, std::string> required_params)
- : name(std::move(name)), required_params(std::move(required_params)) {}
- // Next two fields are used to specify concrete video codec, that should be
- // used in the test. Video code will be negotiated in SDP during offer/
- // answer exchange.
- // Video codec name. You can find valid names in
- // media/base/media_constants.h
- std::string name = cricket::kVp8CodecName;
- // Map of parameters, that have to be specified on SDP codec. Each parameter
- // is described by key and value. Codec parameters will match the specified
- // map if and only if for each key from |required_params| there will be
- // a parameter with name equal to this key and parameter value will be equal
- // to the value from |required_params| for this key.
- // If empty then only name will be used to match the codec.
- std::map<std::string, std::string> required_params;
- };
-
// Contains parameters, that describe how long framework should run quality
// test.
struct RunParams {
@@ -383,6 +391,11 @@
// it will be shut downed.
TimeDelta run_duration;
+ // DEPRECATED: Instead of setting the codecs in RunParams (which apply to
+ // all the participants in the call, please set them with
+ // PeerConfigurer, this will allow more flexibility and let
+ // different Peers support different codecs.
+ //
// List of video codecs to use during the test. These codecs will be
// negotiated in SDP during offer/answer exchange. The order of these codecs
// during negotiation will be the same as in |video_codecs|. Codecs have
diff --git a/test/pc/e2e/peer_configurer.cc b/test/pc/e2e/peer_configurer.cc
index 18570c2..caf29eb 100644
--- a/test/pc/e2e/peer_configurer.cc
+++ b/test/pc/e2e/peer_configurer.cc
@@ -90,11 +90,22 @@
*p->name + "_auto_audio_stream_label_");
audio_stream_names_provider.MaybeSetName(&p->audio_config->stream_label);
}
- }
- if (run_params->video_codecs.empty()) {
- run_params->video_codecs.push_back(
- VideoCodecConfig(cricket::kVp8CodecName));
+ if (p->video_codecs.empty()) {
+ // TODO(mbonadei): Remove the usage of RunParams to set codecs, this is
+ // only needed for backwards compatibility.
+ if (!run_params->video_codecs.empty()) {
+ p->video_codecs = run_params->video_codecs;
+ } else {
+ p->video_codecs.push_back(
+ PeerConnectionE2EQualityTestFixture::VideoCodecConfig(
+ cricket::kVp8CodecName));
+ }
+ } else {
+ RTC_CHECK(run_params->video_codecs.empty())
+ << "Setting video_codecs in both PeerConfigurer and RunParams is not "
+ "supported.";
+ }
}
}
@@ -102,7 +113,6 @@
const RunParams& run_params,
const std::vector<std::unique_ptr<PeerConfigurerImpl>>& peers) {
RTC_CHECK_GT(run_params.video_encoder_bitrate_multiplier, 0.0);
- RTC_CHECK_GE(run_params.video_codecs.size(), 1);
std::set<std::string> peer_names;
std::set<std::string> video_labels;
@@ -113,6 +123,8 @@
for (size_t i = 0; i < peers.size(); ++i) {
Params* p = peers[i]->params();
+ // Each peer should at least support 1 video codec.
+ RTC_CHECK_GE(p->video_codecs.size(), 1);
{
RTC_CHECK(p->name);
@@ -160,14 +172,14 @@
RTC_CHECK_LT(*video_config.simulcast_config->target_spatial_index,
video_config.simulcast_config->simulcast_streams_count);
}
- RTC_CHECK_EQ(run_params.video_codecs.size(), 1)
+ RTC_CHECK_EQ(p->video_codecs.size(), 1)
<< "Only 1 video codec is supported when simulcast is enabled in "
<< "at least 1 video config";
RTC_CHECK(!video_config.max_encode_bitrate_bps)
<< "Setting max encode bitrate is not implemented for simulcast.";
RTC_CHECK(!video_config.min_encode_bitrate_bps)
<< "Setting min encode bitrate is not implemented for simulcast.";
- if (run_params.video_codecs[0].name == cricket::kVp8CodecName &&
+ if (p->video_codecs[0].name == cricket::kVp8CodecName &&
!video_config.simulcast_config->encoding_params.empty()) {
RTC_CHECK_EQ(video_config.simulcast_config->simulcast_streams_count,
video_config.simulcast_config->encoding_params.size())
diff --git a/test/pc/e2e/peer_configurer.h b/test/pc/e2e/peer_configurer.h
index 422d3d7..bacfb7a 100644
--- a/test/pc/e2e/peer_configurer.h
+++ b/test/pc/e2e/peer_configurer.h
@@ -168,6 +168,12 @@
params_->bitrate_settings = bitrate_settings;
return this;
}
+ PeerConfigurer* SetVideoCodecs(
+ std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
+ video_codecs) override {
+ params_->video_codecs = std::move(video_codecs);
+ return this;
+ }
PeerConfigurer* SetIceTransportFactory(
std::unique_ptr<IceTransportFactory> factory) override {
diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc
index 38a9ebf..6fbfd37 100644
--- a/test/pc/e2e/peer_connection_quality_test.cc
+++ b/test/pc/e2e/peer_connection_quality_test.cc
@@ -444,8 +444,9 @@
RtpTransceiverInit transceiver_params;
if (video_config.simulcast_config) {
transceiver_params.direction = RtpTransceiverDirection::kSendOnly;
- // Because simulcast enabled |run_params.video_codecs| has only 1 element.
- if (run_params.video_codecs[0].name == cricket::kVp8CodecName) {
+ // Because simulcast enabled |alice_->params()->video_codecs| has only 1
+ // element.
+ if (alice_->params()->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.
@@ -508,14 +509,14 @@
const RunParams& run_params) {
std::vector<RtpCodecCapability> with_rtx_video_capabilities =
FilterVideoCodecCapabilities(
- run_params.video_codecs, true, run_params.use_ulp_fec,
+ peer->params()->video_codecs, true, run_params.use_ulp_fec,
run_params.use_flex_fec,
peer->pc_factory()
->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
.codecs);
std::vector<RtpCodecCapability> without_rtx_video_capabilities =
FilterVideoCodecCapabilities(
- run_params.video_codecs, false, run_params.use_ulp_fec,
+ peer->params()->video_codecs, false, run_params.use_ulp_fec,
run_params.use_flex_fec,
peer->pc_factory()
->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
@@ -552,8 +553,7 @@
video_config.simulcast_config->simulcast_streams_count});
}
}
- PatchingParams patching_params(run_params.video_codecs,
- run_params.use_conference_mode,
+ PatchingParams patching_params(run_params.use_conference_mode,
stream_label_to_simulcast_streams_count);
return std::make_unique<SignalingInterceptor>(patching_params);
}
@@ -594,8 +594,8 @@
RTC_CHECK(offer);
offer->ToString(&log_output);
RTC_LOG(INFO) << "Original offer: " << log_output;
- LocalAndRemoteSdp patch_result =
- signaling_interceptor->PatchOffer(std::move(offer));
+ LocalAndRemoteSdp patch_result = signaling_interceptor->PatchOffer(
+ std::move(offer), alice_->params()->video_codecs[0]);
patch_result.local_sdp->ToString(&log_output);
RTC_LOG(INFO) << "Offer to set as local description: " << log_output;
patch_result.remote_sdp->ToString(&log_output);
@@ -611,7 +611,8 @@
RTC_CHECK(answer);
answer->ToString(&log_output);
RTC_LOG(INFO) << "Original answer: " << log_output;
- patch_result = signaling_interceptor->PatchAnswer(std::move(answer));
+ patch_result = signaling_interceptor->PatchAnswer(
+ std::move(answer), bob_->params()->video_codecs[0]);
patch_result.local_sdp->ToString(&log_output);
RTC_LOG(INFO) << "Answer to set as local description: " << log_output;
patch_result.remote_sdp->ToString(&log_output);
diff --git a/test/pc/e2e/peer_connection_quality_test_params.h b/test/pc/e2e/peer_connection_quality_test_params.h
index e1c0232..31938f7 100644
--- a/test/pc/e2e/peer_connection_quality_test_params.h
+++ b/test/pc/e2e/peer_connection_quality_test_params.h
@@ -116,6 +116,8 @@
PeerConnectionInterface::RTCConfiguration rtc_configuration;
BitrateSettings bitrate_settings;
+ std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
+ video_codecs;
};
} // namespace webrtc_pc_e2e
diff --git a/test/pc/e2e/sdp/sdp_changer.cc b/test/pc/e2e/sdp/sdp_changer.cc
index b46aea1..aa067f0 100644
--- a/test/pc/e2e/sdp/sdp_changer.cc
+++ b/test/pc/e2e/sdp/sdp_changer.cc
@@ -166,14 +166,15 @@
}
LocalAndRemoteSdp SignalingInterceptor::PatchOffer(
- std::unique_ptr<SessionDescriptionInterface> offer) {
+ std::unique_ptr<SessionDescriptionInterface> offer,
+ const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec) {
for (auto& content : offer->description()->contents()) {
context_.mids_order.push_back(content.mid());
cricket::MediaContentDescription* media_desc = content.media_description();
if (media_desc->type() != cricket::MediaType::MEDIA_TYPE_VIDEO) {
continue;
}
- if (content.media_description()->streams().size() == 0) {
+ if (content.media_description()->streams().empty()) {
// It means that this media section describes receive only media section
// in SDP.
RTC_CHECK_EQ(content.media_description()->direction(),
@@ -183,13 +184,13 @@
media_desc->set_conference_mode(params_.use_conference_mode);
}
- if (params_.stream_label_to_simulcast_streams_count.size() > 0) {
+ if (!params_.stream_label_to_simulcast_streams_count.empty()) {
// Because simulcast enabled |params_.video_codecs| has only 1 element.
- if (params_.video_codecs[0].name == cricket::kVp8CodecName) {
+ if (first_codec.name == cricket::kVp8CodecName) {
return PatchVp8Offer(std::move(offer));
}
- if (params_.video_codecs[0].name == cricket::kVp9CodecName) {
+ if (first_codec.name == cricket::kVp9CodecName) {
return PatchVp9Offer(std::move(offer));
}
}
@@ -362,7 +363,8 @@
}
LocalAndRemoteSdp SignalingInterceptor::PatchAnswer(
- std::unique_ptr<SessionDescriptionInterface> answer) {
+ std::unique_ptr<SessionDescriptionInterface> answer,
+ const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec) {
for (auto& content : answer->description()->contents()) {
cricket::MediaContentDescription* media_desc = content.media_description();
if (media_desc->type() != cricket::MediaType::MEDIA_TYPE_VIDEO) {
@@ -375,13 +377,13 @@
media_desc->set_conference_mode(params_.use_conference_mode);
}
- if (params_.stream_label_to_simulcast_streams_count.size() > 0) {
+ if (!params_.stream_label_to_simulcast_streams_count.empty()) {
// Because simulcast enabled |params_.video_codecs| has only 1 element.
- if (params_.video_codecs[0].name == cricket::kVp8CodecName) {
+ if (first_codec.name == cricket::kVp8CodecName) {
return PatchVp8Answer(std::move(answer));
}
- if (params_.video_codecs[0].name == cricket::kVp9CodecName) {
+ if (first_codec.name == cricket::kVp9CodecName) {
return PatchVp9Answer(std::move(answer));
}
}
@@ -534,7 +536,7 @@
// This is candidate for simulcast section, so it should be transformed
// into candidates for replicated sections. The sdpMLineIndex is set to
// -1 and ignored if the rid is present.
- for (auto rid : simulcast_info_it->second->rids) {
+ for (const std::string& rid : simulcast_info_it->second->rids) {
out.push_back(CreateIceCandidate(rid, -1, candidate->candidate()));
}
} else {
@@ -560,7 +562,7 @@
// section.
out.push_back(CreateIceCandidate(simulcast_info_it->second->mid, 0,
candidate->candidate()));
- } else if (context_.simulcast_infos_by_rid.size()) {
+ } else if (!context_.simulcast_infos_by_rid.empty()) {
// When using simulcast and bundle, put everything on the first m-line.
out.push_back(CreateIceCandidate("", 0, candidate->candidate()));
} else {
diff --git a/test/pc/e2e/sdp/sdp_changer.h b/test/pc/e2e/sdp/sdp_changer.h
index 11e3d42..943d332 100644
--- a/test/pc/e2e/sdp/sdp_changer.h
+++ b/test/pc/e2e/sdp/sdp_changer.h
@@ -61,17 +61,12 @@
struct PatchingParams {
PatchingParams(
- std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
- video_codecs,
bool use_conference_mode,
std::map<std::string, int> stream_label_to_simulcast_streams_count)
- : video_codecs(std::move(video_codecs)),
- use_conference_mode(use_conference_mode),
+ : use_conference_mode(use_conference_mode),
stream_label_to_simulcast_streams_count(
stream_label_to_simulcast_streams_count) {}
- std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
- video_codecs;
bool use_conference_mode;
std::map<std::string, int> stream_label_to_simulcast_streams_count;
};
@@ -81,9 +76,11 @@
explicit SignalingInterceptor(PatchingParams params) : params_(params) {}
LocalAndRemoteSdp PatchOffer(
- std::unique_ptr<SessionDescriptionInterface> offer);
+ std::unique_ptr<SessionDescriptionInterface> offer,
+ const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec);
LocalAndRemoteSdp PatchAnswer(
- std::unique_ptr<SessionDescriptionInterface> offer);
+ std::unique_ptr<SessionDescriptionInterface> answer,
+ const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec);
std::vector<std::unique_ptr<IceCandidateInterface>> PatchOffererIceCandidates(
rtc::ArrayView<const IceCandidateInterface* const> candidates);
@@ -132,9 +129,9 @@
LocalAndRemoteSdp PatchVp9Offer(
std::unique_ptr<SessionDescriptionInterface> offer);
LocalAndRemoteSdp PatchVp8Answer(
- std::unique_ptr<SessionDescriptionInterface> offer);
+ std::unique_ptr<SessionDescriptionInterface> answer);
LocalAndRemoteSdp PatchVp9Answer(
- std::unique_ptr<SessionDescriptionInterface> offer);
+ std::unique_ptr<SessionDescriptionInterface> answer);
void FillSimulcastContext(SessionDescriptionInterface* offer);
std::unique_ptr<cricket::SessionDescription> RestoreMediaSectionsOrder(
