Changed defaults for CreateAnswer in non-constraint mode
This CL also adds control flag in webrtcsession_unittests
that says whether to prefer constraints APIs or non-constraints APIs, and uses it in the test that was needed
to uncover the bug.
BUG=webrtc:4906
Review URL: https://codereview.webrtc.org/1775033002
Cr-Commit-Position: refs/heads/master@{#11947}
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 700714a..e4ab40e 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -417,6 +417,7 @@
bool ExtractMediaSessionOptions(
const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
+ bool is_offer,
cricket::MediaSessionOptions* session_options) {
typedef PeerConnectionInterface::RTCOfferAnswerOptions RTCOfferAnswerOptions;
if (!IsValidOfferToReceiveMedia(rtc_options.offer_to_receive_audio) ||
@@ -424,11 +425,20 @@
return false;
}
+ // If constraints don't prevent us, we always accept video.
if (rtc_options.offer_to_receive_audio != RTCOfferAnswerOptions::kUndefined) {
session_options->recv_audio = (rtc_options.offer_to_receive_audio > 0);
+ } else {
+ session_options->recv_audio = true;
}
+ // For offers, we only offer video if we have it or it's forced by options.
+ // For answers, we will always accept video (if offered).
if (rtc_options.offer_to_receive_video != RTCOfferAnswerOptions::kUndefined) {
session_options->recv_video = (rtc_options.offer_to_receive_video > 0);
+ } else if (is_offer) {
+ session_options->recv_video = false;
+ } else {
+ session_options->recv_video = true;
}
session_options->vad_enabled = rtc_options.voice_activity_detection;
@@ -1497,7 +1507,7 @@
cricket::TransportOptions();
}
}
- if (!ExtractMediaSessionOptions(rtc_options, session_options)) {
+ if (!ExtractMediaSessionOptions(rtc_options, true, session_options)) {
return false;
}
@@ -1568,7 +1578,7 @@
cricket::MediaSessionOptions* session_options) {
session_options->recv_audio = false;
session_options->recv_video = false;
- if (!ExtractMediaSessionOptions(options, session_options)) {
+ if (!ExtractMediaSessionOptions(options, false, session_options)) {
return false;
}
FinishOptionsForAnswer(session_options);
diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h
index 2e9d9b2..3b0d558 100644
--- a/webrtc/api/peerconnection.h
+++ b/webrtc/api/peerconnection.h
@@ -35,6 +35,7 @@
// them to be populated from |rtc_options|.
bool ExtractMediaSessionOptions(
const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
+ bool is_offer,
cricket::MediaSessionOptions* session_options);
// Populates |session_options| from |constraints|, and returns true if all
diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc
index 4a544c4..39bf608 100644
--- a/webrtc/api/peerconnection_unittest.cc
+++ b/webrtc/api/peerconnection_unittest.cc
@@ -155,9 +155,11 @@
const std::string& id,
const MediaConstraintsInterface* constraints,
const PeerConnectionFactory::Options* options,
- rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store) {
+ rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store,
+ bool prefer_constraint_apis) {
PeerConnectionTestClient* client(new PeerConnectionTestClient(id));
- if (!client->Init(constraints, options, std::move(dtls_identity_store))) {
+ if (!client->Init(constraints, options, std::move(dtls_identity_store),
+ prefer_constraint_apis)) {
delete client;
return nullptr;
}
@@ -172,8 +174,19 @@
rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore()
: nullptr);
- return CreateClientWithDtlsIdentityStore(id, constraints, options,
- std::move(dtls_identity_store));
+ return CreateClientWithDtlsIdentityStore(
+ id, constraints, options, std::move(dtls_identity_store), true);
+ }
+
+ static PeerConnectionTestClient* CreateClientPreferNoConstraints(
+ const std::string& id,
+ const PeerConnectionFactory::Options* options) {
+ rtc::scoped_ptr<FakeDtlsIdentityStore> dtls_identity_store(
+ rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore()
+ : nullptr);
+
+ return CreateClientWithDtlsIdentityStore(
+ id, nullptr, options, std::move(dtls_identity_store), false);
}
~PeerConnectionTestClient() {
@@ -336,7 +349,8 @@
}
void IceRestart() {
- session_description_constraints_.SetMandatoryIceRestart(true);
+ offer_answer_constraints_.SetMandatoryIceRestart(true);
+ offer_answer_options_.ice_restart = true;
SetExpectIceRestart(true);
}
@@ -356,13 +370,15 @@
void SetReceiveAudio(bool audio) {
if (audio && can_receive_audio())
return;
- session_description_constraints_.SetMandatoryReceiveAudio(audio);
+ offer_answer_constraints_.SetMandatoryReceiveAudio(audio);
+ offer_answer_options_.offer_to_receive_audio = audio ? 1 : 0;
}
void SetReceiveVideo(bool video) {
if (video && can_receive_video())
return;
- session_description_constraints_.SetMandatoryReceiveVideo(video);
+ offer_answer_constraints_.SetMandatoryReceiveVideo(video);
+ offer_answer_options_.offer_to_receive_video = video ? 1 : 0;
}
void RemoveMsidFromReceivedSdp(bool remove) { remove_msid_ = remove; }
@@ -373,22 +389,34 @@
bool can_receive_audio() {
bool value;
- if (webrtc::FindConstraint(&session_description_constraints_,
- MediaConstraintsInterface::kOfferToReceiveAudio,
- &value, nullptr)) {
- return value;
+ if (prefer_constraint_apis_) {
+ if (webrtc::FindConstraint(
+ &offer_answer_constraints_,
+ MediaConstraintsInterface::kOfferToReceiveAudio, &value,
+ nullptr)) {
+ return value;
+ }
+ return true;
}
- return true;
+ return offer_answer_options_.offer_to_receive_audio > 0 ||
+ offer_answer_options_.offer_to_receive_audio ==
+ PeerConnectionInterface::RTCOfferAnswerOptions::kUndefined;
}
bool can_receive_video() {
bool value;
- if (webrtc::FindConstraint(&session_description_constraints_,
- MediaConstraintsInterface::kOfferToReceiveVideo,
- &value, nullptr)) {
- return value;
+ if (prefer_constraint_apis_) {
+ if (webrtc::FindConstraint(
+ &offer_answer_constraints_,
+ MediaConstraintsInterface::kOfferToReceiveVideo, &value,
+ nullptr)) {
+ return value;
+ }
+ return true;
}
- return true;
+ return offer_answer_options_.offer_to_receive_video > 0 ||
+ offer_answer_options_.offer_to_receive_video ==
+ PeerConnectionInterface::RTCOfferAnswerOptions::kUndefined;
}
void OnDataChannel(DataChannelInterface* data_channel) override {
@@ -745,9 +773,15 @@
bool Init(
const MediaConstraintsInterface* constraints,
const PeerConnectionFactory::Options* options,
- rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store) {
+ rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store,
+ bool prefer_constraint_apis) {
EXPECT_TRUE(!peer_connection_);
EXPECT_TRUE(!peer_connection_factory_);
+ if (!prefer_constraint_apis) {
+ EXPECT_TRUE(!constraints);
+ }
+ prefer_constraint_apis_ = prefer_constraint_apis;
+
rtc::scoped_ptr<cricket::PortAllocator> port_allocator(
new cricket::FakePortAllocator(rtc::Thread::Current(), nullptr));
fake_audio_capture_module_ = FakeAudioCaptureModule::Create();
@@ -819,10 +853,18 @@
rtc::scoped_refptr<MockCreateSessionDescriptionObserver>
observer(new rtc::RefCountedObject<
MockCreateSessionDescriptionObserver>());
- if (offer) {
- pc()->CreateOffer(observer, &session_description_constraints_);
+ if (prefer_constraint_apis_) {
+ if (offer) {
+ pc()->CreateOffer(observer, &offer_answer_constraints_);
+ } else {
+ pc()->CreateAnswer(observer, &offer_answer_constraints_);
+ }
} else {
- pc()->CreateAnswer(observer, &session_description_constraints_);
+ if (offer) {
+ pc()->CreateOffer(observer, offer_answer_options_);
+ } else {
+ pc()->CreateAnswer(observer, offer_answer_options_);
+ }
}
EXPECT_EQ_WAIT(true, observer->called(), kMaxWaitMs);
*desc = observer->release_desc();
@@ -895,6 +937,7 @@
rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface>
peer_connection_factory_;
+ bool prefer_constraint_apis_ = true;
bool auto_add_stream_ = true;
typedef std::pair<std::string, std::string> IceUfragPwdPair;
@@ -923,7 +966,8 @@
// them, if required.
std::vector<cricket::VideoCapturer*> video_capturers_;
- webrtc::FakeConstraints session_description_constraints_;
+ webrtc::FakeConstraints offer_answer_constraints_;
+ PeerConnectionInterface::RTCOfferAnswerOptions offer_answer_options_;
bool remove_msid_ = false; // True if MSID should be removed in received SDP.
bool remove_bundle_ =
false; // True if bundle should be removed in received SDP.
@@ -1033,6 +1077,22 @@
nullptr);
}
+ bool CreateTestClientsThatPreferNoConstraints() {
+ initiating_client_.reset(
+ PeerConnectionTestClient::CreateClientPreferNoConstraints("Caller: ",
+ nullptr));
+ receiving_client_.reset(
+ PeerConnectionTestClient::CreateClientPreferNoConstraints("Callee: ",
+ nullptr));
+ if (!initiating_client_ || !receiving_client_) {
+ return false;
+ }
+ // Remember the choice for possible later resets of the clients.
+ prefer_constraint_apis_ = false;
+ SetSignalingReceivers();
+ return true;
+ }
+
void SetSignalingReceivers() {
initiating_client_->set_signaling_message_receiver(receiving_client_.get());
receiving_client_->set_signaling_message_receiver(initiating_client_.get());
@@ -1141,7 +1201,7 @@
// Make sure the new client is using a different certificate.
return PeerConnectionTestClient::CreateClientWithDtlsIdentityStore(
"New Peer: ", &setup_constraints, nullptr,
- std::move(dtls_identity_store));
+ std::move(dtls_identity_store), prefer_constraint_apis_);
}
void SendRtpData(webrtc::DataChannelInterface* dc, const std::string& data) {
@@ -1186,6 +1246,7 @@
rtc::SocketServerScope ss_scope_;
rtc::scoped_ptr<PeerConnectionTestClient> initiating_client_;
rtc::scoped_ptr<PeerConnectionTestClient> receiving_client_;
+ bool prefer_constraint_apis_ = true;
};
// Disable for TSan v2, see
@@ -1252,6 +1313,12 @@
LocalP2PTest();
}
+TEST_F(P2PTestConductor, OneWayMediaCallWithoutConstraints) {
+ ASSERT_TRUE(CreateTestClientsThatPreferNoConstraints());
+ receiving_client()->set_auto_add_stream(false);
+ LocalP2PTest();
+}
+
// This test sets up a audio call initially and then upgrades to audio/video,
// using DTLS.
TEST_F(P2PTestConductor, LocalP2PTestDtlsRenegotiate) {
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 0448526..d8d05f6 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -2454,11 +2454,11 @@
rtc_options.offer_to_receive_audio = RTCOfferAnswerOptions::kUndefined - 1;
cricket::MediaSessionOptions options;
- EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, true, &options));
rtc_options.offer_to_receive_audio =
RTCOfferAnswerOptions::kMaxOfferToReceiveMedia + 1;
- EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, true, &options));
}
TEST(CreateSessionOptionsTest, GetOptionsForOfferWithInvalidVideoOption) {
@@ -2466,11 +2466,11 @@
rtc_options.offer_to_receive_video = RTCOfferAnswerOptions::kUndefined - 1;
cricket::MediaSessionOptions options;
- EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, true, &options));
rtc_options.offer_to_receive_video =
RTCOfferAnswerOptions::kMaxOfferToReceiveMedia + 1;
- EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_FALSE(ExtractMediaSessionOptions(rtc_options, true, &options));
}
// Test that a MediaSessionOptions is created for an offer if
@@ -2481,7 +2481,7 @@
rtc_options.offer_to_receive_video = 1;
cricket::MediaSessionOptions options;
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_TRUE(options.has_audio());
EXPECT_TRUE(options.has_video());
EXPECT_TRUE(options.bundle_enabled);
@@ -2494,7 +2494,7 @@
rtc_options.offer_to_receive_audio = 1;
cricket::MediaSessionOptions options;
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_TRUE(options.has_audio());
EXPECT_FALSE(options.has_video());
EXPECT_TRUE(options.bundle_enabled);
@@ -2508,7 +2508,7 @@
cricket::MediaSessionOptions options;
options.transport_options["audio"] = cricket::TransportOptions();
options.transport_options["video"] = cricket::TransportOptions();
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_TRUE(options.has_audio());
EXPECT_FALSE(options.has_video());
EXPECT_TRUE(options.bundle_enabled);
@@ -2525,7 +2525,7 @@
rtc_options.offer_to_receive_video = 1;
cricket::MediaSessionOptions options;
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_FALSE(options.has_audio());
EXPECT_TRUE(options.has_video());
EXPECT_TRUE(options.bundle_enabled);
@@ -2541,7 +2541,7 @@
rtc_options.use_rtp_mux = false;
cricket::MediaSessionOptions options;
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_TRUE(options.has_audio());
EXPECT_TRUE(options.has_video());
EXPECT_FALSE(options.bundle_enabled);
@@ -2557,12 +2557,12 @@
cricket::MediaSessionOptions options;
options.transport_options["audio"] = cricket::TransportOptions();
options.transport_options["video"] = cricket::TransportOptions();
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_TRUE(options.transport_options["audio"].ice_restart);
EXPECT_TRUE(options.transport_options["video"].ice_restart);
rtc_options = RTCOfferAnswerOptions();
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, &options));
+ EXPECT_TRUE(ExtractMediaSessionOptions(rtc_options, true, &options));
EXPECT_FALSE(options.transport_options["audio"].ice_restart);
EXPECT_FALSE(options.transport_options["video"].ice_restart);
}
@@ -2584,16 +2584,17 @@
RTCOfferAnswerOptions rtc_offer_options;
cricket::MediaSessionOptions offer_options;
- EXPECT_TRUE(ExtractMediaSessionOptions(rtc_offer_options, &offer_options));
+ EXPECT_TRUE(
+ ExtractMediaSessionOptions(rtc_offer_options, false, &offer_options));
EXPECT_TRUE(offer_options.has_audio());
- EXPECT_FALSE(offer_options.has_video());
+ EXPECT_TRUE(offer_options.has_video());
RTCOfferAnswerOptions updated_rtc_offer_options;
updated_rtc_offer_options.offer_to_receive_audio = 1;
updated_rtc_offer_options.offer_to_receive_video = 1;
cricket::MediaSessionOptions updated_offer_options;
- EXPECT_TRUE(ExtractMediaSessionOptions(updated_rtc_offer_options,
+ EXPECT_TRUE(ExtractMediaSessionOptions(updated_rtc_offer_options, false,
&updated_offer_options));
EXPECT_TRUE(updated_offer_options.has_audio());
EXPECT_TRUE(updated_offer_options.has_video());
diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc
index 87ff927..2c9a99b 100644
--- a/webrtc/api/webrtcsession_unittest.cc
+++ b/webrtc/api/webrtcsession_unittest.cc
@@ -518,7 +518,7 @@
void GetOptionsForOffer(
const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
cricket::MediaSessionOptions* session_options) {
- ASSERT_TRUE(ExtractMediaSessionOptions(rtc_options, session_options));
+ ASSERT_TRUE(ExtractMediaSessionOptions(rtc_options, true, session_options));
AddStreamsToOptions(session_options);
if (rtc_options.offer_to_receive_audio ==
