Revert of Revert Make the new jitter buffer the default jitter buffer.

Revert of Revert Make the new jitter buffer the default jitter buffer. (patchset #1 id:1 of https://codereview.chromium.org/2682073003/ )

Reason for revert:
Fix here: https://codereview.chromium.org/2708593003

Original issue's description:
> Revert Make the new jitter buffer the default jitter buffer.
> 
> Speculative revert of https://codereview.chromium.org/2656983002/ to see if it fixes a downstream bug.
> 
> BUG=webrtc:5514
> 
> Review-Url: https://codereview.webrtc.org/2682073003
> Cr-Commit-Position: refs/heads/master@{#16492}
> Committed: https://chromium.googlesource.com/external/webrtc/+/e525d6aba634b8442a0b9c247a89a6060d225ca3

TBR=nisse@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5514
Review-Url: https://codereview.webrtc.org/2704183002
Cr-Commit-Position: refs/heads/master@{#16772}
Committed: https://chromium.googlesource.com/external/webrtc/+/a45102f7b4a8df1e56d1c42a15d30576614a7551

[philipel, 2017-02-20 14:19:56]

Created Revert of Revert Make the new jitter buffer the default jitter buffer.

[philipel, 2017-02-20 14:22:24]

The CQ bit was checked by philipel@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-20 14:22:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-20 16:29:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-20 16:29:14]

Dry run: This issue passed the CQ dry run.

[stefan-webrtc, 2017-02-21 15:41:25]

lgtm

[philipel, 2017-02-22 09:59:22]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-02-22 09:59:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-22 10:59:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 10:59:39]

Failed to apply patch for webrtc/video/rtp_stream_receiver.cc:
While running git apply --index -p1;
  error: patch failed: webrtc/video/rtp_stream_receiver.cc:420
  error: webrtc/video/rtp_stream_receiver.cc: patch does not apply

Patch:       webrtc/video/rtp_stream_receiver.cc
Index: webrtc/video/rtp_stream_receiver.cc
diff --git a/webrtc/video/rtp_stream_receiver.cc
b/webrtc/video/rtp_stream_receiver.cc
index
d49d0959f32fc826dd3da0cb26d7dda3fe1e0079..af12b51556ce7a77341709d092ed46441b743f14
100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -80,7 +80,6 @@
 static const int kPacketLogIntervalMs = 10000;
 
 RtpStreamReceiver::RtpStreamReceiver(
-    vcm::VideoReceiver* video_receiver,
     Transport* transport,
     RtcpRttStats* rtt_stats,
     PacketRouter* packet_router,
@@ -94,7 +93,6 @@
     VCMTiming* timing)
     : clock_(Clock::GetRealTimeClock()),
       config_(*config),
-      video_receiver_(video_receiver),
       packet_router_(packet_router),
       remb_(remb),
       process_thread_(process_thread),
@@ -186,25 +184,21 @@
 
   process_thread_->RegisterModule(rtp_rtcp_.get());
 
-  jitter_buffer_experiment_ =
-      field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled";
-
-  if (jitter_buffer_experiment_) {
-    nack_module_.reset(
-        new NackModule(clock_, nack_sender, keyframe_request_sender));
-    process_thread_->RegisterModule(nack_module_.get());
-
-    packet_buffer_ = video_coding::PacketBuffer::Create(
-        clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this);
-    reference_finder_.reset(new video_coding::RtpFrameReferenceFinder(this));
-  }
+  nack_module_.reset(
+      new NackModule(clock_, nack_sender, keyframe_request_sender));
+  if (config_.rtp.nack.rtp_history_ms == 0)
+    nack_module_->Stop();
+  process_thread_->RegisterModule(nack_module_.get());
+
+  packet_buffer_ = video_coding::PacketBuffer::Create(
+      clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this);
+  reference_finder_.reset(new video_coding::RtpFrameReferenceFinder(this));
 }
 
 RtpStreamReceiver::~RtpStreamReceiver() {
   process_thread_->DeRegisterModule(rtp_rtcp_.get());
 
-  if (jitter_buffer_experiment_)
-    process_thread_->DeRegisterModule(nack_module_.get());
+  process_thread_->DeRegisterModule(nack_module_.get());
 
   packet_router_->RemoveRtpModule(rtp_rtcp_.get());
   rtp_rtcp_->SetREMBStatus(false);
@@ -249,43 +243,35 @@
   WebRtcRTPHeader rtp_header_with_ntp = *rtp_header;
   rtp_header_with_ntp.ntp_time_ms =
       ntp_estimator_.Estimate(rtp_header->header.timestamp);
-  if (jitter_buffer_experiment_) {
-    VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
-    packet.timesNacked = nack_module_->OnReceivedPacket(packet);
-
-    if (packet.codec == kVideoCodecH264) {
-      // Only when we start to receive packets will we know what payload type
-      // that will be used. When we know the payload type insert the correct
-      // sps/pps into the tracker.
-      if (packet.payloadType != last_payload_type_) {
-        last_payload_type_ = packet.payloadType;
-        InsertSpsPpsIntoTracker(packet.payloadType);
-      }
-
-      switch (tracker_.CopyAndFixBitstream(&packet)) {
-        case video_coding::H264SpsPpsTracker::kRequestKeyframe:
-          keyframe_request_sender_->RequestKeyFrame();
-          FALLTHROUGH();
-        case video_coding::H264SpsPpsTracker::kDrop:
-          return 0;
-        case video_coding::H264SpsPpsTracker::kInsert:
-          break;
-      }
-    } else {
-      uint8_t* data = new uint8_t[packet.sizeBytes];
-      memcpy(data, packet.dataPtr, packet.sizeBytes);
-      packet.dataPtr = data;
-    }
-
-    packet_buffer_->InsertPacket(&packet);
+  VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
+  packet.timesNacked = nack_module_->OnReceivedPacket(packet);
+
+  if (packet.codec == kVideoCodecH264) {
+    // Only when we start to receive packets will we know what payload type
+    // that will be used. When we know the payload type insert the correct
+    // sps/pps into the tracker.
+    if (packet.payloadType != last_payload_type_) {
+      last_payload_type_ = packet.payloadType;
+      InsertSpsPpsIntoTracker(packet.payloadType);
+    }
+
+    switch (tracker_.CopyAndFixBitstream(&packet)) {
+      case video_coding::H264SpsPpsTracker::kRequestKeyframe:
+        keyframe_request_sender_->RequestKeyFrame();
+        FALLTHROUGH();
+      case video_coding::H264SpsPpsTracker::kDrop:
+        return 0;
+      case video_coding::H264SpsPpsTracker::kInsert:
+        break;
+    }
+
   } else {
-    RTC_DCHECK(video_receiver_);
-    if (video_receiver_->IncomingPacket(payload_data, payload_size,
-                                        rtp_header_with_ntp) != 0) {
-      // Check this...
-      return -1;
-    }
-  }
+    uint8_t* data = new uint8_t[packet.sizeBytes];
+    memcpy(data, packet.dataPtr, packet.sizeBytes);
+    packet.dataPtr = data;
+  }
+
+  packet_buffer_->InsertPacket(&packet);
   return 0;
 }
 
