(Auto)update libjingle 63948945-> 64147530

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5825 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/media/webrtc/fakewebrtcvideoengine.h

@@ -433,7 +433,7 @@ class FakeWebRtcVideoEngine
   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);

talk/media/webrtc/fakewebrtcvoiceengine.h

@@ -151,6 +151,7 @@ class FakeWebRtcVoiceEngine
         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 @@ class FakeWebRtcVoiceEngine
     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 @@ class FakeWebRtcVoiceEngine
       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 @@ class FakeWebRtcVoiceEngine
   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_;

talk/media/webrtc/webrtcvideoengine.cc

@@ -164,6 +164,73 @@ static bool IsRembEnabled(const VideoCodec& codec) {
                                               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 @@ bool WebRtcVideoMediaChannel::SetSendCodecs(
     ConvertToCricketVideoCodec(*send_codec_, &current);
   }
   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 @@ bool WebRtcVideoMediaChannel::SetSendCodecs(
       webrtc::VideoCodec wcodec;
       if (engine()->ConvertFromCricketVideoCodec(checked_codec, &wcodec)) {
         if (send_codecs.empty()) {
-          nack_enabled_ = IsNackEnabled(checked_codec);
+          nack_enabled = IsNackEnabled(checked_codec);
-          remb_enabled_ = IsRembEnabled(checked_codec);
+          remb_enabled = IsRembEnabled(checked_codec);
         }
         send_codecs.push_back(wcodec);
       }
@@ -1680,35 +1749,43 @@ bool WebRtcVideoMediaChannel::SetSendCodecs(
   }
 
   // Recv protection.
-  for (RecvChannelMap::iterator it = recv_channels_.begin();
+  // Do not update if the status is same as previously configured.
-      it != recv_channels_.end(); ++it) {
+  if (nack_enabled_ != nack_enabled) {
-    int channel_id = it->second->channel_id();
+    for (RecvChannelMap::iterator it = recv_channels_.begin();
-    if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
+        it != recv_channels_.end(); ++it) {
-                    nack_enabled_)) {
+      int channel_id = it->second->channel_id();
-      return false;
+      if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
-    }
+                      nack_enabled)) {
-    if (engine_->vie()->rtp()->SetRembStatus(channel_id,
+        return false;
-                                             kNotSending,
+      }
-                                             remb_enabled_) != 0) {
+      if (engine_->vie()->rtp()->SetRembStatus(channel_id,
-      LOG_RTCERR3(SetRembStatus, channel_id, kNotSending, remb_enabled_);
+                                               kNotSending,
-      return false;
+                                               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();
+  // Do not update if the status is same as previously configured.
-       iter != send_channels_.end(); ++iter) {
+  if (remb_enabled_ != remb_enabled) {
-    int channel_id = iter->second->channel_id();
+    for (SendChannelMap::iterator iter = send_channels_.begin();
-    if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
+         iter != send_channels_.end(); ++iter) {
-                    nack_enabled_)) {
+      int channel_id = iter->second->channel_id();
-      return false;
+      if (!SetNackFec(channel_id, send_red_type_, send_fec_type_,
-    }
+                      nack_enabled_)) {
-    if (engine_->vie()->rtp()->SetRembStatus(channel_id,
+        return false;
-                                             remb_enabled_,
+      }
-                                             remb_enabled_) != 0) {
+      if (engine_->vie()->rtp()->SetRembStatus(channel_id,
-      LOG_RTCERR3(SetRembStatus, channel_id, remb_enabled_, remb_enabled_);
+                                               remb_enabled,
-      return false;
+                                               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 @@ bool WebRtcVideoMediaChannel::SetSendCodec(
                  << "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;

talk/media/webrtc/webrtcvideoengine_unittest.cc

@@ -313,9 +313,28 @@ TEST_F(WebRtcVideoEngineTestFake, SetSendCodecs) {
   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 @@ TEST_F(WebRtcVideoEngineTestFake, ResetCodecOnScreencast) {
       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 @@ TEST_F(WebRtcVideoEngineTestFake, ResetCodecOnScreencast) {
   // 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);
 }

talk/media/webrtc/webrtcvoiceengine.cc

@@ -220,6 +220,22 @@ static bool IsNackEnabled(const AudioCodec& codec) {
                                               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 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
   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 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
       }
     } else {
       send_codec = voe_codec;
-      nack_enabled_ = IsNackEnabled(*it);
+      nack_enabled = IsNackEnabled(*it);
-      SetNack(channel, nack_enabled_);
     }
     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 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
       }
     }
   }
-
   return true;
 }
 
@@ -2167,8 +2188,8 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
     }
   }
 
+  // Set nack status on receive channels and update |nack_enabled_|.
   SetNack(receive_channels_, nack_enabled_);
-
   return true;
 }
 
@@ -2208,6 +2229,13 @@ bool WebRtcVoiceMediaChannel::SetSendCodec(
   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;

talk/media/webrtc/webrtcvoiceengine_unittest.cc

@@ -743,6 +743,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecs) {
   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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecs) {
   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());

talk/p2p/base/p2ptransportchannel.cc

@@ -629,7 +629,7 @@ void P2PTransportChannel::OnCandidate(const Candidate& candidate) {
 // 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 @@ bool P2PTransportChannel::CreateConnections(const Candidate &remote_candidate,
     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 @@ bool P2PTransportChannel::CreateConnection(PortInterface* port,
     // 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 @@ uint32 P2PTransportChannel::GetRemoteCandidateGeneration(
   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 @@ void P2PTransportChannel::RememberRemoteCandidate(
   }
 
   // Make sure this candidate is not a duplicate.
-  for (uint32 i = 0; i < remote_candidates_.size(); ++i) {
+  if (IsDuplicateRemoteCandidate(remote_candidate)) {
-    if (remote_candidates_[i].IsEquivalent(remote_candidate)) {
+    LOG(INFO) << "Duplicate candidate: " << remote_candidate.ToString();
-      LOG(INFO) << "Duplicate candidate: "
+    return;
-                << remote_candidate.address().ToSensitiveString();
-      return;
-    }
   }
 
   // Try this candidate for all future ports.

talk/p2p/base/p2ptransportchannel.h

@@ -189,6 +189,7 @@ class P2PTransportChannel : public TransportChannelImpl,
   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);