(Auto)update libjingle 63948945-> 64147530
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5825 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h
index 4096ff1..1b6031a 100644
--- a/talk/media/webrtc/fakewebrtcvideoengine.h
+++ b/talk/media/webrtc/fakewebrtcvideoengine.h
@@ -433,7 +433,7 @@
void set_fail_alloc_capturer(bool fail_alloc_capturer) {
fail_alloc_capturer_ = fail_alloc_capturer;
}
- int num_set_send_codecs() const { return num_set_send_codecs_; }
+ int GetNumSetSendCodecs() const { return num_set_send_codecs_; }
int GetCaptureId(int channel) const {
WEBRTC_ASSERT_CHANNEL(channel);
diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h
index bac5331..b552b49 100644
--- a/talk/media/webrtc/fakewebrtcvoiceengine.h
+++ b/talk/media/webrtc/fakewebrtcvoiceengine.h
@@ -151,6 +151,7 @@
fail_create_channel_(false),
codecs_(codecs),
num_codecs_(num_codecs),
+ num_set_send_codecs_(0),
ec_enabled_(false),
ec_metrics_enabled_(false),
cng_enabled_(false),
@@ -297,6 +298,8 @@
return channels_[channel]->receive_absolute_sender_time_ext_;
}
+ int GetNumSetSendCodecs() const { return num_set_send_codecs_; }
+
WEBRTC_STUB(Release, ());
// webrtc::VoEBase
@@ -401,6 +404,7 @@
return -1;
}
channels_[channel]->send_codec = codec;
+ ++num_set_send_codecs_;
return 0;
}
WEBRTC_FUNC(GetSendCodec, (int channel, webrtc::CodecInst& codec)) {
@@ -1082,6 +1086,7 @@
bool fail_create_channel_;
const cricket::AudioCodec* const* codecs_;
int num_codecs_;
+ int num_set_send_codecs_; // how many times we call SetSendCodec().
bool ec_enabled_;
bool ec_metrics_enabled_;
bool cng_enabled_;
diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc
index 53c0383..98e65ca 100644
--- a/talk/media/webrtc/webrtcvideoengine.cc
+++ b/talk/media/webrtc/webrtcvideoengine.cc
@@ -164,6 +164,73 @@
kParamValueEmpty));
}
+// TODO(mallinath) - Remove this after trunk of webrtc is pushed to GTP.
+#if !defined(USE_WEBRTC_DEV_BRANCH)
+bool operator==(const webrtc::VideoCodecVP8& lhs,
+ const webrtc::VideoCodecVP8& rhs) {
+ return lhs.pictureLossIndicationOn == rhs.pictureLossIndicationOn &&
+ lhs.feedbackModeOn == rhs.feedbackModeOn &&
+ lhs.complexity == rhs.complexity &&
+ lhs.resilience == rhs.resilience &&
+ lhs.numberOfTemporalLayers == rhs.numberOfTemporalLayers &&
+ lhs.denoisingOn == rhs.denoisingOn &&
+ lhs.errorConcealmentOn == rhs.errorConcealmentOn &&
+ lhs.automaticResizeOn == rhs.automaticResizeOn &&
+ lhs.frameDroppingOn == rhs.frameDroppingOn &&
+ lhs.keyFrameInterval == rhs.keyFrameInterval;
+}
+
+bool operator!=(const webrtc::VideoCodecVP8& lhs,
+ const webrtc::VideoCodecVP8& rhs) {
+ return !(lhs == rhs);
+}
+
+bool operator==(const webrtc::SimulcastStream& lhs,
+ const webrtc::SimulcastStream& rhs) {
+ return lhs.width == rhs.width &&
+ lhs.height == rhs.height &&
+ lhs.numberOfTemporalLayers == rhs.numberOfTemporalLayers &&
+ lhs.maxBitrate == rhs.maxBitrate &&
+ lhs.targetBitrate == rhs.targetBitrate &&
+ lhs.minBitrate == rhs.minBitrate &&
+ lhs.qpMax == rhs.qpMax;
+}
+
+bool operator!=(const webrtc::SimulcastStream& lhs,
+ const webrtc::SimulcastStream& rhs) {
+ return !(lhs == rhs);
+}
+
+bool operator==(const webrtc::VideoCodec& lhs,
+ const webrtc::VideoCodec& rhs) {
+ bool ret = lhs.codecType == rhs.codecType &&
+ (_stricmp(lhs.plName, rhs.plName) == 0) &&
+ lhs.plType == rhs.plType &&
+ lhs.width == rhs.width &&
+ lhs.height == rhs.height &&
+ lhs.startBitrate == rhs.startBitrate &&
+ lhs.maxBitrate == rhs.maxBitrate &&
+ lhs.minBitrate == rhs.minBitrate &&
+ lhs.maxFramerate == rhs.maxFramerate &&
+ lhs.qpMax == rhs.qpMax &&
+ lhs.numberOfSimulcastStreams == rhs.numberOfSimulcastStreams &&
+ lhs.mode == rhs.mode;
+ if (ret && lhs.codecType == webrtc::kVideoCodecVP8) {
+ ret &= (lhs.codecSpecific.VP8 == rhs.codecSpecific.VP8);
+ }
+
+ for (unsigned char i = 0; i < rhs.numberOfSimulcastStreams && ret; ++i) {
+ ret &= (lhs.simulcastStream[i] == rhs.simulcastStream[i]);
+ }
+ return ret;
+}
+
+bool operator!=(const webrtc::VideoCodec& lhs,
+ const webrtc::VideoCodec& rhs) {
+ return !(lhs == rhs);
+}
+#endif
+
struct FlushBlackFrameData : public talk_base::MessageData {
FlushBlackFrameData(uint32 s, int64 t) : ssrc(s), timestamp(t) {
}
@@ -1647,6 +1714,8 @@
ConvertToCricketVideoCodec(*send_codec_, ¤t);
}
std::map<int, int> primary_rtx_pt_mapping;
+ bool nack_enabled = nack_enabled_;
+ bool remb_enabled = remb_enabled_;
for (std::vector<VideoCodec>::const_iterator iter = codecs.begin();
iter != codecs.end(); ++iter) {
if (_stricmp(iter->name.c_str(), kRedPayloadName) == 0) {
@@ -1663,8 +1732,8 @@
webrtc::VideoCodec wcodec;
if (engine()->ConvertFromCricketVideoCodec(checked_codec, &wcodec)) {
if (send_codecs.empty()) {
- nack_enabled_ = IsNackEnabled(checked_codec);
- remb_enabled_ = IsRembEnabled(checked_codec);
+ nack_enabled = IsNackEnabled(checked_codec);
+ remb_enabled = IsRembEnabled(checked_codec);
}
send_codecs.push_back(wcodec);
}
@@ -1680,35 +1749,43 @@
}
// Recv protection.
- for (RecvChannelMap::iterator it = recv_channels_.begin();
- it != recv_channels_.end(); ++it) {
- int channel_id = it->second->channel_id();
- if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
- nack_enabled_)) {
- return false;
+ // Do not update if the status is same as previously configured.
+ if (nack_enabled_ != nack_enabled) {
+ for (RecvChannelMap::iterator it = recv_channels_.begin();
+ it != recv_channels_.end(); ++it) {
+ int channel_id = it->second->channel_id();
+ if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
+ nack_enabled)) {
+ return false;
+ }
+ if (engine_->vie()->rtp()->SetRembStatus(channel_id,
+ kNotSending,
+ remb_enabled_) != 0) {
+ LOG_RTCERR3(SetRembStatus, channel_id, kNotSending, remb_enabled_);
+ return false;
+ }
}
- if (engine_->vie()->rtp()->SetRembStatus(channel_id,
- kNotSending,
- remb_enabled_) != 0) {
- LOG_RTCERR3(SetRembStatus, channel_id, kNotSending, remb_enabled_);
- return false;
- }
+ nack_enabled_ = nack_enabled;
}
// Send settings.
- for (SendChannelMap::iterator iter = send_channels_.begin();
- iter != send_channels_.end(); ++iter) {
- int channel_id = iter->second->channel_id();
- if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
- nack_enabled_)) {
- return false;
+ // Do not update if the status is same as previously configured.
+ if (remb_enabled_ != remb_enabled) {
+ for (SendChannelMap::iterator iter = send_channels_.begin();
+ iter != send_channels_.end(); ++iter) {
+ int channel_id = iter->second->channel_id();
+ if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
+ nack_enabled_)) {
+ return false;
+ }
+ if (engine_->vie()->rtp()->SetRembStatus(channel_id,
+ remb_enabled,
+ remb_enabled) != 0) {
+ LOG_RTCERR3(SetRembStatus, channel_id, remb_enabled, remb_enabled);
+ return false;
+ }
}
- if (engine_->vie()->rtp()->SetRembStatus(channel_id,
- remb_enabled_,
- remb_enabled_) != 0) {
- LOG_RTCERR3(SetRembStatus, channel_id, remb_enabled_, remb_enabled_);
- return false;
- }
+ remb_enabled_ = remb_enabled;
}
// Select the first matched codec.
@@ -3646,6 +3723,15 @@
<< "for ssrc: " << ssrc << ".";
} else {
MaybeChangeStartBitrate(channel_id, &target_codec);
+ webrtc::VideoCodec current_codec;
+ if (!engine()->vie()->codec()->GetSendCodec(channel_id, current_codec)) {
+ // Compare against existing configured send codec.
+ if (current_codec == target_codec) {
+ // Codec is already configured on channel. no need to apply.
+ return true;
+ }
+ }
+
if (0 != engine()->vie()->codec()->SetSendCodec(channel_id, target_codec)) {
LOG_RTCERR2(SetSendCodec, channel_id, target_codec.plName);
return false;
diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc
index a61b8d8..32e742c 100644
--- a/talk/media/webrtc/webrtcvideoengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc
@@ -313,9 +313,28 @@
VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height);
EXPECT_TRUE(vie_.GetHybridNackFecStatus(channel_num));
EXPECT_FALSE(vie_.GetNackStatus(channel_num));
+ EXPECT_EQ(1, vie_.GetNumSetSendCodecs());
// TODO(juberti): Check RTCP, PLI, TMMBR.
}
+// Test that ViE Channel doesn't call SetSendCodec again if same codec is tried
+// to apply.
+TEST_F(WebRtcVideoEngineTestFake, DontResetSetSendCodec) {
+ EXPECT_TRUE(SetupEngine());
+ int channel_num = vie_.GetLastChannel();
+ std::vector<cricket::VideoCodec> codecs(engine_.codecs());
+ EXPECT_TRUE(channel_->SetSendCodecs(codecs));
+ VerifyVP8SendCodec(channel_num, kVP8Codec.width, kVP8Codec.height);
+ EXPECT_TRUE(vie_.GetHybridNackFecStatus(channel_num));
+ EXPECT_FALSE(vie_.GetNackStatus(channel_num));
+ EXPECT_EQ(1, vie_.GetNumSetSendCodecs());
+ // Try setting same code again.
+ EXPECT_TRUE(channel_->SetSendCodecs(codecs));
+ // Since it's exact same codec which is already set, media channel shouldn't
+ // send the codec to ViE.
+ EXPECT_EQ(1, vie_.GetNumSetSendCodecs());
+}
+
TEST_F(WebRtcVideoEngineTestFake, SetSendCodecsWithMinMaxBitrate) {
EXPECT_TRUE(SetupEngine());
int channel_num = vie_.GetLastChannel();
@@ -1654,7 +1673,7 @@
cricket::StreamParams::CreateLegacy(123)));
EXPECT_TRUE(channel_->SetSendCodecs(codec_list));
EXPECT_TRUE(channel_->SetSend(true));
- EXPECT_EQ(1, vie_.num_set_send_codecs());
+ EXPECT_EQ(1, vie_.GetNumSetSendCodecs());
webrtc::VideoCodec gcodec;
memset(&gcodec, 0, sizeof(gcodec));
@@ -1665,7 +1684,7 @@
// Send a screencast frame with the same size.
// Verify that denoising is turned off.
SendI420ScreencastFrame(kVP8Codec.width, kVP8Codec.height);
- EXPECT_EQ(2, vie_.num_set_send_codecs());
+ EXPECT_EQ(2, vie_.GetNumSetSendCodecs());
EXPECT_EQ(0, vie_.GetSendCodec(channel_num, gcodec));
EXPECT_FALSE(gcodec.codecSpecific.VP8.denoisingOn);
}
diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc
index b308e4e..bf4cef6 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -220,6 +220,22 @@
kParamValueEmpty));
}
+// TODO(mallinath) - Remove this after trunk of webrtc is pushed to GTP.
+#if !defined(USE_WEBRTC_DEV_BRANCH)
+bool operator==(const webrtc::CodecInst& lhs, const webrtc::CodecInst& rhs) {
+ return lhs.pltype == rhs.pltype &&
+ (_stricmp(lhs.plname, rhs.plname) == 0) &&
+ lhs.plfreq == rhs.plfreq &&
+ lhs.pacsize == rhs.pacsize &&
+ lhs.channels == rhs.channels &&
+ lhs.rate == rhs.rate;
+}
+
+bool operator!=(const webrtc::CodecInst& lhs, const webrtc::CodecInst& rhs) {
+ return !(lhs == rhs);
+}
+#endif
+
// Gets the default set of options 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.
@@ -1981,6 +1997,8 @@
webrtc::CodecInst send_codec;
memset(&send_codec, 0, sizeof(send_codec));
+ bool nack_enabled = nack_enabled_;
+
// Set send codec (the first non-telephone-event/CN codec)
for (std::vector<AudioCodec>::const_iterator it = codecs.begin();
it != codecs.end(); ++it) {
@@ -2052,13 +2070,17 @@
}
} else {
send_codec = voe_codec;
- nack_enabled_ = IsNackEnabled(*it);
- SetNack(channel, nack_enabled_);
+ nack_enabled = IsNackEnabled(*it);
}
found_send_codec = true;
break;
}
+ if (nack_enabled_ != nack_enabled) {
+ SetNack(channel, nack_enabled);
+ nack_enabled_ = nack_enabled;
+ }
+
if (!found_send_codec) {
LOG(LS_WARNING) << "Received empty list of codecs.";
return false;
@@ -2142,7 +2164,6 @@
}
}
}
-
return true;
}
@@ -2167,8 +2188,8 @@
}
}
+ // Set nack status on receive channels and update |nack_enabled_|.
SetNack(receive_channels_, nack_enabled_);
-
return true;
}
@@ -2208,6 +2229,13 @@
LOG(LS_INFO) << "Send channel " << channel << " selected voice codec "
<< ToString(send_codec) << ", bitrate=" << send_codec.rate;
+ webrtc::CodecInst current_codec;
+ if (engine()->voe()->codec()->GetSendCodec(channel, current_codec) == 0 &&
+ (send_codec == current_codec)) {
+ // Codec is already configured, we can return without setting it again.
+ return true;
+ }
+
if (engine()->voe()->codec()->SetSendCodec(channel, send_codec) == -1) {
LOG_RTCERR2(SetSendCodec, channel, ToString(send_codec));
return false;
diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
index eba25be..4dbeeba 100644
--- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
@@ -743,6 +743,7 @@
codecs[0].id = 96;
codecs[0].bitrate = 48000;
EXPECT_TRUE(channel_->SetSendCodecs(codecs));
+ EXPECT_EQ(1, voe_.GetNumSetSendCodecs());
webrtc::CodecInst gcodec;
EXPECT_EQ(0, voe_.GetSendCodec(channel_num, gcodec));
EXPECT_EQ(96, gcodec.pltype);
@@ -755,6 +756,24 @@
EXPECT_EQ(106, voe_.GetSendTelephoneEventPayloadType(channel_num));
}
+// Test that VoE Channel doesn't call SetSendCodec again if same codec is tried
+// to apply.
+TEST_F(WebRtcVoiceEngineTestFake, DontResetSetSendCodec) {
+ EXPECT_TRUE(SetupEngine());
+ std::vector<cricket::AudioCodec> codecs;
+ codecs.push_back(kIsacCodec);
+ codecs.push_back(kPcmuCodec);
+ codecs.push_back(kRedCodec);
+ codecs[0].id = 96;
+ codecs[0].bitrate = 48000;
+ EXPECT_TRUE(channel_->SetSendCodecs(codecs));
+ EXPECT_EQ(1, voe_.GetNumSetSendCodecs());
+ // Calling SetSendCodec again with same codec which is already set.
+ // In this case media channel shouldn't send codec to VoE.
+ EXPECT_TRUE(channel_->SetSendCodecs(codecs));
+ EXPECT_EQ(1, voe_.GetNumSetSendCodecs());
+}
+
// Test that if clockrate is not 48000 for opus, we fail.
TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusBadClockrate) {
EXPECT_TRUE(SetupEngine());
diff --git a/talk/p2p/base/p2ptransportchannel.cc b/talk/p2p/base/p2ptransportchannel.cc
index 99ca858..e7f9d45 100644
--- a/talk/p2p/base/p2ptransportchannel.cc
+++ b/talk/p2p/base/p2ptransportchannel.cc
@@ -629,7 +629,7 @@
// Creates connections from all of the ports that we care about to the given
// remote candidate. The return value is true if we created a connection from
// the origin port.
-bool P2PTransportChannel::CreateConnections(const Candidate &remote_candidate,
+bool P2PTransportChannel::CreateConnections(const Candidate& remote_candidate,
PortInterface* origin_port,
bool readable) {
ASSERT(worker_thread_ == talk_base::Thread::Current());
@@ -648,14 +648,25 @@
new_remote_candidate.set_password(remote_ice_pwd_);
}
+ // If we've already seen the new remote candidate (in the current candidate
+ // generation), then we shouldn't try creating connections for it.
+ // We either already have a connection for it, or we previously created one
+ // and then later pruned it. If we don't return, the channel will again
+ // re-create any connections that were previously pruned, which will then
+ // immediately be re-pruned, churning the network for no purpose.
+ // This only applies to candidates received over signaling (i.e. origin_port
+ // is NULL).
+ if (!origin_port && IsDuplicateRemoteCandidate(new_remote_candidate)) {
+ // return true to indicate success, without creating any new connections.
+ return true;
+ }
+
// Add a new connection for this candidate to every port that allows such a
// connection (i.e., if they have compatible protocols) and that does not
// already have a connection to an equivalent candidate. We must be careful
// to make sure that the origin port is included, even if it was pruned,
// since that may be the only port that can create this connection.
-
bool created = false;
-
std::vector<PortInterface *>::reverse_iterator it;
for (it = ports_.rbegin(); it != ports_.rend(); ++it) {
if (CreateConnection(*it, new_remote_candidate, origin_port, readable)) {
@@ -690,7 +701,11 @@
// It is not legal to try to change any of the parameters of an existing
// connection; however, the other side can send a duplicate candidate.
if (!remote_candidate.IsEquivalent(connection->remote_candidate())) {
- LOG(INFO) << "Attempt to change a remote candidate";
+ LOG(INFO) << "Attempt to change a remote candidate."
+ << " Existing remote candidate: "
+ << connection->remote_candidate().ToString()
+ << "New remote candidate: "
+ << remote_candidate.ToString();
return false;
}
} else {
@@ -740,6 +755,17 @@
return remote_candidate_generation_;
}
+// Check if remote candidate is already cached.
+bool P2PTransportChannel::IsDuplicateRemoteCandidate(
+ const Candidate& candidate) {
+ for (uint32 i = 0; i < remote_candidates_.size(); ++i) {
+ if (remote_candidates_[i].IsEquivalent(candidate)) {
+ return true;
+ }
+ }
+ return false;
+}
+
// Maintain our remote candidate list, adding this new remote one.
void P2PTransportChannel::RememberRemoteCandidate(
const Candidate& remote_candidate, PortInterface* origin_port) {
@@ -757,12 +783,9 @@
}
// Make sure this candidate is not a duplicate.
- for (uint32 i = 0; i < remote_candidates_.size(); ++i) {
- if (remote_candidates_[i].IsEquivalent(remote_candidate)) {
- LOG(INFO) << "Duplicate candidate: "
- << remote_candidate.address().ToSensitiveString();
- return;
- }
+ if (IsDuplicateRemoteCandidate(remote_candidate)) {
+ LOG(INFO) << "Duplicate candidate: " << remote_candidate.ToString();
+ return;
}
// Try this candidate for all future ports.
diff --git a/talk/p2p/base/p2ptransportchannel.h b/talk/p2p/base/p2ptransportchannel.h
index 57fc3a6..09dabd5 100644
--- a/talk/p2p/base/p2ptransportchannel.h
+++ b/talk/p2p/base/p2ptransportchannel.h
@@ -189,6 +189,7 @@
bool FindConnection(cricket::Connection* connection) const;
uint32 GetRemoteCandidateGeneration(const Candidate& candidate);
+ bool IsDuplicateRemoteCandidate(const Candidate& candidate);
void RememberRemoteCandidate(const Candidate& remote_candidate,
PortInterface* origin_port);
bool IsPingable(Connection* conn);