Make setting identical RTP extensions a no-op.

Setting extensions are responsible for a lot of stream tear-downs
causing substantial slowdowns in SetRemoteDescription.

BUG=1788,4077
R=pthatcher@webrtc.org

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

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

talk/media/webrtc/webrtcvideoengine2.cc

@@ -28,6 +28,7 @@
 #ifdef HAVE_WEBRTC_VIDEO
 #include "talk/media/webrtc/webrtcvideoengine2.h"
 
+#include <algorithm>
 #include <set>
 #include <string>
 
@@ -165,6 +166,13 @@ static bool ValidateRtpHeaderExtensionIds(
   return true;
 }
 
+static bool CompareRtpHeaderExtensionIds(
+    const webrtc::RtpExtension& extension1,
+    const webrtc::RtpExtension& extension2) {
+  // Sorting on ID is sufficient, more than one extension per ID is unsupported.
+  return extension1.id > extension2.id;
+}
+
 static std::vector<webrtc::RtpExtension> FilterRtpExtensions(
     const std::vector<RtpHeaderExtension>& extensions) {
   std::vector<webrtc::RtpExtension> webrtc_extensions;
@@ -177,9 +185,28 @@ static std::vector<webrtc::RtpExtension> FilterRtpExtensions(
       LOG(LS_WARNING) << "Unsupported RTP extension: " << extensions[i].uri;
     }
   }
+
+  // Sort filtered headers to make sure that they can later be compared
+  // regardless of in which order they were entered.
+  std::sort(webrtc_extensions.begin(), webrtc_extensions.end(),
+            CompareRtpHeaderExtensionIds);
   return webrtc_extensions;
 }
 
+static bool RtpExtensionsHaveChanged(
+    const std::vector<webrtc::RtpExtension>& before,
+    const std::vector<webrtc::RtpExtension>& after) {
+  if (before.size() != after.size())
+    return true;
+  for (size_t i = 0; i < before.size(); ++i) {
+    if (before[i].id != after[i].id)
+      return true;
+    if (before[i].name != after[i].name)
+      return true;
+  }
+  return false;
+}
+
 WebRtcVideoEncoderFactory2::~WebRtcVideoEncoderFactory2() {
 }
 
