Experiment for the nack module.

webrtc/modules/video_coding/jitter_buffer.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include "webrtc/modules/video_coding/jitter_buffer.h"
@@ skipping 10 matching lines @@
 #include "webrtc/modules/video_coding/include/video_coding.h"
 #include "webrtc/modules/video_coding/frame_buffer.h"
 #include "webrtc/modules/video_coding/inter_frame_delay.h"
 #include "webrtc/modules/video_coding/internal_defines.h"
 #include "webrtc/modules/video_coding/jitter_buffer_common.h"
 #include "webrtc/modules/video_coding/jitter_estimator.h"
 #include "webrtc/modules/video_coding/packet.h"
 #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;
 
 // Use this rtt if no value has been reported.
 static const int64_t kDefaultRtt = 200;
 
 // Request a keyframe if no continuous frame has been received for this
 // number of milliseconds and NACKs are disabled.
 static const int64_t kMaxDiscontinuousFramesTime = 1000;
 
@@ skipping 163 matching lines @@
     if (Find(frame_it.second->TimeStamp(), &ss_it)) {
       if (gof_idx >= ss_it->second.num_frames_in_gof) {
         continue;  // Assume corresponding SS not yet received.
       }
       frame_it.second->SetGofInfo(ss_it->second, gof_idx);
     }
   }
 }
 
 VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
-                                 std::unique_ptr<EventWrapper> event)
+                                 std::unique_ptr<EventWrapper> event,
+                                 ProcessThread* module_process_thread,
+                                 NackSender* nack_sender,
+                                 KeyFrameRequestSender* keyframe_request_sender)
     : clock_(clock),
       running_(false),
       crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       frame_event_(std::move(event)),
       max_number_of_frames_(kStartNumberOfFrames),
       free_frames_(),
       decodable_frames_(),
       incomplete_frames_(),
       last_decoded_state_(),
       first_packet_since_reset_(true),
@@ skipping 13 matching lines @@
       rtt_ms_(kDefaultRtt),
       nack_mode_(kNoNack),
       low_rtt_nack_threshold_ms_(-1),
       high_rtt_nack_threshold_ms_(-1),
       missing_sequence_numbers_(SequenceNumberLessThan()),
       max_nack_list_size_(0),
       max_packet_age_to_nack_(0),
       max_incomplete_time_ms_(0),
       decode_error_mode_(kNoErrors),
       average_packets_per_frame_(0.0f),
