From 4f85288e71136671ae194fcdd730e2d0f0241db9 Mon Sep 17 00:00:00 2001 From: "guoweis@webrtc.org" Date: Thu, 12 Mar 2015 20:09:44 +0000 Subject: [PATCH] Socket options are only applied when first setting TransportChannelImpl. Also fixed the issue when we have an TransportChannelImpl, the socket option is not preserved. Since this is a code path that will be modified by bundle (which Peter also has a test case already), we don't need a test case here. BUG=4374 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/42699004 Cr-Commit-Position: refs/heads/master@{#8702} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8702 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/webrtcsession_unittest.cc | 48 +++++++++++++++++++++++ talk/session/media/channel.h | 4 +- webrtc/p2p/base/transportchannelproxy.cc | 15 +++---- webrtc/p2p/base/transportchannelproxy.h | 2 +- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index b23ba5444..6bfa17e4f 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -3546,6 +3546,54 @@ TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesSeparated) { SetLocalDescriptionWithoutError(answer); } +// This verifies that the voice channel after bundle has both options from video +// and voice channels. +TEST_F(WebRtcSessionTest, TestSetSocketOptionBeforeBundle) { + InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced); + mediastream_signaling_.SendAudioVideoStream1(); + + PeerConnectionInterface::RTCOfferAnswerOptions options; + options.use_rtp_mux = true; + + SessionDescriptionInterface* offer = CreateOffer(options); + SetLocalDescriptionWithoutError(offer); + + session_->video_channel()->SetOption(cricket::BaseChannel::ST_RTP, + rtc::Socket::Option::OPT_SNDBUF, 4000); + + session_->voice_channel()->SetOption(cricket::BaseChannel::ST_RTP, + rtc::Socket::Option::OPT_RCVBUF, 8000); + + int option_val; + EXPECT_TRUE(session_->video_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_SNDBUF, &option_val)); + EXPECT_EQ(4000, option_val); + EXPECT_FALSE(session_->voice_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_SNDBUF, &option_val)); + + EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_RCVBUF, &option_val)); + EXPECT_EQ(8000, option_val); + EXPECT_FALSE(session_->video_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_RCVBUF, &option_val)); + + EXPECT_NE(session_->voice_channel()->transport_channel(), + session_->video_channel()->transport_channel()); + + mediastream_signaling_.SendAudioVideoStream2(); + SessionDescriptionInterface* answer = + CreateRemoteAnswer(session_->local_description()); + SetRemoteDescriptionWithoutError(answer); + + EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_SNDBUF, &option_val)); + EXPECT_EQ(4000, option_val); + + EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption( + rtc::Socket::Option::OPT_RCVBUF, &option_val)); + EXPECT_EQ(8000, option_val); +} + // 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/session/media/channel.h b/talk/session/media/channel.h index 05818a0f7..1d0a887ff 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -230,6 +230,9 @@ class BaseChannel // Made public for easier testing. void SetReadyToSend(TransportChannel* channel, bool ready); + // Only public for unit tests. Otherwise, consider protected. + virtual int SetOption(SocketType type, rtc::Socket::Option o, int val); + protected: MediaEngineInterface* media_engine() const { return media_engine_; } virtual MediaChannel* media_channel() const { return media_channel_; } @@ -254,7 +257,6 @@ class BaseChannel rtc::DiffServCodePoint dscp); virtual bool SendRtcp(rtc::Buffer* packet, rtc::DiffServCodePoint dscp); - virtual int SetOption(SocketType type, rtc::Socket::Option o, int val); // From TransportChannel void OnWritableState(TransportChannel* channel); diff --git a/webrtc/p2p/base/transportchannelproxy.cc b/webrtc/p2p/base/transportchannelproxy.cc index a278d9646..511892009 100644 --- a/webrtc/p2p/base/transportchannelproxy.cc +++ b/webrtc/p2p/base/transportchannelproxy.cc @@ -67,17 +67,14 @@ void TransportChannelProxy::SetImplementation(TransportChannelImpl* impl) { this, &TransportChannelProxy::OnReadyToSend); impl_->SignalRouteChange.connect( this, &TransportChannelProxy::OnRouteChange); - for (OptionList::iterator it = pending_options_.begin(); - it != pending_options_.end(); - ++it) { - impl_->SetOption(it->first, it->second); + for (const auto& pair : options_) { + impl_->SetOption(pair.first, pair.second); } // Push down the SRTP ciphers, if any were set. if (!pending_srtp_ciphers_.empty()) { impl_->SetSrtpCiphers(pending_srtp_ciphers_); } - pending_options_.clear(); } // Post ourselves a message to see if we need to fire state callbacks. @@ -97,8 +94,8 @@ int TransportChannelProxy::SendPacket(const char* data, size_t len, int TransportChannelProxy::SetOption(rtc::Socket::Option opt, int value) { ASSERT(rtc::Thread::Current() == worker_thread_); + options_.push_back(OptionPair(opt, value)); if (!impl_) { - pending_options_.push_back(OptionPair(opt, value)); return 0; } return impl_->SetOption(opt, value); @@ -110,9 +107,9 @@ bool TransportChannelProxy::GetOption(rtc::Socket::Option opt, int* value) { return impl_->GetOption(opt, value); } - for (const auto& pending : pending_options_) { - if (pending.first == opt) { - *value = pending.second; + for (const auto& pair : options_) { + if (pair.first == opt) { + *value = pair.second; return true; } } diff --git a/webrtc/p2p/base/transportchannelproxy.h b/webrtc/p2p/base/transportchannelproxy.h index 19870028b..ffdad4326 100644 --- a/webrtc/p2p/base/transportchannelproxy.h +++ b/webrtc/p2p/base/transportchannelproxy.h @@ -88,7 +88,7 @@ class TransportChannelProxy : public TransportChannel, std::string name_; rtc::Thread* worker_thread_; TransportChannelImpl* impl_; - OptionList pending_options_; + OptionList options_; std::vector pending_srtp_ciphers_; DISALLOW_EVIL_CONSTRUCTORS(TransportChannelProxy);