@@ -1156,7 +1183,13 @@ bool WebRtcVideoChannel2::SetRecvRtpHeaderExtensions(
   if (!ValidateRtpHeaderExtensionIds(extensions))
     return false;
 
-  recv_rtp_extensions_ = FilterRtpExtensions(extensions);
+  std::vector<webrtc::RtpExtension> filtered_extensions =
+      FilterRtpExtensions(extensions);
+  if (!RtpExtensionsHaveChanged(recv_rtp_extensions_, filtered_extensions))
+    return true;
+
+  recv_rtp_extensions_ = filtered_extensions;
+
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32, WebRtcVideoReceiveStream*>::iterator it =
            receive_streams_.begin();
@@ -1174,7 +1207,12 @@ bool WebRtcVideoChannel2::SetSendRtpHeaderExtensions(
   if (!ValidateRtpHeaderExtensionIds(extensions))
     return false;
 
-  send_rtp_extensions_ = FilterRtpExtensions(extensions);
+  std::vector<webrtc::RtpExtension> filtered_extensions =
+      FilterRtpExtensions(extensions);
+  if (!RtpExtensionsHaveChanged(send_rtp_extensions_, filtered_extensions))
+    return true;
+
+  send_rtp_extensions_ = filtered_extensions;
 
   rtc::CritScope stream_lock(&stream_crit_);
   for (std::map<uint32, WebRtcVideoSendStream*>::iterator it =

talk/media/webrtc/webrtcvideoengine2_unittest.cc

@@ -25,6 +25,7 @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <algorithm>
 #include <map>
 #include <vector>
 
@@ -188,7 +189,10 @@ void FakeVideoReceiveStream::Stop() {
 }
 
 FakeCall::FakeCall(const webrtc::Call::Config& config)
-    : config_(config), network_state_(kNetworkUp) {
+    : config_(config),
+      network_state_(kNetworkUp),
+      num_created_send_streams_(0),
+      num_created_receive_streams_(0) {
   SetVideoCodecs(GetDefaultVideoCodecs());
 }
 
@@ -265,6 +269,7 @@ webrtc::VideoSendStream* FakeCall::CreateVideoSendStream(
   FakeVideoSendStream* fake_stream =
       new FakeVideoSendStream(config, encoder_config);
   video_send_streams_.push_back(fake_stream);
+  ++num_created_send_streams_;
   return fake_stream;
 }
 
@@ -284,6 +289,7 @@ void FakeCall::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) {
 webrtc::VideoReceiveStream* FakeCall::CreateVideoReceiveStream(
     const webrtc::VideoReceiveStream::Config& config) {
   video_receive_streams_.push_back(new FakeVideoReceiveStream(config));
+  ++num_created_receive_streams_;
   return video_receive_streams_[video_receive_streams_.size() - 1];
 }
 
@@ -310,6 +316,14 @@ void FakeCall::SetStats(const webrtc::Call::Stats& stats) {
   stats_ = stats;
 }
 
+int FakeCall::GetNumCreatedSendStreams() const {
+  return num_created_send_streams_;
+}
+
+int FakeCall::GetNumCreatedReceiveStreams() const {
+  return num_created_receive_streams_;
+}
+
 webrtc::Call::Stats FakeCall::GetStats() const {
   return stats_;
 }
@@ -1049,6 +1063,66 @@ TEST_F(WebRtcVideoChannel2Test, RecvAbsoluteSendTimeHeaderExtensions) {
                                  webrtc::RtpExtension::kAbsSendTime);
 }
 
+TEST_F(WebRtcVideoChannel2Test, IdenticalSendExtensionsDoesntRecreateStream) {
+  const int kTOffsetId = 1;
+  const int kAbsSendTimeId = 2;
+  std::vector<cricket::RtpHeaderExtension> extensions;
+  extensions.push_back(cricket::RtpHeaderExtension(
+      kRtpAbsoluteSenderTimeHeaderExtension, kAbsSendTimeId));
+  extensions.push_back(cricket::RtpHeaderExtension(
+      kRtpTimestampOffsetHeaderExtension, kTOffsetId));
+
+  EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions));
+  FakeVideoSendStream* send_stream =
+      AddSendStream(cricket::StreamParams::CreateLegacy(123));
+
+  EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams());
+  ASSERT_EQ(2u, send_stream->GetConfig().rtp.extensions.size());
+
+  // Setting the same extensions (even if in different order) shouldn't
+  // reallocate the stream.
+  std::reverse(extensions.begin(), extensions.end());
+  EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions));
+
+  EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams());
+
+  // Setting different extensions should recreate the stream.
+  extensions.resize(1);
+  EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions));
+
+  EXPECT_EQ(2, fake_call_->GetNumCreatedSendStreams());
+}
+
+TEST_F(WebRtcVideoChannel2Test, IdenticalRecvExtensionsDoesntRecreateStream) {
+  const int kTOffsetId = 1;
+  const int kAbsSendTimeId = 2;
+  std::vector<cricket::RtpHeaderExtension> extensions;
+  extensions.push_back(cricket::RtpHeaderExtension(
+      kRtpAbsoluteSenderTimeHeaderExtension, kAbsSendTimeId));
+  extensions.push_back(cricket::RtpHeaderExtension(
+      kRtpTimestampOffsetHeaderExtension, kTOffsetId));
+
+  EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions));
+  FakeVideoReceiveStream* send_stream =
+      AddRecvStream(cricket::StreamParams::CreateLegacy(123));
+
+  EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams());
+  ASSERT_EQ(2u, send_stream->GetConfig().rtp.extensions.size());
+
+  // Setting the same extensions (even if in different order) shouldn't
+  // reallocate the stream.
+  std::reverse(extensions.begin(), extensions.end());
+  EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions));
+
+  EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams());
+
+  // Setting different extensions should recreate the stream.
+  extensions.resize(1);
+  EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions));
+
+  EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams());
+}
+
 TEST_F(WebRtcVideoChannel2Test,
        SetSendRtpHeaderExtensionsExcludeUnsupportedExtensions) {
   const int kUnsupportedId = 1;

talk/media/webrtc/webrtcvideoengine2_unittest.h

@@ -113,6 +113,8 @@ class FakeCall : public webrtc::Call {
   std::vector<webrtc::VideoCodec> GetDefaultVideoCodecs();
 
   webrtc::Call::NetworkState GetNetworkState() const;
+  int GetNumCreatedSendStreams() const;
+  int GetNumCreatedReceiveStreams() const;
   void SetStats(const webrtc::Call::Stats& stats);
 
  private:
@@ -142,6 +144,9 @@ class FakeCall : public webrtc::Call {
   std::vector<webrtc::VideoCodec> codecs_;
   std::vector<FakeVideoSendStream*> video_send_streams_;
   std::vector<FakeVideoReceiveStream*> video_receive_streams_;
+
+  int num_created_send_streams_;
+  int num_created_receive_streams_;
 };
 
 }  // namespace cricket