@@ -420,8 +406,7 @@
 }
 
 void RtpStreamReceiver::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
-  if (jitter_buffer_experiment_)
-    nack_module_->UpdateRtt(max_rtt_ms);
+  nack_module_->UpdateRtt(max_rtt_ms);
 }
 
 bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
@@ -549,35 +534,31 @@
 }
 
 void RtpStreamReceiver::FrameContinuous(uint16_t picture_id) {
-  if (jitter_buffer_experiment_) {
-    int seq_num = -1;
-    {
-      rtc::CritScope lock(&last_seq_num_cs_);
-      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
-      if (seq_num_it != last_seq_num_for_pic_id_.end())
-        seq_num = seq_num_it->second;
-    }
-    if (seq_num != -1)
-      nack_module_->ClearUpTo(seq_num);
-  }
+  int seq_num = -1;
+  {
+    rtc::CritScope lock(&last_seq_num_cs_);
+    auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+    if (seq_num_it != last_seq_num_for_pic_id_.end())
+      seq_num = seq_num_it->second;
+  }
+  if (seq_num != -1)
+    nack_module_->ClearUpTo(seq_num);
 }
 
 void RtpStreamReceiver::FrameDecoded(uint16_t picture_id) {
-  if (jitter_buffer_experiment_) {
-    int seq_num = -1;
-    {
-      rtc::CritScope lock(&last_seq_num_cs_);
-      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
-      if (seq_num_it != last_seq_num_for_pic_id_.end()) {
-        seq_num = seq_num_it->second;
-        last_seq_num_for_pic_id_.erase(last_seq_num_for_pic_id_.begin(),
-                                       ++seq_num_it);
-      }
-    }
-    if (seq_num != -1) {
-      packet_buffer_->ClearTo(seq_num);
-      reference_finder_->ClearTo(seq_num);
-    }
+  int seq_num = -1;
+  {
+    rtc::CritScope lock(&last_seq_num_cs_);
+    auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+    if (seq_num_it != last_seq_num_for_pic_id_.end()) {
+      seq_num = seq_num_it->second;
+      last_seq_num_for_pic_id_.erase(last_seq_num_for_pic_id_.begin(),
+                                     ++seq_num_it);
+    }
+  }
+  if (seq_num != -1) {
+    packet_buffer_->ClearTo(seq_num);
+    reference_finder_->ClearTo(seq_num);
   }
 }

[philipel, 2017-02-22 11:33:05]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-02-22 11:33:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-22 11:36:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 11:36:07]

