Modify SdpOfferAnswer to use field trials from PeerConnection
Currently the ones from PeerConnectionFactory is used :(
This patch is a follow up to https://webrtc-review.googlesource.com/c/src/+/409540.
The testcase from that patch "incorrectly" passed due to trickery in
integration_test_helper.
Fix the problem by passing env from PeerConnection into SdpOfferAnswer.
Also,
- untrick integration_test_helper
- make my original testcase pass also for planb
Bug: b/444370738
Change-Id: Ib911822894da47eb333e0b8477200a582d3af448
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/410040
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45676}
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index f056629..2836661 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -622,7 +622,7 @@
}
sdp_handler_ = SdpOfferAnswerHandler::Create(
- this, configuration_, std::move(dependencies.cert_generator),
+ env_, this, configuration_, std::move(dependencies.cert_generator),
std::move(dependencies.video_bitrate_allocator_factory), context_.get(),
codec_lookup_helper_.get());
rtp_manager_ = std::make_unique<RtpTransmissionManager>(
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index e3d3c21..1739c67 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -5029,8 +5029,7 @@
#endif // WEBRTC_HAVE_SCTP
-TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- PerPeerConnectionHeaderExtensions) {
+TEST_P(PeerConnectionIntegrationTest, PerPeerConnectionHeaderExtensions) {
SetFieldTrials("caller", "WebRTC-VideoFrameTrackingIdAdvertised/Enabled/");
SetFieldTrials("callee", "WebRTC-VideoFrameTrackingIdAdvertised/Disabled/");
PeerConnectionInterface::RTCConfiguration config;
@@ -5052,7 +5051,11 @@
const std::string uri =
"http://www.webrtc.org/experiments/rtp-hdrext/video-frame-tracking-id";
{
- caller()->pc()->AddTransceiver(MediaType::VIDEO);
+ scoped_refptr<VideoTrackInterface> caller_track =
+ caller()->CreateLocalVideoTrack();
+ scoped_refptr<RtpSenderInterface> caller_sender =
+ caller()->AddTrack(caller_track);
+
auto session_description = caller()->CreateOfferAndWait();
EXPECT_THAT(session_description->description()
->contents()[0]
@@ -5062,7 +5065,11 @@
}
{
- callee()->pc()->AddTransceiver(MediaType::VIDEO);
+ scoped_refptr<VideoTrackInterface> callee_track =
+ callee()->CreateLocalVideoTrack();
+
+ scoped_refptr<RtpSenderInterface> callee_sender =
+ callee()->AddTrack(callee_track);
auto session_description = callee()->CreateOfferAndWait();
EXPECT_THAT(session_description->description()
->contents()[0]
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 0add487..9bf6557 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -1424,9 +1424,11 @@
std::set<std::pair<std::string, std::string>> ice_credentials_;
};
-SdpOfferAnswerHandler::SdpOfferAnswerHandler(PeerConnectionSdpMethods* pc,
+SdpOfferAnswerHandler::SdpOfferAnswerHandler(const Environment& env,
+ PeerConnectionSdpMethods* pc,
ConnectionContext* context)
- : pc_(pc),
+ : env_(env),
+ pc_(pc),
context_(context),
local_streams_(StreamCollection::Create()),
remote_streams_(StreamCollection::Create()),
@@ -1446,6 +1448,7 @@
// Static
std::unique_ptr<SdpOfferAnswerHandler> SdpOfferAnswerHandler::Create(
+ const Environment& env,
PeerConnectionSdpMethods* pc,
const PeerConnectionInterface::RTCConfiguration& configuration,
std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator,
@@ -1453,7 +1456,7 @@
video_bitrate_allocator_factory,
ConnectionContext* context,
CodecLookupHelper* codec_lookup_helper) {
- auto handler = absl::WrapUnique(new SdpOfferAnswerHandler(pc, context));
+ auto handler = absl::WrapUnique(new SdpOfferAnswerHandler(env, pc, context));
handler->Initialize(configuration, std::move(cert_generator),
std::move(video_bitrate_allocator_factory), context,
codec_lookup_helper);
@@ -4390,8 +4393,7 @@
MediaType::AUDIO, GetDefaultMidForPlanB(MediaType::AUDIO),
RtpTransceiverDirectionFromSendRecv(send_audio, recv_audio), false);
options.header_extensions =
- media_engine()->voice().GetRtpHeaderExtensions(
- &context_->env().field_trials());
+ media_engine()->voice().GetRtpHeaderExtensions(&env_.field_trials());
session_options->media_description_options.push_back(options);
audio_index = session_options->media_description_options.size() - 1;
}
@@ -4400,8 +4402,7 @@
MediaType::VIDEO, GetDefaultMidForPlanB(MediaType::VIDEO),
RtpTransceiverDirectionFromSendRecv(send_video, recv_video), false);
options.header_extensions =
- media_engine()->video().GetRtpHeaderExtensions(
- &context_->env().field_trials());
+ media_engine()->video().GetRtpHeaderExtensions(&env_.field_trials());
session_options->media_description_options.push_back(options);
video_index = session_options->media_description_options.size() - 1;
}
@@ -5535,8 +5536,7 @@
*audio_index = session_options->media_description_options.size() - 1;
}
session_options->media_description_options.back().header_extensions =
- media_engine()->voice().GetRtpHeaderExtensions(
- &context_->env().field_trials());
+ media_engine()->voice().GetRtpHeaderExtensions(&env_.field_trials());
} else if (IsVideoContent(&content)) {
// If we already have an video m= section, reject this extra one.
if (*video_index) {
@@ -5552,8 +5552,7 @@
*video_index = session_options->media_description_options.size() - 1;
}
session_options->media_description_options.back().header_extensions =
- media_engine()->video().GetRtpHeaderExtensions(
- &context_->env().field_trials());
+ media_engine()->video().GetRtpHeaderExtensions(&env_.field_trials());
} else if (IsUnsupportedContent(&content)) {
session_options->media_description_options.push_back(
MediaDescriptionOptions(MediaType::UNSUPPORTED, content.mid(),
diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h
index 85bc00a..3c064e9 100644
--- a/pc/sdp_offer_answer.h
+++ b/pc/sdp_offer_answer.h
@@ -80,6 +80,7 @@
// Creates an SdpOfferAnswerHandler. Modifies dependencies.
static std::unique_ptr<SdpOfferAnswerHandler> Create(
+ const Environment& env,
PeerConnectionSdpMethods* pc,
const PeerConnectionInterface::RTCConfiguration& configuration,
std::unique_ptr<RTCCertificateGeneratorInterface> cert_generator,
@@ -213,7 +214,8 @@
class LocalIceCredentialsToReplace;
// Only called by the Create() function.
- explicit SdpOfferAnswerHandler(PeerConnectionSdpMethods* pc,
+ explicit SdpOfferAnswerHandler(const Environment& env,
+ PeerConnectionSdpMethods* pc,
ConnectionContext* context);
// Called from the `Create()` function. Can only be called
// once. Modifies dependencies.
@@ -586,6 +588,7 @@
const VideoOptions& video_options() { return video_options_; }
bool ConfiguredForMedia() const;
+ const Environment& env_;
PeerConnectionSdpMethods* const pc_;
ConnectionContext* const context_;
diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h
index 2e5ffda..c93f050 100644
--- a/pc/test/integration_test_helpers.h
+++ b/pc/test/integration_test_helpers.h
@@ -1478,12 +1478,13 @@
std::make_unique<FakeRTCCertificateGenerator>();
}
std::string field_trials = field_trials_;
+ EnvironmentFactory env = EnvironmentFactory(env_);
+
auto it = field_trials_overrides_.find(debug_name);
if (it != field_trials_overrides_.end()) {
field_trials = it->second;
+ dependencies.trials = std::make_unique<FieldTrials>(it->second);
}
- // Override the field trials in the environment.
- EnvironmentFactory env = EnvironmentFactory(env_);
env.Set(CreateTestFieldTrialsPtr(field_trials));
std::unique_ptr<PeerConnectionIntegrationWrapper> client(
