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}
This commit is contained in:
parent
50da1d329a
commit
aac2dea765
@ -417,6 +417,7 @@ class RemoteMediaStreamFactory {
|
||||
|
||||
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 @@ bool ExtractMediaSessionOptions(
|
||||
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 @@ bool PeerConnection::GetOptionsForOffer(
|
||||
cricket::TransportOptions();
|
||||
}
|
||||
}
|
||||
if (!ExtractMediaSessionOptions(rtc_options, session_options)) {
|
||||
if (!ExtractMediaSessionOptions(rtc_options, true, session_options)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -1568,7 +1578,7 @@ bool PeerConnection::GetOptionsForAnswer(
|
||||
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);
|
||||
|
||||
@ -35,6 +35,7 @@ class VideoRtpReceiver;
|
||||
// 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
|
||||
|
||||
@ -155,9 +155,11 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
}
|
||||
|
||||
void IceRestart() {
|
||||
session_description_constraints_.SetMandatoryIceRestart(true);
|
||||
offer_answer_constraints_.SetMandatoryIceRestart(true);
|
||||
offer_answer_options_.ice_restart = true;
|
||||
SetExpectIceRestart(true);
|
||||
}
|
||||
|
||||
@ -356,13 +370,15 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
|
||||
// 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 @@ class P2PTestConductor : public testing::Test {
|
||||
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 @@ class P2PTestConductor : public testing::Test {
|
||||
// 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 @@ class P2PTestConductor : public testing::Test {
|
||||
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 @@ TEST_F(P2PTestConductor, OneWayMediaCall) {
|
||||
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) {
|
||||
|
||||
@ -2454,11 +2454,11 @@ TEST(CreateSessionOptionsTest, GetOptionsForOfferWithInvalidAudioOption) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetOptionsForOfferWithInvalidVideoOption) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudioVideo) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudio) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetDefaultMediaSessionOptionsForOffer) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithVideo) {
|
||||
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 @@ TEST(CreateSessionOptionsTest,
|
||||
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 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithIceRestart) {
|
||||
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 @@ TEST(CreateSessionOptionsTest, MediaConstraintsInAnswer) {
|
||||
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());
|
||||
|
||||
@ -518,7 +518,7 @@ class WebRtcSessionTest
|
||||
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 ==
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user