Failed to apply patch for webrtc/video/rtp_stream_receiver.cc:
While running git apply --index -p1;
  error: patch failed: webrtc/video/rtp_stream_receiver.cc:420
  error: webrtc/video/rtp_stream_receiver.cc: patch does not apply

Patch:       webrtc/video/rtp_stream_receiver.cc
Index: webrtc/video/rtp_stream_receiver.cc
diff --git a/webrtc/video/rtp_stream_receiver.cc
b/webrtc/video/rtp_stream_receiver.cc
index
d49d0959f32fc826dd3da0cb26d7dda3fe1e0079..af12b51556ce7a77341709d092ed46441b743f14
100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -80,7 +80,6 @@
 static const int kPacketLogIntervalMs = 10000;
 
 RtpStreamReceiver::RtpStreamReceiver(
-    vcm::VideoReceiver* video_receiver,
     Transport* transport,
     RtcpRttStats* rtt_stats,
     PacketRouter* packet_router,
@@ -94,7 +93,6 @@
     VCMTiming* timing)
     : clock_(Clock::GetRealTimeClock()),
       config_(*config),
-      video_receiver_(video_receiver),
       packet_router_(packet_router),
       remb_(remb),
       process_thread_(process_thread),
@@ -186,25 +184,21 @@
 
   process_thread_->RegisterModule(rtp_rtcp_.get());
 
