From 3828c06a588fa31cacd2a08173713f9afece691e Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Wed, 6 Dec 2017 10:34:51 -0800 Subject: [PATCH] Replace cricket::ContentAction with webrtc::SdpType Bug: webrtc:8613 Change-Id: I9bce2b9d8c8445d2fa1b9f60b06596a5621ebc2f Reviewed-on: https://webrtc-review.googlesource.com/29460 Commit-Queue: Steve Anton Reviewed-by: Peter Thatcher Cr-Commit-Position: refs/heads/master@{#21130} --- ortc/rtptransportcontrolleradapter.cc | 24 ++-- p2p/base/dtlstransport_unittest.cc | 69 +++++------ p2p/base/jseptransport.cc | 22 ++-- p2p/base/jseptransport.h | 9 +- p2p/base/jseptransport_unittest.cc | 147 ++++++++++++----------- p2p/base/sessiondescription.h | 4 - pc/channel.cc | 117 +++++++++--------- pc/channel.h | 41 ++++--- pc/channel_unittest.cc | 166 +++++++++++++------------- pc/peerconnection.cc | 86 ++++++------- pc/peerconnection.h | 19 +-- pc/test/faketransportcontroller.h | 8 +- pc/transportcontroller.cc | 20 ++-- pc/transportcontroller.h | 8 +- pc/transportcontroller_unittest.cc | 60 +++++----- 15 files changed, 399 insertions(+), 401 deletions(-) diff --git a/ortc/rtptransportcontrolleradapter.cc b/ortc/rtptransportcontrolleradapter.cc index 4e1ac1b0ed..9d07be651b 100644 --- a/ortc/rtptransportcontrolleradapter.cc +++ b/ortc/rtptransportcontrolleradapter.cc @@ -247,11 +247,11 @@ RTCError RtpTransportControllerAdapter::SetRtpTransportParameters( &local_audio_description_, &remote_audio_description_); if (!voice_channel_->SetLocalContent(&local_audio_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { break; } if (!voice_channel_->SetRemoteContent(&remote_audio_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { break; } } else if (inner_transport == inner_video_transport_) { @@ -259,11 +259,11 @@ RTCError RtpTransportControllerAdapter::SetRtpTransportParameters( &local_video_description_, &remote_video_description_); if (!video_channel_->SetLocalContent(&local_video_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { break; } if (!video_channel_->SetRemoteContent(&remote_video_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { break; } } @@ -354,12 +354,12 @@ RTCError RtpTransportControllerAdapter::ValidateAndApplyAudioSenderParameters( // Set remote content first, to ensure the stream is created with the correct // codec. if (!voice_channel_->SetRemoteContent(&remote_audio_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply remote parameters to media channel."); } if (!voice_channel_->SetLocalContent(&local_audio_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply local parameters to media channel."); } @@ -442,12 +442,12 @@ RTCError RtpTransportControllerAdapter::ValidateAndApplyVideoSenderParameters( // Set remote content first, to ensure the stream is created with the correct // codec. if (!video_channel_->SetRemoteContent(&remote_video_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply remote parameters to media channel."); } if (!video_channel_->SetLocalContent(&local_video_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply local parameters to media channel."); } @@ -516,12 +516,12 @@ RTCError RtpTransportControllerAdapter::ValidateAndApplyAudioReceiverParameters( RtpTransceiverDirectionReversed(local_direction)); if (!voice_channel_->SetLocalContent(&local_audio_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply local parameters to media channel."); } if (!voice_channel_->SetRemoteContent(&remote_audio_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply remote parameters to media channel."); } @@ -592,12 +592,12 @@ RTCError RtpTransportControllerAdapter::ValidateAndApplyVideoReceiverParameters( RtpTransceiverDirectionReversed(local_direction)); if (!video_channel_->SetLocalContent(&local_video_description_, - cricket::CA_OFFER, nullptr)) { + SdpType::kOffer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply local parameters to media channel."); } if (!video_channel_->SetRemoteContent(&remote_video_description_, - cricket::CA_ANSWER, nullptr)) { + SdpType::kAnswer, nullptr)) { LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, "Failed to apply remote parameters to media channel."); } diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc index ed010e02c4..27e12a4d2a 100644 --- a/p2p/base/dtlstransport_unittest.cc +++ b/p2p/base/dtlstransport_unittest.cc @@ -61,6 +61,7 @@ cricket::TransportDescription MakeTransportDescription( } using cricket::ConnectionRole; +using webrtc::SdpType; enum Flags { NF_REOFFER = 0x1, NF_EXPECT_FAILURE = 0x2 }; @@ -136,7 +137,7 @@ class DtlsTestClient : public sigslot::has_slots<> { // Offer DTLS if we have an identity; pass in a remote fingerprint only if // both sides support DTLS. void Negotiate(DtlsTestClient* peer, - cricket::ContentAction action, + SdpType action, ConnectionRole local_role, ConnectionRole remote_role, int flags) { @@ -146,13 +147,13 @@ class DtlsTestClient : public sigslot::has_slots<> { void SetLocalTransportDescription( const rtc::scoped_refptr& cert, - cricket::ContentAction action, + SdpType action, ConnectionRole role, int flags) { // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when // content action is CA_ANSWER. bool expect_success = - !((action == cricket::CA_ANSWER) && (flags & NF_EXPECT_FAILURE)); + !((action == SdpType::kAnswer) && (flags & NF_EXPECT_FAILURE)); EXPECT_EQ(expect_success, transport_->SetLocalTransportDescription( MakeTransportDescription(cert, role), action, nullptr)); @@ -160,13 +161,13 @@ class DtlsTestClient : public sigslot::has_slots<> { void SetRemoteTransportDescription( const rtc::scoped_refptr& cert, - cricket::ContentAction action, + SdpType action, ConnectionRole role, int flags) { // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when // content action is CA_ANSWER. bool expect_success = - !((action == cricket::CA_ANSWER) && (flags & NF_EXPECT_FAILURE)); + !((action == SdpType::kAnswer) && (flags & NF_EXPECT_FAILURE)); EXPECT_EQ(expect_success, transport_->SetRemoteTransportDescription( MakeTransportDescription(cert, role), action, nullptr)); @@ -175,22 +176,22 @@ class DtlsTestClient : public sigslot::has_slots<> { // Allow any DTLS configuration to be specified (including invalid ones). void Negotiate(const rtc::scoped_refptr& local_cert, const rtc::scoped_refptr& remote_cert, - cricket::ContentAction action, + SdpType action, ConnectionRole local_role, ConnectionRole remote_role, int flags) { - if (action == cricket::CA_OFFER) { - SetLocalTransportDescription(local_cert, cricket::CA_OFFER, local_role, + if (action == SdpType::kOffer) { + SetLocalTransportDescription(local_cert, SdpType::kOffer, local_role, flags); - SetRemoteTransportDescription(remote_cert, cricket::CA_ANSWER, - remote_role, flags); + SetRemoteTransportDescription(remote_cert, SdpType::kAnswer, remote_role, + flags); } else { - SetRemoteTransportDescription(remote_cert, cricket::CA_OFFER, remote_role, + SetRemoteTransportDescription(remote_cert, SdpType::kOffer, remote_role, flags); // If remote if the offerer and has no DTLS support, answer will be // without any fingerprint. SetLocalTransportDescription(remote_cert ? local_cert : nullptr, - cricket::CA_ANSWER, local_role, flags); + SdpType::kAnswer, local_role, flags); } } @@ -475,14 +476,14 @@ class DtlsTransportChannelTestBase { // answer not yet being received on the initiating side. So the // connection will be made before negotiation has finished on both sides. client1_.SetLocalTransportDescription(client1_.certificate(), - cricket::CA_OFFER, client1_role, 0); - client2_.SetRemoteTransportDescription( - client1_.certificate(), cricket::CA_OFFER, client1_role, 0); - client2_.SetLocalTransportDescription( - client2_.certificate(), cricket::CA_ANSWER, client2_role, 0); + SdpType::kOffer, client1_role, 0); + client2_.SetRemoteTransportDescription(client1_.certificate(), + SdpType::kOffer, client1_role, 0); + client2_.SetLocalTransportDescription(client2_.certificate(), + SdpType::kAnswer, client2_role, 0); rv = client1_.Connect(&client2_, false); - client1_.SetRemoteTransportDescription( - client2_.certificate(), cricket::CA_ANSWER, client2_role, 0); + client1_.SetRemoteTransportDescription(client2_.certificate(), + SdpType::kAnswer, client2_role, 0); } EXPECT_TRUE(rv); @@ -548,10 +549,10 @@ class DtlsTransportChannelTestBase { client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); // Expect success from SLTD and SRTD. - client1_.Negotiate(&client2_, cricket::CA_OFFER, client1_role, client2_role, + client1_.Negotiate(&client2_, SdpType::kOffer, client1_role, client2_role, + 0); + client2_.Negotiate(&client1_, SdpType::kAnswer, client2_role, client1_role, 0); - client2_.Negotiate(&client1_, cricket::CA_ANSWER, client2_role, - client1_role, 0); } // Negotiate with legacy client |client2|. Legacy client doesn't use setup @@ -560,10 +561,10 @@ class DtlsTransportChannelTestBase { client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); // Expect success from SLTD and SRTD. - client1_.Negotiate(&client2_, cricket::CA_OFFER, + client1_.Negotiate(&client2_, SdpType::kOffer, cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_NONE, 0); - client2_.Negotiate(&client1_, cricket::CA_ANSWER, + client2_.Negotiate(&client1_, SdpType::kAnswer, cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_NONE, 0); } @@ -573,14 +574,14 @@ class DtlsTransportChannelTestBase { ConnectionRole client2_role, int flags) { if (reoffer_initiator == &client1_) { - client1_.Negotiate(&client2_, cricket::CA_OFFER, client1_role, - client2_role, flags); - client2_.Negotiate(&client1_, cricket::CA_ANSWER, client2_role, + client1_.Negotiate(&client2_, SdpType::kOffer, client1_role, client2_role, + flags); + client2_.Negotiate(&client1_, SdpType::kAnswer, client2_role, client1_role, flags); } else { - client2_.Negotiate(&client1_, cricket::CA_OFFER, client2_role, - client1_role, flags); - client1_.Negotiate(&client2_, cricket::CA_ANSWER, client1_role, + client2_.Negotiate(&client1_, SdpType::kOffer, client2_role, client1_role, + flags); + client1_.Negotiate(&client2_, SdpType::kAnswer, client1_role, client2_role, flags); } } @@ -1019,9 +1020,9 @@ class DtlsEventOrderingTest client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED, simulated_delay_ms); client1_.SetLocalTransportDescription(client1_.certificate(), - cricket::CA_OFFER, + SdpType::kOffer, cricket::CONNECTIONROLE_ACTPASS, 0); - client2_.Negotiate(&client1_, cricket::CA_ANSWER, + client2_.Negotiate(&client1_, SdpType::kAnswer, cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS, 0); @@ -1030,7 +1031,7 @@ class DtlsEventOrderingTest case CALLER_RECEIVES_FINGERPRINT: if (valid_fingerprint) { client1_.SetRemoteTransportDescription( - client2_.certificate(), cricket::CA_ANSWER, + client2_.certificate(), SdpType::kAnswer, cricket::CONNECTIONROLE_ACTIVE, 0); } else { // Create a fingerprint with a correct algorithm but an invalid @@ -1043,7 +1044,7 @@ class DtlsEventOrderingTest // it should return true as long as the fingerprint was formatted // correctly. EXPECT_TRUE(client1_.transport()->SetRemoteTransportDescription( - remote_desc, cricket::CA_ANSWER, nullptr)); + remote_desc, SdpType::kAnswer, nullptr)); } break; case CALLER_WRITABLE: diff --git a/p2p/base/jseptransport.cc b/p2p/base/jseptransport.cc index dbbb8752ae..77ea040a72 100644 --- a/p2p/base/jseptransport.cc +++ b/p2p/base/jseptransport.cc @@ -22,6 +22,8 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" +using webrtc::SdpType; + namespace cricket { static bool VerifyIceParams(const TransportDescription& desc) { @@ -232,7 +234,7 @@ bool JsepTransport::GetLocalCertificate( bool JsepTransport::SetLocalTransportDescription( const TransportDescription& description, - ContentAction action, + SdpType type, std::string* error_desc) { bool ret = true; @@ -266,8 +268,8 @@ bool JsepTransport::SetLocalTransportDescription( } // If PRANSWER/ANSWER is set, we should decide transport protocol type. - if (action == CA_PRANSWER || action == CA_ANSWER) { - ret &= NegotiateTransportDescription(action, error_desc); + if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { + ret &= NegotiateTransportDescription(type, error_desc); } if (!ret) { return false; @@ -285,7 +287,7 @@ bool JsepTransport::SetLocalTransportDescription( bool JsepTransport::SetRemoteTransportDescription( const TransportDescription& description, - ContentAction action, + SdpType type, std::string* error_desc) { bool ret = true; @@ -300,8 +302,8 @@ bool JsepTransport::SetRemoteTransportDescription( } // If PRANSWER/ANSWER is set, we should decide transport protocol type. - if (action == CA_PRANSWER || action == CA_ANSWER) { - ret = NegotiateTransportDescription(CA_OFFER, error_desc); + if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { + ret = NegotiateTransportDescription(SdpType::kOffer, error_desc); } if (ret) { remote_description_set_ = true; @@ -407,7 +409,7 @@ bool JsepTransport::ApplyNegotiatedTransportDescription( } bool JsepTransport::NegotiateTransportDescription( - ContentAction local_description_type, + SdpType local_description_type, std::string* error_desc) { if (!local_description_ || !remote_description_) { const std::string msg = @@ -424,7 +426,7 @@ bool JsepTransport::NegotiateTransportDescription( if (!NegotiateRole(local_description_type, error_desc)) { return false; } - } else if (local_fp && (local_description_type == CA_ANSWER)) { + } else if (local_fp && (local_description_type == SdpType::kAnswer)) { return BadTransportDescription( "Local fingerprint supplied when caller didn't offer DTLS.", error_desc); @@ -445,7 +447,7 @@ bool JsepTransport::NegotiateTransportDescription( return true; } -bool JsepTransport::NegotiateRole(ContentAction local_description_type, +bool JsepTransport::NegotiateRole(SdpType local_description_type, std::string* error_desc) { if (!local_description_ || !remote_description_) { const std::string msg = @@ -481,7 +483,7 @@ bool JsepTransport::NegotiateRole(ContentAction local_description_type, ConnectionRole remote_connection_role = remote_description_->connection_role; bool is_remote_server = false; - if (local_description_type == CA_OFFER) { + if (local_description_type == SdpType::kOffer) { if (local_connection_role != CONNECTIONROLE_ACTPASS) { return BadTransportDescription( "Offerer must use actpass value for setup attribute.", error_desc); diff --git a/p2p/base/jseptransport.h b/p2p/base/jseptransport.h index 7277e1b2bc..f045556f08 100644 --- a/p2p/base/jseptransport.h +++ b/p2p/base/jseptransport.h @@ -17,6 +17,7 @@ #include #include "api/candidate.h" +#include "api/jsep.h" #include "api/optional.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/sessiondescription.h" @@ -280,13 +281,13 @@ class JsepTransport : public sigslot::has_slots<> { // Set the local TransportDescription to be used by DTLS and ICE channels // that are part of this Transport. bool SetLocalTransportDescription(const TransportDescription& description, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); // Set the remote TransportDescription to be used by DTLS and ICE channels // that are part of this Transport. bool SetRemoteTransportDescription(const TransportDescription& description, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); // Set the "needs-ice-restart" flag as described in JSEP. After the flag is @@ -336,13 +337,13 @@ class JsepTransport : public sigslot::has_slots<> { // should be activated. // // Called when an answer TransportDescription is applied. - bool NegotiateTransportDescription(ContentAction local_description_type, + bool NegotiateTransportDescription(webrtc::SdpType local_description_type, std::string* error_desc); // Negotiates the SSL role based off the offer and answer as specified by // RFC 4145, section-4.1. Returns false if the SSL role cannot be determined // from the local description and remote description. - bool NegotiateRole(ContentAction local_description_type, + bool NegotiateRole(webrtc::SdpType local_description_type, std::string* error_desc); // Pushes down the transport parameters from the local description, such diff --git a/p2p/base/jseptransport_unittest.cc b/p2p/base/jseptransport_unittest.cc index 8d09e0f9bd..ff8484d54b 100644 --- a/p2p/base/jseptransport_unittest.cc +++ b/p2p/base/jseptransport_unittest.cc @@ -22,6 +22,7 @@ using cricket::FakeIceTransport; using cricket::IceRole; using cricket::TransportDescription; using rtc::SocketAddress; +using webrtc::SdpType; static const char kIceUfrag1[] = "TESTICEUFRAG0001"; static const char kIcePwd1[] = "TESTICEPWD00000000000001"; @@ -56,8 +57,8 @@ class JsepTransportTest : public testing::Test, public sigslot::has_slots<> { // ufrag/password after a transport description is applied. TEST_F(JsepTransportTest, TestChannelIceParameters) { cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); - ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_OFFER, NULL)); + ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, + SdpType::kOffer, NULL)); EXPECT_TRUE(SetupChannel()); EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode()); EXPECT_EQ(kIceUfrag1, fake_ice_transport_->ice_ufrag()); @@ -65,7 +66,7 @@ TEST_F(JsepTransportTest, TestChannelIceParameters) { cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_ANSWER, NULL)); + remote_desc, SdpType::kAnswer, NULL)); EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode()); EXPECT_EQ(kIceUfrag1, fake_ice_transport_->remote_ice_ufrag()); EXPECT_EQ(kIcePwd1, fake_ice_transport_->remote_ice_pwd()); @@ -87,9 +88,9 @@ TEST_F(JsepTransportTest, NeedsIceRestart) { cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_OFFER, nullptr)); + local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_ANSWER, nullptr)); + remote_desc, SdpType::kAnswer, nullptr)); // Flag initially should be false. EXPECT_FALSE(transport_->NeedsIceRestart()); @@ -100,18 +101,18 @@ TEST_F(JsepTransportTest, NeedsIceRestart) { // Doing an identical offer/answer shouldn't clear the flag. ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_OFFER, nullptr)); + local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_ANSWER, nullptr)); + remote_desc, SdpType::kAnswer, nullptr)); EXPECT_TRUE(transport_->NeedsIceRestart()); // Doing an offer/answer that restarts ICE should clear the flag. cricket::TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2); cricket::TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetLocalTransportDescription( - ice_restart_local_desc, cricket::CA_OFFER, nullptr)); + ice_restart_local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetRemoteTransportDescription( - ice_restart_remote_desc, cricket::CA_ANSWER, nullptr)); + ice_restart_remote_desc, SdpType::kAnswer, nullptr)); EXPECT_FALSE(transport_->NeedsIceRestart()); } @@ -128,8 +129,8 @@ TEST_F(JsepTransportTest, TestGetStats) { rtc::CreateRandomString(cricket::ICE_UFRAG_LENGTH), rtc::CreateRandomString(cricket::ICE_PWD_LENGTH), cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, nullptr); - transport_->SetLocalTransportDescription(faketransport_desc, - cricket::CA_OFFER, nullptr); + transport_->SetLocalTransportDescription(faketransport_desc, SdpType::kOffer, + nullptr); EXPECT_TRUE(transport_->GetStats(&stats)); ASSERT_EQ(1U, stats.channel_stats.size()); EXPECT_EQ(1, stats.channel_stats[0].component); @@ -191,8 +192,8 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { struct NegotiateRoleParams { cricket::ConnectionRole local_role; cricket::ConnectionRole remote_role; - cricket::ContentAction local_action; - cricket::ContentAction remote_action; + SdpType local_type; + SdpType remote_type; }; std::string error_desc; @@ -200,13 +201,13 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Parameters which set the SSL role to SSL_CLIENT. NegotiateRoleParams valid_client_params[] = { {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}}; + SdpType::kOffer, SdpType::kPrAnswer}}; for (auto& param : valid_client_params) { RecreateTransport(); @@ -216,16 +217,16 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { remote_desc.connection_role = param.remote_role; // Set the offer first. - if (param.local_action == cricket::CA_OFFER) { + if (param.local_type == SdpType::kOffer) { EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); } else { EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); } EXPECT_EQ(rtc::SSL_CLIENT, *transport_->GetSslRole()); } @@ -233,13 +234,13 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Parameters which set the SSL role to SSL_SERVER. NegotiateRoleParams valid_server_params[] = { {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}}; + SdpType::kOffer, SdpType::kPrAnswer}}; for (auto& param : valid_server_params) { RecreateTransport(); @@ -249,16 +250,16 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { remote_desc.connection_role = param.remote_role; // Set the offer first. - if (param.local_action == cricket::CA_OFFER) { + if (param.local_type == SdpType::kOffer) { EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); } else { EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); } EXPECT_EQ(rtc::SSL_SERVER, *transport_->GetSslRole()); } @@ -266,29 +267,29 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Invalid parameters due to both peers having a duplicate role. NegotiateRoleParams duplicate_params[] = { {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}, + SdpType::kOffer, SdpType::kPrAnswer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_OFFER, cricket::CA_PRANSWER}, + SdpType::kOffer, SdpType::kPrAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}}; + SdpType::kOffer, SdpType::kPrAnswer}}; for (auto& param : duplicate_params) { RecreateTransport(); @@ -298,45 +299,45 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { remote_desc.connection_role = param.remote_role; // Set the offer first. - if (param.local_action == cricket::CA_OFFER) { + if (param.local_type == SdpType::kOffer) { EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); EXPECT_FALSE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); } else { EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); EXPECT_FALSE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); } } // Invalid parameters due to the offerer not using ACTPASS. NegotiateRoleParams offerer_without_actpass_params[] = { {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_ANSWER, cricket::CA_OFFER}, + SdpType::kAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_PRANSWER, cricket::CA_OFFER}, + SdpType::kPrAnswer, SdpType::kOffer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_OFFER, cricket::CA_ANSWER}, + SdpType::kOffer, SdpType::kAnswer}, {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}, + SdpType::kOffer, SdpType::kPrAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - cricket::CA_OFFER, cricket::CA_PRANSWER}, + SdpType::kOffer, SdpType::kPrAnswer}, {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - cricket::CA_OFFER, cricket::CA_PRANSWER}}; + SdpType::kOffer, SdpType::kPrAnswer}}; for (auto& param : offerer_without_actpass_params) { RecreateTransport(); @@ -348,16 +349,16 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Set the offer first. // TODO(deadbeef): Really this should fail as soon as the offer is // attempted to be applied, and not when the answer is applied. - if (param.local_action == cricket::CA_OFFER) { + if (param.local_type == SdpType::kOffer) { EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); EXPECT_FALSE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); } else { EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); + remote_desc, param.remote_type, nullptr)); EXPECT_FALSE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); + local_desc, param.local_type, nullptr)); } } } @@ -388,9 +389,9 @@ TEST_F(JsepTransportTest, RemoteOfferWithCurrentNegotiatedDtlsRole) { // Normal initial offer/answer with "actpass" in the offer and "active" in // the answer. ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_OFFER, nullptr)); + remote_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_ANSWER, nullptr)); + local_desc, SdpType::kAnswer, nullptr)); // Sanity check that role was actually negotiated. rtc::Optional role = transport_->GetSslRole(); @@ -400,9 +401,9 @@ TEST_F(JsepTransportTest, RemoteOfferWithCurrentNegotiatedDtlsRole) { // Subsequent offer with current negotiated role of "passive". remote_desc.connection_role = cricket::CONNECTIONROLE_PASSIVE; EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_OFFER, nullptr)); + remote_desc, SdpType::kOffer, nullptr)); EXPECT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_ANSWER, nullptr)); + local_desc, SdpType::kAnswer, nullptr)); } // Test that a remote offer with the inverse of the current negotiated DTLS @@ -430,9 +431,9 @@ TEST_F(JsepTransportTest, RemoteOfferThatChangesNegotiatedDtlsRole) { // Normal initial offer/answer with "actpass" in the offer and "active" in // the answer. ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_OFFER, nullptr)); + remote_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_ANSWER, nullptr)); + local_desc, SdpType::kAnswer, nullptr)); // Sanity check that role was actually negotiated. rtc::Optional role = transport_->GetSslRole(); @@ -445,7 +446,7 @@ TEST_F(JsepTransportTest, RemoteOfferThatChangesNegotiatedDtlsRole) { // attempted to be applied, and not when the answer is applied. remote_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE; EXPECT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_OFFER, nullptr)); + remote_desc, SdpType::kOffer, nullptr)); EXPECT_FALSE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_ANSWER, nullptr)); + local_desc, SdpType::kAnswer, nullptr)); } diff --git a/p2p/base/sessiondescription.h b/p2p/base/sessiondescription.h index e7e6d89346..2de0838783 100644 --- a/p2p/base/sessiondescription.h +++ b/p2p/base/sessiondescription.h @@ -181,10 +181,6 @@ class SessionDescription { bool msid_supported_ = true; }; -// Indicates whether a ContentDescription was an offer or an answer, as -// described in http://www.ietf.org/rfc/rfc3264.txt. -enum ContentAction { CA_OFFER, CA_PRANSWER, CA_ANSWER }; - // Indicates whether a ContentDescription was sent by the local client // or received from the remote client. enum ContentSource { diff --git a/pc/channel.cc b/pc/channel.cc index ad2979cb63..f9b0c33711 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -35,6 +35,7 @@ namespace cricket { using rtc::Bind; +using webrtc::SdpType; namespace { // See comment below for why we need to use a pointer to a unique_ptr. @@ -419,21 +420,21 @@ bool BaseChannel::RemoveSendStream(uint32_t ssrc) { } bool BaseChannel::SetLocalContent(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "BaseChannel::SetLocalContent"); return InvokeOnWorker( RTC_FROM_HERE, - Bind(&BaseChannel::SetLocalContent_w, this, content, action, error_desc)); + Bind(&BaseChannel::SetLocalContent_w, this, content, type, error_desc)); } bool BaseChannel::SetRemoteContent(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "BaseChannel::SetRemoteContent"); return InvokeOnWorker( - RTC_FROM_HERE, Bind(&BaseChannel::SetRemoteContent_w, this, content, - action, error_desc)); + RTC_FROM_HERE, + Bind(&BaseChannel::SetRemoteContent_w, this, content, type, error_desc)); } void BaseChannel::StartConnectionMonitor(int cms) { @@ -744,7 +745,7 @@ void BaseChannel::ChannelNotWritable_n() { bool BaseChannel::SetRtpTransportParameters( const MediaContentDescription* content, - ContentAction action, + SdpType type, ContentSource src, const RtpHeaderExtensions& extensions, std::string* error_desc) { @@ -759,25 +760,25 @@ bool BaseChannel::SetRtpTransportParameters( // Cache srtp_required_ for belt and suspenders check on SendPacket return network_thread_->Invoke( - RTC_FROM_HERE, Bind(&BaseChannel::SetRtpTransportParameters_n, this, - content, action, src, encrypted_extension_ids, - error_desc)); + RTC_FROM_HERE, + Bind(&BaseChannel::SetRtpTransportParameters_n, this, content, type, src, + encrypted_extension_ids, error_desc)); } bool BaseChannel::SetRtpTransportParameters_n( const MediaContentDescription* content, - ContentAction action, + SdpType type, ContentSource src, const std::vector& encrypted_extension_ids, std::string* error_desc) { RTC_DCHECK(network_thread_->IsCurrent()); - if (!SetSrtp_n(content->cryptos(), action, src, encrypted_extension_ids, - error_desc)) { + if (!SetSrtp_n(content->cryptos(), type, src, encrypted_extension_ids, + error_desc)) { return false; } - if (!SetRtcpMux_n(content->rtcp_mux(), action, src, error_desc)) { + if (!SetRtcpMux_n(content->rtcp_mux(), type, src, error_desc)) { return false; } @@ -850,7 +851,7 @@ void BaseChannel::EnableDtlsSrtp_n() { } bool BaseChannel::SetSrtp_n(const std::vector& cryptos, - ContentAction action, + SdpType type, ContentSource src, const std::vector& encrypted_extension_ids, std::string* error_desc) { @@ -868,21 +869,21 @@ bool BaseChannel::SetSrtp_n(const std::vector& cryptos, EnableSdes_n(); } - if ((action == CA_ANSWER || action == CA_PRANSWER) && dtls) { + if ((type == SdpType::kAnswer || type == SdpType::kPrAnswer) && dtls) { EnableDtlsSrtp_n(); } UpdateEncryptedHeaderExtensionIds(src, encrypted_extension_ids); if (!dtls) { - switch (action) { - case CA_OFFER: + switch (type) { + case SdpType::kOffer: ret = sdes_negotiator_.SetOffer(cryptos, src); break; - case CA_PRANSWER: + case SdpType::kPrAnswer: ret = sdes_negotiator_.SetProvisionalAnswer(cryptos, src); break; - case CA_ANSWER: + case SdpType::kAnswer: ret = sdes_negotiator_.SetAnswer(cryptos, src); break; default: @@ -891,7 +892,7 @@ bool BaseChannel::SetSrtp_n(const std::vector& cryptos, // If setting an SDES answer succeeded, apply the negotiated parameters // to the SRTP transport. - if ((action == CA_PRANSWER || action == CA_ANSWER) && ret) { + if ((type == SdpType::kPrAnswer || type == SdpType::kAnswer) && ret) { if (sdes_negotiator_.send_cipher_suite() && sdes_negotiator_.recv_cipher_suite()) { RTC_DCHECK(cached_send_extension_ids_); @@ -907,7 +908,7 @@ bool BaseChannel::SetSrtp_n(const std::vector& cryptos, *(cached_recv_extension_ids_)); } else { RTC_LOG(LS_INFO) << "No crypto keys are provided for SDES."; - if (action == CA_ANSWER && sdes_transport_) { + if (type == SdpType::kAnswer && sdes_transport_) { // Explicitly reset the |sdes_transport_| if no crypto param is // provided in the answer. No need to call |ResetParams()| for // |sdes_negotiator_| because it resets the params inside |SetAnswer|. @@ -925,7 +926,7 @@ bool BaseChannel::SetSrtp_n(const std::vector& cryptos, } bool BaseChannel::SetRtcpMux_n(bool enable, - ContentAction action, + SdpType type, ContentSource src, std::string* error_desc) { // Provide a more specific error message for the RTCP mux "require" policy @@ -938,16 +939,16 @@ bool BaseChannel::SetRtcpMux_n(bool enable, return false; } bool ret = false; - switch (action) { - case CA_OFFER: + switch (type) { + case SdpType::kOffer: ret = rtcp_mux_filter_.SetOffer(enable, src); break; - case CA_PRANSWER: + case SdpType::kPrAnswer: // This may activate RTCP muxing, but we don't yet destroy the transport // because the final answer may deactivate it. ret = rtcp_mux_filter_.SetProvisionalAnswer(enable, src); break; - case CA_ANSWER: + case SdpType::kAnswer: ret = rtcp_mux_filter_.SetAnswer(enable, src); if (ret && rtcp_mux_filter_.IsActive()) { ActivateRtcpMux(); @@ -961,9 +962,9 @@ bool BaseChannel::SetRtcpMux_n(bool enable, return false; } rtp_transport_->SetRtcpMuxEnabled(rtcp_mux_filter_.IsActive()); - // |rtcp_mux_filter_| can be active if |action| is CA_PRANSWER or - // CA_ANSWER, but we only want to tear down the RTCP transport if we received - // a final answer. + // |rtcp_mux_filter_| can be active if |action| is SdpType::kPrAnswer or + // SdpType::kAnswer, but we only want to tear down the RTCP transport if we + // received a final answer. if (rtcp_mux_filter_.IsActive()) { // If the RTP transport is already writable, then so are we. if (rtp_transport_->rtp_packet_transport()->writable()) { @@ -985,11 +986,8 @@ bool BaseChannel::RemoveRecvStream_w(uint32_t ssrc) { } bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, - ContentAction action, + SdpType type, std::string* error_desc) { - if (!(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER)) - return false; - // Check for streams that have been removed. bool ret = true; for (StreamParamsVec::const_iterator it = local_streams_.begin(); @@ -1024,11 +1022,8 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, bool BaseChannel::UpdateRemoteStreams_w( const std::vector& streams, - ContentAction action, + SdpType type, std::string* error_desc) { - if (!(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER)) - return false; - // Check for streams that have been removed. bool ret = true; for (StreamParamsVec::const_iterator it = remote_streams_.begin(); @@ -1422,7 +1417,7 @@ void VoiceChannel::UpdateMediaSendRecvState_w() { } bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "VoiceChannel::SetLocalContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1439,8 +1434,8 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); - if (!SetRtpTransportParameters(content, action, CS_LOCAL, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_LOCAL, rtp_header_extensions, + error_desc)) { return false; } @@ -1460,7 +1455,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, // only give it to the media channel once we have a remote // description too (without a remote description, we won't be able // to send them anyway). - if (!UpdateLocalStreams_w(audio->streams(), action, error_desc)) { + if (!UpdateLocalStreams_w(audio->streams(), type, error_desc)) { SafeSetError("Failed to set local audio description streams.", error_desc); return false; } @@ -1471,7 +1466,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, } bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "VoiceChannel::SetRemoteContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1488,8 +1483,8 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); - if (!SetRtpTransportParameters(content, action, CS_REMOTE, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_REMOTE, + rtp_header_extensions, error_desc)) { return false; } @@ -1512,7 +1507,7 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, // and only give it to the media channel once we have a local // description too (without a local description, we won't be able to // recv them anyway). - if (!UpdateRemoteStreams_w(audio->streams(), action, error_desc)) { + if (!UpdateRemoteStreams_w(audio->streams(), type, error_desc)) { SafeSetError("Failed to set remote audio description streams.", error_desc); return false; } @@ -1704,7 +1699,7 @@ void VideoChannel::StopMediaMonitor() { } bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "VideoChannel::SetLocalContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1721,8 +1716,8 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); - if (!SetRtpTransportParameters(content, action, CS_LOCAL, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_LOCAL, rtp_header_extensions, + error_desc)) { return false; } @@ -1742,7 +1737,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, // only give it to the media channel once we have a remote // description too (without a remote description, we won't be able // to send them anyway). - if (!UpdateLocalStreams_w(video->streams(), action, error_desc)) { + if (!UpdateLocalStreams_w(video->streams(), type, error_desc)) { SafeSetError("Failed to set local video description streams.", error_desc); return false; } @@ -1753,7 +1748,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, } bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "VideoChannel::SetRemoteContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1770,8 +1765,8 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); - if (!SetRtpTransportParameters(content, action, CS_REMOTE, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_REMOTE, + rtp_header_extensions, error_desc)) { return false; } @@ -1795,7 +1790,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, // and only give it to the media channel once we have a local // description too (without a local description, we won't be able to // recv them anyway). - if (!UpdateRemoteStreams_w(video->streams(), action, error_desc)) { + if (!UpdateRemoteStreams_w(video->streams(), type, error_desc)) { SafeSetError("Failed to set remote video description streams.", error_desc); return false; } @@ -1905,7 +1900,7 @@ bool RtpDataChannel::CheckDataChannelTypeFromContent( } bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "RtpDataChannel::SetLocalContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1926,8 +1921,8 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(data->rtp_header_extensions()); - if (!SetRtpTransportParameters(content, action, CS_LOCAL, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_LOCAL, rtp_header_extensions, + error_desc)) { return false; } @@ -1947,7 +1942,7 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, // only give it to the media channel once we have a remote // description too (without a remote description, we won't be able // to send them anyway). - if (!UpdateLocalStreams_w(data->streams(), action, error_desc)) { + if (!UpdateLocalStreams_w(data->streams(), type, error_desc)) { SafeSetError("Failed to set local data description streams.", error_desc); return false; } @@ -1958,7 +1953,7 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content, } bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + SdpType type, std::string* error_desc) { TRACE_EVENT0("webrtc", "RtpDataChannel::SetRemoteContent_w"); RTC_DCHECK(worker_thread() == rtc::Thread::Current()); @@ -1984,8 +1979,8 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content, GetFilteredRtpHeaderExtensions(data->rtp_header_extensions()); RTC_LOG(LS_INFO) << "Setting remote data description"; - if (!SetRtpTransportParameters(content, action, CS_REMOTE, - rtp_header_extensions, error_desc)) { + if (!SetRtpTransportParameters(content, type, CS_REMOTE, + rtp_header_extensions, error_desc)) { return false; } @@ -2003,7 +1998,7 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content, // and only give it to the media channel once we have a local // description too (without a local description, we won't be able to // recv them anyway). - if (!UpdateRemoteStreams_w(data->streams(), action, error_desc)) { + if (!UpdateRemoteStreams_w(data->streams(), type, error_desc)) { SafeSetError("Failed to set remote data description streams.", error_desc); return false; diff --git a/pc/channel.h b/pc/channel.h index d7db63a612..5c0f9684d4 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -19,6 +19,7 @@ #include #include "api/call/audio_sink.h" +#include "api/jsep.h" #include "api/rtpreceiverinterface.h" #include "media/base/mediachannel.h" #include "media/base/mediaengine.h" @@ -140,10 +141,10 @@ class BaseChannel rtc::PacketTransportInternal* rtcp_packet_transport); // Channel control bool SetLocalContent(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); bool SetRemoteContent(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); bool Enable(bool enable); @@ -302,22 +303,26 @@ class BaseChannel virtual void UpdateMediaSendRecvState_w() = 0; bool UpdateLocalStreams_w(const std::vector& streams, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); bool UpdateRemoteStreams_w(const std::vector& streams, - ContentAction action, + webrtc::SdpType type, std::string* error_desc); virtual bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) = 0; virtual bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) = 0; bool SetRtpTransportParameters(const MediaContentDescription* content, - ContentAction action, ContentSource src, - const RtpHeaderExtensions& extensions, std::string* error_desc); - bool SetRtpTransportParameters_n(const MediaContentDescription* content, - ContentAction action, ContentSource src, + webrtc::SdpType type, + ContentSource src, + const RtpHeaderExtensions& extensions, + std::string* error_desc); + bool SetRtpTransportParameters_n( + const MediaContentDescription* content, + webrtc::SdpType type, + ContentSource src, const std::vector& encrypted_extension_ids, std::string* error_desc); @@ -336,12 +341,12 @@ class BaseChannel bool* dtls, std::string* error_desc); bool SetSrtp_n(const std::vector& params, - ContentAction action, + webrtc::SdpType type, ContentSource src, const std::vector& encrypted_extension_ids, std::string* error_desc); bool SetRtcpMux_n(bool enable, - ContentAction action, + webrtc::SdpType type, ContentSource src, std::string* error_desc); @@ -526,10 +531,10 @@ class VoiceChannel : public BaseChannel { const rtc::PacketTime& packet_time) override; void UpdateMediaSendRecvState_w() override; bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; void HandleEarlyMediaTimeout(); bool InsertDtmf_w(uint32_t ssrc, int event, int duration); @@ -605,10 +610,10 @@ class VideoChannel : public BaseChannel { // overrides from BaseChannel void UpdateMediaSendRecvState_w() override; bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; bool GetStats_w(VideoMediaInfo* stats); webrtc::RtpParameters GetRtpSendParameters_w(uint32_t ssrc) const; @@ -720,10 +725,10 @@ class RtpDataChannel : public BaseChannel { bool CheckDataChannelTypeFromContent(const DataContentDescription* content, std::string* error_desc); bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action, + webrtc::SdpType type, std::string* error_desc) override; void UpdateMediaSendRecvState_w() override; diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 7cc5dec8af..0d155c4a3f 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -27,13 +27,11 @@ #include "rtc_base/logging.h" #include "rtc_base/sslstreamadapter.h" -using cricket::CA_OFFER; -using cricket::CA_PRANSWER; -using cricket::CA_ANSWER; using cricket::DtlsTransportInternal; using cricket::FakeVoiceMediaChannel; using cricket::StreamParams; using webrtc::RtpTransceiverDirection; +using webrtc::SdpType; namespace { const cricket::AudioCodec kPcmuCodec(0, "PCMU", 64000, 8000, 1); @@ -331,15 +329,15 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { bool SendInitiate() { bool result = channel1_->SetLocalContent(&local_media_content1_, - CA_OFFER, NULL); + SdpType::kOffer, NULL); if (result) { channel1_->Enable(true); result = channel2_->SetRemoteContent(&remote_media_content1_, - CA_OFFER, NULL); + SdpType::kOffer, NULL); if (result) { ConnectFakeTransports(); result = channel2_->SetLocalContent(&local_media_content2_, - CA_ANSWER, NULL); + SdpType::kAnswer, NULL); } } return result; @@ -348,27 +346,27 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { bool SendAccept() { channel2_->Enable(true); return channel1_->SetRemoteContent(&remote_media_content2_, - CA_ANSWER, NULL); + SdpType::kAnswer, NULL); } bool SendOffer() { bool result = channel1_->SetLocalContent(&local_media_content1_, - CA_OFFER, NULL); + SdpType::kOffer, NULL); if (result) { channel1_->Enable(true); result = channel2_->SetRemoteContent(&remote_media_content1_, - CA_OFFER, NULL); + SdpType::kOffer, NULL); } return result; } bool SendProvisionalAnswer() { bool result = channel2_->SetLocalContent(&local_media_content2_, - CA_PRANSWER, NULL); + SdpType::kPrAnswer, NULL); if (result) { channel2_->Enable(true); result = channel1_->SetRemoteContent(&remote_media_content2_, - CA_PRANSWER, NULL); + SdpType::kPrAnswer, NULL); ConnectFakeTransports(); } return result; @@ -376,10 +374,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { bool SendFinalAnswer() { bool result = channel2_->SetLocalContent(&local_media_content2_, - CA_ANSWER, NULL); + SdpType::kAnswer, NULL); if (result) result = channel1_->SetRemoteContent(&remote_media_content2_, - CA_ANSWER, NULL); + SdpType::kAnswer, NULL); return result; } @@ -592,9 +590,9 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateChannels(0, 0); typename T::Content content; CreateContent(0, kPcmuCodec, kH264Codec, &content); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -605,10 +603,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { void TestSetContentsNullOffer() { CreateChannels(0, 0); typename T::Content content; - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); CreateContent(0, kPcmuCodec, kH264Codec, &content); EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -622,12 +620,12 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content); // Both sides agree on mux. Should no longer be a separate RTCP channel. content.set_rtcp_mux(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); // Only initiator supports mux. Should still have a separate RTCP channel. - EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content, SdpType::kOffer, NULL)); content.set_rtcp_mux(false); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); } // Test that SetLocalContent and SetRemoteContent properly set RTCP @@ -637,17 +635,19 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content; CreateContent(0, kPcmuCodec, kH264Codec, &content); content.set_rtcp_mux(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_PRANSWER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); + EXPECT_TRUE( + channel1_->SetRemoteContent(&content, SdpType::kPrAnswer, NULL)); // Both sides agree on mux. Should signal RTCP mux as fully activated. EXPECT_EQ(0, rtcp_mux_activated_callbacks1_); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); EXPECT_EQ(1, rtcp_mux_activated_callbacks1_); // Only initiator supports mux. Should still have a separate RTCP channel. - EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content, SdpType::kOffer, NULL)); content.set_rtcp_mux(false); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_PRANSWER, NULL)); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE( + channel2_->SetRemoteContent(&content, SdpType::kPrAnswer, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); EXPECT_EQ(0, rtcp_mux_activated_callbacks2_); } @@ -665,7 +665,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Test that SetLocalContent and SetRemoteContent properly // handles adding and removing StreamParams when the action is a full - // CA_OFFER / CA_ANSWER. + // SdpType::kOffer / SdpType::kAnswer. void TestChangeStreamParamsInContent() { cricket::StreamParams stream1; stream1.groupid = "group1"; @@ -684,20 +684,20 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content1; CreateContent(0, kPcmuCodec, kH264Codec, &content1); content1.AddStream(stream1); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, NULL)); EXPECT_TRUE(channel1_->Enable(true)); EXPECT_EQ(1u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content1, SdpType::kOffer, NULL)); EXPECT_EQ(1u, media_channel2_->recv_streams().size()); ConnectFakeTransports(); // Channel 2 do not send anything. typename T::Content content2; CreateContent(0, kPcmuCodec, kH264Codec, &content2); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, NULL)); EXPECT_EQ(0u, media_channel1_->recv_streams().size()); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, SdpType::kAnswer, NULL)); EXPECT_TRUE(channel2_->Enable(true)); EXPECT_EQ(0u, media_channel2_->send_streams().size()); @@ -709,21 +709,21 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content3; CreateContent(SECURE, kPcmuCodec, kH264Codec, &content3); content3.AddStream(stream2); - EXPECT_TRUE(channel2_->SetLocalContent(&content3, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content3, SdpType::kOffer, NULL)); ASSERT_EQ(1u, media_channel2_->send_streams().size()); EXPECT_EQ(stream2, media_channel2_->send_streams()[0]); - EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content3, SdpType::kOffer, NULL)); ASSERT_EQ(1u, media_channel1_->recv_streams().size()); EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); // Channel 1 replies but stop sending stream1. typename T::Content content4; CreateContent(SECURE, kPcmuCodec, kH264Codec, &content4); - EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content4, SdpType::kAnswer, NULL)); EXPECT_EQ(0u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel2_->SetRemoteContent(&content4, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content4, SdpType::kAnswer, NULL)); EXPECT_EQ(0u, media_channel2_->recv_streams().size()); EXPECT_TRUE(channel1_->srtp_active()); @@ -812,12 +812,12 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(content_flags, kPcmuCodec, kH264Codec, &content1); content1.AddStream(stream1); content1.set_rtp_header_extensions(extensions1); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, NULL)); EXPECT_TRUE(channel1_->Enable(true)); EXPECT_EQ(1u, media_channel1_->send_streams().size()); packet_listener1.encrypted_headers.push_back(1); - EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content1, SdpType::kOffer, NULL)); EXPECT_EQ(1u, media_channel2_->recv_streams().size()); // Channel 2 sends back |stream2|. @@ -825,7 +825,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(content_flags, kPcmuCodec, kH264Codec, &content2); content2.AddStream(stream2); content2.set_rtp_header_extensions(extensions1); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, SdpType::kAnswer, NULL)); EXPECT_TRUE(channel2_->Enable(true)); EXPECT_EQ(1u, media_channel2_->send_streams().size()); packet_listener2.encrypted_headers.push_back(1); @@ -844,7 +844,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(CheckCustomRtp1(kSsrc2, 0)); } - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, NULL)); EXPECT_EQ(1u, media_channel1_->recv_streams().size()); if (scenario == DEFAULT) { @@ -863,14 +863,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(content_flags, kPcmuCodec, kH264Codec, &content3); content3.AddStream(stream2); content3.set_rtp_header_extensions(extensions2); - EXPECT_TRUE(channel2_->SetLocalContent(&content3, CA_OFFER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content3, SdpType::kOffer, NULL)); ASSERT_EQ(1u, media_channel2_->send_streams().size()); EXPECT_EQ(stream2, media_channel2_->send_streams()[0]); packet_listener2.encrypted_headers.clear(); packet_listener2.encrypted_headers.push_back(2); packet_listener2.encrypted_headers.push_back(4); - EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content3, SdpType::kOffer, NULL)); ASSERT_EQ(1u, media_channel1_->recv_streams().size()); EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); @@ -894,13 +894,13 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(content_flags, kPcmuCodec, kH264Codec, &content4); content4.AddStream(stream1); content4.set_rtp_header_extensions(extensions2); - EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content4, SdpType::kAnswer, NULL)); EXPECT_EQ(1u, media_channel1_->send_streams().size()); packet_listener1.encrypted_headers.clear(); packet_listener1.encrypted_headers.push_back(2); packet_listener1.encrypted_headers.push_back(4); - EXPECT_TRUE(channel2_->SetRemoteContent(&content4, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content4, SdpType::kAnswer, NULL)); EXPECT_EQ(1u, media_channel2_->recv_streams().size()); SendCustomRtp1(kSsrc1, 0); @@ -927,19 +927,19 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { } EXPECT_FALSE(media_channel1_->sending()); EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, - CA_OFFER, NULL)); + SdpType::kOffer, NULL)); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); } EXPECT_FALSE(media_channel1_->sending()); EXPECT_TRUE(channel2_->SetRemoteContent(&local_media_content1_, - CA_OFFER, NULL)); + SdpType::kOffer, NULL)); if (verify_playout_) { EXPECT_FALSE(media_channel2_->playout()); } EXPECT_FALSE(media_channel2_->sending()); EXPECT_TRUE(channel2_->SetLocalContent(&local_media_content2_, - CA_ANSWER, NULL)); + SdpType::kAnswer, NULL)); if (verify_playout_) { EXPECT_FALSE(media_channel2_->playout()); } @@ -959,7 +959,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { } EXPECT_TRUE(media_channel2_->sending()); EXPECT_TRUE(channel1_->SetRemoteContent(&local_media_content2_, - CA_ANSWER, NULL)); + SdpType::kAnswer, NULL)); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); } @@ -988,10 +988,12 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { } EXPECT_FALSE(media_channel2_->sending()); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); - EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER, NULL)); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_PRANSWER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_PRANSWER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content1, SdpType::kOffer, NULL)); + EXPECT_TRUE( + channel2_->SetLocalContent(&content2, SdpType::kPrAnswer, NULL)); + EXPECT_TRUE( + channel1_->SetRemoteContent(&content2, SdpType::kPrAnswer, NULL)); ConnectFakeTransports(); if (verify_playout_) { @@ -1005,8 +1007,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Update |content2| to be RecvOnly. content2.set_direction(RtpTransceiverDirection::kRecvOnly); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_PRANSWER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_PRANSWER, NULL)); + EXPECT_TRUE( + channel2_->SetLocalContent(&content2, SdpType::kPrAnswer, NULL)); + EXPECT_TRUE( + channel1_->SetRemoteContent(&content2, SdpType::kPrAnswer, NULL)); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); @@ -1019,8 +1023,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Update |content2| to be SendRecv. content2.set_direction(RtpTransceiverDirection::kSendRecv); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER, NULL)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, SdpType::kAnswer, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, NULL)); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); @@ -1683,17 +1687,17 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { media_channel1_->set_fail_set_recv_codecs(true); EXPECT_FALSE( - channel1_->SetLocalContent(content.get(), cricket::CA_OFFER, &err)); + channel1_->SetLocalContent(content.get(), SdpType::kOffer, &err)); EXPECT_FALSE( - channel1_->SetLocalContent(content.get(), cricket::CA_ANSWER, &err)); + channel1_->SetLocalContent(content.get(), SdpType::kAnswer, &err)); media_channel1_->set_fail_set_send_codecs(true); EXPECT_FALSE( - channel1_->SetRemoteContent(content.get(), cricket::CA_OFFER, &err)); + channel1_->SetRemoteContent(content.get(), SdpType::kOffer, &err)); media_channel1_->set_fail_set_send_codecs(true); EXPECT_FALSE( - channel1_->SetRemoteContent(content.get(), cricket::CA_ANSWER, &err)); + channel1_->SetRemoteContent(content.get(), SdpType::kAnswer, &err)); } void TestSendTwoOffers() { @@ -1703,13 +1707,13 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content1( CreateMediaContentWithStream(1)); EXPECT_TRUE( - channel1_->SetLocalContent(content1.get(), cricket::CA_OFFER, &err)); + channel1_->SetLocalContent(content1.get(), SdpType::kOffer, &err)); EXPECT_TRUE(media_channel1_->HasSendStream(1)); std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( - channel1_->SetLocalContent(content2.get(), cricket::CA_OFFER, &err)); + channel1_->SetLocalContent(content2.get(), SdpType::kOffer, &err)); EXPECT_FALSE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(2)); } @@ -1721,13 +1725,13 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content1( CreateMediaContentWithStream(1)); EXPECT_TRUE( - channel1_->SetRemoteContent(content1.get(), cricket::CA_OFFER, &err)); + channel1_->SetRemoteContent(content1.get(), SdpType::kOffer, &err)); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( - channel1_->SetRemoteContent(content2.get(), cricket::CA_OFFER, &err)); + channel1_->SetRemoteContent(content2.get(), SdpType::kOffer, &err)); EXPECT_FALSE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(2)); } @@ -1740,14 +1744,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content1( CreateMediaContentWithStream(1)); EXPECT_TRUE( - channel1_->SetRemoteContent(content1.get(), cricket::CA_OFFER, &err)); + channel1_->SetRemoteContent(content1.get(), SdpType::kOffer, &err)); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); // Send PR answer std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( - channel1_->SetLocalContent(content2.get(), cricket::CA_PRANSWER, &err)); + channel1_->SetLocalContent(content2.get(), SdpType::kPrAnswer, &err)); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_TRUE(media_channel1_->HasSendStream(2)); @@ -1755,7 +1759,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content3( CreateMediaContentWithStream(3)); EXPECT_TRUE( - channel1_->SetLocalContent(content3.get(), cricket::CA_ANSWER, &err)); + channel1_->SetLocalContent(content3.get(), SdpType::kAnswer, &err)); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); EXPECT_FALSE(media_channel1_->HasSendStream(2)); EXPECT_TRUE(media_channel1_->HasSendStream(3)); @@ -1769,14 +1773,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content1( CreateMediaContentWithStream(1)); EXPECT_TRUE( - channel1_->SetLocalContent(content1.get(), cricket::CA_OFFER, &err)); + channel1_->SetLocalContent(content1.get(), SdpType::kOffer, &err)); EXPECT_TRUE(media_channel1_->HasSendStream(1)); // Receive PR answer std::unique_ptr content2( CreateMediaContentWithStream(2)); - EXPECT_TRUE(channel1_->SetRemoteContent(content2.get(), - cricket::CA_PRANSWER, &err)); + EXPECT_TRUE( + channel1_->SetRemoteContent(content2.get(), SdpType::kPrAnswer, &err)); EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_TRUE(media_channel1_->HasRecvStream(2)); @@ -1784,7 +1788,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr content3( CreateMediaContentWithStream(3)); EXPECT_TRUE( - channel1_->SetRemoteContent(content3.get(), cricket::CA_ANSWER, &err)); + channel1_->SetRemoteContent(content3.get(), SdpType::kAnswer, &err)); EXPECT_TRUE(media_channel1_->HasSendStream(1)); EXPECT_FALSE(media_channel1_->HasRecvStream(2)); EXPECT_TRUE(media_channel1_->HasRecvStream(3)); @@ -1830,9 +1834,9 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content); // Both sides agree on mux. Should signal that RTCP mux is fully active. content.set_rtcp_mux(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, NULL)); EXPECT_EQ(0, rtcp_mux_activated_callbacks1_); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, NULL)); EXPECT_EQ(1, rtcp_mux_activated_callbacks1_); cricket::FakeDtlsTransport* rtp = fake_rtp_dtls_transport1_.get(); EXPECT_FALSE(media_channel1_->ready_to_send()); @@ -1855,7 +1859,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content; CreateContent(0, kPcmuCodec, kH264Codec, &content); content.set_bandwidth(remote_limit); - return channel1_->SetRemoteContent(&content, CA_OFFER, NULL); + return channel1_->SetRemoteContent(&content, SdpType::kOffer, NULL); } webrtc::RtpParameters BitrateLimitedParameters(rtc::Optional limit) { @@ -1874,8 +1878,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { void DefaultMaxBitrateIsUnlimited() { CreateChannels(0, 0); - EXPECT_TRUE( - channel1_->SetLocalContent(&local_media_content1_, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, + SdpType::kOffer, NULL)); EXPECT_EQ(media_channel1_->max_bps(), -1); VerifyMaxBitrate(media_channel1_->GetRtpSendParameters(kSsrc1), rtc::nullopt); @@ -1883,8 +1887,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { void CanChangeMaxBitrate() { CreateChannels(0, 0); - EXPECT_TRUE( - channel1_->SetLocalContent(&local_media_content1_, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, + SdpType::kOffer, NULL)); EXPECT_TRUE(channel1_->SetRtpSendParameters( kSsrc1, BitrateLimitedParameters(1000))); @@ -3660,8 +3664,10 @@ TEST_F(BaseChannelDeathTest, SetTransportsWithUnneededRtcpTransport) { // Activate RTCP muxing, simulating offer/answer negotiation. cricket::AudioContentDescription content; content.set_rtcp_mux(true); - ASSERT_TRUE(voice_channel_.SetLocalContent(&content, CA_OFFER, nullptr)); - ASSERT_TRUE(voice_channel_.SetRemoteContent(&content, CA_ANSWER, nullptr)); + ASSERT_TRUE( + voice_channel_.SetLocalContent(&content, SdpType::kOffer, nullptr)); + ASSERT_TRUE( + voice_channel_.SetRemoteContent(&content, SdpType::kAnswer, nullptr)); cricket::FakeDtlsTransport new_rtp_transport( "bar", cricket::ICE_CANDIDATE_COMPONENT_RTP); cricket::FakeDtlsTransport new_rtcp_transport( diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index acde8d12e8..fcc3af9227 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -532,20 +532,6 @@ bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc, return false; } -// Converts from SessionDescriptionInterface type to cricket::ContentAction. -cricket::ContentAction ContentActionFromSessionType( - const std::string& session_type) { - if (session_type == SessionDescriptionInterface::kOffer) { - return cricket::CA_OFFER; - } else if (session_type == SessionDescriptionInterface::kPrAnswer) { - return cricket::CA_PRANSWER; - } else if (session_type == SessionDescriptionInterface::kAnswer) { - return cricket::CA_ANSWER; - } - RTC_NOTREACHED() << "unknown action type"; - return cricket::CA_OFFER; -} - } // namespace // Upon completion, posts a task to execute the callback of the @@ -1531,9 +1517,9 @@ RTCError PeerConnection::ApplyLocalDescription( stats_->UpdateStats(kStatsOutputLevelStandard); // Update the initial_offerer flag if this session is the initial_offerer. - cricket::ContentAction action = ContentActionFromSessionType(desc->type()); + SdpType type = desc->GetType(); if (!initial_offerer_.has_value()) { - initial_offerer_.emplace(action == cricket::CA_OFFER); + initial_offerer_.emplace(type == SdpType::kOffer); if (*initial_offerer_) { transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLING); } else { @@ -1541,7 +1527,7 @@ RTCError PeerConnection::ApplyLocalDescription( } } - if (action == cricket::CA_ANSWER) { + if (type == SdpType::kAnswer) { current_local_description_ = std::move(desc); pending_local_description_ = nullptr; current_remote_description_ = std::move(pending_remote_description_); @@ -1553,7 +1539,7 @@ RTCError PeerConnection::ApplyLocalDescription( RTC_DCHECK(local_description()); // Transport and Media channels will be created only when offer is set. - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { // TODO(mallinath) - Handle CreateChannel failure, as new local description // is applied. Restore back to old description. RTCError error = CreateChannels(local_description()->description()); @@ -1565,7 +1551,7 @@ RTCError PeerConnection::ApplyLocalDescription( // Remove unused channels if MediaContentDescription is rejected. RemoveUnusedChannels(local_description()->description()); - error = UpdateSessionState(action, cricket::CS_LOCAL); + error = UpdateSessionState(type, cricket::CS_LOCAL); if (!error.ok()) { return error; } @@ -1700,8 +1686,8 @@ RTCError PeerConnection::ApplyRemoteDescription( // Grab ownership of the description being replaced for the remainder of this // method, since it's used below as |old_remote_description|. std::unique_ptr replaced_remote_description; - cricket::ContentAction action = ContentActionFromSessionType(desc->type()); - if (action == cricket::CA_ANSWER) { + SdpType type = desc->GetType(); + if (type == SdpType::kAnswer) { replaced_remote_description = pending_remote_description_ ? std::move(pending_remote_description_) : std::move(current_remote_description_); @@ -1717,7 +1703,7 @@ RTCError PeerConnection::ApplyRemoteDescription( RTC_DCHECK(remote_description()); // Transport and Media channels will be created only when offer is set. - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { // TODO(mallinath) - Handle CreateChannel failure, as new local description // is applied. Restore back to old description. RTCError error = CreateChannels(remote_description()->description()); @@ -1731,7 +1717,7 @@ RTCError PeerConnection::ApplyRemoteDescription( // NOTE: Candidates allocation will be initiated only when SetLocalDescription // is called. - error = UpdateSessionState(action, cricket::CS_REMOTE); + error = UpdateSessionState(type, cricket::CS_REMOTE); if (!error.ok()) { return error; } @@ -1751,7 +1737,7 @@ RTCError PeerConnection::ApplyRemoteDescription( // against the current description. if (CheckForRemoteIceRestart(old_remote_description, remote_description(), content.name)) { - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { pending_ice_restarts_.insert(content.name); } } else { @@ -3514,7 +3500,7 @@ void PeerConnection::SetSessionError(SessionError error, } } -RTCError PeerConnection::UpdateSessionState(cricket::ContentAction action, +RTCError PeerConnection::UpdateSessionState(SdpType type, cricket::ContentSource source) { RTC_DCHECK_RUN_ON(signaling_thread()); @@ -3524,7 +3510,7 @@ RTCError PeerConnection::UpdateSessionState(cricket::ContentAction action, // If this is an answer then we know whether to BUNDLE or not. If both the // local and remote side have agreed to BUNDLE, go ahead and enable it. - if (action == cricket::CA_ANSWER) { + if (type == SdpType::kAnswer) { const cricket::ContentGroup* local_bundle = local_description()->description()->GetGroupByName( cricket::GROUP_TYPE_BUNDLE); @@ -3545,34 +3531,34 @@ RTCError PeerConnection::UpdateSessionState(cricket::ContentAction action, // Only push down the transport description after potentially enabling BUNDLE; // we don't want to push down a description on a transport about to be // destroyed. - RTCError error = PushdownTransportDescription(source, action); + RTCError error = PushdownTransportDescription(source, type); if (!error.ok()) { return error; } // If this is answer-ish we're ready to let media flow. - if (action == cricket::CA_ANSWER || action == cricket::CA_PRANSWER) { + if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { EnableSending(); } // Update the signaling state according to the specified state machine (see // https://w3c.github.io/webrtc-pc/#rtcsignalingstate-enum). - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { ChangeSignalingState(source == cricket::CS_LOCAL ? PeerConnectionInterface::kHaveLocalOffer : PeerConnectionInterface::kHaveRemoteOffer); - } else if (action == cricket::CA_PRANSWER) { + } else if (type == SdpType::kPrAnswer) { ChangeSignalingState(source == cricket::CS_LOCAL ? PeerConnectionInterface::kHaveLocalPrAnswer : PeerConnectionInterface::kHaveRemotePrAnswer); } else { - RTC_DCHECK_EQ(action, cricket::CA_ANSWER); + RTC_DCHECK(type == SdpType::kAnswer); ChangeSignalingState(PeerConnectionInterface::kStable); } // Update internal objects according to the session description's media // descriptions. - error = PushdownMediaDescription(action, source); + error = PushdownMediaDescription(type, source); if (!error.ok()) { SetSessionError(SessionError::kContent, error.message()); } @@ -3584,7 +3570,7 @@ RTCError PeerConnection::UpdateSessionState(cricket::ContentAction action, } RTCError PeerConnection::PushdownMediaDescription( - cricket::ContentAction action, + SdpType type, cricket::ContentSource source) { const SessionDescriptionInterface* sdesc = (source == cricket::CS_LOCAL ? local_description() @@ -3607,8 +3593,8 @@ RTCError PeerConnection::PushdownMediaDescription( std::string error; bool success = (source == cricket::CS_LOCAL) - ? channel->SetLocalContent(content_desc, action, &error) - : channel->SetRemoteContent(content_desc, action, &error); + ? channel->SetLocalContent(content_desc, type, &error) + : channel->SetRemoteContent(content_desc, type, &error); if (!success) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, std::move(error)); } @@ -3626,8 +3612,8 @@ RTCError PeerConnection::PushdownMediaDescription( std::string error; bool success = (source == cricket::CS_LOCAL) - ? rtp_data_channel_->SetLocalContent(data_desc, action, &error) - : rtp_data_channel_->SetRemoteContent(data_desc, action, + ? rtp_data_channel_->SetLocalContent(data_desc, type, &error) + : rtp_data_channel_->SetRemoteContent(data_desc, type, &error); if (!success) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, @@ -3667,7 +3653,7 @@ bool PeerConnection::PushdownSctpParameters_n(cricket::ContentSource source) { RTCError PeerConnection::PushdownTransportDescription( cricket::ContentSource source, - cricket::ContentAction action) { + SdpType type) { RTC_DCHECK_RUN_ON(signaling_thread()); const SessionDescriptionInterface* sdesc = @@ -3680,10 +3666,10 @@ RTCError PeerConnection::PushdownTransportDescription( bool success; if (source == cricket::CS_LOCAL) { success = transport_controller_->SetLocalTransportDescription( - tinfo.content_name, tinfo.description, action, &error); + tinfo.content_name, tinfo.description, type, &error); } else { success = transport_controller_->SetRemoteTransportDescription( - tinfo.content_name, tinfo.description, action, &error); + tinfo.content_name, tinfo.description, type, &error); } if (!success) { LOG_AND_RETURN_ERROR( @@ -4585,9 +4571,9 @@ RTCError PeerConnection::ValidateSessionDescription( LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, kInvalidSdp); } - cricket::ContentAction action = ContentActionFromSessionType(sdesc->type()); - if ((source == cricket::CS_LOCAL && !ExpectSetLocalDescription(action)) || - (source == cricket::CS_REMOTE && !ExpectSetRemoteDescription(action))) { + SdpType type = sdesc->GetType(); + if ((source == cricket::CS_LOCAL && !ExpectSetLocalDescription(type)) || + (source == cricket::CS_REMOTE && !ExpectSetRemoteDescription(type))) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_PARAMETER, "Called in wrong state: " + GetSignalingStateString(signaling_state())); @@ -4618,7 +4604,7 @@ RTCError PeerConnection::ValidateSessionDescription( // m-lines that do not rtcp-mux enabled. // Verify m-lines in Answer when compared against Offer. - if (action == cricket::CA_ANSWER || action == cricket::CA_PRANSWER) { + if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { const cricket::SessionDescription* offer_desc = (source == cricket::CS_LOCAL) ? remote_description()->description() : local_description()->description(); @@ -4646,25 +4632,25 @@ RTCError PeerConnection::ValidateSessionDescription( return RTCError::OK(); } -bool PeerConnection::ExpectSetLocalDescription(cricket::ContentAction action) { +bool PeerConnection::ExpectSetLocalDescription(SdpType type) { PeerConnectionInterface::SignalingState state = signaling_state(); - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { return (state == PeerConnectionInterface::kStable) || (state == PeerConnectionInterface::kHaveLocalOffer); } else { - RTC_DCHECK(action == cricket::CA_ANSWER || action == cricket::CA_PRANSWER); + RTC_DCHECK(type == SdpType::kPrAnswer || type == SdpType::kAnswer); return (state == PeerConnectionInterface::kHaveRemoteOffer) || (state == PeerConnectionInterface::kHaveLocalPrAnswer); } } -bool PeerConnection::ExpectSetRemoteDescription(cricket::ContentAction action) { +bool PeerConnection::ExpectSetRemoteDescription(SdpType type) { PeerConnectionInterface::SignalingState state = signaling_state(); - if (action == cricket::CA_OFFER) { + if (type == SdpType::kOffer) { return (state == PeerConnectionInterface::kStable) || (state == PeerConnectionInterface::kHaveRemoteOffer); } else { - RTC_DCHECK(action == cricket::CA_ANSWER || action == cricket::CA_PRANSWER); + RTC_DCHECK(type == SdpType::kPrAnswer || type == SdpType::kAnswer); return (state == PeerConnectionInterface::kHaveLocalOffer) || (state == PeerConnectionInterface::kHaveRemotePrAnswer); } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 4c8983bc47..c2b743dec1 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -616,16 +616,15 @@ class PeerConnection : public PeerConnectionInterface, // Updates the error state, signaling if necessary. void SetSessionError(SessionError error, const std::string& error_desc); - RTCError UpdateSessionState(cricket::ContentAction action, - cricket::ContentSource source); + RTCError UpdateSessionState(SdpType type, cricket::ContentSource source); // Push the media parts of the local or remote session description // down to all of the channels. - RTCError PushdownMediaDescription(cricket::ContentAction action, + RTCError PushdownMediaDescription(SdpType type, cricket::ContentSource source); bool PushdownSctpParameters_n(cricket::ContentSource source); RTCError PushdownTransportDescription(cricket::ContentSource source, - cricket::ContentAction action); + SdpType type); // Returns true and the TransportInfo of the given |content_name| // from |description|. Returns false if it's not available. @@ -705,13 +704,15 @@ class PeerConnection : public PeerConnectionInterface, RTCError ValidateSessionDescription(const SessionDescriptionInterface* sdesc, cricket::ContentSource source); - // Check if a call to SetLocalDescription is acceptable with |action|. - bool ExpectSetLocalDescription(cricket::ContentAction action); - // Check if a call to SetRemoteDescription is acceptable with |action|. - bool ExpectSetRemoteDescription(cricket::ContentAction action); + // Check if a call to SetLocalDescription is acceptable with a session + // description of the given type. + bool ExpectSetLocalDescription(SdpType type); + // Check if a call to SetRemoteDescription is acceptable with a session + // description of the given type. + bool ExpectSetRemoteDescription(SdpType type); // Verifies a=setup attribute as per RFC 5763. bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc, - cricket::ContentAction action); + SdpType type); // Returns true if we are ready to push down the remote candidate. // |remote_desc| is the new remote description, or NULL if the current remote diff --git a/pc/test/faketransportcontroller.h b/pc/test/faketransportcontroller.h index e17956769f..4a4cc96d8e 100644 --- a/pc/test/faketransportcontroller.h +++ b/pc/test/faketransportcontroller.h @@ -105,13 +105,13 @@ class FakeTransportController : public TransportController { remote_fingerprint.get()); std::string err; SetLocalTransportDescription(transport_name, local_desc, - cricket::CA_OFFER, &err); + webrtc::SdpType::kOffer, &err); dest->SetRemoteTransportDescription(transport_name, local_desc, - cricket::CA_OFFER, &err); + webrtc::SdpType::kOffer, &err); dest->SetLocalTransportDescription(transport_name, remote_desc, - cricket::CA_ANSWER, &err); + webrtc::SdpType::kAnswer, &err); SetRemoteTransportDescription(transport_name, remote_desc, - cricket::CA_ANSWER, &err); + webrtc::SdpType::kAnswer, &err); } MaybeStartGathering(); dest->MaybeStartGathering(); diff --git a/pc/transportcontroller.cc b/pc/transportcontroller.cc index d58564ce2f..ce49af3594 100644 --- a/pc/transportcontroller.cc +++ b/pc/transportcontroller.cc @@ -18,6 +18,8 @@ #include "rtc_base/checks.h" #include "rtc_base/thread.h" +using webrtc::SdpType; + namespace { enum { @@ -158,23 +160,23 @@ TransportController::GetRemoteSSLCertificate( bool TransportController::SetLocalTransportDescription( const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + SdpType type, std::string* err) { return network_thread_->Invoke( RTC_FROM_HERE, rtc::Bind(&TransportController::SetLocalTransportDescription_n, this, - transport_name, tdesc, action, err)); + transport_name, tdesc, type, err)); } bool TransportController::SetRemoteTransportDescription( const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + SdpType type, std::string* err) { return network_thread_->Invoke( RTC_FROM_HERE, rtc::Bind(&TransportController::SetRemoteTransportDescription_n, this, - transport_name, tdesc, action, err)); + transport_name, tdesc, type, err)); } void TransportController::MaybeStartGathering() { @@ -584,7 +586,7 @@ TransportController::GetRemoteSSLCertificate_n( bool TransportController::SetLocalTransportDescription_n( const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + SdpType type, std::string* err) { RTC_DCHECK(network_thread_->IsCurrent()); @@ -626,18 +628,18 @@ bool TransportController::SetLocalTransportDescription_n( (!transport->remote_description() || transport->remote_description()->ice_mode != ICEMODE_LITE)) { IceRole new_ice_role = - (action == CA_OFFER) ? ICEROLE_CONTROLLING : ICEROLE_CONTROLLED; + (type == SdpType::kOffer) ? ICEROLE_CONTROLLING : ICEROLE_CONTROLLED; SetIceRole(new_ice_role); } RTC_LOG(LS_INFO) << "Set local transport description on " << transport_name; - return transport->SetLocalTransportDescription(tdesc, action, err); + return transport->SetLocalTransportDescription(tdesc, type, err); } bool TransportController::SetRemoteTransportDescription_n( const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + SdpType type, std::string* err) { RTC_DCHECK(network_thread_->IsCurrent()); @@ -668,7 +670,7 @@ bool TransportController::SetRemoteTransportDescription_n( } RTC_LOG(LS_INFO) << "Set remote transport description on " << transport_name; - return transport->SetRemoteTransportDescription(tdesc, action, err); + return transport->SetRemoteTransportDescription(tdesc, type, err); } void TransportController::MaybeStartGathering_n() { diff --git a/pc/transportcontroller.h b/pc/transportcontroller.h index 534f117186..51f870e6a8 100644 --- a/pc/transportcontroller.h +++ b/pc/transportcontroller.h @@ -92,11 +92,11 @@ class TransportController : public sigslot::has_slots<>, const std::string& transport_name) const; bool SetLocalTransportDescription(const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + webrtc::SdpType type, std::string* err); bool SetRemoteTransportDescription(const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + webrtc::SdpType type, std::string* err); // Start gathering candidates for any new transports, or transports doing an // ICE restart. @@ -213,11 +213,11 @@ class TransportController : public sigslot::has_slots<>, const std::string& transport_name) const; bool SetLocalTransportDescription_n(const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + webrtc::SdpType type, std::string* err); bool SetRemoteTransportDescription_n(const std::string& transport_name, const TransportDescription& tdesc, - ContentAction action, + webrtc::SdpType type, std::string* err); void MaybeStartGathering_n(); bool AddRemoteCandidates_n(const std::string& transport_name, diff --git a/pc/transportcontroller_unittest.cc b/pc/transportcontroller_unittest.cc index 32dc1b1496..f6564c5c60 100644 --- a/pc/transportcontroller_unittest.cc +++ b/pc/transportcontroller_unittest.cc @@ -23,6 +23,8 @@ #include "rtc_base/sslidentity.h" #include "rtc_base/thread.h" +using webrtc::SdpType; + static const int kTimeout = 100; static const char kIceUfrag1[] = "TESTICEUFRAG0001"; static const char kIcePwd1[] = "TESTICEPWD00000000000001"; @@ -111,9 +113,9 @@ class TransportControllerTest : public testing::Test, CONNECTIONROLE_ACTPASS, nullptr); std::string err; transport_controller_->SetLocalTransportDescription("audio", local_desc, - CA_OFFER, &err); + SdpType::kOffer, &err); transport_controller_->SetLocalTransportDescription("video", local_desc, - CA_OFFER, &err); + SdpType::kOffer, &err); transport_controller_->MaybeStartGathering(); transport1->fake_ice_transport()->SignalCandidateGathered( transport1->fake_ice_transport(), CreateCandidate(1)); @@ -297,9 +299,9 @@ TEST_F(TransportControllerTest, TestGetSslRole) { CONNECTIONROLE_ACTIVE, fingerprint.get()); std::string err; EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, cricket::CA_OFFER, &err)); + "audio", local_desc, SdpType::kOffer, &err)); EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, cricket::CA_ANSWER, &err)); + "audio", remote_desc, SdpType::kAnswer, &err)); // Finally we can get the role. Should be "server" since the remote // endpoint's role was "active". @@ -369,7 +371,7 @@ TEST_F(TransportControllerTest, TestSetLocalTransportDescription) { CONNECTIONROLE_ACTPASS, nullptr); std::string err; EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_OFFER, &err)); + "audio", local_desc, SdpType::kOffer, &err)); // Check that ICE ufrag and pwd were propagated to transport. EXPECT_EQ(kIceUfrag1, transport->fake_ice_transport()->ice_ufrag()); EXPECT_EQ(kIcePwd1, transport->fake_ice_transport()->ice_pwd()); @@ -388,7 +390,7 @@ TEST_F(TransportControllerTest, TestSetRemoteTransportDescription) { CONNECTIONROLE_ACTPASS, nullptr); std::string err; EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); // Check that ICE ufrag and pwd were propagated to transport. EXPECT_EQ(kIceUfrag1, transport->fake_ice_transport()->remote_ice_ufrag()); EXPECT_EQ(kIcePwd1, transport->fake_ice_transport()->remote_ice_pwd()); @@ -417,14 +419,14 @@ TEST_F(TransportControllerTest, TestReadyForRemoteCandidates) { kIcePwd1, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); EXPECT_FALSE(transport_controller_->ReadyForRemoteCandidates("audio")); TransportDescription local_desc(std::vector(), kIceUfrag2, kIcePwd2, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, &err)); + "audio", local_desc, SdpType::kAnswer, &err)); EXPECT_TRUE(transport_controller_->ReadyForRemoteCandidates("audio")); } @@ -693,7 +695,7 @@ TEST_F(TransportControllerTest, TestSignalCandidatesGathered) { CONNECTIONROLE_ACTPASS, nullptr); std::string err; EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_OFFER, &err)); + "audio", local_desc, SdpType::kOffer, &err)); transport_controller_->MaybeStartGathering(); transport->fake_ice_transport()->SignalCandidateGathered( @@ -740,12 +742,12 @@ TEST_F(TransportControllerTest, IceRoleRedeterminedOnIceRestartByDefault) { kIcePwd1, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); TransportDescription local_desc(std::vector(), kIceUfrag2, kIcePwd2, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, &err)); + "audio", local_desc, SdpType::kAnswer, &err)); EXPECT_EQ(ICEROLE_CONTROLLED, transport->fake_ice_transport()->GetIceRole()); // The endpoint that initiated an ICE restart should take the controlling @@ -754,7 +756,7 @@ TEST_F(TransportControllerTest, IceRoleRedeterminedOnIceRestartByDefault) { kIcePwd3, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", ice_restart_desc, CA_OFFER, &err)); + "audio", ice_restart_desc, SdpType::kOffer, &err)); EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole()); } @@ -773,12 +775,12 @@ TEST_F(TransportControllerTest, IceRoleNotRedetermined) { kIcePwd1, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); TransportDescription local_desc(std::vector(), kIceUfrag2, kIcePwd2, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, &err)); + "audio", local_desc, SdpType::kAnswer, &err)); EXPECT_EQ(ICEROLE_CONTROLLED, transport->fake_ice_transport()->GetIceRole()); // The endpoint that initiated an ICE restart should keep the existing role. @@ -786,7 +788,7 @@ TEST_F(TransportControllerTest, IceRoleNotRedetermined) { kIcePwd3, ICEMODE_FULL, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", ice_restart_desc, CA_OFFER, &err)); + "audio", ice_restart_desc, SdpType::kOffer, &err)); EXPECT_EQ(ICEROLE_CONTROLLED, transport->fake_ice_transport()->GetIceRole()); } @@ -801,10 +803,10 @@ TEST_F(TransportControllerTest, TestSetRemoteIceLiteInOffer) { kIcePwd1, ICEMODE_LITE, CONNECTIONROLE_ACTPASS, nullptr); EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, nullptr)); + "audio", local_desc, SdpType::kAnswer, nullptr)); EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole()); EXPECT_EQ(ICEMODE_LITE, transport->fake_ice_transport()->remote_ice_mode()); @@ -819,7 +821,7 @@ TEST_F(TransportControllerTest, TestSetRemoteIceLiteInAnswer) { transport_controller_->SetIceRole(ICEROLE_CONTROLLING); TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_OFFER, nullptr)); + "audio", local_desc, SdpType::kOffer, nullptr)); EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole()); // Transports will be created in ICEFULL_MODE. EXPECT_EQ(ICEMODE_FULL, transport->fake_ice_transport()->remote_ice_mode()); @@ -827,7 +829,7 @@ TEST_F(TransportControllerTest, TestSetRemoteIceLiteInAnswer) { kIcePwd1, ICEMODE_LITE, CONNECTIONROLE_NONE, nullptr); ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_ANSWER, nullptr)); + "audio", remote_desc, SdpType::kAnswer, nullptr)); EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole()); // After receiving remote description with ICEMODE_LITE, transport should // have mode set to ICEMODE_LITE. @@ -849,18 +851,18 @@ TEST_F(TransportControllerTest, CONNECTIONROLE_ACTPASS, nullptr); TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, nullptr)); + "audio", local_desc, SdpType::kAnswer, nullptr)); // Subsequent ICE restart offer/answer. remote_desc.ice_ufrag = kIceUfrag2; remote_desc.ice_pwd = kIcePwd2; local_desc.ice_ufrag = kIceUfrag2; local_desc.ice_pwd = kIcePwd2; ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_OFFER, &err)); + "audio", remote_desc, SdpType::kOffer, &err)); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_ANSWER, nullptr)); + "audio", local_desc, SdpType::kAnswer, nullptr)); EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole()); } @@ -875,13 +877,13 @@ TEST_F(TransportControllerTest, NeedsIceRestart) { TransportDescription local_desc(kIceUfrag1, kIcePwd1); TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", local_desc, CA_OFFER, nullptr)); + "audio", local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "video", local_desc, CA_OFFER, nullptr)); + "video", local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription( - "audio", remote_desc, CA_ANSWER, nullptr)); + "audio", remote_desc, SdpType::kAnswer, nullptr)); ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription( - "video", remote_desc, CA_ANSWER, nullptr)); + "video", remote_desc, SdpType::kAnswer, nullptr)); // Initially NeedsIceRestart should return false. EXPECT_FALSE(transport_controller_->NeedsIceRestart("audio")); @@ -898,9 +900,9 @@ TEST_F(TransportControllerTest, NeedsIceRestart) { // Do ICE restart but only for audio. TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "audio", ice_restart_local_desc, CA_OFFER, nullptr)); + "audio", ice_restart_local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_controller_->SetLocalTransportDescription( - "video", local_desc, CA_OFFER, nullptr)); + "video", local_desc, SdpType::kOffer, nullptr)); // NeedsIceRestart should still be true for video. EXPECT_FALSE(transport_controller_->NeedsIceRestart("audio")); EXPECT_TRUE(transport_controller_->NeedsIceRestart("video"));