From b5cb19b37c361a263a9cec2e2fb356d16520afd1 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Mon, 23 Nov 2015 16:39:12 -0800 Subject: [PATCH] Fixing direction attribute in answer for non-RTP protocols. "non-RTP protocols" refers to SCTP data channels. Because there are no streams for SCTP data channels, the answer was being set to RECVONLY. BUG=webrtc:5228 Review URL: https://codereview.webrtc.org/1473013002 Cr-Commit-Position: refs/heads/master@{#10762} --- talk/app/webrtc/peerconnection_unittest.cc | 34 ++++++++++++++++++++-- talk/session/media/mediasession.cc | 21 ++++++++----- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index 6307bfc6d8..6dc83a5751 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -1450,8 +1450,9 @@ TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, GetDtls12Recv) { kDefaultSrtpCryptoSuite)); } -// This test sets up a call between two parties with audio, video and data. -TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDataChannel) { +// This test sets up a call between two parties with audio, video and an RTP +// data channel. +TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestRtpDataChannel) { FakeConstraints setup_constraints; setup_constraints.SetAllowRtpDataChannels(); ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints)); @@ -1481,6 +1482,35 @@ TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDataChannel) { EXPECT_FALSE(receiving_client()->data_observer()->IsOpen()); } +// This test sets up a call between two parties with audio, video and an SCTP +// data channel. +TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestSctpDataChannel) { + ASSERT_TRUE(CreateTestClients()); + initializing_client()->CreateDataChannel(); + LocalP2PTest(); + ASSERT_TRUE(initializing_client()->data_channel() != nullptr); + EXPECT_TRUE_WAIT(receiving_client()->data_channel() != nullptr, kMaxWaitMs); + EXPECT_TRUE_WAIT(initializing_client()->data_observer()->IsOpen(), + kMaxWaitMs); + EXPECT_TRUE_WAIT(receiving_client()->data_observer()->IsOpen(), kMaxWaitMs); + + std::string data = "hello world"; + + initializing_client()->data_channel()->Send(DataBuffer(data)); + EXPECT_EQ_WAIT(data, receiving_client()->data_observer()->last_message(), + kMaxWaitMs); + + receiving_client()->data_channel()->Send(DataBuffer(data)); + EXPECT_EQ_WAIT(data, initializing_client()->data_observer()->last_message(), + kMaxWaitMs); + + receiving_client()->data_channel()->Close(); + // Send new offer and answer. + receiving_client()->Negotiate(); + EXPECT_FALSE(initializing_client()->data_observer()->IsOpen()); + EXPECT_FALSE(receiving_client()->data_observer()->IsOpen()); +} + // This test sets up a call between two parties and creates a data channel. // The test tests that received data is buffered unless an observer has been // registered. diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index ed626e2f34..0e9730ce2b 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -633,6 +633,11 @@ static void PruneCryptos(const CryptoParamsVec& filter, target_cryptos->end()); } +static bool IsRtpProtocol(const std::string& protocol) { + return protocol.empty() || + (protocol.find(cricket::kMediaProtocolRtpPrefix) != std::string::npos); +} + static bool IsRtpContent(SessionDescription* sdesc, const std::string& content_name) { bool is_rtp = false; @@ -643,9 +648,7 @@ static bool IsRtpContent(SessionDescription* sdesc, if (!media_desc) { return false; } - is_rtp = media_desc->protocol().empty() || - (media_desc->protocol().find(cricket::kMediaProtocolRtpPrefix) != - std::string::npos); + is_rtp = IsRtpProtocol(media_desc->protocol()); } return is_rtp; } @@ -1067,12 +1070,16 @@ static bool CreateMediaContentAnswer( answer->set_direction(MD_RECVONLY); break; case MD_RECVONLY: - answer->set_direction(answer->streams().empty() ? MD_INACTIVE - : MD_SENDONLY); + answer->set_direction(IsRtpProtocol(answer->protocol()) && + answer->streams().empty() + ? MD_INACTIVE + : MD_SENDONLY); break; case MD_SENDRECV: - answer->set_direction(answer->streams().empty() ? MD_RECVONLY - : MD_SENDRECV); + answer->set_direction(IsRtpProtocol(answer->protocol()) && + answer->streams().empty() + ? MD_RECVONLY + : MD_SENDRECV); break; default: RTC_DCHECK(false && "MediaContentDescription has unexpected direction.");