-  jitter_buffer_experiment_ =
-      field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled";
-
-  if (jitter_buffer_experiment_) {
-    nack_module_.reset(
-        new NackModule(clock_, nack_sender, keyframe_request_sender));
-    process_thread_->RegisterModule(nack_module_.get());
-
-    packet_buffer_ = video_coding::PacketBuffer::Create(
-        clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this);
-    reference_finder_.reset(new video_coding::RtpFrameReferenceFinder(this));
-  }
+  nack_module_.reset(
+      new NackModule(clock_, nack_sender, keyframe_request_sender));
+  if (config_.rtp.nack.rtp_history_ms == 0)
+    nack_module_->Stop();
+  process_thread_->RegisterModule(nack_module_.get());
+
+  packet_buffer_ = video_coding::PacketBuffer::Create(
+      clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this);
+  reference_finder_.reset(new video_coding::RtpFrameReferenceFinder(this));
 }
 
 RtpStreamReceiver::~RtpStreamReceiver() {
   process_thread_->DeRegisterModule(rtp_rtcp_.get());
 
-  if (jitter_buffer_experiment_)
-    process_thread_->DeRegisterModule(nack_module_.get());
+  process_thread_->DeRegisterModule(nack_module_.get());
 
   packet_router_->RemoveRtpModule(rtp_rtcp_.get());
   rtp_rtcp_->SetREMBStatus(false);
@@ -249,43 +243,35 @@
   WebRtcRTPHeader rtp_header_with_ntp = *rtp_header;
   rtp_header_with_ntp.ntp_time_ms =
       ntp_estimator_.Estimate(rtp_header->header.timestamp);
-  if (jitter_buffer_experiment_) {
-    VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
-    packet.timesNacked = nack_module_->OnReceivedPacket(packet);
-
-    if (packet.codec == kVideoCodecH264) {
-      // Only when we start to receive packets will we know what payload type
-      // that will be used. When we know the payload type insert the correct
-      // sps/pps into the tracker.
-      if (packet.payloadType != last_payload_type_) {
-        last_payload_type_ = packet.payloadType;
-        InsertSpsPpsIntoTracker(packet.payloadType);
-      }
-
-      switch (tracker_.CopyAndFixBitstream(&packet)) {
-        case video_coding::H264SpsPpsTracker::kRequestKeyframe:
-          keyframe_request_sender_->RequestKeyFrame();
-          FALLTHROUGH();
-        case video_coding::H264SpsPpsTracker::kDrop:
-          return 0;
-        case video_coding::H264SpsPpsTracker::kInsert:
-          break;
-      }
-    } else {
-      uint8_t* data = new uint8_t[packet.sizeBytes];
-      memcpy(data, packet.dataPtr, packet.sizeBytes);
-      packet.dataPtr = data;
-    }
-
-    packet_buffer_->InsertPacket(&packet);
+  VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
+  packet.timesNacked = nack_module_->OnReceivedPacket(packet);
+
+  if (packet.codec == kVideoCodecH264) {
+    // Only when we start to receive packets will we know what payload type
+    // that will be used. When we know the payload type insert the correct
+    // sps/pps into the tracker.
+    if (packet.payloadType != last_payload_type_) {
+      last_payload_type_ = packet.payloadType;
+      InsertSpsPpsIntoTracker(packet.payloadType);
+    }
+
+    switch (tracker_.CopyAndFixBitstream(&packet)) {
+      case video_coding::H264SpsPpsTracker::kRequestKeyframe:
+        keyframe_request_sender_->RequestKeyFrame();
+        FALLTHROUGH();
+      case video_coding::H264SpsPpsTracker::kDrop:
+        return 0;
+      case video_coding::H264SpsPpsTracker::kInsert:
+        break;
+    }
+
   } else {
-    RTC_DCHECK(video_receiver_);
-    if (video_receiver_->IncomingPacket(payload_data, payload_size,
-                                        rtp_header_with_ntp) != 0) {
-      // Check this...
-      return -1;
-    }
-  }
+    uint8_t* data = new uint8_t[packet.sizeBytes];
+    memcpy(data, packet.dataPtr, packet.sizeBytes);
+    packet.dataPtr = data;
+  }
+
+  packet_buffer_->InsertPacket(&packet);
   return 0;
 }
 
@@ -420,8 +406,7 @@
 }
 
 void RtpStreamReceiver::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
-  if (jitter_buffer_experiment_)
-    nack_module_->UpdateRtt(max_rtt_ms);
+  nack_module_->UpdateRtt(max_rtt_ms);
 }
 
 bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