-      frame_counter_(0) {
+      frame_counter_(0),
+      module_process_thread_(module_process_thread) {
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled") {
+    RTC_DCHECK(module_process_thread);
+    RTC_DCHECK(nack_sender);
+    RTC_DCHECK(keyframe_request_sender);
+
+    nack_module_.reset(
+        new NackModule(clock, nack_sender, keyframe_request_sender));

[stefan-webrtc, 2016/03/10 14:20:40]

Set in initializer list instead so that you can make nack_module_ const.

[philipel, 2016/03/10 16:39:27]

Can't do that since we have to check first if we are running the experiment.

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

You can still do it:

nack_module_(IsInExperiment() ? new NackModule() : nullptr),

I think that's preferable

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

Ok, it is possible, but don't you think it would be really ugly to include this
block of code in the initialization list?

[stefan-webrtc, 2016/03/11 09:57:11]

I think that's fine, just break it out to a static helper function. It makes it
a lot more clear that the nack module doesn't have to be protected by critical
sections.

[philipel, 2016/03/11 10:12:59]

Done.

+    module_process_thread_->RegisterModule(nack_module_.get());
+  }
   for (int i = 0; i < kStartNumberOfFrames; i++)
     free_frames_.push_back(new VCMFrameBuffer());
 }
 
 VCMJitterBuffer::~VCMJitterBuffer() {
   Stop();
   for (UnorderedFrameList::iterator it = free_frames_.begin();
        it != free_frames_.end(); ++it) {
     delete *it;
   }
   for (FrameList::iterator it = incomplete_frames_.begin();
        it != incomplete_frames_.end(); ++it) {
     delete it->second;
   }
   for (FrameList::iterator it = decodable_frames_.begin();
        it != decodable_frames_.end(); ++it) {
     delete it->second;
   }
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled")

[stefan-webrtc, 2016/03/10 14:20:40]

if (nack_module_) instead, here and all places below.

[philipel, 2016/03/10 16:39:27]

Done.

+    module_process_thread_->DeRegisterModule(nack_module_.get());
   delete crit_sect_;
 }
 
 void VCMJitterBuffer::UpdateHistograms() {
   if (num_packets_ <= 0 || !running_) {
     return;
   }
   int64_t elapsed_sec =
       (clock_->TimeInMilliseconds() - time_first_packet_ms_) / 1000;
   if (elapsed_sec < metrics::kMinRunTimeInSeconds) {
@@ skipping 41 matching lines @@
   first_packet_since_reset_ = true;
   rtt_ms_ = kDefaultRtt;
   last_decoded_state_.Reset();
 }
 
 void VCMJitterBuffer::Stop() {
   crit_sect_->Enter();
   UpdateHistograms();
   running_ = false;
   last_decoded_state_.Reset();
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled")
+    nack_module_->Stop();
 
   // Make sure all frames are free and reset.
   for (FrameList::iterator it = decodable_frames_.begin();
        it != decodable_frames_.end(); ++it) {
     free_frames_.push_back(it->second);
   }
   for (FrameList::iterator it = incomplete_frames_.begin();
        it != incomplete_frames_.end(); ++it) {
     free_frames_.push_back(it->second);
   }
   for (UnorderedFrameList::iterator it = free_frames_.begin();
        it != free_frames_.end(); ++it) {
     (*it)->Reset();
   }
   decodable_frames_.clear();
   incomplete_frames_.clear();
   crit_sect_->Leave();
   // Make sure we wake up any threads waiting on these events.
   frame_event_->Set();
 }
 
 bool VCMJitterBuffer::Running() const {
   CriticalSectionScoped cs(crit_sect_);
   return running_;
 }
 
 void VCMJitterBuffer::Flush() {

[stefan-webrtc, 2016/03/10 14:20:40]

Notify the nack module about the fact that we no longer are interested in any
missing packets?

[philipel, 2016/03/10 16:39:28]

Done.

   CriticalSectionScoped cs(crit_sect_);
   decodable_frames_.Reset(&free_frames_);
   incomplete_frames_.Reset(&free_frames_);
   last_decoded_state_.Reset();  // TODO(mikhal): sync reset.
   num_consecutive_old_packets_ = 0;
   // Also reset the jitter and delay estimates
   jitter_estimate_.Reset();
   inter_frame_delay_.Reset(clock_->TimeInMilliseconds());
   waiting_for_completion_.frame_size = 0;
   waiting_for_completion_.timestamp = 0;
@@ skipping 229 matching lines @@
   // decoder. Propagates the missing_frame bit.
   frame->PrepareForDecode(continuous);
 
   // We have a frame - update the last decoded state and nack list.
   last_decoded_state_.SetState(frame);
   DropPacketsFromNackList(last_decoded_state_.sequence_num());
 
   if ((*frame).IsSessionComplete())
     UpdateAveragePacketsPerFrame(frame->NumPackets());
 
   return frame;

[stefan-webrtc, 2016/03/10 14:20:40]

If we extract a frame for decoding I think we should notify the nack module so
that it stops nacking packets we no longer need. Or maybe we should do that as
we move frames into decodable_frames_ at lines 811-812?

[philipel, 2016/03/10 16:39:28]

Better to do it here since this is the actual point when we know for sure that
we aren't interested in older packets.

 }
 
 // Release frame when done with decoding. Should never be used to release
 // frames from within the jitter buffer.
 void VCMJitterBuffer::ReleaseFrame(VCMEncodedFrame* frame) {
   CriticalSectionScoped cs(crit_sect_);
   VCMFrameBuffer* frame_buffer = static_cast<VCMFrameBuffer*>(frame);
   if (frame_buffer) {
     free_frames_.push_back(frame_buffer);
   }
@@ skipping 39 matching lines @@
   const VCMFrameBuffer* frame_buffer =
       static_cast<const VCMFrameBuffer*>(frame);
   *retransmitted = (frame_buffer->GetNackCount() > 0);
   return frame_buffer->LatestPacketTimeMs();
 }
 
 VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(const VCMPacket& packet,
                                                  bool* retransmitted) {
   CriticalSectionScoped cs(crit_sect_);
 
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled")
+    nack_module_->OnReceivedPacket(packet);
+
   ++num_packets_;
   if (num_packets_ == 1) {
     time_first_packet_ms_ = clock_->TimeInMilliseconds();
   }
   // Does this packet belong to an old frame?
   if (last_decoded_state_.IsOldPacket(&packet)) {
     // Account only for media packets.
     if (packet.sizeBytes > 0) {
       num_discarded_packets_++;
       num_consecutive_old_packets_++;
@@ skipping 242 matching lines @@
     // when waiting for retransmissions.
     rtt_mult = 0.0f;
   }
   return jitter_estimate_.GetJitterEstimate(rtt_mult);
 }
 
 void VCMJitterBuffer::UpdateRtt(int64_t rtt_ms) {
   CriticalSectionScoped cs(crit_sect_);
   rtt_ms_ = rtt_ms;
   jitter_estimate_.UpdateRtt(rtt_ms);
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled")
+    nack_module_->UpdateRtt(rtt_ms);
 }
 
 void VCMJitterBuffer::SetNackMode(VCMNackMode mode,
                                   int64_t low_rtt_nack_threshold_ms,
                                   int64_t high_rtt_nack_threshold_ms) {
   CriticalSectionScoped cs(crit_sect_);
   nack_mode_ = mode;
   if (mode == kNoNack) {
     missing_sequence_numbers_.clear();
   }
@@ skipping 95 matching lines @@
       if (rit == incomplete_frames_.rend()) {
         // Request a key frame if we don't have one already.
         *request_key_frame = true;
         return std::vector<uint16_t>();
       } else {
         // Skip to the last key frame. If it's incomplete we will start
         // NACKing it.
         // Note that the estimated low sequence number is correct for VP8
         // streams because only the first packet of a key frame is marked.
         last_decoded_state_.Reset();
         DropPacketsFromNackList(EstimatedLowSequenceNumber(*rit->second));

[stefan-webrtc, 2016/03/10 14:20:40]

If we should run this code despite using the nack module, should we then notify
the nack module here?

[philipel, 2016/03/10 16:39:27]

No, this is logic related to the old nack behavior, the nack module has its own
behavior.

       }
     }
   }

[stefan-webrtc, 2016/03/10 14:20:40]

Should we really run lines 1017-1070 if using the nack module?

[philipel, 2016/03/10 16:39:27]

Yes, the GetNackList interface does more than just return the nack list, it also
is a way for the jitter buffer to signal that it wants to request a keyframe.

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

Ok, comment on why we don't return earlier to avoid future problems. And maybe
add a TODO to remove the GetNackList() method entirely. Btw, where is the code
above going to move when that is done?

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

Expanded my comment.

This code won't be necessary as the jitter buffer will have a
KeyframeRequestSender available, so whenever the jitter buffer wants to request
a keyframe according to some rule, then it can simply request the keyframe
immediately and not having to set a variable that will be picked up later by
some other modules Process function.

+  // The experiment is running, the nack module will send Nacks.
+  if (field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled")
+    return std::vector<uint16_t>();
+
   std::vector<uint16_t> nack_list(missing_sequence_numbers_.begin(),
                                   missing_sequence_numbers_.end());
   return nack_list;
 }
 
 void VCMJitterBuffer::SetDecodeErrorMode(VCMDecodeErrorMode error_mode) {
   CriticalSectionScoped cs(crit_sect_);
   decode_error_mode_ = error_mode;
 }
 
@@ skipping 278 matching lines @@
   }
   // Evaluate if the RTT is higher than |high_rtt_nack_threshold_ms_|, and in
   // that case we don't wait for retransmissions.
   if (high_rtt_nack_threshold_ms_ >= 0 &&
       rtt_ms_ >= high_rtt_nack_threshold_ms_) {
     return false;
   }
   return true;
 }
 }  // namespace webrtc