Experiment for the nack module.

webrtc/modules/video_coding/jitter_buffer.cc

Index: webrtc/modules/video_coding/jitter_buffer.cc
diff --git a/webrtc/modules/video_coding/jitter_buffer.cc b/webrtc/modules/video_coding/jitter_buffer.cc
index ca8c0d957148233f56ca07debc8dbf8aad155bb8..afe282b5456552eb1641b00778d36ab8b58881f1 100644
--- a/webrtc/modules/video_coding/jitter_buffer.cc
+++ b/webrtc/modules/video_coding/jitter_buffer.cc
@@ -28,10 +28,10 @@
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/include/event_wrapper.h"
+#include "webrtc/system_wrappers/include/field_trial.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 
 namespace webrtc {
-
 // Interval for updating SS data.
 static const uint32_t kSsCleanupIntervalSec = 60;
 
@@ -215,7 +215,9 @@ void Vp9SsMap::UpdateFrames(FrameList* frames) {
 }
 
 VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
-                                 std::unique_ptr<EventWrapper> event)
+                                 std::unique_ptr<EventWrapper> event,
+                                 NackSender* nack_sender,
+                                 KeyFrameRequestSender* keyframe_request_sender)
     : clock_(clock),
       running_(false),
       crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
@@ -250,6 +252,17 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
       decode_error_mode_(kNoErrors),
       average_packets_per_frame_(0.0f),
       frame_counter_(0) {
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") {
+    // Still need to check if the nack module should be created since
+    // a jitter buffer is created for the video send stream as well.

[stefan-webrtc, 2016/03/10 17:06:29]

Change this comment, since when the refactoring is done so that this isn't the
case, we shouldn't have a comment lying around saying so. Better to just say
"Only create a nack module if we are going to send nacks"

[philipel, 2016/03/11 09:24:14]

Done.

+    if (nack_sender) {
+    RTC_DCHECK(nack_sender);

[stefan-webrtc, 2016/03/10 17:06:30]

Indentation

[philipel, 2016/03/11 09:24:14]

Done.

+    RTC_DCHECK(keyframe_request_sender);
+
+    nack_module_.reset(
+        new NackModule(clock, nack_sender, keyframe_request_sender));
+    }
+  }
   for (int i = 0; i < kStartNumberOfFrames; i++)
     free_frames_.push_back(new VCMFrameBuffer());
 }
@@ -329,6 +342,8 @@ void VCMJitterBuffer::Stop() {
   UpdateHistograms();
   running_ = false;
   last_decoded_state_.Reset();
+  if (nack_module_)
+    nack_module_->Stop();
 
   // Make sure all frames are free and reset.
   for (FrameList::iterator it = decodable_frames_.begin();
@@ -369,6 +384,8 @@ void VCMJitterBuffer::Flush() {
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   missing_sequence_numbers_.clear();
+  if (nack_module_)
+    nack_module_->Flush();
 }
 
 // Get received key and delta frames
@@ -605,6 +622,8 @@ VCMEncodedFrame* VCMJitterBuffer::ExtractAndSetDecode(uint32_t timestamp) {
   if ((*frame).IsSessionComplete())
     UpdateAveragePacketsPerFrame(frame->NumPackets());
 
+  if (nack_module_)
+    nack_module_->ClearUpTo(frame->GetHighSeqNum());
   return frame;
 }
 
@@ -665,6 +684,9 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
                                                  bool* retransmitted) {
   CriticalSectionScoped cs(crit_sect_);
 
+  if (nack_module_)
+    nack_module_->OnReceivedPacket(packet);
+
   ++num_packets_;
   if (num_packets_ == 1) {
     time_first_packet_ms_ = clock_->TimeInMilliseconds();
@@ -927,6 +949,8 @@ void VCMJitterBuffer::UpdateRtt(int64_t rtt_ms) {
   CriticalSectionScoped cs(crit_sect_);
   rtt_ms_ = rtt_ms;
   jitter_estimate_.UpdateRtt(rtt_ms);
+  if (nack_module_)
+    nack_module_->UpdateRtt(rtt_ms);
 }
 
 void VCMJitterBuffer::SetNackMode(VCMNackMode mode,
@@ -1046,6 +1070,10 @@ std::vector<uint16_t> VCMJitterBuffer::GetNackList(bool* request_key_frame) {
       }
     }
   }
+  // The experiment is running, the nack module will send Nacks.
+  if (nack_module_)
+    return std::vector<uint16_t>();
+
   std::vector<uint16_t> nack_list(missing_sequence_numbers_.begin(),
                                   missing_sequence_numbers_.end());
   return nack_list;
@@ -1343,4 +1371,15 @@ bool VCMJitterBuffer::WaitForRetransmissions() {
   }
   return true;
 }
+
+int64_t VCMJitterBuffer::TimeUntilNextProcess() {
+  if (nack_module_)
+    return nack_module_->TimeUntilNextProcess();
+  return 100000;

[stefan-webrtc, 2016/03/10 17:06:29]

This is a bit ugly, but it might be necessary. Is it possible to return max of
int64_t? I have some memory that it might cause problems in ModuleProcessThread,
i.e., it will sleep for ever if this is the first module that is added... In
that case 100000 might not work well either.

[philipel, 2016/03/11 09:24:14]

Changed to int64_t max. Just testing using video_loopback I don't get any
strange (hanging) behavior.

+}
+
+void VCMJitterBuffer::Process() {
+  if (nack_module_)
+    nack_module_->Process();
+}
 }  // namespace webrtc