@@ -549,35 +534,31 @@
 }
 
 void RtpStreamReceiver::FrameContinuous(uint16_t picture_id) {
-  if (jitter_buffer_experiment_) {
-    int seq_num = -1;
-    {
-      rtc::CritScope lock(&last_seq_num_cs_);
-      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
-      if (seq_num_it != last_seq_num_for_pic_id_.end())
-        seq_num = seq_num_it->second;
-    }
-    if (seq_num != -1)
-      nack_module_->ClearUpTo(seq_num);
-  }
+  int seq_num = -1;
+  {
+    rtc::CritScope lock(&last_seq_num_cs_);
+    auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+    if (seq_num_it != last_seq_num_for_pic_id_.end())
+      seq_num = seq_num_it->second;
+  }
+  if (seq_num != -1)
+    nack_module_->ClearUpTo(seq_num);
 }
 
 void RtpStreamReceiver::FrameDecoded(uint16_t picture_id) {
-  if (jitter_buffer_experiment_) {
-    int seq_num = -1;
-    {
-      rtc::CritScope lock(&last_seq_num_cs_);
-      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
-      if (seq_num_it != last_seq_num_for_pic_id_.end()) {
-        seq_num = seq_num_it->second;
-        last_seq_num_for_pic_id_.erase(last_seq_num_for_pic_id_.begin(),
-                                       ++seq_num_it);
-      }
-    }
-    if (seq_num != -1) {
-      packet_buffer_->ClearTo(seq_num);
-      reference_finder_->ClearTo(seq_num);
-    }
+  int seq_num = -1;
+  {
+    rtc::CritScope lock(&last_seq_num_cs_);
+    auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+    if (seq_num_it != last_seq_num_for_pic_id_.end()) {
+      seq_num = seq_num_it->second;
+      last_seq_num_for_pic_id_.erase(last_seq_num_for_pic_id_.begin(),
+                                     ++seq_num_it);
+    }
+  }
+  if (seq_num != -1) {
+    packet_buffer_->ClearTo(seq_num);
+    reference_finder_->ClearTo(seq_num);
   }
 }

[philipel, 2017-02-22 12:06:04]

The CQ bit was checked by philipel@webrtc.org

[philipel, 2017-02-22 12:06:05]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2704183002/#ps260001 (title:
"Rebase")

[commit-bot: I haz the power, 2017-02-22 12:06:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-22 12:22:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 12:22:48]

Try jobs failed on following builders:
  win_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_baremetal/builds/18950)

[philipel, 2017-02-22 13:11:54]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-02-22 13:12:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-22 13:30:40]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1487769114655720,
"parent_rev": "5650a7d1c41a264a0da15c3bb17d2c14335fcc27", "commit_rev":
"a45102f7b4a8df1e56d1c42a15d30576614a7551"}

[commit-bot: I haz the power, 2017-02-22 13:30:43]

Description was changed from

==========
Revert of Revert Make the new jitter buffer the default jitter buffer. (patchset
#1 id:1 of https://codereview.chromium.org/2682073003/ )

Reason for revert:
Fix here: https://codereview.chromium.org/2708593003

Original issue's description:
> Revert Make the new jitter buffer the default jitter buffer.
> 
> Speculative revert of https://codereview.chromium.org/2656983002/ to see if it
fixes a downstream bug.
> 
> BUG=webrtc:5514
> 
> Review-Url: https://codereview.webrtc.org/2682073003
> Cr-Commit-Position: refs/heads/master@{#16492}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/e525d6aba634b8442a0b9c247...

TBR=nisse@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5514
==========

to

==========
Revert of Revert Make the new jitter buffer the default jitter buffer. (patchset
#1 id:1 of https://codereview.chromium.org/2682073003/ )

Reason for revert:
Fix here: https://codereview.chromium.org/2708593003

Original issue's description:
> Revert Make the new jitter buffer the default jitter buffer.
> 
> Speculative revert of https://codereview.chromium.org/2656983002/ to see if it
fixes a downstream bug.
> 
> BUG=webrtc:5514
> 
> Review-Url: https://codereview.webrtc.org/2682073003
> Cr-Commit-Position: refs/heads/master@{#16492}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/e525d6aba634b8442a0b9c247...

TBR=nisse@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5514

Review-Url: https://codereview.webrtc.org/2704183002
Cr-Commit-Position: refs/heads/master@{#16772}
Committed:
https://chromium.googlesource.com/external/webrtc/+/a45102f7b4a8df1e56d1c42a1...
==========

[commit-bot: I haz the power, 2017-02-22 13:30:44]

Committed patchset #2 (id:260001) as
https://chromium.googlesource.com/external/webrtc/+/a45102f7b4a8df1e56d1c42a1...