diff options
author | sergeyu@chromium.org <sergeyu@chromium.org@4adac7df-926f-26a2-2b94-8c16560cd09d> | 2014-01-15 23:15:54 +0000 |
---|---|---|
committer | sergeyu@chromium.org <sergeyu@chromium.org@4adac7df-926f-26a2-2b94-8c16560cd09d> | 2014-01-15 23:15:54 +0000 |
commit | 4b26e2eee3e3b2a0c22946372a38f7efa6cee146 (patch) | |
tree | c02b5c4ba1d7f86ad955341cee8642457bf87194 | |
parent | 7a2ca7c6215d918a02ed2d5576668406a25c103b (diff) | |
download | webrtc-4b26e2eee3e3b2a0c22946372a38f7efa6cee146.tar.gz |
Update libjingle to 59676287
R=wu@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/7229004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5390 4adac7df-926f-26a2-2b94-8c16560cd09d
47 files changed, 1203 insertions, 718 deletions
diff --git a/talk/app/webrtc/videosource_unittest.cc b/talk/app/webrtc/videosource_unittest.cc index 69e9b3f0f7..213508f28e 100644 --- a/talk/app/webrtc/videosource_unittest.cc +++ b/talk/app/webrtc/videosource_unittest.cc @@ -238,7 +238,7 @@ TEST_F(VideoSourceTest, MandatoryConstraintCif5Fps) { ASSERT_TRUE(format != NULL); EXPECT_EQ(352, format->width); EXPECT_EQ(288, format->height); - EXPECT_EQ(5, format->framerate()); + EXPECT_EQ(30, format->framerate()); } // Test that the capture output is 720P if the camera support it and the @@ -491,7 +491,7 @@ TEST_F(VideoSourceTest, MixedOptionsAndConstraints) { ASSERT_TRUE(format != NULL); EXPECT_EQ(352, format->width); EXPECT_EQ(288, format->height); - EXPECT_EQ(5, format->framerate()); + EXPECT_EQ(30, format->framerate()); bool value = true; EXPECT_TRUE(source_->options()->video_noise_reduction.Get(&value)); @@ -554,6 +554,6 @@ TEST_F(VideoSourceTest, OptionalSubOneFpsConstraints) { kMaxWaitMs); const cricket::VideoFormat* format = capturer_->GetCaptureFormat(); ASSERT_TRUE(format != NULL); - EXPECT_EQ(1, format->framerate()); + EXPECT_EQ(30, format->framerate()); } diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 404d963995..0de46e71bf 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -55,29 +55,22 @@ using cricket::TransportInfo; namespace webrtc { // Error messages -const char kSetLocalSdpFailed[] = "SetLocalDescription failed: "; -const char kSetRemoteSdpFailed[] = "SetRemoteDescription failed: "; -const char kCreateChannelFailed[] = "Failed to create channels."; const char kBundleWithoutRtcpMux[] = "RTCP-MUX must be enabled when BUNDLE " "is enabled."; +const char kCreateChannelFailed[] = "Failed to create channels."; const char kInvalidCandidates[] = "Description contains invalid candidates."; const char kInvalidSdp[] = "Invalid session description."; const char kMlineMismatch[] = - "Offer and answer descriptions m-lines are not matching. " - "Rejecting answer."; + "Offer and answer descriptions m-lines are not matching. Rejecting answer."; +const char kPushDownTDFailed[] = + "Failed to push down transport description:"; const char kSdpWithoutCrypto[] = "Called with a SDP without crypto enabled."; -const char kSdpWithoutSdesAndDtlsDisabled[] = - "Called with an SDP without SDES crypto and DTLS disabled locally."; const char kSdpWithoutIceUfragPwd[] = - "Called with an SDP without ice-ufrag and ice-pwd."; + "Called with a SDP without ice-ufrag and ice-pwd."; +const char kSdpWithoutSdesAndDtlsDisabled[] = + "Called with a SDP without SDES crypto and DTLS disabled locally."; const char kSessionError[] = "Session error code: "; -const char kUpdateStateFailed[] = "Failed to update session state: "; -const char kPushDownOfferTDFailed[] = - "Failed to push down offer transport description."; -const char kPushDownPranswerTDFailed[] = - "Failed to push down pranswer transport description."; -const char kPushDownAnswerTDFailed[] = - "Failed to push down answer transport description."; +const char kSessionErrorDesc[] = "Session error description: "; // Compares |answer| against |offer|. Comparision is done // for number of m-lines in answer against offer. If matches true will be @@ -245,39 +238,60 @@ static bool GetTrackIdBySsrc(const SessionDescription* session_description, return false; } -static bool BadSdp(const std::string& desc, std::string* err_desc) { +static bool BadSdp(const std::string& source, + const std::string& type, + const std::string& reason, + std::string* err_desc) { + std::ostringstream desc; + desc << "Failed to set " << source << " " << type << " sdp: " << reason; + if (err_desc) { - *err_desc = desc; + *err_desc = desc.str(); } - LOG(LS_ERROR) << desc; + LOG(LS_ERROR) << desc.str(); return false; } -static bool BadLocalSdp(const std::string& desc, std::string* err_desc) { - std::string set_local_sdp_failed = kSetLocalSdpFailed; - set_local_sdp_failed.append(desc); - return BadSdp(set_local_sdp_failed, err_desc); -} - -static bool BadRemoteSdp(const std::string& desc, std::string* err_desc) { - std::string set_remote_sdp_failed = kSetRemoteSdpFailed; - set_remote_sdp_failed.append(desc); - return BadSdp(set_remote_sdp_failed, err_desc); -} - static bool BadSdp(cricket::ContentSource source, - const std::string& desc, std::string* err_desc) { + const std::string& type, + const std::string& reason, + std::string* err_desc) { if (source == cricket::CS_LOCAL) { - return BadLocalSdp(desc, err_desc); + return BadSdp("local", type, reason, err_desc); } else { - return BadRemoteSdp(desc, err_desc); + return BadSdp("remote", type, reason, err_desc); } } -static std::string SessionErrorMsg(cricket::BaseSession::Error error) { - std::ostringstream desc; - desc << kSessionError << error; - return desc.str(); +static bool BadLocalSdp(const std::string& type, + const std::string& reason, + std::string* err_desc) { + return BadSdp(cricket::CS_LOCAL, type, reason, err_desc); +} + +static bool BadRemoteSdp(const std::string& type, + const std::string& reason, + std::string* err_desc) { + return BadSdp(cricket::CS_REMOTE, type, reason, err_desc); +} + +static bool BadOfferSdp(cricket::ContentSource source, + const std::string& reason, + std::string* err_desc) { + return BadSdp(source, SessionDescriptionInterface::kOffer, reason, err_desc); +} + +static bool BadPranswerSdp(cricket::ContentSource source, + const std::string& reason, + std::string* err_desc) { + return BadSdp(source, SessionDescriptionInterface::kPrAnswer, + reason, err_desc); +} + +static bool BadAnswerSdp(cricket::ContentSource source, + const std::string& reason, + std::string* err_desc) { + return BadSdp(source, SessionDescriptionInterface::kAnswer, reason, err_desc); } #define GET_STRING_OF_STATE(state) \ @@ -311,20 +325,20 @@ static std::string GetStateString(cricket::BaseSession::State state) { return result; } -#define GET_STRING_OF_ERROR(err) \ +#define GET_STRING_OF_ERROR_CODE(err) \ case cricket::BaseSession::err: \ result = #err; \ break; -static std::string GetErrorString(cricket::BaseSession::Error err) { +static std::string GetErrorCodeString(cricket::BaseSession::Error err) { std::string result; switch (err) { - GET_STRING_OF_ERROR(ERROR_NONE) - GET_STRING_OF_ERROR(ERROR_TIME) - GET_STRING_OF_ERROR(ERROR_RESPONSE) - GET_STRING_OF_ERROR(ERROR_NETWORK) - GET_STRING_OF_ERROR(ERROR_CONTENT) - GET_STRING_OF_ERROR(ERROR_TRANSPORT) + GET_STRING_OF_ERROR_CODE(ERROR_NONE) + GET_STRING_OF_ERROR_CODE(ERROR_TIME) + GET_STRING_OF_ERROR_CODE(ERROR_RESPONSE) + GET_STRING_OF_ERROR_CODE(ERROR_NETWORK) + GET_STRING_OF_ERROR_CODE(ERROR_CONTENT) + GET_STRING_OF_ERROR_CODE(ERROR_TRANSPORT) default: ASSERT(false); break; @@ -332,12 +346,15 @@ static std::string GetErrorString(cricket::BaseSession::Error err) { return result; } -static bool SetSessionStateFailed(cricket::ContentSource source, - cricket::BaseSession::Error err, - std::string* err_desc) { - std::string set_state_err = kUpdateStateFailed; - set_state_err.append(GetErrorString(err)); - return BadSdp(source, set_state_err, err_desc); +static std::string MakeErrorString(const std::string& error, + const std::string& desc) { + std::ostringstream ret; + ret << error << " " << desc; + return ret.str(); +} + +static std::string MakeTdErrorString(const std::string& desc) { + return MakeErrorString(kPushDownTDFailed, desc); } // Help class used to remember if a a remote peer has requested ice restart by @@ -604,15 +621,14 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, if (action == kOffer && !CreateChannels(local_desc_->description())) { // TODO(mallinath) - Handle CreateChannel failure, as new local description // is applied. Restore back to old description. - return BadLocalSdp(kCreateChannelFailed, err_desc); + return BadLocalSdp(desc->type(), kCreateChannelFailed, err_desc); } // Remove channel and transport proxies, if MediaContentDescription is // rejected. RemoveUnusedChannelsAndTransports(local_desc_->description()); - if (!UpdateSessionState(action, cricket::CS_LOCAL, - local_desc_->description(), err_desc)) { + if (!UpdateSessionState(action, cricket::CS_LOCAL, err_desc)) { return false; } // Kick starting the ice candidates allocation. @@ -627,7 +643,7 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, mediastream_signaling_->OnDtlsRoleReadyForSctp(role); } if (error() != cricket::BaseSession::ERROR_NONE) { - return BadLocalSdp(SessionErrorMsg(error()), err_desc); + return BadLocalSdp(desc->type(), GetSessionErrorMsg(), err_desc); } return true; } @@ -647,7 +663,7 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, if (action == kOffer && !CreateChannels(desc->description())) { // TODO(mallinath) - Handle CreateChannel failure, as new local description // is applied. Restore back to old description. - return BadRemoteSdp(kCreateChannelFailed, err_desc); + return BadRemoteSdp(desc->type(), kCreateChannelFailed, err_desc); } // Remove channel and transport proxies, if MediaContentDescription is @@ -657,15 +673,14 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, // NOTE: Candidates allocation will be initiated only when SetLocalDescription // is called. set_remote_description(desc->description()->Copy()); - if (!UpdateSessionState(action, cricket::CS_REMOTE, - desc->description(), err_desc)) { + if (!UpdateSessionState(action, cricket::CS_REMOTE, err_desc)) { return false; } // Update remote MediaStreams. mediastream_signaling_->OnRemoteDescriptionChanged(desc); if (local_description() && !UseCandidatesInSessionDescription(desc)) { - return BadRemoteSdp(kInvalidCandidates, err_desc); + return BadRemoteSdp(desc->type(), kInvalidCandidates, err_desc); } // Copy all saved candidates. @@ -685,47 +700,47 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, } if (error() != cricket::BaseSession::ERROR_NONE) { - return BadRemoteSdp(SessionErrorMsg(error()), err_desc); + return BadRemoteSdp(desc->type(), GetSessionErrorMsg(), err_desc); } return true; } bool WebRtcSession::UpdateSessionState( Action action, cricket::ContentSource source, - const cricket::SessionDescription* desc, std::string* err_desc) { // If there's already a pending error then no state transition should happen. // But all call-sites should be verifying this before calling us! ASSERT(error() == cricket::BaseSession::ERROR_NONE); + std::string td_err; if (action == kOffer) { - if (!PushdownTransportDescription(source, cricket::CA_OFFER)) { - return BadSdp(source, kPushDownOfferTDFailed, err_desc); + if (!PushdownTransportDescription(source, cricket::CA_OFFER, &td_err)) { + return BadOfferSdp(source, MakeTdErrorString(td_err), err_desc); } SetState(source == cricket::CS_LOCAL ? STATE_SENTINITIATE : STATE_RECEIVEDINITIATE); if (error() != cricket::BaseSession::ERROR_NONE) { - return SetSessionStateFailed(source, error(), err_desc); + return BadOfferSdp(source, GetSessionErrorMsg(), err_desc); } } else if (action == kPrAnswer) { - if (!PushdownTransportDescription(source, cricket::CA_PRANSWER)) { - return BadSdp(source, kPushDownPranswerTDFailed, err_desc); + if (!PushdownTransportDescription(source, cricket::CA_PRANSWER, &td_err)) { + return BadPranswerSdp(source, MakeTdErrorString(td_err), err_desc); } EnableChannels(); SetState(source == cricket::CS_LOCAL ? STATE_SENTPRACCEPT : STATE_RECEIVEDPRACCEPT); if (error() != cricket::BaseSession::ERROR_NONE) { - return SetSessionStateFailed(source, error(), err_desc); + return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc); } } else if (action == kAnswer) { - if (!PushdownTransportDescription(source, cricket::CA_ANSWER)) { - return BadSdp(source, kPushDownAnswerTDFailed, err_desc); + if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) { + return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc); } MaybeEnableMuxingSupport(); EnableChannels(); SetState(source == cricket::CS_LOCAL ? STATE_SENTACCEPT : STATE_RECEIVEDACCEPT); if (error() != cricket::BaseSession::ERROR_NONE) { - return SetSessionStateFailed(source, error(), err_desc); + return BadAnswerSdp(source, GetSessionErrorMsg(), err_desc); } } return true; @@ -801,11 +816,9 @@ bool WebRtcSession::GetRemoteTrackId(uint32 ssrc, std::string* track_id) { BaseSession::remote_description(), ssrc, track_id); } -std::string WebRtcSession::BadStateErrMsg( - const std::string& type, State state) { +std::string WebRtcSession::BadStateErrMsg(State state) { std::ostringstream desc; - desc << "Called with type in wrong state, " - << "type: " << type << " state: " << GetStateString(state); + desc << "Called in wrong state: " << GetStateString(state); return desc.str(); } @@ -1447,40 +1460,40 @@ bool WebRtcSession::HasRtcpMuxEnabled( bool WebRtcSession::ValidateSessionDescription( const SessionDescriptionInterface* sdesc, - cricket::ContentSource source, std::string* error_desc) { - + cricket::ContentSource source, std::string* err_desc) { + std::string type; if (error() != cricket::BaseSession::ERROR_NONE) { - return BadSdp(source, SessionErrorMsg(error()), error_desc); + return BadSdp(source, type, GetSessionErrorMsg(), err_desc); } if (!sdesc || !sdesc->description()) { - return BadSdp(source, kInvalidSdp, error_desc); + return BadSdp(source, type, kInvalidSdp, err_desc); } - std::string type = sdesc->type(); + type = sdesc->type(); Action action = GetAction(sdesc->type()); if (source == cricket::CS_LOCAL) { if (!ExpectSetLocalDescription(action)) - return BadSdp(source, BadStateErrMsg(type, state()), error_desc); + return BadLocalSdp(type, BadStateErrMsg(state()), err_desc); } else { if (!ExpectSetRemoteDescription(action)) - return BadSdp(source, BadStateErrMsg(type, state()), error_desc); + return BadRemoteSdp(type, BadStateErrMsg(state()), err_desc); } // Verify crypto settings. std::string crypto_error; if (webrtc_session_desc_factory_->Secure() == cricket::SEC_REQUIRED && !VerifyCrypto(sdesc->description(), dtls_enabled_, &crypto_error)) { - return BadSdp(source, crypto_error, error_desc); + return BadSdp(source, type, crypto_error, err_desc); } // Verify ice-ufrag and ice-pwd. if (!VerifyIceUfragPwdPresent(sdesc->description())) { - return BadSdp(source, kSdpWithoutIceUfragPwd, error_desc); + return BadSdp(source, type, kSdpWithoutIceUfragPwd, err_desc); } if (!ValidateBundleSettings(sdesc->description())) { - return BadSdp(source, kBundleWithoutRtcpMux, error_desc); + return BadSdp(source, type, kBundleWithoutRtcpMux, err_desc); } // Verify m-lines in Answer when compared against Offer. @@ -1489,7 +1502,7 @@ bool WebRtcSession::ValidateSessionDescription( (source == cricket::CS_LOCAL) ? remote_description()->description() : local_description()->description(); if (!VerifyMediaDescriptions(sdesc->description(), offer_desc)) { - return BadSdp(source, kMlineMismatch, error_desc); + return BadAnswerSdp(source, kMlineMismatch, err_desc); } } @@ -1526,4 +1539,11 @@ bool WebRtcSession::ExpectSetRemoteDescription(Action action) { (action == kPrAnswer && state() == STATE_RECEIVEDPRACCEPT)); } +std::string WebRtcSession::GetSessionErrorMsg() { + std::ostringstream desc; + desc << kSessionError << GetErrorCodeString(error()) << ". "; + desc << kSessionErrorDesc << error_desc() << "."; + return desc.str(); +} + } // namespace webrtc diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 6b5887798c..384ac47241 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -57,21 +57,17 @@ class IceRestartAnswerLatch; class MediaStreamSignaling; class WebRtcSessionDescriptionFactory; -extern const char kSetLocalSdpFailed[]; -extern const char kSetRemoteSdpFailed[]; -extern const char kCreateChannelFailed[]; extern const char kBundleWithoutRtcpMux[]; +extern const char kCreateChannelFailed[]; extern const char kInvalidCandidates[]; extern const char kInvalidSdp[]; extern const char kMlineMismatch[]; +extern const char kPushDownTDFailed[]; extern const char kSdpWithoutCrypto[]; -extern const char kSdpWithoutSdesAndDtlsDisabled[]; extern const char kSdpWithoutIceUfragPwd[]; +extern const char kSdpWithoutSdesAndDtlsDisabled[]; extern const char kSessionError[]; -extern const char kUpdateStateFailed[]; -extern const char kPushDownOfferTDFailed[]; -extern const char kPushDownPranswerTDFailed[]; -extern const char kPushDownAnswerTDFailed[]; +extern const char kSessionErrorDesc[]; // ICE state callback interface. class IceObserver { @@ -226,7 +222,6 @@ class WebRtcSession : public cricket::BaseSession, // candidates allocation. bool StartCandidatesAllocation(); bool UpdateSessionState(Action action, cricket::ContentSource source, - const cricket::SessionDescription* desc, std::string* err_desc); static Action GetAction(const std::string& type); @@ -285,7 +280,7 @@ class WebRtcSession : public cricket::BaseSession, bool GetLocalTrackId(uint32 ssrc, std::string* track_id); bool GetRemoteTrackId(uint32 ssrc, std::string* track_id); - std::string BadStateErrMsg(const std::string& type, State state); + std::string BadStateErrMsg(State state); void SetIceConnectionState(PeerConnectionInterface::IceConnectionState state); bool ValidateBundleSettings(const cricket::SessionDescription* desc); @@ -293,7 +288,7 @@ class WebRtcSession : public cricket::BaseSession, // Below methods are helper methods which verifies SDP. bool ValidateSessionDescription(const SessionDescriptionInterface* sdesc, cricket::ContentSource source, - std::string* error_desc); + std::string* err_desc); // Check if a call to SetLocalDescription is acceptable with |action|. bool ExpectSetLocalDescription(Action action); @@ -303,6 +298,8 @@ class WebRtcSession : public cricket::BaseSession, bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc, Action action); + std::string GetSessionErrorMsg(); + talk_base::scoped_ptr<cricket::VoiceChannel> voice_channel_; talk_base::scoped_ptr<cricket::VideoChannel> video_channel_; talk_base::scoped_ptr<cricket::DataChannel> data_channel_; diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 3fa47752aa..6d3a58131e 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -87,15 +87,15 @@ using webrtc::SessionDescriptionInterface; using webrtc::StreamCollection; using webrtc::WebRtcSession; using webrtc::kBundleWithoutRtcpMux; +using webrtc::kCreateChannelFailed; +using webrtc::kInvalidSdp; using webrtc::kMlineMismatch; -using webrtc::kPushDownAnswerTDFailed; -using webrtc::kPushDownPranswerTDFailed; +using webrtc::kPushDownTDFailed; using webrtc::kSdpWithoutCrypto; using webrtc::kSdpWithoutIceUfragPwd; using webrtc::kSdpWithoutSdesAndDtlsDisabled; using webrtc::kSessionError; -using webrtc::kSetLocalSdpFailed; -using webrtc::kSetRemoteSdpFailed; +using webrtc::kSessionErrorDesc; static const int kClientAddrPort = 0; static const char kClientAddrHost1[] = "11.11.11.11"; @@ -475,8 +475,8 @@ class WebRtcSessionTest : public testing::Test { CreateRemoteOffer(options, cricket::SEC_DISABLED)); ASSERT_TRUE(offer != NULL); VerifyNoCryptoParams(offer->description(), false); - SetRemoteDescriptionExpectError("Called with a SDP without crypto enabled", - offer); + SetRemoteDescriptionOfferExpectError( + "Called with a SDP without crypto enabled", offer); const webrtc::SessionDescriptionInterface* answer = CreateAnswer(NULL); // Answer should be NULL as no crypto params in offer. ASSERT_TRUE(answer == NULL); @@ -567,13 +567,26 @@ class WebRtcSessionTest : public testing::Test { SetLocalDescriptionWithoutError(desc); EXPECT_EQ(expected_state, session_->state()); } - void SetLocalDescriptionExpectError(const std::string& expected_error, + void SetLocalDescriptionExpectError(const std::string& action, + const std::string& expected_error, SessionDescriptionInterface* desc) { std::string error; EXPECT_FALSE(session_->SetLocalDescription(desc, &error)); - EXPECT_NE(std::string::npos, error.find(kSetLocalSdpFailed)); + std::string sdp_type = "local "; + sdp_type.append(action); + EXPECT_NE(std::string::npos, error.find(sdp_type)); EXPECT_NE(std::string::npos, error.find(expected_error)); } + void SetLocalDescriptionOfferExpectError(const std::string& expected_error, + SessionDescriptionInterface* desc) { + SetLocalDescriptionExpectError(SessionDescriptionInterface::kOffer, + expected_error, desc); + } + void SetLocalDescriptionAnswerExpectError(const std::string& expected_error, + SessionDescriptionInterface* desc) { + SetLocalDescriptionExpectError(SessionDescriptionInterface::kAnswer, + expected_error, desc); + } void SetRemoteDescriptionWithoutError(SessionDescriptionInterface* desc) { EXPECT_TRUE(session_->SetRemoteDescription(desc, NULL)); } @@ -582,13 +595,31 @@ class WebRtcSessionTest : public testing::Test { SetRemoteDescriptionWithoutError(desc); EXPECT_EQ(expected_state, session_->state()); } - void SetRemoteDescriptionExpectError(const std::string& expected_error, + void SetRemoteDescriptionExpectError(const std::string& action, + const std::string& expected_error, SessionDescriptionInterface* desc) { std::string error; EXPECT_FALSE(session_->SetRemoteDescription(desc, &error)); - EXPECT_NE(std::string::npos, error.find(kSetRemoteSdpFailed)); + std::string sdp_type = "remote "; + sdp_type.append(action); + EXPECT_NE(std::string::npos, error.find(sdp_type)); EXPECT_NE(std::string::npos, error.find(expected_error)); } + void SetRemoteDescriptionOfferExpectError( + const std::string& expected_error, SessionDescriptionInterface* desc) { + SetRemoteDescriptionExpectError(SessionDescriptionInterface::kOffer, + expected_error, desc); + } + void SetRemoteDescriptionPranswerExpectError( + const std::string& expected_error, SessionDescriptionInterface* desc) { + SetRemoteDescriptionExpectError(SessionDescriptionInterface::kPrAnswer, + expected_error, desc); + } + void SetRemoteDescriptionAnswerExpectError( + const std::string& expected_error, SessionDescriptionInterface* desc) { + SetRemoteDescriptionExpectError(SessionDescriptionInterface::kAnswer, + expected_error, desc); + } void CreateCryptoOfferAndNonCryptoAnswer(SessionDescriptionInterface** offer, SessionDescriptionInterface** nocrypto_answer) { @@ -998,6 +1029,15 @@ TEST_F(WebRtcSessionTest, TestStunError) { EXPECT_EQ(6u, observer_.mline_1_candidates_.size()); } +TEST_F(WebRtcSessionTest, SetSdpFailedOnInvalidSdp) { + Init(NULL); + SessionDescriptionInterface* offer = NULL; + // Since |offer| is NULL, there's no way to tell if it's an offer or answer. + std::string unknown_action; + SetLocalDescriptionExpectError(unknown_action, kInvalidSdp, offer); + SetRemoteDescriptionExpectError(unknown_action, kInvalidSdp, offer); +} + // Test creating offers and receive answers and make sure the // media engine creates the expected send and receive streams. TEST_F(WebRtcSessionTest, TestCreateOfferReceiveAnswer) { @@ -1109,6 +1149,20 @@ TEST_F(WebRtcSessionTest, TestReceiveOfferCreateAnswer) { EXPECT_EQ(0u, voice_channel_->send_streams().size()); } +TEST_F(WebRtcSessionTest, SetLocalSdpFailedOnCreateChannel) { + Init(NULL); + media_engine_->set_fail_create_channel(true); + + SessionDescriptionInterface* offer = CreateOffer(NULL); + ASSERT_TRUE(offer != NULL); + // SetRemoteDescription and SetLocalDescription will take the ownership of + // the offer. + SetRemoteDescriptionOfferExpectError(kCreateChannelFailed, offer); + offer = CreateOffer(NULL); + ASSERT_TRUE(offer != NULL); + SetLocalDescriptionOfferExpectError(kCreateChannelFailed, offer); +} + // Test we will return fail when apply an offer that doesn't have // crypto enabled. TEST_F(WebRtcSessionTest, SetNonCryptoOffer) { @@ -1121,10 +1175,10 @@ TEST_F(WebRtcSessionTest, SetNonCryptoOffer) { VerifyNoCryptoParams(offer->description(), false); // SetRemoteDescription and SetLocalDescription will take the ownership of // the offer. - SetRemoteDescriptionExpectError(kSdpWithoutCrypto, offer); + SetRemoteDescriptionOfferExpectError(kSdpWithoutCrypto, offer); offer = CreateRemoteOffer(options, cricket::SEC_DISABLED); ASSERT_TRUE(offer != NULL); - SetLocalDescriptionExpectError(kSdpWithoutCrypto, offer); + SetLocalDescriptionOfferExpectError(kSdpWithoutCrypto, offer); } // Test we will return fail when apply an answer that doesn't have @@ -1137,7 +1191,7 @@ TEST_F(WebRtcSessionTest, SetLocalNonCryptoAnswer) { // SetRemoteDescription and SetLocalDescription will take the ownership of // the offer. SetRemoteDescriptionWithoutError(offer); - SetLocalDescriptionExpectError(kSdpWithoutCrypto, answer); + SetLocalDescriptionAnswerExpectError(kSdpWithoutCrypto, answer); } // Test we will return fail when apply an answer that doesn't have @@ -1150,7 +1204,7 @@ TEST_F(WebRtcSessionTest, SetRemoteNonCryptoAnswer) { // SetRemoteDescription and SetLocalDescription will take the ownership of // the offer. SetLocalDescriptionWithoutError(offer); - SetRemoteDescriptionExpectError(kSdpWithoutCrypto, answer); + SetRemoteDescriptionAnswerExpectError(kSdpWithoutCrypto, answer); } // Test that we can create and set an offer with a DTLS fingerprint. @@ -1245,9 +1299,8 @@ TEST_F(WebRtcSessionTest, TestSetLocalAndRemoteOffer) { SessionDescriptionInterface* offer = CreateOffer(NULL); SetLocalDescriptionWithoutError(offer); offer = CreateOffer(NULL); - SetRemoteDescriptionExpectError( - "Called with type in wrong state, type: offer state: STATE_SENTINITIATE", - offer); + SetRemoteDescriptionOfferExpectError( + "Called in wrong state: STATE_SENTINITIATE", offer); } TEST_F(WebRtcSessionTest, TestSetRemoteAndLocalOffer) { @@ -1256,10 +1309,8 @@ TEST_F(WebRtcSessionTest, TestSetRemoteAndLocalOffer) { SessionDescriptionInterface* offer = CreateOffer(NULL); SetRemoteDescriptionWithoutError(offer); offer = CreateOffer(NULL); - SetLocalDescriptionExpectError( - "Called with type in wrong state, type: " - "offer state: STATE_RECEIVEDINITIATE", - offer); + SetLocalDescriptionOfferExpectError( + "Called in wrong state: STATE_RECEIVEDINITIATE", offer); } TEST_F(WebRtcSessionTest, TestSetLocalPrAnswer) { @@ -1319,9 +1370,8 @@ TEST_F(WebRtcSessionTest, TestSetLocalAnswerWithoutOffer) { CreateOffer(NULL)); SessionDescriptionInterface* answer = CreateRemoteAnswer(offer.get()); - SetLocalDescriptionExpectError( - "Called with type in wrong state, type: answer state: STATE_INIT", - answer); + SetLocalDescriptionAnswerExpectError("Called in wrong state: STATE_INIT", + answer); } TEST_F(WebRtcSessionTest, TestSetRemoteAnswerWithoutOffer) { @@ -1331,9 +1381,8 @@ TEST_F(WebRtcSessionTest, TestSetRemoteAnswerWithoutOffer) { CreateOffer(NULL)); SessionDescriptionInterface* answer = CreateRemoteAnswer(offer.get()); - SetRemoteDescriptionExpectError( - "Called with type in wrong state, type: answer state: STATE_INIT", - answer); + SetRemoteDescriptionAnswerExpectError( + "Called in wrong state: STATE_INIT", answer); } TEST_F(WebRtcSessionTest, TestAddRemoteCandidate) { @@ -1979,7 +2028,7 @@ TEST_F(WebRtcSessionTest, TestSetLocalDescriptionWithoutIce) { RemoveIceUfragPwdLines(offer.get(), &sdp); SessionDescriptionInterface* modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); - SetLocalDescriptionExpectError(kSdpWithoutIceUfragPwd, modified_offer); + SetLocalDescriptionOfferExpectError(kSdpWithoutIceUfragPwd, modified_offer); } // This test verifies that setRemoteDescription fails if @@ -1991,7 +2040,7 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithoutIce) { RemoveIceUfragPwdLines(offer.get(), &sdp); SessionDescriptionInterface* modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); - SetRemoteDescriptionExpectError(kSdpWithoutIceUfragPwd, modified_offer); + SetRemoteDescriptionOfferExpectError(kSdpWithoutIceUfragPwd, modified_offer); } TEST_F(WebRtcSessionTest, VerifyBundleFlagInPA) { @@ -2071,11 +2120,11 @@ TEST_F(WebRtcSessionTest, TestDisabledRtcpMuxWithBundleEnabled) { JsepSessionDescription *local_offer = new JsepSessionDescription(JsepSessionDescription::kOffer); EXPECT_TRUE((local_offer)->Initialize(offer_str, NULL)); - SetLocalDescriptionExpectError(kBundleWithoutRtcpMux, local_offer); + SetLocalDescriptionOfferExpectError(kBundleWithoutRtcpMux, local_offer); JsepSessionDescription *remote_offer = new JsepSessionDescription(JsepSessionDescription::kOffer); EXPECT_TRUE((remote_offer)->Initialize(offer_str, NULL)); - SetRemoteDescriptionExpectError(kBundleWithoutRtcpMux, remote_offer); + SetRemoteDescriptionOfferExpectError(kBundleWithoutRtcpMux, remote_offer); // Trying unmodified SDP. SetLocalDescriptionWithoutError(offer); } @@ -2319,13 +2368,13 @@ TEST_F(WebRtcSessionTest, TestIceOfferGIceOnlyAnswer) { SessionDescriptionInterface* pranswer_with_gice = CreateSessionDescription(JsepSessionDescription::kPrAnswer, original_offer_sdp, NULL); - SetRemoteDescriptionExpectError(kPushDownPranswerTDFailed, - pranswer_with_gice); + SetRemoteDescriptionPranswerExpectError(kPushDownTDFailed, + pranswer_with_gice); SessionDescriptionInterface* answer_with_gice = CreateSessionDescription(JsepSessionDescription::kAnswer, original_offer_sdp, NULL); - SetRemoteDescriptionExpectError(kPushDownAnswerTDFailed, - answer_with_gice); + SetRemoteDescriptionAnswerExpectError(kPushDownTDFailed, + answer_with_gice); } // Verifing local offer and remote answer have matching m-lines as per RFC 3264. @@ -2345,7 +2394,7 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) { EXPECT_TRUE(modified_answer->Initialize(answer_copy, answer->session_id(), answer->session_version())); - SetRemoteDescriptionExpectError(kMlineMismatch, modified_answer); + SetRemoteDescriptionAnswerExpectError(kMlineMismatch, modified_answer); // Modifying content names. std::string sdp; @@ -2361,7 +2410,7 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInRemoteAnswer) { SessionDescriptionInterface* modified_answer1 = CreateSessionDescription(JsepSessionDescription::kAnswer, sdp, NULL); - SetRemoteDescriptionExpectError(kMlineMismatch, modified_answer1); + SetRemoteDescriptionAnswerExpectError(kMlineMismatch, modified_answer1); SetRemoteDescriptionWithoutError(answer.release()); } @@ -2383,7 +2432,7 @@ TEST_F(WebRtcSessionTest, TestIncorrectMLinesInLocalAnswer) { EXPECT_TRUE(modified_answer->Initialize(answer_copy, answer->session_id(), answer->session_version())); - SetLocalDescriptionExpectError(kMlineMismatch, modified_answer); + SetLocalDescriptionAnswerExpectError(kMlineMismatch, modified_answer); SetLocalDescriptionWithoutError(answer); } @@ -2537,7 +2586,7 @@ TEST_F(WebRtcSessionTest, TestSessionContentError) { mediastream_signaling_.SendAudioVideoStream2(); SessionDescriptionInterface* answer = CreateRemoteAnswer(session_->local_description()); - SetRemoteDescriptionExpectError("ERROR_CONTENT", answer); + SetRemoteDescriptionAnswerExpectError("ERROR_CONTENT", answer); } // Runs the loopback call test with BUNDLE and STUN disabled. @@ -2550,20 +2599,27 @@ TEST_F(WebRtcSessionTest, TestIceStatesBasic) { TestLoopbackCall(); } -// Regression-test for a crash which should have been an error. -TEST_F(WebRtcSessionTest, TestNoStateTransitionPendingError) { +TEST_F(WebRtcSessionTest, SetSdpFailedOnSessionError) { Init(NULL); cricket::MediaSessionOptions options; options.has_audio = true; options.has_video = true; - session_->SetError(cricket::BaseSession::ERROR_CONTENT); + cricket::BaseSession::Error error_code = cricket::BaseSession::ERROR_CONTENT; + std::string error_code_str = "ERROR_CONTENT"; + std::string error_desc = "Fake session error description."; + session_->SetError(error_code, error_desc); + SessionDescriptionInterface* offer = CreateRemoteOffer(options); SessionDescriptionInterface* answer = CreateRemoteAnswer(offer, options); - SetRemoteDescriptionExpectError(kSessionError, offer); - SetLocalDescriptionExpectError(kSessionError, answer); - // Not crashing is our success. + + std::string action; + std::ostringstream session_error_msg; + session_error_msg << kSessionError << error_code_str << ". "; + session_error_msg << kSessionErrorDesc << error_desc << "."; + SetRemoteDescriptionExpectError(action, session_error_msg.str(), offer); + SetLocalDescriptionExpectError(action, session_error_msg.str(), answer); } TEST_F(WebRtcSessionTest, TestRtpDataChannel) { @@ -2803,8 +2859,8 @@ TEST_F(WebRtcSessionTest, TestSetRemoteOfferFailIfDtlsDisabledAndNoCrypto) { audio->description.identity_fingerprint.reset( talk_base::SSLFingerprint::CreateFromRfc4572( talk_base::DIGEST_SHA_256, kFakeDtlsFingerprint)); - SetRemoteDescriptionExpectError(kSdpWithoutSdesAndDtlsDisabled, - offer); + SetRemoteDescriptionOfferExpectError(kSdpWithoutSdesAndDtlsDisabled, + offer); } // This test verifies DSCP is properly applied on the media channels. diff --git a/talk/base/sslfingerprint.cc b/talk/base/sslfingerprint.cc new file mode 100644 index 0000000000..dfd5551abc --- /dev/null +++ b/talk/base/sslfingerprint.cc @@ -0,0 +1,114 @@ +/* + * libjingle + * Copyright 2012, Google Inc. + * Copyright 2012, RTFM Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO + * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "talk/base/sslfingerprint.h" + +#include <ctype.h> +#include <string> + +#include "talk/base/helpers.h" +#include "talk/base/messagedigest.h" +#include "talk/base/stringencode.h" + +namespace talk_base { + +SSLFingerprint* SSLFingerprint::Create( + const std::string& algorithm, const talk_base::SSLIdentity* identity) { + if (!identity) { + return NULL; + } + + return Create(algorithm, &(identity->certificate())); +} + +SSLFingerprint* SSLFingerprint::Create( + const std::string& algorithm, const talk_base::SSLCertificate* cert) { + uint8 digest_val[64]; + size_t digest_len; + bool ret = cert->ComputeDigest( + algorithm, digest_val, sizeof(digest_val), &digest_len); + if (!ret) { + return NULL; + } + + return new SSLFingerprint(algorithm, digest_val, digest_len); +} + +SSLFingerprint* SSLFingerprint::CreateFromRfc4572( + const std::string& algorithm, const std::string& fingerprint) { + if (algorithm.empty() || !talk_base::IsFips180DigestAlgorithm(algorithm)) + return NULL; + + if (fingerprint.empty()) + return NULL; + + size_t value_len; + char value[talk_base::MessageDigest::kMaxSize]; + value_len = talk_base::hex_decode_with_delimiter(value, sizeof(value), + fingerprint.c_str(), + fingerprint.length(), + ':'); + if (!value_len) + return NULL; + + return new SSLFingerprint(algorithm, + reinterpret_cast<uint8*>(value), + value_len); +} + +SSLFingerprint::SSLFingerprint( + const std::string& algorithm, const uint8* digest_in, size_t digest_len) + : algorithm(algorithm) { + digest.SetData(digest_in, digest_len); +} + +SSLFingerprint::SSLFingerprint(const SSLFingerprint& from) + : algorithm(from.algorithm), digest(from.digest) {} + +bool SSLFingerprint::operator==(const SSLFingerprint& other) const { + return algorithm == other.algorithm && + digest == other.digest; +} + +std::string SSLFingerprint::GetRfc4572Fingerprint() const { + std::string fingerprint = + talk_base::hex_encode_with_delimiter( + digest.data(), digest.length(), ':'); + std::transform(fingerprint.begin(), fingerprint.end(), + fingerprint.begin(), ::toupper); + return fingerprint; +} + +std::string SSLFingerprint::ToString() { + std::string fp_str = algorithm; + fp_str.append(" "); + fp_str.append(GetRfc4572Fingerprint()); + return fp_str; +} + +} // namespace talk_base diff --git a/talk/base/sslfingerprint.h b/talk/base/sslfingerprint.h index 0dfcdd95e6..a803d2129a 100644 --- a/talk/base/sslfingerprint.h +++ b/talk/base/sslfingerprint.h @@ -29,81 +29,35 @@ #ifndef TALK_BASE_SSLFINGERPRINT_H_ #define TALK_BASE_SSLFINGERPRINT_H_ -#include <ctype.h> #include <string> #include "talk/base/buffer.h" -#include "talk/base/helpers.h" -#include "talk/base/messagedigest.h" #include "talk/base/sslidentity.h" -#include "talk/base/stringencode.h" namespace talk_base { +class SSLCertificate; + struct SSLFingerprint { static SSLFingerprint* Create(const std::string& algorithm, - const talk_base::SSLIdentity* identity) { - if (!identity) { - return NULL; - } - - return Create(algorithm, &(identity->certificate())); - } + const talk_base::SSLIdentity* identity); static SSLFingerprint* Create(const std::string& algorithm, - const talk_base::SSLCertificate* cert) { - uint8 digest_val[64]; - size_t digest_len; - bool ret = cert->ComputeDigest( - algorithm, digest_val, sizeof(digest_val), &digest_len); - if (!ret) { - return NULL; - } - - return new SSLFingerprint(algorithm, digest_val, digest_len); - } + const talk_base::SSLCertificate* cert); static SSLFingerprint* CreateFromRfc4572(const std::string& algorithm, - const std::string& fingerprint) { - if (algorithm.empty() || !talk_base::IsFips180DigestAlgorithm(algorithm)) - return NULL; - - if (fingerprint.empty()) - return NULL; - - size_t value_len; - char value[talk_base::MessageDigest::kMaxSize]; - value_len = talk_base::hex_decode_with_delimiter(value, sizeof(value), - fingerprint.c_str(), - fingerprint.length(), - ':'); - if (!value_len) - return NULL; - - return new SSLFingerprint(algorithm, - reinterpret_cast<uint8*>(value), - value_len); - } + const std::string& fingerprint); SSLFingerprint(const std::string& algorithm, const uint8* digest_in, - size_t digest_len) : algorithm(algorithm) { - digest.SetData(digest_in, digest_len); - } - SSLFingerprint(const SSLFingerprint& from) - : algorithm(from.algorithm), digest(from.digest) {} - bool operator==(const SSLFingerprint& other) const { - return algorithm == other.algorithm && - digest == other.digest; - } - - std::string GetRfc4572Fingerprint() const { - std::string fingerprint = - talk_base::hex_encode_with_delimiter( - digest.data(), digest.length(), ':'); - std::transform(fingerprint.begin(), fingerprint.end(), - fingerprint.begin(), ::toupper); - return fingerprint; - } + size_t digest_len); + + SSLFingerprint(const SSLFingerprint& from); + + bool operator==(const SSLFingerprint& other) const; + + std::string GetRfc4572Fingerprint() const; + + std::string ToString(); std::string algorithm; talk_base::Buffer digest; diff --git a/talk/examples/chat/chatapp.cc b/talk/examples/chat/chatapp.cc index 59b1c69fb9..972086aa6a 100644 --- a/talk/examples/chat/chatapp.cc +++ b/talk/examples/chat/chatapp.cc @@ -26,6 +26,7 @@ */ #include "talk/examples/chat/chatapp.h" +#include "talk/base/stringencode.h" #include "talk/examples/chat/consoletask.h" #include "talk/examples/chat/textchatsendtask.h" #include "talk/examples/chat/textchatreceivetask.h" diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp index 0b434356e4..e77e48a3e3 100755 --- a/talk/libjingle.gyp +++ b/talk/libjingle.gyp @@ -410,6 +410,7 @@ 'base/ssladapter.cc', 'base/ssladapter.h', 'base/sslconfig.h', + 'base/sslfingerprint.cc', 'base/sslfingerprint.h', 'base/sslidentity.cc', 'base/sslidentity.h', diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index d71c660064..9ffe07453b 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -281,7 +281,8 @@ class FakeVoiceMediaChannel : public RtpHelper<VoiceMediaChannel> { } return set_sending(flag != SEND_NOTHING); } - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { return true; } virtual bool AddRecvStream(const StreamParams& sp) { if (!RtpHelper<VoiceMediaChannel>::AddRecvStream(sp)) return false; @@ -446,7 +447,9 @@ class FakeVideoMediaChannel : public RtpHelper<VideoMediaChannel> { explicit FakeVideoMediaChannel(FakeVideoEngine* engine) : engine_(engine), sent_intra_frame_(false), - requested_intra_frame_(false) {} + requested_intra_frame_(false), + start_bps_(-1), + max_bps_(-1) {} ~FakeVideoMediaChannel(); const std::vector<VideoCodec>& recv_codecs() const { return recv_codecs_; } @@ -457,6 +460,8 @@ class FakeVideoMediaChannel : public RtpHelper<VideoMediaChannel> { const std::map<uint32, VideoRenderer*>& renderers() const { return renderers_; } + int start_bps() const { return start_bps_; } + int max_bps() const { return max_bps_; } bool GetSendStreamFormat(uint32 ssrc, VideoFormat* format) { if (send_formats_.find(ssrc) == send_formats_.end()) { return false; @@ -534,7 +539,14 @@ class FakeVideoMediaChannel : public RtpHelper<VideoMediaChannel> { bool HasCapturer(uint32 ssrc) const { return capturers_.find(ssrc) != capturers_.end(); } - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { + start_bps_ = bps; + return true; + } + virtual bool SetMaxSendBandwidth(int bps) { + max_bps_ = bps; + return true; + } virtual bool AddRecvStream(const StreamParams& sp) { if (!RtpHelper<VideoMediaChannel>::AddRecvStream(sp)) return false; @@ -591,6 +603,8 @@ class FakeVideoMediaChannel : public RtpHelper<VideoMediaChannel> { bool sent_intra_frame_; bool requested_intra_frame_; VideoOptions options_; + int start_bps_; + int max_bps_; }; class FakeSoundclipMedia : public SoundclipMedia { @@ -601,12 +615,11 @@ class FakeSoundclipMedia : public SoundclipMedia { class FakeDataMediaChannel : public RtpHelper<DataMediaChannel> { public: explicit FakeDataMediaChannel(void* unused) - : auto_bandwidth_(false), send_blocked_(false), max_bps_(-1) {} + : send_blocked_(false), max_bps_(-1) {} ~FakeDataMediaChannel() {} const std::vector<DataCodec>& recv_codecs() const { return recv_codecs_; } const std::vector<DataCodec>& send_codecs() const { return send_codecs_; } const std::vector<DataCodec>& codecs() const { return send_codecs(); } - bool auto_bandwidth() const { return auto_bandwidth_; } int max_bps() const { return max_bps_; } virtual bool SetRecvCodecs(const std::vector<DataCodec>& codecs) { @@ -630,8 +643,8 @@ class FakeDataMediaChannel : public RtpHelper<DataMediaChannel> { set_playout(receive); return true; } - virtual bool SetSendBandwidth(bool autobw, int bps) { - auto_bandwidth_ = autobw; + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { max_bps_ = bps; return true; } @@ -669,7 +682,6 @@ class FakeDataMediaChannel : public RtpHelper<DataMediaChannel> { std::vector<DataCodec> send_codecs_; SendDataParams last_sent_data_params_; std::string last_sent_data_; - bool auto_bandwidth_; bool send_blocked_; int max_bps_; }; diff --git a/talk/media/base/filemediaengine.h b/talk/media/base/filemediaengine.h index 843806b76f..e7956ecfac 100644 --- a/talk/media/base/filemediaengine.h +++ b/talk/media/base/filemediaengine.h @@ -243,7 +243,8 @@ class FileVoiceChannel : public VoiceMediaChannel { virtual bool AddRecvStream(const StreamParams& sp) { return true; } virtual bool RemoveRecvStream(uint32 ssrc) { return true; } virtual bool MuteStream(uint32 ssrc, bool on) { return false; } - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { return true; } virtual bool SetOptions(const AudioOptions& options) { options_ = options; return true; @@ -311,7 +312,8 @@ class FileVideoChannel : public VideoMediaChannel { virtual bool AddRecvStream(const StreamParams& sp) { return true; } virtual bool RemoveRecvStream(uint32 ssrc) { return true; } virtual bool MuteStream(uint32 ssrc, bool on) { return false; } - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { return true; } virtual bool SetOptions(const VideoOptions& options) { options_ = options; return true; diff --git a/talk/media/base/hybridvideoengine.cc b/talk/media/base/hybridvideoengine.cc index 6863311f2d..f855417b3c 100644 --- a/talk/media/base/hybridvideoengine.cc +++ b/talk/media/base/hybridvideoengine.cc @@ -183,9 +183,12 @@ bool HybridVideoMediaChannel::SetSendRtpHeaderExtensions( active_channel_->SetSendRtpHeaderExtensions(extensions); } -bool HybridVideoMediaChannel::SetSendBandwidth(bool autobw, int bps) { - return active_channel_ && - active_channel_->SetSendBandwidth(autobw, bps); +bool HybridVideoMediaChannel::SetStartSendBandwidth(int bps) { + return active_channel_ && active_channel_->SetStartSendBandwidth(bps); +} + +bool HybridVideoMediaChannel::SetMaxSendBandwidth(int bps) { + return active_channel_ && active_channel_->SetMaxSendBandwidth(bps); } bool HybridVideoMediaChannel::SetSend(bool send) { diff --git a/talk/media/base/hybridvideoengine.h b/talk/media/base/hybridvideoengine.h index a49a1aa2c8..9f919205a9 100644 --- a/talk/media/base/hybridvideoengine.h +++ b/talk/media/base/hybridvideoengine.h @@ -75,7 +75,8 @@ class HybridVideoMediaChannel : public VideoMediaChannel { virtual bool SetSendStreamFormat(uint32 ssrc, const VideoFormat& format); virtual bool SetSendRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions); - virtual bool SetSendBandwidth(bool autobw, int bps); + virtual bool SetStartSendBandwidth(int bps); + virtual bool SetMaxSendBandwidth(int bps); virtual bool SetSend(bool send); virtual bool AddRecvStream(const StreamParams& sp); diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 9c74762260..c2ea26f1bd 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -535,8 +535,10 @@ class MediaChannel : public sigslot::has_slots<> { const std::vector<RtpHeaderExtension>& extensions) = 0; virtual bool SetSendRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions) = 0; - // Sets the rate control to use when sending data. - virtual bool SetSendBandwidth(bool autobw, int bps) = 0; + // Sets the initial bandwidth to use when sending starts. + virtual bool SetStartSendBandwidth(int bps) = 0; + // Sets the maximum allowed bandwidth to use when sending data. + virtual bool SetMaxSendBandwidth(int bps) = 0; // Base method to send packet using NetworkInterface. bool SendPacket(talk_base::Buffer* packet) { diff --git a/talk/media/base/rtpdataengine.cc b/talk/media/base/rtpdataengine.cc index 0f84c836fc..3d0efc43b0 100644 --- a/talk/media/base/rtpdataengine.cc +++ b/talk/media/base/rtpdataengine.cc @@ -290,8 +290,8 @@ void RtpDataMediaChannel::OnPacketReceived( SignalDataReceived(params, data, data_len); } -bool RtpDataMediaChannel::SetSendBandwidth(bool autobw, int bps) { - if (autobw || bps <= 0) { +bool RtpDataMediaChannel::SetMaxSendBandwidth(int bps) { + if (bps <= 0) { bps = kDataMaxBandwidth; } send_limiter_.reset(new talk_base::RateLimiter(bps / 8, 1.0)); diff --git a/talk/media/base/rtpdataengine.h b/talk/media/base/rtpdataengine.h index 59e6589532..d5abeef68a 100644 --- a/talk/media/base/rtpdataengine.h +++ b/talk/media/base/rtpdataengine.h @@ -96,7 +96,8 @@ class RtpDataMediaChannel : public DataMediaChannel { timing_ = timing; } - virtual bool SetSendBandwidth(bool autobw, int bps); + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps); virtual bool SetRecvRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions) { return true; } virtual bool SetSendRtpHeaderExtensions( diff --git a/talk/media/base/rtpdataengine_unittest.cc b/talk/media/base/rtpdataengine_unittest.cc index a86ab3b312..3c8318493b 100644 --- a/talk/media/base/rtpdataengine_unittest.cc +++ b/talk/media/base/rtpdataengine_unittest.cc @@ -387,7 +387,7 @@ TEST_F(RtpDataMediaChannelTest, SendDataRate) { // With rtp overhead of 32 bytes, each one of our packets is 36 // bytes, or 288 bits. So, a limit of 872bps will allow 3 packets, // but not four. - dmc->SetSendBandwidth(false, 872); + dmc->SetMaxSendBandwidth(872); EXPECT_TRUE(dmc->SendData(params, payload, &result)); EXPECT_TRUE(dmc->SendData(params, payload, &result)); diff --git a/talk/media/base/videoadapter.cc b/talk/media/base/videoadapter.cc index 22b1f7d8dd..588f1950e1 100644 --- a/talk/media/base/videoadapter.cc +++ b/talk/media/base/videoadapter.cc @@ -165,8 +165,8 @@ VideoAdapter::VideoAdapter() frames_(0), adapted_frames_(0), adaption_changes_(0), - previous_width_(0), - previous_height_(0), + previous_width(0), + previous_height(0), black_output_(false), is_black_(false), interval_next_frame_(0) { @@ -240,11 +240,11 @@ int VideoAdapter::GetOutputNumPixels() const { // TODO(fbarchard): Add AdaptFrameRate function that only drops frames but // not resolution. bool VideoAdapter::AdaptFrame(const VideoFrame* in_frame, - VideoFrame** out_frame) { + const VideoFrame** out_frame) { + talk_base::CritScope cs(&critical_section_); if (!in_frame || !out_frame) { return false; } - talk_base::CritScope cs(&critical_section_); ++frames_; // Update input to actual frame dimensions. @@ -306,8 +306,8 @@ bool VideoAdapter::AdaptFrame(const VideoFrame* in_frame, // resolution changes as well. Consider dropping the statistics into their // own class which could be queried publically. bool changed = false; - if (previous_width_ && (previous_width_ != (*out_frame)->GetWidth() || - previous_height_ != (*out_frame)->GetHeight())) { + if (previous_width && (previous_width != (*out_frame)->GetWidth() || + previous_height != (*out_frame)->GetHeight())) { show = true; ++adaption_changes_; changed = true; @@ -325,8 +325,8 @@ bool VideoAdapter::AdaptFrame(const VideoFrame* in_frame, << "x" << (*out_frame)->GetHeight() << " Changed: " << (changed ? "true" : "false"); } - previous_width_ = (*out_frame)->GetWidth(); - previous_height_ = (*out_frame)->GetHeight(); + previous_width = (*out_frame)->GetWidth(); + previous_height = (*out_frame)->GetHeight(); return true; } diff --git a/talk/media/base/videoadapter.h b/talk/media/base/videoadapter.h index 12be4fab1a..38a8c9d63a 100644 --- a/talk/media/base/videoadapter.h +++ b/talk/media/base/videoadapter.h @@ -62,7 +62,7 @@ class VideoAdapter { // successfully. Return false otherwise. // output_frame_ is owned by the VideoAdapter that has the best knowledge on // the output frame. - bool AdaptFrame(const VideoFrame* in_frame, VideoFrame** out_frame); + bool AdaptFrame(const VideoFrame* in_frame, const VideoFrame** out_frame); void set_scale_third(bool enable) { LOG(LS_INFO) << "Video Adapter third scaling is now " @@ -90,8 +90,8 @@ class VideoAdapter { int frames_; // Number of input frames. int adapted_frames_; // Number of frames scaled. int adaption_changes_; // Number of changes in scale factor. - size_t previous_width_; // Previous adapter output width. - size_t previous_height_; // Previous adapter output height. + size_t previous_width; // Previous adapter output width. + size_t previous_height; // Previous adapter output height. bool black_output_; // Flag to tell if we need to black output_frame_. bool is_black_; // Flag to tell if output_frame_ is currently black. int64 interval_next_frame_; diff --git a/talk/media/base/videocapturer.cc b/talk/media/base/videocapturer.cc index 26fcfa9fbb..355cc64dd8 100644 --- a/talk/media/base/videocapturer.cc +++ b/talk/media/base/videocapturer.cc @@ -257,7 +257,7 @@ bool VideoCapturer::GetBestCaptureFormat(const VideoFormat& format, best_format->width = best->width; best_format->height = best->height; best_format->fourcc = best->fourcc; - best_format->interval = talk_base::_max(format.interval, best->interval); + best_format->interval = best->interval; LOG(LS_INFO) << " Best " << best_format->ToString() << " Interval " << best_format->interval << " distance " << best_distance; } @@ -301,7 +301,7 @@ std::string VideoCapturer::ToString(const CapturedFrame* captured_frame) const { std::ostringstream ss; ss << fourcc_name << captured_frame->width << "x" << captured_frame->height - << "x" << VideoFormat::IntervalToFps(captured_frame->elapsed_time); + << "x" << VideoFormat::IntervalToFpsFloat(captured_frame->elapsed_time); return ss.str(); } @@ -475,25 +475,14 @@ void VideoCapturer::OnFrameCaptured(VideoCapturer*, << desired_width << " x " << desired_height; return; } - - VideoFrame* adapted_frame = &i420_frame; - if (!SignalAdaptFrame.is_empty() && !IsScreencast()) { - VideoFrame* out_frame = NULL; - SignalAdaptFrame(this, adapted_frame, &out_frame); - if (!out_frame) { - return; // VideoAdapter dropped the frame. - } - adapted_frame = out_frame; - } - - if (!muted_ && !ApplyProcessors(adapted_frame)) { + if (!muted_ && !ApplyProcessors(&i420_frame)) { // Processor dropped the frame. return; } if (muted_) { - adapted_frame->SetToBlack(); + i420_frame.SetToBlack(); } - SignalVideoFrame(this, adapted_frame); + SignalVideoFrame(this, &i420_frame); #endif // VIDEO_FRAME_NAME } @@ -577,9 +566,9 @@ int64 VideoCapturer::GetFormatDistance(const VideoFormat& desired, int desired_width = desired.width; int desired_height = desired.height; int64 delta_w = supported.width - desired_width; - int64 supported_fps = VideoFormat::IntervalToFps(supported.interval); - int64 delta_fps = - supported_fps - VideoFormat::IntervalToFps(desired.interval); + float supported_fps = VideoFormat::IntervalToFpsFloat(supported.interval); + float delta_fps = + supported_fps - VideoFormat::IntervalToFpsFloat(desired.interval); // Check height of supported height compared to height we would like it to be. int64 aspect_h = desired_width ? supported.width * desired_height / desired_width @@ -606,9 +595,9 @@ int64 VideoCapturer::GetFormatDistance(const VideoFormat& desired, // Require camera fps to be at least 96% of what is requested, or higher, // if resolution differs. 96% allows for slight variations in fps. e.g. 29.97 if (delta_fps < 0) { - int64 min_desirable_fps = delta_w ? - VideoFormat::IntervalToFps(desired.interval) * 29 / 30 : - VideoFormat::IntervalToFps(desired.interval) * 24 / 30; + float min_desirable_fps = delta_w ? + VideoFormat::IntervalToFpsFloat(desired.interval) * 28.f / 30.f : + VideoFormat::IntervalToFpsFloat(desired.interval) * 23.f / 30.f; delta_fps = -delta_fps; if (supported_fps < min_desirable_fps) { distance |= static_cast<int64>(1) << 62; @@ -616,10 +605,11 @@ int64 VideoCapturer::GetFormatDistance(const VideoFormat& desired, distance |= static_cast<int64>(1) << 15; } } + int64 idelta_fps = static_cast<int>(delta_fps); // 12 bits for width and height and 8 bits for fps and fourcc. distance |= - (delta_w << 28) | (delta_h << 16) | (delta_fps << 8) | delta_fourcc; + (delta_w << 28) | (delta_h << 16) | (idelta_fps << 8) | delta_fourcc; return distance; } diff --git a/talk/media/base/videocapturer.h b/talk/media/base/videocapturer.h index 2bd68bca16..933fc82500 100644 --- a/talk/media/base/videocapturer.h +++ b/talk/media/base/videocapturer.h @@ -255,11 +255,6 @@ class VideoCapturer // Signal the captured frame to downstream. sigslot::signal2<VideoCapturer*, const CapturedFrame*, sigslot::multi_threaded_local> SignalFrameCaptured; - // If slots are connected to SignalAdaptFrame, this signals with parameters - // of this capturer instance, the input video frame and output frame - // pointer, respectively. - sigslot::signal3<VideoCapturer*, const VideoFrame*, VideoFrame**, - sigslot::multi_threaded_local> SignalAdaptFrame; // Signal the captured frame converted to I420 to downstream. sigslot::signal2<VideoCapturer*, const VideoFrame*, sigslot::multi_threaded_local> SignalVideoFrame; diff --git a/talk/media/base/videocapturer_unittest.cc b/talk/media/base/videocapturer_unittest.cc index 82a95fb637..0ba932a6c7 100644 --- a/talk/media/base/videocapturer_unittest.cc +++ b/talk/media/base/videocapturer_unittest.cc @@ -507,23 +507,23 @@ TEST_F(VideoCapturerTest, TestFpsFormats) { cricket::VideoFormat::FpsToInterval(10), cricket::FOURCC_ANY)); cricket::VideoFormat best; - // expect 30 fps to choose 30 fps format + // Expect 30 fps to choose 30 fps format. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[0], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(400, best.height); EXPECT_EQ(cricket::VideoFormat::FpsToInterval(30), best.interval); - // expect 20 fps to choose 20 fps format + // Expect 20 fps to choose 30 fps format. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[1], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(400, best.height); - EXPECT_EQ(cricket::VideoFormat::FpsToInterval(20), best.interval); + EXPECT_EQ(cricket::VideoFormat::FpsToInterval(30), best.interval); - // expect 10 fps to choose 15 fps format but set fps to 10 + // Expect 10 fps to choose 15 fps format and set fps to 15. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[2], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(480, best.height); - EXPECT_EQ(cricket::VideoFormat::FpsToInterval(10), best.interval); + EXPECT_EQ(cricket::VideoFormat::FpsToInterval(15), best.interval); // We have VGA 60 fps and 15 fps. Choose best fps. supported_formats.clear(); @@ -539,23 +539,23 @@ TEST_F(VideoCapturerTest, TestFpsFormats) { cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420)); capturer_.ResetSupportedFormats(supported_formats); - // expect 30 fps to choose 60 fps format, but will set best fps to 30 + // Expect 30 fps to choose 60 fps format and will set best fps to 60. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[0], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(480, best.height); - EXPECT_EQ(cricket::VideoFormat::FpsToInterval(30), best.interval); + EXPECT_EQ(cricket::VideoFormat::FpsToInterval(60), best.interval); - // expect 20 fps to choose 60 fps format, but will set best fps to 20 + // Expect 20 fps to choose 60 fps format, and will set best fps to 60. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[1], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(480, best.height); - EXPECT_EQ(cricket::VideoFormat::FpsToInterval(20), best.interval); + EXPECT_EQ(cricket::VideoFormat::FpsToInterval(60), best.interval); - // expect 10 fps to choose 10 fps + // Expect 10 fps to choose 15 fps. EXPECT_TRUE(capturer_.GetBestCaptureFormat(required_formats[2], &best)); EXPECT_EQ(640, best.width); EXPECT_EQ(480, best.height); - EXPECT_EQ(cricket::VideoFormat::FpsToInterval(10), best.interval); + EXPECT_EQ(cricket::VideoFormat::FpsToInterval(15), best.interval); } TEST_F(VideoCapturerTest, TestRequest16x10_9) { diff --git a/talk/media/base/videocommon.cc b/talk/media/base/videocommon.cc index b051d526a3..12d0ee7101 100644 --- a/talk/media/base/videocommon.cc +++ b/talk/media/base/videocommon.cc @@ -236,7 +236,8 @@ std::string VideoFormat::ToString() const { } std::ostringstream ss; - ss << fourcc_name << width << "x" << height << "x" << IntervalToFps(interval); + ss << fourcc_name << width << "x" << height << "x" + << IntervalToFpsFloat(interval); return ss.str(); } diff --git a/talk/media/base/videocommon.h b/talk/media/base/videocommon.h index cf24f6fbb3..20e01c0bc9 100644 --- a/talk/media/base/videocommon.h +++ b/talk/media/base/videocommon.h @@ -212,11 +212,17 @@ struct VideoFormat : VideoFormatPod { } static int IntervalToFps(int64 interval) { - // Normalize the interval first. - interval = talk_base::_max(interval, kMinimumInterval); return static_cast<int>(talk_base::kNumNanosecsPerSec / interval); } + static float IntervalToFpsFloat(int64 interval) { + if (!interval) { + return 0.f; + } + return static_cast<float>(talk_base::kNumNanosecsPerSec) / + static_cast<float>(interval); + } + bool operator==(const VideoFormat& format) const { return width == format.width && height == format.height && interval == format.interval && fourcc == format.fourcc; diff --git a/talk/media/base/videocommon_unittest.cc b/talk/media/base/videocommon_unittest.cc index 455a47b79a..20bd43d430 100644 --- a/talk/media/base/videocommon_unittest.cc +++ b/talk/media/base/videocommon_unittest.cc @@ -70,7 +70,7 @@ TEST(VideoCommonTest, TestVideoFormatIsSize0x0) { // Test ToString: print fourcc when it is printable. TEST(VideoCommonTest, TestVideoFormatToString) { VideoFormat format; - EXPECT_EQ("0x0x10000", format.ToString()); + EXPECT_EQ("0x0x0", format.ToString()); format.fourcc = FOURCC_I420; format.width = 640; diff --git a/talk/media/base/videoengine_unittest.h b/talk/media/base/videoengine_unittest.h index 2ffd14bd2f..893f0f3488 100644 --- a/talk/media/base/videoengine_unittest.h +++ b/talk/media/base/videoengine_unittest.h @@ -928,12 +928,11 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_TRUE(channel_->SetCapturer(5678, NULL)); } - // Test that we can set the bandwidth to auto or a specific value. + // Test that we can set the bandwidth. void SetSendBandwidth() { - EXPECT_TRUE(channel_->SetSendBandwidth(true, -1)); - EXPECT_TRUE(channel_->SetSendBandwidth(true, 128 * 1024)); - EXPECT_TRUE(channel_->SetSendBandwidth(false, -1)); - EXPECT_TRUE(channel_->SetSendBandwidth(false, 128 * 1024)); + EXPECT_TRUE(channel_->SetStartSendBandwidth(64 * 1024)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(-1)); // <= 0 means unlimited. + EXPECT_TRUE(channel_->SetMaxSendBandwidth(128 * 1024)); } // Test that we can set the SSRC for the default send source. void SetSendSsrc() { diff --git a/talk/media/other/linphonemediaengine.h b/talk/media/other/linphonemediaengine.h index 69b2d2f6a7..db3e69f6bc 100644 --- a/talk/media/other/linphonemediaengine.h +++ b/talk/media/other/linphonemediaengine.h @@ -145,7 +145,8 @@ class LinphoneVoiceChannel : public VoiceMediaChannel { virtual void SetSendSsrc(uint32 id) {} // TODO: change RTP packet? virtual bool SetRtcpCName(const std::string& cname) { return true; } virtual bool Mute(bool on) { return mute_; } - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { return true; } virtual bool SetOptions(int options) { return true; } virtual bool SetRecvRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions) { return true; } diff --git a/talk/media/sctp/sctpdataengine.h b/talk/media/sctp/sctpdataengine.h index b34a810092..f2322ab27c 100644 --- a/talk/media/sctp/sctpdataengine.h +++ b/talk/media/sctp/sctpdataengine.h @@ -30,7 +30,7 @@ #include <errno.h> #include <string> -#include <set> +#include <vector> namespace cricket { // Some ERRNO values get re-#defined to WSA* equivalents in some talk/ @@ -166,7 +166,8 @@ class SctpDataMediaChannel : public DataMediaChannel, // TODO(pthatcher): Cleanup MediaChannel interface, or at least // don't try calling these and return false. Right now, things // don't work if we return false. - virtual bool SetSendBandwidth(bool autobw, int bps) { return true; } + virtual bool SetStartSendBandwidth(int bps) { return true; } + virtual bool SetMaxSendBandwidth(int bps) { return true; } virtual bool SetRecvRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions) { return true; } virtual bool SetSendRtpHeaderExtensions( @@ -216,6 +217,7 @@ class SctpDataMediaChannel : public DataMediaChannel, talk_base::Buffer* buffer); void OnNotificationFromSctp(talk_base::Buffer* buffer); void OnNotificationAssocChange(const sctp_assoc_change& change); + void OnStreamResetEvent(const struct sctp_stream_reset_event* evt); // Responsible for marshalling incoming data to the channels listeners, and diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index f33d6f06fe..63c9effda4 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -632,6 +632,10 @@ class WebRtcVideoChannelSendInfo : public sigslot::has_slots<> { } } + bool AdaptFrame(const VideoFrame* in_frame, const VideoFrame** out_frame) { + *out_frame = NULL; + return video_adapter_->AdaptFrame(in_frame, out_frame); + } int CurrentAdaptReason() const { return video_adapter_->adapt_reason(); } @@ -672,18 +676,9 @@ class WebRtcVideoChannelSendInfo : public sigslot::has_slots<> { // video capturer. video_adapter_->SetInputFormat(*capture_format); } - // TODO(thorcarpenter): When the adapter supports "only frame dropping" - // mode, also hook it up to screencast capturers. - video_capturer->SignalAdaptFrame.connect( - this, &WebRtcVideoChannelSendInfo::AdaptFrame); } } - void AdaptFrame(VideoCapturer* capturer, const VideoFrame* input, - VideoFrame** adapted) { - video_adapter_->AdaptFrame(input, adapted); - } - void ApplyCpuOptions(const VideoOptions& options) { bool cpu_adapt, cpu_smoothing, adapt_third; float low, med, high; @@ -2470,7 +2465,9 @@ bool WebRtcVideoMediaChannel::SetCapturer(uint32 ssrc, MaybeDisconnectCapturer(old_capturer); send_channel->set_video_capturer(capturer); - MaybeConnectCapturer(capturer); + capturer->SignalVideoFrame.connect( + this, + &WebRtcVideoMediaChannel::SendFrame); if (!capturer->IsScreencast() && ratio_w_ != 0 && ratio_h_ != 0) { capturer->UpdateAspectRatio(ratio_w_, ratio_h_); } @@ -2624,11 +2621,27 @@ bool WebRtcVideoMediaChannel::SetSendRtpHeaderExtensions( return true; } -bool WebRtcVideoMediaChannel::SetSendBandwidth(bool autobw, int bps) { - LOG(LS_INFO) << "WebRtcVideoMediaChanne::SetSendBandwidth"; +bool WebRtcVideoMediaChannel::SetStartSendBandwidth(int bps) { + LOG(LS_INFO) << "WebRtcVideoMediaChannel::SetStartSendBandwidth"; + + if (!send_codec_) { + LOG(LS_INFO) << "The send codec has not been set up yet"; + return true; + } + + // On success, SetSendCodec() will reset |send_start_bitrate_| to |bps/1000|, + // by calling MaybeChangeStartBitrate. That method will also clamp the + // start bitrate between min and max, consistent with the override behavior + // in SetMaxSendBandwidth. + return SetSendCodec(*send_codec_, + send_min_bitrate_, bps / 1000, send_max_bitrate_); +} + +bool WebRtcVideoMediaChannel::SetMaxSendBandwidth(int bps) { + LOG(LS_INFO) << "WebRtcVideoMediaChannel::SetMaxSendBandwidth"; if (InConferenceMode()) { - LOG(LS_INFO) << "Conference mode ignores SetSendBandWidth"; + LOG(LS_INFO) << "Conference mode ignores SetMaxSendBandwidth"; return true; } @@ -2637,28 +2650,17 @@ bool WebRtcVideoMediaChannel::SetSendBandwidth(bool autobw, int bps) { return true; } - int min_bitrate; - int start_bitrate; - int max_bitrate; - if (autobw) { - // Use the default values for min bitrate. - min_bitrate = send_min_bitrate_; - // Use the default value or the bps for the max - max_bitrate = (bps <= 0) ? send_max_bitrate_ : (bps / 1000); - // Maximum start bitrate can be kStartVideoBitrate. - start_bitrate = talk_base::_min(kStartVideoBitrate, max_bitrate); - } else { - // Use the default start or the bps as the target bitrate. - int target_bitrate = (bps <= 0) ? kStartVideoBitrate : (bps / 1000); - min_bitrate = target_bitrate; - start_bitrate = target_bitrate; - max_bitrate = target_bitrate; - } + // Use the default value or the bps for the max + int max_bitrate = (bps <= 0) ? send_max_bitrate_ : (bps / 1000); + + // Reduce the current minimum and start bitrates if necessary. + int min_bitrate = talk_base::_min(send_min_bitrate_, max_bitrate); + int start_bitrate = talk_base::_min(send_start_bitrate_, max_bitrate); if (!SetSendCodec(*send_codec_, min_bitrate, start_bitrate, max_bitrate)) { return false; } - LogSendCodecChange("SetSendBandwidth()"); + LogSendCodecChange("SetMaxSendBandwidth()"); return true; } @@ -2860,6 +2862,7 @@ bool WebRtcVideoMediaChannel::GetRenderer(uint32 ssrc, return true; } +// TODO(zhurunz): Add unittests to test this function. void WebRtcVideoMediaChannel::SendFrame(VideoCapturer* capturer, const VideoFrame* frame) { // If the |capturer| is registered to any send channel, then send the frame diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index 289903a072..d4949c473c 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -276,7 +276,8 @@ class WebRtcVideoMediaChannel : public talk_base::MessageHandler, const std::vector<RtpHeaderExtension>& extensions); virtual bool SetSendRtpHeaderExtensions( const std::vector<RtpHeaderExtension>& extensions); - virtual bool SetSendBandwidth(bool autobw, int bps); + virtual bool SetStartSendBandwidth(int bps); + virtual bool SetMaxSendBandwidth(int bps); virtual bool SetOptions(const VideoOptions &options); virtual bool GetOptions(VideoOptions *options) const { *options = options_; @@ -292,6 +293,8 @@ class WebRtcVideoMediaChannel : public talk_base::MessageHandler, bool SendFrame(WebRtcVideoChannelSendInfo* channel_info, const VideoFrame* frame, bool is_screencast); + void AdaptAndSendFrame(VideoCapturer* capturer, const VideoFrame* frame); + // Thunk functions for use with HybridVideoEngine void OnLocalFrame(VideoCapturer* capturer, const VideoFrame* frame) { SendFrame(0u, frame, capturer->IsScreencast()); diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index b10b11f855..24eae46944 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -1100,30 +1100,62 @@ TEST_F(WebRtcVideoEngineTestFake, SetBandwidthAuto) { EXPECT_TRUE(SetupEngine()); int channel_num = vie_.GetLastChannel(); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); - EXPECT_TRUE(channel_->SetSendBandwidth(true, cricket::kAutoBandwidth)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(cricket::kAutoBandwidth)); VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height); } // Test that we set bandwidth properly when using auto with upper bound. -TEST_F(WebRtcVideoEngineTestFake, SetBandwidthAutoCapped) { +TEST_F(WebRtcVideoEngineTestFake, SetBandwidthCapped) { EXPECT_TRUE(SetupEngine()); int channel_num = vie_.GetLastChannel(); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); - EXPECT_TRUE(channel_->SetSendBandwidth(true, 768000)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(768000)); VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, 768U); } -// Test that we set bandwidth properly when using a fixed bandwidth. -TEST_F(WebRtcVideoEngineTestFake, SetBandwidthFixed) { +// Test that we reduce the start bandwidth when the requested max is less than +// the default start bandwidth. +TEST_F(WebRtcVideoEngineTestFake, SetMaxBandwidthBelowDefaultStart) { EXPECT_TRUE(SetupEngine()); int channel_num = vie_.GetLastChannel(); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); - EXPECT_TRUE(channel_->SetSendBandwidth(false, 768000)); + int max_bandwidth_kbps = (kMinBandwidthKbps + kStartBandwidthKbps) / 2; + EXPECT_TRUE(channel_->SetMaxSendBandwidth(max_bandwidth_kbps * 1000)); VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, - 768U, 768U, 768U); + max_bandwidth_kbps, kMinBandwidthKbps, max_bandwidth_kbps); } -// Test that SetSendBandwidth is ignored in conference mode. +// Test that we reduce the min bandwidth when the requested max is less than +// the min bandwidth. +TEST_F(WebRtcVideoEngineTestFake, SetMaxBandwidthBelowMin) { + EXPECT_TRUE(SetupEngine()); + int channel_num = vie_.GetLastChannel(); + EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); + int max_bandwidth_kbps = kMinBandwidthKbps / 2; + EXPECT_TRUE(channel_->SetMaxSendBandwidth(max_bandwidth_kbps * 1000)); + VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, + max_bandwidth_kbps, max_bandwidth_kbps, max_bandwidth_kbps); +} + +// Test that the start bandwidth can be controlled separately from the max +// bandwidth. +TEST_F(WebRtcVideoEngineTestFake, SetStartBandwidth) { + EXPECT_TRUE(SetupEngine()); + int channel_num = vie_.GetLastChannel(); + EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); + int start_bandwidth_kbps = kStartBandwidthKbps + 1; + EXPECT_TRUE(channel_->SetStartSendBandwidth(start_bandwidth_kbps * 1000)); + VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, + kMaxBandwidthKbps, kMinBandwidthKbps, start_bandwidth_kbps); + + // Check that SetMaxSendBandwidth doesn't overwrite the start bandwidth. + int max_bandwidth_kbps = kMaxBandwidthKbps + 1; + EXPECT_TRUE(channel_->SetMaxSendBandwidth(max_bandwidth_kbps * 1000)); + VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, + max_bandwidth_kbps, kMinBandwidthKbps, start_bandwidth_kbps); +} + +// Test that SetMaxSendBandwidth is ignored in conference mode. TEST_F(WebRtcVideoEngineTestFake, SetBandwidthInConference) { EXPECT_TRUE(SetupEngine()); int channel_num = vie_.GetLastChannel(); @@ -1134,7 +1166,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetBandwidthInConference) { VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height); // Set send bandwidth. - EXPECT_TRUE(channel_->SetSendBandwidth(false, 768000)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(768000)); // Verify bitrate not changed. webrtc::VideoCodec gcodec; @@ -1159,12 +1191,11 @@ TEST_F(WebRtcVideoEngineTestFake, SetBandwidthScreencast) { EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(123))); EXPECT_TRUE(channel_->SetSendCodecs(codec_list)); - EXPECT_TRUE(channel_->SetSendBandwidth(false, 111000)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(111000)); EXPECT_TRUE(channel_->SetSend(true)); SendI420ScreencastFrame(kVP8Codec.width, kVP8Codec.height); - VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, - 111, 111, 111); + VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height, 0, 111); } @@ -1243,7 +1274,11 @@ TEST_F(WebRtcVideoEngineTestFake, SetOptionsWithDenoising) { EXPECT_FALSE(vie_.GetCaptureDenoising(capture_id)); } -TEST_F(WebRtcVideoEngineTestFake, MultipleSendStreamsWithOneCapturer) { +// Test disabled because it drops frames when adapt-before-effects is turned +// off (turned off because it was exposing a crash - see bug 12250150). This is +// safe for now because this test exercises an unused feature. +// TODO(tpsiaki) reenable once adapt-before-effects is turned back on. +TEST_F(WebRtcVideoEngineTestFake, DISABLED_MultipleSendStreamsWithOneCapturer) { EXPECT_TRUE(SetupEngine()); // Start the capturer diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 95e8c853bb..4d1705736e 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -1655,7 +1655,6 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine *engine) engine, engine->CreateMediaVoiceChannel()), send_bw_setting_(false), - send_autobw_(false), send_bw_bps_(0), options_(), dtmf_allowed_(false), @@ -2023,7 +2022,7 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( send_codec_.reset(new webrtc::CodecInst(send_codec)); if (send_bw_setting_) { - SetSendBandwidthInternal(send_autobw_, send_bw_bps_); + SetSendBandwidthInternal(send_bw_bps_); } return true; @@ -2928,18 +2927,23 @@ bool WebRtcVoiceMediaChannel::MuteStream(uint32 ssrc, bool muted) { return true; } -bool WebRtcVoiceMediaChannel::SetSendBandwidth(bool autobw, int bps) { - LOG(LS_INFO) << "WebRtcVoiceMediaChanne::SetSendBandwidth."; +bool WebRtcVoiceMediaChannel::SetStartSendBandwidth(int bps) { + // TODO(andresp): Add support for setting an independent start bandwidth when + // bandwidth estimation is enabled for voice engine. + return false; +} - send_bw_setting_ = true; - send_autobw_ = autobw; - send_bw_bps_ = bps; +bool WebRtcVoiceMediaChannel::SetMaxSendBandwidth(int bps) { + LOG(LS_INFO) << "WebRtcVoiceMediaChanne::SetSendBandwidth."; - return SetSendBandwidthInternal(send_autobw_, send_bw_bps_); + return SetSendBandwidthInternal(bps); } -bool WebRtcVoiceMediaChannel::SetSendBandwidthInternal(bool autobw, int bps) { - LOG(LS_INFO) << "WebRtcVoiceMediaChanne::SetSendBandwidthInternal."; +bool WebRtcVoiceMediaChannel::SetSendBandwidthInternal(int bps) { + LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetSendBandwidthInternal."; + + send_bw_setting_ = true; + send_bw_bps_ = bps; if (!send_codec_) { LOG(LS_INFO) << "The send codec has not been set up yet. " @@ -2948,7 +2952,9 @@ bool WebRtcVoiceMediaChannel::SetSendBandwidthInternal(bool autobw, int bps) { } // Bandwidth is auto by default. - if (autobw || bps <= 0) + // TODO(bemasc): Fix this so that if SetMaxSendBandwidth(50) is followed by + // SetMaxSendBandwith(0), the second call removes the previous limit. + if (bps <= 0) return true; webrtc::CodecInst codec = *send_codec_; diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index adf4853667..e50bb3ccf5 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -365,7 +365,8 @@ class WebRtcVoiceMediaChannel const talk_base::PacketTime& packet_time); virtual void OnReadyToSend(bool ready) {} virtual bool MuteStream(uint32 ssrc, bool on); - virtual bool SetSendBandwidth(bool autobw, int bps); + virtual bool SetStartSendBandwidth(int bps); + virtual bool SetMaxSendBandwidth(int bps); virtual bool GetStats(VoiceMediaInfo* info); // Gets last reported error from WebRtc voice engine. This should be only // called in response a failure. @@ -411,7 +412,7 @@ class WebRtcVoiceMediaChannel return channel_id == voe_channel(); } bool SetSendCodecs(int channel, const std::vector<AudioCodec>& codecs); - bool SetSendBandwidthInternal(bool autobw, int bps); + bool SetSendBandwidthInternal(int bps); talk_base::scoped_ptr<WebRtcSoundclipStream> ringback_tone_; std::set<int> ringback_channels_; // channels playing ringback @@ -419,7 +420,6 @@ class WebRtcVoiceMediaChannel std::vector<AudioCodec> send_codecs_; talk_base::scoped_ptr<webrtc::CodecInst> send_codec_; bool send_bw_setting_; - bool send_autobw_; int send_bw_bps_; AudioOptions options_; bool dtmf_allowed_; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 7fa5834b2f..5dace8d4de 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -201,39 +201,26 @@ class WebRtcVoiceEngineTestFake : public testing::Test { // Test that send bandwidth is set correctly. // |codec| is the codec under test. - // |default_bitrate| is the default bitrate for the codec. - // |auto_bitrate| is a parameter to set to SetSendBandwidth(). - // |desired_bitrate| is a parameter to set to SetSendBandwidth(). - // |expected_result| is expected results from SetSendBandwidth(). + // |max_bitrate| is a parameter to set to SetMaxSendBandwidth(). + // |expected_result| is the expected result from SetMaxSendBandwidth(). + // |expected_bitrate| is the expected audio bitrate afterward. void TestSendBandwidth(const cricket::AudioCodec& codec, - int default_bitrate, - bool auto_bitrate, - int desired_bitrate, - bool expected_result) { + int max_bitrate, + bool expected_result, + int expected_bitrate) { int channel_num = voe_.GetLastChannel(); std::vector<cricket::AudioCodec> codecs; codecs.push_back(codec); EXPECT_TRUE(channel_->SetSendCodecs(codecs)); - bool result = channel_->SetSendBandwidth(auto_bitrate, desired_bitrate); + bool result = channel_->SetMaxSendBandwidth(max_bitrate); EXPECT_EQ(expected_result, result); webrtc::CodecInst temp_codec; EXPECT_FALSE(voe_.GetSendCodec(channel_num, temp_codec)); - if (result) { - // If SetSendBandwidth() returns true then bitrate is set correctly. - if (auto_bitrate) { - EXPECT_EQ(default_bitrate, temp_codec.rate); - } else { - EXPECT_EQ(desired_bitrate, temp_codec.rate); - } - } else { - // If SetSendBandwidth() returns false then bitrate is set to the - // default value. - EXPECT_EQ(default_bitrate, temp_codec.rate); - } + EXPECT_EQ(expected_bitrate, temp_codec.rate); } @@ -575,47 +562,67 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthAuto) { EXPECT_TRUE(SetupEngine()); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); - // Test that when autobw is true, bitrate is kept as the default - // value. autobw is true for the following tests. + // Test that when autobw is enabled, bitrate is kept as the default + // value. autobw is enabled for the following tests because the target + // bitrate is <= 0. // ISAC, default bitrate == 32000. - TestSendBandwidth(kIsacCodec, 32000, true, 96000, true); + TestSendBandwidth(kIsacCodec, 0, true, 32000); // PCMU, default bitrate == 64000. - TestSendBandwidth(kPcmuCodec, 64000, true, 96000, true); + TestSendBandwidth(kPcmuCodec, -1, true, 64000); // CELT, default bitrate == 64000. - TestSendBandwidth(kCeltCodec, 64000, true, 96000, true); + TestSendBandwidth(kCeltCodec, 0, true, 64000); // opus, default bitrate == 64000. - TestSendBandwidth(kOpusCodec, 64000, true, 96000, true); + TestSendBandwidth(kOpusCodec, -1, true, 64000); } -TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthFixedMultiRateAsCaller) { +TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCaller) { EXPECT_TRUE(SetupEngine()); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); - // Test that we can set bitrate if a multi-rate codec is used. - // autobw is false for the following tests. + // Test that the bitrate of a multi-rate codec is always the maximum. // ISAC, default bitrate == 32000. - TestSendBandwidth(kIsacCodec, 32000, false, 128000, true); + TestSendBandwidth(kIsacCodec, 128000, true, 128000); + TestSendBandwidth(kIsacCodec, 16000, true, 16000); // CELT, default bitrate == 64000. - TestSendBandwidth(kCeltCodec, 64000, false, 96000, true); + TestSendBandwidth(kCeltCodec, 96000, true, 96000); + TestSendBandwidth(kCeltCodec, 32000, true, 32000); // opus, default bitrate == 64000. - TestSendBandwidth(kOpusCodec, 64000, false, 96000, true); + TestSendBandwidth(kOpusCodec, 96000, true, 96000); + TestSendBandwidth(kOpusCodec, 48000, true, 48000); +} + +TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthFixedRateAsCaller) { + EXPECT_TRUE(SetupEngine()); + EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); + + // Test that we can only set a maximum bitrate for a fixed-rate codec + // if it's bigger than the fixed rate. + + // PCMU, fixed bitrate == 64000. + TestSendBandwidth(kPcmuCodec, 0, true, 64000); + TestSendBandwidth(kPcmuCodec, 1, false, 64000); + TestSendBandwidth(kPcmuCodec, 128000, true, 64000); + TestSendBandwidth(kPcmuCodec, 32000, false, 64000); + TestSendBandwidth(kPcmuCodec, 64000, true, 64000); + TestSendBandwidth(kPcmuCodec, 63999, false, 64000); + TestSendBandwidth(kPcmuCodec, 64001, true, 64000); } -TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthFixedMultiRateAsCallee) { +TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCallee) { EXPECT_TRUE(engine_.Init(talk_base::Thread::Current())); channel_ = engine_.CreateChannel(); EXPECT_TRUE(channel_ != NULL); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); int desired_bitrate = 128000; - EXPECT_TRUE(channel_->SetSendBandwidth(false, desired_bitrate)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(desired_bitrate)); EXPECT_TRUE(channel_->AddSendStream( cricket::StreamParams::CreateLegacy(kSsrc1))); @@ -629,7 +636,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthFixedMultiRateAsCallee) { // Test that bitrate cannot be set for CBR codecs. // Bitrate is ignored if it is higher than the fixed bitrate. // Bitrate less then the fixed bitrate is an error. -TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthFixedCbr) { +TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthCbr) { EXPECT_TRUE(SetupEngine()); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); @@ -642,10 +649,10 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendBandwidthFixedCbr) { EXPECT_TRUE(channel_->SetSendCodecs(codecs)); EXPECT_EQ(0, voe_.GetSendCodec(channel_num, codec)); EXPECT_EQ(64000, codec.rate); - EXPECT_TRUE(channel_->SetSendBandwidth(false, 128000)); + EXPECT_TRUE(channel_->SetMaxSendBandwidth(128000)); EXPECT_EQ(0, voe_.GetSendCodec(channel_num, codec)); EXPECT_EQ(64000, codec.rate); - EXPECT_FALSE(channel_->SetSendBandwidth(false, 128)); + EXPECT_FALSE(channel_->SetMaxSendBandwidth(128)); EXPECT_EQ(0, voe_.GetSendCodec(channel_num, codec)); EXPECT_EQ(64000, codec.rate); } diff --git a/talk/p2p/base/dtlstransport.h b/talk/p2p/base/dtlstransport.h index 7492171ee1..6bef185ce8 100644 --- a/talk/p2p/base/dtlstransport.h +++ b/talk/p2p/base/dtlstransport.h @@ -66,8 +66,8 @@ class DtlsTransport : public Base { return true; } - virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* - channel) { + virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* channel, + std::string* error_desc) { talk_base::SSLFingerprint* local_fp = Base::local_description()->identity_fingerprint.get(); @@ -79,30 +79,36 @@ class DtlsTransport : public Base { identity_)); ASSERT(local_fp_tmp.get() != NULL); if (!(*local_fp_tmp == *local_fp)) { - LOG(LS_WARNING) << "Local fingerprint does not match identity"; - return false; + std::ostringstream desc; + desc << "Local fingerprint does not match identity. Expected: "; + desc << local_fp_tmp->ToString(); + desc << " Got: " << local_fp->ToString(); + return BadTransportDescription(desc.str(), error_desc); } } else { - LOG(LS_WARNING) - << "Local fingerprint provided but no identity available"; - return false; + return BadTransportDescription( + "Local fingerprint provided but no identity available.", + error_desc); } } else { identity_ = NULL; } - if (!channel->SetLocalIdentity(identity_)) - return false; + if (!channel->SetLocalIdentity(identity_)) { + return BadTransportDescription("Failed to set local identity.", + error_desc); + } // Apply the description in the base class. - return Base::ApplyLocalTransportDescription_w(channel); + return Base::ApplyLocalTransportDescription_w(channel, error_desc); } - virtual bool NegotiateTransportDescription_w(ContentAction local_role) { + virtual bool NegotiateTransportDescription_w(ContentAction local_role, + std::string* error_desc) { if (!Base::local_description() || !Base::remote_description()) { - LOG(LS_INFO) << "Local and Remote description must be set before " - << "transport descriptions are negotiated"; - return false; + const std::string msg = "Local and Remote description must be set before " + "transport descriptions are negotiated"; + return BadTransportDescription(msg, error_desc); } talk_base::SSLFingerprint* local_fp = @@ -144,8 +150,9 @@ class DtlsTransport : public Base { bool is_remote_server = false; if (local_role == CA_OFFER) { if (local_connection_role != CONNECTIONROLE_ACTPASS) { - LOG(LS_ERROR) << "Offerer must use actpass value for setup attribute"; - return false; + return BadTransportDescription( + "Offerer must use actpass value for setup attribute.", + error_desc); } if (remote_connection_role == CONNECTIONROLE_ACTIVE || @@ -153,25 +160,28 @@ class DtlsTransport : public Base { remote_connection_role == CONNECTIONROLE_NONE) { is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE); } else { - LOG(LS_ERROR) << "Answerer must use either active or passive value " - << "for setup attribute"; - return false; + const std::string msg = + "Answerer must use either active or passive value " + "for setup attribute."; + return BadTransportDescription(msg, error_desc); } // If remote is NONE or ACTIVE it will act as client. } else { if (remote_connection_role != CONNECTIONROLE_ACTPASS && remote_connection_role != CONNECTIONROLE_NONE) { - LOG(LS_ERROR) << "Offerer must use actpass value for setup attribute"; - return false; + return BadTransportDescription( + "Offerer must use actpass value for setup attribute.", + error_desc); } if (local_connection_role == CONNECTIONROLE_ACTIVE || local_connection_role == CONNECTIONROLE_PASSIVE) { is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE); } else { - LOG(LS_ERROR) << "Answerer must use either active or passive value " - << "for setup attribute"; - return false; + const std::string msg = + "Answerer must use either active or passive value " + "for setup attribute."; + return BadTransportDescription(msg, error_desc); } // If local is passive, local will act as server. @@ -181,9 +191,9 @@ class DtlsTransport : public Base { talk_base::SSL_SERVER; } else if (local_fp && (local_role == CA_ANSWER)) { - LOG(LS_ERROR) - << "Local fingerprint supplied when caller didn't offer DTLS"; - return false; + return BadTransportDescription( + "Local fingerprint supplied when caller didn't offer DTLS.", + error_desc); } else { // We are not doing DTLS remote_fingerprint_.reset(new talk_base::SSLFingerprint( @@ -191,7 +201,7 @@ class DtlsTransport : public Base { } // Now run the negotiation for the base class. - return Base::NegotiateTransportDescription_w(local_role); + return Base::NegotiateTransportDescription_w(local_role, error_desc); } virtual DtlsTransportChannelWrapper* CreateTransportChannel(int component) { @@ -216,12 +226,13 @@ class DtlsTransport : public Base { private: virtual bool ApplyNegotiatedTransportDescription_w( - TransportChannelImpl* channel) { + TransportChannelImpl* channel, + std::string* error_desc) { // Set ssl role. Role must be set before fingerprint is applied, which // initiates DTLS setup. if (!channel->SetSslRole(secure_role_)) { - LOG(LS_INFO) << "Failed to set ssl role for the channel."; - return false; + return BadTransportDescription("Failed to set ssl role for the channel.", + error_desc); } // Apply remote fingerprint. if (!channel->SetRemoteFingerprint( @@ -229,9 +240,10 @@ class DtlsTransport : public Base { reinterpret_cast<const uint8 *>(remote_fingerprint_-> digest.data()), remote_fingerprint_->digest.length())) { - return false; + return BadTransportDescription("Failed to apply remote fingerprint.", + error_desc); } - return Base::ApplyNegotiatedTransportDescription_w(channel); + return Base::ApplyNegotiatedTransportDescription_w(channel, error_desc); } talk_base::SSLIdentity* identity_; diff --git a/talk/p2p/base/dtlstransportchannel_unittest.cc b/talk/p2p/base/dtlstransportchannel_unittest.cc index 1fd82d7107..7517026c25 100644 --- a/talk/p2p/base/dtlstransportchannel_unittest.cc +++ b/talk/p2p/base/dtlstransportchannel_unittest.cc @@ -185,14 +185,14 @@ class DtlsTestClient : public sigslot::has_slots<> { // content action is CA_ANSWER. if (action == cricket::CA_OFFER) { ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, cricket::CA_OFFER)); + local_desc, cricket::CA_OFFER, NULL)); ASSERT_EQ(expect_success, transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_ANSWER)); + remote_desc, cricket::CA_ANSWER, NULL)); } else { ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, cricket::CA_OFFER)); + remote_desc, cricket::CA_OFFER, NULL)); ASSERT_EQ(expect_success, transport_->SetLocalTransportDescription( - local_desc, cricket::CA_ANSWER)); + local_desc, cricket::CA_ANSWER, NULL)); } negotiated_dtls_ = (local_identity && remote_identity); } diff --git a/talk/p2p/base/session.cc b/talk/p2p/base/session.cc index e0f8dd3f4f..0bbd15d8de 100644 --- a/talk/p2p/base/session.cc +++ b/talk/p2p/base/session.cc @@ -277,21 +277,29 @@ void TransportProxy::SetIceRole(IceRole role) { } bool TransportProxy::SetLocalTransportDescription( - const TransportDescription& description, ContentAction action) { + const TransportDescription& description, + ContentAction action, + std::string* error_desc) { // If this is an answer, finalize the negotiation. if (action == CA_ANSWER) { CompleteNegotiation(); } - return transport_->get()->SetLocalTransportDescription(description, action); + return transport_->get()->SetLocalTransportDescription(description, + action, + error_desc); } bool TransportProxy::SetRemoteTransportDescription( - const TransportDescription& description, ContentAction action) { + const TransportDescription& description, + ContentAction action, + std::string* error_desc) { // If this is an answer, finalize the negotiation. if (action == CA_ANSWER) { CompleteNegotiation(); } - return transport_->get()->SetRemoteTransportDescription(description, action); + return transport_->get()->SetRemoteTransportDescription(description, + action, + error_desc); } void TransportProxy::OnSignalingReady() { @@ -418,16 +426,22 @@ bool BaseSession::SetIdentity(talk_base::SSLIdentity* identity) { } bool BaseSession::PushdownTransportDescription(ContentSource source, - ContentAction action) { + ContentAction action, + std::string* error_desc) { if (source == CS_LOCAL) { - return PushdownLocalTransportDescription(local_description_, action); + return PushdownLocalTransportDescription(local_description_, + action, + error_desc); } - return PushdownRemoteTransportDescription(remote_description_, action); + return PushdownRemoteTransportDescription(remote_description_, + action, + error_desc); } bool BaseSession::PushdownLocalTransportDescription( const SessionDescription* sdesc, - ContentAction action) { + ContentAction action, + std::string* error_desc) { // Update the Transports with the right information, and trigger them to // start connecting. for (TransportMap::iterator iter = transports_.begin(); @@ -438,7 +452,8 @@ bool BaseSession::PushdownLocalTransportDescription( bool ret = GetTransportDescription( sdesc, iter->second->content_name(), &tdesc); if (ret) { - if (!iter->second->SetLocalTransportDescription(tdesc, action)) { + if (!iter->second->SetLocalTransportDescription(tdesc, action, + error_desc)) { return false; } @@ -451,7 +466,8 @@ bool BaseSession::PushdownLocalTransportDescription( bool BaseSession::PushdownRemoteTransportDescription( const SessionDescription* sdesc, - ContentAction action) { + ContentAction action, + std::string* error_desc) { // Update the Transports with the right information. for (TransportMap::iterator iter = transports_.begin(); iter != transports_.end(); ++iter) { @@ -462,7 +478,8 @@ bool BaseSession::PushdownRemoteTransportDescription( bool ret = GetTransportDescription( sdesc, iter->second->content_name(), &tdesc); if (ret) { - if (!iter->second->SetRemoteTransportDescription(tdesc, action)) { + if (!iter->second->SetRemoteTransportDescription(tdesc, action, + error_desc)) { return false; } } @@ -611,10 +628,11 @@ void BaseSession::SetState(State state) { SignalNewDescription(); } -void BaseSession::SetError(Error error) { +void BaseSession::SetError(Error error, const std::string& error_desc) { ASSERT(signaling_thread_->IsCurrent()); if (error != error_) { error_ = error; + error_desc_ = error_desc; SignalError(this, error); } } @@ -856,7 +874,7 @@ void BaseSession::OnMessage(talk_base::Message *pmsg) { switch (pmsg->message_id) { case MSG_TIMEOUT: // Session timeout has occured. - SetError(ERROR_TIME); + SetError(ERROR_TIME, "Session timeout has occured."); break; case MSG_STATE: @@ -925,7 +943,7 @@ bool Session::Initiate(const std::string &to, // the TransportDescriptions. SpeculativelyConnectAllTransportChannels(); - PushdownTransportDescription(CS_LOCAL, CA_OFFER); + PushdownTransportDescription(CS_LOCAL, CA_OFFER, NULL); SetState(Session::STATE_SENTINITIATE); return true; } @@ -946,7 +964,7 @@ bool Session::Accept(const SessionDescription* sdesc) { return false; } // TODO(juberti): Add BUNDLE support to transport-info messages. - PushdownTransportDescription(CS_LOCAL, CA_ANSWER); + PushdownTransportDescription(CS_LOCAL, CA_ANSWER, NULL); MaybeEnableMuxingSupport(); // Enable transport channel mux if supported. SetState(Session::STATE_SENTACCEPT); return true; @@ -1261,8 +1279,10 @@ void Session::OnFailedSend(const buzz::XmlElement* orig_stanza, if (!OnRedirectError(redirect, &error)) { // TODO: Should we send a message back? The standard // says nothing about it. - LOG(LS_ERROR) << "Failed to redirect: " << error.text; - SetError(ERROR_RESPONSE); + std::ostringstream desc; + desc << "Failed to redirect: " << error.text; + LOG(LS_ERROR) << desc.str(); + SetError(ERROR_RESPONSE, desc.str()); } return; } @@ -1290,7 +1310,7 @@ void Session::OnFailedSend(const buzz::XmlElement* orig_stanza, } else if ((error_type != "continue") && (error_type != "wait")) { // We do not set an error if the other side said it is okay to continue // (possibly after waiting). These errors can be ignored. - SetError(ERROR_RESPONSE); + SetError(ERROR_RESPONSE, ""); } } @@ -1318,7 +1338,7 @@ bool Session::OnInitiateMessage(const SessionMessage& msg, init.transports, init.groups)); // Updating transport with TransportDescription. - PushdownTransportDescription(CS_REMOTE, CA_OFFER); + PushdownTransportDescription(CS_REMOTE, CA_OFFER, NULL); SetState(STATE_RECEIVEDINITIATE); // Users of Session may listen to state change and call Reject(). @@ -1352,7 +1372,7 @@ bool Session::OnAcceptMessage(const SessionMessage& msg, MessageError* error) { accept.transports, accept.groups)); // Updating transport with TransportDescription. - PushdownTransportDescription(CS_REMOTE, CA_ANSWER); + PushdownTransportDescription(CS_REMOTE, CA_ANSWER, NULL); MaybeEnableMuxingSupport(); // Enable transport channel mux if supported. SetState(STATE_RECEIVEDACCEPT); @@ -1496,8 +1516,8 @@ bool Session::CheckState(State expected, MessageError* error) { return true; } -void Session::SetError(Error error) { - BaseSession::SetError(error); +void Session::SetError(Error error, const std::string& error_desc) { + BaseSession::SetError(error, error_desc); if (error != ERROR_NONE) signaling_thread()->Post(this, MSG_ERROR); } diff --git a/talk/p2p/base/session.h b/talk/p2p/base/session.h index 637c9424b7..8386434f94 100644 --- a/talk/p2p/base/session.h +++ b/talk/p2p/base/session.h @@ -145,9 +145,11 @@ class TransportProxy : public sigslot::has_slots<>, void SetIceRole(IceRole role); void SetIdentity(talk_base::SSLIdentity* identity); bool SetLocalTransportDescription(const TransportDescription& description, - ContentAction action); + ContentAction action, + std::string* error_desc); bool SetRemoteTransportDescription(const TransportDescription& description, - ContentAction action); + ContentAction action, + std::string* error_desc); void OnSignalingReady(); bool OnRemoteCandidates(const Candidates& candidates, std::string* error); @@ -327,13 +329,15 @@ class BaseSession : public sigslot::has_slots<>, // Returns the last error in the session. See the enum above for details. // Each time the an error occurs, we will fire this signal. Error error() const { return error_; } + const std::string& error_desc() const { return error_desc_; } sigslot::signal2<BaseSession* , Error> SignalError; // Updates the state, signaling if necessary. virtual void SetState(State state); // Updates the error state, signaling if necessary. - virtual void SetError(Error error); + // TODO(ronghuawu): remove the SetError method that doesn't take |error_desc|. + virtual void SetError(Error error, const std::string& error_desc); // Fired when the remote description is updated, with the updated // contents. @@ -384,7 +388,8 @@ class BaseSession : public sigslot::has_slots<>, bool SetIdentity(talk_base::SSLIdentity* identity); bool PushdownTransportDescription(ContentSource source, - ContentAction action); + ContentAction action, + std::string* error_desc); void set_initiator(bool initiator) { initiator_ = initiator; } const TransportMap& transport_proxies() const { return transports_; } @@ -466,13 +471,16 @@ class BaseSession : public sigslot::has_slots<>, protected: State state_; Error error_; + std::string error_desc_; private: // Helper methods to push local and remote transport descriptions. bool PushdownLocalTransportDescription( - const SessionDescription* sdesc, ContentAction action); + const SessionDescription* sdesc, ContentAction action, + std::string* error_desc); bool PushdownRemoteTransportDescription( - const SessionDescription* sdesc, ContentAction action); + const SessionDescription* sdesc, ContentAction action, + std::string* error_desc); bool IsCandidateAllocationDone() const; void MaybeCandidateAllocationDone(); @@ -553,7 +561,7 @@ class Session : public BaseSession { } // Updates the error state, signaling if necessary. - virtual void SetError(Error error); + virtual void SetError(Error error, const std::string& error_desc); // When the session needs to send signaling messages, it beings by requesting // signaling. The client should handle this by calling OnSignalingReady once diff --git a/talk/p2p/base/transport.cc b/talk/p2p/base/transport.cc index 4404c081a8..d177833ba8 100644 --- a/talk/p2p/base/transport.cc +++ b/talk/p2p/base/transport.cc @@ -73,6 +73,33 @@ struct ChannelParams : public talk_base::MessageData { Candidate* candidate; }; +static std::string IceProtoToString(TransportProtocol proto) { + std::string proto_str; + switch (proto) { + case ICEPROTO_GOOGLE: + proto_str = "gice"; + break; + case ICEPROTO_HYBRID: + proto_str = "hybrid"; + break; + case ICEPROTO_RFC5245: + proto_str = "ice"; + break; + default: + ASSERT(false); + break; + } + return proto_str; +} + +bool BadTransportDescription(const std::string& desc, std::string* err_desc) { + if (err_desc) { + *err_desc = desc; + } + LOG(LS_ERROR) << desc; + return false; +} + Transport::Transport(talk_base::Thread* signaling_thread, talk_base::Thread* worker_thread, const std::string& content_name, @@ -131,15 +158,21 @@ bool Transport::GetRemoteCertificate_w(talk_base::SSLCertificate** cert) { } bool Transport::SetLocalTransportDescription( - const TransportDescription& description, ContentAction action) { + const TransportDescription& description, + ContentAction action, + std::string* error_desc) { return worker_thread_->Invoke<bool>(Bind( - &Transport::SetLocalTransportDescription_w, this, description, action)); + &Transport::SetLocalTransportDescription_w, this, + description, action, error_desc)); } bool Transport::SetRemoteTransportDescription( - const TransportDescription& description, ContentAction action) { + const TransportDescription& description, + ContentAction action, + std::string* error_desc) { return worker_thread_->Invoke<bool>(Bind( - &Transport::SetRemoteTransportDescription_w, this, description, action)); + &Transport::SetRemoteTransportDescription_w, this, + description, action, error_desc)); } TransportChannelImpl* Transport::CreateChannel(int component) { @@ -176,10 +209,12 @@ TransportChannelImpl* Transport::CreateChannel_w(int component) { impl->SetIceRole(ice_role_); impl->SetIceTiebreaker(tiebreaker_); if (local_description_) { - ApplyLocalTransportDescription_w(impl); + // TODO(ronghuawu): Change CreateChannel_w to be able to return error since + // below Apply**Description_w calls can fail. + ApplyLocalTransportDescription_w(impl, NULL); if (remote_description_) { - ApplyRemoteTransportDescription_w(impl); - ApplyNegotiatedTransportDescription_w(impl); + ApplyRemoteTransportDescription_w(impl, NULL); + ApplyNegotiatedTransportDescription_w(impl, NULL); } } @@ -276,7 +311,7 @@ void Transport::ConnectChannels_w() { talk_base::CreateRandomString(ICE_PWD_LENGTH), ICEMODE_FULL, CONNECTIONROLE_NONE, NULL, Candidates()); - SetLocalTransportDescription_w(desc, CA_OFFER); + SetLocalTransportDescription_w(desc, CA_OFFER, NULL); } CallChannels_w(&TransportChannelImpl::Connect); @@ -606,63 +641,70 @@ void Transport::SetRemoteIceMode_w(IceMode mode) { } bool Transport::SetLocalTransportDescription_w( - const TransportDescription& desc, ContentAction action) { + const TransportDescription& desc, + ContentAction action, + std::string* error_desc) { bool ret = true; talk_base::CritScope cs(&crit_); local_description_.reset(new TransportDescription(desc)); for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { - ret &= ApplyLocalTransportDescription_w(iter->second.get()); + ret &= ApplyLocalTransportDescription_w(iter->second.get(), error_desc); } if (!ret) return false; // If PRANSWER/ANSWER is set, we should decide transport protocol type. if (action == CA_PRANSWER || action == CA_ANSWER) { - ret &= NegotiateTransportDescription_w(action); + ret &= NegotiateTransportDescription_w(action, error_desc); } return ret; } bool Transport::SetRemoteTransportDescription_w( - const TransportDescription& desc, ContentAction action) { + const TransportDescription& desc, + ContentAction action, + std::string* error_desc) { bool ret = true; talk_base::CritScope cs(&crit_); remote_description_.reset(new TransportDescription(desc)); for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { - ret &= ApplyRemoteTransportDescription_w(iter->second.get()); + ret &= ApplyRemoteTransportDescription_w(iter->second.get(), error_desc); } // If PRANSWER/ANSWER is set, we should decide transport protocol type. if (action == CA_PRANSWER || action == CA_ANSWER) { - ret = NegotiateTransportDescription_w(CA_OFFER); + ret = NegotiateTransportDescription_w(CA_OFFER, error_desc); } return ret; } -bool Transport::ApplyLocalTransportDescription_w(TransportChannelImpl* ch) { +bool Transport::ApplyLocalTransportDescription_w(TransportChannelImpl* ch, + std::string* error_desc) { ch->SetIceCredentials(local_description_->ice_ufrag, local_description_->ice_pwd); return true; } -bool Transport::ApplyRemoteTransportDescription_w(TransportChannelImpl* ch) { +bool Transport::ApplyRemoteTransportDescription_w(TransportChannelImpl* ch, + std::string* error_desc) { ch->SetRemoteIceCredentials(remote_description_->ice_ufrag, remote_description_->ice_pwd); return true; } bool Transport::ApplyNegotiatedTransportDescription_w( - TransportChannelImpl* channel) { + TransportChannelImpl* channel, std::string* error_desc) { channel->SetIceProtocolType(protocol_); channel->SetRemoteIceMode(remote_ice_mode_); return true; } -bool Transport::NegotiateTransportDescription_w(ContentAction local_role) { +bool Transport::NegotiateTransportDescription_w(ContentAction local_role, + std::string* error_desc) { // TODO(ekr@rtfm.com): This is ICE-specific stuff. Refactor into // P2PTransport. const TransportDescription* offer; @@ -690,7 +732,12 @@ bool Transport::NegotiateTransportDescription_w(ContentAction local_role) { // answer must be treated as error. if ((offer_proto == ICEPROTO_GOOGLE || offer_proto == ICEPROTO_RFC5245) && (offer_proto != answer_proto)) { - return false; + std::ostringstream desc; + desc << "Offer and answer protocol mismatch: " + << IceProtoToString(offer_proto) + << " vs " + << IceProtoToString(answer_proto); + return BadTransportDescription(desc.str(), error_desc); } protocol_ = answer_proto == ICEPROTO_HYBRID ? ICEPROTO_GOOGLE : answer_proto; @@ -712,7 +759,7 @@ bool Transport::NegotiateTransportDescription_w(ContentAction local_role) { for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { - if (!ApplyNegotiatedTransportDescription_w(iter->second.get())) + if (!ApplyNegotiatedTransportDescription_w(iter->second.get(), error_desc)) return false; } return true; diff --git a/talk/p2p/base/transport.h b/talk/p2p/base/transport.h index f9e9d88745..49ab0ea97d 100644 --- a/talk/p2p/base/transport.h +++ b/talk/p2p/base/transport.h @@ -187,6 +187,8 @@ struct TransportStats { TransportChannelStatsList channel_stats; }; +bool BadTransportDescription(const std::string& desc, std::string* err_desc); + class Transport : public talk_base::MessageHandler, public sigslot::has_slots<> { public: @@ -270,11 +272,13 @@ class Transport : public talk_base::MessageHandler, // Set the local TransportDescription to be used by TransportChannels. // This should be called before ConnectChannels(). bool SetLocalTransportDescription(const TransportDescription& description, - ContentAction action); + ContentAction action, + std::string* error_desc); // Set the remote TransportDescription to be used by TransportChannels. bool SetRemoteTransportDescription(const TransportDescription& description, - ContentAction action); + ContentAction action, + std::string* error_desc); // Tells all current and future channels to start connecting. When the first // channel begins connecting, the following signal is raised. @@ -362,25 +366,27 @@ class Transport : public talk_base::MessageHandler, // Pushes down the transport parameters from the local description, such // as the ICE ufrag and pwd. // Derived classes can override, but must call the base as well. - virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* - channel); + virtual bool ApplyLocalTransportDescription_w(TransportChannelImpl* channel, + std::string* error_desc); // Pushes down remote ice credentials from the remote description to the // transport channel. - virtual bool ApplyRemoteTransportDescription_w(TransportChannelImpl* ch); + virtual bool ApplyRemoteTransportDescription_w(TransportChannelImpl* ch, + std::string* error_desc); // Negotiates the transport parameters based on the current local and remote // transport description, such at the version of ICE to use, and whether DTLS // should be activated. // Derived classes can negotiate their specific parameters here, but must call // the base as well. - virtual bool NegotiateTransportDescription_w(ContentAction local_role); + virtual bool NegotiateTransportDescription_w(ContentAction local_role, + std::string* error_desc); // Pushes down the transport parameters obtained via negotiation. // Derived classes can set their specific parameters here, but must call the // base as well. virtual bool ApplyNegotiatedTransportDescription_w( - TransportChannelImpl* channel); + TransportChannelImpl* channel, std::string* error_desc); virtual bool GetSslRole_w(talk_base::SSLRole* ssl_role) const { return false; @@ -468,9 +474,11 @@ class Transport : public talk_base::MessageHandler, void SetIceRole_w(IceRole role); void SetRemoteIceMode_w(IceMode mode); bool SetLocalTransportDescription_w(const TransportDescription& desc, - ContentAction action); + ContentAction action, + std::string* error_desc); bool SetRemoteTransportDescription_w(const TransportDescription& desc, - ContentAction action); + ContentAction action, + std::string* error_desc); bool GetStats_w(TransportStats* infos); bool GetRemoteCertificate_w(talk_base::SSLCertificate** cert); diff --git a/talk/p2p/base/transport_unittest.cc b/talk/p2p/base/transport_unittest.cc index e3b7badfff..c7cbc497fd 100644 --- a/talk/p2p/base/transport_unittest.cc +++ b/talk/p2p/base/transport_unittest.cc @@ -147,7 +147,8 @@ TEST_F(TransportTest, TestChannelIceParameters) { cricket::TransportDescription local_desc( cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, - cricket::CA_OFFER)); + cricket::CA_OFFER, + NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); EXPECT_TRUE(SetupChannel()); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); @@ -158,7 +159,8 @@ TEST_F(TransportTest, TestChannelIceParameters) { cricket::TransportDescription remote_desc( cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, - cricket::CA_ANSWER)); + cricket::CA_ANSWER, + NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); EXPECT_EQ(99U, channel_->IceTiebreaker()); EXPECT_EQ(cricket::ICEMODE_FULL, channel_->remote_ice_mode()); @@ -178,11 +180,13 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInOffer) { kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, cricket::CONNECTIONROLE_ACTPASS, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, - cricket::CA_OFFER)); + cricket::CA_OFFER, + NULL)); cricket::TransportDescription local_desc( cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, - cricket::CA_ANSWER)); + cricket::CA_ANSWER, + NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); EXPECT_TRUE(SetupChannel()); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); @@ -195,7 +199,8 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { cricket::TransportDescription local_desc( cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, - cricket::CA_OFFER)); + cricket::CA_OFFER, + NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); EXPECT_TRUE(SetupChannel()); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); @@ -206,7 +211,8 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, cricket::CONNECTIONROLE_NONE, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, - cricket::CA_ANSWER)); + cricket::CA_ANSWER, + NULL)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); // After receiving remote description with ICEMODE_LITE, channel should // have mode set to ICEMODE_LITE. diff --git a/talk/p2p/base/turnport.cc b/talk/p2p/base/turnport.cc index 01d7f9c899..d8c0cb6e29 100644 --- a/talk/p2p/base/turnport.cc +++ b/talk/p2p/base/turnport.cc @@ -195,6 +195,9 @@ TurnPort::~TurnPort() { while (!entries_.empty()) { DestroyEntry(entries_.front()->address()); } + if (resolver_) { + resolver_->Destroy(false); + } } void TurnPort::PrepareAddress() { diff --git a/talk/session/media/call.cc b/talk/session/media/call.cc index 967846bd21..5939bff568 100644 --- a/talk/session/media/call.cc +++ b/talk/session/media/call.cc @@ -523,7 +523,7 @@ bool Call::StartScreencast(Session* session, VideoContentDescription* video = CreateVideoStreamUpdate(stream); // TODO(pthatcher): Wait until view request before sending video. - video_channel->SetLocalContent(video, CA_UPDATE); + video_channel->SetLocalContent(video, CA_UPDATE, NULL); SendVideoStreamUpdate(session, video); return true; } @@ -546,7 +546,7 @@ bool Call::StopScreencast(Session* session, // No ssrcs VideoContentDescription* video = CreateVideoStreamUpdate(stream); - video_channel->SetLocalContent(video, CA_UPDATE); + video_channel->SetLocalContent(video, CA_UPDATE, NULL); SendVideoStreamUpdate(session, video); return true; } @@ -870,9 +870,11 @@ void Call::OnRemoteDescriptionUpdate(BaseSession* base_session, bool Call::UpdateVoiceChannelRemoteContent( Session* session, const AudioContentDescription* audio) { VoiceChannel* voice_channel = GetVoiceChannel(session); - if (!voice_channel->SetRemoteContent(audio, CA_UPDATE)) { - LOG(LS_ERROR) << "Failure in audio SetRemoteContent with CA_UPDATE"; - session->SetError(BaseSession::ERROR_CONTENT); + if (!voice_channel->SetRemoteContent(audio, CA_UPDATE, NULL)) { + const std::string error_desc = + "Failure in audio SetRemoteContent with CA_UPDATE"; + LOG(LS_ERROR) << error_desc; + session->SetError(BaseSession::ERROR_CONTENT, error_desc); return false; } return true; @@ -881,9 +883,11 @@ bool Call::UpdateVoiceChannelRemoteContent( bool Call::UpdateVideoChannelRemoteContent( Session* session, const VideoContentDescription* video) { VideoChannel* video_channel = GetVideoChannel(session); - if (!video_channel->SetRemoteContent(video, CA_UPDATE)) { - LOG(LS_ERROR) << "Failure in video SetRemoteContent with CA_UPDATE"; - session->SetError(BaseSession::ERROR_CONTENT); + if (!video_channel->SetRemoteContent(video, CA_UPDATE, NULL)) { + const std::string error_desc = + "Failure in video SetRemoteContent with CA_UPDATE"; + LOG(LS_ERROR) << error_desc; + session->SetError(BaseSession::ERROR_CONTENT, error_desc); return false; } return true; @@ -892,9 +896,11 @@ bool Call::UpdateVideoChannelRemoteContent( bool Call::UpdateDataChannelRemoteContent( Session* session, const DataContentDescription* data) { DataChannel* data_channel = GetDataChannel(session); - if (!data_channel->SetRemoteContent(data, CA_UPDATE)) { - LOG(LS_ERROR) << "Failure in data SetRemoteContent with CA_UPDATE"; - session->SetError(BaseSession::ERROR_CONTENT); + if (!data_channel->SetRemoteContent(data, CA_UPDATE, NULL)) { + const std::string error_desc = + "Failure in data SetRemoteContent with CA_UPDATE"; + LOG(LS_ERROR) << error_desc; + session->SetError(BaseSession::ERROR_CONTENT, error_desc); return false; } return true; diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 61bc3d796e..9a76d51f8b 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -60,7 +60,6 @@ enum { MSG_REMOVESENDSTREAM, MSG_SETRINGBACKTONE, MSG_PLAYRINGBACKTONE, - MSG_SETMAXSENDBANDWIDTH, MSG_ADDSCREENCAST, MSG_REMOVESCREENCAST, MSG_SENDINTRAFRAME, @@ -88,6 +87,17 @@ static const char kDtlsSrtpExporterLabel[] = "EXTRACTOR-dtls_srtp"; static const int kAgcMinus10db = -10; +static void SetSessionError(BaseSession* session, BaseSession::Error error, + const std::string& error_desc) { + session->SetError(error, error_desc); +} + +static void SafeSetError(const std::string& message, std::string* error_desc) { + if (error_desc) { + *error_desc = message; + } +} + // TODO(hellner): use the device manager for creation of screen capturers when // the cl enabling it has landed. class NullScreenCapturerFactory : public VideoChannel::ScreenCapturerFactory { @@ -103,13 +113,17 @@ VideoChannel::ScreenCapturerFactory* CreateScreenCapturerFactory() { } struct SetContentData : public talk_base::MessageData { - SetContentData(const MediaContentDescription* content, ContentAction action) + SetContentData(const MediaContentDescription* content, + ContentAction action, + std::string* error_desc) : content(content), action(action), + error_desc(error_desc), result(false) { } const MediaContentDescription* content; ContentAction action; + std::string* error_desc; bool result; }; @@ -273,10 +287,13 @@ struct DataChannelErrorMessageData : public talk_base::MessageData { }; struct SessionErrorMessageData : public talk_base::MessageData { - explicit SessionErrorMessageData(cricket::BaseSession::Error error) - : error_(error) {} + SessionErrorMessageData(cricket::BaseSession::Error error, + const std::string& error_desc) + : error_(error), + error_desc_(error_desc) {} BaseSession::Error error_; + std::string error_desc_; }; struct SsrcMessageData : public talk_base::MessageData { @@ -505,25 +522,21 @@ bool BaseChannel::RemoveSendStream(uint32 ssrc) { } bool BaseChannel::SetLocalContent(const MediaContentDescription* content, - ContentAction action) { - SetContentData data(content, action); + ContentAction action, + std::string* error_desc) { + SetContentData data(content, action, error_desc); Send(MSG_SETLOCALCONTENT, &data); return data.result; } bool BaseChannel::SetRemoteContent(const MediaContentDescription* content, - ContentAction action) { - SetContentData data(content, action); + ContentAction action, + std::string* error_desc) { + SetContentData data(content, action, error_desc); Send(MSG_SETREMOTECONTENT, &data); return data.result; } -bool BaseChannel::SetMaxSendBandwidth(int max_bandwidth) { - SetBandwidthData data(max_bandwidth); - Send(MSG_SETMAXSENDBANDWIDTH, &data); - return data.result; -} - void BaseChannel::StartConnectionMonitor(int cms) { socket_monitor_.reset(new SocketMonitor(transport_channel_, worker_thread(), @@ -857,10 +870,11 @@ void BaseChannel::OnNewLocalDescription( GetFirstContent(session->local_description()); const MediaContentDescription* content_desc = GetContentDescription(content_info); + std::string error_desc; if (content_desc && content_info && !content_info->rejected && - !SetLocalContent(content_desc, action)) { + !SetLocalContent(content_desc, action, &error_desc)) { + SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc); LOG(LS_ERROR) << "Failure in SetLocalContent with action " << action; - session->SetError(BaseSession::ERROR_CONTENT); } } @@ -870,10 +884,11 @@ void BaseChannel::OnNewRemoteDescription( GetFirstContent(session->remote_description()); const MediaContentDescription* content_desc = GetContentDescription(content_info); + std::string error_desc; if (content_desc && content_info && !content_info->rejected && - !SetRemoteContent(content_desc, action)) { - LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action; - session->SetError(BaseSession::ERROR_CONTENT); + !SetRemoteContent(content_desc, action, &error_desc)) { + SetSessionError(session_, BaseSession::ERROR_CONTENT, error_desc); + LOG(LS_ERROR) << "Failure in SetRemoteContent with action " << action; } } @@ -938,8 +953,9 @@ void BaseChannel::ChannelWritable_w() { // If we're doing DTLS-SRTP, now is the time. if (!was_ever_writable_ && ShouldSetupDtlsSrtp()) { if (!SetupDtlsSrtp(false)) { - LOG(LS_ERROR) << "Couldn't finish DTLS-SRTP on RTP channel"; - SessionErrorMessageData data(BaseSession::ERROR_TRANSPORT); + const std::string error_desc = + "Couldn't set up DTLS-SRTP on RTP channel."; + SessionErrorMessageData data(BaseSession::ERROR_TRANSPORT, error_desc); // Sent synchronously. signaling_thread()->Send(this, MSG_SESSION_ERROR, &data); return; @@ -947,8 +963,9 @@ void BaseChannel::ChannelWritable_w() { if (rtcp_transport_channel_) { if (!SetupDtlsSrtp(true)) { - LOG(LS_ERROR) << "Couldn't finish DTLS-SRTP on RTCP channel"; - SessionErrorMessageData data(BaseSession::ERROR_TRANSPORT); + const std::string error_desc = + "Couldn't set up DTLS-SRTP on RTCP channel"; + SessionErrorMessageData data(BaseSession::ERROR_TRANSPORT, error_desc); // Sent synchronously. signaling_thread()->Send(this, MSG_SESSION_ERROR, &data); return; @@ -1085,62 +1102,69 @@ void BaseChannel::ChannelNotWritable_w() { ChangeState(); } -// Sets the maximum video bandwidth for automatic bandwidth adjustment. -bool BaseChannel::SetMaxSendBandwidth_w(int max_bandwidth) { - return media_channel()->SetSendBandwidth(true, max_bandwidth); -} - // |dtls| will be set to true if DTLS is active for transport channel and // crypto is empty. bool BaseChannel::CheckSrtpConfig(const std::vector<CryptoParams>& cryptos, - bool* dtls) { + bool* dtls, + std::string* error_desc) { *dtls = transport_channel_->IsDtlsActive(); if (*dtls && !cryptos.empty()) { - LOG(LS_WARNING) << "Cryptos must be empty when DTLS is active."; + SafeSetError("Cryptos must be empty when DTLS is active.", + error_desc); return false; } return true; } bool BaseChannel::SetSrtp_w(const std::vector<CryptoParams>& cryptos, - ContentAction action, ContentSource src) { + ContentAction action, + ContentSource src, + std::string* error_desc) { + if (action == CA_UPDATE) { + // no crypto params. + return true; + } bool ret = false; bool dtls = false; - ret = CheckSrtpConfig(cryptos, &dtls); + ret = CheckSrtpConfig(cryptos, &dtls, error_desc); + if (!ret) { + return false; + } switch (action) { case CA_OFFER: // If DTLS is already active on the channel, we could be renegotiating // here. We don't update the srtp filter. - if (ret && !dtls) { + if (!dtls) { ret = srtp_filter_.SetOffer(cryptos, src); } break; case CA_PRANSWER: // If we're doing DTLS-SRTP, we don't want to update the filter // with an answer, because we already have SRTP parameters. - if (ret && !dtls) { + if (!dtls) { ret = srtp_filter_.SetProvisionalAnswer(cryptos, src); } break; case CA_ANSWER: // If we're doing DTLS-SRTP, we don't want to update the filter // with an answer, because we already have SRTP parameters. - if (ret && !dtls) { + if (!dtls) { ret = srtp_filter_.SetAnswer(cryptos, src); } break; - case CA_UPDATE: - // no crypto params. - ret = true; - break; default: break; } - return ret; + if (!ret) { + SafeSetError("Failed to setup SRTP filter.", error_desc); + return false; + } + return true; } bool BaseChannel::SetRtcpMux_w(bool enable, ContentAction action, - ContentSource src) { + ContentSource src, + std::string* error_desc) { bool ret = false; switch (action) { case CA_OFFER: @@ -1162,17 +1186,21 @@ bool BaseChannel::SetRtcpMux_w(bool enable, ContentAction action, default: break; } + if (!ret) { + SafeSetError("Failed to setup RTCP mux filter.", error_desc); + return false; + } // |rtcp_mux_filter_| can be active if |action| is CA_PRANSWER or // CA_ANSWER, but we only want to tear down the RTCP transport channel if we // received a final answer. - if (ret && rtcp_mux_filter_.IsActive()) { + if (rtcp_mux_filter_.IsActive()) { // If the RTP transport is already writable, then so are we. if (transport_channel_->writable()) { ChannelWritable_w(); } } - return ret; + return true; } bool BaseChannel::AddRecvStream_w(const StreamParams& sp) { @@ -1200,7 +1228,8 @@ bool BaseChannel::RemoveSendStream_w(uint32 ssrc) { } bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams, - ContentAction action) { + ContentAction action, + std::string* error_desc) { if (!VERIFY(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER || action == CA_UPDATE)) return false; @@ -1217,15 +1246,18 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams, local_streams_.push_back(*it); LOG(LS_INFO) << "Add send stream ssrc: " << it->first_ssrc(); } else { - LOG(LS_INFO) << "Failed to add send stream ssrc: " - << it->first_ssrc(); + std::ostringstream desc; + desc << "Failed to add send stream ssrc: " << it->first_ssrc(); + SafeSetError(desc.str(), error_desc); return false; } } else if (stream_exist && !it->has_ssrcs()) { if (!media_channel()->RemoveSendStream(existing_stream.first_ssrc())) { - LOG(LS_ERROR) << "Failed to remove send stream with ssrc " - << it->first_ssrc() << "."; - return false; + std::ostringstream desc; + desc << "Failed to remove send stream with ssrc " + << it->first_ssrc() << "."; + SafeSetError(desc.str(), error_desc); + return false; } RemoveStreamBySsrc(&local_streams_, existing_stream.first_ssrc()); } else { @@ -1242,8 +1274,10 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams, it != local_streams_.end(); ++it) { if (!GetStreamBySsrc(streams, it->first_ssrc(), NULL)) { if (!media_channel()->RemoveSendStream(it->first_ssrc())) { - LOG(LS_ERROR) << "Failed to remove send stream with ssrc " - << it->first_ssrc() << "."; + std::ostringstream desc; + desc << "Failed to remove send stream with ssrc " + << it->first_ssrc() << "."; + SafeSetError(desc.str(), error_desc); ret = false; } } @@ -1255,7 +1289,9 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams, if (media_channel()->AddSendStream(*it)) { LOG(LS_INFO) << "Add send ssrc: " << it->ssrcs[0]; } else { - LOG(LS_INFO) << "Failed to add send stream ssrc: " << it->first_ssrc(); + std::ostringstream desc; + desc << "Failed to add send stream ssrc: " << it->first_ssrc(); + SafeSetError(desc.str(), error_desc); ret = false; } } @@ -1266,7 +1302,8 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams, bool BaseChannel::UpdateRemoteStreams_w( const std::vector<StreamParams>& streams, - ContentAction action) { + ContentAction action, + std::string* error_desc) { if (!VERIFY(action == CA_OFFER || action == CA_ANSWER || action == CA_PRANSWER || action == CA_UPDATE)) return false; @@ -1283,15 +1320,18 @@ bool BaseChannel::UpdateRemoteStreams_w( remote_streams_.push_back(*it); LOG(LS_INFO) << "Add remote stream ssrc: " << it->first_ssrc(); } else { - LOG(LS_INFO) << "Failed to add remote stream ssrc: " - << it->first_ssrc(); + std::ostringstream desc; + desc << "Failed to add remote stream ssrc: " << it->first_ssrc(); + SafeSetError(desc.str(), error_desc); return false; } } else if (stream_exists && !it->has_ssrcs()) { if (!RemoveRecvStream_w(existing_stream.first_ssrc())) { - LOG(LS_ERROR) << "Failed to remove remote stream with ssrc " - << it->first_ssrc() << "."; - return false; + std::ostringstream desc; + desc << "Failed to remove remote stream with ssrc " + << it->first_ssrc() << "."; + SafeSetError(desc.str(), error_desc); + return false; } RemoveStreamBySsrc(&remote_streams_, existing_stream.first_ssrc()); } else { @@ -1311,8 +1351,10 @@ bool BaseChannel::UpdateRemoteStreams_w( it != remote_streams_.end(); ++it) { if (!GetStreamBySsrc(streams, it->first_ssrc(), NULL)) { if (!RemoveRecvStream_w(it->first_ssrc())) { - LOG(LS_ERROR) << "Failed to remove remote stream with ssrc " - << it->first_ssrc() << "."; + std::ostringstream desc; + desc << "Failed to remove remote stream with ssrc " + << it->first_ssrc() << "."; + SafeSetError(desc.str(), error_desc); ret = false; } } @@ -1324,8 +1366,9 @@ bool BaseChannel::UpdateRemoteStreams_w( if (AddRecvStream_w(*it)) { LOG(LS_INFO) << "Add remote ssrc: " << it->ssrcs[0]; } else { - LOG(LS_INFO) << "Failed to add remote stream ssrc: " - << it->first_ssrc(); + std::ostringstream desc; + desc << "Failed to add remote stream ssrc: " << it->first_ssrc(); + SafeSetError(desc.str(), error_desc); ret = false; } } @@ -1335,37 +1378,56 @@ bool BaseChannel::UpdateRemoteStreams_w( } bool BaseChannel::SetBaseLocalContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { // Cache secure_required_ for belt and suspenders check on SendPacket secure_required_ = content->crypto_required(); - bool ret = UpdateLocalStreams_w(content->streams(), action); + bool ret = UpdateLocalStreams_w(content->streams(), action, error_desc); // Set local SRTP parameters (what we will encrypt with). - ret &= SetSrtp_w(content->cryptos(), action, CS_LOCAL); + ret &= SetSrtp_w(content->cryptos(), action, CS_LOCAL, error_desc); // Set local RTCP mux parameters. - ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_LOCAL); + ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_LOCAL, error_desc); // Set local RTP header extensions. if (content->rtp_header_extensions_set()) { - ret &= media_channel()->SetRecvRtpHeaderExtensions( - content->rtp_header_extensions()); + if (!media_channel()->SetRecvRtpHeaderExtensions( + content->rtp_header_extensions())) { + std::ostringstream desc; + desc << "Failed to set receive rtp header extensions for " + << MediaTypeToString(content->type()) << " content."; + SafeSetError(desc.str(), error_desc); + ret = false; + } } set_local_content_direction(content->direction()); return ret; } bool BaseChannel::SetBaseRemoteContent_w(const MediaContentDescription* content, - ContentAction action) { - bool ret = UpdateRemoteStreams_w(content->streams(), action); + ContentAction action, + std::string* error_desc) { + bool ret = UpdateRemoteStreams_w(content->streams(), action, error_desc); // Set remote SRTP parameters (what the other side will encrypt with). - ret &= SetSrtp_w(content->cryptos(), action, CS_REMOTE); + ret &= SetSrtp_w(content->cryptos(), action, CS_REMOTE, error_desc); // Set remote RTCP mux parameters. - ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_REMOTE); + ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_REMOTE, error_desc); // Set remote RTP header extensions. if (content->rtp_header_extensions_set()) { - ret &= media_channel()->SetSendRtpHeaderExtensions( - content->rtp_header_extensions()); + if (!media_channel()->SetSendRtpHeaderExtensions( + content->rtp_header_extensions())) { + std::ostringstream desc; + desc << "Failed to set send rtp header extensions for " + << MediaTypeToString(content->type()) << " content."; + SafeSetError(desc.str(), error_desc); + ret = false; + } } - if (content->bandwidth() != kAutoBandwidth) { - ret &= media_channel()->SetSendBandwidth(false, content->bandwidth()); + + if (!media_channel()->SetMaxSendBandwidth(content->bandwidth())) { + std::ostringstream desc; + desc << "Failed to set max send bandwidth for " + << MediaTypeToString(content->type()) << " content."; + SafeSetError(desc.str(), error_desc); + ret = false; } set_remote_content_direction(content->direction()); return ret; @@ -1391,12 +1453,16 @@ void BaseChannel::OnMessage(talk_base::Message *pmsg) { } case MSG_SETLOCALCONTENT: { SetContentData* data = static_cast<SetContentData*>(pmsg->pdata); - data->result = SetLocalContent_w(data->content, data->action); + data->result = SetLocalContent_w(data->content, + data->action, + data->error_desc); break; } case MSG_SETREMOTECONTENT: { SetContentData* data = static_cast<SetContentData*>(pmsg->pdata); - data->result = SetRemoteContent_w(data->content, data->action); + data->result = SetRemoteContent_w(data->content, + data->action, + data->error_desc); break; } case MSG_ADDRECVSTREAM: { @@ -1419,11 +1485,6 @@ void BaseChannel::OnMessage(talk_base::Message *pmsg) { data->result = RemoveSendStream_w(data->ssrc); break; } - case MSG_SETMAXSENDBANDWIDTH: { - SetBandwidthData* data = static_cast<SetBandwidthData*>(pmsg->pdata); - data->result = SetMaxSendBandwidth_w(data->value); - break; - } case MSG_RTPPACKET: case MSG_RTCPPACKET: { @@ -1439,7 +1500,7 @@ void BaseChannel::OnMessage(talk_base::Message *pmsg) { case MSG_SESSION_ERROR: { SessionErrorMessageData* data = static_cast<SessionErrorMessageData*> (pmsg->pdata); - session_->SetError(data->error_); + SetSessionError(session_, data->error_, data->error_desc_); break; } } @@ -1685,21 +1746,28 @@ const ContentInfo* VoiceChannel::GetFirstContent( } bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); LOG(LS_INFO) << "Setting local voice description"; const AudioContentDescription* audio = static_cast<const AudioContentDescription*>(content); ASSERT(audio != NULL); - if (!audio) return false; + if (!audio) { + SafeSetError("Can't find audio content in local description.", error_desc); + return false; + } - bool ret = SetBaseLocalContent_w(content, action); + bool ret = SetBaseLocalContent_w(content, action, error_desc); // Set local audio codecs (what we want to receive). // TODO(whyuan): Change action != CA_UPDATE to !audio->partial() when partial // is set properly. if (action != CA_UPDATE || audio->has_codecs()) { - ret &= media_channel()->SetRecvCodecs(audio->codecs()); + if (!media_channel()->SetRecvCodecs(audio->codecs())) { + SafeSetError("Failed to set audio receive codecs.", error_desc); + ret = false; + } } // If everything worked, see if we can start receiving. @@ -1712,22 +1780,29 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, } bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); LOG(LS_INFO) << "Setting remote voice description"; const AudioContentDescription* audio = static_cast<const AudioContentDescription*>(content); ASSERT(audio != NULL); - if (!audio) return false; + if (!audio) { + SafeSetError("Can't find audio content in remote description.", error_desc); + return false; + } bool ret = true; // Set remote video codecs (what the other side wants to receive). if (action != CA_UPDATE || audio->has_codecs()) { - ret &= media_channel()->SetSendCodecs(audio->codecs()); + if (!media_channel()->SetSendCodecs(audio->codecs())) { + SafeSetError("Failed to set audio send codecs.", error_desc); + ret = false; + } } - ret &= SetBaseRemoteContent_w(content, action); + ret &= SetBaseRemoteContent_w(content, action, error_desc); if (action != CA_UPDATE) { // Tweak our audio processing settings, if needed. @@ -2104,19 +2179,26 @@ const ContentInfo* VideoChannel::GetFirstContent( } bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); LOG(LS_INFO) << "Setting local video description"; const VideoContentDescription* video = static_cast<const VideoContentDescription*>(content); ASSERT(video != NULL); - if (!video) return false; + if (!video) { + SafeSetError("Can't find video content in local description.", error_desc); + return false; + } - bool ret = SetBaseLocalContent_w(content, action); + bool ret = SetBaseLocalContent_w(content, action, error_desc); // Set local video codecs (what we want to receive). if (action != CA_UPDATE || video->has_codecs()) { - ret &= media_channel()->SetRecvCodecs(video->codecs()); + if (!media_channel()->SetRecvCodecs(video->codecs())) { + SafeSetError("Failed to set video receive codecs.", error_desc); + ret = false; + } } if (action != CA_UPDATE) { @@ -2140,22 +2222,29 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, } bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); LOG(LS_INFO) << "Setting remote video description"; const VideoContentDescription* video = static_cast<const VideoContentDescription*>(content); ASSERT(video != NULL); - if (!video) return false; + if (!video) { + SafeSetError("Can't find video content in remote description.", error_desc); + return false; + } bool ret = true; // Set remote video codecs (what the other side wants to receive). if (action != CA_UPDATE || video->has_codecs()) { - ret &= media_channel()->SetSendCodecs(video->codecs()); + if (!media_channel()->SetSendCodecs(video->codecs())) { + SafeSetError("Failed to set video send codecs.", error_desc); + ret = false; + } } - ret &= SetBaseRemoteContent_w(content, action); + ret &= SetBaseRemoteContent_w(content, action, error_desc); if (action != CA_UPDATE) { // Tweak our video processing settings, if needed. @@ -2553,13 +2642,8 @@ bool DataChannel::WantsPacket(bool rtcp, talk_base::Buffer* packet) { return false; } -// Sets the maximum bandwidth. Anything over this will be dropped. -bool DataChannel::SetMaxSendBandwidth_w(int max_bps) { - LOG(LS_INFO) << "DataChannel: Setting max bandwidth to " << max_bps; - return media_channel()->SetSendBandwidth(false, max_bps); -} - -bool DataChannel::SetDataChannelType(DataChannelType new_data_channel_type) { +bool DataChannel::SetDataChannelType(DataChannelType new_data_channel_type, + std::string* error_desc) { // It hasn't been set before, so set it now. if (data_channel_type_ == DCT_NONE) { data_channel_type_ = new_data_channel_type; @@ -2568,9 +2652,11 @@ bool DataChannel::SetDataChannelType(DataChannelType new_data_channel_type) { // It's been set before, but doesn't match. That's bad. if (data_channel_type_ != new_data_channel_type) { - LOG(LS_WARNING) << "Data channel type mismatch." - << " Expected " << data_channel_type_ - << " Got " << new_data_channel_type; + std::ostringstream desc; + desc << "Data channel type mismatch." + << " Expected " << data_channel_type_ + << " Got " << new_data_channel_type; + SafeSetError(desc.str(), error_desc); return false; } @@ -2579,42 +2665,53 @@ bool DataChannel::SetDataChannelType(DataChannelType new_data_channel_type) { } bool DataChannel::SetDataChannelTypeFromContent( - const DataContentDescription* content) { + const DataContentDescription* content, + std::string* error_desc) { bool is_sctp = ((content->protocol() == kMediaProtocolSctp) || (content->protocol() == kMediaProtocolDtlsSctp)); DataChannelType data_channel_type = is_sctp ? DCT_SCTP : DCT_RTP; - return SetDataChannelType(data_channel_type); + return SetDataChannelType(data_channel_type, error_desc); } bool DataChannel::SetLocalContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); LOG(LS_INFO) << "Setting local data description"; const DataContentDescription* data = static_cast<const DataContentDescription*>(content); ASSERT(data != NULL); - if (!data) return false; + if (!data) { + SafeSetError("Can't find data content in local description.", error_desc); + return false; + } bool ret = false; - if (!SetDataChannelTypeFromContent(data)) { + if (!SetDataChannelTypeFromContent(data, error_desc)) { return false; } if (data_channel_type_ == DCT_SCTP) { // SCTP data channels don't need the rest of the stuff. - ret = UpdateLocalStreams_w(data->streams(), action); + ret = UpdateLocalStreams_w(data->streams(), action, error_desc); if (ret) { set_local_content_direction(content->direction()); // As in SetRemoteContent_w, make sure we set the local SCTP port // number as specified in our DataContentDescription. - ret = media_channel()->SetRecvCodecs(data->codecs()); + if (!media_channel()->SetRecvCodecs(data->codecs())) { + SafeSetError("Failed to set data receive codecs.", error_desc); + ret = false; + } } } else { - ret = SetBaseLocalContent_w(content, action); + ret = SetBaseLocalContent_w(content, action, error_desc); if (action != CA_UPDATE || data->has_codecs()) { - ret &= media_channel()->SetRecvCodecs(data->codecs()); + if (!media_channel()->SetRecvCodecs(data->codecs())) { + SafeSetError("Failed to set data receive codecs.", error_desc); + ret = false; + } } } @@ -2628,28 +2725,35 @@ bool DataChannel::SetLocalContent_w(const MediaContentDescription* content, } bool DataChannel::SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action) { + ContentAction action, + std::string* error_desc) { ASSERT(worker_thread() == talk_base::Thread::Current()); const DataContentDescription* data = static_cast<const DataContentDescription*>(content); ASSERT(data != NULL); - if (!data) return false; + if (!data) { + SafeSetError("Can't find data content in remote description.", error_desc); + return false; + } bool ret = true; - if (!SetDataChannelTypeFromContent(data)) { + if (!SetDataChannelTypeFromContent(data, error_desc)) { return false; } if (data_channel_type_ == DCT_SCTP) { LOG(LS_INFO) << "Setting SCTP remote data description"; // SCTP data channels don't need the rest of the stuff. - ret = UpdateRemoteStreams_w(content->streams(), action); + ret = UpdateRemoteStreams_w(content->streams(), action, error_desc); if (ret) { set_remote_content_direction(content->direction()); // We send the SCTP port number (not to be confused with the underlying // UDP port number) as a codec parameter. Make sure it gets there. - ret = media_channel()->SetSendCodecs(data->codecs()); + if (!media_channel()->SetSendCodecs(data->codecs())) { + SafeSetError("Failed to set data send codecs.", error_desc); + ret = false; + } } } else { // If the remote data doesn't have codecs and isn't an update, it @@ -2661,17 +2765,24 @@ bool DataChannel::SetRemoteContent_w(const MediaContentDescription* content, // Set remote video codecs (what the other side wants to receive). if (action != CA_UPDATE || data->has_codecs()) { - ret &= media_channel()->SetSendCodecs(data->codecs()); + if (!media_channel()->SetSendCodecs(data->codecs())) { + SafeSetError("Failed to set data send codecs.", error_desc); + ret = false; + } } if (ret) { - ret &= SetBaseRemoteContent_w(content, action); + ret &= SetBaseRemoteContent_w(content, action, error_desc); } if (action != CA_UPDATE) { int bandwidth_bps = data->bandwidth(); - bool auto_bandwidth = (bandwidth_bps == kAutoBandwidth); - ret &= media_channel()->SetSendBandwidth(auto_bandwidth, bandwidth_bps); + if (!media_channel()->SetMaxSendBandwidth(bandwidth_bps)) { + std::ostringstream desc; + desc << "Failed to set max send bandwidth for data content."; + SafeSetError(desc.str(), error_desc); + ret = false; + } } } diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index bf9a837910..91ea6222d4 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -111,10 +111,11 @@ class BaseChannel // Channel control bool SetLocalContent(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); bool SetRemoteContent(const MediaContentDescription* content, - ContentAction action); - bool SetMaxSendBandwidth(int max_bandwidth); + ContentAction action, + std::string* error_desc); bool Enable(bool enable); // Mute sending media on the stream with SSRC |ssrc| @@ -306,24 +307,35 @@ class BaseChannel virtual const ContentInfo* GetFirstContent( const SessionDescription* sdesc) = 0; bool UpdateLocalStreams_w(const std::vector<StreamParams>& streams, - ContentAction action); + ContentAction action, + std::string* error_desc); bool UpdateRemoteStreams_w(const std::vector<StreamParams>& streams, - ContentAction action); + ContentAction action, + std::string* error_desc); bool SetBaseLocalContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action) = 0; + ContentAction action, + std::string* error_desc) = 0; bool SetBaseRemoteContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action) = 0; - - bool CheckSrtpConfig(const std::vector<CryptoParams>& cryptos, bool* dtls); - bool SetSrtp_w(const std::vector<CryptoParams>& params, ContentAction action, - ContentSource src); - bool SetRtcpMux_w(bool enable, ContentAction action, ContentSource src); - - virtual bool SetMaxSendBandwidth_w(int max_bandwidth); + ContentAction action, + std::string* error_desc) = 0; + + bool CheckSrtpConfig(const std::vector<CryptoParams>& cryptos, + bool* dtls, + std::string* error_desc); + bool SetSrtp_w(const std::vector<CryptoParams>& params, + ContentAction action, + ContentSource src, + std::string* error_desc); + bool SetRtcpMux_w(bool enable, + ContentAction action, + ContentSource src, + std::string* error_desc); // From MessageHandler virtual void OnMessage(talk_base::Message* pmsg); @@ -450,9 +462,11 @@ class VoiceChannel : public BaseChannel { virtual void ChangeState(); virtual const ContentInfo* GetFirstContent(const SessionDescription* sdesc); virtual bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); bool SetRingbackTone_w(const void* buf, int len); bool PlayRingbackTone_w(uint32 ssrc, bool play, bool loop); void HandleEarlyMediaTimeout(); @@ -549,9 +563,11 @@ class VideoChannel : public BaseChannel { virtual void ChangeState(); virtual const ContentInfo* GetFirstContent(const SessionDescription* sdesc); virtual bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); void SendIntraFrame_w() { media_channel()->SendIntraFrame(); } @@ -677,15 +693,18 @@ class DataChannel : public BaseChannel { // If data_channel_type_ is DCT_NONE, set it. Otherwise, check that // it's the same as what was set previously. Returns false if it's // set to one type one type and changed to another type later. - bool SetDataChannelType(DataChannelType new_data_channel_type); + bool SetDataChannelType(DataChannelType new_data_channel_type, + std::string* error_desc); // Same as SetDataChannelType, but extracts the type from the // DataContentDescription. - bool SetDataChannelTypeFromContent(const DataContentDescription* content); - virtual bool SetMaxSendBandwidth_w(int max_bandwidth); + bool SetDataChannelTypeFromContent(const DataContentDescription* content, + std::string* error_desc); virtual bool SetLocalContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual bool SetRemoteContent_w(const MediaContentDescription* content, - ContentAction action); + ContentAction action, + std::string* error_desc); virtual void ChangeState(); virtual bool WantsPacket(bool rtcp, talk_base::Buffer* packet); diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index 48a9bdef56..0a82bef412 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -318,14 +318,17 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { } bool SendInitiate() { - bool result = channel1_->SetLocalContent(&local_media_content1_, CA_OFFER); + bool result = channel1_->SetLocalContent(&local_media_content1_, + CA_OFFER, NULL); if (result) { channel1_->Enable(true); - result = channel2_->SetRemoteContent(&remote_media_content1_, CA_OFFER); + result = channel2_->SetRemoteContent(&remote_media_content1_, + CA_OFFER, NULL); if (result) { session1_.Connect(&session2_); - result = channel2_->SetLocalContent(&local_media_content2_, CA_ANSWER); + result = channel2_->SetLocalContent(&local_media_content2_, + CA_ANSWER, NULL); } } return result; @@ -333,34 +336,39 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { bool SendAccept() { channel2_->Enable(true); - return channel1_->SetRemoteContent(&remote_media_content2_, CA_ANSWER); + return channel1_->SetRemoteContent(&remote_media_content2_, + CA_ANSWER, NULL); } bool SendOffer() { - bool result = channel1_->SetLocalContent(&local_media_content1_, CA_OFFER); + bool result = channel1_->SetLocalContent(&local_media_content1_, + CA_OFFER, NULL); if (result) { channel1_->Enable(true); - result = channel2_->SetRemoteContent(&remote_media_content1_, CA_OFFER); + result = channel2_->SetRemoteContent(&remote_media_content1_, + CA_OFFER, NULL); } return result; } bool SendProvisionalAnswer() { bool result = channel2_->SetLocalContent(&local_media_content2_, - CA_PRANSWER); + CA_PRANSWER, NULL); if (result) { channel2_->Enable(true); result = channel1_->SetRemoteContent(&remote_media_content2_, - CA_PRANSWER); + CA_PRANSWER, NULL); session1_.Connect(&session2_); } return result; } bool SendFinalAnswer() { - bool result = channel2_->SetLocalContent(&local_media_content2_, CA_ANSWER); + bool result = channel2_->SetLocalContent(&local_media_content2_, + CA_ANSWER, NULL); if (result) - result = channel1_->SetRemoteContent(&remote_media_content2_, CA_ANSWER); + result = channel1_->SetRemoteContent(&remote_media_content2_, + CA_ANSWER, NULL); return result; } @@ -591,9 +599,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)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -604,10 +612,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)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); CreateContent(0, kPcmuCodec, kH264Codec, &content); EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -623,13 +631,13 @@ 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)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); EXPECT_TRUE(channel1_->rtcp_transport_channel() == NULL); // Only initiator supports mux. Should still have a separate RTCP channel. - EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER)); + EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER, NULL)); content.set_rtcp_mux(false); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER, NULL)); EXPECT_TRUE(channel2_->rtcp_transport_channel() != NULL); } @@ -642,17 +650,17 @@ 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)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_PRANSWER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_PRANSWER, NULL)); EXPECT_TRUE(channel1_->rtcp_transport_channel() != NULL); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); // Both sides agree on mux. Should no longer be a separate RTCP channel. EXPECT_TRUE(channel1_->rtcp_transport_channel() == NULL); // Only initiator supports mux. Should still have a separate RTCP channel. - EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER)); + EXPECT_TRUE(channel2_->SetLocalContent(&content, CA_OFFER, NULL)); content.set_rtcp_mux(false); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_PRANSWER)); - EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_PRANSWER, NULL)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content, CA_ANSWER, NULL)); EXPECT_TRUE(channel2_->rtcp_transport_channel() != NULL); } @@ -663,7 +671,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content; CreateContent(0, kPcmuCodec, kH264Codec, &content); content.set_buffered_mode_latency(101); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); EXPECT_EQ(0U, media_channel1_->codecs().size()); cricket::VideoOptions options; ASSERT_TRUE(media_channel1_->GetOptions(&options)); @@ -671,7 +679,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(options.buffered_mode_latency.Get(&latency)); EXPECT_EQ(101, latency); content.set_buffered_mode_latency(102); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -688,8 +696,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { kPcmuCodec, kH264Codec, &content); EXPECT_EQ(0U, media_channel1_->codecs().size()); - EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(content.codecs()[0], media_channel1_->codecs()[0])); @@ -698,14 +706,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { update_content.set_partial(true); CreateContent(0, kIsacCodec, kH264SvcCodec, &update_content); - EXPECT_TRUE(channel1_->SetRemoteContent(&update_content, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetRemoteContent(&update_content, CA_UPDATE, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(update_content.codecs()[0], media_channel1_->codecs()[0])); // Now update without any codecs. This is ignored. typename T::Content empty_content; empty_content.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&empty_content, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetRemoteContent(&empty_content, CA_UPDATE, NULL)); ASSERT_EQ(1U, media_channel1_->codecs().size()); EXPECT_TRUE(CodecMatches(update_content.codecs()[0], media_channel1_->codecs()[0])); @@ -751,7 +759,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content1); content1.AddStream(stream1); EXPECT_EQ(0u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); ASSERT_EQ(1u, media_channel1_->send_streams().size()); EXPECT_EQ(stream1, media_channel1_->send_streams()[0]); @@ -762,7 +770,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { content2.AddStream(stream2); content2.AddStream(stream3); content2.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content2, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetLocalContent(&content2, CA_UPDATE, NULL)); ASSERT_EQ(3u, media_channel1_->send_streams().size()); EXPECT_EQ(stream1, media_channel1_->send_streams()[0]); EXPECT_EQ(stream2, media_channel1_->send_streams()[1]); @@ -774,7 +782,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { stream1.ssrcs.clear(); content3.AddStream(stream1); content3.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content3, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetLocalContent(&content3, CA_UPDATE, NULL)); ASSERT_EQ(2u, media_channel1_->send_streams().size()); EXPECT_EQ(stream2, media_channel1_->send_streams()[0]); EXPECT_EQ(stream3, media_channel1_->send_streams()[1]); @@ -784,7 +792,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content4; content4.AddStream(stream2); content4.set_partial(true); - EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_UPDATE, NULL)); ASSERT_EQ(2u, media_channel1_->send_streams().size()); EXPECT_EQ(stream2, media_channel1_->send_streams()[0]); EXPECT_EQ(stream3, media_channel1_->send_streams()[1]); @@ -818,7 +826,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content1); content1.AddStream(stream1); EXPECT_EQ(0u, media_channel1_->recv_streams().size()); - EXPECT_TRUE(channel1_->SetRemoteContent(&content1, CA_OFFER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content1, CA_OFFER, NULL)); ASSERT_EQ(1u, media_channel1_->codecs().size()); ASSERT_EQ(1u, media_channel1_->recv_streams().size()); @@ -830,7 +838,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { content2.AddStream(stream2); content2.AddStream(stream3); content2.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_UPDATE, NULL)); ASSERT_EQ(3u, media_channel1_->recv_streams().size()); EXPECT_EQ(stream1, media_channel1_->recv_streams()[0]); EXPECT_EQ(stream2, media_channel1_->recv_streams()[1]); @@ -842,7 +850,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { stream1.ssrcs.clear(); content3.AddStream(stream1); content3.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_UPDATE, NULL)); ASSERT_EQ(2u, media_channel1_->recv_streams().size()); EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); EXPECT_EQ(stream3, media_channel1_->recv_streams()[1]); @@ -852,7 +860,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::Content content4; content4.AddStream(stream2); content4.set_partial(true); - EXPECT_TRUE(channel1_->SetRemoteContent(&content4, CA_UPDATE)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content4, CA_UPDATE, NULL)); ASSERT_EQ(2u, media_channel1_->recv_streams().size()); EXPECT_EQ(stream2, media_channel1_->recv_streams()[0]); EXPECT_EQ(stream3, media_channel1_->recv_streams()[1]); @@ -879,20 +887,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)); + EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER, NULL)); EXPECT_TRUE(channel1_->Enable(true)); EXPECT_EQ(1u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER, NULL)); EXPECT_EQ(1u, media_channel2_->recv_streams().size()); session1_.Connect(&session2_); // Channel 2 do not send anything. typename T::Content content2; CreateContent(0, kPcmuCodec, kH264Codec, &content2); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER, NULL)); EXPECT_EQ(0u, media_channel1_->recv_streams().size()); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER, NULL)); EXPECT_TRUE(channel2_->Enable(true)); EXPECT_EQ(0u, media_channel2_->send_streams().size()); @@ -903,21 +911,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)); + EXPECT_TRUE(channel2_->SetLocalContent(&content3, CA_OFFER, NULL)); ASSERT_EQ(1u, media_channel2_->send_streams().size()); EXPECT_EQ(stream2, media_channel2_->send_streams()[0]); - EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_OFFER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content3, CA_OFFER, 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)); + EXPECT_TRUE(channel1_->SetLocalContent(&content4, CA_ANSWER, NULL)); EXPECT_EQ(0u, media_channel1_->send_streams().size()); - EXPECT_TRUE(channel2_->SetRemoteContent(&content4, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetRemoteContent(&content4, CA_ANSWER, NULL)); EXPECT_EQ(0u, media_channel2_->recv_streams().size()); EXPECT_TRUE(channel1_->secure()); @@ -936,13 +944,16 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel1_->Enable(true)); EXPECT_FALSE(media_channel1_->playout()); EXPECT_FALSE(media_channel1_->sending()); - EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, CA_OFFER)); + EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, + CA_OFFER, NULL)); EXPECT_TRUE(media_channel1_->playout()); EXPECT_FALSE(media_channel1_->sending()); - EXPECT_TRUE(channel2_->SetRemoteContent(&local_media_content1_, CA_OFFER)); + EXPECT_TRUE(channel2_->SetRemoteContent(&local_media_content1_, + CA_OFFER, NULL)); EXPECT_FALSE(media_channel2_->playout()); EXPECT_FALSE(media_channel2_->sending()); - EXPECT_TRUE(channel2_->SetLocalContent(&local_media_content2_, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetLocalContent(&local_media_content2_, + CA_ANSWER, NULL)); EXPECT_FALSE(media_channel2_->playout()); EXPECT_FALSE(media_channel2_->sending()); session1_.Connect(&session2_); @@ -953,7 +964,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel2_->Enable(true)); EXPECT_TRUE(media_channel2_->playout()); EXPECT_TRUE(media_channel2_->sending()); - EXPECT_TRUE(channel1_->SetRemoteContent(&local_media_content2_, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetRemoteContent(&local_media_content2_, + CA_ANSWER, NULL)); EXPECT_TRUE(media_channel1_->playout()); EXPECT_TRUE(media_channel1_->sending()); } @@ -998,10 +1010,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_FALSE(media_channel2_->playout()); EXPECT_FALSE(media_channel2_->sending()); - EXPECT_TRUE(channel1_->SetLocalContent(&content1, CA_OFFER)); - EXPECT_TRUE(channel2_->SetRemoteContent(&content1, CA_OFFER)); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_PRANSWER)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_PRANSWER)); + 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)); session1_.Connect(&session2_); EXPECT_TRUE(media_channel1_->playout()); @@ -1011,8 +1023,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Update |content2| to be RecvOnly. content2.set_direction(cricket::MD_RECVONLY); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_PRANSWER)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_PRANSWER)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_PRANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_PRANSWER, NULL)); EXPECT_TRUE(media_channel1_->playout()); EXPECT_TRUE(media_channel1_->sending()); @@ -1021,8 +1033,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Update |content2| to be SendRecv. content2.set_direction(cricket::MD_SENDRECV); - EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER)); + EXPECT_TRUE(channel2_->SetLocalContent(&content2, CA_ANSWER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content2, CA_ANSWER, NULL)); EXPECT_TRUE(media_channel1_->playout()); EXPECT_TRUE(media_channel1_->sending()); @@ -1587,21 +1599,21 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { // Test failures in SetLocalContent. media_channel1_->set_fail_set_recv_codecs(true); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_SENTINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error()); media_channel1_->set_fail_set_recv_codecs(true); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_SENTACCEPT); EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error()); // Test failures in SetRemoteContent. media_channel1_->set_fail_set_send_codecs(true); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_RECEIVEDINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error()); media_channel1_->set_fail_set_send_codecs(true); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_RECEIVEDACCEPT); EXPECT_EQ(cricket::BaseSession::ERROR_CONTENT, session1_.error()); } @@ -1613,7 +1625,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); EXPECT_TRUE(session1_.set_local_description(sdesc)); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_SENTINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error()); EXPECT_TRUE(media_channel1_->HasSendStream(1)); @@ -1635,7 +1647,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); EXPECT_TRUE(session1_.set_remote_description(sdesc)); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_RECEIVEDINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error()); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); @@ -1655,7 +1667,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); EXPECT_TRUE(session1_.set_remote_description(sdesc)); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_RECEIVEDINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error()); EXPECT_TRUE(media_channel1_->HasRecvStream(1)); @@ -1687,7 +1699,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { cricket::SessionDescription* sdesc = CreateSessionDescriptionWithStream(1); EXPECT_TRUE(session1_.set_local_description(sdesc)); - session1_.SetError(cricket::BaseSession::ERROR_NONE); + session1_.SetError(cricket::BaseSession::ERROR_NONE, ""); session1_.SetState(cricket::Session::STATE_SENTINITIATE); EXPECT_EQ(cricket::BaseSession::ERROR_NONE, session1_.error()); EXPECT_TRUE(media_channel1_->HasSendStream(1)); @@ -1810,8 +1822,8 @@ 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)); - EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER)); + EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL)); + EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL)); EXPECT_TRUE(channel1_->rtcp_transport_channel() == NULL); TransportChannel* rtp = channel1_->transport_channel(); EXPECT_FALSE(media_channel1_->ready_to_send()); diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index ba510b9415..b28dce50e6 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -1024,6 +1024,25 @@ static bool IsDtlsActive( return current_tdesc->secure(); } +std::string MediaTypeToString(MediaType type) { + std::string type_str; + switch (type) { + case MEDIA_TYPE_AUDIO: + type_str = "audio"; + break; + case MEDIA_TYPE_VIDEO: + type_str = "video"; + break; + case MEDIA_TYPE_DATA: + type_str = "data"; + break; + default: + ASSERT(false); + break; + } + return type_str; +} + void MediaSessionOptions::AddStream(MediaType type, const std::string& id, const std::string& sync_label) { diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h index ff25f5a040..e2e556db0e 100644 --- a/talk/session/media/mediasession.h +++ b/talk/session/media/mediasession.h @@ -63,6 +63,8 @@ enum MediaType { MEDIA_TYPE_DATA }; +std::string MediaTypeToString(MediaType type); + enum MediaContentDirection { MD_INACTIVE, MD_SENDONLY, |