diff --git a/talk/app/webrtc/datachannel.cc b/talk/app/webrtc/datachannel.cc index 3de001b54..3a6046e81 100644 --- a/talk/app/webrtc/datachannel.cc +++ b/talk/app/webrtc/datachannel.cc @@ -31,6 +31,7 @@ #include "talk/app/webrtc/mediastreamprovider.h" #include "talk/base/logging.h" #include "talk/base/refcount.h" +#include "talk/media/sctp/sctputils.h" namespace webrtc { @@ -68,36 +69,35 @@ DataChannel::DataChannel( } bool DataChannel::Init(const DataChannelInit* config) { - if (config) { - if (data_channel_type_ == cricket::DCT_RTP && - (config->reliable || - config->id != -1 || - config->maxRetransmits != -1 || - config->maxRetransmitTime != -1)) { - LOG(LS_ERROR) << "Failed to initialize the RTP data channel due to " + if (data_channel_type_ == cricket::DCT_RTP && + (config->reliable || + config->id != -1 || + config->maxRetransmits != -1 || + config->maxRetransmitTime != -1)) { + LOG(LS_ERROR) << "Failed to initialize the RTP data channel due to " + << "invalid DataChannelInit."; + return false; + } else if (data_channel_type_ == cricket::DCT_SCTP) { + if (config->id < -1 || + config->maxRetransmits < -1 || + config->maxRetransmitTime < -1) { + LOG(LS_ERROR) << "Failed to initialize the SCTP data channel due to " << "invalid DataChannelInit."; return false; - } else if (data_channel_type_ == cricket::DCT_SCTP) { - if (config->id < -1 || - config->maxRetransmits < -1 || - config->maxRetransmitTime < -1) { - LOG(LS_ERROR) << "Failed to initialize the SCTP data channel due to " - << "invalid DataChannelInit."; - return false; - } - if (config->maxRetransmits != -1 && config->maxRetransmitTime != -1) { - LOG(LS_ERROR) << - "maxRetransmits and maxRetransmitTime should not be both set."; - return false; - } + } + if (config->maxRetransmits != -1 && config->maxRetransmitTime != -1) { + LOG(LS_ERROR) << + "maxRetransmits and maxRetransmitTime should not be both set."; + return false; } config_ = *config; - } - return true; -} -bool DataChannel::HasNegotiationCompleted() { - return send_ssrc_set_ == receive_ssrc_set_; + // Try to connect to the transport in case the transport channel already + // exists. + OnTransportChannelCreated(); + } + + return true; } DataChannel::~DataChannel() { @@ -169,16 +169,13 @@ void DataChannel::QueueControl(const talk_base::Buffer* buffer) { queued_control_data_.push(buffer); } -bool DataChannel::SendControl(const talk_base::Buffer* buffer) { - if (data_channel_type_ == cricket::DCT_RTP) { - delete buffer; - return false; - } +bool DataChannel::SendOpenMessage(const talk_base::Buffer* raw_buffer) { + ASSERT(data_channel_type_ == cricket::DCT_SCTP && + was_ever_writable_ && + config_.id >= 0 && + !config_.negotiated); - if (state_ != kOpen) { - QueueControl(buffer); - return true; - } + talk_base::scoped_ptr buffer(raw_buffer); cricket::SendDataParams send_params; send_params.ssrc = config_.id; @@ -189,18 +186,15 @@ bool DataChannel::SendControl(const talk_base::Buffer* buffer) { bool retval = provider_->SendData(send_params, *buffer, &send_result); if (!retval && send_result == cricket::SDR_BLOCK) { // Link is congested. Queue for later. - QueueControl(buffer); - } else { - delete buffer; + QueueControl(buffer.release()); } return retval; } void DataChannel::SetReceiveSsrc(uint32 receive_ssrc) { + ASSERT(data_channel_type_ == cricket::DCT_RTP); + if (receive_ssrc_set_) { - ASSERT(data_channel_type_ == cricket::DCT_RTP || - !send_ssrc_set_ || - receive_ssrc_ == send_ssrc_); return; } receive_ssrc_ = receive_ssrc; @@ -214,10 +208,8 @@ void DataChannel::RemotePeerRequestClose() { } void DataChannel::SetSendSsrc(uint32 send_ssrc) { + ASSERT(data_channel_type_ == cricket::DCT_RTP); if (send_ssrc_set_) { - ASSERT(data_channel_type_ == cricket::DCT_RTP || - !receive_ssrc_set_ || - receive_ssrc_ == send_ssrc_); return; } send_ssrc_ = send_ssrc; @@ -263,8 +255,18 @@ void DataChannel::OnChannelReady(bool writable) { // for sending and now unblocked, so send the queued data now. if (!was_ever_writable_) { was_ever_writable_ = true; + + if (data_channel_type_ == cricket::DCT_SCTP && !config_.negotiated) { + talk_base::Buffer* payload = new talk_base::Buffer; + if (!cricket::WriteDataChannelOpenMessage(label_, config_, payload)) { + // TODO(jiayl): close the data channel on this error. + LOG(LS_ERROR) << "Could not write data channel OPEN message"; + return; + } + SendOpenMessage(payload); + } + UpdateState(); - DeliverQueuedControlData(); ASSERT(queued_send_data_.empty()); } else if (state_ == kOpen) { DeliverQueuedSendData(); @@ -281,11 +283,15 @@ void DataChannel::DoClose() { void DataChannel::UpdateState() { switch (state_) { case kConnecting: { - if (HasNegotiationCompleted()) { - if (!connected_to_provider_) { - ConnectToDataSession(); + if (send_ssrc_set_ == receive_ssrc_set_) { + if (data_channel_type_ == cricket::DCT_RTP && !connected_to_provider_) { + connected_to_provider_ = provider_->ConnectDataChannel(this); + provider_->AddRtpDataStream(send_ssrc_, receive_ssrc_); } if (was_ever_writable_) { + // TODO(jiayl): Do not transition to kOpen if we failed to send the + // OPEN message. + DeliverQueuedControlData(); SetState(kOpen); // If we have received buffers before the channel got writable. // Deliver them now. @@ -298,10 +304,9 @@ void DataChannel::UpdateState() { break; } case kClosing: { - if (connected_to_provider_) { - DisconnectFromDataSession(); - } - if (HasNegotiationCompleted()) { + DisconnectFromTransport(); + + if (!send_ssrc_set_ && !receive_ssrc_set_) { SetState(kClosed); } break; @@ -318,13 +323,18 @@ void DataChannel::SetState(DataState state) { } } -void DataChannel::ConnectToDataSession() { - connected_to_provider_ = provider_->ConnectDataChannel(this); -} +void DataChannel::DisconnectFromTransport() { + if (!connected_to_provider_) + return; -void DataChannel::DisconnectFromDataSession() { provider_->DisconnectDataChannel(this); connected_to_provider_ = false; + + if (data_channel_type_ == cricket::DCT_RTP) { + provider_->RemoveRtpDataStream(send_ssrc_, receive_ssrc_); + } else { + provider_->RemoveSctpDataStream(config_.id); + } } void DataChannel::DeliverQueuedReceivedData() { @@ -349,10 +359,12 @@ void DataChannel::ClearQueuedReceivedData() { } void DataChannel::DeliverQueuedSendData() { + ASSERT(was_ever_writable_ && state_ == kOpen); + + // TODO(jiayl): Sending OPEN message here contradicts with the pre-condition + // that the readyState is open. According to the standard, the channel should + // not become open before the OPEN message is sent. DeliverQueuedControlData(); - if (!was_ever_writable_) { - return; - } while (!queued_send_data_.empty()) { DataBuffer* buffer = queued_send_data_.front(); @@ -376,12 +388,11 @@ void DataChannel::ClearQueuedControlData() { } void DataChannel::DeliverQueuedControlData() { - if (was_ever_writable_) { - while (!queued_control_data_.empty()) { - const talk_base::Buffer *buf = queued_control_data_.front(); - queued_control_data_.pop(); - SendControl(buf); - } + ASSERT(was_ever_writable_); + while (!queued_control_data_.empty()) { + const talk_base::Buffer* buf = queued_control_data_.front(); + queued_control_data_.pop(); + SendOpenMessage(buf); } } @@ -417,4 +428,22 @@ bool DataChannel::QueueSendData(const DataBuffer& buffer) { return true; } +void DataChannel::SetSctpSid(int sid) { + ASSERT(config_.id < 0 && sid >= 0 && data_channel_type_ == cricket::DCT_SCTP); + config_.id = sid; + provider_->AddSctpDataStream(sid); +} + +void DataChannel::OnTransportChannelCreated() { + ASSERT(data_channel_type_ == cricket::DCT_SCTP); + if (!connected_to_provider_) { + connected_to_provider_ = provider_->ConnectDataChannel(this); + } + // The sid may have been unassigned when provider_->ConnectDataChannel was + // done. So always add the streams even if connected_to_provider_ is true. + if (config_.id >= 0) { + provider_->AddSctpDataStream(config_.id); + } +} + } // namespace webrtc diff --git a/talk/app/webrtc/datachannel.h b/talk/app/webrtc/datachannel.h index a09c7f6d6..0d67293db 100644 --- a/talk/app/webrtc/datachannel.h +++ b/talk/app/webrtc/datachannel.h @@ -44,24 +44,35 @@ class DataChannel; class DataChannelProviderInterface { public: + // Sends the data to the transport. virtual bool SendData(const cricket::SendDataParams& params, const talk_base::Buffer& payload, cricket::SendDataResult* result) = 0; + // Connects to the transport signals. virtual bool ConnectDataChannel(DataChannel* data_channel) = 0; + // Disconnects from the transport signals. virtual void DisconnectDataChannel(DataChannel* data_channel) = 0; + // Adds the send and receive stream ssrc to the transport for RTP. + virtual void AddRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) = 0; + // Adds the data channel SID to the transport for SCTP. + virtual void AddSctpDataStream(uint32 sid) = 0; + // Removes the data channel ssrcs from the transport for RTP. + virtual void RemoveRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) = 0; + // Removes the data channel SID from the transport for SCTP. + virtual void RemoveSctpDataStream(uint32 sid) = 0; protected: virtual ~DataChannelProviderInterface() {} }; // DataChannel is a an implementation of the DataChannelInterface based on -// libjingle's data engine. It provides an implementation of unreliable data -// channels. Currently this class is specifically designed to use RtpDataEngine, -// and will changed to use SCTP in the future. +// libjingle's data engine. It provides an implementation of unreliable or +// reliabledata channels. Currently this class is specifically designed to use +// both RtpDataEngine and SctpDataEngine. // DataChannel states: -// kConnecting: The channel has been created but SSRC for sending and receiving -// has not yet been set and the transport might not yet be ready. +// kConnecting: The channel has been created the transport might not yet be +// ready. // kOpen: The channel have a local SSRC set by a call to UpdateSendSsrc // and a remote SSRC set by call to UpdateReceiveSsrc and the transport // has been writable once. @@ -73,7 +84,7 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> { public: static talk_base::scoped_refptr Create( - DataChannelProviderInterface* client, + DataChannelProviderInterface* provider, cricket::DataChannelType dct, const std::string& label, const DataChannelInit* config); @@ -97,20 +108,6 @@ class DataChannel : public DataChannelInterface, virtual void Close(); virtual DataState state() const { return state_; } virtual bool Send(const DataBuffer& buffer); - // Send a control message right now, or queue for later. - virtual bool SendControl(const talk_base::Buffer* buffer); - void ConnectToDataSession(); - - // Set the SSRC this channel should use to receive data from the - // underlying data engine. - void SetReceiveSsrc(uint32 receive_ssrc); - // The remote peer request that this channel should be closed. - void RemotePeerRequestClose(); - - // Set the SSRC this channel should use to send data on the - // underlying data engine. |send_ssrc| == 0 means that the channel is no - // longer part of the session negotiation. - void SetSendSsrc(uint32 send_ssrc); // Called if the underlying data engine is closing. void OnDataEngineClose(); @@ -125,20 +122,38 @@ class DataChannel : public DataChannelInterface, const cricket::ReceiveDataParams& params, const talk_base::Buffer& payload); + // The remote peer request that this channel should be closed. + void RemotePeerRequestClose(); + + // The following methods are for SCTP only. + + // Sets the SCTP sid and adds to transport layer if not set yet. + void SetSctpSid(int sid); + // Called when the transport channel is created. + void OnTransportChannelCreated(); + + // The following methods are for RTP only. + + // Set the SSRC this channel should use to send data on the + // underlying data engine. |send_ssrc| == 0 means that the channel is no + // longer part of the session negotiation. + void SetSendSsrc(uint32 send_ssrc); + // Set the SSRC this channel should use to receive data from the + // underlying data engine. + void SetReceiveSsrc(uint32 receive_ssrc); + protected: DataChannel(DataChannelProviderInterface* client, cricket::DataChannelType dct, const std::string& label); virtual ~DataChannel(); - bool Init(const DataChannelInit* config); - bool HasNegotiationCompleted(); - private: + bool Init(const DataChannelInit* config); void DoClose(); void UpdateState(); void SetState(DataState state); - void DisconnectFromDataSession(); + void DisconnectFromTransport(); void DeliverQueuedControlData(); void QueueControl(const talk_base::Buffer* buffer); void ClearQueuedControlData(); @@ -149,6 +164,8 @@ class DataChannel : public DataChannelInterface, bool InternalSendWithoutQueueing(const DataBuffer& buffer, cricket::SendDataResult* send_result); bool QueueSendData(const DataBuffer& buffer); + bool SendOpenMessage(const talk_base::Buffer* buffer); + std::string label_; DataChannelInit config_; diff --git a/talk/app/webrtc/datachannel_unittest.cc b/talk/app/webrtc/datachannel_unittest.cc index 4d669078d..5d298110c 100644 --- a/talk/app/webrtc/datachannel_unittest.cc +++ b/talk/app/webrtc/datachannel_unittest.cc @@ -26,56 +26,56 @@ */ #include "talk/app/webrtc/datachannel.h" +#include "talk/app/webrtc/test/fakedatachannelprovider.h" #include "talk/base/gunit.h" using webrtc::DataChannel; -class FakeDataChannelClient : public webrtc::DataChannelProviderInterface { - public: - FakeDataChannelClient() : send_blocked_(false) {} - virtual ~FakeDataChannelClient() {} - - virtual bool SendData(const cricket::SendDataParams& params, - const talk_base::Buffer& payload, - cricket::SendDataResult* result) OVERRIDE { - if (send_blocked_) { - *result = cricket::SDR_BLOCK; - return false; - } - last_send_data_params_ = params; - return true; - } - virtual bool ConnectDataChannel(DataChannel* data_channel) OVERRIDE { - return true; - } - virtual void DisconnectDataChannel(DataChannel* data_channel) OVERRIDE {} - - void set_send_blocked(bool blocked) { send_blocked_ = blocked; } - cricket::SendDataParams last_send_data_params() { - return last_send_data_params_; - } - - private: - cricket::SendDataParams last_send_data_params_; - bool send_blocked_; -}; - class SctpDataChannelTest : public testing::Test { protected: SctpDataChannelTest() : webrtc_data_channel_( - DataChannel::Create(&client_, cricket::DCT_SCTP, "test", &init_)) { + DataChannel::Create(&provider_, cricket::DCT_SCTP, "test", &init_)) { } void SetChannelReady() { + webrtc_data_channel_->OnTransportChannelCreated(); + if (webrtc_data_channel_->id() < 0) { + webrtc_data_channel_->SetSctpSid(0); + } webrtc_data_channel_->OnChannelReady(true); } webrtc::DataChannelInit init_; - FakeDataChannelClient client_; + FakeDataChannelProvider provider_; talk_base::scoped_refptr webrtc_data_channel_; }; +// Verifies that the data channel is connected to the transport after creation. +TEST_F(SctpDataChannelTest, ConnectedToTransportOnCreated) { + EXPECT_TRUE(provider_.IsConnected(webrtc_data_channel_.get())); + // The sid is not set yet, so it should not have added the streams. + EXPECT_FALSE(provider_.IsSendStreamAdded(webrtc_data_channel_->id())); + EXPECT_FALSE(provider_.IsRecvStreamAdded(webrtc_data_channel_->id())); + + webrtc_data_channel_->SetSctpSid(0); + EXPECT_TRUE(provider_.IsSendStreamAdded(webrtc_data_channel_->id())); + EXPECT_TRUE(provider_.IsRecvStreamAdded(webrtc_data_channel_->id())); +} + +// Verifies that the data channel is connected to the transport if the transport +// is not available initially and becomes available later. +TEST_F(SctpDataChannelTest, ConnectedAfterTransportBecomesAvailable) { + provider_.set_transport_available(false); + talk_base::scoped_refptr dc = DataChannel::Create( + &provider_, cricket::DCT_SCTP, "test1", &init_); + EXPECT_FALSE(provider_.IsConnected(dc.get())); + + provider_.set_transport_available(true); + dc->OnTransportChannelCreated(); + EXPECT_TRUE(provider_.IsConnected(dc.get())); +} + // Tests the state of the data channel. TEST_F(SctpDataChannelTest, StateTransition) { EXPECT_EQ(webrtc::DataChannelInterface::kConnecting, @@ -85,6 +85,8 @@ TEST_F(SctpDataChannelTest, StateTransition) { webrtc_data_channel_->Close(); EXPECT_EQ(webrtc::DataChannelInterface::kClosed, webrtc_data_channel_->state()); + // Verifies that it's disconnected from the transport. + EXPECT_FALSE(provider_.IsConnected(webrtc_data_channel_.get())); } // Tests that DataChannel::buffered_amount() is correct after the channel is @@ -96,7 +98,7 @@ TEST_F(SctpDataChannelTest, BufferedAmountWhenBlocked) { EXPECT_EQ(0U, webrtc_data_channel_->buffered_amount()); - client_.set_send_blocked(true); + provider_.set_send_blocked(true); const int number_of_packets = 3; for (int i = 0; i < number_of_packets; ++i) { @@ -111,18 +113,22 @@ TEST_F(SctpDataChannelTest, BufferedAmountWhenBlocked) { TEST_F(SctpDataChannelTest, QueuedDataSentWhenUnblocked) { SetChannelReady(); webrtc::DataBuffer buffer("abcd"); - client_.set_send_blocked(true); + provider_.set_send_blocked(true); EXPECT_TRUE(webrtc_data_channel_->Send(buffer)); - client_.set_send_blocked(false); + provider_.set_send_blocked(false); SetChannelReady(); EXPECT_EQ(0U, webrtc_data_channel_->buffered_amount()); } // Tests that the queued control message is sent when channel is ready. -TEST_F(SctpDataChannelTest, QueuedControlMessageSent) { - talk_base::Buffer* buffer = new talk_base::Buffer("abcd", 4); - EXPECT_TRUE(webrtc_data_channel_->SendControl(buffer)); +TEST_F(SctpDataChannelTest, OpenMessageSent) { + // Initially the id is unassigned. + EXPECT_EQ(-1, webrtc_data_channel_->id()); + SetChannelReady(); - EXPECT_EQ(cricket::DMT_CONTROL, client_.last_send_data_params().type); + EXPECT_GE(webrtc_data_channel_->id(), 0); + EXPECT_EQ(cricket::DMT_CONTROL, provider_.last_send_data_params().type); + EXPECT_EQ(provider_.last_send_data_params().ssrc, + static_cast(webrtc_data_channel_->id())); } diff --git a/talk/app/webrtc/mediastreamsignaling.cc b/talk/app/webrtc/mediastreamsignaling.cc index ef9f3e020..91dde24e0 100644 --- a/talk/app/webrtc/mediastreamsignaling.cc +++ b/talk/app/webrtc/mediastreamsignaling.cc @@ -37,6 +37,7 @@ #include "talk/app/webrtc/videosource.h" #include "talk/app/webrtc/videotrack.h" #include "talk/base/bytebuffer.h" +#include "talk/base/stringutils.h" #include "talk/media/sctp/sctpdataengine.h" static const char kDefaultStreamLabel[] = "default"; @@ -189,7 +190,8 @@ MediaStreamSignaling::MediaStreamSignaling( remote_streams_(StreamCollection::Create()), remote_stream_factory_(new RemoteMediaStreamFactory(signaling_thread, channel_manager)), - last_allocated_sctp_id_(0) { + last_allocated_sctp_even_sid_(-2), + last_allocated_sctp_odd_sid_(-1) { options_.has_video = false; options_.has_audio = false; } @@ -203,36 +205,37 @@ void MediaStreamSignaling::TearDown() { OnDataChannelClose(); } -bool MediaStreamSignaling::IsSctpIdAvailable(int id) const { - if (id < 0 || id > static_cast(cricket::kMaxSctpSid)) +bool MediaStreamSignaling::IsSctpSidAvailable(int sid) const { + if (sid < 0 || sid > static_cast(cricket::kMaxSctpSid)) return false; for (DataChannels::const_iterator iter = data_channels_.begin(); iter != data_channels_.end(); ++iter) { - if (iter->second->id() == id) { + if (iter->second->id() == sid) { return false; } } return true; } -// Gets the first id that has not been taken by existing data -// channels. Starting from 1. -// Returns false if no id can be allocated. -// TODO(jiayl): Update to some kind of even/odd random number selection when the -// rules are fully standardized. -bool MediaStreamSignaling::AllocateSctpId(int* id) { - do { - last_allocated_sctp_id_++; - } while (last_allocated_sctp_id_ <= static_cast(cricket::kMaxSctpSid) && - !IsSctpIdAvailable(last_allocated_sctp_id_)); +// Gets the first unused odd/even id based on the DTLS role. If |role| is +// SSL_CLIENT, the allocated id starts from 0 and takes even numbers; otherwise, +// the id starts from 1 and takes odd numbers. Returns false if no id can be +// allocated. +bool MediaStreamSignaling::AllocateSctpSid(talk_base::SSLRole role, int* sid) { + int& last_id = (role == talk_base::SSL_CLIENT) ? + last_allocated_sctp_even_sid_ : last_allocated_sctp_odd_sid_; - if (last_allocated_sctp_id_ > static_cast(cricket::kMaxSctpSid)) { - last_allocated_sctp_id_ = cricket::kMaxSctpSid; + do { + last_id += 2; + } while (last_id <= static_cast(cricket::kMaxSctpSid) && + !IsSctpSidAvailable(last_id)); + + if (last_id > static_cast(cricket::kMaxSctpSid)) { return false; } - *id = last_allocated_sctp_id_; + *sid = last_id; return true; } @@ -392,9 +395,8 @@ void MediaStreamSignaling::OnRemoteDescriptionChanged( const cricket::DataContentDescription* data_desc = static_cast( data_content->description); - if (data_desc->protocol() == cricket::kMediaProtocolDtlsSctp) { - UpdateRemoteSctpDataChannels(); - } else { + if (talk_base::starts_with( + data_desc->protocol().data(), cricket::kMediaProtocolRtpPrefix)) { UpdateRemoteRtpDataChannels(data_desc->streams()); } } @@ -448,9 +450,8 @@ void MediaStreamSignaling::OnLocalDescriptionChanged( const cricket::DataContentDescription* data_desc = static_cast( data_content->description); - if (data_desc->protocol() == cricket::kMediaProtocolDtlsSctp) { - UpdateLocalSctpDataChannels(); - } else { + if (talk_base::starts_with( + data_desc->protocol().data(), cricket::kMediaProtocolRtpPrefix)) { UpdateLocalRtpDataChannels(data_desc->streams()); } } @@ -919,20 +920,26 @@ void MediaStreamSignaling::CreateRemoteDataChannel(const std::string& label, stream_observer_->OnAddDataChannel(channel); } - -void MediaStreamSignaling::UpdateLocalSctpDataChannels() { +void MediaStreamSignaling::OnDataTransportCreatedForSctp() { DataChannels::iterator it = data_channels_.begin(); for (; it != data_channels_.end(); ++it) { DataChannel* data_channel = it->second; - data_channel->SetSendSsrc(data_channel->id()); + data_channel->OnTransportChannelCreated(); } } -void MediaStreamSignaling::UpdateRemoteSctpDataChannels() { +void MediaStreamSignaling::OnDtlsRoleReadyForSctp(talk_base::SSLRole role) { DataChannels::iterator it = data_channels_.begin(); for (; it != data_channels_.end(); ++it) { DataChannel* data_channel = it->second; - data_channel->SetReceiveSsrc(data_channel->id()); + if (data_channel->id() < 0) { + int sid; + if (!AllocateSctpSid(role, &sid)) { + LOG(LS_ERROR) << "Failed to allocate SCTP sid."; + continue; + } + data_channel->SetSctpSid(sid); + } } } diff --git a/talk/app/webrtc/mediastreamsignaling.h b/talk/app/webrtc/mediastreamsignaling.h index 067ed2f3b..a0ed6199d 100644 --- a/talk/app/webrtc/mediastreamsignaling.h +++ b/talk/app/webrtc/mediastreamsignaling.h @@ -174,11 +174,11 @@ class MediaStreamSignaling { } // Checks if |id| is available to be assigned to a new SCTP data channel. - bool IsSctpIdAvailable(int id) const; + bool IsSctpSidAvailable(int sid) const; // Gets the first available SCTP id that is not assigned to any existing // data channels. - bool AllocateSctpId(int* id); + bool AllocateSctpSid(talk_base::SSLRole role, int* sid); // Adds |local_stream| to the collection of known MediaStreams that will be // offered in a SessionDescription. @@ -249,8 +249,8 @@ class MediaStreamSignaling { StreamCollectionInterface* remote_streams() const { return remote_streams_.get(); } - void UpdateLocalSctpDataChannels(); - void UpdateRemoteSctpDataChannels(); + void OnDataTransportCreatedForSctp(); + void OnDtlsRoleReadyForSctp(talk_base::SSLRole role); private: struct RemotePeerInfo { @@ -380,7 +380,9 @@ class MediaStreamSignaling { TrackInfos local_audio_tracks_; TrackInfos local_video_tracks_; - int last_allocated_sctp_id_; + int last_allocated_sctp_even_sid_; + int last_allocated_sctp_odd_sid_; + typedef std::map > DataChannels; DataChannels data_channels_; diff --git a/talk/app/webrtc/mediastreamsignaling_unittest.cc b/talk/app/webrtc/mediastreamsignaling_unittest.cc index ea1336450..df4b1f506 100644 --- a/talk/app/webrtc/mediastreamsignaling_unittest.cc +++ b/talk/app/webrtc/mediastreamsignaling_unittest.cc @@ -32,6 +32,7 @@ #include "talk/app/webrtc/mediastreamsignaling.h" #include "talk/app/webrtc/streamcollection.h" #include "talk/app/webrtc/test/fakeconstraints.h" +#include "talk/app/webrtc/test/fakedatachannelprovider.h" #include "talk/app/webrtc/videotrack.h" #include "talk/base/gunit.h" #include "talk/base/scoped_ptr.h" @@ -127,7 +128,7 @@ static const char kSdpStringWithMsidWithoutStreams[] = "o=- 0 0 IN IP4 127.0.0.1\r\n" "s=-\r\n" "t=0 0\r\n" - "a:msid-semantic: WMS\r\n" + "a=msid-semantic: WMS\r\n" "m=audio 1 RTP/AVPF 103\r\n" "a=mid:audio\r\n" "a=rtpmap:103 ISAC/16000\r\n" @@ -1012,4 +1013,43 @@ TEST_F(MediaStreamSignalingTest, ChangeSsrcOnTrackInLocalSessionDescription) { observer_->VerifyLocalVideoTrack(kStreams[0], kVideoTracks[0], 98); } +// Verifies that an even SCTP id is allocated for SSL_CLIENT and an odd id for +// SSL_SERVER. +TEST_F(MediaStreamSignalingTest, SctpIdAllocationBasedOnRole) { + int id; + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_SERVER, &id)); + EXPECT_EQ(1, id); + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_CLIENT, &id)); + EXPECT_EQ(0, id); + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_SERVER, &id)); + EXPECT_EQ(3, id); + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_CLIENT, &id)); + EXPECT_EQ(2, id); +} +// Verifies that SCTP ids of existing DataChannels are not reused. +TEST_F(MediaStreamSignalingTest, SctpIdAllocationNoReuse) { + talk_base::scoped_ptr provider( + new FakeDataChannelProvider()); + // Creates a DataChannel with id 1. + webrtc::DataChannelInit config; + config.id = 1; + talk_base::scoped_refptr data_channel( + webrtc::DataChannel::Create( + provider.get(), cricket::DCT_SCTP, "a", &config)); + ASSERT_TRUE(data_channel.get() != NULL); + ASSERT_TRUE(signaling_->AddDataChannel(data_channel.get())); + + int new_id; + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_SERVER, &new_id)); + EXPECT_NE(config.id, new_id); + + // Creates a DataChannel with id 0. + config.id = 0; + data_channel = webrtc::DataChannel::Create( + provider.get(), cricket::DCT_SCTP, "b", &config); + ASSERT_TRUE(data_channel.get() != NULL); + ASSERT_TRUE(signaling_->AddDataChannel(data_channel.get())); + ASSERT_TRUE(signaling_->AllocateSctpSid(talk_base::SSL_CLIENT, &new_id)); + EXPECT_NE(config.id, new_id); +} diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index de88f8862..bc69d486a 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -427,14 +427,6 @@ PeerConnection::CreateDataChannel( if (!channel.get()) return NULL; - // If we've already passed the underlying channel's setup phase, have the - // MediaStreamSignaling update data channels manually. - if (session_->data_channel() != NULL && - session_->data_channel_type() == cricket::DCT_SCTP) { - mediastream_signaling_->UpdateLocalSctpDataChannels(); - mediastream_signaling_->UpdateRemoteSctpDataChannels(); - } - observer_->OnRenegotiationNeeded(); return DataChannelProxy::Create(signaling_thread(), channel.get()); diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc index b1d354484..ea94ee134 100644 --- a/talk/app/webrtc/peerconnectioninterface_unittest.cc +++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc @@ -981,31 +981,6 @@ TEST_F(PeerConnectionInterfaceTest, EXPECT_TRUE(channel == NULL); } -// The test verifies that the first id not used by existing data channels is -// assigned to a new data channel if no id is specified. -TEST_F(PeerConnectionInterfaceTest, AssignSctpDataChannelId) { - FakeConstraints constraints; - constraints.SetAllowDtlsSctpDataChannels(); - CreatePeerConnection(&constraints); - - webrtc::DataChannelInit config; - - scoped_refptr channel = - pc_->CreateDataChannel("1", &config); - EXPECT_TRUE(channel != NULL); - EXPECT_EQ(1, channel->id()); - - config.id = 4; - channel = pc_->CreateDataChannel("4", &config); - EXPECT_TRUE(channel != NULL); - EXPECT_EQ(config.id, channel->id()); - - config.id = -1; - channel = pc_->CreateDataChannel("2", &config); - EXPECT_TRUE(channel != NULL); - EXPECT_EQ(2, channel->id()); -} - // The test verifies that creating a SCTP data channel with an id already in use // or out of range should fail. TEST_F(PeerConnectionInterfaceTest, @@ -1015,13 +990,13 @@ TEST_F(PeerConnectionInterfaceTest, CreatePeerConnection(&constraints); webrtc::DataChannelInit config; + scoped_refptr channel; - scoped_refptr channel = - pc_->CreateDataChannel("1", &config); + config.id = 1; + channel = pc_->CreateDataChannel("1", &config); EXPECT_TRUE(channel != NULL); EXPECT_EQ(1, channel->id()); - config.id = 1; channel = pc_->CreateDataChannel("x", &config); EXPECT_TRUE(channel == NULL); @@ -1095,6 +1070,8 @@ TEST_F(PeerConnectionInterfaceTest, TestRejectDataChannelInAnswer) { // Test that we can create a session description from an SDP string from // FireFox, use it as a remote session description, generate an answer and use // the answer as a local description. +// TODO(mallinath): re-enable per +// https://code.google.com/p/webrtc/issues/detail?id=2574 TEST_F(PeerConnectionInterfaceTest, DISABLED_ReceiveFireFoxOffer) { MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp); FakeConstraints constraints; diff --git a/talk/app/webrtc/test/fakedatachannelprovider.h b/talk/app/webrtc/test/fakedatachannelprovider.h new file mode 100644 index 000000000..332641930 --- /dev/null +++ b/talk/app/webrtc/test/fakedatachannelprovider.h @@ -0,0 +1,109 @@ +/* + * libjingle + * Copyright 2013, Google 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/app/webrtc/datachannel.h" + +class FakeDataChannelProvider : public webrtc::DataChannelProviderInterface { + public: + FakeDataChannelProvider() + : id_allocation_should_fail_(false), + send_blocked_(false), + transport_available_(true) {} + virtual ~FakeDataChannelProvider() {} + + virtual bool SendData(const cricket::SendDataParams& params, + const talk_base::Buffer& payload, + cricket::SendDataResult* result) OVERRIDE { + if (send_blocked_) { + *result = cricket::SDR_BLOCK; + return false; + } + last_send_data_params_ = params; + return true; + } + virtual bool ConnectDataChannel(webrtc::DataChannel* data_channel) OVERRIDE { + ASSERT(connected_channels_.find(data_channel) == connected_channels_.end()); + if (!transport_available_) { + return false; + } + LOG(LS_INFO) << "DataChannel connected " << data_channel; + connected_channels_.insert(data_channel); + return true; + } + virtual void DisconnectDataChannel( + webrtc::DataChannel* data_channel) OVERRIDE { + ASSERT(connected_channels_.find(data_channel) != connected_channels_.end()); + LOG(LS_INFO) << "DataChannel disconnected " << data_channel; + connected_channels_.erase(data_channel); + } + virtual void AddRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) OVERRIDE { + send_ssrcs_.insert(send_ssrc); + recv_ssrcs_.insert(recv_ssrc); + } + virtual void AddSctpDataStream(uint32 sid) OVERRIDE { + AddRtpDataStream(sid, sid); + } + virtual void RemoveRtpDataStream( + uint32 send_ssrc, uint32 recv_ssrc) OVERRIDE { + send_ssrcs_.erase(send_ssrc); + recv_ssrcs_.erase(recv_ssrc); + } + virtual void RemoveSctpDataStream(uint32 sid) OVERRIDE { + RemoveRtpDataStream(sid, sid); + } + + void set_send_blocked(bool blocked) { + send_blocked_ = blocked; + } + cricket::SendDataParams last_send_data_params() const { + return last_send_data_params_; + } + void set_id_allocaiton_should_fail(bool fail) { + id_allocation_should_fail_ = fail; + } + void set_transport_available(bool available) { + transport_available_ = available; + } + bool IsConnected(webrtc::DataChannel* data_channel) const { + return connected_channels_.find(data_channel) != connected_channels_.end(); + } + bool IsSendStreamAdded(uint32 stream) const { + return send_ssrcs_.find(stream) != send_ssrcs_.end(); + } + bool IsRecvStreamAdded(uint32 stream) const { + return recv_ssrcs_.find(stream) != recv_ssrcs_.end(); + } + + private: + cricket::SendDataParams last_send_data_params_; + bool id_allocation_should_fail_; + bool send_blocked_; + bool transport_available_; + std::set connected_channels_; + std::set send_ssrcs_; + std::set recv_ssrcs_; +}; diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index 610250ead..ce23459e4 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -2188,6 +2188,12 @@ bool ParseMediaDescription(const std::string& message, return ParseFailed("", description.str(), error); } } + + size_t end_of_message = message.size(); + if (mline_index == -1 && *pos != end_of_message) { + ParseFailed(message, *pos, "Expects m line.", error); + return false; + } return true; } diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index 777a707b6..97ec843fc 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -1484,6 +1484,19 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescription) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc)); } +TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutMline) { + JsepSessionDescription jdesc(kDummyString); + const char kSdpWithoutMline[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=msid-semantic: WMS local_stream_1 local_stream_2\r\n"; + // Deserialize + EXPECT_TRUE(SdpDeserialize(kSdpWithoutMline, &jdesc)); + EXPECT_EQ(0u, jdesc.description()->contents().size()); +} + TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutCarriageReturn) { JsepSessionDescription jdesc(kDummyString); std::string sdp_without_carriage_return = kSdpFullString; @@ -1886,6 +1899,7 @@ TEST_F(WebRtcSdpTest, DeserializeBrokenSdp) { ReplaceAndTryToParse("t=", kSdpDestroyer); // Broken media description + ReplaceAndTryToParse("m=audio", "c=IN IP4 74.125.224.39"); ReplaceAndTryToParse("m=video", kSdpDestroyer); // Invalid lines diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 69a47c468..3f1426837 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -42,7 +42,6 @@ #include "talk/base/stringencode.h" #include "talk/media/base/constants.h" #include "talk/media/base/videocapturer.h" -#include "talk/media/sctp/sctputils.h" #include "talk/session/media/channel.h" #include "talk/session/media/channelmanager.h" #include "talk/session/media/mediasession.h" @@ -629,6 +628,10 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, // local session description. mediastream_signaling_->OnLocalDescriptionChanged(local_desc_.get()); + talk_base::SSLRole role; + if (data_channel_type_ == cricket::DCT_SCTP && GetSslRole(&role)) { + mediastream_signaling_->OnDtlsRoleReadyForSctp(role); + } if (error() != cricket::BaseSession::ERROR_NONE) { return BadLocalSdp(SessionErrorMsg(error()), err_desc); } @@ -681,6 +684,12 @@ bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, ice_restart_latch_->CheckForRemoteIceRestart(remote_desc_.get(), desc); remote_desc_.reset(desc_temp.release()); + + talk_base::SSLRole role; + if (data_channel_type_ == cricket::DCT_SCTP && GetSslRole(&role)) { + mediastream_signaling_->OnDtlsRoleReadyForSctp(role); + } + if (error() != cricket::BaseSession::ERROR_NONE) { return BadRemoteSdp(SessionErrorMsg(error()), err_desc); } @@ -961,25 +970,49 @@ bool WebRtcSession::ConnectDataChannel(DataChannel* webrtc_data_channel) { LOG(LS_ERROR) << "ConnectDataChannel called when data_channel_ is NULL."; return false; } - data_channel_->SignalReadyToSendData.connect(webrtc_data_channel, &DataChannel::OnChannelReady); data_channel_->SignalDataReceived.connect(webrtc_data_channel, &DataChannel::OnDataReceived); - cricket::StreamParams params = - cricket::StreamParams::CreateLegacy(webrtc_data_channel->id()); - data_channel_->AddRecvStream(params); - data_channel_->AddSendStream(params); return true; } void WebRtcSession::DisconnectDataChannel(DataChannel* webrtc_data_channel) { - data_channel_->RemoveSendStream(webrtc_data_channel->id()); - data_channel_->RemoveRecvStream(webrtc_data_channel->id()); + if (!data_channel_.get()) { + LOG(LS_ERROR) << "DisconnectDataChannel called when data_channel_ is NULL."; + return; + } data_channel_->SignalReadyToSendData.disconnect(webrtc_data_channel); data_channel_->SignalDataReceived.disconnect(webrtc_data_channel); } +void WebRtcSession::AddRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) { + if (!data_channel_.get()) { + LOG(LS_ERROR) << "AddDataChannelStreams called when data_channel_ is NULL."; + return; + } + data_channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(recv_ssrc)); + data_channel_->AddSendStream(cricket::StreamParams::CreateLegacy(send_ssrc)); +} + +void WebRtcSession::AddSctpDataStream(uint32 sid) { + AddRtpDataStream(sid, sid); +} + +void WebRtcSession::RemoveRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) { + if (!data_channel_.get()) { + LOG(LS_ERROR) << "RemoveDataChannelStreams called when data_channel_ is " + << "NULL."; + return; + } + data_channel_->RemoveRecvStream(recv_ssrc); + data_channel_->RemoveSendStream(send_ssrc); +} + +void WebRtcSession::RemoveSctpDataStream(uint32 sid) { + RemoveRtpDataStream(sid, sid); +} + talk_base::scoped_refptr WebRtcSession::CreateDataChannel( const std::string& label, const DataChannelInit* config) { @@ -994,11 +1027,13 @@ talk_base::scoped_refptr WebRtcSession::CreateDataChannel( if (data_channel_type_ == cricket::DCT_SCTP) { if (new_config.id < 0) { - if (!mediastream_signaling_->AllocateSctpId(&new_config.id)) { + talk_base::SSLRole role; + if (GetSslRole(&role) && + !mediastream_signaling_->AllocateSctpSid(role, &new_config.id)) { LOG(LS_ERROR) << "No id can be allocated for the SCTP data channel."; return NULL; } - } else if (!mediastream_signaling_->IsSctpIdAvailable(new_config.id)) { + } else if (!mediastream_signaling_->IsSctpSidAvailable(new_config.id)) { LOG(LS_ERROR) << "Failed to create a SCTP data channel " << "because the id is already in use or out of range."; return NULL; @@ -1007,30 +1042,9 @@ talk_base::scoped_refptr WebRtcSession::CreateDataChannel( talk_base::scoped_refptr channel( DataChannel::Create(this, data_channel_type_, label, &new_config)); - if (channel == NULL) + if (channel && !mediastream_signaling_->AddDataChannel(channel)) return NULL; - if (!mediastream_signaling_->AddDataChannel(channel)) - return NULL; - if (data_channel_type_ == cricket::DCT_SCTP) { - if (config == NULL) { - LOG(LS_WARNING) << "Could not send data channel OPEN message" - << " because of NULL config."; - return NULL; - } - if (data_channel_.get()) { - channel->SetReceiveSsrc(new_config.id); - channel->SetSendSsrc(new_config.id); - } - if (!config->negotiated) { - talk_base::Buffer *payload = new talk_base::Buffer; - if (!cricket::WriteDataChannelOpenMessage(label, *config, payload)) { - LOG(LS_WARNING) << "Could not write data channel OPEN message"; - } - // SendControl may queue the message until the data channel's set up, - // or congestion clears. - channel->SendControl(payload); - } - } + return channel; } @@ -1353,14 +1367,17 @@ bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) { } bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) { - bool rtcp = (data_channel_type_ == cricket::DCT_RTP); + bool sctp = (data_channel_type_ == cricket::DCT_SCTP); data_channel_.reset(channel_manager_->CreateDataChannel( - this, content->name, rtcp, data_channel_type_)); + this, content->name, !sctp, data_channel_type_)); if (!data_channel_.get()) { return false; } - data_channel_->SignalNewStreamReceived.connect( - this, &WebRtcSession::OnNewDataChannelReceived); + if (sctp) { + mediastream_signaling_->OnDataTransportCreatedForSctp(); + data_channel_->SignalNewStreamReceived.connect( + this, &WebRtcSession::OnNewDataChannelReceived); + } return true; } diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index dde33cae2..c4f605591 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -190,7 +190,10 @@ class WebRtcSession : public cricket::BaseSession, cricket::SendDataResult* result) OVERRIDE; virtual bool ConnectDataChannel(DataChannel* webrtc_data_channel) OVERRIDE; virtual void DisconnectDataChannel(DataChannel* webrtc_data_channel) OVERRIDE; - + virtual void AddRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) OVERRIDE; + virtual void AddSctpDataStream(uint32 sid) OVERRIDE; + virtual void RemoveRtpDataStream(uint32 send_ssrc, uint32 recv_ssrc) OVERRIDE; + virtual void RemoveSctpDataStream(uint32 sid) OVERRIDE; talk_base::scoped_refptr CreateDataChannel( const std::string& label, diff --git a/talk/base/base64.cc b/talk/base/base64.cc index 7765f107e..79b045e80 100644 --- a/talk/base/base64.cc +++ b/talk/base/base64.cc @@ -20,7 +20,6 @@ #include "talk/base/common.h" -using std::string; using std::vector; namespace talk_base { @@ -96,7 +95,8 @@ bool Base64::IsBase64Encoded(const std::string& str) { return true; } -void Base64::EncodeFromArray(const void* data, size_t len, string* result) { +void Base64::EncodeFromArray(const void* data, size_t len, + std::string* result) { ASSERT(NULL != result); result->clear(); result->resize(((len + 2) / 3) * 4); @@ -190,8 +190,9 @@ size_t Base64::GetNextQuantum(DecodeFlags parse_flags, bool illegal_pads, } bool Base64::DecodeFromArray(const char* data, size_t len, DecodeFlags flags, - string* result, size_t* data_used) { - return DecodeFromArrayTemplate(data, len, flags, result, data_used); + std::string* result, size_t* data_used) { + return DecodeFromArrayTemplate( + data, len, flags, result, data_used); } bool Base64::DecodeFromArray(const char* data, size_t len, DecodeFlags flags, diff --git a/talk/base/logging.h b/talk/base/logging.h index b563302b2..49e126bab 100644 --- a/talk/base/logging.h +++ b/talk/base/logging.h @@ -312,8 +312,10 @@ class LogMessageVoidify { // The _F version prefixes the message with the current function name. #if (defined(__GNUC__) && defined(_DEBUG)) || defined(WANT_PRETTY_LOG_F) #define LOG_F(sev) LOG(sev) << __PRETTY_FUNCTION__ << ": " +#define LOG_T_F(sev) LOG(sev) << this << ": " << __PRETTY_FUNCTION__ << ": " #else #define LOG_F(sev) LOG(sev) << __FUNCTION__ << ": " +#define LOG_T_F(sev) LOG(sev) << this << ": " << __FUNCTION__ << ": " #endif #define LOG_CHECK_LEVEL(sev) \ @@ -331,7 +333,6 @@ inline bool LogCheckLevel(LoggingSeverity sev) { .stream() #define LOG_T(sev) LOG(sev) << this << ": " -#define LOG_T_F(level) LOG_F(level) << this << ": " #else // !LOGGING @@ -354,7 +355,7 @@ inline bool LogCheckLevel(LoggingSeverity sev) { .stream() #define LOG_T(sev) LOG(sev) << this << ": " -#define LOG_T_F(level) LOG_F(level) << this << " " +#define LOG_T_F(sev) LOG(sev) << this << ": " << __FUNCTION__ << #endif // !LOGGING #define LOG_ERRNO_EX(sev, err) \ diff --git a/talk/base/physicalsocketserver.cc b/talk/base/physicalsocketserver.cc index 891330a54..58a22fa1d 100644 --- a/talk/base/physicalsocketserver.cc +++ b/talk/base/physicalsocketserver.cc @@ -466,6 +466,10 @@ class PhysicalSocket : public AsyncSocket, public sigslot::has_slots<> { ASSERT((0 <= value) && (value <= 65536)); *mtu = value; return 0; +#elif defined(__native_client__) + // Most socket operations, including this, will fail in NaCl's sandbox. + error_ = EACCES; + return -1; #endif } diff --git a/talk/base/profiler.cc b/talk/base/profiler.cc index 68bcfe4d0..4c2aac416 100644 --- a/talk/base/profiler.cc +++ b/talk/base/profiler.cc @@ -71,8 +71,7 @@ void ProfilerEvent::Start() { ++start_count_; } -void ProfilerEvent::Stop() { - uint64 stop_time = TimeNanos(); +void ProfilerEvent::Stop(uint64 stop_time) { --start_count_; ASSERT(start_count_ >= 0); if (start_count_ == 0) { @@ -94,6 +93,10 @@ void ProfilerEvent::Stop() { } } +void ProfilerEvent::Stop() { + Stop(TimeNanos()); +} + double ProfilerEvent::standard_deviation() const { if (event_count_ <= 1) return 0.0; return sqrt(sum_of_squared_differences_ / (event_count_ - 1.0)); @@ -105,11 +108,29 @@ Profiler* Profiler::Instance() { } void Profiler::StartEvent(const std::string& event_name) { - events_[event_name].Start(); + lock_.LockShared(); + EventMap::iterator it = events_.find(event_name); + bool needs_insert = (it == events_.end()); + lock_.UnlockShared(); + + if (needs_insert) { + // Need an exclusive lock to modify the map. + ExclusiveScope scope(&lock_); + it = events_.insert( + EventMap::value_type(event_name, ProfilerEvent())).first; + } + + it->second.Start(); } void Profiler::StopEvent(const std::string& event_name) { - events_[event_name].Stop(); + // Get the time ASAP, then wait for the lock. + uint64 stop_time = TimeNanos(); + SharedScope scope(&lock_); + EventMap::iterator it = events_.find(event_name); + if (it != events_.end()) { + it->second.Stop(stop_time); + } } void Profiler::ReportToLog(const char* file, int line, @@ -118,6 +139,9 @@ void Profiler::ReportToLog(const char* file, int line, if (!LogMessage::Loggable(severity_to_use)) { return; } + + SharedScope scope(&lock_); + { // Output first line. LogMessage msg(file, line, severity_to_use); msg.stream() << "=== Profile report "; @@ -126,8 +150,8 @@ void Profiler::ReportToLog(const char* file, int line, } msg.stream() << "==="; } - typedef std::map::const_iterator iterator; - for (iterator it = events_.begin(); it != events_.end(); ++it) { + for (EventMap::const_iterator it = events_.begin(); + it != events_.end(); ++it) { if (event_prefix.empty() || it->first.find(event_prefix) == 0) { LogMessage(file, line, severity_to_use).stream() << it->first << " " << it->second; @@ -143,15 +167,17 @@ void Profiler::ReportAllToLog(const char* file, int line, } const ProfilerEvent* Profiler::GetEvent(const std::string& event_name) const { - std::map::const_iterator it = + SharedScope scope(&lock_); + EventMap::const_iterator it = events_.find(event_name); return (it == events_.end()) ? NULL : &it->second; } bool Profiler::Clear() { + ExclusiveScope scope(&lock_); bool result = true; // Clear all events that aren't started. - std::map::iterator it = events_.begin(); + EventMap::iterator it = events_.begin(); while (it != events_.end()) { if (it->second.is_started()) { ++it; // Can't clear started events. diff --git a/talk/base/profiler.h b/talk/base/profiler.h index 91ad6a541..90c5c722a 100644 --- a/talk/base/profiler.h +++ b/talk/base/profiler.h @@ -37,7 +37,7 @@ // } // Another example: // void StartAsyncProcess() { -// PROFILE_START("My event"); +// PROFILE_START("My async event"); // DoSomethingAsyncAndThenCall(&Callback); // } // void Callback() { @@ -54,6 +54,7 @@ #include "talk/base/basictypes.h" #include "talk/base/common.h" #include "talk/base/logging.h" +#include "talk/base/sharedexclusivelock.h" // Profiling could be switched via a build flag, but for now, it's always on. #define ENABLE_PROFILING @@ -105,6 +106,7 @@ class ProfilerEvent { ProfilerEvent(); void Start(); void Stop(); + void Stop(uint64 stop_time); double standard_deviation() const; double total_time() const { return total_time_; } double mean() const { return mean_; } @@ -142,7 +144,9 @@ class Profiler { private: Profiler() {} - std::map events_; + typedef std::map EventMap; + EventMap events_; + mutable SharedExclusiveLock lock_; DISALLOW_COPY_AND_ASSIGN(Profiler); }; diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp index bce81a584..80e1e96eb 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -391,6 +391,7 @@ 'app/webrtc/test/fakeaudiocapturemodule.h', 'app/webrtc/test/fakeaudiocapturemodule_unittest.cc', 'app/webrtc/test/fakeconstraints.h', + 'app/webrtc/test/fakedatachannelprovider.h', 'app/webrtc/test/fakedtlsidentityservice.h', 'app/webrtc/test/fakemediastreamsignaling.h', 'app/webrtc/test/fakeperiodicvideocapturer.h', diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 3dc9c5635..49b333624 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -306,6 +306,7 @@ struct VideoOptions { system_high_adaptation_threshhold.SetFrom( change.system_high_adaptation_threshhold); buffered_mode_latency.SetFrom(change.buffered_mode_latency); + lower_min_bitrate.SetFrom(change.lower_min_bitrate); } bool operator==(const VideoOptions& o) const { @@ -329,7 +330,8 @@ struct VideoOptions { o.system_low_adaptation_threshhold && system_high_adaptation_threshhold == o.system_high_adaptation_threshhold && - buffered_mode_latency == o.buffered_mode_latency; + buffered_mode_latency == o.buffered_mode_latency && + lower_min_bitrate == o.lower_min_bitrate; } std::string ToString() const { @@ -356,6 +358,7 @@ struct VideoOptions { ost << ToStringIfSet("low", system_low_adaptation_threshhold); ost << ToStringIfSet("high", system_high_adaptation_threshhold); ost << ToStringIfSet("buffered mode latency", buffered_mode_latency); + ost << ToStringIfSet("lower min bitrate", lower_min_bitrate); ost << "}"; return ost.str(); } @@ -400,6 +403,8 @@ struct VideoOptions { SettablePercent system_high_adaptation_threshhold; // Specify buffered mode latency in milliseconds. Settable buffered_mode_latency; + // Make minimum configured send bitrate even lower than usual, at 30kbit. + Settable lower_min_bitrate; }; // A class for playing out soundclips. diff --git a/talk/media/base/streamparams.cc b/talk/media/base/streamparams.cc index 08eeea7a4..c508b6849 100644 --- a/talk/media/base/streamparams.cc +++ b/talk/media/base/streamparams.cc @@ -33,6 +33,7 @@ namespace cricket { const char kFecSsrcGroupSemantics[] = "FEC"; const char kFidSsrcGroupSemantics[] = "FID"; +const char kSimSsrcGroupSemantics[] = "SIM"; static std::string SsrcsToString(const std::vector& ssrcs) { std::ostringstream ost; diff --git a/talk/media/base/streamparams.h b/talk/media/base/streamparams.h index 1561d6f92..dc25a6ef8 100644 --- a/talk/media/base/streamparams.h +++ b/talk/media/base/streamparams.h @@ -31,6 +31,14 @@ // described by one StreamParams object // SsrcGroup is used to describe the relationship between the SSRCs that // are used for this media source. +// E.x: Consider a source that is sent as 3 simulcast streams +// Let the simulcast elements have SSRC 10, 20, 30. +// Let each simulcast element use FEC and let the protection packets have +// SSRC 11,21,31. +// To describe this 4 SsrcGroups are needed, +// StreamParams would then contain ssrc = {10,11,20,21,30,31} and +// ssrc_groups = {{SIM,{10,20,30}, {FEC,{10,11}, {FEC, {20,21}, {FEC {30,31}}} +// Please see RFC 5576. #ifndef TALK_MEDIA_BASE_STREAMPARAMS_H_ #define TALK_MEDIA_BASE_STREAMPARAMS_H_ @@ -46,6 +54,7 @@ namespace cricket { extern const char kFecSsrcGroupSemantics[]; extern const char kFidSsrcGroupSemantics[]; +extern const char kSimSsrcGroupSemantics[]; struct SsrcGroup { SsrcGroup(const std::string& usage, const std::vector& ssrcs) diff --git a/talk/media/devices/devicemanager.cc b/talk/media/devices/devicemanager.cc index 6f4aa33ff..150b55855 100644 --- a/talk/media/devices/devicemanager.cc +++ b/talk/media/devices/devicemanager.cc @@ -278,18 +278,14 @@ VideoCapturer* DeviceManager::CreateDesktopCapturer( bool DeviceManager::GetAudioDevices(bool input, std::vector* devs) { devs->clear(); -#if defined(IOS) || defined(ANDROID) - // Under Android, we don't access the device file directly. - // Arbitrary use 0 for the mic and 1 for the output. - // These ids are used in MediaEngine::SetSoundDevices(in, out); - // The strings are for human consumption. - if (input) { - devs->push_back(Device("audiorecord", 0)); - } else { - devs->push_back(Device("audiotrack", 1)); - } +#if defined(ANDROID) + // Under Android, 0 is always required for the playout device and 0 is the + // default for the recording device. + devs->push_back(Device("default-device", 0)); return true; #else + // Other platforms either have their own derived class implementation + // (desktop) or don't use device manager for audio devices (iOS). return false; #endif } diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index 0b687287d..b3922ff05 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -1046,14 +1046,12 @@ class FakeWebRtcVideoEngine return 0; } WEBRTC_STUB(EnableColorEnhancement, (const int, const bool)); -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_VOID_STUB(RegisterPreEncodeCallback, (int, webrtc::I420FrameCallback*)); WEBRTC_VOID_STUB(DeRegisterPreEncodeCallback, (int)); WEBRTC_VOID_STUB(RegisterPreRenderCallback, (int, webrtc::I420FrameCallback*)); WEBRTC_VOID_STUB(DeRegisterPreRenderCallback, (int)); -#endif // webrtc::ViEExternalCodec WEBRTC_FUNC(RegisterExternalSendCodec, (const int channel, const unsigned char pl_type, webrtc::VideoEncoder*, diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 05f8b2b3c..f2827db02 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -93,6 +93,9 @@ static const int kStartVideoBitrate = 300; static const int kMaxVideoBitrate = 2000; static const int kDefaultConferenceModeMaxVideoBitrate = 500; +// Controlled by exp, try a super low minimum bitrate for poor connections. +static const int kLowerMinBitrate = 30; + static const int kVideoMtu = 1200; static const int kVideoRtpBufferSize = 65536; @@ -2568,7 +2571,7 @@ bool WebRtcVideoMediaChannel::SetSendBandwidth(bool autobw, int bps) { int max_bitrate; if (autobw) { // Use the default values for min bitrate. - min_bitrate = kMinVideoBitrate; + 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. @@ -2631,6 +2634,17 @@ bool WebRtcVideoMediaChannel::SetOptions(const VideoOptions &options) { // Adjust send codec bitrate if needed. int conf_max_bitrate = kDefaultConferenceModeMaxVideoBitrate; + // Save altered min_bitrate level and apply if necessary. + bool adjusted_min_bitrate = false; + if (options.lower_min_bitrate.IsSet()) { + bool lower; + options.lower_min_bitrate.Get(&lower); + + int new_send_min_bitrate = lower ? kLowerMinBitrate : kMinVideoBitrate; + adjusted_min_bitrate = (new_send_min_bitrate != send_min_bitrate_); + send_min_bitrate_ = new_send_min_bitrate; + } + int expected_bitrate = send_max_bitrate_; if (InConferenceMode()) { expected_bitrate = conf_max_bitrate; @@ -2642,7 +2656,8 @@ bool WebRtcVideoMediaChannel::SetOptions(const VideoOptions &options) { } if (send_codec_ && - (send_max_bitrate_ != expected_bitrate || denoiser_changed)) { + (send_max_bitrate_ != expected_bitrate || denoiser_changed || + adjusted_min_bitrate)) { // On success, SetSendCodec() will reset send_max_bitrate_ to // expected_bitrate. if (!SetSendCodec(*send_codec_, diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 9fbbbe4e3..0b8bdda0e 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -380,6 +380,25 @@ TEST_F(WebRtcVideoEngineTestFake, SetOptionsWithMaxBitrate) { channel_num, kVP8Codec.width, kVP8Codec.height, 0, 20, 10, 20); } +TEST_F(WebRtcVideoEngineTestFake, SetOptionsWithLoweredBitrate) { + EXPECT_TRUE(SetupEngine()); + int channel_num = vie_.GetLastChannel(); + std::vector codecs(engine_.codecs()); + codecs[0].params[cricket::kCodecParamMinBitrate] = "50"; + codecs[0].params[cricket::kCodecParamMaxBitrate] = "100"; + EXPECT_TRUE(channel_->SetSendCodecs(codecs)); + + VerifyVP8SendCodec( + channel_num, kVP8Codec.width, kVP8Codec.height, 0, 100, 50, 100); + + // Verify that min bitrate changes after SetOptions(). + cricket::VideoOptions options; + options.lower_min_bitrate.Set(true); + EXPECT_TRUE(channel_->SetOptions(options)); + VerifyVP8SendCodec( + channel_num, kVP8Codec.width, kVP8Codec.height, 0, 100, 30, 100); +} + TEST_F(WebRtcVideoEngineTestFake, MaxBitrateResetWithConferenceMode) { EXPECT_TRUE(SetupEngine()); int channel_num = vie_.GetLastChannel(); diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 121dd4624..7f0600927 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -885,7 +885,7 @@ struct ResumeEntry { // soundclip device. At that time, reinstate the soundclip pause/resume code. bool WebRtcVoiceEngine::SetDevices(const Device* in_device, const Device* out_device) { -#if !defined(IOS) && !defined(ANDROID) +#if !defined(IOS) int in_id = in_device ? talk_base::FromString(in_device->id) : kDefaultAudioDeviceId; int out_id = out_device ? talk_base::FromString(out_device->id) : @@ -982,13 +982,13 @@ bool WebRtcVoiceEngine::SetDevices(const Device* in_device, return ret; #else return true; -#endif // !IOS && !ANDROID +#endif // !IOS } bool WebRtcVoiceEngine::FindWebRtcAudioDeviceId( bool is_input, const std::string& dev_name, int dev_id, int* rtc_id) { // In Linux, VoiceEngine uses the same device dev_id as the device manager. -#ifdef LINUX +#if defined(LINUX) || defined(ANDROID) *rtc_id = dev_id; return true; #else diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index ae3fb577b..ba510b941 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -242,13 +242,12 @@ static bool GenerateCname(const StreamParamsVec& params_vec, } // Generate random SSRC values that are not already present in |params_vec|. -// Either 2 or 1 ssrcs will be generated based on |include_rtx_stream| being -// true or false. The generated values are added to |ssrcs|. +// The generated values are added to |ssrcs|. +// |num_ssrcs| is the number of the SSRC will be generated. static void GenerateSsrcs(const StreamParamsVec& params_vec, - bool include_rtx_stream, + int num_ssrcs, std::vector* ssrcs) { - unsigned int num_ssrcs = include_rtx_stream ? 2 : 1; - for (unsigned int i = 0; i < num_ssrcs; i++) { + for (int i = 0; i < num_ssrcs; i++) { uint32 candidate; do { candidate = talk_base::CreateRandomNonZeroId(); @@ -428,7 +427,8 @@ static bool AddStreamParams( if (IsSctp(content_description)) { GenerateSctpSids(*current_streams, &ssrcs); } else { - GenerateSsrcs(*current_streams, include_rtx_stream, &ssrcs); + int num_ssrcs = include_rtx_stream ? 2 : 1; + GenerateSsrcs(*current_streams, num_ssrcs, &ssrcs); } if (include_rtx_stream) { content_description->AddLegacyStream(ssrcs[0], ssrcs[1]); @@ -462,13 +462,23 @@ static bool AddStreamParams( if (IsSctp(content_description)) { GenerateSctpSids(*current_streams, &ssrcs); } else { - GenerateSsrcs(*current_streams, include_rtx_stream, &ssrcs); + GenerateSsrcs(*current_streams, stream_it->num_sim_layers, &ssrcs); } StreamParams stream_param; stream_param.id = stream_it->id; - stream_param.ssrcs.push_back(ssrcs[0]); + // Add the generated ssrc. + for (size_t i = 0; i < ssrcs.size(); ++i) { + stream_param.ssrcs.push_back(ssrcs[i]); + } + if (stream_it->num_sim_layers > 1) { + SsrcGroup group(kSimSsrcGroupSemantics, stream_param.ssrcs); + stream_param.ssrc_groups.push_back(group); + } + // Generate an extra ssrc for include_rtx_stream case. if (include_rtx_stream) { - stream_param.AddFidSsrc(ssrcs[0], ssrcs[1]); + std::vector rtx_ssrc; + GenerateSsrcs(*current_streams, 1, &rtx_ssrc); + stream_param.AddFidSsrc(ssrcs[0], rtx_ssrc[0]); content_description->set_multistream(true); } stream_param.cname = cname; @@ -1017,7 +1027,22 @@ static bool IsDtlsActive( void MediaSessionOptions::AddStream(MediaType type, const std::string& id, const std::string& sync_label) { - streams.push_back(Stream(type, id, sync_label)); + AddStreamInternal(type, id, sync_label, 1); +} + +void MediaSessionOptions::AddVideoStream( + const std::string& id, + const std::string& sync_label, + int num_sim_layers) { + AddStreamInternal(MEDIA_TYPE_VIDEO, id, sync_label, num_sim_layers); +} + +void MediaSessionOptions::AddStreamInternal( + MediaType type, + const std::string& id, + const std::string& sync_label, + int num_sim_layers) { + streams.push_back(Stream(type, id, sync_label, num_sim_layers)); if (type == MEDIA_TYPE_VIDEO) has_video = true; diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h index 5dfc765e7..ff25f5a04 100644 --- a/talk/session/media/mediasession.h +++ b/talk/session/media/mediasession.h @@ -105,8 +105,18 @@ struct MediaSessionOptions { void AddStream(MediaType type, const std::string& id, const std::string& sync_label); + void AddVideoStream(const std::string& id, + const std::string& sync_label, + int num_sim_layers); void RemoveStream(MediaType type, const std::string& id); + + // Helper function. + void AddStreamInternal(MediaType type, + const std::string& id, + const std::string& sync_label, + int num_sim_layers); + bool has_audio; bool has_video; DataChannelType data_channel_type; @@ -122,12 +132,15 @@ struct MediaSessionOptions { struct Stream { Stream(MediaType type, const std::string& id, - const std::string& sync_label) - : type(type), id(id), sync_label(sync_label) { + const std::string& sync_label, + int num_sim_layers) + : type(type), id(id), sync_label(sync_label), + num_sim_layers(num_sim_layers) { } MediaType type; std::string id; std::string sync_label; + int num_sim_layers; }; typedef std::vector Streams; diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc index ceb2bcd09..dbd8db94b 100644 --- a/talk/session/media/mediasession_unittest.cc +++ b/talk/session/media/mediasession_unittest.cc @@ -1162,6 +1162,28 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { EXPECT_EQ(updated_data_streams[0].cname, updated_data_streams[1].cname); } +// Create an offer with simulcast video stream. +TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSimulcastVideoOffer) { + MediaSessionOptions opts; + const int num_sim_layers = 3; + opts.AddVideoStream(kVideoTrack1, kMediaStream1, num_sim_layers); + talk_base::scoped_ptr offer(f1_.CreateOffer(opts, NULL)); + + ASSERT_TRUE(offer.get() != NULL); + const ContentInfo* vc = offer->GetContentByName("video"); + ASSERT_TRUE(vc != NULL); + const VideoContentDescription* vcd = + static_cast(vc->description); + + const StreamParamsVec& video_streams = vcd->streams(); + ASSERT_EQ(1U, video_streams.size()); + EXPECT_EQ(kVideoTrack1, video_streams[0].id); + const SsrcGroup* sim_ssrc_group = + video_streams[0].get_ssrc_group(cricket::kSimSsrcGroupSemantics); + ASSERT_TRUE(sim_ssrc_group != NULL); + EXPECT_EQ(static_cast(num_sim_layers), sim_ssrc_group->ssrcs.size()); +} + // Create an audio and video answer to a standard video offer with: // - one video track // - two audio tracks