admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Stop video candidates getting down to audio.

Browse Source 
Second attempt at adding a check to make sure that the video
transportproxy doesn't send down candidates to the audio transport
channel when things are bundled.

BUG=4665
R=juberti@google.com, pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/50059004

Cr-Commit-Position: refs/heads/master@{#9316}
This commit is contained in:
Donald Curtis 2015-05-28 09:48:21 -07:00
parent a743794d06
commit d4f769d8fc
2 changed files with 92 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
talk/app/webrtc/webrtcsession_unittest.cc
View File

@ -2602,6 +2602,89 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) {
EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error));
}
// Test that candidates sent to the "video" transport do not get pushed down to
// the "audio" transport channel when bundling using TransportProxy.
TEST_F(WebRtcSessionTest, TestIgnoreCandidatesForUnusedTransportWhenBundling) {
AddInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort));
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
mediastream_signaling_.SendAudioVideoStream1();
PeerConnectionInterface::RTCOfferAnswerOptions options;
options.use_rtp_mux = true;
SessionDescriptionInterface* offer = CreateRemoteOffer();
SetRemoteDescriptionWithoutError(offer);
SessionDescriptionInterface* answer = CreateAnswer(NULL);
SetLocalDescriptionWithoutError(answer);
EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
session_->GetTransportProxy("video")->impl());
cricket::Transport* t = session_->GetTransport("audio");
// Checks if one of the transport channels contains a connection using a given
// port.
auto connection_with_remote_port = [t](int port) {
cricket::TransportStats stats;
t->GetStats(&stats);
for (auto& chan_stat : stats.channel_stats) {
for (auto& conn_info : chan_stat.connection_infos) {
if (conn_info.remote_candidate.address().port() == port) {
return true;
}
}
}
return false;
};
EXPECT_FALSE(connection_with_remote_port(5000));
EXPECT_FALSE(connection_with_remote_port(5001));
EXPECT_FALSE(connection_with_remote_port(6000));
// The way the *_WAIT checks work is they only wait if the condition fails,
// which does not help in the case where state is not changing. This is
// problematic in this test since we want to verify that adding a video
// candidate does _not_ change state. So we interleave candidates and assume
// that messages are executed in the order they were posted.
// First audio candidate.
cricket::Candidate candidate0;
candidate0.set_address(rtc::SocketAddress("1.1.1.1", 5000));
candidate0.set_component(1);
candidate0.set_protocol("udp");
JsepIceCandidate ice_candidate0(kMediaContentName0, kMediaContentIndex0,
candidate0);
EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate0));
// Video candidate.
cricket::Candidate candidate1;
candidate1.set_address(rtc::SocketAddress("1.1.1.1", 6000));
candidate1.set_component(1);
candidate1.set_protocol("udp");
JsepIceCandidate ice_candidate1(kMediaContentName1, kMediaContentIndex1,
candidate1);
EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate1));
// Second audio candidate.
cricket::Candidate candidate2;
candidate2.set_address(rtc::SocketAddress("1.1.1.1", 5001));
candidate2.set_component(1);
candidate2.set_protocol("udp");
JsepIceCandidate ice_candidate2(kMediaContentName0, kMediaContentIndex0,
candidate2);
EXPECT_TRUE(session_->ProcessIceMessage(&ice_candidate2));
EXPECT_TRUE_WAIT(connection_with_remote_port(5000), 1000);
EXPECT_TRUE_WAIT(connection_with_remote_port(5001), 1000);
// No need here for a _WAIT check since we are checking that state hasn't
// changed: if this is false we would be doing waits for nothing and if this
// is true then there will be no messages processed anyways.
EXPECT_FALSE(connection_with_remote_port(6000));
}
// kBundlePolicyBalanced bundle policy and answer contains BUNDLE.
TEST_F(WebRtcSessionTest, TestBalancedBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
@ -3459,7 +3542,7 @@ TEST_F(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) {
ASSERT_TRUE(ch != NULL);
ASSERT_EQ(1UL, ch->send_codecs().size());
EXPECT_EQ(cricket::kGoogleSctpDataCodecId, ch->send_codecs()[0].id);
EXPECT_TRUE(!strcmp(cricket::kGoogleSctpDataCodecName,
EXPECT_EQ(0, strcmp(cricket::kGoogleSctpDataCodecName,
ch->send_codecs()[0].name.c_str()));
EXPECT_TRUE(ch->send_codecs()[0].GetParam(cricket::kCodecParamPort,
&portnum));
@ -3467,7 +3550,7 @@ TEST_F(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) {
ASSERT_EQ(1UL, ch->recv_codecs().size());
EXPECT_EQ(cricket::kGoogleSctpDataCodecId, ch->recv_codecs()[0].id);
EXPECT_TRUE(!strcmp(cricket::kGoogleSctpDataCodecName,
EXPECT_EQ(0, strcmp(cricket::kGoogleSctpDataCodecName,
ch->recv_codecs()[0].name.c_str()));
EXPECT_TRUE(ch->recv_codecs()[0].GetParam(cricket::kCodecParamPort,
&portnum));

7
webrtc/p2p/base/session.cc
View File

@ -259,6 +259,13 @@ bool TransportProxy::OnRemoteCandidates(const Candidates& candidates,
// TODO(juberti): Remove this once everybody calls SetLocalTD.
CompleteNegotiation();
// Ignore candidates for if the proxy content_name doesn't match the content
// name of the actual transport. This stops video candidates from being sent
// down to the audio transport when BUNDLE is enabled.
if (content_name_ != transport_->get()->content_name()) {
return true;
}
// Verify each candidate before passing down to transport layer.
for (Candidates::const_iterator cand = candidates.begin();
cand != candidates.end(); ++cand) {