From a27be8e4a1f59a51ecafba71ba30ddd0bcc9f1f1 Mon Sep 17 00:00:00 2001 From: "mallinath@webrtc.org" Date: Fri, 27 Sep 2013 23:04:10 +0000 Subject: [PATCH] Update libjingle to CL 53398036. Review URL: https://webrtc-codereview.appspot.com/2323004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4872 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/webrtcsession.cc | 46 +++--- talk/app/webrtc/webrtcsession.h | 2 + talk/app/webrtc/webrtcsession_unittest.cc | 27 ++++ talk/base/linuxwindowpicker_unittest.cc | 3 + talk/base/network.cc | 57 +++++++- talk/base/network.h | 16 ++- talk/base/network_unittest.cc | 25 +++- talk/base/openssladapter.cc | 3 - talk/base/testutils.h | 39 +++++ talk/base/thread.cc | 8 -- talk/base/thread.h | 1 - talk/base/virtualsocket_unittest.cc | 6 +- talk/base/windowpicker_unittest.cc | 3 + talk/media/base/fakemediaengine.h | 32 +++-- talk/media/base/filemediaengine.h | 5 +- talk/media/base/filemediaengine_unittest.cc | 6 +- talk/media/base/hybridvideoengine.h | 4 +- talk/media/base/mediaengine.h | 46 ++---- talk/media/base/rtpdataengine.cc | 16 +-- talk/media/base/rtpdataengine_unittest.cc | 9 +- talk/media/base/testutils.h | 39 ----- talk/media/webrtc/webrtcvideoengine.cc | 16 ++- talk/media/webrtc/webrtcvideoengine.h | 2 +- .../webrtc/webrtcvideoengine_unittest.cc | 6 +- talk/media/webrtc/webrtcvoiceengine.cc | 74 +++++----- talk/media/webrtc/webrtcvoiceengine.h | 8 +- .../webrtc/webrtcvoiceengine_unittest.cc | 135 ++---------------- talk/p2p/base/fakesession.h | 6 + talk/p2p/base/p2ptransportchannel.cc | 24 ++-- talk/p2p/base/session.cc | 52 +++++-- talk/p2p/base/session.h | 23 ++- talk/p2p/base/transportchannelproxy.cc | 12 +- talk/session/media/call.cc | 6 +- talk/session/media/call.h | 6 +- talk/session/media/channel.cc | 6 - talk/session/media/channelmanager.cc | 29 ++-- talk/session/media/channelmanager.h | 15 +- talk/session/media/channelmanager_unittest.cc | 102 +++++-------- talk/session/media/mediasession.cc | 7 +- talk/session/media/mediasessionclient.h | 4 +- talk/xmpp/mucroomdiscoverytask.cc | 6 +- 41 files changed, 496 insertions(+), 436 deletions(-) diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index f1fb40d51..83f891282 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -54,7 +54,7 @@ using cricket::TransportInfo; namespace webrtc { -const char kInternalConstraintPrefix[] = "internal"; +const char MediaConstraintsInterface::kInternalConstraintPrefix[] = "internal"; // Supported MediaConstraints. // DTLS-SRTP pseudo-constraints. @@ -81,6 +81,8 @@ const char kMlineMismatch[] = "Offer and answer descriptions m-lines are not matching. " "Rejecting answer."; const char kSdpWithoutCrypto[] = "Called with a SDP without crypto enabled."; +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[] = @@ -110,10 +112,9 @@ static bool VerifyMediaDescriptions( // fingerprint. Mismatches, such as replying with a DTLS fingerprint to SDES // keys, will be caught in Transport negotiation, and backstopped by Channel's // |secure_required| check. -static bool VerifyCrypto(const SessionDescription* desc) { - if (!desc) { - return false; - } +static bool VerifyCrypto(const SessionDescription* desc, + bool dtls_enabled, + std::string* error) { const ContentInfos& contents = desc->contents(); for (size_t index = 0; index < contents.size(); ++index) { const ContentInfo* cinfo = &contents[index]; @@ -128,13 +129,22 @@ static bool VerifyCrypto(const SessionDescription* desc) { if (!media || !tinfo) { // Something is not right. LOG(LS_ERROR) << kInvalidSdp; + *error = kInvalidSdp; return false; } - if (media->cryptos().empty() && - !tinfo->description.identity_fingerprint) { - // Crypto must be supplied. - LOG(LS_WARNING) << "Session description must have SDES or DTLS-SRTP."; - return false; + if (media->cryptos().empty()) { + if (!tinfo->description.identity_fingerprint) { + // Crypto must be supplied. + LOG(LS_WARNING) << "Session description must have SDES or DTLS-SRTP."; + *error = kSdpWithoutCrypto; + return false; + } + if (!dtls_enabled) { + LOG(LS_WARNING) << + "Session description must have SDES when DTLS disabled."; + *error = kSdpWithoutSdesAndDtlsDisabled; + return false; + } } } @@ -392,6 +402,7 @@ WebRtcSession::WebRtcSession( ice_observer_(NULL), ice_connection_state_(PeerConnectionInterface::kIceConnectionNew), older_version_remote_peer_(false), + dtls_enabled_(false), data_channel_type_(cricket::DCT_NONE), ice_restart_latch_(new IceRestartAnswerLatch) { } @@ -424,13 +435,13 @@ bool WebRtcSession::Initialize( bool value; // Enable DTLS by default if |dtls_identity_service| is valid. - bool dtls_enabled = (dtls_identity_service != NULL); - // |constraints| can override the default |dtls_enabled| value. + dtls_enabled_ = (dtls_identity_service != NULL); + // |constraints| can override the default |dtls_enabled_| value. if (FindConstraint( constraints, MediaConstraintsInterface::kEnableDtlsSrtp, &value, NULL)) { - dtls_enabled = value; + dtls_enabled_ = value; } // Enable creation of RTP data channels if the kEnableRtpDataChannels is set. @@ -446,7 +457,7 @@ bool WebRtcSession::Initialize( MediaConstraintsInterface::kEnableSctpDataChannels, &value, NULL) && value; // DTLS has to be enabled to use SCTP. - if (sctp_enabled && dtls_enabled) { + if (sctp_enabled && dtls_enabled_) { LOG(LS_INFO) << "Allowing SCTP data engine."; data_channel_type_ = cricket::DCT_SCTP; } @@ -473,7 +484,7 @@ bool WebRtcSession::Initialize( this, id(), data_channel_type_, - dtls_enabled)); + dtls_enabled_)); webrtc_session_desc_factory_->SignalIdentityReady.connect( this, &WebRtcSession::OnIdentityReady); @@ -1383,9 +1394,10 @@ bool WebRtcSession::ValidateSessionDescription( } // Verify crypto settings. + std::string crypto_error; if (webrtc_session_desc_factory_->secure() == cricket::SEC_REQUIRED && - !VerifyCrypto(sdesc->description())) { - return BadSdp(source, kSdpWithoutCrypto, error_desc); + !VerifyCrypto(sdesc->description(), dtls_enabled_, &crypto_error)) { + return BadSdp(source, crypto_error, error_desc); } if (!ValidateBundleSettings(sdesc->description())) { diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 0cb049f49..eee579d19 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -68,6 +68,7 @@ extern const char kInvalidCandidates[]; extern const char kInvalidSdp[]; extern const char kMlineMismatch[]; extern const char kSdpWithoutCrypto[]; +extern const char kSdpWithoutSdesAndDtlsDisabled[]; extern const char kSessionError[]; extern const char kUpdateStateFailed[]; extern const char kPushDownOfferTDFailed[]; @@ -299,6 +300,7 @@ class WebRtcSession : public cricket::BaseSession, std::vector saved_candidates_; // If the remote peer is using a older version of implementation. bool older_version_remote_peer_; + bool dtls_enabled_; // Specifies which kind of data channel is allowed. This is controlled // by the chrome command-line flag and constraints: // 1. If chrome command-line switch 'enable-sctp-data-channels' is enabled, diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 1531e0efc..acbc92439 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -86,6 +86,7 @@ using webrtc::StreamCollection; using webrtc::WebRtcSession; using webrtc::kMlineMismatch; using webrtc::kSdpWithoutCrypto; +using webrtc::kSdpWithoutSdesAndDtlsDisabled; using webrtc::kSessionError; using webrtc::kSetLocalSdpFailed; using webrtc::kSetRemoteSdpFailed; @@ -114,6 +115,10 @@ static const cricket::AudioCodec static const cricket::AudioCodec kCNCodec1(102, "CN", 8000, 0, 1, 0); static const cricket::AudioCodec kCNCodec2(103, "CN", 16000, 0, 1, 0); +static const char kFakeDtlsFingerprint[] = + "BB:CD:72:F7:2F:D0:BA:43:F3:68:B1:0C:23:72:B6:4A:" + "0F:DE:34:06:BC:E0:FE:01:BC:73:C8:6D:F4:65:D5:24"; + // Add some extra |newlines| to the |message| after |line|. static void InjectAfter(const std::string& line, const std::string& newlines, @@ -2629,6 +2634,28 @@ TEST_F(WebRtcSessionTest, VerifyMultipleAsyncCreateDescription( false, CreateSessionDescriptionRequest::kAnswer); } + +// Verifies that setRemoteDescription fails when DTLS is disabled and the remote +// offer has no SDES crypto but only DTLS fingerprint. +TEST_F(WebRtcSessionTest, TestSetRemoteOfferFailIfDtlsDisabledAndNoCrypto) { + // Init without DTLS. + Init(NULL); + // Create a remote offer with secured transport disabled. + cricket::MediaSessionOptions options; + JsepSessionDescription* offer(CreateRemoteOffer( + options, cricket::SEC_DISABLED)); + // Adds a DTLS fingerprint to the remote offer. + cricket::SessionDescription* sdp = offer->description(); + TransportInfo* audio = sdp->GetTransportInfoByName("audio"); + ASSERT_TRUE(audio != NULL); + ASSERT_TRUE(audio->description.identity_fingerprint.get() == NULL); + audio->description.identity_fingerprint.reset( + talk_base::SSLFingerprint::CreateFromRfc4572( + talk_base::DIGEST_SHA_256, kFakeDtlsFingerprint)); + SetRemoteDescriptionExpectError(kSdpWithoutSdesAndDtlsDisabled, + offer); +} + // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled. That test // currently fails because upon disconnection and reconnection OnIceComplete is // called more than once without returning to IceGatheringGathering. diff --git a/talk/base/linuxwindowpicker_unittest.cc b/talk/base/linuxwindowpicker_unittest.cc index 5ea9c93fd..c248bbac2 100644 --- a/talk/base/linuxwindowpicker_unittest.cc +++ b/talk/base/linuxwindowpicker_unittest.cc @@ -28,6 +28,7 @@ #include "talk/base/gunit.h" #include "talk/base/linuxwindowpicker.h" #include "talk/base/logging.h" +#include "talk/base/testutils.h" #include "talk/base/windowpicker.h" #ifndef LINUX @@ -37,6 +38,7 @@ namespace talk_base { TEST(LinuxWindowPickerTest, TestGetWindowList) { + MAYBE_SKIP_SCREENCAST_TEST(); LinuxWindowPicker window_picker; WindowDescriptionList descriptions; window_picker.Init(); @@ -44,6 +46,7 @@ TEST(LinuxWindowPickerTest, TestGetWindowList) { } TEST(LinuxWindowPickerTest, TestGetDesktopList) { + MAYBE_SKIP_SCREENCAST_TEST(); LinuxWindowPicker window_picker; DesktopDescriptionList descriptions; EXPECT_TRUE(window_picker.Init()); diff --git a/talk/base/network.cc b/talk/base/network.cc index d6367c32b..cbfcb5fcb 100644 --- a/talk/base/network.cc +++ b/talk/base/network.cc @@ -32,10 +32,18 @@ #include "talk/base/network.h" #ifdef POSIX +// linux/if.h can't be included at the same time as the posix sys/if.h, and +// it's transitively required by linux/route.h, so include that version on +// linux instead of the standard posix one. +#if defined(ANDROID) || defined(LINUX) +#include +#include +#else +#include +#endif #include #include #include -#include #include #include #ifdef ANDROID @@ -173,7 +181,8 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks, } BasicNetworkManager::BasicNetworkManager() - : thread_(NULL), sent_first_update_(false), start_count_(0) { + : thread_(NULL), sent_first_update_(false), start_count_(0), + ignore_non_default_routes_(false) { } BasicNetworkManager::~BasicNetworkManager() { @@ -397,14 +406,52 @@ bool BasicNetworkManager::CreateNetworks(bool include_ignored, } #endif // WIN32 -bool BasicNetworkManager::IsIgnoredNetwork(const Network& network) { +#if defined(ANDROID) || defined(LINUX) +bool IsDefaultRoute(const std::string& network_name) { + FileStream fs; + if (!fs.Open("/proc/net/route", "r", NULL)) { + LOG(LS_WARNING) << "Couldn't read /proc/net/route, skipping default " + << "route check (assuming everything is a default route)."; + return true; + } else { + std::string line; + while (fs.ReadLine(&line) == SR_SUCCESS) { + char iface_name[256]; + unsigned int iface_ip, iface_gw, iface_mask, iface_flags; + if (sscanf(line.c_str(), + "%255s %8X %8X %4X %*d %*u %*d %8X", + iface_name, &iface_ip, &iface_gw, + &iface_flags, &iface_mask) == 5 && + network_name == iface_name && + iface_mask == 0 && + (iface_flags & (RTF_UP | RTF_HOST)) == RTF_UP) { + return true; + } + } + } + return false; +} +#endif + +bool BasicNetworkManager::IsIgnoredNetwork(const Network& network) const { + // Ignore networks on the explicit ignore list. + for (size_t i = 0; i < network_ignore_list_.size(); ++i) { + if (network.name() == network_ignore_list_[i]) { + return true; + } + } #ifdef POSIX - // Ignore local networks (lo, lo0, etc) - // Also filter out VMware interfaces, typically named vmnet1 and vmnet8 + // Filter out VMware interfaces, typically named vmnet1 and vmnet8 if (strncmp(network.name().c_str(), "vmnet", 5) == 0 || strncmp(network.name().c_str(), "vnic", 4) == 0) { return true; } +#if defined(ANDROID) || defined(LINUX) + // Make sure this is a default route, if we're ignoring non-defaults. + if (ignore_non_default_routes_ && !IsDefaultRoute(network.name())) { + return true; + } +#endif #elif defined(WIN32) // Ignore any HOST side vmware adapters with a description like: // VMware Virtual Ethernet Adapter for VMnet1 diff --git a/talk/base/network.h b/talk/base/network.h index f87063da5..63f3e732f 100644 --- a/talk/base/network.h +++ b/talk/base/network.h @@ -127,6 +127,18 @@ class BasicNetworkManager : public NetworkManagerBase, virtual void OnMessage(Message* msg); bool started() { return start_count_ > 0; } + // Sets the network ignore list, which is empty by default. Any network on + // the ignore list will be filtered from network enumeration results. + void set_network_ignore_list(const std::vector& list) { + network_ignore_list_ = list; + } +#if defined(ANDROID) || defined(LINUX) + // Sets the flag for ignoring non-default routes. + void set_ignore_non_default_routes(bool value) { + ignore_non_default_routes_ = true; + } +#endif + protected: #if defined(POSIX) // Separated from CreateNetworks for tests. @@ -139,7 +151,7 @@ class BasicNetworkManager : public NetworkManagerBase, bool CreateNetworks(bool include_ignored, NetworkList* networks) const; // Determines if a network should be ignored. - static bool IsIgnoredNetwork(const Network& network); + bool IsIgnoredNetwork(const Network& network) const; private: friend class NetworkTest; @@ -149,6 +161,8 @@ class BasicNetworkManager : public NetworkManagerBase, Thread* thread_; bool sent_first_update_; int start_count_; + std::vector network_ignore_list_; + bool ignore_non_default_routes_; }; // Represents a Unix-type network interface, with a name and single address. diff --git a/talk/base/network_unittest.cc b/talk/base/network_unittest.cc index ee0f0aa09..d12a1eb8f 100644 --- a/talk/base/network_unittest.cc +++ b/talk/base/network_unittest.cc @@ -54,8 +54,9 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<> { network_manager.MergeNetworkList(list, changed); } - bool IsIgnoredNetwork(const Network& network) { - return BasicNetworkManager::IsIgnoredNetwork(network); + bool IsIgnoredNetwork(BasicNetworkManager& network_manager, + const Network& network) { + return network_manager.IsIgnoredNetwork(network); } NetworkManager::NetworkList GetNetworks( @@ -96,8 +97,24 @@ TEST_F(NetworkTest, TestNetworkIgnore) { IPAddress(0x12345600U), 24); Network ipv4_network2("test_eth1", "Test Network Adapter 2", IPAddress(0x00010000U), 16); - EXPECT_FALSE(IsIgnoredNetwork(ipv4_network1)); - EXPECT_TRUE(IsIgnoredNetwork(ipv4_network2)); + BasicNetworkManager network_manager; + EXPECT_FALSE(IsIgnoredNetwork(network_manager, ipv4_network1)); + EXPECT_TRUE(IsIgnoredNetwork(network_manager, ipv4_network2)); +} + +TEST_F(NetworkTest, TestIgnoreList) { + Network ignore_me("ignore_me", "Ignore me please!", + IPAddress(0x12345600U), 24); + Network include_me("include_me", "Include me please!", + IPAddress(0x12345600U), 24); + BasicNetworkManager network_manager; + EXPECT_FALSE(IsIgnoredNetwork(network_manager, ignore_me)); + EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me)); + std::vector ignore_list; + ignore_list.push_back("ignore_me"); + network_manager.set_network_ignore_list(ignore_list); + EXPECT_TRUE(IsIgnoredNetwork(network_manager, ignore_me)); + EXPECT_FALSE(IsIgnoredNetwork(network_manager, include_me)); } TEST_F(NetworkTest, TestCreateNetworks) { diff --git a/talk/base/openssladapter.cc b/talk/base/openssladapter.cc index af92f0c45..50391e5c2 100644 --- a/talk/base/openssladapter.cc +++ b/talk/base/openssladapter.cc @@ -233,10 +233,7 @@ VerificationCallback OpenSSLAdapter::custom_verify_callback_ = NULL; bool OpenSSLAdapter::InitializeSSL(VerificationCallback callback) { if (!InitializeSSLThread() || !SSL_library_init()) return false; -#if !defined(ADDRESS_SANITIZER) || !defined(OSX) - // Loading the error strings crashes mac_asan. Omit this debugging aid there. SSL_load_error_strings(); -#endif ERR_load_BIO_strings(); OpenSSL_add_all_algorithms(); RAND_poll(); diff --git a/talk/base/testutils.h b/talk/base/testutils.h index 769d95f64..e8ad72009 100644 --- a/talk/base/testutils.h +++ b/talk/base/testutils.h @@ -30,6 +30,13 @@ // Utilities for testing talk_base infrastructure in unittests +#ifdef LINUX +#include +// X defines a few macros that stomp on types that gunit.h uses. +#undef None +#undef Bool +#endif + #include #include #include "talk/base/asyncsocket.h" @@ -565,6 +572,38 @@ inline AssertionResult CmpHelperFileEq(const char* expected_expression, /////////////////////////////////////////////////////////////////////////////// +// Helpers for determining if X/screencasting is available (on linux). + +#define MAYBE_SKIP_SCREENCAST_TEST() \ + if (!testing::IsScreencastingAvailable()) { \ + LOG(LS_WARNING) << "Skipping test, since it doesn't have the requisite " \ + << "X environment for screen capture."; \ + return; \ + } \ + +#ifdef LINUX +struct XDisplay { + XDisplay() : display_(XOpenDisplay(NULL)) { } + ~XDisplay() { if (display_) XCloseDisplay(display_); } + bool IsValid() const { return display_ != NULL; } + operator Display*() { return display_; } + private: + Display* display_; +}; +#endif + +// Returns true if screencasting is available. When false, anything that uses +// screencasting features may fail. +inline bool IsScreencastingAvailable() { +#ifdef LINUX + XDisplay display; + if (!display.IsValid()) { + LOG(LS_WARNING) << "No X Display available."; + return false; + } +#endif + return true; +} } // namespace testing #endif // TALK_BASE_TESTUTILS_H__ diff --git a/talk/base/thread.cc b/talk/base/thread.cc index d21d5f195..0c43e635e 100644 --- a/talk/base/thread.cc +++ b/talk/base/thread.cc @@ -146,7 +146,6 @@ Thread::Thread(SocketServer* ss) : MessageQueue(ss), priority_(PRIORITY_NORMAL), started_(false), - has_sends_(false), #if defined(WIN32) thread_(NULL), thread_id_(0), @@ -405,7 +404,6 @@ void Thread::Send(MessageHandler *phandler, uint32 id, MessageData *pdata) { smsg.msg = msg; smsg.ready = &ready; sendlist_.push_back(smsg); - has_sends_ = true; } // Wait for a reply @@ -436,11 +434,6 @@ void Thread::Send(MessageHandler *phandler, uint32 id, MessageData *pdata) { } void Thread::ReceiveSends() { - // Before entering critical section, check boolean. - - if (!has_sends_) - return; - // Receive a sent message. Cleanup scenarios: // - thread sending exits: We don't allow this, since thread can exit // only via Join, so Send must complete. @@ -456,7 +449,6 @@ void Thread::ReceiveSends() { *smsg.ready = true; smsg.thread->socketserver()->WakeUp(); } - has_sends_ = false; crit_.Leave(); } diff --git a/talk/base/thread.h b/talk/base/thread.h index 55ec0daf7..e679ea457 100644 --- a/talk/base/thread.h +++ b/talk/base/thread.h @@ -255,7 +255,6 @@ class Thread : public MessageQueue { std::string name_; ThreadPriority priority_; bool started_; - bool has_sends_; #ifdef POSIX pthread_t thread_; diff --git a/talk/base/virtualsocket_unittest.cc b/talk/base/virtualsocket_unittest.cc index 7f233dfac..7bbb5f0bc 100644 --- a/talk/base/virtualsocket_unittest.cc +++ b/talk/base/virtualsocket_unittest.cc @@ -46,7 +46,7 @@ struct Sender : public MessageHandler { Sender(Thread* th, AsyncSocket* s, uint32 rt) : thread(th), socket(new AsyncUDPSocket(s)), done(false), rate(rt), count(0) { - last_send = Time(); + last_send = talk_base::Time(); thread->PostDelayed(NextDelay(), this, 1); } @@ -61,7 +61,7 @@ struct Sender : public MessageHandler { if (done) return; - uint32 cur_time = Time(); + uint32 cur_time = talk_base::Time(); uint32 delay = cur_time - last_send; uint32 size = rate * delay / 1000; size = std::min(size, 4096); @@ -105,7 +105,7 @@ struct Receiver : public MessageHandler, public sigslot::has_slots<> { sec_count += size; uint32 send_time = *reinterpret_cast(data); - uint32 recv_time = Time(); + uint32 recv_time = talk_base::Time(); uint32 delay = recv_time - send_time; sum += delay; sum_sq += delay * delay; diff --git a/talk/base/windowpicker_unittest.cc b/talk/base/windowpicker_unittest.cc index e1a815d40..436854a3c 100644 --- a/talk/base/windowpicker_unittest.cc +++ b/talk/base/windowpicker_unittest.cc @@ -1,4 +1,5 @@ #include "talk/base/gunit.h" +#include "talk/base/testutils.h" #include "talk/base/window.h" #include "talk/base/windowpicker.h" #include "talk/base/windowpickerfactory.h" @@ -10,6 +11,7 @@ #endif TEST(WindowPickerTest, GetWindowList) { + MAYBE_SKIP_SCREENCAST_TEST(); if (!talk_base::WindowPickerFactory::IsSupported()) { LOG(LS_INFO) << "skipping test: window capturing is not supported with " << "current configuration."; @@ -24,6 +26,7 @@ TEST(WindowPickerTest, GetWindowList) { // TODO(hughv) Investigate why this fails on pulse but not locally after // upgrading to XCode 4.5. The failure is GetDesktopList returning FALSE. TEST(WindowPickerTest, DISABLE_ON_MAC(GetDesktopList)) { + MAYBE_SKIP_SCREENCAST_TEST(); if (!talk_base::WindowPickerFactory::IsSupported()) { LOG(LS_INFO) << "skipping test: window capturing is not supported with " << "current configuration."; diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 7ef0c9b1a..257c1981d 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -678,18 +678,11 @@ class FakeBaseEngine { public: FakeBaseEngine() : loglevel_(-1), - options_(0), options_changed_(false), fail_create_channel_(false) {} bool Init(talk_base::Thread* worker_thread) { return true; } void Terminate() {} - bool SetOptions(int options) { - options_ = options; - options_changed_ = true; - return true; - } - void SetLogging(int level, const char* filter) { loglevel_ = level; logfilter_ = filter; @@ -704,7 +697,6 @@ class FakeBaseEngine { protected: int loglevel_; std::string logfilter_; - int options_; // Flag used by optionsmessagehandler_unittest for checking whether any // relevant setting has been updated. // TODO(thaloun): Replace with explicit checks of before & after values. @@ -725,6 +717,17 @@ class FakeVoiceEngine : public FakeBaseEngine { codecs_.push_back(AudioCodec(101, "fake_audio_codec", 0, 0, 1, 0)); } int GetCapabilities() { return AUDIO_SEND | AUDIO_RECV; } + AudioOptions GetAudioOptions() const { + return options_; + } + AudioOptions GetOptions() const { + return options_; + } + bool SetOptions(const AudioOptions& options) { + options_ = options; + options_changed_ = true; + return true; + } VoiceMediaChannel* CreateChannel() { if (fail_create_channel_) { @@ -808,6 +811,7 @@ class FakeVoiceEngine : public FakeBaseEngine { std::string out_device_; VoiceProcessor* rx_processor_; VoiceProcessor* tx_processor_; + AudioOptions options_; friend class FakeMediaEngine; }; @@ -819,6 +823,15 @@ class FakeVideoEngine : public FakeBaseEngine { // sanity checks against that. codecs_.push_back(VideoCodec(0, "fake_video_codec", 0, 0, 0, 0)); } + bool GetOptions(VideoOptions* options) const { + *options = options_; + return true; + } + bool SetOptions(const VideoOptions& options) { + options_ = options; + options_changed_ = true; + return true; + } int GetCapabilities() { return VIDEO_SEND | VIDEO_RECV; } bool SetDefaultEncoderConfig(const VideoEncoderConfig& config) { default_encoder_config_ = config; @@ -883,6 +896,7 @@ class FakeVideoEngine : public FakeBaseEngine { VideoRenderer* renderer_; bool capture_; VideoProcessor* processor_; + VideoOptions options_; friend class FakeMediaEngine; }; @@ -912,7 +926,7 @@ class FakeMediaEngine : return video_.GetChannel(index); } - int audio_options() const { return voice_.options_; } + AudioOptions audio_options() const { return voice_.options_; } int audio_delay_offset() const { return voice_.delay_offset_; } int output_volume() const { return voice_.output_volume_; } const VideoEncoderConfig& default_video_encoder_config() const { diff --git a/talk/media/base/filemediaengine.h b/talk/media/base/filemediaengine.h index 2c8525d96..b7a7ac17d 100644 --- a/talk/media/base/filemediaengine.h +++ b/talk/media/base/filemediaengine.h @@ -86,8 +86,9 @@ class FileMediaEngine : public MediaEngineInterface { virtual VoiceMediaChannel* CreateChannel(); virtual VideoMediaChannel* CreateVideoChannel(VoiceMediaChannel* voice_ch); virtual SoundclipMedia* CreateSoundclip() { return NULL; } - virtual bool SetAudioOptions(int options) { return true; } - virtual bool SetVideoOptions(int options) { return true; } + virtual AudioOptions GetAudioOptions() const { return AudioOptions(); } + virtual bool SetAudioOptions(const AudioOptions& options) { return true; } + virtual bool SetVideoOptions(const VideoOptions& options) { return true; } virtual bool SetAudioDelayOffset(int offset) { return true; } virtual bool SetDefaultVideoEncoderConfig(const VideoEncoderConfig& config) { return true; diff --git a/talk/media/base/filemediaengine_unittest.cc b/talk/media/base/filemediaengine_unittest.cc index e4d72bbe6..a72e0dc6a 100644 --- a/talk/media/base/filemediaengine_unittest.cc +++ b/talk/media/base/filemediaengine_unittest.cc @@ -220,8 +220,10 @@ TEST_F(FileMediaEngineTest, TestDefaultImplementation) { EXPECT_TRUE(NULL == voice_channel_.get()); EXPECT_TRUE(NULL == video_channel_.get()); EXPECT_TRUE(NULL == engine_->CreateSoundclip()); - EXPECT_TRUE(engine_->SetAudioOptions(0)); - EXPECT_TRUE(engine_->SetVideoOptions(0)); + cricket::AudioOptions audio_options; + EXPECT_TRUE(engine_->SetAudioOptions(audio_options)); + cricket::VideoOptions video_options; + EXPECT_TRUE(engine_->SetVideoOptions(video_options)); VideoEncoderConfig video_encoder_config; EXPECT_TRUE(engine_->SetDefaultVideoEncoderConfig(video_encoder_config)); EXPECT_TRUE(engine_->SetSoundDevices(NULL, NULL)); diff --git a/talk/media/base/hybridvideoengine.h b/talk/media/base/hybridvideoengine.h index ab638c9e4..7bef97ebd 100644 --- a/talk/media/base/hybridvideoengine.h +++ b/talk/media/base/hybridvideoengine.h @@ -183,8 +183,8 @@ class HybridVideoEngine : public HybridVideoEngineInterface { channel1.release(), channel2.release()); } - bool SetOptions(int o) { - return video1_.SetOptions(o) && video2_.SetOptions(o); + bool SetOptions(const VideoOptions& options) { + return video1_.SetOptions(options) && video2_.SetOptions(options); } bool SetDefaultEncoderConfig(const VideoEncoderConfig& config) { VideoEncoderConfig conf = config; diff --git a/talk/media/base/mediaengine.h b/talk/media/base/mediaengine.h index 8ebc13b10..c9baeb2ec 100644 --- a/talk/media/base/mediaengine.h +++ b/talk/media/base/mediaengine.h @@ -60,30 +60,6 @@ class VideoCapturer; // proper synchronization between both media types. class MediaEngineInterface { public: - // Bitmask flags for options that may be supported by the media engine - // implementation. This can be converted to and from an - // AudioOptions struct for backwards compatibility with calls that - // use flags until we transition to using structs everywhere. - enum AudioFlags { - // Audio processing that attempts to filter away the output signal from - // later inbound pickup. - ECHO_CANCELLATION = 1 << 0, - // Audio processing to adjust the sensitivity of the local mic dynamically. - AUTO_GAIN_CONTROL = 1 << 1, - // Audio processing to filter out background noise. - NOISE_SUPPRESSION = 1 << 2, - // Audio processing to remove background noise of lower frequencies. - HIGHPASS_FILTER = 1 << 3, - // A switch to swap which captured signal is left and right in stereo mode. - STEREO_FLIPPING = 1 << 4, - // Controls delegation echo cancellation to use the OS' facility. - SYSTEM_AEC_MODE = 1 << 5, - - ALL_AUDIO_OPTIONS = (1 << 6) - 1, - DEFAULT_AUDIO_OPTIONS = ECHO_CANCELLATION | AUTO_GAIN_CONTROL | - NOISE_SUPPRESSION | HIGHPASS_FILTER, - }; - // Default value to be used for SetAudioDelayOffset(). static const int kDefaultAudioDelayOffset; @@ -109,10 +85,12 @@ class MediaEngineInterface { virtual SoundclipMedia *CreateSoundclip() = 0; // Configuration + // Gets global audio options. + virtual AudioOptions GetAudioOptions() const = 0; // Sets global audio options. "options" are from AudioOptions, above. - virtual bool SetAudioOptions(int options) = 0; + virtual bool SetAudioOptions(const AudioOptions& options) = 0; // Sets global video options. "options" are from VideoOptions, above. - virtual bool SetVideoOptions(int options) = 0; + virtual bool SetVideoOptions(const VideoOptions& options) = 0; // Sets the value used by the echo canceller to offset delay values obtained // from the OS. virtual bool SetAudioDelayOffset(int offset) = 0; @@ -210,11 +188,14 @@ class CompositeMediaEngine : public MediaEngineInterface { return voice_.CreateSoundclip(); } - virtual bool SetAudioOptions(int o) { - return voice_.SetOptions(o); + virtual AudioOptions GetAudioOptions() const { + return voice_.GetOptions(); } - virtual bool SetVideoOptions(int o) { - return video_.SetOptions(o); + virtual bool SetAudioOptions(const AudioOptions& options) { + return voice_.SetOptions(options); + } + virtual bool SetVideoOptions(const VideoOptions& options) { + return video_.SetOptions(options); } virtual bool SetAudioDelayOffset(int offset) { return voice_.SetDelayOffset(offset); @@ -304,7 +285,8 @@ class NullVoiceEngine { return NULL; } bool SetDelayOffset(int offset) { return true; } - bool SetOptions(int opts) { return true; } + AudioOptions GetOptions() const { return AudioOptions(); } + bool SetOptions(const AudioOptions& options) { return true; } bool SetDevices(const Device* in_device, const Device* out_device) { return true; } @@ -344,7 +326,7 @@ class NullVideoEngine { VoiceMediaChannel* voice_media_channel) { return NULL; } - bool SetOptions(int opts) { return true; } + bool SetOptions(const VideoOptions& options) { return true; } bool SetDefaultEncoderConfig(const VideoEncoderConfig& config) { return true; } diff --git a/talk/media/base/rtpdataengine.cc b/talk/media/base/rtpdataengine.cc index 2a23c1005..1254b29a8 100644 --- a/talk/media/base/rtpdataengine.cc +++ b/talk/media/base/rtpdataengine.cc @@ -342,10 +342,6 @@ bool RtpDataMediaChannel::SendData( << "; already sent " << send_limiter_->used_in_period() << "/" << send_limiter_->max_per_period(); return false; - } else { - LOG(LS_VERBOSE) << "Sending data packet of len=" << packet_len - << "; already sent " << send_limiter_->used_in_period() - << "/" << send_limiter_->max_per_period(); } RtpHeader header; @@ -363,12 +359,12 @@ bool RtpDataMediaChannel::SendData( packet.AppendData(&kReservedSpace, sizeof(kReservedSpace)); packet.AppendData(payload.data(), payload.length()); - // Uncomment this for easy debugging. - // LOG(LS_INFO) << "Sent packet: " - // << " stream=" << found_stream.id - // << ", seqnum=" << header.seq_num - // << ", timestamp=" << header.timestamp - // << ", len=" << data_len; + LOG(LS_VERBOSE) << "Sent RTP data packet: " + << " stream=" << found_stream.id + << " ssrc=" << header.ssrc + << ", seqnum=" << header.seq_num + << ", timestamp=" << header.timestamp + << ", len=" << payload.length(); MediaChannel::SendPacket(&packet); send_limiter_->Use(packet_len, now); diff --git a/talk/media/base/rtpdataengine_unittest.cc b/talk/media/base/rtpdataengine_unittest.cc index 456f7b0cb..37ea968df 100644 --- a/talk/media/base/rtpdataengine_unittest.cc +++ b/talk/media/base/rtpdataengine_unittest.cc @@ -294,7 +294,8 @@ TEST_F(RtpDataMediaChannelTest, SendData) { cricket::RtpHeader header1 = GetSentDataHeader(1); EXPECT_EQ(header1.ssrc, 42U); EXPECT_EQ(header1.payload_type, 103); - EXPECT_EQ(header0.seq_num + 1, header1.seq_num); + EXPECT_EQ(static_cast(header0.seq_num + 1), + static_cast(header1.seq_num)); EXPECT_EQ(header0.timestamp + 180000, header1.timestamp); } @@ -354,9 +355,11 @@ TEST_F(RtpDataMediaChannelTest, DISABLED_SendDataMultipleClocks) { cricket::RtpHeader header1b = GetSentDataHeader(2); cricket::RtpHeader header2b = GetSentDataHeader(3); - EXPECT_EQ(header1a.seq_num + 1, header1b.seq_num); + EXPECT_EQ(static_cast(header1a.seq_num + 1), + static_cast(header1b.seq_num)); EXPECT_EQ(header1a.timestamp + 90000, header1b.timestamp); - EXPECT_EQ(header2a.seq_num + 1, header2b.seq_num); + EXPECT_EQ(static_cast(header2a.seq_num + 1), + static_cast(header2b.seq_num)); EXPECT_EQ(header2a.timestamp + 180000, header2b.timestamp); } diff --git a/talk/media/base/testutils.h b/talk/media/base/testutils.h index 4d037b7ba..136b7b9d6 100644 --- a/talk/media/base/testutils.h +++ b/talk/media/base/testutils.h @@ -28,13 +28,6 @@ #ifndef TALK_MEDIA_BASE_TESTUTILS_H_ #define TALK_MEDIA_BASE_TESTUTILS_H_ -#ifdef LINUX -#include -// X defines a few macros that stomp on types that gunit.h uses. -#undef None -#undef Bool -#endif - #include #include @@ -244,38 +237,6 @@ bool ContainsMatchingCodec(const std::vector& codecs, const C& codec) { } return false; } - -#define MAYBE_SKIP_SCREENCAST_TEST() \ - if (!cricket::IsScreencastingAvailable()) { \ - LOG(LS_WARNING) << "Skipping test, since it doesn't have the requisite " \ - << "X environment for screen capture."; \ - return; \ - } \ - -#ifdef LINUX -struct XDisplay { - XDisplay() : display_(XOpenDisplay(NULL)) { } - ~XDisplay() { if (display_) XCloseDisplay(display_); } - bool IsValid() const { return display_ != NULL; } - operator Display*() { return display_; } - private: - Display* display_; -}; -#endif - -// Returns true if screencasting is available. When false, anything that uses -// screencasting features may fail. -inline bool IsScreencastingAvailable() { -#ifdef LINUX - XDisplay display; - if (!display.IsValid()) { - LOG(LS_WARNING) << "No X Display available."; - return false; - } -#endif - return true; -} - } // namespace cricket #endif // TALK_MEDIA_BASE_TESTUTILS_H_ diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index fd7e5bfa3..cdc172727 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -514,7 +514,8 @@ class WebRtcVideoChannelSendInfo : public sigslot::has_slots<> { external_capture_(external_capture), capturer_updated_(false), interval_(0), - video_adapter_(new CoordinatedVideoAdapter) { + video_adapter_(new CoordinatedVideoAdapter), + cpu_monitor_(cpu_monitor) { overuse_observer_.reset(new WebRtcOveruseObserver(video_adapter_.get())); SignalCpuAdaptationUnable.repeat(video_adapter_->SignalCpuAdaptationUnable); if (cpu_monitor) { @@ -633,6 +634,9 @@ class WebRtcVideoChannelSendInfo : public sigslot::has_slots<> { } void SetCpuOveruseDetection(bool enable) { + if (cpu_monitor_ && enable) { + cpu_monitor_->SignalUpdate.disconnect(video_adapter_.get()); + } overuse_observer_->Enable(enable); video_adapter_->set_cpu_adaptation(enable); } @@ -689,6 +693,7 @@ class WebRtcVideoChannelSendInfo : public sigslot::has_slots<> { int64 interval_; talk_base::scoped_ptr video_adapter_; + talk_base::CpuMonitor* cpu_monitor_; talk_base::scoped_ptr overuse_observer_; }; @@ -900,7 +905,7 @@ int WebRtcVideoEngine::GetCapabilities() { return VIDEO_RECV | VIDEO_SEND; } -bool WebRtcVideoEngine::SetOptions(int options) { +bool WebRtcVideoEngine::SetOptions(const VideoOptions &options) { return true; } @@ -1223,7 +1228,8 @@ static void AddDefaultFeedbackParams(VideoCodec* codec) { } // Rebuilds the codec list to be only those that are less intensive -// than the specified codec. Prefers internal codec over external. +// than the specified codec. Prefers internal codec over external with +// higher preference field. bool WebRtcVideoEngine::RebuildCodecList(const VideoCodec& in_codec) { if (!FindCodec(in_codec)) return false; @@ -1262,7 +1268,9 @@ bool WebRtcVideoEngine::RebuildCodecList(const VideoCodec& in_codec) { codecs[i].max_width, codecs[i].max_height, codecs[i].max_fps, - static_cast(codecs.size() + ARRAY_SIZE(kVideoCodecPrefs) - i)); + // Use negative preference on external codec to ensure the internal + // codec is preferred. + static_cast(0 - i)); AddDefaultFeedbackParams(&codec); video_codecs_.push_back(codec); } diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index f0293bb5d..248975351 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -104,7 +104,7 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, void Terminate(); int GetCapabilities(); - bool SetOptions(int options); + bool SetOptions(const VideoOptions &options); bool SetDefaultEncoderConfig(const VideoEncoderConfig& config); WebRtcVideoMediaChannel* CreateChannel(VoiceMediaChannel* voice_channel); diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 04662caf7..953767397 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -1785,8 +1785,12 @@ TEST_F(WebRtcVideoEngineTestFake, ExternalCodecAddedToTheEnd) { encoder_factory_.NotifyCodecsAvailable(); codecs = engine_.codecs(); + cricket::VideoCodec internal_codec = codecs[0]; + cricket::VideoCodec external_codec = codecs[codecs.size() - 1]; // The external codec will appear at last. - EXPECT_EQ("GENERIC", codecs[codecs.size() - 1].name); + EXPECT_EQ("GENERIC", external_codec.name); + // The internal codec is preferred. + EXPECT_GE(internal_codec.preference, external_codec.preference); } // Test that external codec with be ignored if it has the same name as one of diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 2a6ccd761..0e964729e 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -50,6 +50,7 @@ #include "talk/media/base/streamparams.h" #include "talk/media/base/voiceprocessor.h" #include "talk/media/webrtc/webrtcvoe.h" +#include "webrtc/common.h" #include "webrtc/modules/audio_processing/include/audio_processing.h" #ifdef WIN32 @@ -476,6 +477,24 @@ bool WebRtcVoiceEngine::Init(talk_base::Thread* worker_thread) { return res; } +// Gets the default set of optoins applied to the engine. Historically, these +// were supplied as a combination of flags from the channel manager (ec, agc, +// ns, and highpass) and the rest hardcoded in InitInternal. +static AudioOptions GetDefaultEngineOptions() { + AudioOptions options; + options.echo_cancellation.Set(true); + options.auto_gain_control.Set(true); + options.noise_suppression.Set(true); + options.highpass_filter.Set(true); + options.typing_detection.Set(true); + options.conference_mode.Set(false); + options.adjust_agc_delta.Set(0); + options.experimental_agc.Set(false); + options.experimental_aec.Set(false); + options.aec_dump.Set(false); + return options; +} + bool WebRtcVoiceEngine::InitInternal() { // Temporarily turn logging level up for the Init call int old_filter = log_filter_; @@ -506,7 +525,10 @@ bool WebRtcVoiceEngine::InitInternal() { return false; } - if (!SetOptions(MediaEngineInterface::DEFAULT_AUDIO_OPTIONS)) { + // Set defaults for options, so that ApplyOptions applies them explicitly + // when we clear option (channel) overrides. External clients can still + // modify the defaults via SetOptions (on the media engine). + if (!SetOptions(GetDefaultEngineOptions())) { return false; } @@ -594,35 +616,7 @@ SoundclipMedia *WebRtcVoiceEngine::CreateSoundclip() { return soundclip; } -// TODO(zhurunz): Add a comprehensive unittests for SetOptions(). -bool WebRtcVoiceEngine::SetOptions(int flags) { - AudioOptions options; - - // Convert flags to AudioOptions. - options.echo_cancellation.Set( - ((flags & MediaEngineInterface::ECHO_CANCELLATION) != 0)); - options.auto_gain_control.Set( - ((flags & MediaEngineInterface::AUTO_GAIN_CONTROL) != 0)); - options.noise_suppression.Set( - ((flags & MediaEngineInterface::NOISE_SUPPRESSION) != 0)); - options.highpass_filter.Set( - ((flags & MediaEngineInterface::HIGHPASS_FILTER) != 0)); - options.stereo_swapping.Set( - ((flags & MediaEngineInterface::STEREO_FLIPPING) != 0)); - - // Set defaults for flagless options here. Make sure they are all set so that - // ApplyOptions applies all of them when we clear overrides. - options.typing_detection.Set(true); - options.conference_mode.Set(false); - options.adjust_agc_delta.Set(0); - options.experimental_agc.Set(false); - options.experimental_aec.Set(false); - options.aec_dump.Set(false); - - return SetAudioOptions(options); -} - -bool WebRtcVoiceEngine::SetAudioOptions(const AudioOptions& options) { +bool WebRtcVoiceEngine::SetOptions(const AudioOptions& options) { if (!ApplyOptions(options)) { return false; } @@ -684,7 +678,6 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { options.experimental_aec.Set(false); #endif - LOG(LS_INFO) << "Applying audio options: " << options.ToString(); webrtc::VoEAudioProcessing* voep = voe_wrapper_->processing(); @@ -766,6 +759,20 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { StopAecDump(); } + bool experimental_aec; + if (options.experimental_aec.Get(&experimental_aec)) { + webrtc::AudioProcessing* audioproc = + voe_wrapper_->base()->audio_processing(); + // We check audioproc for the benefit of tests, since FakeWebRtcVoiceEngine + // returns NULL on audio_processing(). + if (audioproc) { + webrtc::Config config; + config.Set( + new webrtc::DelayCorrection(experimental_aec)); + audioproc->SetExtraOptions(config); + } + } + return true; } @@ -2751,7 +2758,6 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info) { } } - webrtc::CallStatistics cs; unsigned int ssrc; webrtc::CodecInst codec; @@ -2819,6 +2825,8 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info) { sinfo.echo_return_loss_enhancement = echo_return_loss_enhancement; sinfo.echo_delay_median_ms = echo_delay_median_ms; sinfo.echo_delay_std_ms = echo_delay_std_ms; + // TODO(ajm): Re-enable this metric once we have a reliable implementation. + sinfo.aec_quality_min = -1; sinfo.typing_noise_detected = typing_noise_detected_; info->senders.push_back(sinfo); @@ -2928,13 +2936,11 @@ bool WebRtcVoiceMediaChannel::FindSsrc(int channel_num, uint32* ssrc) { } void WebRtcVoiceMediaChannel::OnError(uint32 ssrc, int error) { -#ifdef USE_WEBRTC_DEV_BRANCH if (error == VE_TYPING_NOISE_WARNING) { typing_noise_detected_ = true; } else if (error == VE_TYPING_NOISE_OFF_WARNING) { typing_noise_detected_ = false; } -#endif SignalMediaError(ssrc, WebRtcErrorToChannelError(error)); } diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 6cb0b3052..6e11485fc 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -105,12 +105,8 @@ class WebRtcVoiceEngine SoundclipMedia* CreateSoundclip(); - // TODO(pthatcher): Rename to SetOptions and replace the old - // flags-based SetOptions. - bool SetAudioOptions(const AudioOptions& options); - // Eventually, we will replace them with AudioOptions. - // In the meantime, we leave this here for backwards compat. - bool SetOptions(int flags); + AudioOptions GetOptions() const { return options_; } + bool SetOptions(const AudioOptions& options); // Overrides, when set, take precedence over the options on a // per-option basis. For example, if AGC is set in options and AEC // is set in overrides, AGC and AEC will be both be set. Overrides diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 3710e7ebc..2b2d36a40 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -2321,7 +2321,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { // Nothing set, so all ignored. cricket::AudioOptions options; - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetEcStatus(ec_enabled, ec_mode); voe_.GetEcMetricsStatus(ec_metrics_enabled); voe_.GetAecmMode(aecm_mode, cng_enabled); @@ -2345,14 +2345,14 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { // Turn echo cancellation off options.echo_cancellation.Set(false); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetEcStatus(ec_enabled, ec_mode); EXPECT_FALSE(ec_enabled); // Turn echo cancellation back on, with settings, and make sure // nothing else changed. options.echo_cancellation.Set(true); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetEcStatus(ec_enabled, ec_mode); voe_.GetEcMetricsStatus(ec_metrics_enabled); voe_.GetAecmMode(aecm_mode, cng_enabled); @@ -2375,14 +2375,14 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { // Turn off AGC options.auto_gain_control.Set(false); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetAgcStatus(agc_enabled, agc_mode); EXPECT_FALSE(agc_enabled); // Turn AGC back on options.auto_gain_control.Set(true); options.adjust_agc_delta.Clear(); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetAgcStatus(agc_enabled, agc_mode); EXPECT_TRUE(agc_enabled); voe_.GetAgcConfig(agc_config); @@ -2393,7 +2393,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { options.highpass_filter.Set(false); options.typing_detection.Set(false); options.stereo_swapping.Set(true); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetNsStatus(ns_enabled, ns_mode); highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); @@ -2405,7 +2405,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { // Turn on "conference mode" to ensure it has no impact. options.conference_mode.Set(true); - ASSERT_TRUE(engine_.SetAudioOptions(options)); + ASSERT_TRUE(engine_.SetOptions(options)); voe_.GetEcStatus(ec_enabled, ec_mode); voe_.GetNsStatus(ns_enabled, ns_mode); EXPECT_TRUE(ec_enabled); @@ -2414,7 +2414,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { EXPECT_EQ(webrtc::kNsHighSuppression, ns_mode); } -TEST_F(WebRtcVoiceEngineTestFake, SetOptions) { +TEST_F(WebRtcVoiceEngineTestFake, DefaultOptions) { EXPECT_TRUE(SetupEngine()); bool ec_enabled; @@ -2428,103 +2428,6 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptions) { bool stereo_swapping_enabled; bool typing_detection_enabled; - ASSERT_TRUE(engine_.SetOptions(0)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_FALSE(ec_enabled); - EXPECT_FALSE(agc_enabled); - EXPECT_FALSE(ns_enabled); - EXPECT_FALSE(highpass_filter_enabled); - EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::ECHO_CANCELLATION)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_TRUE(ec_enabled); - EXPECT_FALSE(agc_enabled); - EXPECT_FALSE(ns_enabled); - EXPECT_FALSE(highpass_filter_enabled); - EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::AUTO_GAIN_CONTROL)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_FALSE(ec_enabled); - EXPECT_TRUE(agc_enabled); - EXPECT_FALSE(ns_enabled); - EXPECT_FALSE(highpass_filter_enabled); - EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::NOISE_SUPPRESSION)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_FALSE(ec_enabled); - EXPECT_FALSE(agc_enabled); - EXPECT_TRUE(ns_enabled); - EXPECT_FALSE(highpass_filter_enabled); - EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::HIGHPASS_FILTER)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_FALSE(ec_enabled); - EXPECT_FALSE(agc_enabled); - EXPECT_FALSE(ns_enabled); - EXPECT_TRUE(highpass_filter_enabled); - EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::STEREO_FLIPPING)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_FALSE(ec_enabled); - EXPECT_FALSE(agc_enabled); - EXPECT_FALSE(ns_enabled); - EXPECT_FALSE(highpass_filter_enabled); - EXPECT_TRUE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::DEFAULT_AUDIO_OPTIONS)); voe_.GetEcStatus(ec_enabled, ec_mode); voe_.GetEcMetricsStatus(ec_metrics_enabled); voe_.GetAgcStatus(agc_enabled, agc_mode); @@ -2536,24 +2439,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptions) { EXPECT_TRUE(agc_enabled); EXPECT_TRUE(ns_enabled); EXPECT_TRUE(highpass_filter_enabled); + EXPECT_TRUE(typing_detection_enabled); EXPECT_FALSE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); - - ASSERT_TRUE(engine_.SetOptions( - cricket::MediaEngineInterface::ALL_AUDIO_OPTIONS)); - voe_.GetEcStatus(ec_enabled, ec_mode); - voe_.GetEcMetricsStatus(ec_metrics_enabled); - voe_.GetAgcStatus(agc_enabled, agc_mode); - voe_.GetNsStatus(ns_enabled, ns_mode); - highpass_filter_enabled = voe_.IsHighPassFilterEnabled(); - stereo_swapping_enabled = voe_.IsStereoChannelSwappingEnabled(); - voe_.GetTypingDetectionStatus(typing_detection_enabled); - EXPECT_TRUE(ec_enabled); - EXPECT_TRUE(agc_enabled); - EXPECT_TRUE(ns_enabled); - EXPECT_TRUE(highpass_filter_enabled); - EXPECT_TRUE(stereo_swapping_enabled); - EXPECT_TRUE(typing_detection_enabled); } TEST_F(WebRtcVoiceEngineTestFake, InitDoesNotOverwriteDefaultAgcConfig) { @@ -2625,7 +2512,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { ASSERT_TRUE(channel2->GetOptions(&actual_options)); EXPECT_EQ(expected_options, actual_options); - ASSERT_TRUE(engine_.SetAudioOptions(options_all)); + ASSERT_TRUE(engine_.SetOptions(options_all)); bool ec_enabled; webrtc::EcModes ec_mode; bool agc_enabled; @@ -2672,7 +2559,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_TRUE(ns_enabled); // Make sure settings take effect while we are sending. - ASSERT_TRUE(engine_.SetAudioOptions(options_all)); + ASSERT_TRUE(engine_.SetOptions(options_all)); cricket::AudioOptions options_no_agc_nor_ns; options_no_agc_nor_ns.auto_gain_control.Set(false); options_no_agc_nor_ns.noise_suppression.Set(false); diff --git a/talk/p2p/base/fakesession.h b/talk/p2p/base/fakesession.h index d162950a3..bc05ce2e1 100644 --- a/talk/p2p/base/fakesession.h +++ b/talk/p2p/base/fakesession.h @@ -391,6 +391,12 @@ class FakeSession : public BaseSession { NULL, "", "", initiator), fail_create_channel_(false) { } + FakeSession(bool initiator, talk_base::Thread* worker_thread) + : BaseSession(talk_base::Thread::Current(), + worker_thread, + NULL, "", "", initiator), + fail_create_channel_(false) { + } FakeTransport* GetTransport(const std::string& content_name) { return static_cast( diff --git a/talk/p2p/base/p2ptransportchannel.cc b/talk/p2p/base/p2ptransportchannel.cc index d45a66c40..8bdaa9a30 100644 --- a/talk/p2p/base/p2ptransportchannel.cc +++ b/talk/p2p/base/p2ptransportchannel.cc @@ -518,19 +518,21 @@ void P2PTransportChannel::OnUnknownAddress( // request came from. // There shouldn't be an existing connection with this remote address. - // When ports are muxed, this channel might get multiple unknown addres + // When ports are muxed, this channel might get multiple unknown address // signals. In that case if the connection is already exists, we should // simply ignore the signal othewise send server error. - if (port->GetConnection(new_remote_candidate.address()) && port_muxed) { - LOG(LS_INFO) << "Connection already exist for PeerReflexive candidate: " - << new_remote_candidate.ToString(); - return; - } else if (port->GetConnection(new_remote_candidate.address())) { - ASSERT(false); - port->SendBindingErrorResponse(stun_msg, address, - STUN_ERROR_SERVER_ERROR, - STUN_ERROR_REASON_SERVER_ERROR); - return; + if (port->GetConnection(new_remote_candidate.address())) { + if (port_muxed) { + LOG(LS_INFO) << "Connection already exists for peer reflexive " + << "candidate: " << new_remote_candidate.ToString(); + return; + } else { + ASSERT(false); + port->SendBindingErrorResponse(stun_msg, address, + STUN_ERROR_SERVER_ERROR, + STUN_ERROR_REASON_SERVER_ERROR); + return; + } } Connection* connection = port->CreateConnection( diff --git a/talk/p2p/base/session.cc b/talk/p2p/base/session.cc index 3128393c3..e5c86c188 100644 --- a/talk/p2p/base/session.cc +++ b/talk/p2p/base/session.cc @@ -26,6 +26,8 @@ */ #include "talk/p2p/base/session.h" + +#include "talk/base/bind.h" #include "talk/base/common.h" #include "talk/base/logging.h" #include "talk/base/helpers.h" @@ -44,6 +46,8 @@ namespace cricket { +using talk_base::Bind; + bool BadMessage(const buzz::QName type, const std::string& text, MessageError* err) { @@ -65,11 +69,13 @@ std::string TransportProxy::type() const { } TransportChannel* TransportProxy::GetChannel(int component) { + ASSERT(talk_base::Thread::Current() == worker_thread_); return GetChannelProxy(component); } TransportChannel* TransportProxy::CreateChannel( const std::string& name, int component) { + ASSERT(talk_base::Thread::Current() == worker_thread_); ASSERT(GetChannel(component) == NULL); ASSERT(!transport_->get()->HasChannel(component)); @@ -81,9 +87,9 @@ TransportChannel* TransportProxy::CreateChannel( // If we're already negotiated, create an impl and hook it up to the proxy // channel. If we're connecting, create an impl but don't hook it up yet. if (negotiated_) { - SetChannelProxyImpl(component, channel); + SetupChannelProxy_w(component, channel); } else if (connecting_) { - GetOrCreateChannelProxyImpl(component); + GetOrCreateChannelProxyImpl_w(component); } return channel; } @@ -93,6 +99,7 @@ bool TransportProxy::HasChannel(int component) { } void TransportProxy::DestroyChannel(int component) { + ASSERT(talk_base::Thread::Current() == worker_thread_); TransportChannel* channel = GetChannel(component); if (channel) { // If the state of TransportProxy is not NEGOTIATED @@ -100,7 +107,7 @@ void TransportProxy::DestroyChannel(int component) { // connected. Both must be connected before // deletion. if (!negotiated_) { - SetChannelProxyImpl(component, GetChannelProxy(component)); + SetupChannelProxy_w(component, GetChannelProxy(component)); } channels_.erase(component); @@ -130,7 +137,7 @@ void TransportProxy::CompleteNegotiation() { if (!negotiated_) { for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { - SetChannelProxyImpl(iter->first, iter->second); + SetupChannelProxy(iter->first, iter->second); } negotiated_ = true; } @@ -191,6 +198,13 @@ TransportChannelProxy* TransportProxy::GetChannelProxyByName( TransportChannelImpl* TransportProxy::GetOrCreateChannelProxyImpl( int component) { + return worker_thread_->Invoke(Bind( + &TransportProxy::GetOrCreateChannelProxyImpl_w, this, component)); +} + +TransportChannelImpl* TransportProxy::GetOrCreateChannelProxyImpl_w( + int component) { + ASSERT(talk_base::Thread::Current() == worker_thread_); TransportChannelImpl* impl = transport_->get()->GetChannel(component); if (impl == NULL) { impl = transport_->get()->CreateChannel(component); @@ -198,13 +212,33 @@ TransportChannelImpl* TransportProxy::GetOrCreateChannelProxyImpl( return impl; } -void TransportProxy::SetChannelProxyImpl( +void TransportProxy::SetupChannelProxy( int component, TransportChannelProxy* transproxy) { + worker_thread_->Invoke(Bind( + &TransportProxy::SetupChannelProxy_w, this, component, transproxy)); +} + +void TransportProxy::SetupChannelProxy_w( + int component, TransportChannelProxy* transproxy) { + ASSERT(talk_base::Thread::Current() == worker_thread_); TransportChannelImpl* impl = GetOrCreateChannelProxyImpl(component); ASSERT(impl != NULL); transproxy->SetImplementation(impl); } +void TransportProxy::ReplaceChannelProxyImpl(TransportChannelProxy* proxy, + TransportChannelImpl* impl) { + worker_thread_->Invoke(Bind( + &TransportProxy::ReplaceChannelProxyImpl_w, this, proxy, impl)); +} + +void TransportProxy::ReplaceChannelProxyImpl_w(TransportChannelProxy* proxy, + TransportChannelImpl* impl) { + ASSERT(talk_base::Thread::Current() == worker_thread_); + ASSERT(proxy != NULL); + proxy->SetImplementation(impl); +} + // This function muxes |this| onto |target| by repointing |this| at // |target|'s transport and setting our TransportChannelProxies // to point to |target|'s underlying implementations. @@ -220,12 +254,12 @@ bool TransportProxy::SetupMux(TransportProxy* target) { iter != channels_.end(); ++iter) { if (!target->transport_->get()->HasChannel(iter->first)) { // Remove if channel doesn't exist in |transport_|. - iter->second->SetImplementation(NULL); + ReplaceChannelProxyImpl(iter->second, NULL); } else { // Replace the impl for all the TransportProxyChannels with the channels // from |target|'s transport. Fail if there's not an exact match. - iter->second->SetImplementation( - target->transport_->get()->CreateChannel(iter->first)); + ReplaceChannelProxyImpl( + iter->second, target->transport_->get()->CreateChannel(iter->first)); } } @@ -489,7 +523,7 @@ TransportProxy* BaseSession::GetOrCreateTransportProxy( transport->SignalRoleConflict.connect( this, &BaseSession::OnRoleConflict); - transproxy = new TransportProxy(sid_, content_name, + transproxy = new TransportProxy(worker_thread_, sid_, content_name, new TransportWrapper(transport)); transproxy->SignalCandidatesReady.connect( this, &BaseSession::OnTransportProxyCandidatesReady); diff --git a/talk/p2p/base/session.h b/talk/p2p/base/session.h index e05e3c0ac..12310bc8e 100644 --- a/talk/p2p/base/session.h +++ b/talk/p2p/base/session.h @@ -91,10 +91,12 @@ class TransportProxy : public sigslot::has_slots<>, public CandidateTranslator { public: TransportProxy( + talk_base::Thread* worker_thread, const std::string& sid, const std::string& content_name, TransportWrapper* transport) - : sid_(sid), + : worker_thread_(worker_thread), + sid_(sid), content_name_(content_name), transport_(transport), connecting_(false), @@ -168,12 +170,21 @@ class TransportProxy : public sigslot::has_slots<>, private: TransportChannelProxy* GetChannelProxy(int component) const; TransportChannelProxy* GetChannelProxyByName(const std::string& name) const; - void ReplaceChannelProxyImpl(TransportChannelProxy* channel_proxy, - size_t index); - TransportChannelImpl* GetOrCreateChannelProxyImpl(int component); - void SetChannelProxyImpl(int component, - TransportChannelProxy* proxy); + TransportChannelImpl* GetOrCreateChannelProxyImpl(int component); + TransportChannelImpl* GetOrCreateChannelProxyImpl_w(int component); + + // Manipulators of transportchannelimpl in channel proxy. + void SetupChannelProxy(int component, + TransportChannelProxy* proxy); + void SetupChannelProxy_w(int component, + TransportChannelProxy* proxy); + void ReplaceChannelProxyImpl(TransportChannelProxy* proxy, + TransportChannelImpl* impl); + void ReplaceChannelProxyImpl_w(TransportChannelProxy* proxy, + TransportChannelImpl* impl); + + talk_base::Thread* worker_thread_; std::string sid_; std::string content_name_; talk_base::scoped_refptr transport_; diff --git a/talk/p2p/base/transportchannelproxy.cc b/talk/p2p/base/transportchannelproxy.cc index 04b32ce64..318d133a0 100644 --- a/talk/p2p/base/transportchannelproxy.cc +++ b/talk/p2p/base/transportchannelproxy.cc @@ -27,6 +27,7 @@ #include "talk/p2p/base/transportchannelproxy.h" #include "talk/base/common.h" +#include "talk/base/logging.h" #include "talk/base/thread.h" #include "talk/p2p/base/transport.h" #include "talk/p2p/base/transportchannelimpl.h" @@ -54,8 +55,15 @@ TransportChannelProxy::~TransportChannelProxy() { } void TransportChannelProxy::SetImplementation(TransportChannelImpl* impl) { - // TODO(juberti): Fix this to occur on the correct thread. - // ASSERT(talk_base::Thread::Current() == worker_thread_); + ASSERT(talk_base::Thread::Current() == worker_thread_); + + if (impl == impl_) { + ASSERT(false); + // Ignore if the |impl| has already been set. + LOG(LS_WARNING) << "Ignored TransportChannelProxy::SetImplementation call " + << "with a same impl as the existing one."; + return; + } // Destroy any existing impl_. if (impl_) { diff --git a/talk/session/media/call.cc b/talk/session/media/call.cc index c963c36ec..51a721b0e 100644 --- a/talk/session/media/call.cc +++ b/talk/session/media/call.cc @@ -1079,7 +1079,11 @@ Session* Call::InternalInitiateSession(const std::string& id, const SessionDescription* offer = session_client_->CreateOffer(options); Session* session = session_client_->CreateSession(id, this); - session->set_initiator_name(initiator_name); + // Only override the initiator_name if it was manually supplied. Otherwise, + // session_client_ will supply the local jid as initiator in CreateOffer. + if (!initiator_name.empty()) { + session->set_initiator_name(initiator_name); + } AddSession(session, offer); session->Initiate(to.Str(), offer); diff --git a/talk/session/media/call.h b/talk/session/media/call.h index 9b0a6c9c4..efb4ea396 100644 --- a/talk/session/media/call.h +++ b/talk/session/media/call.h @@ -117,6 +117,9 @@ class Call : public talk_base::MessageHandler, public sigslot::has_slots<> { MediaStreams* recv_streams = GetMediaStreams(session); return recv_streams ? &recv_streams->audio() : NULL; } + VoiceChannel* GetVoiceChannel(Session* session) const; + VideoChannel* GetVideoChannel(Session* session) const; + DataChannel* GetDataChannel(Session* session) const; // Public just for unit tests VideoContentDescription* CreateVideoStreamUpdate(const StreamParams& stream); // Takes ownership of video. @@ -193,9 +196,6 @@ class Call : public talk_base::MessageHandler, public sigslot::has_slots<> { void OnDataReceived(DataChannel* channel, const ReceiveDataParams& params, const talk_base::Buffer& payload); - VoiceChannel* GetVoiceChannel(Session* session) const; - VideoChannel* GetVideoChannel(Session* session) const; - DataChannel* GetDataChannel(Session* session) const; MediaStreams* GetMediaStreams(Session* session) const; void UpdateRemoteMediaStreams(Session* session, const ContentInfos& updated_contents, diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index f6259e9a3..cfe58a71d 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -641,12 +641,6 @@ bool BaseChannel::PacketIsRtcp(const TransportChannel* channel, bool BaseChannel::SendPacket(bool rtcp, talk_base::Buffer* packet, talk_base::DiffServCodePoint dscp) { - // Unless we're sending optimistically, we only allow packets through when we - // are completely writable. - if (!optimistic_data_send_ && !writable_) { - return false; - } - // SendPacket gets called from MediaEngine, typically on an encoder thread. // If the thread is not our worker thread, we will post to our worker // so that the real work happens on our worker. This avoids us having to diff --git a/talk/session/media/channelmanager.cc b/talk/session/media/channelmanager.cc index 36c71832d..47f0fc57a 100644 --- a/talk/session/media/channelmanager.cc +++ b/talk/session/media/channelmanager.cc @@ -116,9 +116,10 @@ void ChannelManager::Construct(MediaEngineInterface* me, initialized_ = false; main_thread_ = talk_base::Thread::Current(); worker_thread_ = worker_thread; + // Get the default audio options from the media engine. + audio_options_ = media_engine_->GetAudioOptions(); audio_in_device_ = DeviceManagerInterface::kDefaultDeviceName; audio_out_device_ = DeviceManagerInterface::kDefaultDeviceName; - audio_options_ = MediaEngineInterface::DEFAULT_AUDIO_OPTIONS; audio_delay_offset_ = MediaEngineInterface::kDefaultAudioDelayOffset; audio_output_volume_ = kNotSetOutputVolume; local_renderer_ = NULL; @@ -251,7 +252,7 @@ bool ChannelManager::Init() { LOG(LS_WARNING) << "Failed to SetAudioOptions with" << " microphone: " << audio_in_device_ << " speaker: " << audio_out_device_ - << " options: " << audio_options_ + << " options: " << audio_options_.ToString() << " delay: " << audio_delay_offset_; } @@ -502,23 +503,26 @@ void ChannelManager::DestroySoundclip_w(Soundclip* soundclip) { } bool ChannelManager::GetAudioOptions(std::string* in_name, - std::string* out_name, int* opts) { + std::string* out_name, + AudioOptions* options) { if (in_name) *in_name = audio_in_device_; if (out_name) *out_name = audio_out_device_; - if (opts) - *opts = audio_options_; + if (options) + *options = audio_options_; return true; } bool ChannelManager::SetAudioOptions(const std::string& in_name, - const std::string& out_name, int opts) { - return SetAudioOptions(in_name, out_name, opts, audio_delay_offset_); + const std::string& out_name, + const AudioOptions& options) { + return SetAudioOptions(in_name, out_name, options, audio_delay_offset_); } bool ChannelManager::SetAudioOptions(const std::string& in_name, - const std::string& out_name, int opts, + const std::string& out_name, + const AudioOptions& options, int delay_offset) { // Get device ids from DeviceManager. Device in_dev, out_dev; @@ -536,12 +540,12 @@ bool ChannelManager::SetAudioOptions(const std::string& in_name, if (initialized_) { ret = worker_thread_->Invoke( Bind(&ChannelManager::SetAudioOptions_w, this, - opts, delay_offset, &in_dev, &out_dev)); + options, delay_offset, &in_dev, &out_dev)); } // If all worked well, save the values for use in GetAudioOptions. if (ret) { - audio_options_ = opts; + audio_options_ = options; audio_in_device_ = in_name; audio_out_device_ = out_name; audio_delay_offset_ = delay_offset; @@ -549,13 +553,14 @@ bool ChannelManager::SetAudioOptions(const std::string& in_name, return ret; } -bool ChannelManager::SetAudioOptions_w(int opts, int delay_offset, +bool ChannelManager::SetAudioOptions_w( + const AudioOptions& options, int delay_offset, const Device* in_dev, const Device* out_dev) { ASSERT(worker_thread_ == talk_base::Thread::Current()); ASSERT(initialized_); // Set audio options - bool ret = media_engine_->SetAudioOptions(opts); + bool ret = media_engine_->SetAudioOptions(options); if (ret) { ret = media_engine_->SetAudioDelayOffset(delay_offset); diff --git a/talk/session/media/channelmanager.h b/talk/session/media/channelmanager.h index 04af5e196..af95f9209 100644 --- a/talk/session/media/channelmanager.h +++ b/talk/session/media/channelmanager.h @@ -137,9 +137,11 @@ class ChannelManager : public talk_base::MessageHandler, // Configures the audio and video devices. A null pointer can be passed to // GetAudioOptions() for any parameter of no interest. bool GetAudioOptions(std::string* wave_in_device, - std::string* wave_out_device, int* opts); + std::string* wave_out_device, + AudioOptions* options); bool SetAudioOptions(const std::string& wave_in_device, - const std::string& wave_out_device, int opts); + const std::string& wave_out_device, + const AudioOptions& options); bool GetOutputVolume(int* level); bool SetOutputVolume(int level); bool IsSameCapturer(const std::string& capturer_name, @@ -227,7 +229,8 @@ class ChannelManager : public talk_base::MessageHandler, // Adds non-transient parameters which can only be changed through the // options store. bool SetAudioOptions(const std::string& wave_in_device, - const std::string& wave_out_device, int opts, + const std::string& wave_out_device, + const AudioOptions& options, int delay_offset); int audio_delay_offset() const { return audio_delay_offset_; } @@ -256,8 +259,8 @@ class ChannelManager : public talk_base::MessageHandler, void DestroyDataChannel_w(DataChannel* data_channel); Soundclip* CreateSoundclip_w(); void DestroySoundclip_w(Soundclip* soundclip); - bool SetAudioOptions_w(int opts, int delay_offset, const Device* in_dev, - const Device* out_dev); + bool SetAudioOptions_w(const AudioOptions& options, int delay_offset, + const Device* in_dev, const Device* out_dev); bool SetCaptureDevice_w(const Device* cam_device); void OnVideoCaptureStateChange(VideoCapturer* capturer, CaptureState result); @@ -283,7 +286,7 @@ class ChannelManager : public talk_base::MessageHandler, std::string audio_in_device_; std::string audio_out_device_; - int audio_options_; + AudioOptions audio_options_; int audio_delay_offset_; int audio_output_volume_; std::string camera_device_; diff --git a/talk/session/media/channelmanager_unittest.cc b/talk/session/media/channelmanager_unittest.cc index 6f7c76871..d0ba8b934 100644 --- a/talk/session/media/channelmanager_unittest.cc +++ b/talk/session/media/channelmanager_unittest.cc @@ -149,11 +149,12 @@ TEST_F(ChannelManagerTest, CreateDestroyChannels) { } // Test that we can create and destroy a voice and video channel with a worker. -// BUG=https://code.google.com/p/webrtc/issues/detail?id=2355 -TEST_F(ChannelManagerTest, DISABLED_CreateDestroyChannelsOnThread) { +TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { worker_.Start(); EXPECT_TRUE(cm_->set_worker_thread(&worker_)); EXPECT_TRUE(cm_->Init()); + delete session_; + session_ = new cricket::FakeSession(true, &worker_); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( session_, cricket::CN_AUDIO, false); EXPECT_TRUE(voice_channel != NULL); @@ -254,44 +255,43 @@ TEST_F(ChannelManagerTest, SetDefaultVideoCodecBeforeInit) { TEST_F(ChannelManagerTest, SetAudioOptionsBeforeInit) { // Test that values that we set before Init are applied. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", 0x2)); + AudioOptions options; + options.auto_gain_control.Set(true); + options.echo_cancellation.Set(false); + EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", options)); + std::string audio_in, audio_out; + AudioOptions set_options; + // Check options before Init. + EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &set_options)); + EXPECT_EQ("audio-in1", audio_in); + EXPECT_EQ("audio-out1", audio_out); + EXPECT_EQ(options, set_options); EXPECT_TRUE(cm_->Init()); - EXPECT_EQ("audio-in1", fme_->audio_in_device()); - EXPECT_EQ("audio-out1", fme_->audio_out_device()); - EXPECT_EQ(0x2, fme_->audio_options()); - EXPECT_EQ(0, fme_->audio_delay_offset()); + // Check options after Init. + EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &set_options)); + EXPECT_EQ("audio-in1", audio_in); + EXPECT_EQ("audio-out1", audio_out); + EXPECT_EQ(options, set_options); + // At this point, the media engine should also be initialized. + EXPECT_EQ(options, fme_->audio_options()); EXPECT_EQ(cricket::MediaEngineInterface::kDefaultAudioDelayOffset, fme_->audio_delay_offset()); } -TEST_F(ChannelManagerTest, GetAudioOptionsBeforeInit) { - std::string audio_in, audio_out; - int opts; - // Test that GetAudioOptions works before Init. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in2", "audio-out2", 0x1)); - EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &opts)); - EXPECT_EQ("audio-in2", audio_in); - EXPECT_EQ("audio-out2", audio_out); - EXPECT_EQ(0x1, opts); - // Test that options set before Init can be gotten after Init. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", 0x2)); - EXPECT_TRUE(cm_->Init()); - EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &opts)); - EXPECT_EQ("audio-in1", audio_in); - EXPECT_EQ("audio-out1", audio_out); - EXPECT_EQ(0x2, opts); -} - TEST_F(ChannelManagerTest, GetAudioOptionsWithNullParameters) { std::string audio_in, audio_out; - int opts; - EXPECT_TRUE(cm_->SetAudioOptions("audio-in2", "audio-out2", 0x1)); + AudioOptions options; + options.echo_cancellation.Set(true); + EXPECT_TRUE(cm_->SetAudioOptions("audio-in2", "audio-out2", options)); EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, NULL, NULL)); EXPECT_EQ("audio-in2", audio_in); EXPECT_TRUE(cm_->GetAudioOptions(NULL, &audio_out, NULL)); EXPECT_EQ("audio-out2", audio_out); - EXPECT_TRUE(cm_->GetAudioOptions(NULL, NULL, &opts)); - EXPECT_EQ(0x1, opts); + AudioOptions out_options; + EXPECT_TRUE(cm_->GetAudioOptions(NULL, NULL, &out_options)); + bool echo_cancellation = false; + EXPECT_TRUE(out_options.echo_cancellation.Get(&echo_cancellation)); + EXPECT_TRUE(echo_cancellation); } TEST_F(ChannelManagerTest, SetAudioOptions) { @@ -301,47 +301,22 @@ TEST_F(ChannelManagerTest, SetAudioOptions) { fme_->audio_in_device()); EXPECT_EQ(std::string(cricket::DeviceManagerInterface::kDefaultDeviceName), fme_->audio_out_device()); - EXPECT_EQ(cricket::MediaEngineInterface::DEFAULT_AUDIO_OPTIONS, - fme_->audio_options()); - EXPECT_EQ(cricket::MediaEngineInterface::kDefaultAudioDelayOffset, - fme_->audio_delay_offset()); - // Test setting defaults. - EXPECT_TRUE(cm_->SetAudioOptions("", "", - cricket::MediaEngineInterface::DEFAULT_AUDIO_OPTIONS)); - EXPECT_EQ("", fme_->audio_in_device()); - EXPECT_EQ("", fme_->audio_out_device()); - EXPECT_EQ(cricket::MediaEngineInterface::DEFAULT_AUDIO_OPTIONS, - fme_->audio_options()); EXPECT_EQ(cricket::MediaEngineInterface::kDefaultAudioDelayOffset, fme_->audio_delay_offset()); // Test setting specific values. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", 0x2)); + AudioOptions options; + options.auto_gain_control.Set(true); + EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", options)); EXPECT_EQ("audio-in1", fme_->audio_in_device()); EXPECT_EQ("audio-out1", fme_->audio_out_device()); - EXPECT_EQ(0x2, fme_->audio_options()); + bool auto_gain_control = false; + EXPECT_TRUE( + fme_->audio_options().auto_gain_control.Get(&auto_gain_control)); + EXPECT_TRUE(auto_gain_control); EXPECT_EQ(cricket::MediaEngineInterface::kDefaultAudioDelayOffset, fme_->audio_delay_offset()); // Test setting bad values. - EXPECT_FALSE(cm_->SetAudioOptions("audio-in9", "audio-out2", 0x1)); -} - -TEST_F(ChannelManagerTest, GetAudioOptions) { - std::string audio_in, audio_out; - int opts; - // Test initial state. - EXPECT_TRUE(cm_->Init()); - EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &opts)); - EXPECT_EQ(std::string(cricket::DeviceManagerInterface::kDefaultDeviceName), - audio_in); - EXPECT_EQ(std::string(cricket::DeviceManagerInterface::kDefaultDeviceName), - audio_out); - EXPECT_EQ(cricket::MediaEngineInterface::DEFAULT_AUDIO_OPTIONS, opts); - // Test that we get back specific values that we set. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", 0x2)); - EXPECT_TRUE(cm_->GetAudioOptions(&audio_in, &audio_out, &opts)); - EXPECT_EQ("audio-in1", audio_in); - EXPECT_EQ("audio-out1", audio_out); - EXPECT_EQ(0x2, opts); + EXPECT_FALSE(cm_->SetAudioOptions("audio-in9", "audio-out2", options)); } TEST_F(ChannelManagerTest, SetCaptureDeviceBeforeInit) { @@ -380,7 +355,8 @@ TEST_F(ChannelManagerTest, SetCaptureDevice) { // device is plugged back, we use it. TEST_F(ChannelManagerTest, SetAudioOptionsUnplugPlug) { // Set preferences "audio-in1" and "audio-out1" before init. - EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", 0x2)); + AudioOptions options; + EXPECT_TRUE(cm_->SetAudioOptions("audio-in1", "audio-out1", options)); // Unplug device "audio-in1" and "audio-out1". std::vector in_device_list, out_device_list; in_device_list.push_back("audio-in2"); diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index 856123082..799fe5f5c 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -38,12 +38,17 @@ #include "talk/base/stringutils.h" #include "talk/media/base/constants.h" #include "talk/media/base/cryptoparams.h" -#include "talk/media/sctp/sctpdataengine.h" #include "talk/p2p/base/constants.h" #include "talk/session/media/channelmanager.h" #include "talk/session/media/srtpfilter.h" #include "talk/xmpp/constants.h" +#ifdef HAVE_SCTP +#include "talk/media/sctp/sctpdataengine.h" +#else +static const uint32 kMaxSctpSid = USHRT_MAX; +#endif + namespace { const char kInline[] = "inline:"; } diff --git a/talk/session/media/mediasessionclient.h b/talk/session/media/mediasessionclient.h index 1ade753f9..d0034cafe 100644 --- a/talk/session/media/mediasessionclient.h +++ b/talk/session/media/mediasessionclient.h @@ -112,8 +112,8 @@ class MediaSessionClient : public SessionClient, public sigslot::has_slots<> { } bool SetAudioOptions(const std::string& in_name, const std::string& out_name, - int opts) { - return channel_manager_->SetAudioOptions(in_name, out_name, opts); + const AudioOptions& options) { + return channel_manager_->SetAudioOptions(in_name, out_name, options); } bool SetOutputVolume(int level) { return channel_manager_->SetOutputVolume(level); diff --git a/talk/xmpp/mucroomdiscoverytask.cc b/talk/xmpp/mucroomdiscoverytask.cc index a5055d2da..c7477ae3e 100644 --- a/talk/xmpp/mucroomdiscoverytask.cc +++ b/talk/xmpp/mucroomdiscoverytask.cc @@ -56,11 +56,11 @@ void MucRoomDiscoveryTask::HandleResult(const XmlElement* stanza) { const std::string name(identity->Attr(QN_NAME)); // Get the conversation id - const XmlElement* convIdElement = + const XmlElement* conversation = identity->FirstNamed(QN_GOOGLE_MUC_HANGOUT_CONVERSATION_ID); std::string conversation_id; - if (convIdElement != NULL) { - conversation_id = convIdElement->BodyText(); + if (conversation != NULL) { + conversation_id = conversation->BodyText(); } for (const XmlElement* feature = query->FirstNamed(QN_DISCO_FEATURE);